diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index 7ccd12db3..3aa117f67 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -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::(vec![42u8; T::maximal_message_size() as _]); - }: increase_message_fee(RawOrigin::Signed(sender.clone()), lane_id, nonce, additional_fee) - verify { - assert_eq!( - OutboundMessages::::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::(vec![42u8; i as _]); - }: increase_message_fee(RawOrigin::Signed(sender.clone()), lane_id, nonce, additional_fee) - verify { - assert_eq!( - OutboundMessages::::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; @@ -596,11 +552,6 @@ fn send_regular_message, I: 'static>() { outbound_lane.send_message(MessageData { payload: vec![], fee: T::message_fee() }); } -fn send_regular_message_with_payload, I: 'static>(payload: Vec) { - let mut outbound_lane = outbound_lane::(T::bench_lane_id()); - outbound_lane.send_message(MessageData { payload, fee: T::message_fee() }); -} - fn confirm_message_delivery, I: 'static>(nonce: MessageNonce) { let mut outbound_lane = outbound_lane::(T::bench_lane_id()); let latest_received_nonce = outbound_lane.data().latest_received_nonce; diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 5a3f52c12..66bc61dc1 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -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}; @@ -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, - lane_id: LaneId, - nonce: MessageNonce, - additional_fee: T::OutboundMessageFee, - ) -> DispatchResultWithPostInfo { - Self::ensure_not_halted().map_err(Error::::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::(lane_id); - ensure!( - nonce > lane.data().latest_received_nonce, - Error::::MessageIsAlreadyDelivered - ); - ensure!( - nonce <= lane.data().latest_generated_nonce, - Error::::MessageIsNotYetSent - ); - - let fund_account = &relayer_fund_account_id::(); - - // 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::::FailedToWithdrawMessageFee - })?; - - // and finally update fee in the storage - let message_key = MessageKey { lane_id, nonce }; - let message_size = OutboundMessages::::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, relayer_id_at_bridged_chain: T::InboundRelayer, @@ -507,6 +439,7 @@ pub mod pallet { relayers_state, T::DbWeight::get(), ))] + #[pallet::call_index(6)] pub fn receive_messages_delivery_proof( origin: OriginFor, proof: MessagesDeliveryProofOf, @@ -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, @@ -1373,11 +1302,6 @@ mod tests { Error::::NotOperatingNormally, ); - assert_noop!( - Pallet::::increase_message_fee(Origin::signed(1), TEST_LANE_ID, 1, 1,), - Error::::BridgeModule(bp_runtime::OwnedBridgeModuleError::Halted), - ); - assert_noop!( Pallet::::receive_messages_proof( Origin::signed(1), @@ -1433,13 +1357,6 @@ mod tests { Error::::NotOperatingNormally, ); - assert_ok!(Pallet::::increase_message_fee( - Origin::signed(1), - TEST_LANE_ID, - 1, - 1, - )); - assert_ok!(Pallet::::receive_messages_proof( Origin::signed(1), TEST_RELAYER_A, @@ -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::::increase_message_fee( - Origin::signed(1), - TEST_LANE_ID, - 1, - 100, - ), - Error::::MessageIsAlreadyDelivered, - ); - }); - } - - #[test] - fn increase_message_fee_fails_if_message_is_not_yet_sent() { - run_test(|| { - assert_noop!( - Pallet::::increase_message_fee( - Origin::signed(1), - TEST_LANE_ID, - 1, - 100, - ), - Error::::MessageIsNotYetSent, - ); - }); - } - - #[test] - fn increase_message_fee_fails_if_submitter_cant_pay_additional_fee() { - run_test(|| { - send_regular_message(); - - TestMessageDeliveryAndDispatchPayment::reject_payments(); - - assert_noop!( - Pallet::::increase_message_fee( - Origin::signed(1), - TEST_LANE_ID, - 1, - 100, - ), - Error::::FailedToWithdrawMessageFee, - ); - }); - } - - #[test] - fn increase_message_fee_succeeds() { - run_test(|| { - send_regular_message(); - - assert_ok!(Pallet::::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(|| { @@ -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::::send_message( - Origin::signed(1), - TEST_LANE_ID, - small_payload, - 100, - )); - assert_ok!(Pallet::::send_message( - Origin::signed(1), - TEST_LANE_ID, - large_payload, - 100, - )); - - let small_weight = - Pallet::::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::::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(|| { diff --git a/modules/messages/src/weights.rs b/modules/messages/src/weights.rs index b6256eec9..c6d4c22bc 100644 --- a/modules/messages/src/weights.rs +++ b/modules/messages/src/weights.rs @@ -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; @@ -91,13 +90,6 @@ impl WeightInfo for MillauWeight { .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)) @@ -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))