Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Merge branch 'master' into ao-5535-followup
Browse files Browse the repository at this point in the history
* master:
  Log PVF retries (#6504)
  Issue 6274: keeping all backing votes in provisioner vote set (#6494)
  Fix `polkadot-runtime-constants` std build (#6503)
  Add `try-runtime` to Rococo runtime (#6501)
  Co #13045: Selectable on-runtime-upgrade checks (#6498)
  Migrate Staking pallet to v13 (#6365)
  Deploy scheduler agenda cleanup migration (#6465)
  BlockId removal: refactor: BlockBackend::block|block_status (#6477)
  • Loading branch information
ordian committed Jan 5, 2023
2 parents 6358cb1 + 52e8606 commit b2f82ca
Show file tree
Hide file tree
Showing 15 changed files with 346 additions and 199 deletions.
365 changes: 185 additions & 180 deletions Cargo.lock

Large diffs are not rendered by default.

13 changes: 8 additions & 5 deletions node/client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use sp_api::{CallApiAt, Encode, NumberFor, ProvideRuntimeApi};
use sp_blockchain::{HeaderBackend, HeaderMetadata};
use sp_consensus::BlockStatus;
use sp_runtime::{
generic::{BlockId, SignedBlock},
generic::SignedBlock,
traits::{BlakeTwo256, Block as BlockT},
Justifications,
};
Expand Down Expand Up @@ -338,22 +338,25 @@ impl sc_client_api::BlockBackend<Block> for Client {
}
}

fn block(&self, id: &BlockId<Block>) -> sp_blockchain::Result<Option<SignedBlock<Block>>> {
fn block(
&self,
hash: <Block as BlockT>::Hash,
) -> sp_blockchain::Result<Option<SignedBlock<Block>>> {
with_client! {
self,
client,
{
client.block(id)
client.block(hash)
}
}
}

fn block_status(&self, id: &BlockId<Block>) -> sp_blockchain::Result<BlockStatus> {
fn block_status(&self, hash: <Block as BlockT>::Hash) -> sp_blockchain::Result<BlockStatus> {
with_client! {
self,
client,
{
client.block_status(id)
client.block_status(hash)
}
}
}
Expand Down
18 changes: 14 additions & 4 deletions node/core/candidate-validation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -604,13 +604,16 @@ async fn validate_candidate_exhaustive(

#[async_trait]
trait ValidationBackend {
/// Tries executing a PVF a single time (no retries).
async fn validate_candidate(
&mut self,
pvf: Pvf,
timeout: Duration,
encoded_params: Vec<u8>,
) -> Result<WasmValidationResult, ValidationError>;

/// Tries executing a PVF. Will retry once if an error is encountered that may have been
/// transient.
async fn validate_candidate_with_retry(
&mut self,
raw_validation_code: Vec<u8>,
Expand All @@ -620,7 +623,7 @@ trait ValidationBackend {
// Construct the PVF a single time, since it is an expensive operation. Cloning it is cheap.
let pvf = Pvf::from_code(raw_validation_code);

let validation_result =
let mut validation_result =
self.validate_candidate(pvf.clone(), timeout, params.encode()).await;

// If we get an AmbiguousWorkerDeath error, retry once after a brief delay, on the
Expand All @@ -630,12 +633,19 @@ trait ValidationBackend {
{
// Wait a brief delay before retrying.
futures_timer::Delay::new(PVF_EXECUTION_RETRY_DELAY).await;

gum::debug!(
target: LOG_TARGET,
?pvf,
"Re-trying failed candidate validation due to AmbiguousWorkerDeath."
);

// Encode the params again when re-trying. We expect the retry case to be relatively
// rare, and we want to avoid unconditionally cloning data.
self.validate_candidate(pvf, timeout, params.encode()).await
} else {
validation_result
validation_result = self.validate_candidate(pvf, timeout, params.encode()).await;
}

validation_result
}

async fn precheck_pvf(&mut self, pvf: Pvf) -> Result<Duration, PrepareError>;
Expand Down
18 changes: 14 additions & 4 deletions node/core/provisioner/src/disputes/prioritized_selection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use polkadot_node_subsystem::{
};
use polkadot_primitives::v2::{
supermajority_threshold, CandidateHash, DisputeState, DisputeStatement, DisputeStatementSet,
Hash, MultiDisputeStatementSet, SessionIndex, ValidatorIndex,
Hash, MultiDisputeStatementSet, SessionIndex, ValidDisputeStatementKind, ValidatorIndex,
};
use std::{
collections::{BTreeMap, HashMap},
Expand Down Expand Up @@ -364,10 +364,20 @@ fn is_vote_worth_to_keep(
dispute_statement: DisputeStatement,
onchain_state: &DisputeState,
) -> bool {
let offchain_vote = match dispute_statement {
DisputeStatement::Valid(_) => true,
DisputeStatement::Invalid(_) => false,
let (offchain_vote, valid_kind) = match dispute_statement {
DisputeStatement::Valid(kind) => (true, Some(kind)),
DisputeStatement::Invalid(_) => (false, None),
};
// We want to keep all backing votes. This maximizes the number of backers
// punished when misbehaving.
if let Some(kind) = valid_kind {
match kind {
ValidDisputeStatementKind::BackingValid(_) |
ValidDisputeStatementKind::BackingSeconded(_) => return true,
_ => (),
}
}

let in_validators_for = onchain_state
.validators_for
.get(validator_index.0 as usize)
Expand Down
22 changes: 21 additions & 1 deletion node/core/pvf/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,16 @@ async fn handle_execute_pvf(
},
ArtifactState::FailedToProcess { last_time_failed, num_failures, error } => {
if can_retry_prepare_after_failure(*last_time_failed, *num_failures, error) {
gum::debug!(
target: LOG_TARGET,
?pvf,
?artifact_id,
?last_time_failed,
%num_failures,
%error,
"handle_execute_pvf: Re-trying failed PVF preparation."
);

// If we are allowed to retry the failed prepare job, change the state to
// Preparing and re-queue this job.
*state = ArtifactState::Preparing {
Expand Down Expand Up @@ -585,6 +595,16 @@ async fn handle_heads_up(
},
ArtifactState::FailedToProcess { last_time_failed, num_failures, error } => {
if can_retry_prepare_after_failure(*last_time_failed, *num_failures, error) {
gum::debug!(
target: LOG_TARGET,
?active_pvf,
?artifact_id,
?last_time_failed,
%num_failures,
%error,
"handle_heads_up: Re-trying failed PVF preparation."
);

// If we are allowed to retry the failed prepare job, change the state to
// Preparing and re-queue this job.
*state = ArtifactState::Preparing {
Expand Down Expand Up @@ -1393,7 +1413,7 @@ mod tests {
}

// Test that multiple execution requests don't trigger preparation retries if the first one
// failed due to reproducible error (e.g. Prevalidation).
// failed due to a reproducible error (e.g. Prevalidation).
#[async_std::test]
async fn test_execute_prepare_no_retry() {
let mut test = Builder::default().build();
Expand Down
2 changes: 1 addition & 1 deletion roadmap/implementers-guide/src/node/utility/provisioner.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ The end result of this process is a vector of `BackedCandidate`s, sorted in orde

This is the point at which the block author provides further votes to active disputes or initiates new disputes in the runtime state.

The block-authoring logic of the runtime has an extra step between handling the inherent-data and producing the actual inherent call, which we assume performs the work of filtering out disputes which are not relevant to the on-chain state.
The block-authoring logic of the runtime has an extra step between handling the inherent-data and producing the actual inherent call, which we assume performs the work of filtering out disputes which are not relevant to the on-chain state. Backing votes are always kept in the dispute statement set. This ensures we punish the maximum number of misbehaving backers.

To select disputes:

Expand Down
3 changes: 3 additions & 0 deletions runtime/kusama/constants/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ sp-core = { git = "https://github.com/paritytech/substrate", default-features =
[features]
default = ["std"]
std = [
"frame-support/std",
"primitives/std",
"runtime-common/std",
"sp-core/std",
"sp-runtime/std",
"sp-weights/std"
Expand Down
7 changes: 6 additions & 1 deletion runtime/kusama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,9 @@ impl Get<&'static str> for StakingMigrationV11OldPallet {
}
}

/// All migrations that will run on the next runtime upgrade.
///
/// Should be cleared after every release.
pub type Migrations = (
pallet_balances::migration::MigrateToTrackInactive<Runtime, CheckAccount>,
crowdloan::migration::MigrateToTrackInactive<Runtime>,
Expand All @@ -1488,6 +1491,8 @@ pub type Migrations = (
Runtime,
governance::FellowshipReferendaInstance,
>,
pallet_scheduler::migration::v4::CleanupAgendas<Runtime>,
pallet_staking::migrations::v13::MigrateToV13<Runtime>,
);

/// Unchecked extrinsic type as expected by this runtime.
Expand Down Expand Up @@ -1912,7 +1917,7 @@ sp_api::impl_runtime_apis! {

#[cfg(feature = "try-runtime")]
impl frame_try_runtime::TryRuntime<Block> for Runtime {
fn on_runtime_upgrade(checks: bool) -> (Weight, Weight) {
fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) {
log::info!("try-runtime::on_runtime_upgrade kusama.");
let weight = Executive::try_runtime_upgrade(checks).unwrap();
(weight, BlockWeights::get().max_block)
Expand Down
3 changes: 3 additions & 0 deletions runtime/polkadot/constants/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ sp-core = { git = "https://github.com/paritytech/substrate", default-features =
[features]
default = ["std"]
std = [
"frame-support/std",
"primitives/std",
"runtime-common/std",
"sp-core/std",
"sp-runtime/std",
"sp-weights/std"
Expand Down
7 changes: 6 additions & 1 deletion runtime/polkadot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1597,9 +1597,14 @@ impl Get<&'static str> for StakingMigrationV11OldPallet {
}
}

/// All migrations that will run on the next runtime upgrade.
///
/// Should be cleared after every release.
pub type Migrations = (
pallet_balances::migration::MigrateToTrackInactive<Runtime, xcm_config::CheckAccount>,
crowdloan::migration::MigrateToTrackInactive<Runtime>,
pallet_scheduler::migration::v4::CleanupAgendas<Runtime>,
pallet_staking::migrations::v13::MigrateToV13<Runtime>,
);

/// Unchecked extrinsic type as expected by this runtime.
Expand Down Expand Up @@ -2014,7 +2019,7 @@ sp_api::impl_runtime_apis! {

#[cfg(feature = "try-runtime")]
impl frame_try_runtime::TryRuntime<Block> for Runtime {
fn on_runtime_upgrade(checks: bool) -> (Weight, Weight) {
fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) {
log::info!("try-runtime::on_runtime_upgrade polkadot.");
let weight = Executive::try_runtime_upgrade(checks).unwrap();
(weight, BlockWeights::get().max_block)
Expand Down
15 changes: 14 additions & 1 deletion runtime/rococo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pallet-xcm = { path = "../../xcm/pallet-xcm", default-features = false }
pallet-xcm-benchmarks = { path = "../../xcm/pallet-xcm-benchmarks", default-features = false, optional = true }

frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false, optional = true }
frame-try-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false, optional = true }
frame-system-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false, optional = true }
hex-literal = { version = "0.3.4" }

Expand All @@ -92,9 +93,12 @@ xcm-builder = { package = "xcm-builder", path = "../../xcm/xcm-builder", default
[dev-dependencies]
tiny-keccak = { version = "2.0.2", features = ["keccak"] }
keyring = { package = "sp-keyring", git = "https://github.com/paritytech/substrate", branch = "master" }
remote-externalities = { git = "https://github.com/paritytech/substrate", branch = "master", package = "frame-remote-externalities" }
sp-trie = { git = "https://github.com/paritytech/substrate", branch = "master" }
separator = "0.4.1"
serde_json = "1.0.81"
sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
tokio = { version = "1.22.0", features = ["macros"] }

[build-dependencies]
substrate-wasm-builder = { git = "https://github.com/paritytech/substrate", branch = "master" }
Expand Down Expand Up @@ -168,6 +172,7 @@ std = [
"sp-session/std",
"runtime-common/std",
"runtime-parachains/std",
"frame-try-runtime/std",
"beefy-primitives/std",
"rococo-runtime-constants/std",
"xcm/std",
Expand Down Expand Up @@ -214,10 +219,14 @@ runtime-benchmarks = [
]
try-runtime = [
"frame-executive/try-runtime",
"frame-try-runtime",
"frame-system/try-runtime",
"pallet-authority-discovery/try-runtime",
"pallet-authorship/try-runtime",
"pallet-balances/try-runtime",
"pallet-babe/try-runtime",
"pallet-beefy/try-runtime",
"pallet-beefy-mmr/try-runtime",
"pallet-bounties/try-runtime",
"pallet-child-bounties/try-runtime",
"pallet-transaction-payment/try-runtime",
Expand All @@ -229,7 +238,9 @@ try-runtime = [
"pallet-im-online/try-runtime",
"pallet-indices/try-runtime",
"pallet-membership/try-runtime",
"pallet-mmr/try-runtime",
"pallet-multisig/try-runtime",
"pallet-nis/try-runtime",
"pallet-offences/try-runtime",
"pallet-preimage/try-runtime",
"pallet-proxy/try-runtime",
Expand All @@ -239,13 +250,15 @@ try-runtime = [
"pallet-society/try-runtime",
"pallet-sudo/try-runtime",
"pallet-staking/try-runtime",
"pallet-state-trie-migration/try-runtime",
"pallet-timestamp/try-runtime",
"pallet-tips/try-runtime",
"pallet-treasury/try-runtime",
"pallet-utility/try-runtime",
"pallet-vesting/try-runtime",
"pallet-babe/try-runtime",
"pallet-xcm/try-runtime",
"runtime-common/try-runtime",
"runtime-parachains/try-runtime",
]
# When enabled, the runtime API will not be build.
#
Expand Down
3 changes: 3 additions & 0 deletions runtime/rococo/constants/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ sp-core = { git = "https://github.com/paritytech/substrate", default-features =
[features]
default = ["std"]
std = [
"frame-support/std",
"primitives/std",
"runtime-common/std",
"sp-core/std",
"sp-runtime/std",
"sp-weights/std"
Expand Down
Loading

0 comments on commit b2f82ca

Please sign in to comment.