Skip to content

Commit

Permalink
Signed extension for rejecting obsolete messages pallet transactions (p…
Browse files Browse the repository at this point in the history
…aritytech#1446)

* BridgeRejectObsoleteMessages

* add obsolete confirmations verification to the BridgeRejectObsoleteMessages

* move tests where they belong
  • Loading branch information
svyatonik authored and serban300 committed Apr 9, 2024
1 parent ba0efa9 commit 32123f7
Show file tree
Hide file tree
Showing 13 changed files with 345 additions and 10 deletions.
10 changes: 10 additions & 0 deletions bridges/bin/millau/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ use sp_std::prelude::*;
use sp_version::NativeVersion;
use sp_version::RuntimeVersion;

// to be able to use Millau runtime in `bridge-runtime-common` tests
pub use bridge_runtime_common;

// A few exports that help ease life for downstream crates.
pub use frame_support::{
construct_runtime, parameter_types,
Expand Down Expand Up @@ -570,6 +573,12 @@ pallet_bridge_parachains::declare_bridge_reject_obsolete_parachain_header! {
Call::BridgeRialtoParachains => WithRialtoParachainsInstance
}

bridge_runtime_common::declare_bridge_reject_obsolete_messages! {
Runtime,
Call::BridgeRialtoMessages => WithRialtoMessagesInstance,
Call::BridgeRialtoParachainMessages => WithRialtoParachainMessagesInstance
}

/// The address format for describing accounts.
pub type Address = AccountId;
/// Block header type as expected by this runtime.
Expand All @@ -592,6 +601,7 @@ pub type SignedExtra = (
pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
BridgeRejectObsoleteGrandpaHeader,
BridgeRejectObsoleteParachainHeader,
BridgeRejectObsoleteMessages,
);
/// The payload being signed in transactions.
pub type SignedPayload = generic::SignedPayload<Call, SignedExtra>;
Expand Down
3 changes: 3 additions & 0 deletions bridges/bin/runtime-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ xcm = { git = "https://github.com/paritytech/polkadot", branch = "gav-xcm-v3", d
xcm-builder = { git = "https://github.com/paritytech/polkadot", branch = "gav-xcm-v3", default-features = false }
xcm-executor = { git = "https://github.com/paritytech/polkadot", branch = "gav-xcm-v3", default-features = false }

[dev-dependencies]
millau-runtime = { path = "../millau/runtime" }

[features]
default = ["std"]
std = [
Expand Down
1 change: 1 addition & 0 deletions bridges/bin/runtime-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
pub mod messages;
pub mod messages_api;
pub mod messages_benchmarking;
pub mod messages_extension;
pub mod parachains_benchmarking;

#[cfg(feature = "integrity-test")]
Expand Down
4 changes: 1 addition & 3 deletions bridges/bin/runtime-common/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,6 @@ pub fn transaction_payment<Balance: AtLeast32BitUnsigned + FixedPointOperand>(
pub mod source {
use super::*;

/// Encoded Call of the Bridged chain. We never try to decode it on This chain.
pub type BridgedChainOpaqueCall = Vec<u8>;

/// Message payload for This -> Bridged chain messages.
pub type FromThisChainMessagePayload = Vec<u8>;

Expand Down Expand Up @@ -523,6 +520,7 @@ pub mod target {
pub bridged_header_hash: BridgedHeaderHash,
/// A storage trie proof of messages being delivered.
pub storage_proof: RawStorageProof,
/// Messages in this proof are sent over this lane.
pub lane: LaneId,
/// Nonce of the first message being delivered.
pub nonces_start: MessageNonce,
Expand Down
261 changes: 261 additions & 0 deletions bridges/bin/runtime-common/src/messages_extension.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
// Copyright 2021 Parity Technologies (UK) Ltd.
// This file is part of Parity Bridges Common.

// Parity Bridges Common is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Parity Bridges Common is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>.

/// Declares a runtime-specific `BridgeRejectObsoleteMessages` and
/// `BridgeRejectObsoleteMessageConfirmations` signed extensions.
///
/// ## Example
///
/// ```nocompile
/// bridge_runtime_common::declare_bridge_reject_obsolete_messages!{
/// Runtime,
/// Call::BridgeRialtoMessages => WithRialtoMessagesInstance,
/// Call::BridgeRialtoParachainMessages => WithRialtoParachainMessagesInstance,
/// }
/// ```
///
/// The goal of this extension is to avoid "mining" messages delivery and delivery confirmation
/// transactions, that are delivering outdated messages/confirmations. Without that extension,
/// even honest relayers may lose their funds if there are multiple relays running and submitting
/// the same messages/confirmations.
#[macro_export]
macro_rules! declare_bridge_reject_obsolete_messages {
($runtime:ident, $($call:path => $instance:ty),*) => {
/// Transaction-with-obsolete-messages check that will reject transaction if
/// it submits obsolete messages/confirmations.
#[derive(Clone, codec::Decode, codec::Encode, Eq, PartialEq, frame_support::RuntimeDebug, scale_info::TypeInfo)]
pub struct BridgeRejectObsoleteMessages;

impl sp_runtime::traits::SignedExtension for BridgeRejectObsoleteMessages {
const IDENTIFIER: &'static str = "BridgeRejectObsoleteMessages";
type AccountId = <$runtime as frame_system::Config>::AccountId;
type Call = <$runtime as frame_system::Config>::Call;
type AdditionalSigned = ();
type Pre = ();

fn additional_signed(&self) -> sp_std::result::Result<
(),
sp_runtime::transaction_validity::TransactionValidityError,
> {
Ok(())
}

fn validate(
&self,
_who: &Self::AccountId,
call: &Self::Call,
_info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
_len: usize,
) -> sp_runtime::transaction_validity::TransactionValidity {
match *call {
$(
$call(pallet_bridge_messages::Call::<$runtime, $instance>::receive_messages_proof {
ref proof,
..
}) => {
let nonces_end = proof.nonces_end;

let inbound_lane_data = pallet_bridge_messages::InboundLanes::<$runtime, $instance>::get(&proof.lane);
if proof.nonces_end <= inbound_lane_data.last_delivered_nonce() {
return sp_runtime::transaction_validity::InvalidTransaction::Stale.into();
}

Ok(sp_runtime::transaction_validity::ValidTransaction::default())
},
$call(pallet_bridge_messages::Call::<$runtime, $instance>::receive_messages_delivery_proof {
ref proof,
ref relayers_state,
..
}) => {
let latest_delivered_nonce = relayers_state.last_delivered_nonce;

let outbound_lane_data = pallet_bridge_messages::OutboundLanes::<$runtime, $instance>::get(&proof.lane);
if latest_delivered_nonce <= outbound_lane_data.latest_received_nonce {
return sp_runtime::transaction_validity::InvalidTransaction::Stale.into();
}

Ok(sp_runtime::transaction_validity::ValidTransaction::default())
}
)*
_ => Ok(sp_runtime::transaction_validity::ValidTransaction::default()),
}
}

fn pre_dispatch(
self,
who: &Self::AccountId,
call: &Self::Call,
info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
len: usize,
) -> Result<Self::Pre, sp_runtime::transaction_validity::TransactionValidityError> {
self.validate(who, call, info, len).map(drop)
}

fn post_dispatch(
_maybe_pre: Option<Self::Pre>,
_info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
_post_info: &sp_runtime::traits::PostDispatchInfoOf<Self::Call>,
_len: usize,
_result: &sp_runtime::DispatchResult,
) -> Result<(), sp_runtime::transaction_validity::TransactionValidityError> {
Ok(())
}
}
};
}

#[cfg(test)]
mod tests {
use bp_messages::UnrewardedRelayersState;
use frame_support::weights::{DispatchClass, DispatchInfo, Pays};
use millau_runtime::{
bridge_runtime_common::messages::{
source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof,
},
BridgeRejectObsoleteMessages, Call, Runtime, WithRialtoMessagesInstance,
};
use sp_runtime::traits::SignedExtension;

fn deliver_message_10() {
pallet_bridge_messages::InboundLanes::<Runtime, WithRialtoMessagesInstance>::insert(
[0, 0, 0, 0],
bp_messages::InboundLaneData { relayers: Default::default(), last_confirmed_nonce: 10 },
);
}

fn validate_message_delivery(
nonces_start: bp_messages::MessageNonce,
nonces_end: bp_messages::MessageNonce,
) -> bool {
BridgeRejectObsoleteMessages
.validate(
&[0u8; 32].into(),
&Call::BridgeRialtoMessages(pallet_bridge_messages::Call::<
Runtime,
WithRialtoMessagesInstance,
>::receive_messages_proof {
relayer_id_at_bridged_chain: [0u8; 32].into(),
messages_count: (nonces_end - nonces_start + 1) as u32,
dispatch_weight: 0,
proof: FromBridgedChainMessagesProof {
bridged_header_hash: Default::default(),
storage_proof: vec![],
lane: [0, 0, 0, 0],
nonces_start,
nonces_end,
},
}),
&DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: Pays::Yes },
0,
)
.is_ok()
}

#[test]
fn extension_rejects_obsolete_messages() {
sp_io::TestExternalities::new(Default::default()).execute_with(|| {
// when current best delivered is message#10 and we're trying to deliver message#5 => tx
// is rejected
deliver_message_10();
assert!(!validate_message_delivery(8, 9));
});
}

#[test]
fn extension_rejects_same_message() {
sp_io::TestExternalities::new(Default::default()).execute_with(|| {
// when current best delivered is message#10 and we're trying to import message#10 => tx
// is rejected
deliver_message_10();
assert!(!validate_message_delivery(8, 10));
});
}

#[test]
fn extension_accepts_new_message() {
sp_io::TestExternalities::new(Default::default()).execute_with(|| {
// when current best delivered is message#10 and we're trying to deliver message#15 =>
// tx is accepted
deliver_message_10();
assert!(validate_message_delivery(10, 15));
});
}

fn confirm_message_10() {
pallet_bridge_messages::OutboundLanes::<Runtime, WithRialtoMessagesInstance>::insert(
[0, 0, 0, 0],
bp_messages::OutboundLaneData {
oldest_unpruned_nonce: 0,
latest_received_nonce: 10,
latest_generated_nonce: 10,
},
);
}

fn validate_message_confirmation(last_delivered_nonce: bp_messages::MessageNonce) -> bool {
BridgeRejectObsoleteMessages
.validate(
&[0u8; 32].into(),
&Call::BridgeRialtoMessages(pallet_bridge_messages::Call::<
Runtime,
WithRialtoMessagesInstance,
>::receive_messages_delivery_proof {
proof: FromBridgedChainMessagesDeliveryProof {
bridged_header_hash: Default::default(),
storage_proof: Vec::new(),
lane: [0, 0, 0, 0],
},
relayers_state: UnrewardedRelayersState {
last_delivered_nonce,
..Default::default()
},
}),
&DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: Pays::Yes },
0,
)
.is_ok()
}

#[test]
fn extension_rejects_obsolete_confirmations() {
sp_io::TestExternalities::new(Default::default()).execute_with(|| {
// when current best confirmed is message#10 and we're trying to confirm message#5 => tx
// is rejected
confirm_message_10();
assert!(!validate_message_confirmation(5));
});
}

#[test]
fn extension_rejects_same_confirmation() {
sp_io::TestExternalities::new(Default::default()).execute_with(|| {
// when current best confirmed is message#10 and we're trying to confirm message#10 =>
// tx is rejected
confirm_message_10();
assert!(!validate_message_confirmation(10));
});
}

#[test]
fn extension_accepts_new_confirmation() {
sp_io::TestExternalities::new(Default::default()).execute_with(|| {
// when current best confirmed is message#10 and we're trying to confirm message#15 =>
// tx is accepted
confirm_message_10();
assert!(validate_message_confirmation(15));
});
}
}
3 changes: 3 additions & 0 deletions bridges/modules/messages/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,7 @@ benchmarks_instance_pallet! {
unrewarded_relayer_entries: 1,
messages_in_oldest_entry: 1,
total_messages: 1,
last_delivered_nonce: 1,
};
let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams {
lane: T::bench_lane_id(),
Expand Down Expand Up @@ -534,6 +535,7 @@ benchmarks_instance_pallet! {
unrewarded_relayer_entries: 1,
messages_in_oldest_entry: 2,
total_messages: 2,
last_delivered_nonce: 2,
};
let mut delivered_messages = DeliveredMessages::new(1, true);
delivered_messages.note_dispatched_message(true);
Expand Down Expand Up @@ -576,6 +578,7 @@ benchmarks_instance_pallet! {
unrewarded_relayer_entries: 2,
messages_in_oldest_entry: 1,
total_messages: 2,
last_delivered_nonce: 2,
};
let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams {
lane: T::bench_lane_id(),
Expand Down
Loading

0 comments on commit 32123f7

Please sign in to comment.