fail with InsufficientDispatchWeight if dispatch_weight doesn't cover weight of all bundled messages (#2018)

This commit is contained in:
Svyatoslav Nikolsky
2023-04-07 09:55:06 +03:00
committed by Bastian Köcher
parent 98542685b1
commit f659ebed7a
5 changed files with 67 additions and 40 deletions
+10
View File
@@ -160,6 +160,16 @@ pub mod pallet {
/// ///
/// If successful in verification, it will write the target header to the underlying storage /// If successful in verification, it will write the target header to the underlying storage
/// pallet. /// 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::call_index(0)]
#[pallet::weight(<T::WeightInfo as WeightInfo>::submit_finality_proof( #[pallet::weight(<T::WeightInfo as WeightInfo>::submit_finality_proof(
justification.commit.precommits.len().saturated_into(), justification.commit.precommits.len().saturated_into(),
+43 -30
View File
@@ -249,6 +249,22 @@ pub mod pallet {
/// The weight of the call assumes that the transaction always brings outbound lane /// 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 /// 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. /// 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::call_index(2)]
#[pallet::weight(T::WeightInfo::receive_messages_proof_weight(proof, *messages_count, *dispatch_weight))] #[pallet::weight(T::WeightInfo::receive_messages_proof_weight(proof, *messages_count, *dispatch_weight))]
pub fn receive_messages_proof( pub fn receive_messages_proof(
@@ -324,18 +340,10 @@ pub mod pallet {
let mut lane_messages_received_status = let mut lane_messages_received_status =
ReceivedMessages::new(lane_id, Vec::with_capacity(lane_data.messages.len())); 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 { for mut message in lane_data.messages {
debug_assert_eq!(message.key.lane_id, lane_id); debug_assert_eq!(message.key.lane_id, lane_id);
total_messages += 1; 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 // 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 // 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 // weight is not enough, let's move to next lane
@@ -348,10 +356,8 @@ pub mod pallet {
message_dispatch_weight, message_dispatch_weight,
dispatch_weight_left, dispatch_weight_left,
); );
lane_messages_received_status
.push_skipped_for_not_enough_weight(message.key.nonce); fail!(Error::<T, I>::InsufficientDispatchWeight);
is_lane_processing_stopped_no_weight_left = true;
continue
} }
let receival_result = lane.receive_message::<T::MessageDispatch, T::AccountId>( let receival_result = lane.receive_message::<T::MessageDispatch, T::AccountId>(
@@ -554,8 +560,9 @@ pub mod pallet {
/// The relayer has declared invalid unrewarded relayers state in the /// The relayer has declared invalid unrewarded relayers state in the
/// `receive_messages_delivery_proof` call. /// `receive_messages_delivery_proof` call.
InvalidUnrewardedRelayersState, InvalidUnrewardedRelayersState,
/// The message someone is trying to work with (i.e. increase fee) is already-delivered. /// The cumulative dispatch weight, passed by relayer is not enough to cover dispatch
MessageIsAlreadyDelivered, /// of all bundled messages.
InsufficientDispatchWeight,
/// The message someone is trying to work with (i.e. increase fee) is not yet sent. /// The message someone is trying to work with (i.e. increase fee) is not yet sent.
MessageIsNotYetSent, MessageIsNotYetSent,
/// The number of actually confirmed messages is going to be larger than the number of /// The number of actually confirmed messages is going to be larger than the number of
@@ -1277,13 +1284,16 @@ mod tests {
run_test(|| { run_test(|| {
let mut declared_weight = REGULAR_PAYLOAD.declared_weight; let mut declared_weight = REGULAR_PAYLOAD.declared_weight;
*declared_weight.ref_time_mut() -= 1; *declared_weight.ref_time_mut() -= 1;
assert_ok!(Pallet::<TestRuntime>::receive_messages_proof( assert_noop!(
RuntimeOrigin::signed(1), Pallet::<TestRuntime>::receive_messages_proof(
TEST_RELAYER_A, RuntimeOrigin::signed(1),
Ok(vec![message(1, REGULAR_PAYLOAD)]).into(), TEST_RELAYER_A,
1, Ok(vec![message(1, REGULAR_PAYLOAD)]).into(),
declared_weight, 1,
)); declared_weight,
),
Error::<TestRuntime, ()>::InsufficientDispatchWeight
);
assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 0); assert_eq!(InboundLanes::<TestRuntime>::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 message2 = message(2, message_payload(0, u64::MAX / 2));
let message3 = message(3, message_payload(0, u64::MAX / 2)); let message3 = message(3, message_payload(0, u64::MAX / 2));
assert_ok!(Pallet::<TestRuntime, ()>::receive_messages_proof( assert_noop!(
RuntimeOrigin::signed(1), Pallet::<TestRuntime, ()>::receive_messages_proof(
TEST_RELAYER_A, RuntimeOrigin::signed(1),
// this may cause overflow if source chain storage is invalid TEST_RELAYER_A,
Ok(vec![message1, message2, message3]).into(), // this may cause overflow if source chain storage is invalid
3, Ok(vec![message1, message2, message3]).into(),
Weight::MAX, 3,
)); Weight::MAX,
assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 2); ),
Error::<TestRuntime, ()>::InsufficientDispatchWeight
);
assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 0);
}); });
} }
+10
View File
@@ -294,6 +294,16 @@ pub mod pallet {
/// `polkadot-runtime-parachains::paras` pallet instance, deployed at the bridged chain. /// `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 /// The proof is supposed to be crafted at the `relay_header_hash` that must already be
/// imported by corresponding GRANDPA pallet at this chain. /// 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::call_index(0)]
#[pallet::weight(WeightInfoOf::<T, I>::submit_parachain_heads_weight( #[pallet::weight(WeightInfoOf::<T, I>::submit_parachain_heads_weight(
T::DbWeight::get(), T::DbWeight::get(),
+1 -7
View File
@@ -238,8 +238,6 @@ pub struct ReceivedMessages<DispatchLevelResult> {
pub lane: LaneId, pub lane: LaneId,
/// Result of messages which we tried to dispatch /// Result of messages which we tried to dispatch
pub receive_results: Vec<(MessageNonce, ReceivalResult<DispatchLevelResult>)>, pub receive_results: Vec<(MessageNonce, ReceivalResult<DispatchLevelResult>)>,
/// Messages which were skipped and never dispatched
pub skipped_for_not_enough_weight: Vec<MessageNonce>,
} }
impl<DispatchLevelResult> ReceivedMessages<DispatchLevelResult> { impl<DispatchLevelResult> ReceivedMessages<DispatchLevelResult> {
@@ -247,16 +245,12 @@ impl<DispatchLevelResult> ReceivedMessages<DispatchLevelResult> {
lane: LaneId, lane: LaneId,
receive_results: Vec<(MessageNonce, ReceivalResult<DispatchLevelResult>)>, receive_results: Vec<(MessageNonce, ReceivalResult<DispatchLevelResult>)>,
) -> Self { ) -> 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<DispatchLevelResult>) { pub fn push(&mut self, message: MessageNonce, result: ReceivalResult<DispatchLevelResult>) {
self.receive_results.push((message, result)); 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. /// Result of single message receival.
@@ -6043,7 +6043,6 @@ pub mod api {
::core::primitive::u64, ::core::primitive::u64,
runtime_types::bp_messages::ReceivalResult<_0>, runtime_types::bp_messages::ReceivalResult<_0>,
)>, )>,
pub skipped_for_not_enough_weight: ::std::vec::Vec<::core::primitive::u64>,
} }
#[derive( #[derive(
:: subxt :: ext :: codec :: Decode, :: subxt :: ext :: codec :: Encode, Clone, Debug, :: subxt :: ext :: codec :: Decode, :: subxt :: ext :: codec :: Encode, Clone, Debug,
@@ -7508,8 +7507,9 @@ pub mod api {
#[doc = "`receive_messages_delivery_proof` call."] #[doc = "`receive_messages_delivery_proof` call."]
InvalidUnrewardedRelayersState, InvalidUnrewardedRelayersState,
#[codec(index = 11)] #[codec(index = 11)]
#[doc = "The message someone is trying to work with (i.e. increase fee) is already-delivered."] #[doc = "The cumulative dispatch weight, passed by relayer is not enough to cover dispatch"]
MessageIsAlreadyDelivered, #[doc = "of all bundled messages."]
InsufficientDispatchWeight,
#[codec(index = 12)] #[codec(index = 12)]
#[doc = "The message someone is trying to work with (i.e. increase fee) is not yet sent."] #[doc = "The message someone is trying to work with (i.e. increase fee) is not yet sent."]
MessageIsNotYetSent, MessageIsNotYetSent,