Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dmp: Check that the para exist before delivering a message #6604

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
3 changes: 3 additions & 0 deletions polkadot/runtime/common/src/paras_sudo_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ pub mod pallet {
/// A DMP message couldn't be sent because it exceeds the maximum size allowed for a
/// downward message.
ExceedsMaxMessageSize,
/// A DMP message couldn't be sent because the destination is unreachable.
Unroutable,
/// Could not schedule para cleanup.
CouldntCleanup,
/// Not a parathread (on-demand parachain).
Expand Down Expand Up @@ -152,6 +154,7 @@ pub mod pallet {
{
dmp::QueueDownwardMessageError::ExceedsMaxMessageSize =>
Error::<T>::ExceedsMaxMessageSize.into(),
dmp::QueueDownwardMessageError::Unroutable => Error::<T>::Unroutable.into(),
})
}

Expand Down
8 changes: 7 additions & 1 deletion polkadot/runtime/common/src/xcm_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ mod tests {
use crate::integration_tests::new_test_ext;
use alloc::vec;
use frame_support::{assert_ok, parameter_types};
use polkadot_runtime_parachains::FeeTracker;
use polkadot_primitives::HeadData;
use polkadot_runtime_parachains::{paras, FeeTracker};
use sp_runtime::FixedU128;
use xcm::MAX_XCM_DECODE_DEPTH;

Expand Down Expand Up @@ -349,6 +350,11 @@ mod tests {
c.max_downward_message_size = u32::MAX;
});

paras::Heads::<crate::integration_tests::Test>::insert(
ParaId::from(5555),
HeadData(vec![]),
);

