-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
validate communicating with correct sequencer chain #986
Comments
github-merge-queue bot
pushed a commit
that referenced
this issue
Jun 6, 2024
… config env vars (#1063) ## Summary Added two new configuration options specifying the expected chain IDs of the sequencer and Celestia networks. ## Background This is a guard rail to help avoid an operator accidentally connecting the relayer to the wrong network. ## Changes The configured values are used in the initialization phase, where the relevant clients query the appropriate servers for their chain IDs. The Celestia client was already retrieving this info prior to this PR. Now it returns an error if the chain ID doesn't match, and this particular error variant causes the endless retry loop (and ultimately the process) to exit. The sequencer client was not previously requesting this info, so it now does this in a somewhat similar manner to the Celestia client. Likewise, a chain ID mismatch there causes the process to exit. ## Testing A blackbox test for a mismatch in each of the chain IDs was added. ## Breaking Changelist - added `ASTRIA_SEQUENCER_RELAYER_SEQUENCER_CHAIN_ID` - added `ASTRIA_SEQUENCER_RELAYER_CELESTIA_CHAIN_ID` ## Related Issues Part of #986. Closes #978.
github-merge-queue bot
pushed a commit
that referenced
this issue
Jul 22, 2024
## Summary Added check to ensure the executor's configured chain_id and the sequencer client's actual chain_id match. ## Background This check ensures that the composer is communicating with the sequencer RPC on the correct network. ## Changes Added chain_id check to the composer executor builder. Made executor builder async to accommodate for async communication with sequencer client. Moved nonce guard declaration from mock startup to individual tests in executor tests. Added function to mount a status response with a given chain_id to the mock. This is so the executor builder can gather the status response without throwing an HTTP 404 error. ## Testing Added executor test to validate the failure of an executor build with a chain_id mismatch. ## Related Issues Part of #986 --------- Co-authored-by: Richard Janis Goldschmidt <github@aberrat.io> Co-authored-by: Ethan Oroshiba <ethan@astria.org>
bharath-123
pushed a commit
that referenced
this issue
Jul 25, 2024
## Summary Added check to ensure the executor's configured chain_id and the sequencer client's actual chain_id match. ## Background This check ensures that the composer is communicating with the sequencer RPC on the correct network. ## Changes Added chain_id check to the composer executor builder. Made executor builder async to accommodate for async communication with sequencer client. Moved nonce guard declaration from mock startup to individual tests in executor tests. Added function to mount a status response with a given chain_id to the mock. This is so the executor builder can gather the status response without throwing an HTTP 404 error. ## Testing Added executor test to validate the failure of an executor build with a chain_id mismatch. ## Related Issues Part of #986 --------- Co-authored-by: Richard Janis Goldschmidt <github@aberrat.io> Co-authored-by: Ethan Oroshiba <ethan@astria.org>
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 1, 2024
## Summary Implemented chain ID validation for both the sequencer and Celestia readers in Conductor. ## Background This is to safeguard against connecting to an incorrect network. This change was previously blocking on a switch to solely using gRPC for communication with sequencer, but after offline discussion it was decided to implement the chain ID check anyways using the HTTP client. ## Changes - Added sequencer and Celestia chain ID environment variables. - Added chain ID checks to the initialization of both the sequencer and Celestia readers. - Changed conductor `run_until_stopped()` to return with an error so that it will not fail silently. ## Testing Added 2 blackbox tests to ensure the conductor exits with the proper error in the event of both a Celestia and sequencer chain ID mismatch. ## Breaking Changelist - Added `ASTRIA_CONDUCTOR_SEQUENCER_CHAIN_ID` and `ASTRIA_CONDUCTOR_CELESTIA_CHAIN_ID` environment variables, without which conductor will not function. ## Related Issues closes #986 --------- Co-authored-by: Richard Janis Goldschmidt <github@aberrat.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We should add a check to conductor, composer, and sequencer relayer to validate that the sequencer RPC they communicate with is of the expected Astria sequencer network.
It is possible that RPCs change, or are confusingly named. On startup the services should validate this. There should be an env var specifying the expected chain id and then it can be verified from genesis file.
The text was updated successfully, but these errors were encountered: