Skip to content

Commit

Permalink
fix: GoAhead signal from schedule_code_upgrade
Browse files Browse the repository at this point in the history
  • Loading branch information
Daanvdplas committed Aug 25, 2023
1 parent 1a38d6d commit 0ecf484
Show file tree
Hide file tree
Showing 4 changed files with 237 additions and 29 deletions.
6 changes: 4 additions & 2 deletions polkadot/runtime/parachains/src/inclusion/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,9 @@ impl fmt::Debug for UmpAcceptanceCheckErr {
"the ump queue would have grown past the max size permitted by config ({} > {})",
total_size, limit,
),
UmpAcceptanceCheckErr::IsOffboarding =>
write!(fmt, "upward message rejected because the para is off-boarding",),
UmpAcceptanceCheckErr::IsOffboarding => {
write!(fmt, "upward message rejected because the para is off-boarding",)
},
}
}
}
Expand Down Expand Up @@ -911,6 +912,7 @@ impl<T: Config> Pallet<T> {
new_code,
now,
&config,
true,
));
}

Expand Down
10 changes: 8 additions & 2 deletions polkadot/runtime/parachains/src/inclusion/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,13 @@ fn candidate_checks() {
let cfg = Configuration::config();
let expected_at = 10 + cfg.validation_upgrade_delay;
assert_eq!(expected_at, 12);
Paras::schedule_code_upgrade(chain_a, vec![1, 2, 3, 4].into(), expected_at, &cfg);
Paras::schedule_code_upgrade(
chain_a,
vec![1, 2, 3, 4].into(),
expected_at,
&cfg,
true,
);
}

assert_noop!(
Expand Down Expand Up @@ -2277,7 +2283,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 }
paras::PvfCheckCause::Upgrade { id, included_at, set_go_ahead: true }
if id == &chain_a && included_at == &7
);
});
Expand Down
40 changes: 28 additions & 12 deletions polkadot/runtime/parachains/src/paras/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,8 @@ pub(crate) enum PvfCheckCause<BlockNumber> {
///
/// See https://github.com/paritytech/polkadot/issues/4601 for detailed explanation.
included_at: BlockNumber,
/// Whether or not the given para should be sent the `GoAhead` signal.
set_go_ahead: bool,
},
}

Expand Down Expand Up @@ -723,7 +725,9 @@ pub mod pallet {
StorageMap<_, Twox64Concat, ParaId, ValidationCodeHash>;

/// This is used by the relay-chain to communicate to a parachain a go-ahead with in the upgrade
/// procedure.
/// procedure. The `GoAhead` is only set when it is assured the parachain is ready for the
/// upgrade, i.e. when the upgrade is enacted on the parachain side
/// (`enact_authorized_upgrade`).
///
/// This value is absent when there are no upgrades scheduled or during the time the relay chain
/// performs the checks. It is set at the first relay-chain block when the corresponding
Expand Down Expand Up @@ -869,7 +873,7 @@ pub mod pallet {
) -> DispatchResult {
ensure_root(origin)?;
let config = configuration::Pallet::<T>::config();
Self::schedule_code_upgrade(para, new_code, relay_parent_number, &config);
Self::schedule_code_upgrade(para, new_code, relay_parent_number, &config, false);
Self::deposit_event(Event::CodeUpgradeScheduled(para));
Ok(())
}
Expand Down Expand Up @@ -1174,7 +1178,7 @@ impl<T: Config> Pallet<T> {
let current_block = frame_system::Pallet::<T>::block_number();
// Schedule the upgrade with a delay just like if a parachain triggered the upgrade.
let upgrade_block = current_block.saturating_add(config.validation_upgrade_delay);
Self::schedule_code_upgrade(id, new_code, upgrade_block, &config);
Self::schedule_code_upgrade(id, new_code, upgrade_block, &config, false);
Self::deposit_event(Event::CodeUpgradeScheduled(id));
Ok(())
}
Expand Down Expand Up @@ -1515,8 +1519,15 @@ impl<T: Config> Pallet<T> {
PvfCheckCause::Onboarding(id) => {
weight += Self::proceed_with_onboarding(*id, sessions_observed);
},
PvfCheckCause::Upgrade { id, included_at } => {
weight += Self::proceed_with_upgrade(*id, code_hash, now, *included_at, cfg);
PvfCheckCause::Upgrade { id, included_at, set_go_ahead } => {
weight += Self::proceed_with_upgrade(
*id,
code_hash,
now,
*included_at,
cfg,
*set_go_ahead,
);
},
}
}
Expand Down Expand Up @@ -1549,6 +1560,7 @@ impl<T: Config> Pallet<T> {
now: BlockNumberFor<T>,
relay_parent_number: BlockNumberFor<T>,
cfg: &configuration::HostConfiguration<BlockNumberFor<T>>,
set_go_ahead: bool,
) -> Weight {
let mut weight = Weight::zero();

Expand All @@ -1572,12 +1584,15 @@ impl<T: Config> Pallet<T> {
weight += T::DbWeight::get().reads_writes(1, 4);
FutureCodeUpgrades::<T>::insert(&id, expected_at);

UpcomingUpgrades::<T>::mutate(|upcoming_upgrades| {
let insert_idx = upcoming_upgrades
.binary_search_by_key(&expected_at, |&(_, b)| b)
.unwrap_or_else(|idx| idx);
upcoming_upgrades.insert(insert_idx, (id, expected_at));
});
// Only set an upcoming upgrade if `GoAhead` signal should be set for the respective para.
if set_go_ahead {
UpcomingUpgrades::<T>::mutate(|upcoming_upgrades| {
let insert_idx = upcoming_upgrades
.binary_search_by_key(&expected_at, |&(_, b)| b)
.unwrap_or_else(|idx| idx);
upcoming_upgrades.insert(insert_idx, (id, expected_at));
});
}

let expected_at = expected_at.saturated_into();
let log = ConsensusLog::ParaScheduleUpgradeCode(id, *code_hash, expected_at);
Expand Down Expand Up @@ -1816,6 +1831,7 @@ impl<T: Config> Pallet<T> {
new_code: ValidationCode,
inclusion_block_number: BlockNumberFor<T>,
cfg: &configuration::HostConfiguration<BlockNumberFor<T>>,
set_go_ahead: bool,
) -> Weight {
let mut weight = T::DbWeight::get().reads(1);

Expand Down Expand Up @@ -1865,7 +1881,7 @@ impl<T: Config> Pallet<T> {
});

weight += Self::kick_off_pvf_check(
PvfCheckCause::Upgrade { id, included_at: inclusion_block_number },
PvfCheckCause::Upgrade { id, included_at: inclusion_block_number, set_go_ahead },
code_hash,
new_code,
cfg,
Expand Down
Loading

0 comments on commit 0ecf484

Please sign in to comment.