Skip to content

Commit

Permalink
Merge branch 'master' into pg/hack-indexer
Browse files Browse the repository at this point in the history
  • Loading branch information
pgherveou authored Jan 14, 2025
2 parents 4ef9b5c + ddffa02 commit f86adfa
Show file tree
Hide file tree
Showing 27 changed files with 403 additions and 154 deletions.
15 changes: 5 additions & 10 deletions polkadot/node/core/candidate-validation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -912,15 +912,10 @@ async fn validate_candidate_exhaustive(
// invalid.
Ok(ValidationResult::Invalid(InvalidCandidate::CommitmentsHashMismatch))
} else {
let core_index = candidate_receipt.descriptor.core_index();

match (core_index, exec_kind) {
match exec_kind {
// Core selectors are optional for V2 descriptors, but we still check the
// descriptor core index.
(
Some(_core_index),
PvfExecKind::Backing(_) | PvfExecKind::BackingSystemParas(_),
) => {
PvfExecKind::Backing(_) | PvfExecKind::BackingSystemParas(_) => {
let Some(claim_queue) = maybe_claim_queue else {
let error = "cannot fetch the claim queue from the runtime";
gum::warn!(
Expand All @@ -937,17 +932,17 @@ async fn validate_candidate_exhaustive(
{
gum::warn!(
target: LOG_TARGET,
?err,
candidate_hash = ?candidate_receipt.hash(),
"Candidate core index is invalid",
"Candidate core index is invalid: {}",
err
);
return Ok(ValidationResult::Invalid(
InvalidCandidate::InvalidCoreIndex,
))
}
},
// No checks for approvals and disputes
(_, _) => {},
_ => {},
}

Ok(ValidationResult::Valid(
Expand Down
71 changes: 67 additions & 4 deletions polkadot/node/core/candidate-validation/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ use polkadot_node_subsystem_util::reexports::SubsystemContext;
use polkadot_overseer::ActivatedLeaf;
use polkadot_primitives::{
vstaging::{
CandidateDescriptorV2, ClaimQueueOffset, CoreSelector, MutateDescriptorV2, UMPSignal,
UMP_SEPARATOR,
CandidateDescriptorV2, CandidateDescriptorVersion, ClaimQueueOffset, CoreSelector,
MutateDescriptorV2, UMPSignal, UMP_SEPARATOR,
},
CandidateDescriptor, CoreIndex, GroupIndex, HeadData, Id as ParaId, OccupiedCoreAssumption,
SessionInfo, UpwardMessage, ValidatorId,
Expand Down Expand Up @@ -851,7 +851,7 @@ fn invalid_session_or_core_index() {
))
.unwrap();

// Validation doesn't fail for approvals, core/session index is not checked.
// Validation doesn't fail for disputes, core/session index is not checked.
assert_matches!(v, ValidationResult::Valid(outputs, used_validation_data) => {
assert_eq!(outputs.head_data, HeadData(vec![1, 1, 1]));
assert_eq!(outputs.upward_messages, commitments.upward_messages);
Expand Down Expand Up @@ -911,6 +911,69 @@ fn invalid_session_or_core_index() {
assert_eq!(outputs.hrmp_watermark, 0);
assert_eq!(used_validation_data, validation_data);
});

// Test that a v1 candidate that outputs the core selector UMP signal is invalid.
let descriptor_v1 = make_valid_candidate_descriptor(
ParaId::from(1_u32),
dummy_hash(),
dummy_hash(),
pov.hash(),
validation_code.hash(),
validation_result.head_data.hash(),
dummy_hash(),
sp_keyring::Sr25519Keyring::Ferdie,
);
let descriptor: CandidateDescriptorV2 = descriptor_v1.into();

perform_basic_checks(&descriptor, validation_data.max_pov_size, &pov, &validation_code.hash())
.unwrap();
assert_eq!(descriptor.version(), CandidateDescriptorVersion::V1);
let candidate_receipt = CandidateReceipt { descriptor, commitments_hash: commitments.hash() };

for exec_kind in
[PvfExecKind::Backing(dummy_hash()), PvfExecKind::BackingSystemParas(dummy_hash())]
{
let result = executor::block_on(validate_candidate_exhaustive(
Some(1),
MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())),
validation_data.clone(),
validation_code.clone(),
candidate_receipt.clone(),
Arc::new(pov.clone()),
ExecutorParams::default(),
exec_kind,
&Default::default(),
Some(Default::default()),
))
.unwrap();
assert_matches!(result, ValidationResult::Invalid(InvalidCandidate::InvalidCoreIndex));
}

// Validation doesn't fail for approvals and disputes, core/session index is not checked.
for exec_kind in [PvfExecKind::Approval, PvfExecKind::Dispute] {
let v = executor::block_on(validate_candidate_exhaustive(
Some(1),
MockValidateCandidateBackend::with_hardcoded_result(Ok(validation_result.clone())),
validation_data.clone(),
validation_code.clone(),
candidate_receipt.clone(),
Arc::new(pov.clone()),
ExecutorParams::default(),
exec_kind,
&Default::default(),
Default::default(),
))
.unwrap();

assert_matches!(v, ValidationResult::Valid(outputs, used_validation_data) => {
assert_eq!(outputs.head_data, HeadData(vec![1, 1, 1]));
assert_eq!(outputs.upward_messages, commitments.upward_messages);
assert_eq!(outputs.horizontal_messages, Vec::new());
assert_eq!(outputs.new_validation_code, Some(vec![2, 2, 2].into()));
assert_eq!(outputs.hrmp_watermark, 0);
assert_eq!(used_validation_data, validation_data);
});
}
}

#[test]
Expand Down Expand Up @@ -1407,7 +1470,7 @@ fn compressed_code_works() {
ExecutorParams::default(),
PvfExecKind::Backing(dummy_hash()),
&Default::default(),
Default::default(),
Some(Default::default()),
));

assert_matches!(v, Ok(ValidationResult::Valid(_, _)));
Expand Down
11 changes: 3 additions & 8 deletions polkadot/node/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,14 +944,9 @@ pub fn new_full<
secure_validator_mode,
prep_worker_path,
exec_worker_path,
pvf_execute_workers_max_num: execute_workers_max_num.unwrap_or_else(
|| match config.chain_spec.identify_chain() {
// The intention is to use this logic for gradual increasing from 2 to 4
// of this configuration chain by chain until it reaches production chain.
Chain::Polkadot | Chain::Kusama => 2,
Chain::Rococo | Chain::Westend | Chain::Unknown => 4,
},
),
// Default execution workers is 4 because we have 8 cores on the reference hardware,
// and this accounts for 50% of that cpu capacity.
pvf_execute_workers_max_num: execute_workers_max_num.unwrap_or(4),
pvf_prepare_workers_soft_max_num: prepare_workers_soft_max_num.unwrap_or(1),
pvf_prepare_workers_hard_max_num: prepare_workers_hard_max_num.unwrap_or(2),
})
Expand Down
30 changes: 23 additions & 7 deletions polkadot/primitives/src/vstaging/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,10 @@ pub enum CommittedCandidateReceiptError {
/// Currenly only one such message is allowed.
#[cfg_attr(feature = "std", error("Too many UMP signals"))]
TooManyUMPSignals,
/// If the parachain runtime started sending core selectors, v1 descriptors are no longer
/// allowed.
#[cfg_attr(feature = "std", error("Version 1 receipt does not support core selectors"))]
CoreSelectorWithV1Decriptor,
}

macro_rules! impl_getter {
Expand Down Expand Up @@ -603,15 +607,25 @@ impl<H: Copy> CommittedCandidateReceiptV2<H> {
&self,
cores_per_para: &TransposedClaimQueue,
) -> Result<(), CommittedCandidateReceiptError> {
let maybe_core_selector = self.commitments.core_selector()?;

match self.descriptor.version() {
// Don't check v1 descriptors.
CandidateDescriptorVersion::V1 => return Ok(()),
CandidateDescriptorVersion::V1 => {
// If the parachain runtime started sending core selectors, v1 descriptors are no
// longer allowed.
if maybe_core_selector.is_some() {
return Err(CommittedCandidateReceiptError::CoreSelectorWithV1Decriptor)
} else {
// Nothing else to check for v1 descriptors.
return Ok(())
}
},
CandidateDescriptorVersion::V2 => {},
CandidateDescriptorVersion::Unknown =>
return Err(CommittedCandidateReceiptError::UnknownVersion(self.descriptor.version)),
}

let (maybe_core_index_selector, cq_offset) = self.commitments.core_selector()?.map_or_else(
let (maybe_core_index_selector, cq_offset) = maybe_core_selector.map_or_else(
|| (None, ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET)),
|(sel, off)| (Some(sel), off),
);
Expand Down Expand Up @@ -1207,8 +1221,7 @@ mod tests {
assert_eq!(new_ccr.hash(), v2_ccr.hash());
}

// Only check descriptor `core_index` field of v2 descriptors. If it is v1, that field
// will be garbage.
// V1 descriptors are forbidden once the parachain runtime started sending UMP signals.
#[test]
fn test_v1_descriptors_with_ump_signal() {
let mut ccr = dummy_old_committed_candidate_receipt();
Expand All @@ -1234,9 +1247,12 @@ mod tests {
cq.insert(CoreIndex(0), vec![v1_ccr.descriptor.para_id()].into());
cq.insert(CoreIndex(1), vec![v1_ccr.descriptor.para_id()].into());

assert!(v1_ccr.check_core_index(&transpose_claim_queue(cq)).is_ok());

assert_eq!(v1_ccr.descriptor.core_index(), None);

assert_eq!(
v1_ccr.check_core_index(&transpose_claim_queue(cq)),
Err(CommittedCandidateReceiptError::CoreSelectorWithV1Decriptor)
);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2276,7 +2276,7 @@ sp_api::impl_runtime_apis! {
}

fn current_set_id() -> fg_primitives::SetId {
Grandpa::current_set_id()
pallet_grandpa::CurrentSetId::<Runtime>::get()
}

fn submit_report_equivocation_unsigned_extrinsic(
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/test-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,7 @@ sp_api::impl_runtime_apis! {
}

fn current_set_id() -> fg_primitives::SetId {
Grandpa::current_set_id()
pallet_grandpa::CurrentSetId::<Runtime>::get()
}

fn submit_report_equivocation_unsigned_extrinsic(
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2300,7 +2300,7 @@ sp_api::impl_runtime_apis! {
}

fn current_set_id() -> fg_primitives::SetId {
Grandpa::current_set_id()
pallet_grandpa::CurrentSetId::<Runtime>::get()
}

fn submit_report_equivocation_unsigned_extrinsic(
Expand Down
22 changes: 22 additions & 0 deletions prdoc/pr_4529.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Removed `pallet::getter` usage from pallet-grandpa

doc:
- audience: Runtime Dev
description: |
This PR removed the `pallet::getter`s from `pallet-grandpa`.
The syntax `StorageItem::<T, I>::get()` should be used instead

crates:
- name: pallet-grandpa
bump: minor
- name: kitchensink-runtime
bump: none
- name: westend-runtime
bump: none
- name: polkadot-test-runtime
bump: none
- name: rococo-runtime
bump: none
8 changes: 8 additions & 0 deletions prdoc/pr_7102.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
title: '`fatxpool`: rotator cache size now depends on pool''s limits'
doc:
- audience: Node Dev
description: |-
This PR modifies the hard-coded size of extrinsics cache within `PoolRotator` to be inline with pool limits. It only applies to fork-aware transaction pool. For the legacy (single-state) transaction pool the logic remains untouched.
crates:
- name: sc-transaction-pool
bump: minor
8 changes: 8 additions & 0 deletions prdoc/pr_7116.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
title: Increase the number of pvf execution workers from 2 to 4
doc:
- audience: Node Dev
description: |-
Increase the number of pvf execution workers from 2 to 4.
crates:
- name: polkadot-service
bump: patch
9 changes: 9 additions & 0 deletions prdoc/pr_7127.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
title: 'Forbid v1 descriptors with UMP signals'
doc:
- audience: [Runtime Dev, Node Dev]
description: Adds a check that parachain candidates do not send out UMP signals with v1 descriptors.
crates:
- name: polkadot-node-core-candidate-validation
bump: minor
- name: polkadot-primitives
bump: major
2 changes: 1 addition & 1 deletion substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2979,7 +2979,7 @@ impl_runtime_apis! {
}

fn current_set_id() -> sp_consensus_grandpa::SetId {
Grandpa::current_set_id()
pallet_grandpa::CurrentSetId::<Runtime>::get()
}

fn submit_report_equivocation_unsigned_extrinsic(
Expand Down
12 changes: 10 additions & 2 deletions substrate/client/transaction-pool/benches/basics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,22 @@ fn benchmark_main(c: &mut Criterion) {
c.bench_function("sequential 50 tx", |b| {
b.iter(|| {
let api = Arc::from(TestApi::new_dependant());
bench_configured(Pool::new(Default::default(), true.into(), api.clone()), 50, api);
bench_configured(
Pool::new_with_staticly_sized_rotator(Default::default(), true.into(), api.clone()),
50,
api,
);
});
});

c.bench_function("random 100 tx", |b| {
b.iter(|| {
let api = Arc::from(TestApi::default());
bench_configured(Pool::new(Default::default(), true.into(), api.clone()), 100, api);
bench_configured(
Pool::new_with_staticly_sized_rotator(Default::default(), true.into(), api.clone()),
100,
api,
);
});
});
}
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/transaction-pool/src/common/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,5 +222,5 @@ pub(crate) fn uxt(transfer: Transfer) -> Extrinsic {

pub(crate) fn pool() -> (Pool<TestApi>, Arc<TestApi>) {
let api = Arc::new(TestApi::default());
(Pool::new(Default::default(), true.into(), api.clone()), api)
(Pool::new_with_staticly_sized_rotator(Default::default(), true.into(), api.clone()), api)
}
Original file line number Diff line number Diff line change
Expand Up @@ -329,14 +329,14 @@ where
let stream_map = futures::stream::unfold(ctx, |mut ctx| async move {
loop {
if let Some(dropped) = ctx.get_pending_dropped_transaction() {
debug!("dropped_watcher: sending out (pending): {dropped:?}");
trace!("dropped_watcher: sending out (pending): {dropped:?}");
return Some((dropped, ctx));
}
tokio::select! {
biased;
Some(event) = next_event(&mut ctx.stream_map) => {
if let Some(dropped) = ctx.handle_event(event.0, event.1) {
debug!("dropped_watcher: sending out: {dropped:?}");
trace!("dropped_watcher: sending out: {dropped:?}");
return Some((dropped, ctx));
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ where
pool_api.clone(),
listener.clone(),
metrics.clone(),
TXMEMPOOL_TRANSACTION_LIMIT_MULTIPLIER * (options.ready.count + options.future.count),
TXMEMPOOL_TRANSACTION_LIMIT_MULTIPLIER * options.total_count(),
options.ready.total_bytes + options.future.total_bytes,
));

Expand Down
Loading

0 comments on commit f86adfa

Please sign in to comment.