Skip to content
This repository was archived by the owner on Feb 29, 2024. It is now read-only.

Delete increase_message_fee #207

Merged
aurexav merged 6 commits into from
Oct 13, 2022
Merged

Delete increase_message_fee #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7e1369c
Delete useless call
boundless-forest Oct 9, 2022
11f0af2
Use call_index
boundless-forest Oct 10, 2022
d9e4f70
Remove useless error types
boundless-forest Oct 10, 2022
23275c8
Merge remote-tracking branch 'origin/xavier/latest' into bear-increas…
boundless-forest Oct 11, 2022
c90b921
Merge remote-tracking branch 'origin/xavier/latest' into bear-increas…
boundless-forest Oct 13, 2022
71367c3
Update
boundless-forest Oct 13, 2022
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
49 changes: 0 additions & 49 deletions modules/messages/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,50 +221,6 @@ benchmarks_instance_pallet! {
);
}

// Benchmark `increase_message_fee` with following conditions:
// * message has maximal message;
// * submitter account is killed because its balance is less than ED after payment.
//
// Result of this benchmark is directly used by weight formula of the call.
maximal_increase_message_fee {
let sender = account("sender", 42, SEED);
T::endow_account(&sender);

let additional_fee = T::account_balance(&sender);
let lane_id = T::bench_lane_id();
let nonce = 1;

send_regular_message_with_payload::<T, I>(vec![42u8; T::maximal_message_size() as _]);
}: increase_message_fee(RawOrigin::Signed(sender.clone()), lane_id, nonce, additional_fee)
verify {
assert_eq!(
OutboundMessages::<T, I>::get(MessageKey { lane_id, nonce }).unwrap().fee,
T::message_fee() + additional_fee,
);
}

// Benchmark `increase_message_fee` with following conditions:
// * message size varies from minimal to maximal;
// * submitter account is killed because its balance is less than ED after payment.
increase_message_fee {
let i in 0..T::maximal_message_size();

let sender = account("sender", 42, SEED);
T::endow_account(&sender);

let additional_fee = T::account_balance(&sender);
let lane_id = T::bench_lane_id();
let nonce = 1;

send_regular_message_with_payload::<T, I>(vec![42u8; i as _]);
}: increase_message_fee(RawOrigin::Signed(sender.clone()), lane_id, nonce, additional_fee)
verify {
assert_eq!(
OutboundMessages::<T, I>::get(MessageKey { lane_id, nonce }).unwrap().fee,
T::message_fee() + additional_fee,
);
}

// Benchmark `receive_messages_proof` extrinsic with single minimal-weight message and following conditions:
// * proof does not include outbound lane state proof;
// * inbound lane already has state, so it needs to be read and decoded;
Expand Down Expand Up @@ -596,11 +552,6 @@ fn send_regular_message<T: Config<I>, I: 'static>() {
outbound_lane.send_message(MessageData { payload: vec![], fee: T::message_fee() });
}

fn send_regular_message_with_payload<T: Config<I>, I: 'static>(payload: Vec<u8>) {
let mut outbound_lane = outbound_lane::<T, I>(T::bench_lane_id());
outbound_lane.send_message(MessageData { payload, fee: T::message_fee() });
}

fn confirm_message_delivery<T: Config<I>, I: 'static>(nonce: MessageNonce) {
let mut outbound_lane = outbound_lane::<T, I>(T::bench_lane_id());
let latest_received_nonce = outbound_lane.data().latest_received_nonce;
Expand Down
196 changes: 2 additions & 194 deletions modules/messages/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ pub use weights_ext::{
ensure_weights_are_correct, WeightInfoExt, EXPECTED_DEFAULT_MESSAGE_LENGTH,
};


// crates.io
use codec::{Decode, Encode, MaxEncodedLen};
use num_traits::{SaturatingAdd, Zero};
Expand Down Expand Up @@ -290,80 +289,13 @@ pub mod pallet {
)
}

