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

Fail with InsufficientDispatchWeight if dispatch_weight doesn't cover the weight of all bundled messages #2018

Merged
merged 1 commit into from
Apr 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions modules/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(<T::WeightInfo as WeightInfo>::submit_finality_proof(
justification.commit.precommits.len().saturated_into(),
Expand Down
73 changes: 43 additions & 30 deletions modules/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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::<T, I>::InsufficientDispatchWeight);
}

let receival_result = lane.receive_message::<T::MessageDispatch, T::AccountId>(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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::<TestRuntime>::receive_messages_proof(
RuntimeOrigin::signed(1),
TEST_RELAYER_A,
Ok(vec![message(1, REGULAR_PAYLOAD)]).into(),
1,
declared_weight,
));
assert_noop!(
Pallet::<TestRuntime>::receive_messages_proof(
RuntimeOrigin::signed(1),
TEST_RELAYER_A,
Ok(vec![message(1, REGULAR_PAYLOAD)]).into(),
1,
declared_weight,
),
Error::<TestRuntime, ()>::InsufficientDispatchWeight
);
assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 0);
});
}
Expand Down Expand Up @@ -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::<TestRuntime, ()>::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::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 2);
assert_noop!(
Pallet::<TestRuntime, ()>::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::<TestRuntime, ()>::InsufficientDispatchWeight
);
assert_eq!(InboundLanes::<TestRuntime>::get(TEST_LANE_ID).last_delivered_nonce(), 0);
});
}

Expand Down
10 changes: 10 additions & 0 deletions modules/parachains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<T, I>::submit_parachain_heads_weight(
T::DbWeight::get(),
Expand Down
8 changes: 1 addition & 7 deletions primitives/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,25 +238,19 @@ pub struct ReceivedMessages<DispatchLevelResult> {
pub lane: LaneId,
/// Result of messages which we tried to dispatch
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> {
pub fn new(
lane: LaneId,
receive_results: Vec<(MessageNonce, ReceivalResult<DispatchLevelResult>)>,
) -> 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>) {
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.
Expand Down
6 changes: 3 additions & 3 deletions relays/client-rialto-parachain/src/codegen_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down