Skip to content

Commit

Permalink
inclusion emulator: correctly handle UMP signals (#6178)
Browse files Browse the repository at this point in the history
Changes inclusion emulator to not count the UMP signals when checking
ump message constraints.

---------

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
sandreim and actions-user authored Nov 4, 2024
1 parent 8b6f815 commit b326540
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 7 deletions.
57 changes: 51 additions & 6 deletions polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,9 @@ pub struct ConstraintModifications {
pub hrmp_watermark: Option<HrmpWatermarkUpdate>,
/// Outbound HRMP channel modifications.
pub outbound_hrmp: HashMap<ParaId, OutboundHrmpChannelModification>,
/// The amount of UMP messages sent.
/// The amount of UMP XCM messages sent. `UMPSignal` and separator are excluded.
pub ump_messages_sent: usize,
/// The amount of UMP bytes sent.
/// The amount of UMP XCM bytes sent. `UMPSignal` and separator are excluded.
pub ump_bytes_sent: usize,
/// The amount of DMP messages processed.
pub dmp_messages_processed: usize,
Expand Down Expand Up @@ -600,6 +600,18 @@ impl Fragment {
validation_code_hash: &ValidationCodeHash,
persisted_validation_data: &PersistedValidationData,
) -> Result<ConstraintModifications, FragmentValidityError> {
// Filter UMP signals and the separator.
let upward_messages = if let Some(separator_index) =
commitments.upward_messages.iter().position(|message| message.is_empty())
{
commitments.upward_messages.split_at(separator_index).0
} else {
&commitments.upward_messages
};

let ump_messages_sent = upward_messages.len();
let ump_bytes_sent = upward_messages.iter().map(|msg| msg.len()).sum();

let modifications = {
ConstraintModifications {
required_parent: Some(commitments.head_data.clone()),
Expand Down Expand Up @@ -632,8 +644,8 @@ impl Fragment {

outbound_hrmp
},
ump_messages_sent: commitments.upward_messages.len(),
ump_bytes_sent: commitments.upward_messages.iter().map(|msg| msg.len()).sum(),
ump_messages_sent,
ump_bytes_sent,
dmp_messages_processed: commitments.processed_downward_messages as _,
code_upgrade_applied: operating_constraints
.future_validation_code
Expand Down Expand Up @@ -750,7 +762,7 @@ fn validate_against_constraints(
})
}

if commitments.upward_messages.len() > constraints.max_ump_num_per_candidate {
if modifications.ump_messages_sent > constraints.max_ump_num_per_candidate {
return Err(FragmentValidityError::UmpMessagesPerCandidateOverflow {
messages_allowed: constraints.max_ump_num_per_candidate,
messages_submitted: commitments.upward_messages.len(),
Expand Down Expand Up @@ -814,7 +826,11 @@ impl HypotheticalOrConcreteCandidate for HypotheticalCandidate {
#[cfg(test)]
mod tests {
use super::*;
use polkadot_primitives::{HorizontalMessages, OutboundHrmpMessage, ValidationCode};
use codec::Encode;
use polkadot_primitives::{
vstaging::{ClaimQueueOffset, CoreSelector, UMPSignal, UMP_SEPARATOR},
HorizontalMessages, OutboundHrmpMessage, ValidationCode,
};

#[test]
fn stack_modifications() {
Expand Down Expand Up @@ -1267,6 +1283,35 @@ mod tests {
);
}

#[test]
fn ump_signals_ignored() {
let relay_parent = RelayChainBlockInfo {
number: 6,
hash: Hash::repeat_byte(0xbe),
storage_root: Hash::repeat_byte(0xff),
};

let constraints = make_constraints();
let mut candidate = make_candidate(&constraints, &relay_parent);
let max_ump = constraints.max_ump_num_per_candidate;

// Fill ump queue to the limit.
candidate
.commitments
.upward_messages
.try_extend((0..max_ump).map(|i| vec![i as u8]))
.unwrap();

// Add ump signals.
candidate.commitments.upward_messages.force_push(UMP_SEPARATOR);
candidate
.commitments
.upward_messages
.force_push(UMPSignal::SelectCore(CoreSelector(0), ClaimQueueOffset(1)).encode());

Fragment::new(relay_parent, constraints, Arc::new(candidate)).unwrap();
}

#[test]
fn fragment_relay_parent_too_old() {
let relay_parent = RelayChainBlockInfo {
Expand Down
2 changes: 1 addition & 1 deletion polkadot/primitives/test-helpers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ sp-keyring = { workspace = true, default-features = true }
sp-application-crypto = { workspace = true }
sp-runtime = { workspace = true, default-features = true }
sp-core = { features = ["std"], workspace = true, default-features = true }
polkadot-primitives = { workspace = true, default-features = true }
polkadot-primitives = { features = ["test"], workspace = true, default-features = true }
rand = { workspace = true, default-features = true }

0 comments on commit b326540

Please sign in to comment.