-
Notifications
You must be signed in to change notification settings - Fork 108
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
Set Chain ID to 11155111 #1042
Set Chain ID to 11155111 #1042
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1042 +/- ##
=======================================
Coverage 81.27% 81.27%
=======================================
Files 54 54
Lines 2238 2238
Branches 71 71
=======================================
Hits 1819 1819
Misses 402 402
Partials 17 17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -130,7 +130,7 @@ impl Verifier for MockVerifier { | |||
const GATEWAY_ADDRESS: [u8; 20] = hex!["eda338e4dc46038493b885327842fd3e301cab39"]; | |||
|
|||
parameter_types! { | |||
pub const EthereumNetwork: xcm::v3::NetworkId = xcm::v3::NetworkId::Ethereum { chain_id: 15 }; | |||
pub const EthereumNetwork: xcm::v3::NetworkId = xcm::v3::NetworkId::Ethereum { chain_id: 5 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ok to hardcode the chain ID in tests? Do we want to use the constant here? I am not sure about adding the rococo-common
crate here as a dependency just to use the chain ID.
@@ -6,11 +6,11 @@ use sp_core::crypto::Ss58Codec; | |||
use xcm::v3::prelude::*; | |||
use xcm_executor::traits::ConvertLocation; | |||
|
|||
const NETWORK: NetworkId = Ethereum { chain_id: 15 }; | |||
const NETWORK: NetworkId = Ethereum { chain_id: 5 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -1,6 +1,6 @@ | |||
{ | |||
"config": { | |||
"chainId": 15, | |||
"chainId": 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can config the chain ID too but not sure it's worth the effort because it's unlikely to change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, approved. Just rename the PR title though please.
--params.DEPOSIT_CHAIN_ID 11155111 \ | ||
--params.DEPOSIT_NETWORK_ID 11155111 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious about what are these two parameters for or required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are the only Lodestar config I could find related to the chain ID. I will double check if they are required. 😄
Resolves: SNO-773
Polkadot-sdk companion: Snowfork/polkadot-sdk#59