/// Pay additional fee for the message.
#[pallet::weight(T::WeightInfo::maximal_increase_message_fee())]
pub fn increase_message_fee(
origin: OriginFor<T>,
lane_id: LaneId,
nonce: MessageNonce,
additional_fee: T::OutboundMessageFee,
) -> DispatchResultWithPostInfo {
Self::ensure_not_halted().map_err(Error::<T, I>::BridgeModule)?;
// if someone tries to pay for already-delivered message, we're rejecting this intention
// (otherwise this additional fee will be locked forever in relayers fund)
//
// if someone tries to pay for not-yet-sent message, we're rejecting this intention, or
// we're risking to have mess in the storage
let lane = outbound_lane::<T, I>(lane_id);
ensure!(
nonce > lane.data().latest_received_nonce,
Error::<T, I>::MessageIsAlreadyDelivered
);
ensure!(
nonce <= lane.data().latest_generated_nonce,
Error::<T, I>::MessageIsNotYetSent
);

let fund_account = &relayer_fund_account_id::<T::AccountId, T::AccountIdConverter>();

// withdraw additional fee from submitter
T::MessageDeliveryAndDispatchPayment::pay_delivery_and_dispatch_fee(
&origin,
&additional_fee,
fund_account,
)
.map_err(|err| {
log::trace!(
target: LOG_TARGET,
"Submitter can't pay additional fee {:?} for the message {:?}/{:?} to {:?}: {:?}",
additional_fee,
lane_id,
nonce,
fund_account,
err,
);

Error::<T, I>::FailedToWithdrawMessageFee
})?;

// and finally update fee in the storage
let message_key = MessageKey { lane_id, nonce };
let message_size = OutboundMessages::<T, I>::mutate(message_key, |message_data| {
// saturating_add is fine here - overflow here means that someone controls all
// chain funds, which shouldn't ever happen + `pay_delivery_and_dispatch_fee`
// above will fail before we reach here
let message_data = message_data.as_mut().expect(
"the message is sent and not yet delivered; so it is in the storage; qed",
);
message_data.fee = message_data.fee.saturating_add(&additional_fee);
message_data.payload.len()
});

// compute actual dispatch weight that depends on the stored message size
let actual_weight = sp_std::cmp::min(
T::WeightInfo::maximal_increase_message_fee(),
T::WeightInfo::increase_message_fee(message_size as _),
);

Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes })
}