// Check that the good message is validated:
assert_ok!(<Router as SendXcm>::validate(
&mut Some(dest.into()),
Expand Down
17 changes: 15 additions & 2 deletions polkadot/runtime/parachains/src/dmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

use crate::{
configuration::{self, HostConfiguration},
initializer, FeeTracker,
initializer, paras, FeeTracker,
};
use alloc::vec::Vec;
use core::fmt;
Expand Down Expand Up @@ -72,12 +72,15 @@ const MESSAGE_SIZE_FEE_BASE: FixedU128 = FixedU128::from_rational(1, 1000); // 0
pub enum QueueDownwardMessageError {
/// The message being sent exceeds the configured max message size.
ExceedsMaxMessageSize,
/// The destination is unknown.
Unroutable,
}

impl From<QueueDownwardMessageError> for SendError {
fn from(err: QueueDownwardMessageError) -> Self {
match err {
QueueDownwardMessageError::ExceedsMaxMessageSize => SendError::ExceedsMaxMessageSize,
QueueDownwardMessageError::Unroutable => SendError::Unroutable,
}
}
}
Expand Down Expand Up @@ -116,7 +119,7 @@ pub mod pallet {
pub struct Pallet<T>(_);

#[pallet::config]
pub trait Config: frame_system::Config + configuration::Config {}
pub trait Config: frame_system::Config + configuration::Config + paras::Config {}

/// The downward messages addressed for a certain para.
#[pallet::storage]
Expand Down Expand Up @@ -200,6 +203,11 @@ impl<T: Config> Pallet<T> {
return Err(QueueDownwardMessageError::ExceedsMaxMessageSize)
}

// If the head exists, we assume the parachain is legit and exists.
if !paras::Heads::<T>::contains_key(para) {
return Err(QueueDownwardMessageError::Unroutable)
}

Ok(())
}

Expand All @@ -226,6 +234,11 @@ impl<T: Config> Pallet<T> {
return Err(QueueDownwardMessageError::ExceedsMaxMessageSize)
}

// If the head exists, we assume the parachain is legit and exists.
if !paras::Heads::<T>::contains_key(para) {
return Err(QueueDownwardMessageError::Unroutable)
}

bkontur marked this conversation as resolved.
Show resolved Hide resolved
let inbound =
InboundDownwardMessage { msg, sent_at: frame_system::Pallet::<T>::block_number() };

Expand Down
46 changes: 45 additions & 1 deletion polkadot/runtime/parachains/src/dmp/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
use codec::Encode;
use frame_support::assert_ok;
use hex_literal::hex;
use polkadot_primitives::BlockNumber;
use polkadot_primitives::{BlockNumber, HeadData};

pub(crate) fn run_to_block(to: BlockNumber, new_session: Option<Vec<BlockNumber>>) {
while System::block_number() < to {
Expand Down Expand Up @@ -61,13 +61,21 @@ fn queue_downward_message(
Dmp::queue_downward_message(&configuration::ActiveConfig::<Test>::get(), para_id, msg)
}

fn register_paras(paras: &[ParaId]) {
paras.iter().for_each(|p| {
paras::Heads::<Test>::insert(p, HeadData(p.encode().into()));
});
}

#[test]
fn clean_dmp_works() {
let a = ParaId::from(1312);
let b = ParaId::from(228);
let c = ParaId::from(123);

new_test_ext(default_genesis_config()).execute_with(|| {
register_paras(&[a, b, c]);

// enqueue downward messages to A, B and C.
queue_downward_message(a, vec![1, 2, 3]).unwrap();
queue_downward_message(b, vec![4, 5, 6]).unwrap();
Expand All @@ -89,6 +97,8 @@ fn dmq_length_and_head_updated_properly() {
let b = ParaId::from(228);

new_test_ext(default_genesis_config()).execute_with(|| {
register_paras(&[a, b]);

assert_eq!(Dmp::dmq_length(a), 0);
assert_eq!(Dmp::dmq_length(b), 0);

Expand All @@ -101,11 +111,30 @@ fn dmq_length_and_head_updated_properly() {
});
}

#[test]
fn dmq_fail_if_para_does_not_exist() {
let a = ParaId::from(1312);

new_test_ext(default_genesis_config()).execute_with(|| {
assert_eq!(Dmp::dmq_length(a), 0);

assert!(matches!(
queue_downward_message(a, vec![1, 2, 3]),
Err(QueueDownwardMessageError::Unroutable)
));

assert_eq!(Dmp::dmq_length(a), 0);
assert!(Dmp::dmq_mqc_head(a).is_zero());
});
}

#[test]
fn dmp_mqc_head_fixture() {
let a = ParaId::from(2000);

new_test_ext(default_genesis_config()).execute_with(|| {
register_paras(&[a]);

run_to_block(2, None);
assert!(Dmp::dmq_mqc_head(a).is_zero());
queue_downward_message(a, vec![1, 2, 3]).unwrap();
Expand All @@ -125,6 +154,8 @@ fn check_processed_downward_messages() {
let a = ParaId::from(1312);

new_test_ext(default_genesis_config()).execute_with(|| {
register_paras(&[a]);

let block_number = System::block_number();

// processed_downward_messages=0 is allowed when the DMQ is empty.
Expand All @@ -150,6 +181,8 @@ fn check_processed_downward_messages_advancement_rule() {
let a = ParaId::from(1312);

new_test_ext(default_genesis_config()).execute_with(|| {
register_paras(&[a]);

let block_number = System::block_number();

run_to_block(block_number + 1, None);
Expand All @@ -170,6 +203,8 @@ fn dmq_pruning() {
let a = ParaId::from(1312);

new_test_ext(default_genesis_config()).execute_with(|| {
register_paras(&[a]);
bkchr marked this conversation as resolved.
Show resolved Hide resolved

assert_eq!(Dmp::dmq_length(a), 0);

queue_downward_message(a, vec![1, 2, 3]).unwrap();
Expand All @@ -194,6 +229,8 @@ fn queue_downward_message_critical() {
genesis.configuration.config.max_downward_message_size = 7;

new_test_ext(genesis).execute_with(|| {
register_paras(&[a]);

let smol = [0; 3].to_vec();
let big = [0; 8].to_vec();

Expand All @@ -215,6 +252,8 @@ fn verify_dmq_mqc_head_is_externally_accessible() {
let a = ParaId::from(2020);

new_test_ext(default_genesis_config()).execute_with(|| {
register_paras(&[a]);

let head = sp_io::storage::get(&well_known_keys::dmq_mqc_head(a));
assert_eq!(head, None);

Expand All @@ -235,9 +274,12 @@ fn verify_dmq_mqc_head_is_externally_accessible() {
#[test]
fn verify_fee_increase_and_decrease() {
let a = ParaId::from(123);

let mut genesis = default_genesis_config();
genesis.configuration.config.max_downward_message_size = 16777216;
new_test_ext(genesis).execute_with(|| {
register_paras(&[a]);

let initial = InitialFactor::get();
assert_eq!(DeliveryFeeFactor::<Test>::get(a), initial);

Expand Down Expand Up @@ -287,6 +329,8 @@ fn verify_fee_factor_reaches_high_value() {
let mut genesis = default_genesis_config();
genesis.configuration.config.max_downward_message_size = 51200;
new_test_ext(genesis).execute_with(|| {
register_paras(&[a]);

let max_messages =
Dmp::dmq_max_length(ActiveConfig::<Test>::get().max_downward_message_size);
let mut total_fee_factor = FixedU128::from_float(1.0);
Expand Down
9 changes: 9 additions & 0 deletions prdoc/pr_6604.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
title: 'dmp: Check that the para exist before delivering a message'
doc:
- audience: Runtime Dev
description: Or otherwise throw an error.
crates:
- name: polkadot-runtime-parachains
bump: major
bkchr marked this conversation as resolved.
Show resolved Hide resolved
- name: polkadot-runtime-common
bump: major
Loading