Skip to content
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

feat(sequencer-relayer)!: add chain IDs for sequencer and Celestia to config env vars #1063

Merged
merged 9 commits into from
Jun 6, 2024

Conversation

Fraser999
Copy link
Contributor

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.

@Fraser999 Fraser999 requested review from a team as code owners May 9, 2024 19:01
@github-actions github-actions bot added sequencer-relayer pertaining to the astria-sequencer-relayer crate cd labels May 9, 2024
{
trace!(?response);
}
response.map(|status_response| status_response.node_info.network.to_string())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fact that this var is called network and not chain_id as in other rpc responses (such as genesis and block) bothers me a bit :p I think it's fine to leave though, as it should be the same as the chain id

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - it doesn't sit well with me either, but it's part of the tendermint API, so my hands are pretty much tied on this one I think.

Copy link
Member

@SuperFluffy SuperFluffy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only minor comments. I tend to prefer received over actual nowadays because it requires less thinking. :)

@Fraser999 Fraser999 enabled auto-merge May 16, 2024 16:11
Copy link
Member

@joroshiba joroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

infra approval

@Fraser999 Fraser999 added this pull request to the merge queue Jun 5, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 5, 2024
@Fraser999 Fraser999 added this pull request to the merge queue Jun 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 6, 2024
@Fraser999 Fraser999 added this pull request to the merge queue Jun 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 6, 2024
@Fraser999 Fraser999 enabled auto-merge June 6, 2024 17:35
@Fraser999 Fraser999 added this pull request to the merge queue Jun 6, 2024
Merged via the queue into main with commit e3595cb Jun 6, 2024
38 checks passed
@Fraser999 Fraser999 deleted the fraser/add-chain-ids-to-config branch June 6, 2024 17:46
steezeburger added a commit that referenced this pull request Jun 10, 2024
* main:
  fix: ignore RUSTSEC-2021-0139 (#1171)
  chore(sequencer-relayer)!: remove functionality to restrict relaying blocks to only those proposed by a given validator (#1168)
  chore(metrics): update `metric_name` macro to handle a collection of names (#1163)
  fix(bridge-withdrawer): skip linting generated contract code (#1172)
  fix(core, sequencer): prefix removal source non-refund ics20 packet (#1162)
  chore(docs): add sequencer-relayer doc to specs (#1126)
  feat(bridge-withdrawer): sync logic (#1165)
  chore(withdrawer): replace contracts with `astria-bridge-contracts` submodule (#1164)
  feat(sequencer)!: implement bridge sudo and withdrawer addresses (#1142)
  feat(sequencer): implement refund to rollup logic upon ics20 transfer refund (#1161)
  feat(bridge-withdrawer): bridge withdrawer startup (#1160)
  feat(core, proto)!: add bech32m addresses (#1124)
  feat(withdrawer): bridged ERC20 token withdrawals (#1149)
  feat(sequencer-relayer)!: add chain IDs for sequencer and Celestia to config env vars (#1063)
  test(bridge-withdrawer): add submitter tests (#1133)
  chore: bump penumbra deps (#1159)
  feat(sequencer): implement `bridge/account_last_tx_hash` abci query (#1158)
  fix(withdrawer): use block subscription in batcher; send to destination_chain_address (#1157)
  fix(withdrawer): update AstriaWithdrawer to check that withdrawal value is sufficient (#1148)
  chore(ci): build bridge withdrawer images (#1156)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cd sequencer-relayer pertaining to the astria-sequencer-relayer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add network ID of DA to env vars of sequencer-relayer
4 participants