Fix override encoding, verify spawned forks, carry runtime upgrades - #128
Conversation
| #[arg(long = "rc-upgrade", verbatim_doc_comment)] | ||
| relay_upgrade: Option<String>, | ||
| /// Same as --rc-upgrade but for a parachain, format: <para_id>=<wasm_path> | ||
| /// (can be repeated) |
There was a problem hiding this comment.
| /// (can be repeated) | |
| /// Can be set multiple times, onces per para to upgrde. |
| /// clamped between the two nodes the spawner always starts (alice, bob) and | ||
| /// the seven well-known dev accounts. Used by both the state overrides and the | ||
| /// spawner so the validator set in state always matches the nodes running. | ||
| pub fn num_validators_for_cores(req_cores: u32) -> u32 { |
There was a problem hiding this comment.
NOTE: At some point we should remove this dev validators constraint.
There was a problem hiding this comment.
Agreed — added a TODO. The cap is only there because we reuse the well-known dev accounts; generating keys would let a fork scale past 7.
| let mut seen_chains: Vec<&str> = vec![]; | ||
| for para in network_config.parachains() { | ||
| let chain = para.chain().map(|c| c.as_str()); | ||
| if para.chain_spec_path().is_some() && chain.is_none() { |
There was a problem hiding this comment.
can we read the id from the chain-spec and use that, since if we have the chain-spec we will not use the chain.
There was a problem hiding this comment.
Done. Identity is now chain when set, else the spec's id. Also checked zombienet 0.4.13: with no chain it falls back to unique_id (the para id), so a missing chain isn't fatal there — no reason to reject it.
| if let Some(chain) = chain { | ||
| if seen_chains.contains(&chain) { | ||
| bail!( | ||
| "parachain {} reuses chain name '{chain}'; each parachain needs a unique 'chain' or they will share one chain spec", |
There was a problem hiding this comment.
hm, I don't think we need to ensure that all chains are unique but is a nice to have 👍
There was a problem hiding this comment.
Kept it, now keyed on the resolved identity instead of the raw chain name.
| error!("--apply-upgrade can only be used with --and-spawn"); | ||
| std::process::exit(1); |
There was a problem hiding this comment.
I think we can replace it for bail macro here and in the rest.
| error!("--apply-upgrade can only be used with --and-spawn"); | |
| std::process::exit(1); | |
| bail!("--apply-upgrade can only be used with --and-spawn"); |
There was a problem hiding this comment.
Switched all three to bail!. Left the "bite dir doesn't exist" block as-is since it prints the colored usage help.
| /// `authorize_upgrade(hash)` referendum leaves behind. The permissionless | ||
| /// `apply_authorized_upgrade(blob)` can then enact the upgrade through the | ||
| /// production path, which needs no sudo (usable on Kusama/Polkadot forks). | ||
| async fn inject_authorized_upgrade(injects: &mut Value, upgrade_wasm: &str) { |
|
|
||
| /// Parachain upgrades wait for relay-side PVF pre-checking plus the go-ahead | ||
| /// signal before enacting. | ||
| const UPGRADE_TIMEOUT_SECS: u64 = 900; |
There was a problem hiding this comment.
Is this mesure for prod environments? I mean, the 900s
There was a problem hiding this comment.
no, not yet, going to test this with custom implementation now on PPN and will adjust if needed
…ble upgrade timeout
| if para.chain_spec_path().is_some() && chain.is_none() { | ||
| let identity = if let Some(chain) = para.chain() { | ||
| chain.as_str().to_string() | ||
| } else if let Some(AssetLocation::FilePath(path)) = para.chain_spec_path() { |
There was a problem hiding this comment.
We have support for file and url as assetLocation, I will rewrite this to support both 👍
2 participants
ValidatorGroupsencoding (inner compact lengths), drop no-opClaimQueue/AvailabilityCoresoverrides, share validator count between overrides and spawner.chain_spec_pathwithoutchain.--rc-upgrade/--para-upgradeseedSystem::AuthorizedUpgradeinstead of installing the runtime;--apply-upgradesubmitsapply_authorized_upgradeunsigned (no sudo needed) and waits for the spec version bump.Closes #121
Closes #122
Closes #127