Skip to content

Commit

Permalink
Fix parachain upgrade scheduling when done by the owner/root (#3341)
Browse files Browse the repository at this point in the history
When using `schedule_code_upgrade` to change the code of a parachain in
the relay chain runtime, we had already fixed to not set the `GoAhead`
signal. This was done to not brick any parachain after the upgrade,
because they were seeing the signal without having any upgrade prepared.
The remaining problem is that the parachain code is only upgraded after
a parachain header was enacted, aka the parachain made some progress.
However, this is quite complicated if the parachain is bricked (which is
the most common scenario why to manually schedule a code upgrade). Thus,
this pull request replaces `SetGoAhead` with `UpgradeStrategy` to signal
to the logic kind of strategy want to use. The strategies are either
`SetGoAheadSignal` or `ApplyAtExpectedBlock`. `SetGoAheadSignal` sets
the go ahead signal as before and awaits a parachain block.
`ApplyAtExpectedBlock` schedules the upgrade and applies it directly at
the `expected_block` without waiting for the parachain to make any kind
of progress.
  • Loading branch information
bkchr authored Apr 2, 2024
1 parent d0ebb85 commit 12eb285
Show file tree
Hide file tree
Showing 9 changed files with 237 additions and 169 deletions.
15 changes: 13 additions & 2 deletions polkadot/runtime/common/src/paras_registrar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use frame_system::{self, ensure_root, ensure_signed};
use primitives::{HeadData, Id as ParaId, ValidationCode, LOWEST_PUBLIC_ID, MIN_CODE_SIZE};
use runtime_parachains::{
configuration, ensure_parachain,
paras::{self, ParaGenesisArgs, SetGoAhead},
paras::{self, ParaGenesisArgs, UpgradeStrategy},
Origin, ParaLifecycle,
};
use sp_std::{prelude::*, result};
Expand Down Expand Up @@ -408,6 +408,13 @@ pub mod pallet {

/// Schedule a parachain upgrade.
///
/// This will kick off a check of `new_code` by all validators. After the majority of the
/// validators have reported on the validity of the code, the code will either be enacted
/// or the upgrade will be rejected. If the code will be enacted, the current code of the
/// parachain will be overwritten directly. This means that any PoV will be checked by this
/// new code. The parachain itself will not be informed explictely that the validation code
/// has changed.
///
/// Can be called by Root, the parachain, or the parachain manager if the parachain is
/// unlocked.
#[pallet::call_index(7)]
Expand All @@ -418,7 +425,11 @@ pub mod pallet {
new_code: ValidationCode,
) -> DispatchResult {
Self::ensure_root_para_or_owner(origin, para)?;
runtime_parachains::schedule_code_upgrade::<T>(para, new_code, SetGoAhead::No)?;
runtime_parachains::schedule_code_upgrade::<T>(
para,
new_code,
UpgradeStrategy::ApplyAtExpectedBlock,
)?;
Ok(())
}

Expand Down
4 changes: 2 additions & 2 deletions polkadot/runtime/parachains/src/inclusion/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use crate::{
configuration::{self, HostConfiguration},
disputes, dmp, hrmp,
paras::{self, SetGoAhead},
paras::{self, UpgradeStrategy},
scheduler,
shared::{self, AllowedRelayParentsTracker},
util::make_persisted_validation_data_with_parent,
Expand Down Expand Up @@ -839,7 +839,7 @@ impl<T: Config> Pallet<T> {
new_code,
now,
&config,
SetGoAhead::Yes,
UpgradeStrategy::SetGoAheadSignal,
));
}

Expand Down
4 changes: 2 additions & 2 deletions polkadot/runtime/parachains/src/inclusion/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1590,7 +1590,7 @@ fn candidate_checks() {
vec![9, 8, 7, 6, 5, 4, 3, 2, 1].into(),
expected_at,
&cfg,
SetGoAhead::Yes,
UpgradeStrategy::SetGoAheadSignal,
);
}

Expand Down Expand Up @@ -2857,7 +2857,7 @@ fn para_upgrade_delay_scheduled_from_inclusion() {
let cause = &active_vote_state.causes()[0];
// Upgrade block is the block of inclusion, not candidate's parent.
assert_matches!(cause,
paras::PvfCheckCause::Upgrade { id, included_at, set_go_ahead: SetGoAhead::Yes }
paras::PvfCheckCause::Upgrade { id, included_at, upgrade_strategy: UpgradeStrategy::SetGoAheadSignal }
if id == &chain_a && included_at == &7
);
});
Expand Down
4 changes: 2 additions & 2 deletions polkadot/runtime/parachains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ mod mock;
mod ump_tests;

pub use origin::{ensure_parachain, Origin};
pub use paras::{ParaLifecycle, SetGoAhead};
pub use paras::{ParaLifecycle, UpgradeStrategy};
use primitives::{HeadData, Id as ParaId, ValidationCode};
use sp_runtime::{DispatchResult, FixedU128};

Expand Down Expand Up @@ -104,7 +104,7 @@ pub fn schedule_parachain_downgrade<T: paras::Config>(id: ParaId) -> Result<(),
pub fn schedule_code_upgrade<T: paras::Config>(
id: ParaId,
new_code: ValidationCode,
set_go_ahead: SetGoAhead,
set_go_ahead: UpgradeStrategy,
) -> DispatchResult {
paras::Pallet::<T>::schedule_code_upgrade_external(id, new_code, set_go_ahead)
}
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/parachains/src/paras/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ benchmarks! {
ValidationCode(vec![0]),
expired,
&config,
SetGoAhead::Yes,
UpgradeStrategy::SetGoAheadSignal,
);
}: _(RawOrigin::Root, para_id, new_head)
verify {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ where
validation_code,
/* relay_parent_number */ 1u32.into(),
&configuration::Pallet::<T>::config(),
SetGoAhead::Yes,
UpgradeStrategy::SetGoAheadSignal,
);
} else {
let r = Pallet::<T>::schedule_para_initialize(
Expand Down
Loading

0 comments on commit 12eb285

Please sign in to comment.