From 3d0e110c3fe6816ed690a153f5fd6f727c99ead6 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 7 Apr 2023 09:55:06 +0300 Subject: [PATCH] fail with InsufficientDispatchWeight if dispatch_weight doesn't cover weight of all bundled messages (#2018) --- bridges/modules/grandpa/src/lib.rs | 10 +++ bridges/modules/messages/src/lib.rs | 73 +++++++++++-------- bridges/modules/parachains/src/lib.rs | 10 +++ bridges/primitives/messages/src/lib.rs | 8 +- .../src/codegen_runtime.rs | 6 +- 5 files changed, 67 insertions(+), 40 deletions(-) diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index df51aa1eb7ed6..9d38c9723d7ac 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -160,6 +160,16 @@ pub mod pallet { /// /// If successful in verification, it will write the target header to the underlying storage /// pallet. + /// + /// The call fails if: + /// + /// - the pallet is halted; + /// + /// - the pallet knows better header than the `finality_target`; + /// + /// - verification is not optimized or invalid; + /// + /// - header contains forced authorities set change or change with non-zero delay. #[pallet::call_index(0)] #[pallet::weight(::submit_finality_proof( justification.commit.precommits.len().saturated_into(), diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index 3faf00b9e75bf..f7bc7d8a109f9 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -249,6 +249,22 @@ pub mod pallet { /// The weight of the call assumes that the transaction always brings outbound lane /// state update. Because of that, the submitter (relayer) has no benefit of not including /// this data in the transaction, so reward confirmations lags should be minimal. + /// + /// The call fails if: + /// + /// - the pallet is halted; + /// + /// - the call origin is not `Signed(_)`; + /// + /// - there are too many messages in the proof; + /// + /// - the proof verification procedure returns an error - e.g. because header used to craft + /// proof is not imported by the associated finality pallet; + /// + /// - the `dispatch_weight` argument is not sufficient to dispatch all bundled messages. + /// + /// The call may succeed, but some messages may not be delivered e.g. if they are not fit + /// into the unrewarded relayers vector. #[pallet::call_index(2)] #[pallet::weight(T::WeightInfo::receive_messages_proof_weight(proof, *messages_count, *dispatch_weight))] pub fn receive_messages_proof( @@ -324,18 +340,10 @@ pub mod pallet { let mut lane_messages_received_status = ReceivedMessages::new(lane_id, Vec::with_capacity(lane_data.messages.len())); - let mut is_lane_processing_stopped_no_weight_left = false; - for mut message in lane_data.messages { debug_assert_eq!(message.key.lane_id, lane_id); total_messages += 1; - if is_lane_processing_stopped_no_weight_left { - lane_messages_received_status - .push_skipped_for_not_enough_weight(message.key.nonce); - continue - } - // ensure that relayer has declared enough weight for dispatching next message // on this lane. We can't dispatch lane messages out-of-order, so if declared // weight is not enough, let's move to next lane @@ -348,10 +356,8 @@ pub mod pallet { message_dispatch_weight, dispatch_weight_left, ); - lane_messages_received_status - .push_skipped_for_not_enough_weight(message.key.nonce); - is_lane_processing_stopped_no_weight_left = true; - continue + + fail!(Error::::InsufficientDispatchWeight); } let receival_result = lane.receive_message::( @@ -554,8 +560,9 @@ pub mod pallet { /// The relayer has declared invalid unrewarded relayers state in the /// `receive_messages_delivery_proof` call. InvalidUnrewardedRelayersState, - /// The message someone is trying to work with (i.e. increase fee) is already-delivered. - MessageIsAlreadyDelivered, + /// The cumulative dispatch weight, passed by relayer is not enough to cover dispatch + /// of all bundled messages. + InsufficientDispatchWeight, /// The message someone is trying to work with (i.e. increase fee) is not yet sent. MessageIsNotYetSent, /// The number of actually confirmed messages is going to be larger than the number of @@ -1277,13 +1284,16 @@ mod tests { run_test(|| { let mut declared_weight = REGULAR_PAYLOAD.declared_weight; *declared_weight.ref_time_mut() -= 1; - assert_ok!(Pallet::::receive_messages_proof( - RuntimeOrigin::signed(1), - TEST_RELAYER_A, - Ok(vec![message(1, REGULAR_PAYLOAD)]).into(), - 1, - declared_weight, - )); + assert_noop!( + Pallet::::receive_messages_proof( + RuntimeOrigin::signed(1), + TEST_RELAYER_A, + Ok(vec![message(1, REGULAR_PAYLOAD)]).into(), + 1, + declared_weight, + ), + Error::::InsufficientDispatchWeight + ); assert_eq!(InboundLanes::::get(TEST_LANE_ID).last_delivered_nonce(), 0); }); } @@ -1541,15 +1551,18 @@ mod tests { let message2 = message(2, message_payload(0, u64::MAX / 2)); let message3 = message(3, message_payload(0, u64::MAX / 2)); - assert_ok!(Pallet::::receive_messages_proof( - RuntimeOrigin::signed(1), - TEST_RELAYER_A, - // this may cause overflow if source chain storage is invalid - Ok(vec![message1, message2, message3]).into(), - 3, - Weight::MAX, - )); - assert_eq!(InboundLanes::::get(TEST_LANE_ID).last_delivered_nonce(), 2); + assert_noop!( + Pallet::::receive_messages_proof( + RuntimeOrigin::signed(1), + TEST_RELAYER_A, + // this may cause overflow if source chain storage is invalid + Ok(vec![message1, message2, message3]).into(), + 3, + Weight::MAX, + ), + Error::::InsufficientDispatchWeight + ); + assert_eq!(InboundLanes::::get(TEST_LANE_ID).last_delivered_nonce(), 0); }); } diff --git a/bridges/modules/parachains/src/lib.rs b/bridges/modules/parachains/src/lib.rs index 8fe6c4e938391..6c89b09513cd6 100644 --- a/bridges/modules/parachains/src/lib.rs +++ b/bridges/modules/parachains/src/lib.rs @@ -294,6 +294,16 @@ pub mod pallet { /// `polkadot-runtime-parachains::paras` pallet instance, deployed at the bridged chain. /// The proof is supposed to be crafted at the `relay_header_hash` that must already be /// imported by corresponding GRANDPA pallet at this chain. + /// + /// The call fails if: + /// + /// - the pallet is halted; + /// + /// - the relay chain block `at_relay_block` is not imported by the associated bridge + /// GRANDPA pallet. + /// + /// The call may succeed, but some heads may not be updated e.g. because pallet knows + /// better head or it isn't tracked by the pallet. #[pallet::call_index(0)] #[pallet::weight(WeightInfoOf::::submit_parachain_heads_weight( T::DbWeight::get(), diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs index 754349d634eef..8c10989d62011 100644 --- a/bridges/primitives/messages/src/lib.rs +++ b/bridges/primitives/messages/src/lib.rs @@ -238,8 +238,6 @@ pub struct ReceivedMessages { pub lane: LaneId, /// Result of messages which we tried to dispatch pub receive_results: Vec<(MessageNonce, ReceivalResult)>, - /// Messages which were skipped and never dispatched - pub skipped_for_not_enough_weight: Vec, } impl ReceivedMessages { @@ -247,16 +245,12 @@ impl ReceivedMessages { lane: LaneId, receive_results: Vec<(MessageNonce, ReceivalResult)>, ) -> Self { - ReceivedMessages { lane, receive_results, skipped_for_not_enough_weight: Vec::new() } + ReceivedMessages { lane, receive_results } } pub fn push(&mut self, message: MessageNonce, result: ReceivalResult) { self.receive_results.push((message, result)); } - - pub fn push_skipped_for_not_enough_weight(&mut self, message: MessageNonce) { - self.skipped_for_not_enough_weight.push(message); - } } /// Result of single message receival. diff --git a/bridges/relays/client-rialto-parachain/src/codegen_runtime.rs b/bridges/relays/client-rialto-parachain/src/codegen_runtime.rs index 893bca1e27eff..3ea4a0ac2b5a0 100644 --- a/bridges/relays/client-rialto-parachain/src/codegen_runtime.rs +++ b/bridges/relays/client-rialto-parachain/src/codegen_runtime.rs @@ -6043,7 +6043,6 @@ pub mod api { ::core::primitive::u64, runtime_types::bp_messages::ReceivalResult<_0>, )>, - pub skipped_for_not_enough_weight: ::std::vec::Vec<::core::primitive::u64>, } #[derive( :: subxt :: ext :: codec :: Decode, :: subxt :: ext :: codec :: Encode, Clone, Debug, @@ -7508,8 +7507,9 @@ pub mod api { #[doc = "`receive_messages_delivery_proof` call."] InvalidUnrewardedRelayersState, #[codec(index = 11)] - #[doc = "The message someone is trying to work with (i.e. increase fee) is already-delivered."] - MessageIsAlreadyDelivered, + #[doc = "The cumulative dispatch weight, passed by relayer is not enough to cover dispatch"] + #[doc = "of all bundled messages."] + InsufficientDispatchWeight, #[codec(index = 12)] #[doc = "The message someone is trying to work with (i.e. increase fee) is not yet sent."] MessageIsNotYetSent,