diff --git a/polkadot/node/core/provisioner/src/lib.rs b/polkadot/node/core/provisioner/src/lib.rs index 8f9421b25f..84bff27dca 100644 --- a/polkadot/node/core/provisioner/src/lib.rs +++ b/polkadot/node/core/provisioner/src/lib.rs @@ -448,7 +448,7 @@ async fn select_candidates( selected_candidates.clone(), tx, )).into()).await.map_err(|err| Error::GetBackedCandidatesSend(err))?; - let candidates = rx.await.map_err(|err| Error::CanceledBackedCandidates(err))?; + let mut candidates = rx.await.map_err(|err| Error::CanceledBackedCandidates(err))?; // `selected_candidates` is generated in ascending order by core index, and `GetBackedCandidates` // _should_ preserve that property, but let's just make sure. @@ -466,6 +466,20 @@ async fn select_candidates( Err(Error::BackedCandidateOrderingProblem)?; } + // keep only one candidate with validation code. + let mut with_validation_code = false; + candidates.retain(|c| { + if c.candidate.commitments.new_validation_code.is_some() { + if with_validation_code { + return false + } + + with_validation_code = true; + } + + true + }); + Ok(candidates) } diff --git a/polkadot/node/core/provisioner/src/tests.rs b/polkadot/node/core/provisioner/src/tests.rs index f6977d69ec..40a1c51e1a 100644 --- a/polkadot/node/core/provisioner/src/tests.rs +++ b/polkadot/node/core/provisioner/src/tests.rs @@ -426,4 +426,63 @@ mod select_candidates { ); }) } + + #[test] + fn selects_max_one_code_upgrade() { + let mock_cores = mock_availability_cores(); + let n_cores = mock_cores.len(); + + let empty_hash = PersistedValidationData::::default().hash(); + + // why those particular indices? see the comments on mock_availability_cores() + // the first candidate with code is included out of [1, 4, 7, 8, 10]. + let cores = [1, 7, 10]; + let cores_with_code = [1, 4, 8]; + + let committed_receipts: Vec<_> = (0..mock_cores.len()) + .map(|i| CommittedCandidateReceipt { + descriptor: CandidateDescriptor { + para_id: i.into(), + persisted_validation_data_hash: empty_hash, + ..Default::default() + }, + commitments: CandidateCommitments { + new_validation_code: if cores_with_code.contains(&i) { Some(vec![].into()) } else { None }, + ..Default::default() + }, + ..Default::default() + }) + .collect(); + + let candidates: Vec<_> = committed_receipts.iter().map(|r| r.to_plain()).collect(); + + let expected_candidates: Vec<_> = cores + .iter() + .map(|&idx| candidates[idx].clone()) + .collect(); + + let expected_backed: Vec<_> = cores + .iter() + .map(|&idx| BackedCandidate { + candidate: committed_receipts[idx].clone(), + validity_votes: Vec::new(), + validator_indices: default_bitvec(n_cores), + }) + .collect(); + + test_harness(|r| mock_overseer(r, expected_backed), |mut tx: mpsc::Sender| async move { + let result = + select_candidates(&mock_cores, &[], &candidates, Default::default(), &mut tx) + .await.unwrap(); + + result.into_iter() + .for_each(|c| + assert!( + expected_candidates.iter().any(|c2| c.candidate.corresponds_to(c2)), + "Failed to find candidate: {:?}", + c, + ) + ); + }) + } } diff --git a/polkadot/roadmap/implementers-guide/src/node/utility/provisioner.md b/polkadot/roadmap/implementers-guide/src/node/utility/provisioner.md index f7bf66dd46..ea62f863f6 100644 --- a/polkadot/roadmap/implementers-guide/src/node/utility/provisioner.md +++ b/polkadot/roadmap/implementers-guide/src/node/utility/provisioner.md @@ -69,7 +69,7 @@ To determine availability: - There are two constraints: `backed_candidate.candidate.descriptor.para_id == scheduled_core.para_id && candidate.candidate.descriptor.validation_data_hash == computed_validation_data_hash`. - In the event that more than one candidate meets the constraints, selection between the candidates is arbitrary. However, not more than one candidate can be selected per core. -The end result of this process is a vector of `BackedCandidate`s, sorted in order of their core index. +The end result of this process is a vector of `BackedCandidate`s, sorted in order of their core index. Furthermore, this process should select at maximum one candidate which upgrades the runtime validation code. ### Determining Bitfield Availability diff --git a/polkadot/runtime/parachains/src/inclusion_inherent.rs b/polkadot/runtime/parachains/src/inclusion_inherent.rs index 12012e4e6f..56516f8f76 100644 --- a/polkadot/runtime/parachains/src/inclusion_inherent.rs +++ b/polkadot/runtime/parachains/src/inclusion_inherent.rs @@ -169,8 +169,29 @@ decl_module! { /// This is somewhat less desirable than attempting to fit some of them, but is more fair in the /// even that we can't trust the provisioner to provide a fair / random ordering of candidates. fn limit_backed_candidates( - backed_candidates: Vec>, + mut backed_candidates: Vec>, ) -> Vec> { + const MAX_CODE_UPGRADES: usize = 1; + + // Ignore any candidates beyond one that contain code upgrades. + // + // This is an artificial limitation that does not appear in the guide as it is a practical + // concern around execution. + { + let mut code_upgrades = 0; + backed_candidates.retain(|c| { + if c.candidate.commitments.new_validation_code.is_some() { + if code_upgrades >= MAX_CODE_UPGRADES { + return false + } + + code_upgrades +=1; + } + + true + }); + } + // the weight of the inclusion inherent is already included in the current block weight, // so our operation is simple: if the block is currently overloaded, make this intrinsic smaller if frame_system::Module::::block_weight().total() > ::BlockWeights::get().max_block { @@ -272,6 +293,16 @@ mod tests { assert_eq!(limit_backed_candidates::(backed_candidates).len(), 0); }); } + + #[test] + fn ignores_subsequent_code_upgrades() { + new_test_ext(MockGenesisConfig::default()).execute_with(|| { + let mut backed = BackedCandidate::default(); + backed.candidate.commitments.new_validation_code = Some(Vec::new().into()); + let backed_candidates = (0..3).map(|_| backed.clone()).collect(); + assert_eq!(limit_backed_candidates::(backed_candidates).len(), 1); + }); + } } mod inclusion_inherent_weight {