Skip to content

Commit

Permalink
reject too large (by size) messages (paritytech#551)
Browse files Browse the repository at this point in the history
  • Loading branch information
svyatonik authored and serban300 committed Apr 9, 2024
1 parent eaa4592 commit 6f9cbfa
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 17 deletions.
2 changes: 1 addition & 1 deletion bridges/bin/millau/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ parameter_types! {
pub const ExtrinsicBaseWeight: Weight = 10_000_000;
pub const AvailableBlockRatio: Perbill = Perbill::from_percent(bp_millau::AVAILABLE_BLOCK_RATIO);
pub MaximumExtrinsicWeight: Weight = bp_millau::MAXIMUM_EXTRINSIC_WEIGHT;
pub const MaximumBlockLength: u32 = 5 * 1024 * 1024;
pub const MaximumBlockLength: u32 = bp_millau::MAXIMUM_BLOCK_SIZE;
pub const Version: RuntimeVersion = VERSION;
pub const DbWeight: RuntimeDbWeight = RuntimeDbWeight {
read: 60_000_000, // ~0.06 ms = ~60 µs
Expand Down
11 changes: 5 additions & 6 deletions bridges/bin/millau/runtime/src/rialto_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ impl MessageBridge for WithRialtoMessageBridge {
type ThisChain = Millau;
type BridgedChain = Rialto;

fn maximal_extrinsic_size_on_target_chain() -> u32 {
bp_rialto::MAXIMUM_EXTRINSIC_SIZE
}

fn weight_limits_of_message_on_bridged_chain(message_payload: &[u8]) -> RangeInclusive<Weight> {
// we don't want to relay too large messages + keep reserve for future upgrades
let upper_limit = bp_rialto::MAXIMUM_EXTRINSIC_WEIGHT / 2;
Expand Down Expand Up @@ -167,12 +171,7 @@ impl TargetHeaderChain<ToRialtoMessagePayload, bp_rialto::AccountId> for Rialto
type MessagesDeliveryProof = ToRialtoMessagesDeliveryProof;

fn verify_message(payload: &ToRialtoMessagePayload) -> Result<(), Self::Error> {
let weight_limits = WithRialtoMessageBridge::weight_limits_of_message_on_bridged_chain(&payload.call);
if !weight_limits.contains(&payload.weight) {
return Err("Incorrect message weight declared");
}

Ok(())
messages::source::verify_chain_message::<WithRialtoMessageBridge>(payload)
}

fn verify_messages_delivery_proof(
Expand Down
2 changes: 1 addition & 1 deletion bridges/bin/rialto/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ parameter_types! {
pub const ExtrinsicBaseWeight: Weight = 10_000_000;
pub const AvailableBlockRatio: Perbill = Perbill::from_percent(bp_rialto::AVAILABLE_BLOCK_RATIO);
pub MaximumExtrinsicWeight: Weight = bp_rialto::MAXIMUM_EXTRINSIC_WEIGHT;
pub const MaximumBlockLength: u32 = 5 * 1024 * 1024;
pub const MaximumBlockLength: u32 = bp_rialto::MAXIMUM_BLOCK_SIZE;
pub const Version: RuntimeVersion = VERSION;
pub const DbWeight: RuntimeDbWeight = RuntimeDbWeight {
read: 60_000_000, // ~0.06 ms = ~60 µs
Expand Down
11 changes: 5 additions & 6 deletions bridges/bin/rialto/runtime/src/millau_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ impl MessageBridge for WithMillauMessageBridge {
type ThisChain = Rialto;
type BridgedChain = Millau;

fn maximal_extrinsic_size_on_target_chain() -> u32 {
bp_millau::MAXIMUM_EXTRINSIC_SIZE
}

fn weight_limits_of_message_on_bridged_chain(message_payload: &[u8]) -> RangeInclusive<Weight> {
// we don't want to relay too large messages + keep reserve for future upgrades
let upper_limit = bp_millau::MAXIMUM_EXTRINSIC_WEIGHT / 2;
Expand Down Expand Up @@ -167,12 +171,7 @@ impl TargetHeaderChain<ToMillauMessagePayload, bp_millau::AccountId> for Millau
type MessagesDeliveryProof = ToMillauMessagesDeliveryProof;

fn verify_message(payload: &ToMillauMessagePayload) -> Result<(), Self::Error> {
let weight_limits = WithMillauMessageBridge::weight_limits_of_message_on_bridged_chain(&payload.call);
if !weight_limits.contains(&payload.weight) {
return Err("Incorrect message weight declared");
}

Ok(())
messages::source::verify_chain_message::<WithMillauMessageBridge>(payload)
}

fn verify_messages_delivery_proof(
Expand Down
110 changes: 107 additions & 3 deletions bridges/bin/runtime-common/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ pub trait MessageBridge {
/// Bridged chain in context of message bridge.
type BridgedChain: ChainWithMessageLanes;

/// Maximal extrinsic size on target chain.
fn maximal_extrinsic_size_on_target_chain() -> u32;

/// Returns feasible weights range for given message payload on the target chain.
///
/// If message is being sent with the weight that is out of this range, then it
Expand Down Expand Up @@ -98,7 +101,7 @@ pub trait ChainWithMessageLanes {
/// `frame_support::weight::Weight`. But since the meaning of weight on different chains
/// may be different, the `WeightOf<>` construct is used to avoid confusion between
/// different weights.
type Weight: From<frame_support::weights::Weight>;
type Weight: From<frame_support::weights::Weight> + PartialOrd;
/// Type of balances that is used on the chain.
type Balance: Decode + CheckedAdd + CheckedDiv + CheckedMul + PartialOrd + From<u32> + Copy;

Expand Down Expand Up @@ -171,6 +174,36 @@ pub mod source {
}
}

/// Do basic Bridged-chain specific verification of This -> Bridged chain message.
///
/// Ok result from this function means that the delivery transaction with this message
/// may be 'mined' by the target chain. But the lane may have its own checks (e.g. fee
/// check) that would reject message (see `FromThisChainMessageVerifier`).
pub fn verify_chain_message<B: MessageBridge>(
payload: &FromThisChainMessagePayload<B>,
) -> Result<(), &'static str> {
let weight_limits = B::weight_limits_of_message_on_bridged_chain(&payload.call);
if !weight_limits.contains(&payload.weight.into()) {
return Err("Incorrect message weight declared");
}

// The maximal size of extrinsic at Substrate-based chain depends on the
// `frame_system::Trait::MaximumBlockLength` and `frame_system::Trait::AvailableBlockRatio`
// constants. This check is here to be sure that the lane won't stuck because message is too
// large to fit into delivery transaction.
//
// **IMPORTANT NOTE**: the delivery transaction contains storage proof of the message, not
// the message itself. The proof is always larger than the message. But unless chain state
// is enormously large, it should be several dozens/hundreds of bytes. The delivery
// transaction also contains signatures and signed extensions. Because of this, we reserve
// 1/3 of the the maximal extrinsic weight for this data.
if payload.call.len() > B::maximal_extrinsic_size_on_target_chain() as usize * 2 / 3 {
return Err("The message is too large to be sent over the lane");
}

Ok(())
}

/// Estimate delivery and dispatch fee that must be paid for delivering a message to the Bridged chain.
///
/// The fee is paid in This chain Balance, but we use Bridged chain balance to avoid additional conversions.
Expand Down Expand Up @@ -511,6 +544,8 @@ mod tests {
const THIS_CHAIN_WEIGHT_TO_BALANCE_RATE: Weight = 2;
const BRIDGED_CHAIN_WEIGHT_TO_BALANCE_RATE: Weight = 4;
const THIS_CHAIN_TO_BRIDGED_CHAIN_BALANCE_RATE: u32 = 6;
const BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT: Weight = 2048;
const BRIDGED_CHAIN_MAX_EXTRINSIC_SIZE: u32 = 1024;

/// Bridge that is deployed on ThisChain and allows sending/receiving messages to/from BridgedChain;
struct OnThisChainBridge;
Expand All @@ -522,8 +557,13 @@ mod tests {
type ThisChain = ThisChain;
type BridgedChain = BridgedChain;

fn weight_limits_of_message_on_bridged_chain(_message_payload: &[u8]) -> RangeInclusive<Weight> {
unreachable!()
fn maximal_extrinsic_size_on_target_chain() -> u32 {
BRIDGED_CHAIN_MAX_EXTRINSIC_SIZE
}

fn weight_limits_of_message_on_bridged_chain(message_payload: &[u8]) -> RangeInclusive<Weight> {
let begin = std::cmp::min(BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT, message_payload.len() as Weight);
begin..=BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT
}

fn weight_of_delivery_transaction() -> Weight {
Expand Down Expand Up @@ -561,6 +601,10 @@ mod tests {
type ThisChain = BridgedChain;
type BridgedChain = ThisChain;

fn maximal_extrinsic_size_on_target_chain() -> u32 {
unreachable!()
}

fn weight_limits_of_message_on_bridged_chain(_message_payload: &[u8]) -> RangeInclusive<Weight> {
unreachable!()
}
Expand Down Expand Up @@ -773,6 +817,66 @@ mod tests {
);
}

#[test]
fn verify_chain_message_rejects_message_with_too_small_declared_weight() {
assert!(
source::verify_chain_message::<OnThisChainBridge>(&source::FromThisChainMessagePayload::<
OnThisChainBridge,
> {
spec_version: 1,
weight: 5,
origin: pallet_bridge_call_dispatch::CallOrigin::SourceRoot,
call: vec![1, 2, 3, 4, 5, 6],
},)
.is_err()
);
}

#[test]
fn verify_chain_message_rejects_message_with_too_large_declared_weight() {
assert!(
source::verify_chain_message::<OnThisChainBridge>(&source::FromThisChainMessagePayload::<
OnThisChainBridge,
> {
spec_version: 1,
weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT + 1,
origin: pallet_bridge_call_dispatch::CallOrigin::SourceRoot,
call: vec![1, 2, 3, 4, 5, 6],
},)
.is_err()
);
}

#[test]
fn verify_chain_message_rejects_message_too_large_message() {
assert!(
source::verify_chain_message::<OnThisChainBridge>(&source::FromThisChainMessagePayload::<
OnThisChainBridge,
> {
spec_version: 1,
weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT,
origin: pallet_bridge_call_dispatch::CallOrigin::SourceRoot,
call: vec![0; BRIDGED_CHAIN_MAX_EXTRINSIC_SIZE as usize * 2 / 3 + 1],
},)
.is_err()
);
}

#[test]
fn verify_chain_message_accepts_maximal_message() {
assert_eq!(
source::verify_chain_message::<OnThisChainBridge>(&source::FromThisChainMessagePayload::<
OnThisChainBridge,
> {
spec_version: 1,
weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT,
origin: pallet_bridge_call_dispatch::CallOrigin::SourceRoot,
call: vec![0; BRIDGED_CHAIN_MAX_EXTRINSIC_SIZE as usize * 2 / 3],
},),
Ok(()),
);
}

#[derive(Debug)]
struct TestMessageProofParser {
failing: bool,
Expand Down
4 changes: 4 additions & 0 deletions bridges/primitives/millau/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ pub const AVAILABLE_BLOCK_RATIO: u32 = 75;
/// Maximal weight of single Millau extrinsic (65% of maximum block weight = 75% for regular
/// transactions minus 10% for initialization).
pub const MAXIMUM_EXTRINSIC_WEIGHT: Weight = MAXIMUM_BLOCK_WEIGHT / 100 * (AVAILABLE_BLOCK_RATIO as Weight - 10);
/// Maximal size of Millau block.
pub const MAXIMUM_BLOCK_SIZE: u32 = 2 * 1024 * 1024;
/// Maximal size of single normal Millau extrinsic (75% of maximal block size).
pub const MAXIMUM_EXTRINSIC_SIZE: u32 = MAXIMUM_BLOCK_SIZE / 100 * AVAILABLE_BLOCK_RATIO;

// TODO: may need to be updated after https://github.com/paritytech/parity-bridges-common/issues/78
/// Maximal number of messages in single delivery transaction.
Expand Down
4 changes: 4 additions & 0 deletions bridges/primitives/rialto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ pub const AVAILABLE_BLOCK_RATIO: u32 = 75;
/// Maximal weight of single Rialto extrinsic (65% of maximum block weight = 75% for regular
/// transactions minus 10% for initialization).
pub const MAXIMUM_EXTRINSIC_WEIGHT: Weight = MAXIMUM_BLOCK_WEIGHT / 100 * (AVAILABLE_BLOCK_RATIO as Weight - 10);
/// Maximal size of Rialto block.
pub const MAXIMUM_BLOCK_SIZE: u32 = 5 * 1024 * 1024;
/// Maximal size of single normal Rialto extrinsic (75% of maximal block size).
pub const MAXIMUM_EXTRINSIC_SIZE: u32 = MAXIMUM_BLOCK_SIZE / 100 * AVAILABLE_BLOCK_RATIO;

// TODO: may need to be updated after https://github.com/paritytech/parity-bridges-common/issues/78
/// Maximal number of messages in single delivery transaction.
Expand Down

0 comments on commit 6f9cbfa

Please sign in to comment.