test: basic E2E MsgPayForBlobs test #4778
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…require usage more consistent
chatton
requested review from
yarikbratashchuk and
cmwaters
and removed request for
a team
rootulp
approved these changes
May 8, 2025
.github/workflows/test-e2e.yml
Outdated
| @@ -17,6 +17,7 @@ jobs: | |||
| testcase: | |||
| - TestE2ESimple | |||
| - TestCelestiaChainStateSync | |||
| - TestE2ENodeSimple | |||
There was a problem hiding this comment.
[uber nit][optional] Simple makes me conflate this with TestE2ESimple. To disambiguate, consider renaming to TestE2EBridgeNode
There was a problem hiding this comment.
Also I often find "node" not specific enough to differentiate between consensus node or DA node so I have been trying to clarify via: "consensus node" vs. "bridge node" / "light node"
There was a problem hiding this comment.
yes good call, will make this change.
test/docker-e2e/e2e_test.go
Outdated
| return maps.SetField(bytes, "consensus.params.version.app", appVersion) | ||
| }, | ||
| EncodingConfig: &enc, | ||
| AdditionalStartArgs: []string{"--force-no-bbr", "--grpc.enable", "--grpc.address", "0.0.0.0:9090", "--rpc.grpc_laddr=tcp://0.0.0.0:9099"}, |
There was a problem hiding this comment.
[nit] does the RCP GRPC listen address need to differ from the default 9098?
celestia-app/app/default_overrides.go
Line 272 in 6a2e751
Suggested change
| AdditionalStartArgs: []string{"--force-no-bbr", "--grpc.enable", "--grpc.address", "0.0.0.0:9090", "--rpc.grpc_laddr=tcp://0.0.0.0:9099"}, | |
| AdditionalStartArgs: []string{"--force-no-bbr", "--grpc.enable", "--grpc.address", "0.0.0.0:9090", "--rpc.grpc_laddr=tcp://0.0.0.0:9098"}, |
There was a problem hiding this comment.
it shouldn't actually matter which port is used here, but no harm being consistent with the default.
evan-forbes
reviewed
May 8, 2025
evan-forbes
previously approved these changes
May 9, 2025
rootulp
previously approved these changes
May 12, 2025
2 tasks
|
converting to draft, will bump to the new celestiaorg repo and re-open |
chatton
dismissed stale reviews from rootulp and evan-forbes
The base branch was changed.
chatton
added a commit
that referenced
this pull request
May 15, 2025
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> ## Overview closes #4623 (based on #4636 by @tac0turtle ) ~NOTE: this PR uses my personal fork of interchaintest. If people like this approach of testing, we can further clean up the fork and move it into the celestiaorg and possibly rename.~ This uses a new repo that leverages a lot of the logic that was in interchaintest to spin up docker nodes, but tailored to celestia. This PR adds the `E2ESimple` and `TestCelestiaChainStateSync` tests. Everything is also in a separate go mod to avoid introducing test dependencies into the main app go mod. In order to run the sync test locally, you can run the following commands. ```sh cd test/docker-e2e make test-e2e test=TestCelestiaChainStateSync ``` There is a follow up PR which bumps the test library to add support for bridge nodes #4778 <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. -->
chatton
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR introduces a new E2E test which deploys a celestia-app, txsim and a bridge node in docker and ensures that the bridge node syncs headers.This is still using my personal repo here which can be moved into the celestiaorg in the before or after merging this PR.A nice follow up to this test would be to do something which uses theBlockApito ensure we don't accidentally break compatibility.Little bit of scope creep on this PR, but it does the following.
This should ensure the core components are all functioning together. In a follow up, I will update the CI to pass a celestia-app image built on the PR itself to this test.