Skip to content

Commit

Permalink
stricter UMP signal checks and tests
Browse files Browse the repository at this point in the history
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
  • Loading branch information
sandreim committed Sep 23, 2024
1 parent ba9d3ff commit 9c4e2ae
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 3 deletions.
11 changes: 10 additions & 1 deletion polkadot/runtime/parachains/src/inclusion/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,8 @@ pub(crate) enum UmpAcceptanceCheckErr {
/// The allowed number of `UMPSignal` messages in the queue was exceeded.
/// Currenly only one such message is allowed.
TooManyUMPSignals { count: u32 },
/// The UMP queue contains an invalid `UMPSignal`
InvalidUMPSignal,
}

impl fmt::Debug for UmpAcceptanceCheckErr {
Expand Down Expand Up @@ -472,7 +474,10 @@ impl fmt::Debug for UmpAcceptanceCheckErr {
write!(fmt, "upward message rejected because the para is off-boarding")
},
UmpAcceptanceCheckErr::TooManyUMPSignals { count } => {
write!(fmt, "the umpq queue has too many `UMPSignal` mesages ({} > 1 )", count)
write!(fmt, "the ump queue has too many `UMPSignal` mesages ({} > 1 )", count)
},
UmpAcceptanceCheckErr::InvalidUMPSignal => {
write!(fmt, "the ump queue contains an invalid UMP signal")
},
}
}
Expand Down Expand Up @@ -953,6 +958,10 @@ impl<T: Config> Pallet<T> {
})
}

if ump_signals.len() == 1 {
return Err(UmpAcceptanceCheckErr::InvalidUMPSignal)
}

upward_messages
} else {
upward_messages
Expand Down
69 changes: 68 additions & 1 deletion polkadot/runtime/parachains/src/paras_inherent/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1742,6 +1742,73 @@ mod enter {
});
}

#[test]
fn invalid_ump_signals() {
let config = default_config();
assert!(config.configuration.config.scheduler_params.lookahead > 0);
new_test_ext(config).execute_with(|| {
// Set the elastic scaling MVP feature.
configuration::Pallet::<Test>::set_node_feature(
RuntimeOrigin::root(),
FeatureIndex::CandidateReceiptV2 as u8,
true,
)
.unwrap();

let mut backed_and_concluding = BTreeMap::new();
backed_and_concluding.insert(0, 1);
backed_and_concluding.insert(1, 1);
backed_and_concluding.insert(2, 1);

let unavailable_cores = vec![];

let scenario = make_inherent_data(TestConfig {
dispute_statements: BTreeMap::new(),
dispute_sessions: vec![], // No disputes
backed_and_concluding,
num_validators_per_core: 5,
code_upgrade: None,
fill_claimqueue: true,
elastic_paras: [(2, 8)].into_iter().collect(),
unavailable_cores: unavailable_cores.clone(),
v2_descriptor: true,
candidate_modifier: Some(|mut candidate: CommittedCandidateReceiptV2| {
if candidate.descriptor.para_id() == 1.into() {
// Drop the core selector to make it invalid
candidate
.commitments
.upward_messages
.truncate(candidate.commitments.upward_messages.len() - 1);
}
candidate
}),
});

let unfiltered_para_inherent_data = scenario.data.clone();

// Check the para inherent data is as expected:
// * 1 bitfield per validator (5 validators per core, 10 backed candidates)
assert_eq!(unfiltered_para_inherent_data.bitfields.len(), 50);
// * 10 v2 candidate descriptors.
assert_eq!(unfiltered_para_inherent_data.backed_candidates.len(), 10);

let mut inherent_data = InherentData::new();
inherent_data
.put_data(PARACHAINS_INHERENT_IDENTIFIER, &unfiltered_para_inherent_data)
.unwrap();

let dispatch_error = Pallet::<Test>::enter(
frame_system::RawOrigin::None.into(),
unfiltered_para_inherent_data,
)
.unwrap_err()
.error;

// We expect `enter` to fail because the inherent data contains backed candidates with
// v2 descriptors.
assert_eq!(dispatch_error, Error::<Test>::CandidatesFilteredDuringExecution.into());
});
}
#[test]
fn v2_descriptors_are_accepted() {
let config = default_config();
Expand Down Expand Up @@ -1937,7 +2004,7 @@ mod enter {
});
}

// A test to ensure that the `paras_inherent`` filters out candidates with invalid
// A test to ensure that the `paras_inherent` filters out candidates with invalid
// session index in the descriptor.
#[test]
fn invalid_session_index() {
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/parachains/src/ump_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ mod check_upward_messages {
let max_per_candidate =
configuration::ActiveConfig::<Test>::get().max_upward_message_num_per_candidate;

for msg_size in 0..=max_size {
for msg_size in 1..=max_size {
check(P_0, vec![vec![0; msg_size as usize]], None);
}
for msg_size in max_size + 1..max_size + 10 {
Expand Down

0 comments on commit 9c4e2ae

Please sign in to comment.