/// Receive messages proof from bridged chain.
///
/// 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.
#[pallet::weight(T::WeightInfo::receive_messages_proof_weight(proof, *messages_count, *dispatch_weight))]
#[pallet::call_index(5)]
pub fn receive_messages_proof(
origin: OriginFor<T>,
relayer_id_at_bridged_chain: T::InboundRelayer,
Expand Down Expand Up @@ -507,6 +439,7 @@ pub mod pallet {
relayers_state,
T::DbWeight::get(),
))]
#[pallet::call_index(6)]
pub fn receive_messages_delivery_proof(
origin: OriginFor<T>,
proof: MessagesDeliveryProofOf<T, I>,
Expand Down Expand Up @@ -691,10 +624,6 @@ 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 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
/// messages in the proof. This may mean that this or bridged chain storage is corrupted.
TryingToConfirmMoreMessagesThanExpected,
Expand Down Expand Up @@ -1373,11 +1302,6 @@ mod tests {
Error::<TestRuntime, ()>::NotOperatingNormally,
);

assert_noop!(
Pallet::<TestRuntime>::increase_message_fee(Origin::signed(1), TEST_LANE_ID, 1, 1,),
Error::<TestRuntime, ()>::BridgeModule(bp_runtime::OwnedBridgeModuleError::Halted),
);

assert_noop!(
Pallet::<TestRuntime>::receive_messages_proof(
Origin::signed(1),
Expand Down Expand Up @@ -1433,13 +1357,6 @@ mod tests {
Error::<TestRuntime, ()>::NotOperatingNormally,
);

assert_ok!(Pallet::<TestRuntime>::increase_message_fee(
Origin::signed(1),
TEST_LANE_ID,
1,
1,
));

assert_ok!(Pallet::<TestRuntime>::receive_messages_proof(
Origin::signed(1),
TEST_RELAYER_A,
Expand Down Expand Up @@ -1914,73 +1831,6 @@ mod tests {
});
}

#[test]
fn increase_message_fee_fails_if_message_is_already_delivered() {
run_test(|| {
send_regular_message();
receive_messages_delivery_proof();

assert_noop!(
Pallet::<TestRuntime, ()>::increase_message_fee(
Origin::signed(1),
TEST_LANE_ID,
1,
100,
),
Error::<TestRuntime, ()>::MessageIsAlreadyDelivered,
);
});
}

#[test]
fn increase_message_fee_fails_if_message_is_not_yet_sent() {
run_test(|| {
assert_noop!(
Pallet::<TestRuntime, ()>::increase_message_fee(
Origin::signed(1),
TEST_LANE_ID,
1,
100,
),
Error::<TestRuntime, ()>::MessageIsNotYetSent,
);
});
}

#[test]
fn increase_message_fee_fails_if_submitter_cant_pay_additional_fee() {
run_test(|| {
send_regular_message();

TestMessageDeliveryAndDispatchPayment::reject_payments();

assert_noop!(
Pallet::<TestRuntime, ()>::increase_message_fee(
Origin::signed(1),
TEST_LANE_ID,
1,
100,
),
Error::<TestRuntime, ()>::FailedToWithdrawMessageFee,
);
});
}

#[test]
fn increase_message_fee_succeeds() {
run_test(|| {
send_regular_message();

assert_ok!(Pallet::<TestRuntime, ()>::increase_message_fee(
Origin::signed(1),
TEST_LANE_ID,
1,
100,
),);
assert!(TestMessageDeliveryAndDispatchPayment::is_fee_paid(1, 100));
});
}

#[test]
fn weight_refund_from_receive_messages_proof_works() {
run_test(|| {
Expand Down Expand Up @@ -2205,48 +2055,6 @@ mod tests {
});
}

#[test]
fn increase_message_fee_weight_depends_on_message_size() {
run_test(|| {
let mut small_payload = message_payload(0, 100);
let mut large_payload = message_payload(1, 100);
small_payload.extra = vec![1; MAX_OUTBOUND_PAYLOAD_SIZE as usize / 10];
large_payload.extra = vec![2; MAX_OUTBOUND_PAYLOAD_SIZE as usize / 5];

assert_ok!(Pallet::<TestRuntime>::send_message(
Origin::signed(1),
TEST_LANE_ID,
small_payload,
100,
));
assert_ok!(Pallet::<TestRuntime>::send_message(
Origin::signed(1),
TEST_LANE_ID,
large_payload,
100,
));

let small_weight =
Pallet::<TestRuntime>::increase_message_fee(Origin::signed(1), TEST_LANE_ID, 1, 1)
.expect("increase_message_fee has failed")
.actual_weight
.expect("increase_message_fee always returns Some");

let large_weight =
Pallet::<TestRuntime>::increase_message_fee(Origin::signed(1), TEST_LANE_ID, 2, 1)
.expect("increase_message_fee has failed")
.actual_weight
.expect("increase_message_fee always returns Some");

assert!(
large_weight > small_weight,
"Actual post-dispatch weigth for larger message {} must be larger than {} for small message",
large_weight,
small_weight,
);
});
}

#[test]
fn weight_is_refunded_for_messages_that_are_not_pruned() {
run_test(|| {
Expand Down
15 changes: 0 additions & 15 deletions modules/messages/src/weights.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ pub trait WeightInfo {
fn send_1_kb_message_worst_case() -> Weight;
fn send_16_kb_message_worst_case() -> Weight;
fn maximal_increase_message_fee() -> Weight;
fn increase_message_fee(i: u32) -> Weight;
fn receive_single_message_proof() -> Weight;
fn receive_two_messages_proof() -> Weight;
fn receive_single_message_proof_with_outbound_lane_state() -> Weight;
Expand Down Expand Up @@ -91,13 +90,6 @@ impl<T: frame_system::Config> WeightInfo for MillauWeight<T> {
.saturating_add(T::DbWeight::get().writes(3 as Weight))
}

fn increase_message_fee(i: u32) -> Weight {
(0 as Weight)
.saturating_add((2_000 as Weight).saturating_mul(i as Weight))
.saturating_add(T::DbWeight::get().reads(5 as Weight))
.saturating_add(T::DbWeight::get().writes(3 as Weight))
}

fn receive_single_message_proof() -> Weight {
(179_892_000 as Weight)
.saturating_add(T::DbWeight::get().reads(6 as Weight))
Expand Down Expand Up @@ -179,13 +171,6 @@ impl WeightInfo for () {
.saturating_add(RocksDbWeight::get().writes(3 as Weight))
}

fn increase_message_fee(i: u32) -> Weight {
(0 as Weight)
.saturating_add((2_000 as Weight).saturating_mul(i as Weight))
.saturating_add(RocksDbWeight::get().reads(5 as Weight))
.saturating_add(RocksDbWeight::get().writes(3 as Weight))
}

fn receive_single_message_proof() -> Weight {
(179_892_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(6 as Weight))
Expand Down