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

Init agents and channels with migration #1063

Merged
merged 11 commits into from
Dec 18, 2023

Conversation

alistair-singh
Copy link
Contributor

@alistair-singh alistair-singh commented Dec 17, 2023

Polkadot-sdk: Snowfork/polkadot-sdk#70

Adds a migration script that will initialize the agents and channels if not initialized by the genesis build.
Adds a new extrinsic which can be used to force initialization.

Tested the extrinsic manually and via unit tests. Tested the migration by doing a manual runtime upgrade and by using the try-runtime tool from Parity.

$ try-runtime  --runtime  gen.raw.wasm  on-runtime-upgrade  live --uri wss://rococo-bridge-hub-rpc.dwellir.com:443 2>&1 | grep ethereum_system
[2023-12-18T15:55:55Z INFO  ethereum_system::migration] Agents and channels not initialized. Initialization will run.
[2023-12-18T15:55:55Z INFO  ethereum_system::migration] Ethereum system initialized.
[2023-12-18T15:55:56Z INFO  ethereum_system::migration] Ethereum system initialized.
[2023-12-18T15:55:56Z INFO  ethereum_system::migration] Agents and channels are initialized. Initialization will not run.
[2023-12-18T15:55:56Z INFO  ethereum_system::migration] Ethereum system already initialized. Skipping.

Copy link

codecov bot commented Dec 17, 2023

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (3b3e9c2) 81.31% compared to head (0a0263d) 80.60%.
Report is 1 commits behind head on main.

Files Patch % Lines
parachain/pallets/system/src/migration.rs 0.00% 21 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
- Coverage   81.31%   80.60%   -0.72%     
==========================================
  Files          54       55       +1     
  Lines        2243     2269      +26     
  Branches       71       71              
==========================================
+ Hits         1824     1829       +5     
- Misses        402      423      +21     
  Partials       17       17              
Flag Coverage Δ
rust 80.60% <48.78%> (-0.87%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 257 to +260
#[pallet::genesis_build]
impl<T: Config> BuildGenesisConfig for GenesisConfig<T> {
fn build(&self) {
let bridge_hub_agent_id =
agent_id_of::<T>(&MultiLocation::here()).expect("infallible; qed");
// Agent for BridgeHub
Agents::<T>::insert(bridge_hub_agent_id, ());

// Primary governance channel
Channels::<T>::insert(
PRIMARY_GOVERNANCE_CHANNEL,
Channel { agent_id: bridge_hub_agent_id, para_id: self.para_id },
);

// Secondary governance channel
Channels::<T>::insert(
SECONDARY_GOVERNANCE_CHANNEL,
Channel { agent_id: bridge_hub_agent_id, para_id: self.para_id },
);

// Asset Hub
let asset_hub_location: MultiLocation =
ParentThen(X1(Parachain(self.asset_hub_para_id.into()))).into();
let asset_hub_agent_id =
agent_id_of::<T>(&asset_hub_location).expect("infallible; qed");
let asset_hub_channel_id: ChannelId = self.asset_hub_para_id.into();
Agents::<T>::insert(asset_hub_agent_id, ());
Channels::<T>::insert(
asset_hub_channel_id,
Channel { agent_id: asset_hub_agent_id, para_id: self.asset_hub_para_id },
);
Pallet::<T>::initialize(self.para_id, self.asset_hub_para_id).expect("infallible; qed");
Copy link
Contributor

@yrong yrong Dec 17, 2023

Choose a reason for hiding this comment

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

Since the initialize call was introduced, maybe we can just totally remove the GenesisConfig?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the initialize call.

#[pallet::weight((T::WeightInfo::force_initialize(), DispatchClass::Operational))]
pub fn force_initialize(
origin: OriginFor<T>,
own_para_id: ParaId,
Copy link
Contributor

Choose a reason for hiding this comment

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

own_para_id seems unnecessary which can be retrieved from runtime.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Disagree, actually, the runtime is configured with the para_id using the genesis config for the ParachainInfo pallet.

Copy link
Collaborator

@vgeddes vgeddes left a comment

Choose a reason for hiding this comment

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

As per offline discussions, lets deliver this is as migration script, and remove the governance extrinsic.

@alistair-singh
Copy link
Contributor Author

As per offline discussions, lets deliver this is as migration script, and remove the governance extrinsic.

@vgeddes @yrong I have removed the extrinsic and this is ready for final review.

@vgeddes
Copy link
Collaborator

vgeddes commented Dec 18, 2023

We also need to initialize the storage for the runtime tests,

for example in

pub fn send_transfer_token_message<Runtime, XcmConfig>(

To ensure that channels have been created before sending tokens back to Ethereum

@alistair-singh alistair-singh merged commit dfb0150 into main Dec 18, 2023
7 checks passed
@alistair-singh alistair-singh deleted the alistair/init-agents-with-migration branch December 18, 2023 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants