Skip to content

Accept EVM ChainID as parameter through Domain Instantiation #3681

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 19, 2025
Merged

Accept EVM ChainID as parameter through Domain Instantiation #3681

merged 3 commits into from
Aug 19, 2025

Conversation

vedhavyas
Copy link
Contributor

This PR brings following changes

  • Accept EVM chainID as a parameter of Runtime Info at the time of Domain instantiation
  • Removes existing NextEvmChainId storage used to allocate evm chainID for new EVM domains.
  • Migration for existing EVM chains to store already allocated EVM chain IDs into storage to avoid re-allocation.
  • Bumped TransactionVersion since instantiate_domain parameter type has changed

spec_version will be bumped before runtime release

Code contributor checklist:

@vedhavyas vedhavyas requested a review from nazar-pc as a code owner August 19, 2025 04:53
@immunefi-magnus Immunefi Magnus
Copy link

immunefi-magnus bot commented Aug 19, 2025 •
edited
Loading

🛡️ Immunefi PR Reviews

We’ve assigned 6 code reviewer(s) to this PR.

They’ll begin the review shortly and leave feedback directly in the pull request.

This review is based on the current state of your pull request. If you make changes after the review starts, they won’t be reflected here. To ensure the review includes your latest updates, you’ll need to open a new pull request.

@vedhavyas vedhavyas requested a review from teor2345 August 19, 2025 04:53
@vedhavyas vedhavyas enabled auto-merge August 19, 2025 04:56
@vedhavyas vedhavyas added execution Subspace execution audit-P3 Low audit priority labels Aug 19, 2025
teor2345
teor2345 reviewed Aug 19, 2025
View reviewed changes
Copy link
Member

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but we might want more test coverage.

All my other comments aren't blockers.

@vedhavyas vedhavyas requested a review from teor2345 August 19, 2025 09:24
@vedhavyas vedhavyas added this pull request to the merge queue Aug 19, 2025
Merged via the queue into main with commit a15c11a Aug 19, 2025
13 checks passed
@vedhavyas vedhavyas deleted the eth-chain-id-params branch August 19, 2025 21:47
immunefi-magnus[bot]
immunefi-magnus bot reviewed Aug 20, 2025
View reviewed changes
Copy link

@immunefi-magnus immunefi-magnus bot left a comment

Choose a reason for hiding this comment

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

Check if insertion function and migration works intended.
Code looks good.

  • OxStrausses

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teor2345 teor2345 teor2345 approved these changes

@nazar-pc nazar-pc Awaiting requested review from nazar-pc nazar-pc is a code owner

+1 more reviewer

@immunefi-magnus immunefi-magnus[bot] immunefi-magnus[bot] left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
audit-P3 Low audit priority execution Subspace execution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vedhavyas @teor2345