Skip to content

Commit

Permalink
Merge branch 'master' into fix/xcm-api
Browse files Browse the repository at this point in the history
  • Loading branch information
franciscoaguirre authored Jan 14, 2025
2 parents 69bbb4b + 105c5b9 commit 97e4f03
Show file tree
Hide file tree
Showing 25 changed files with 1,513 additions and 273 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
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
8 changes: 8 additions & 0 deletions prdoc/pr_6647.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
title: '`fatxpool`: proper handling of priorities when mempool is full'
doc:
- audience: Node Dev
description: |-
Higher-priority transactions can now replace lower-priority transactions even when the internal _tx_mem_pool_ is full.
crates:
- name: sc-transaction-pool
bump: minor
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
15 changes: 15 additions & 0 deletions prdoc/pr_7133.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# 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: Sufix litep2p to the identify agent version for visibility

doc:
- audience: [Node Dev, Node Operator]
description: |
This PR adds the `(litep2p)` suffix to the agent version (user agent) of the identify protocol.
The change is needed to gain visibility into network backends and determine exactly the number of validators that are running litep2p.
Using tools like subp2p-explorer, we can determine if the validators are running litep2p nodes.

crates:
- name: sc-network
bump: patch
2 changes: 1 addition & 1 deletion substrate/client/network/src/litep2p/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ impl Discovery {
_peerstore_handle: Arc<dyn PeerStoreProvider>,
) -> (Self, PingConfig, IdentifyConfig, KademliaConfig, Option<MdnsConfig>) {
let (ping_config, ping_event_stream) = PingConfig::default();
let user_agent = format!("{} ({})", config.client_version, config.node_name);
let user_agent = format!("{} ({}) (litep2p)", config.client_version, config.node_name);

let (identify_config, identify_event_stream) =
IdentifyConfig::new("/substrate/1.0".to_string(), Some(user_agent));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,13 @@ pub struct DroppedTransaction<Hash> {
}

impl<Hash> DroppedTransaction<Hash> {
fn new_usurped(tx_hash: Hash, by: Hash) -> Self {
/// Creates a new instance with reason set to `DroppedReason::Usurped(by)`.
pub fn new_usurped(tx_hash: Hash, by: Hash) -> Self {
Self { reason: DroppedReason::Usurped(by), tx_hash }
}

fn new_enforced_by_limts(tx_hash: Hash) -> Self {
/// Creates a new instance with reason set to `DroppedReason::LimitsEnforced`.
pub fn new_enforced_by_limts(tx_hash: Hash) -> Self {
Self { reason: DroppedReason::LimitsEnforced, tx_hash }
}
}
Expand Down Expand Up @@ -256,11 +258,13 @@ where
self.future_transaction_views.entry(tx_hash).or_default().insert(block_hash);
},
TransactionStatus::Ready | TransactionStatus::InBlock(..) => {
// note: if future transaction was once seens as the ready we may want to treat it
// as ready transactions. Unreferenced future transactions are more likely to be
// removed when the last referencing view is removed then ready transactions.
// Transcaction seen as ready is likely quite close to be included in some
// future fork.
// note: if future transaction was once seen as the ready we may want to treat it
// as ready transaction. The rationale behind this is as follows: we want to remove
// unreferenced future transactions when the last referencing view is removed (to
// avoid clogging mempool). For ready transactions we prefer to keep them in mempool
// even if no view is currently referencing them. Future transcaction once seen as
// ready is likely quite close to be included in some future fork (it is close to be
// ready, so we make exception and treat such transaction as ready).
if let Some(mut views) = self.future_transaction_views.remove(&tx_hash) {
views.insert(block_hash);
self.ready_transaction_views.insert(tx_hash, views);
Expand Down
Loading

0 comments on commit 97e4f03

Please sign in to comment.