From 029253a5a94539b317e1cb5628778ac75d6dbfcb Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 19 Apr 2021 17:00:18 +0300 Subject: [PATCH 01/14] pay dispatch fee at target chain --- Cargo.lock | 1 + bin/millau/runtime/src/rialto_messages.rs | 1 + bin/rialto/runtime/src/millau_messages.rs | 1 + bin/runtime-common/Cargo.toml | 2 + bin/runtime-common/src/messages.rs | 87 +++++++++-- modules/dispatch/src/lib.rs | 140 +++++++++++++++--- modules/messages/src/inbound_lane.rs | 94 +++++++----- modules/messages/src/lib.rs | 18 ++- modules/messages/src/mock.rs | 6 +- primitives/message-dispatch/src/lib.rs | 11 +- primitives/messages/src/target_chain.rs | 20 ++- .../bin-substrate/src/cli/encode_message.rs | 4 +- relays/bin-substrate/src/cli/send_message.rs | 5 + 13 files changed, 303 insertions(+), 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bcd8e5ace2..e59c6a7d05 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -862,6 +862,7 @@ dependencies = [ "pallet-bridge-dispatch", "pallet-bridge-grandpa", "pallet-bridge-messages", + "pallet-transaction-payment", "parity-scale-codec 2.0.1", "sp-core", "sp-runtime", diff --git a/bin/millau/runtime/src/rialto_messages.rs b/bin/millau/runtime/src/rialto_messages.rs index a800117dc5..2af0d5c381 100644 --- a/bin/millau/runtime/src/rialto_messages.rs +++ b/bin/millau/runtime/src/rialto_messages.rs @@ -64,6 +64,7 @@ type ToRialtoMessagesDeliveryProof = messages::source::FromBridgedChainMessagesD pub type FromRialtoMessageDispatch = messages::target::FromBridgedChainMessageDispatch< WithRialtoMessageBridge, crate::Runtime, + pallet_balances::Pallet, pallet_bridge_dispatch::DefaultInstance, >; diff --git a/bin/rialto/runtime/src/millau_messages.rs b/bin/rialto/runtime/src/millau_messages.rs index 8ee2094660..be0e48c381 100644 --- a/bin/rialto/runtime/src/millau_messages.rs +++ b/bin/rialto/runtime/src/millau_messages.rs @@ -58,6 +58,7 @@ pub type FromMillauEncodedCall = messages::target::FromBridgedChainEncodedMessag pub type FromMillauMessageDispatch = messages::target::FromBridgedChainMessageDispatch< WithMillauMessageBridge, crate::Runtime, + pallet_balances::Pallet, pallet_bridge_dispatch::DefaultInstance, >; diff --git a/bin/runtime-common/Cargo.toml b/bin/runtime-common/Cargo.toml index 83803d06de..07fe8910c2 100644 --- a/bin/runtime-common/Cargo.toml +++ b/bin/runtime-common/Cargo.toml @@ -24,6 +24,7 @@ pallet-bridge-messages = { path = "../../modules/messages", default-features = f # Substrate dependencies frame-support = { git = "https://github.com/paritytech/substrate", branch = "master" , default-features = false } +pallet-transaction-payment = { git = "https://github.com/paritytech/substrate", branch = "master" , default-features = false } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" , default-features = false } sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master" , default-features = false } sp-state-machine = { git = "https://github.com/paritytech/substrate", branch = "master" , default-features = false, optional = true } @@ -42,6 +43,7 @@ std = [ "pallet-bridge-dispatch/std", "pallet-bridge-grandpa/std", "pallet-bridge-messages/std", + "pallet-transaction-payment/std", "sp-core/std", "sp-runtime/std", "sp-state-machine/std", diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index 8e83c0f94a..0df9577176 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -28,7 +28,11 @@ use bp_messages::{ }; use bp_runtime::{InstanceId, Size, StorageProofChecker}; use codec::{Decode, Encode}; -use frame_support::{traits::Instance, weights::Weight, RuntimeDebug}; +use frame_support::{ + traits::{Currency, ExistenceRequirement, Instance}, + weights::{Weight, WeightToFeePolynomial}, + RuntimeDebug, +}; use hash_db::Hasher; use sp_runtime::{ traits::{AtLeast32BitUnsigned, CheckedAdd, CheckedDiv, CheckedMul}, @@ -326,8 +330,17 @@ pub mod source { relayer_fee_percent: u32, ) -> Result>, &'static str> { // the fee (in Bridged tokens) of all transactions that are made on the Bridged chain - let delivery_transaction = - BridgedChain::::estimate_delivery_transaction(&payload.call, payload.weight.into()); + // + // if we're going to pay dispatch fee at the target chain, then we don't include weight + // of the message dispatch in the delivery transaction cost + let delivery_transaction = BridgedChain::::estimate_delivery_transaction( + &payload.call, + if payload.pay_dispatch_fee_at_target_chain { + 0.into() + } else { + payload.weight.into() + }, + ); let delivery_transaction_fee = BridgedChain::::transaction_payment(delivery_transaction); // the fee (in This tokens) of all transactions that are made on This chain @@ -456,20 +469,28 @@ pub mod target { /// Dispatching Bridged -> This chain messages. #[derive(RuntimeDebug, Clone, Copy)] - pub struct FromBridgedChainMessageDispatch { - _marker: PhantomData<(B, ThisRuntime, ThisDispatchInstance)>, + pub struct FromBridgedChainMessageDispatch { + _marker: PhantomData<(B, ThisRuntime, ThisCurrency, ThisDispatchInstance)>, } - impl - MessageDispatch< as ChainWithMessages>::Balance> - for FromBridgedChainMessageDispatch + impl + MessageDispatch>, BalanceOf>> + for FromBridgedChainMessageDispatch where ThisDispatchInstance: frame_support::traits::Instance, - ThisRuntime: pallet_bridge_dispatch::Config, - >::Event: - From>, - pallet_bridge_dispatch::Pallet: - bp_message_dispatch::MessageDispatch<(LaneId, MessageNonce), Message = FromBridgedChainMessagePayload>, + ThisRuntime: pallet_bridge_dispatch::Config + + pallet_transaction_payment::Config, + ::OnChargeTransaction: + pallet_transaction_payment::OnChargeTransaction>>, + ThisCurrency: Currency>, Balance = BalanceOf>>, + >::Event: From< + pallet_bridge_dispatch::RawEvent<(LaneId, MessageNonce), AccountIdOf>, ThisDispatchInstance>, + >, + pallet_bridge_dispatch::Pallet: bp_message_dispatch::MessageDispatch< + AccountIdOf>, + (LaneId, MessageNonce), + Message = FromBridgedChainMessagePayload, + >, { type DispatchPayload = FromBridgedChainMessagePayload; @@ -479,13 +500,25 @@ pub mod target { message.data.payload.as_ref().map(|payload| payload.weight).unwrap_or(0) } - fn dispatch(message: DispatchMessage>>) { + fn dispatch( + relayer_account: &AccountIdOf>, + message: DispatchMessage>>, + ) -> Weight { let message_id = (message.key.lane_id, message.key.nonce); pallet_bridge_dispatch::Pallet::::dispatch( B::INSTANCE, message_id, message.data.payload.map_err(drop), - ); + |dispatch_origin, dispatch_weight| { + ThisCurrency::transfer( + dispatch_origin, + relayer_account, + ThisRuntime::WeightToFee::calc(&dispatch_weight), + ExistenceRequirement::AllowDeath, + ) + .map_err(drop) + }, + ) } } @@ -930,6 +963,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, + pay_dispatch_fee_at_target_chain: true, call: ThisChainCall::Transfer.encode(), } .encode(); @@ -944,6 +978,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, + pay_dispatch_fee_at_target_chain: true, call: target::FromBridgedChainEncodedMessageCall:: { encoded_call: ThisChainCall::Transfer.encode(), _marker: PhantomData::default(), @@ -961,6 +996,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, + pay_dispatch_fee_at_target_chain: false, call: vec![42], } } @@ -981,6 +1017,21 @@ mod tests { Ok(ThisChainBalance(EXPECTED_MINIMAL_FEE)), ); + // let's check if estimation is less than hardcoded, if dispatch is paid at target chain + let mut payload_with_pay_on_target = regular_outbound_message_payload(); + payload_with_pay_on_target.pay_dispatch_fee_at_target_chain = true; + let fee_at_source = source::estimate_message_dispatch_and_delivery_fee::( + &payload_with_pay_on_target, + OnThisChainBridge::RELAYER_FEE_PERCENT, + ) + .expect("estimate_message_dispatch_and_delivery_fee failed for pay-at-target-chain message"); + assert!( + fee_at_source < EXPECTED_MINIMAL_FEE.into(), + "Computed fee {:?} without prepaid dispatch must be less than the fee with prepaid dispatch {}", + fee_at_source, + EXPECTED_MINIMAL_FEE, + ); + // and now check that the verifier checks the fee assert_eq!( source::FromThisChainMessageVerifier::::verify_message( @@ -1011,6 +1062,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, + pay_dispatch_fee_at_target_chain: false, call: vec![42], }; @@ -1054,6 +1106,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceAccount(ThisChainAccountId(1)), + pay_dispatch_fee_at_target_chain: false, call: vec![42], }; @@ -1121,6 +1174,7 @@ mod tests { spec_version: 1, weight: 5, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, + pay_dispatch_fee_at_target_chain: false, call: vec![1, 2, 3, 4, 5, 6], },) .is_err() @@ -1136,6 +1190,7 @@ mod tests { spec_version: 1, weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT + 1, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, + pay_dispatch_fee_at_target_chain: false, call: vec![1, 2, 3, 4, 5, 6], },) .is_err() @@ -1151,6 +1206,7 @@ mod tests { spec_version: 1, weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, + pay_dispatch_fee_at_target_chain: false, call: vec![0; source::maximal_message_size::() as usize + 1], },) .is_err() @@ -1166,6 +1222,7 @@ mod tests { spec_version: 1, weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, + pay_dispatch_fee_at_target_chain: false, call: vec![0; source::maximal_message_size::() as _], },), Ok(()), diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index 416d080b0c..ab8287780d 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -102,6 +102,13 @@ pub struct MessagePayload, + /// If true, then the sender has decided to pay dispatch fee at the target chain. + /// + /// The fee is paid right before the message is dispatched. So in case of any other + /// issues (like invalid call encoding, invalid signature, ...) we won't do any direct + /// transfers. Instead, we're returning fee related to this message dispatch to the + /// relayer. + pub pay_dispatch_fee_at_target_chain: bool, /// The call itself. pub call: Call, } @@ -160,7 +167,8 @@ decl_storage! { decl_event!( pub enum Event where - >::MessageId + >::MessageId, + AccountId = ::AccountId, { /// Message has been rejected before reaching dispatch. MessageRejected(InstanceId, MessageId), @@ -178,6 +186,8 @@ decl_event!( MessageCallDecodeFailed(InstanceId, MessageId), /// The call from the message has been rejected by the call filter. MessageCallRejected(InstanceId, MessageId), + /// The origin account has failed to pay fee for dispatching the message. + MessageDispatchPaymentFailed(InstanceId, MessageId, AccountId, Weight), /// Phantom member, never used. Needed to handle multiple pallet instances. _Dummy(PhantomData), } @@ -191,7 +201,7 @@ decl_module! { } } -impl, I: Instance> MessageDispatch for Pallet { +impl, I: Instance> MessageDispatch for Pallet { type Message = MessagePayload; @@ -199,14 +209,19 @@ impl, I: Instance> MessageDispatch for Pallet { message.weight } - fn dispatch(bridge: InstanceId, id: T::MessageId, message: Result) { + fn dispatch Result<(), ()>>( + bridge: InstanceId, + id: T::MessageId, + message: Result, + pay_dispatch_fee: P, + ) -> Weight { // emit special even if message has been rejected by external component let message = match message { Ok(message) => message, Err(_) => { log::trace!(target: "runtime::bridge-dispatch", "Message {:?}/{:?}: rejected before actual dispatch", bridge, id); Self::deposit_event(RawEvent::MessageRejected(bridge, id)); - return; + return 0; } }; @@ -227,7 +242,7 @@ impl, I: Instance> MessageDispatch for Pallet { expected_version, message.spec_version, )); - return; + return message.weight; } // now that we have spec version checked, let's decode the call @@ -236,7 +251,7 @@ impl, I: Instance> MessageDispatch for Pallet { Err(_) => { log::trace!(target: "runtime::bridge-dispatch", "Failed to decode Call from message {:?}/{:?}", bridge, id,); Self::deposit_event(RawEvent::MessageCallDecodeFailed(bridge, id)); - return; + return message.weight; } }; @@ -262,7 +277,7 @@ impl, I: Instance> MessageDispatch for Pallet { target_signature, ); Self::deposit_event(RawEvent::MessageSignatureMismatch(bridge, id)); - return; + return message.weight; } log::trace!(target: "runtime::bridge-dispatch", "Target Account: {:?}", &target_account); @@ -286,7 +301,7 @@ impl, I: Instance> MessageDispatch for Pallet { call, ); Self::deposit_event(RawEvent::MessageCallRejected(bridge, id)); - return; + return message.weight; } // verify weight @@ -309,7 +324,27 @@ impl, I: Instance> MessageDispatch for Pallet { expected_weight, message.weight, )); - return; + return message.weight; + } + + // pay dispatch fee right before dispatch + if message.pay_dispatch_fee_at_target_chain { + if pay_dispatch_fee(&origin_account, message.weight).is_err() { + log::trace!( + target: "runtime::bridge-dispatch", + "Failed to pay dispatch fee for dispatching message {:?}/{:?} with weight {}", + bridge, + id, + message.weight, + ); + Self::deposit_event(RawEvent::MessageDispatchPaymentFailed( + bridge, + id, + origin_account, + message.weight, + )); + return message.weight; + } } // finally dispatch message @@ -318,6 +353,7 @@ impl, I: Instance> MessageDispatch for Pallet { log::trace!(target: "runtime::bridge-dispatch", "Message being dispatched is: {:?}", &call); let dispatch_result = call.dispatch(origin); let actual_call_weight = extract_actual_weight(&dispatch_result, &dispatch_info); + let unused_dispatch_weight = message.weight.saturating_sub(actual_call_weight); log::trace!( target: "runtime::bridge-dispatch", @@ -334,6 +370,8 @@ impl, I: Instance> MessageDispatch for Pallet { id, dispatch_result.map(drop).map_err(|e| e.error), )); + + unused_dispatch_weight } } @@ -537,31 +575,32 @@ mod tests { fn prepare_message( origin: CallOrigin, call: Call, - ) -> as MessageDispatch<::MessageId>>::Message { + ) -> as MessageDispatch::MessageId>>::Message { MessagePayload { spec_version: TEST_SPEC_VERSION, weight: TEST_WEIGHT, origin, + pay_dispatch_fee_at_target_chain: false, call: EncodedCall(call.encode()), } } fn prepare_root_message( call: Call, - ) -> as MessageDispatch<::MessageId>>::Message { + ) -> as MessageDispatch::MessageId>>::Message { prepare_message(CallOrigin::SourceRoot, call) } fn prepare_target_message( call: Call, - ) -> as MessageDispatch<::MessageId>>::Message { + ) -> as MessageDispatch::MessageId>>::Message { let origin = CallOrigin::TargetAccount(1, TestAccountPublic(1), TestSignature(1)); prepare_message(origin, call) } fn prepare_source_message( call: Call, - ) -> as MessageDispatch<::MessageId>>::Message { + ) -> as MessageDispatch::MessageId>>::Message { let origin = CallOrigin::SourceAccount(1); prepare_message(origin, call) } @@ -578,7 +617,7 @@ mod tests { message.spec_version = BAD_SPEC_VERSION; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message)); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); assert_eq!( System::events(), @@ -606,7 +645,7 @@ mod tests { message.weight = 0; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message)); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); assert_eq!( System::events(), @@ -634,7 +673,7 @@ mod tests { ); System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message)); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); assert_eq!( System::events(), @@ -656,7 +695,7 @@ mod tests { let id = [0; 4]; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Err(())); + Dispatch::dispatch(bridge, id, Err(()), |_, _| unreachable!()); assert_eq!( System::events(), @@ -680,7 +719,7 @@ mod tests { message.call.0 = vec![]; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message)); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); assert_eq!( System::events(), @@ -707,7 +746,7 @@ mod tests { message.weight = weight; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message)); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); assert_eq!( System::events(), @@ -720,6 +759,63 @@ mod tests { }); } + #[test] + fn should_emit_event_for_unpaid_calls() { + new_test_ext().execute_with(|| { + let bridge = b"ethb".to_owned(); + let id = [0; 4]; + + let mut message = + prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); + message.pay_dispatch_fee_at_target_chain = true; + + System::set_block_number(1); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| Err(())); + + assert_eq!( + System::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: Event::call_dispatch(call_dispatch::Event::::MessageDispatchPaymentFailed( + bridge, + id, + AccountIdConverter::convert(derive_account_id::(bridge, SourceAccount::Root)), + TEST_WEIGHT, + )), + topics: vec![], + }], + ); + }); + } + + #[test] + fn should_dispatch_calls_paid_at_target_chain() { + new_test_ext().execute_with(|| { + let bridge = b"ethb".to_owned(); + let id = [0; 4]; + + let mut message = + prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); + message.pay_dispatch_fee_at_target_chain = true; + + System::set_block_number(1); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| Ok(())); + + assert_eq!( + System::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: Event::call_dispatch(call_dispatch::Event::::MessageDispatched( + bridge, + id, + Ok(()) + )), + topics: vec![], + }], + ); + }); + } + #[test] fn should_dispatch_bridge_message_from_root_origin() { new_test_ext().execute_with(|| { @@ -728,7 +824,7 @@ mod tests { let message = prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message)); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); assert_eq!( System::events(), @@ -755,7 +851,7 @@ mod tests { let message = prepare_target_message(call); System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message)); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); assert_eq!( System::events(), @@ -782,7 +878,7 @@ mod tests { let message = prepare_source_message(call); System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message)); + Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); assert_eq!( System::events(), diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index b5576bc30a..c671d2c6ae 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -27,7 +27,7 @@ pub trait InboundLaneStorage { /// Delivery and dispatch fee type on source chain. type MessageFee; /// Id of relayer on source chain. - type Relayer: PartialEq; + type Relayer: Clone + PartialEq; /// Lane id. fn id(&self) -> LaneId; @@ -90,9 +90,10 @@ impl InboundLane { } /// Receive new message. - pub fn receive_message>( + pub fn receive_message, AccountId>( &mut self, - relayer: S::Relayer, + relayer_at_brdiged_chain: &S::Relayer, + relayer_at_this_chain: &AccountId, nonce: MessageNonce, message_data: DispatchMessageData, ) -> bool { @@ -114,25 +115,29 @@ impl InboundLane { } let push_new = match data.relayers.back_mut() { - Some((_, nonce_high, last_relayer)) if last_relayer == &relayer => { + Some((_, nonce_high, last_relayer)) if last_relayer == relayer_at_brdiged_chain => { *nonce_high = nonce; false } _ => true, }; if push_new { - data.relayers.push_back((nonce, nonce, relayer)); + data.relayers + .push_back((nonce, nonce, (*relayer_at_brdiged_chain).clone())); } self.storage.set_data(data); - P::dispatch(DispatchMessage { - key: MessageKey { - lane_id: self.storage.id(), - nonce, + P::dispatch( + relayer_at_this_chain, + DispatchMessage { + key: MessageKey { + lane_id: self.storage.id(), + nonce, + }, + data: message_data, }, - data: message_data, - }); + ); true } @@ -154,8 +159,9 @@ mod tests { lane: &mut InboundLane>, nonce: MessageNonce, ) { - assert!(lane.receive_message::( - TEST_RELAYER_A, + assert!(lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, nonce, message_data(REGULAR_PAYLOAD).into() )); @@ -269,8 +275,9 @@ mod tests { fn fails_to_receive_message_with_incorrect_nonce() { run_test(|| { let mut lane = inbound_lane::(TEST_LANE_ID); - assert!(!lane.receive_message::( - TEST_RELAYER_A, + assert!(!lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, 10, message_data(REGULAR_PAYLOAD).into() )); @@ -284,8 +291,9 @@ mod tests { let mut lane = inbound_lane::(TEST_LANE_ID); let max_nonce = ::MaxUnrewardedRelayerEntriesAtInboundLane::get(); for current_nonce in 1..max_nonce + 1 { - assert!(lane.receive_message::( - TEST_RELAYER_A + current_nonce, + assert!(lane.receive_message::( + &(TEST_RELAYER_A + current_nonce), + &(TEST_RELAYER_A + current_nonce), current_nonce, message_data(REGULAR_PAYLOAD).into() )); @@ -293,8 +301,9 @@ mod tests { // Fails to dispatch new message from different than latest relayer. assert_eq!( false, - lane.receive_message::( - TEST_RELAYER_A + max_nonce + 1, + lane.receive_message::( + &(TEST_RELAYER_A + max_nonce + 1), + &(TEST_RELAYER_A + max_nonce + 1), max_nonce + 1, message_data(REGULAR_PAYLOAD).into() ) @@ -302,8 +311,9 @@ mod tests { // Fails to dispatch new messages from latest relayer. Prevents griefing attacks. assert_eq!( false, - lane.receive_message::( - TEST_RELAYER_A + max_nonce, + lane.receive_message::( + &(TEST_RELAYER_A + max_nonce), + &(TEST_RELAYER_A + max_nonce), max_nonce + 1, message_data(REGULAR_PAYLOAD).into() ) @@ -317,8 +327,9 @@ mod tests { let mut lane = inbound_lane::(TEST_LANE_ID); let max_nonce = ::MaxUnconfirmedMessagesAtInboundLane::get(); for current_nonce in 1..=max_nonce { - assert!(lane.receive_message::( - TEST_RELAYER_A, + assert!(lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, current_nonce, message_data(REGULAR_PAYLOAD).into() )); @@ -326,8 +337,9 @@ mod tests { // Fails to dispatch new message from different than latest relayer. assert_eq!( false, - lane.receive_message::( - TEST_RELAYER_B, + lane.receive_message::( + &TEST_RELAYER_B, + &TEST_RELAYER_B, max_nonce + 1, message_data(REGULAR_PAYLOAD).into() ) @@ -335,8 +347,9 @@ mod tests { // Fails to dispatch new messages from latest relayer. assert_eq!( false, - lane.receive_message::( - TEST_RELAYER_A, + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, max_nonce + 1, message_data(REGULAR_PAYLOAD).into() ) @@ -348,18 +361,21 @@ mod tests { fn correctly_receives_following_messages_from_two_relayers_alternately() { run_test(|| { let mut lane = inbound_lane::(TEST_LANE_ID); - assert!(lane.receive_message::( - TEST_RELAYER_A, + assert!(lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, 1, message_data(REGULAR_PAYLOAD).into() )); - assert!(lane.receive_message::( - TEST_RELAYER_B, + assert!(lane.receive_message::( + &TEST_RELAYER_B, + &TEST_RELAYER_B, 2, message_data(REGULAR_PAYLOAD).into() )); - assert!(lane.receive_message::( - TEST_RELAYER_A, + assert!(lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, 3, message_data(REGULAR_PAYLOAD).into() )); @@ -374,14 +390,20 @@ mod tests { fn rejects_same_message_from_two_different_relayers() { run_test(|| { let mut lane = inbound_lane::(TEST_LANE_ID); - assert!(lane.receive_message::( - TEST_RELAYER_A, + assert!(lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, 1, message_data(REGULAR_PAYLOAD).into() )); assert_eq!( false, - lane.receive_message::(TEST_RELAYER_B, 1, message_data(REGULAR_PAYLOAD).into()) + lane.receive_message::( + &TEST_RELAYER_B, + &TEST_RELAYER_B, + 1, + message_data(REGULAR_PAYLOAD).into() + ) ); }); } diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 9e2563498f..6da45f950a 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -148,7 +148,11 @@ pub trait Config: frame_system::Config { /// Source header chain, as it is represented on target chain. type SourceHeaderChain: SourceHeaderChain; /// Message dispatch. - type MessageDispatch: MessageDispatch; + type MessageDispatch: MessageDispatch< + Self::AccountId, + Self::InboundMessageFee, + DispatchPayload = Self::InboundPayload, + >; } /// Shortcut to messages proof type for Config. @@ -434,13 +438,13 @@ decl_module! { #[weight = T::WeightInfo::receive_messages_proof_weight(proof, *messages_count, *dispatch_weight)] pub fn receive_messages_proof( origin, - relayer_id: T::InboundRelayer, + relayer_id_at_bridged_chain: T::InboundRelayer, proof: MessagesProofOf, messages_count: u32, dispatch_weight: Weight, ) -> DispatchResult { ensure_operational::()?; - let _ = ensure_signed(origin)?; + let relayer_id_at_this_chain = ensure_signed(origin)?; // reject transactions that are declaring too many messages ensure!( @@ -507,7 +511,13 @@ decl_module! { debug_assert_eq!(message.key.lane_id, lane_id); total_messages += 1; - if lane.receive_message::(relayer_id.clone(), message.key.nonce, message.data) { + // TODO: refund unspent weight here + if lane.receive_message::( + &relayer_id_at_bridged_chain, + &relayer_id_at_this_chain, + message.key.nonce, + message.data, + ) { valid_messages += 1; } } diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index e640fa7805..b3d5905739 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -357,7 +357,7 @@ impl SourceHeaderChain for TestSourceHeaderChain { #[derive(Debug)] pub struct TestMessageDispatch; -impl MessageDispatch for TestMessageDispatch { +impl MessageDispatch for TestMessageDispatch { type DispatchPayload = TestPayload; fn dispatch_weight(message: &DispatchMessage) -> Weight { @@ -367,7 +367,9 @@ impl MessageDispatch for TestMessageDispatch { } } - fn dispatch(_message: DispatchMessage) {} + fn dispatch(_relayer_account: &AccountId, _message: DispatchMessage) -> Weight { + 0 + } } /// Return test lane message with given nonce and payload. diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index 3b83e38517..2fbcfc86e8 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -25,7 +25,7 @@ use bp_runtime::InstanceId; pub type Weight = u64; /// A generic trait to dispatch arbitrary messages delivered over the bridge. -pub trait MessageDispatch { +pub trait MessageDispatch { /// A type of the message to be dispatched. type Message: codec::Decode; @@ -45,5 +45,12 @@ pub trait MessageDispatch { /// a sign that some other component has rejected the message even before it has /// reached `dispatch` method (right now this may only be caused if we fail to decode /// the whole message). - fn dispatch(bridge: InstanceId, id: MessageId, message: Result); + /// + /// Returns unspent dispatch weight. + fn dispatch Result<(), ()>>( + bridge: InstanceId, + id: MessageId, + message: Result, + pay_dispatch_fee: P, + ) -> Weight; } diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index 676e919bc6..9db2c1a8eb 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -84,7 +84,7 @@ pub trait SourceHeaderChain { } /// Called when inbound message is received. -pub trait MessageDispatch { +pub trait MessageDispatch { /// Decoded message payload type. Valid message may contain invalid payload. In this case /// message is delivered, but dispatch fails. Therefore, two separate types of payload /// (opaque `MessagePayload` used in delivery and this `DispatchPayload` used in dispatch). @@ -100,7 +100,17 @@ pub trait MessageDispatch { /// /// It is up to the implementers of this trait to determine whether the message /// is invalid (i.e. improperly encoded, has too large weight, ...) or not. - fn dispatch(message: DispatchMessage); + /// + /// If your configuration allows paying dispatch fee at the target chain, then + /// it must be paid inside this method to the `relayer_account`. + /// + /// Returns 'unused' dispatch weight that will be deducted from total delivery transaction + /// weight, thus reducing the transaction cost. This shall not be zero in two cases: + /// + /// 1) if message has been dispatched successfully, but post-dispatch weight is less than + /// the weight, declared by the message sender; + /// 2) if message has not been dispatched at all. + fn dispatch(relayer_account: &AccountId, message: DispatchMessage) -> Weight; } impl Default for ProvedLaneMessages { @@ -149,12 +159,14 @@ impl SourceHeaderChain for ForbidInboundMessages { } } -impl MessageDispatch for ForbidInboundMessages { +impl MessageDispatch for ForbidInboundMessages { type DispatchPayload = (); fn dispatch_weight(_message: &DispatchMessage) -> Weight { Weight::MAX } - fn dispatch(_message: DispatchMessage) {} + fn dispatch(_relayer_account: &AccountId, _message: DispatchMessage) -> Weight { + 0 + } } diff --git a/relays/bin-substrate/src/cli/encode_message.rs b/relays/bin-substrate/src/cli/encode_message.rs index a29aa8597d..b5a38ec3de 100644 --- a/relays/bin-substrate/src/cli/encode_message.rs +++ b/relays/bin-substrate/src/cli/encode_message.rs @@ -72,7 +72,7 @@ mod tests { #[test] fn should_encode_raw_message() { // given - let msg = "01000000e88514000000000002d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d3c040130000000000000000000000000"; + let msg = "01000000e88514000000000002d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d003c040130000000000000000000000000"; let encode_message = EncodeMessage::from_iter(vec!["encode-message", "MillauToRialto", "raw", msg]); // when @@ -101,6 +101,6 @@ mod tests { let hex = encode_message.encode().unwrap(); // then - assert_eq!(format!("{:?}", hex), "0x01000000e88514000000000002d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d3c040130000000000000000000000000"); + assert_eq!(format!("{:?}", hex), "0x01000000e88514000000000002d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d003c040130000000000000000000000000"); } } diff --git a/relays/bin-substrate/src/cli/send_message.rs b/relays/bin-substrate/src/cli/send_message.rs index 64448f0f1d..36f7a52af5 100644 --- a/relays/bin-substrate/src/cli/send_message.rs +++ b/relays/bin-substrate/src/cli/send_message.rs @@ -210,6 +210,7 @@ where spec_version, weight, origin, + pay_dispatch_fee_at_target_chain: false, call: HexBytes::encode(call), }; @@ -221,12 +222,14 @@ where spec_version, weight, origin, + pay_dispatch_fee_at_target_chain, call, } = payload; MessagePayload { spec_version, weight, origin, + pay_dispatch_fee_at_target_chain, call: call.0, } } @@ -267,6 +270,7 @@ mod tests { spec_version: relay_millau_client::Millau::RUNTIME_VERSION.spec_version, weight: 1345000, origin: CallOrigin::SourceAccount(sp_keyring::AccountKeyring::Alice.to_account_id()), + pay_dispatch_fee_at_target_chain: false, call: hex!("0401081234").to_vec(), } ); @@ -310,6 +314,7 @@ mod tests { sp_keyring::AccountKeyring::Bob.into(), signature, ), + pay_dispatch_fee_at_target_chain: false, call: hex!("0701081234").to_vec(), } ); From b61a966b7dd4095aa19b8fc7a91266a4d530f2d0 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 20 Apr 2021 13:57:37 +0300 Subject: [PATCH 02/14] refund unspent dispatch weight to messages relayer --- modules/messages/src/inbound_lane.rs | 191 +++++++++++++++++---------- modules/messages/src/lib.rs | 142 ++++++++++++++++---- modules/messages/src/mock.rs | 32 ++++- 3 files changed, 264 insertions(+), 101 deletions(-) diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index c671d2c6ae..9fca9aac64 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -18,8 +18,9 @@ use bp_messages::{ target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, - InboundLaneData, LaneId, MessageKey, MessageNonce, OutboundLaneData, + InboundLaneData, LaneId, MessageKey, MessageNonce, OutboundLaneData, Weight, }; +use frame_support::RuntimeDebug; use sp_std::prelude::PartialEq; /// Inbound lane storage. @@ -41,6 +42,22 @@ pub trait InboundLaneStorage { fn set_data(&mut self, data: InboundLaneData); } +/// Result of single message receival. +#[derive(RuntimeDebug, PartialEq, Eq)] +pub enum ReceivalResult { + /// Message has been received and dispatched. Note that we don't care whether dispatch has + /// been successful or not - in both case message falls into this category. + /// + /// The unspent message dispatch weight is also returned. + Dispatched(Weight), + /// Message has invalid nonce and lane has rejected to accept this message. + InvalidNonce, + /// There are too many unrewarded relayer entires at the lane. + TooManyUnrewardedRelayers, + /// There are too many unconfirmed messages at the lane. + TooManyUnconfirmedMessages, +} + /// Inbound messages lane. pub struct InboundLane { storage: S, @@ -96,22 +113,22 @@ impl InboundLane { relayer_at_this_chain: &AccountId, nonce: MessageNonce, message_data: DispatchMessageData, - ) -> bool { + ) -> ReceivalResult { let mut data = self.storage.data(); let is_correct_message = nonce == data.last_delivered_nonce() + 1; if !is_correct_message { - return false; + return ReceivalResult::InvalidNonce; } // if there are more unrewarded relayer entries than we may accept, reject this message if data.relayers.len() as MessageNonce >= self.storage.max_unrewarded_relayer_entries() { - return false; + return ReceivalResult::TooManyUnrewardedRelayers; } // if there are more unconfirmed messages than we may accept, reject this message let unconfirmed_messages_count = nonce.saturating_sub(data.last_confirmed_nonce); if unconfirmed_messages_count > self.storage.max_unconfirmed_messages() { - return false; + return ReceivalResult::TooManyUnconfirmedMessages; } let push_new = match data.relayers.back_mut() { @@ -128,7 +145,7 @@ impl InboundLane { self.storage.set_data(data); - P::dispatch( + ReceivalResult::Dispatched(P::dispatch( relayer_at_this_chain, DispatchMessage { key: MessageKey { @@ -137,9 +154,7 @@ impl InboundLane { }, data: message_data, }, - ); - - true + )) } } @@ -159,12 +174,15 @@ mod tests { lane: &mut InboundLane>, nonce: MessageNonce, ) { - assert!(lane.receive_message::( - &TEST_RELAYER_A, - &TEST_RELAYER_A, - nonce, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + nonce, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(0) + ); } #[test] @@ -275,12 +293,15 @@ mod tests { fn fails_to_receive_message_with_incorrect_nonce() { run_test(|| { let mut lane = inbound_lane::(TEST_LANE_ID); - assert!(!lane.receive_message::( - &TEST_RELAYER_A, - &TEST_RELAYER_A, - 10, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + 10, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::InvalidNonce + ); assert_eq!(lane.storage.data().last_delivered_nonce(), 0); }); } @@ -291,32 +312,35 @@ mod tests { let mut lane = inbound_lane::(TEST_LANE_ID); let max_nonce = ::MaxUnrewardedRelayerEntriesAtInboundLane::get(); for current_nonce in 1..max_nonce + 1 { - assert!(lane.receive_message::( - &(TEST_RELAYER_A + current_nonce), - &(TEST_RELAYER_A + current_nonce), - current_nonce, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + lane.receive_message::( + &(TEST_RELAYER_A + current_nonce), + &(TEST_RELAYER_A + current_nonce), + current_nonce, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(0) + ); } // Fails to dispatch new message from different than latest relayer. assert_eq!( - false, lane.receive_message::( &(TEST_RELAYER_A + max_nonce + 1), &(TEST_RELAYER_A + max_nonce + 1), max_nonce + 1, message_data(REGULAR_PAYLOAD).into() - ) + ), + ReceivalResult::TooManyUnrewardedRelayers, ); // Fails to dispatch new messages from latest relayer. Prevents griefing attacks. assert_eq!( - false, lane.receive_message::( &(TEST_RELAYER_A + max_nonce), &(TEST_RELAYER_A + max_nonce), max_nonce + 1, message_data(REGULAR_PAYLOAD).into() - ) + ), + ReceivalResult::TooManyUnrewardedRelayers, ); }); } @@ -327,32 +351,35 @@ mod tests { let mut lane = inbound_lane::(TEST_LANE_ID); let max_nonce = ::MaxUnconfirmedMessagesAtInboundLane::get(); for current_nonce in 1..=max_nonce { - assert!(lane.receive_message::( - &TEST_RELAYER_A, - &TEST_RELAYER_A, - current_nonce, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + current_nonce, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(0) + ); } // Fails to dispatch new message from different than latest relayer. assert_eq!( - false, lane.receive_message::( &TEST_RELAYER_B, &TEST_RELAYER_B, max_nonce + 1, message_data(REGULAR_PAYLOAD).into() - ) + ), + ReceivalResult::TooManyUnconfirmedMessages, ); // Fails to dispatch new messages from latest relayer. assert_eq!( - false, lane.receive_message::( &TEST_RELAYER_A, &TEST_RELAYER_A, max_nonce + 1, message_data(REGULAR_PAYLOAD).into() - ) + ), + ReceivalResult::TooManyUnconfirmedMessages, ); }); } @@ -361,24 +388,33 @@ mod tests { fn correctly_receives_following_messages_from_two_relayers_alternately() { run_test(|| { let mut lane = inbound_lane::(TEST_LANE_ID); - assert!(lane.receive_message::( - &TEST_RELAYER_A, - &TEST_RELAYER_A, - 1, - message_data(REGULAR_PAYLOAD).into() - )); - assert!(lane.receive_message::( - &TEST_RELAYER_B, - &TEST_RELAYER_B, - 2, - message_data(REGULAR_PAYLOAD).into() - )); - assert!(lane.receive_message::( - &TEST_RELAYER_A, - &TEST_RELAYER_A, - 3, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + 1, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(0) + ); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_B, + &TEST_RELAYER_B, + 2, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(0) + ); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + 3, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(0) + ); assert_eq!( lane.storage.data().relayers, vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B), (3, 3, TEST_RELAYER_A)] @@ -390,20 +426,23 @@ mod tests { fn rejects_same_message_from_two_different_relayers() { run_test(|| { let mut lane = inbound_lane::(TEST_LANE_ID); - assert!(lane.receive_message::( - &TEST_RELAYER_A, - &TEST_RELAYER_A, - 1, - message_data(REGULAR_PAYLOAD).into() - )); assert_eq!( - false, + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + 1, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(0) + ); + assert_eq!( lane.receive_message::( &TEST_RELAYER_B, &TEST_RELAYER_B, 1, message_data(REGULAR_PAYLOAD).into() - ) + ), + ReceivalResult::InvalidNonce, ); }); } @@ -416,4 +455,22 @@ mod tests { assert_eq!(lane.storage.data().last_delivered_nonce(), 1); }); } + + #[test] + fn unspent_weight_is_returned_by_receive_message() { + run_test(|| { + let mut lane = inbound_lane::(TEST_LANE_ID); + let mut payload = REGULAR_PAYLOAD; + payload.unspent_weight = 1; + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + 1, + message_data(payload).into() + ), + ReceivalResult::Dispatched(1) + ); + }); + } } diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 6da45f950a..835cec2f96 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -40,7 +40,7 @@ pub use crate::weights_ext::{ EXPECTED_DEFAULT_MESSAGE_LENGTH, }; -use crate::inbound_lane::{InboundLane, InboundLaneStorage}; +use crate::inbound_lane::{InboundLane, InboundLaneStorage, ReceivalResult}; use crate::outbound_lane::{OutboundLane, OutboundLaneStorage}; use crate::weights::WeightInfo; @@ -53,9 +53,11 @@ use bp_messages::{ use bp_runtime::Size; use codec::{Decode, Encode}; use frame_support::{ - decl_error, decl_event, decl_module, decl_storage, ensure, + decl_error, decl_event, decl_module, decl_storage, + dispatch::DispatchResultWithPostInfo, + ensure, traits::Get, - weights::{DispatchClass, Weight}, + weights::{DispatchClass, Pays, PostDispatchInfo, Weight}, Parameter, StorageMap, }; use frame_system::{ensure_signed, RawOrigin}; @@ -442,7 +444,7 @@ decl_module! { proof: MessagesProofOf, messages_count: u32, dispatch_weight: Weight, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { ensure_operational::()?; let relayer_id_at_this_chain = ensure_signed(origin)?; @@ -452,6 +454,23 @@ decl_module! { Error::::TooManyMessagesInTheProof ); + // why do we need to know the weight of this (`receive_messages_proof`) call? Because + // we may want to return some funds for not-dispatching (or partially dispatching) some + // messages to the call origin (relayer). And this is done by returning actual weight + // from the call. But we only know dispatch weight of every messages. So to refund relayer + // because we have not dispatched Message, we need to: + // + // ActualWeight = DeclaredWeight - Message.DispatchWeight + // + // The DeclaredWeight is exactly what's computed here. Unfortunately it is impossible + // to get pre-computed value (and it has been already computed by the executive). + let declared_weight = T::WeightInfo::receive_messages_proof_weight( + &proof, + messages_count, + dispatch_weight, + ); + let mut actual_weight = declared_weight; + // verify messages proof && convert proof into messages let messages = verify_and_decode_messages_proof::< T::SourceHeaderChain, @@ -511,26 +530,46 @@ decl_module! { debug_assert_eq!(message.key.lane_id, lane_id); total_messages += 1; - // TODO: refund unspent weight here - if lane.receive_message::( + let dispatch_weight = T::MessageDispatch::dispatch_weight(&message); + let receival_result = lane.receive_message::( &relayer_id_at_bridged_chain, &relayer_id_at_this_chain, message.key.nonce, message.data, - ) { - valid_messages += 1; - } + ); + + // note that we're returning unspent weight to relayer even if message has been + // rejected by the lane. This allows relayers to submit spam transactions with + // e.g. the same set of already delivered messages over and over again, without + // losing funds for messages dispatch. But keep in mind that relayer pays base + // delivery transaction cost anyway. And base cost covers everything except + // dispatch, so we have a balance here. + let unspent_weight = match receival_result { + ReceivalResult::Dispatched(unspent_weight) => { + valid_messages += 1; + unspent_weight + }, + ReceivalResult::InvalidNonce + | ReceivalResult::TooManyUnrewardedRelayers + | ReceivalResult::TooManyUnconfirmedMessages => dispatch_weight, + }; + actual_weight = actual_weight.saturating_sub(sp_std::cmp::min(unspent_weight, dispatch_weight)); } } log::trace!( target: "runtime::bridge-messages", - "Received messages: total={}, valid={}", + "Received messages: total={}, valid={}. Weight used: {}/{}", total_messages, valid_messages, + actual_weight, + declared_weight, ); - Ok(()) + Ok(PostDispatchInfo { + actual_weight: Some(actual_weight), + pays_fee: Pays::Yes, + }) } /// Receive messages delivery proof from bridged chain. @@ -857,10 +896,9 @@ fn verify_and_decode_messages_proof, Fee, Dispatch mod tests { use super::*; use crate::mock::{ - message, run_test, Event as TestEvent, Origin, TestMessageDeliveryAndDispatchPayment, - TestMessagesDeliveryProof, TestMessagesParameter, TestMessagesProof, TestPayload, TestRuntime, - TokenConversionRate, PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_RELAYER_A, - TEST_RELAYER_B, + message, message_payload, run_test, Event as TestEvent, Origin, TestMessageDeliveryAndDispatchPayment, + TestMessagesDeliveryProof, TestMessagesParameter, TestMessagesProof, TestRuntime, TokenConversionRate, + PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_RELAYER_A, TEST_RELAYER_B, }; use bp_messages::UnrewardedRelayersState; use frame_support::{assert_noop, assert_ok}; @@ -880,7 +918,7 @@ mod tests { Origin::signed(1), TEST_LANE_ID, REGULAR_PAYLOAD, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, )); // check event with assigned nonce @@ -894,7 +932,10 @@ mod tests { ); // check that fee has been withdrawn from submitter - assert!(TestMessageDeliveryAndDispatchPayment::is_fee_paid(1, REGULAR_PAYLOAD.1)); + assert!(TestMessageDeliveryAndDispatchPayment::is_fee_paid( + 1, + REGULAR_PAYLOAD.declared_weight + )); } fn receive_messages_delivery_proof() { @@ -1089,7 +1130,7 @@ mod tests { Origin::signed(1), TEST_LANE_ID, REGULAR_PAYLOAD, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, ), Error::::Halted, ); @@ -1100,7 +1141,7 @@ mod tests { TEST_RELAYER_A, Ok(vec![message(2, REGULAR_PAYLOAD)]).into(), 1, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, ), Error::::Halted, ); @@ -1138,7 +1179,7 @@ mod tests { Origin::signed(1), TEST_LANE_ID, PAYLOAD_REJECTED_BY_TARGET_CHAIN, - PAYLOAD_REJECTED_BY_TARGET_CHAIN.1 + PAYLOAD_REJECTED_BY_TARGET_CHAIN.declared_weight ), Error::::MessageRejectedByChainVerifier, ); @@ -1165,7 +1206,7 @@ mod tests { Origin::signed(1), TEST_LANE_ID, REGULAR_PAYLOAD, - REGULAR_PAYLOAD.1 + REGULAR_PAYLOAD.declared_weight ), Error::::FailedToWithdrawMessageFee, ); @@ -1180,7 +1221,7 @@ mod tests { TEST_RELAYER_A, Ok(vec![message(1, REGULAR_PAYLOAD)]).into(), 1, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, )); assert_eq!(InboundLanes::::get(TEST_LANE_ID).last_delivered_nonce(), 1); @@ -1221,7 +1262,7 @@ mod tests { TEST_RELAYER_A, message_proof, 1, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, )); assert_eq!( @@ -1253,7 +1294,7 @@ mod tests { TEST_RELAYER_A, Ok(vec![message(1, REGULAR_PAYLOAD)]).into(), 1, - REGULAR_PAYLOAD.1 - 1, + REGULAR_PAYLOAD.declared_weight - 1, ), Error::::InvalidMessagesDispatchWeight, ); @@ -1475,7 +1516,7 @@ mod tests { ]) .into(), 3, - REGULAR_PAYLOAD.1 + REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight + REGULAR_PAYLOAD.declared_weight, ),); assert_eq!( @@ -1527,9 +1568,9 @@ mod tests { #[test] fn actual_dispatch_weight_does_not_overlow() { run_test(|| { - let message1 = message(1, TestPayload(0, Weight::MAX / 2)); - let message2 = message(2, TestPayload(0, Weight::MAX / 2)); - let message3 = message(2, TestPayload(0, Weight::MAX / 2)); + let message1 = message(1, message_payload(0, Weight::MAX / 2)); + let message2 = message(2, message_payload(0, Weight::MAX / 2)); + let message3 = message(2, message_payload(0, Weight::MAX / 2)); assert_noop!( Pallet::::receive_messages_proof( @@ -1596,4 +1637,49 @@ mod tests { assert!(TestMessageDeliveryAndDispatchPayment::is_fee_paid(1, 100)); }); } + + #[test] + fn weight_refund_from_receive_messages_proof_works() { + run_test(|| { + fn submit_with_unspent_weight(nonce: MessageNonce, unspent_weight: Weight) -> (Weight, Weight) { + let mut payload = REGULAR_PAYLOAD; + payload.unspent_weight = unspent_weight; + let proof = Ok(vec![message(nonce, payload)]).into(); + let messages_count = 1; + let pre_dispatch_weight = ::WeightInfo::receive_messages_proof_weight( + &proof, + messages_count, + REGULAR_PAYLOAD.declared_weight, + ); + let post_dispatch_weight = Pallet::::receive_messages_proof( + Origin::signed(1), + TEST_RELAYER_A, + proof, + messages_count, + REGULAR_PAYLOAD.declared_weight, + ) + .expect("delivery has failed") + .actual_weight + .expect("receive_messages_proof always returns Some"); + + (pre_dispatch_weight, post_dispatch_weight) + } + + // when dispatch is returning `unspent_weight < declared_weight` + let (pre, post) = submit_with_unspent_weight(1, 1); + assert_eq!(post, pre - 1); + + // when dispatch is returning `unspent_weight = declared_weight` + let (pre, post) = submit_with_unspent_weight(2, REGULAR_PAYLOAD.declared_weight); + assert_eq!(post, pre - REGULAR_PAYLOAD.declared_weight); + + // when dispatch is returning `unspent_weight > declared_weight` + let (pre, post) = submit_with_unspent_weight(3, REGULAR_PAYLOAD.declared_weight + 1); + assert_eq!(post, pre - REGULAR_PAYLOAD.declared_weight); + + // when there's no unspent weight + let (pre, post) = submit_with_unspent_weight(4, 0); + assert_eq!(post, pre); + }); + } } diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index b3d5905739..96d2e5f4ec 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -41,7 +41,15 @@ use std::collections::BTreeMap; pub type AccountId = u64; pub type Balance = u64; #[derive(Decode, Encode, Clone, Debug, PartialEq, Eq)] -pub struct TestPayload(pub u64, pub Weight); +pub struct TestPayload { + /// Field that may be used to identify messages. + pub id: u64, + /// Dispatch weight that is declared by the message sender. + pub declared_weight: Weight, + /// Unspent weight. In correct code it'll always be <= `declared_weight`, but for test + /// purposes we'll be making it larger than `declared_weight` sometimes. + pub unspent_weight: Weight, +} pub type TestMessageFee = u64; pub type TestRelayer = u64; @@ -187,10 +195,10 @@ pub const TEST_ERROR: &str = "Test error"; pub const TEST_LANE_ID: LaneId = [0, 0, 0, 1]; /// Regular message payload. -pub const REGULAR_PAYLOAD: TestPayload = TestPayload(0, 50); +pub const REGULAR_PAYLOAD: TestPayload = message_payload(0, 50); /// Payload that is rejected by `TestTargetHeaderChain`. -pub const PAYLOAD_REJECTED_BY_TARGET_CHAIN: TestPayload = TestPayload(1, 50); +pub const PAYLOAD_REJECTED_BY_TARGET_CHAIN: TestPayload = message_payload(1, 50); /// Vec of proved messages, grouped by lane. pub type MessagesByLaneVec = Vec<(LaneId, ProvedLaneMessages>)>; @@ -362,13 +370,16 @@ impl MessageDispatch for TestMessageDispatch { fn dispatch_weight(message: &DispatchMessage) -> Weight { match message.data.payload.as_ref() { - Ok(payload) => payload.1, + Ok(payload) => payload.declared_weight, Err(_) => 0, } } - fn dispatch(_relayer_account: &AccountId, _message: DispatchMessage) -> Weight { - 0 + fn dispatch(_relayer_account: &AccountId, message: DispatchMessage) -> Weight { + match message.data.payload.as_ref() { + Ok(payload) => payload.unspent_weight, + Err(_) => 0, + } } } @@ -383,6 +394,15 @@ pub fn message(nonce: MessageNonce, payload: TestPayload) -> Message TestPayload { + TestPayload { + id, + declared_weight, + unspent_weight: 0, + } +} + /// Return message data with valid fee for given payload. pub fn message_data(payload: TestPayload) -> MessageData { MessageData { From cae890244cd97eb2bcda3ea58b01f0d5f5a59632 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 20 Apr 2021 16:11:36 +0300 Subject: [PATCH 03/14] test that transfer actually happens --- bin/rialto/runtime/src/millau_messages.rs | 84 +++++++++++++++++++++++ bin/runtime-common/src/messages.rs | 21 ++++-- 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/bin/rialto/runtime/src/millau_messages.rs b/bin/rialto/runtime/src/millau_messages.rs index be0e48c381..da85b6efa9 100644 --- a/bin/rialto/runtime/src/millau_messages.rs +++ b/bin/rialto/runtime/src/millau_messages.rs @@ -252,3 +252,87 @@ impl MessagesParameter for RialtoToMillauMessagesParameter { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{AccountId, Call, ExistentialDeposit, Runtime, SystemCall, SystemConfig, VERSION}; + use bp_messages::{ + target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, + MessageKey, + }; + use bp_runtime::{derive_account_id, SourceAccount}; + use bridge_runtime_common::messages::target::{FromBridgedChainEncodedMessageCall, FromBridgedChainMessagePayload}; + use frame_support::{ + traits::Currency, + weights::{GetDispatchInfo, WeightToFeePolynomial}, + }; + use pallet_bridge_dispatch::CallOrigin; + use sp_runtime::traits::Convert; + + #[test] + fn transfer_happens_when_dispatch_fee_is_paid_at_target_chain() { + // this test actually belongs to the `bridge-runtime-common` crate, but there we have no + // mock runtime. Making another one there just for this test, given that both crates + // live n single repo is an overkill + let mut ext: sp_io::TestExternalities = SystemConfig::default().build_storage::().unwrap().into(); + ext.execute_with(|| { + let bridge = MILLAU_BRIDGE_INSTANCE; + let call: Call = SystemCall::remark(vec![]).into(); + let dispatch_weight = call.get_dispatch_info().weight; + let dispatch_fee = ::WeightToFee::calc(&dispatch_weight); + assert!(dispatch_fee > 0); + + // create relayer account with minimal balance + let relayer_account: AccountId = [1u8; 32].into(); + let initial_amount = ExistentialDeposit::get(); + let _ = as Currency>::deposit_creating( + &relayer_account, + initial_amount, + ); + + // create dispatch account with minimal balance + dispatch fee + let dispatch_account = derive_account_id::<::SourceChainAccountId>( + bridge, + SourceAccount::Root, + ); + let dispatch_account = + ::AccountIdConverter::convert(dispatch_account); + let _ = as Currency>::deposit_creating( + &dispatch_account, + initial_amount + dispatch_fee, + ); + + // dispatch message with intention to pay dispatch fee at the target chain + FromMillauMessageDispatch::dispatch( + &relayer_account, + DispatchMessage { + key: MessageKey { + lane_id: Default::default(), + nonce: 0, + }, + data: DispatchMessageData { + payload: Ok(FromBridgedChainMessagePayload:: { + spec_version: VERSION.spec_version, + weight: dispatch_weight, + origin: CallOrigin::SourceRoot, + pay_dispatch_fee_at_target_chain: true, + call: FromBridgedChainEncodedMessageCall::new(call.encode()), + }), + fee: 1, + }, + }, + ); + + // ensure that fee has been transferred from dispatch to relayer account + assert_eq!( + as Currency>::free_balance(&relayer_account), + initial_amount + dispatch_fee, + ); + assert_eq!( + as Currency>::free_balance(&dispatch_account), + initial_amount, + ); + }); + } +} diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index 0df9577176..241fd3f148 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -457,8 +457,18 @@ pub mod target { /// vector length. Custom decode implementation here is exactly to deal with this. #[derive(Decode, Encode, RuntimeDebug, PartialEq)] pub struct FromBridgedChainEncodedMessageCall { - pub(crate) encoded_call: Vec, - pub(crate) _marker: PhantomData, + encoded_call: Vec, + _marker: PhantomData, + } + + impl FromBridgedChainEncodedMessageCall { + /// Create encoded call. + pub fn new(encoded_call: Vec) -> Self { + FromBridgedChainEncodedMessageCall { + encoded_call, + _marker: PhantomData::default(), + } + } } impl From> for Result>, ()> { @@ -979,10 +989,9 @@ mod tests { weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, pay_dispatch_fee_at_target_chain: true, - call: target::FromBridgedChainEncodedMessageCall:: { - encoded_call: ThisChainCall::Transfer.encode(), - _marker: PhantomData::default(), - }, + call: target::FromBridgedChainEncodedMessageCall::::new( + ThisChainCall::Transfer.encode(), + ), } ); assert_eq!(Ok(ThisChainCall::Transfer), message_on_this_chain.call.into()); From eb35c615ed58f56721d996bb2ce6ccf3dc7b8015 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 20 Apr 2021 17:44:06 +0300 Subject: [PATCH 04/14] pay-at-target-cchain benchmarks + fix previous benchmarks (invalid signature) --- bin/rialto/runtime/src/lib.rs | 23 ++- .../src/messages_benchmarking.rs | 13 +- modules/messages/src/benchmarking.rs | 107 +++++++++- modules/messages/src/weights.rs | 184 ++++++++++-------- 4 files changed, 236 insertions(+), 91 deletions(-) diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index 4e81d3efb1..fbcb66c9c9 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -883,6 +883,7 @@ impl_runtime_apis! { weight: params.size as _, origin: dispatch_origin, call: message_payload, + pay_dispatch_fee_at_target_chain: false, }; (message, pallet_bridge_messages::benchmarking::MESSAGE_FEE.into()) } @@ -899,7 +900,7 @@ impl_runtime_apis! { use codec::Encode; use frame_support::weights::GetDispatchInfo; use pallet_bridge_messages::storage_keys; - use sp_runtime::traits::Header; + use sp_runtime::traits::{Header, IdentifyAccount}; let remark = match params.size { MessagesProofSize::Minimal(ref size) => vec![0u8; *size as _], @@ -912,12 +913,18 @@ impl_runtime_apis! { let (rialto_raw_public, rialto_raw_signature) = ed25519_sign( &call, &millau_account_id, + VERSION.spec_version, + bp_runtime::MILLAU_BRIDGE_INSTANCE, ); let rialto_public = MultiSigner::Ed25519(sp_core::ed25519::Public::from_raw(rialto_raw_public)); let rialto_signature = MultiSignature::Ed25519(sp_core::ed25519::Signature::from_raw( rialto_raw_signature, )); + if params.pay_dispatch_fee_at_target_chain { + Self::endow_account(&rialto_public.clone().into_account()); + } + let make_millau_message_key = |message_key: MessageKey| storage_keys::message_key::< Runtime, ::MessagesInstance, @@ -938,6 +945,7 @@ impl_runtime_apis! { Default::default(), ); + let pay_dispatch_fee_at_target_chain = params.pay_dispatch_fee_at_target_chain; prepare_message_proof::( params, make_millau_message_key, @@ -956,6 +964,7 @@ impl_runtime_apis! { rialto_public, rialto_signature, ), + pay_dispatch_fee_at_target_chain, call: call.encode(), }.encode(), ) @@ -988,6 +997,18 @@ impl_runtime_apis! { ), ) } + + fn is_message_dispatched(nonce: bp_messages::MessageNonce) -> bool { + frame_system::Pallet::::events() + .into_iter() + .map(|event_record| event_record.event) + .any(|event| matches!( + event, + Event::pallet_bridge_dispatch(pallet_bridge_dispatch::Event::::MessageDispatched( + _, ([0, 0, 0, 0], nonce_from_event), _, + )) if nonce_from_event == nonce + )) + } } add_benchmark!( diff --git a/bin/runtime-common/src/messages_benchmarking.rs b/bin/runtime-common/src/messages_benchmarking.rs index 639e5f6c50..0f23e0bba5 100644 --- a/bin/runtime-common/src/messages_benchmarking.rs +++ b/bin/runtime-common/src/messages_benchmarking.rs @@ -25,6 +25,7 @@ use crate::messages::{ }; use bp_messages::{LaneId, MessageData, MessageKey, MessagePayload}; +use bp_runtime::InstanceId; use codec::Encode; use ed25519_dalek::{PublicKey, SecretKey, Signer, KEYPAIR_LENGTH, SECRET_KEY_LENGTH}; use frame_support::weights::Weight; @@ -37,7 +38,12 @@ use sp_trie::{record_all_keys, trie_types::TrieDBMut, Layout, MemoryDB, Recorder /// Generate ed25519 signature to be used in `pallet_brdige_call_dispatch::CallOrigin::TargetAccount`. /// /// Returns public key of the signer and the signature itself. -pub fn ed25519_sign(target_call: &impl Encode, source_account_id: &impl Encode) -> ([u8; 32], [u8; 64]) { +pub fn ed25519_sign( + target_call: &impl Encode, + source_account_id: &impl Encode, + target_spec_version: u32, + bridge: InstanceId, +) -> ([u8; 32], [u8; 64]) { // key from the repo example (https://docs.rs/ed25519-dalek/1.0.1/ed25519_dalek/struct.SecretKey.html) let target_secret = SecretKey::from_bytes(&[ 157, 097, 177, 157, 239, 253, 090, 096, 186, 132, 074, 244, 146, 236, 044, 196, 068, 073, 197, 105, 123, 050, @@ -51,9 +57,8 @@ pub fn ed25519_sign(target_call: &impl Encode, source_account_id: &impl Encode) target_pair_bytes[SECRET_KEY_LENGTH..].copy_from_slice(&target_public.to_bytes()); let target_pair = ed25519_dalek::Keypair::from_bytes(&target_pair_bytes).expect("hardcoded pair is valid"); - let mut signature_message = Vec::new(); - target_call.encode_to(&mut signature_message); - source_account_id.encode_to(&mut signature_message); + let signature_message = + pallet_bridge_dispatch::account_ownership_digest(target_call, source_account_id, target_spec_version, bridge); let target_origin_signature = target_pair .try_sign(&signature_message) .expect("Ed25519 try_sign should not fail in benchmarks"); diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index d1ecf77500..c70281b44f 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -55,6 +55,8 @@ pub struct MessageParams { pub size: u32, /// Message sender account. pub sender_account: ThisAccountId, + /// If true, dispatch fee is paid at the target chain (if supported by configuration). + pub pay_dispatch_fee_at_target_chain: bool, } /// Benchmark-specific message proof parameters. @@ -67,6 +69,8 @@ pub struct MessageProofParams { pub outbound_lane_data: Option, /// Proof size requirements. pub size: ProofSize, + /// If true, dispatch fee is paid at the target chain (if supported by configuration). + pub pay_dispatch_fee_at_target_chain: bool, } /// Benchmark-specific message delivery proof parameters. @@ -94,6 +98,8 @@ pub trait Config: crate::Config { /// Create given account and give it enough balance for test purposes. fn endow_account(account: &Self::AccountId); /// Prepare message to send over lane. + /// + /// Dispatch fee is assumed to be paid at the source chain. fn prepare_outbound_message( params: MessageParams, ) -> (Self::OutboundPayload, Self::OutboundMessageFee); @@ -108,6 +114,8 @@ pub trait Config: crate::Config { fn prepare_message_delivery_proof( params: MessageDeliveryProofParams, ) -> >::MessagesDeliveryProof; + /// Returns true if message has been dispatched (either successfully or not). + fn is_message_dispatched(nonce: MessageNonce) -> bool; } benchmarks_instance! { @@ -119,7 +127,8 @@ benchmarks_instance! { // * outbound lane already has state, so it needs to be read and decoded; // * relayers fund account does not exists (in practice it needs to exist in production environment); // * maximal number of messages is being pruned during the call; - // * message size is minimal for the target chain. + // * message size is minimal for the target chain; + // * message is paid at the source (this) chain. // // Result of this benchmark is used as a base weight for `send_message` call. Then the 'message weight' // (estimated using `send_half_maximal_message_worst_case` and `send_maximal_message_worst_case`) is @@ -138,6 +147,40 @@ benchmarks_instance! { let (payload, fee) = T::prepare_outbound_message(MessageParams { size: 0, sender_account: sender.clone(), + pay_dispatch_fee_at_target_chain: false, + }); + }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) + verify { + assert_eq!( + crate::Pallet::::outbound_latest_generated_nonce(T::bench_lane_id()), + T::MaxMessagesToPruneAtOnce::get() + 1, + ); + } + + // Benchmark `send_message` extrinsic with the following conditions: + // * outbound lane already has state, so it needs to be read and decoded; + // * relayers fund account does not exists (in practice it needs to exist in production environment); + // * maximal number of messages is being pruned during the call; + // * message size is minimal for the target chain; + // * message is paid at the target (bridged) chain. + // + // Result of this benchmark is used to compute weight of the pay-dispatch-fee-at-source-chain + // operation and refund it to the message sender if dispatch is paid at the target chain. + send_minimal_message_with_dispatch_paid_at_the_target_chain { + let lane_id = T::bench_lane_id(); + let sender = account("sender", 0, SEED); + T::endow_account(&sender); + + // 'send' messages that are to be pruned when our message is sent + for _nonce in 1..=T::MaxMessagesToPruneAtOnce::get() { + send_regular_message::(); + } + confirm_message_delivery::(T::MaxMessagesToPruneAtOnce::get()); + + let (payload, fee) = T::prepare_outbound_message(MessageParams { + size: 0, + sender_account: sender.clone(), + pay_dispatch_fee_at_target_chain: true, }); }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) verify { @@ -175,6 +218,7 @@ benchmarks_instance! { let (payload, fee) = T::prepare_outbound_message(MessageParams { size, sender_account: sender.clone(), + pay_dispatch_fee_at_target_chain: false, }); }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) verify { @@ -212,6 +256,7 @@ benchmarks_instance! { let (payload, fee) = T::prepare_outbound_message(MessageParams { size, sender_account: sender.clone(), + pay_dispatch_fee_at_target_chain: false, }); }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) verify { @@ -242,7 +287,8 @@ benchmarks_instance! { // * proof does not include outbound lane state proof; // * inbound lane already has state, so it needs to be read and decoded; // * message is successfully dispatched; - // * message requires all heavy checks done by dispatcher. + // * message requires all heavy checks done by dispatcher; + // * message dispatch fee is pait at target (this) chain. // // This is base benchmark for all other message delivery benchmarks. receive_single_message_proof { @@ -257,6 +303,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), + pay_dispatch_fee_at_target_chain: true, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -264,13 +311,15 @@ benchmarks_instance! { crate::Pallet::::inbound_latest_received_nonce(T::bench_lane_id()), 21, ); + assert!(T::is_message_dispatched(21)); } // Benchmark `receive_messages_proof` extrinsic with two minimal-weight messages and following conditions: // * proof does not include outbound lane state proof; // * inbound lane already has state, so it needs to be read and decoded; // * message is successfully dispatched; - // * message requires all heavy checks done by dispatcher. + // * message requires all heavy checks done by dispatcher; + // * message dispatch fee is pait at target (this) chain. // // The weight of single message delivery could be approximated as // `weight(receive_two_messages_proof) - weight(receive_single_message_proof)`. @@ -288,6 +337,7 @@ benchmarks_instance! { message_nonces: 21..=22, outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), + pay_dispatch_fee_at_target_chain: true, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 2, dispatch_weight) verify { @@ -295,13 +345,15 @@ benchmarks_instance! { crate::Pallet::::inbound_latest_received_nonce(T::bench_lane_id()), 22, ); + assert!(T::is_message_dispatched(22)); } // Benchmark `receive_messages_proof` extrinsic with single minimal-weight message and following conditions: // * proof includes outbound lane state proof; // * inbound lane already has state, so it needs to be read and decoded; // * message is successfully dispatched; - // * message requires all heavy checks done by dispatcher. + // * message requires all heavy checks done by dispatcher; + // * message dispatch fee is pait at target (this) chain. // // The weight of outbound lane state delivery would be // `weight(receive_single_message_proof_with_outbound_lane_state) - weight(receive_single_message_proof)`. @@ -323,6 +375,7 @@ benchmarks_instance! { latest_generated_nonce: 21, }), size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), + pay_dispatch_fee_at_target_chain: true, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -334,6 +387,7 @@ benchmarks_instance! { crate::Pallet::::inbound_latest_confirmed_nonce(T::bench_lane_id()), 20, ); + assert!(T::is_message_dispatched(21)); } // Benchmark `receive_messages_proof` extrinsic with single minimal-weight message and following conditions: @@ -357,6 +411,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasExtraNodes(1024), + pay_dispatch_fee_at_target_chain: true, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -364,6 +419,7 @@ benchmarks_instance! { crate::Pallet::::inbound_latest_received_nonce(T::bench_lane_id()), 21, ); + assert!(T::is_message_dispatched(21)); } // Benchmark `receive_messages_proof` extrinsic with single minimal-weight message and following conditions: @@ -389,6 +445,40 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasExtraNodes(16 * 1024), + pay_dispatch_fee_at_target_chain: true, + }); + }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) + verify { + assert_eq!( + crate::Pallet::::inbound_latest_received_nonce(T::bench_lane_id()), + 21, + ); + assert!(T::is_message_dispatched(21)); + } + + // 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; + // * message is successfully dispatched; + // * message requires all heavy checks done by dispatcher; + // * message dispatch fee is pait at source (bridged) chain. + // + // This benchmark is used to compute extra weight spent at target chain when fee is paid there. Then we use + // this information in two places: (1) to reduce weight of delivery tx if sender pays fee at the source chain + // and (2) to refund relayer with this weight if fee has been paid at the source chain. + receive_single_prepaid_message_proof { + let relayer_id_on_source = T::bridged_relayer_id(); + let relayer_id_on_target = account("relayer", 0, SEED); + + // mark messages 1..=20 as delivered + receive_messages::(20); + + let (proof, dispatch_weight) = T::prepare_message_proof(MessageProofParams { + lane: T::bench_lane_id(), + message_nonces: 21..=21, + outbound_lane_data: None, + size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), + pay_dispatch_fee_at_target_chain: false, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -396,6 +486,7 @@ benchmarks_instance! { crate::Pallet::::inbound_latest_received_nonce(T::bench_lane_id()), 21, ); + assert!(T::is_message_dispatched(21)); } // Benchmark `receive_messages_delivery_proof` extrinsic with following conditions: @@ -517,7 +608,8 @@ benchmarks_instance! { // * outbound lane already has state, so it needs to be read and decoded; // * relayers fund account does not exists (in practice it needs to exist in production environment); // * maximal number of messages is being pruned during the call; - // * message size varies from minimal to maximal for the target chain. + // * message size varies from minimal to maximal for the target chain; + // * message dispatch fee is paid at this (source) chain. // // Results of this benchmark may be used to check how message size affects `send_message` performance. send_messages_of_various_lengths { @@ -536,6 +628,7 @@ benchmarks_instance! { let (payload, fee) = T::prepare_outbound_message(MessageParams { size: i as _, sender_account: sender.clone(), + pay_dispatch_fee_at_target_chain: false, }); }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) verify { @@ -569,6 +662,7 @@ benchmarks_instance! { message_nonces: 21..=(20 + i as MessageNonce), outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), + pay_dispatch_fee_at_target_chain: true, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), @@ -606,6 +700,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasExtraNodes(i as _), + pay_dispatch_fee_at_target_chain: true, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), @@ -643,6 +738,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasLargeLeaf(i as _), + pay_dispatch_fee_at_target_chain: true, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), @@ -686,6 +782,7 @@ benchmarks_instance! { latest_generated_nonce: 21, }), size: ProofSize::Minimal(0), + pay_dispatch_fee_at_target_chain: true, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), diff --git a/modules/messages/src/weights.rs b/modules/messages/src/weights.rs index 0eecd0d846..8a51e3f784 100644 --- a/modules/messages/src/weights.rs +++ b/modules/messages/src/weights.rs @@ -17,7 +17,7 @@ //! Autogenerated weights for pallet_bridge_messages //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0 -//! DATE: 2021-04-14, STEPS: [50, ], REPEAT: 20 +//! DATE: 2021-04-20, STEPS: [50, ], REPEAT: 20 //! LOW RANGE: [], HIGH RANGE: [] //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled //! CHAIN: Some("dev"), DB CACHE: 128 @@ -49,6 +49,7 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_bridge_messages. pub trait WeightInfo { fn send_minimal_message_worst_case() -> Weight; + fn send_minimal_message_with_dispatch_paid_at_the_target_chain() -> Weight; fn send_1_kb_message_worst_case() -> Weight; fn send_16_kb_message_worst_case() -> Weight; fn increase_message_fee() -> Weight; @@ -57,6 +58,7 @@ pub trait WeightInfo { fn receive_single_message_proof_with_outbound_lane_state() -> Weight; fn receive_single_message_proof_1_kb() -> Weight; fn receive_single_message_proof_16_kb() -> Weight; + fn receive_single_prepaid_message_proof() -> Weight; fn receive_delivery_proof_for_single_message() -> Weight; fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight; fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight; @@ -73,105 +75,115 @@ pub trait WeightInfo { pub struct RialtoWeight(PhantomData); impl WeightInfo for RialtoWeight { fn send_minimal_message_worst_case() -> Weight { - (149_497_000 as Weight) + (168_380_000 as Weight) + .saturating_add(T::DbWeight::get().reads(5 as Weight)) + .saturating_add(T::DbWeight::get().writes(12 as Weight)) + } + fn send_minimal_message_with_dispatch_paid_at_the_target_chain() -> Weight { + (177_461_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn send_1_kb_message_worst_case() -> Weight { - (154_339_000 as Weight) + (183_032_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn send_16_kb_message_worst_case() -> Weight { - (200_066_000 as Weight) + (234_995_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn increase_message_fee() -> Weight { - (6_432_637_000 as Weight) + (6_645_750_000 as Weight) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof() -> Weight { - (141_671_000 as Weight) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (229_797_000 as Weight) + .saturating_add(T::DbWeight::get().reads(5 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_two_messages_proof() -> Weight { - (247_393_000 as Weight) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (381_948_000 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_with_outbound_lane_state() -> Weight { - (159_312_000 as Weight) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (249_688_000 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_1_kb() -> Weight { - (167_935_000 as Weight) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (259_516_000 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_16_kb() -> Weight { - (449_846_000 as Weight) + (543_558_000 as Weight) + .saturating_add(T::DbWeight::get().reads(5 as Weight)) + .saturating_add(T::DbWeight::get().writes(3 as Weight)) + } + fn receive_single_prepaid_message_proof() -> Weight { + (156_777_000 as Weight) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn receive_delivery_proof_for_single_message() -> Weight { - (127_322_000 as Weight) + (144_411_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { - (134_120_000 as Weight) + (152_865_000 as Weight) .saturating_add(T::DbWeight::get().reads(7 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { - (191_193_000 as Weight) + (215_712_000 as Weight) .saturating_add(T::DbWeight::get().reads(8 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn send_messages_of_various_lengths(i: u32) -> Weight { - (115_699_000 as Weight) + (121_683_000 as Weight) .saturating_add((3_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn receive_multiple_messages_proof(i: u32) -> Weight { (0 as Weight) - .saturating_add((113_551_000 as Weight).saturating_mul(i as Weight)) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + .saturating_add((161_753_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_message_proofs_with_extra_nodes(i: u32) -> Weight { - (458_731_000 as Weight) - .saturating_add((9_000 as Weight).saturating_mul(i as Weight)) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (125_849_000 as Weight) + .saturating_add((12_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_message_proofs_with_large_leaf(i: u32) -> Weight { - (82_314_000 as Weight) - .saturating_add((7_000 as Weight).saturating_mul(i as Weight)) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (197_283_000 as Weight) + .saturating_add((9_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_multiple_messages_proof_with_outbound_lane_state(i: u32) -> Weight { - (16_766_000 as Weight) - .saturating_add((115_533_000 as Weight).saturating_mul(i as Weight)) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (242_925_000 as Weight) + .saturating_add((187_149_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_delivery_proof_for_multiple_messages_by_single_relayer(i: u32) -> Weight { - (122_146_000 as Weight) - .saturating_add((6_789_000 as Weight).saturating_mul(i as Weight)) + (122_411_000 as Weight) + .saturating_add((10_096_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(i as Weight))) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_multiple_messages_by_multiple_relayers(i: u32) -> Weight { - (155_671_000 as Weight) - .saturating_add((63_020_000 as Weight).saturating_mul(i as Weight)) + (206_031_000 as Weight) + .saturating_add((80_639_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().reads((2 as Weight).saturating_mul(i as Weight))) .saturating_add(T::DbWeight::get().writes(3 as Weight)) @@ -182,105 +194,115 @@ impl WeightInfo for RialtoWeight { // For backwards compatibility and tests impl WeightInfo for () { fn send_minimal_message_worst_case() -> Weight { - (149_497_000 as Weight) + (168_380_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + .saturating_add(RocksDbWeight::get().writes(12 as Weight)) + } + fn send_minimal_message_with_dispatch_paid_at_the_target_chain() -> Weight { + (177_461_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn send_1_kb_message_worst_case() -> Weight { - (154_339_000 as Weight) + (183_032_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn send_16_kb_message_worst_case() -> Weight { - (200_066_000 as Weight) + (234_995_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn increase_message_fee() -> Weight { - (6_432_637_000 as Weight) + (6_645_750_000 as Weight) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof() -> Weight { - (141_671_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (229_797_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_two_messages_proof() -> Weight { - (247_393_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (381_948_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_with_outbound_lane_state() -> Weight { - (159_312_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (249_688_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_1_kb() -> Weight { - (167_935_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (259_516_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_16_kb() -> Weight { - (449_846_000 as Weight) + (543_558_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) + .saturating_add(RocksDbWeight::get().writes(3 as Weight)) + } + fn receive_single_prepaid_message_proof() -> Weight { + (156_777_000 as Weight) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn receive_delivery_proof_for_single_message() -> Weight { - (127_322_000 as Weight) + (144_411_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { - (134_120_000 as Weight) + (152_865_000 as Weight) .saturating_add(RocksDbWeight::get().reads(7 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { - (191_193_000 as Weight) + (215_712_000 as Weight) .saturating_add(RocksDbWeight::get().reads(8 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn send_messages_of_various_lengths(i: u32) -> Weight { - (115_699_000 as Weight) + (121_683_000 as Weight) .saturating_add((3_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn receive_multiple_messages_proof(i: u32) -> Weight { (0 as Weight) - .saturating_add((113_551_000 as Weight).saturating_mul(i as Weight)) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + .saturating_add((161_753_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_message_proofs_with_extra_nodes(i: u32) -> Weight { - (458_731_000 as Weight) - .saturating_add((9_000 as Weight).saturating_mul(i as Weight)) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (125_849_000 as Weight) + .saturating_add((12_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_message_proofs_with_large_leaf(i: u32) -> Weight { - (82_314_000 as Weight) - .saturating_add((7_000 as Weight).saturating_mul(i as Weight)) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (197_283_000 as Weight) + .saturating_add((9_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_multiple_messages_proof_with_outbound_lane_state(i: u32) -> Weight { - (16_766_000 as Weight) - .saturating_add((115_533_000 as Weight).saturating_mul(i as Weight)) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (242_925_000 as Weight) + .saturating_add((187_149_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_delivery_proof_for_multiple_messages_by_single_relayer(i: u32) -> Weight { - (122_146_000 as Weight) - .saturating_add((6_789_000 as Weight).saturating_mul(i as Weight)) + (122_411_000 as Weight) + .saturating_add((10_096_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().reads((1 as Weight).saturating_mul(i as Weight))) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_multiple_messages_by_multiple_relayers(i: u32) -> Weight { - (155_671_000 as Weight) - .saturating_add((63_020_000 as Weight).saturating_mul(i as Weight)) + (206_031_000 as Weight) + .saturating_add((80_639_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().reads((2 as Weight).saturating_mul(i as Weight))) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) From f9a860018d7467fd6d7b0eb24d80959cc93d890e Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 21 Apr 2021 11:44:21 +0300 Subject: [PATCH 05/14] include/exclude pay-dispatch-fee weight from delivery_and_dispatch_fee/delivery tx cost --- Cargo.lock | 2 + bin/millau/runtime/src/lib.rs | 1 + bin/millau/runtime/src/rialto_messages.rs | 6 ++ bin/rialto/runtime/src/lib.rs | 1 + bin/rialto/runtime/src/millau_messages.rs | 6 ++ bin/runtime-common/src/messages.rs | 8 +- modules/dispatch/src/lib.rs | 102 +++++++++++-------- modules/messages/src/benchmarking.rs | 33 ------- modules/messages/src/inbound_lane.rs | 30 +++--- modules/messages/src/lib.rs | 45 ++++++--- modules/messages/src/mock.rs | 29 ++++-- modules/messages/src/weights.rs | 113 ++++++++++------------ modules/messages/src/weights_ext.rs | 17 ++++ primitives/chain-millau/src/lib.rs | 9 +- primitives/chain-rialto/src/lib.rs | 9 +- primitives/message-dispatch/Cargo.toml | 5 + primitives/message-dispatch/src/lib.rs | 20 +++- primitives/messages/Cargo.toml | 2 + primitives/messages/src/target_chain.rs | 21 ++-- relays/bin-substrate/src/messages_lane.rs | 2 +- relays/messages/src/message_lane_loop.rs | 1 - 21 files changed, 275 insertions(+), 187 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e59c6a7d05..45eb2e6ed7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -711,6 +711,7 @@ name = "bp-message-dispatch" version = "0.1.0" dependencies = [ "bp-runtime", + "frame-support", "parity-scale-codec 2.0.1", ] @@ -718,6 +719,7 @@ dependencies = [ name = "bp-messages" version = "0.1.0" dependencies = [ + "bp-message-dispatch", "bp-runtime", "frame-support", "frame-system", diff --git a/bin/millau/runtime/src/lib.rs b/bin/millau/runtime/src/lib.rs index 30cf1bd87c..cb6449bf28 100644 --- a/bin/millau/runtime/src/lib.rs +++ b/bin/millau/runtime/src/lib.rs @@ -679,6 +679,7 @@ mod tests { bp_millau::DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT, bp_millau::ADDITIONAL_MESSAGE_BYTE_DELIVERY_WEIGHT, bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, + bp_millau::PAY_INBOUND_DISPATCH_FEE_WEIGHT, ); let max_incoming_message_proof_size = bp_rialto::EXTRA_STORAGE_PROOF_SIZE.saturating_add( diff --git a/bin/millau/runtime/src/rialto_messages.rs b/bin/millau/runtime/src/rialto_messages.rs index 2af0d5c381..bcb28f15a0 100644 --- a/bin/millau/runtime/src/rialto_messages.rs +++ b/bin/millau/runtime/src/rialto_messages.rs @@ -171,6 +171,7 @@ impl messages::BridgedChainWithMessages for Rialto { fn estimate_delivery_transaction( message_payload: &[u8], + include_pay_dispatch_fee_cost: bool, message_dispatch_weight: Weight, ) -> MessageTransaction { let message_payload_len = u32::try_from(message_payload.len()).unwrap_or(u32::MAX); @@ -181,6 +182,11 @@ impl messages::BridgedChainWithMessages for Rialto { dispatch_weight: extra_bytes_in_payload .saturating_mul(bp_rialto::ADDITIONAL_MESSAGE_BYTE_DELIVERY_WEIGHT) .saturating_add(bp_rialto::DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT) + .saturating_sub(if include_pay_dispatch_fee_cost { + 0 + } else { + bp_rialto::PAY_INBOUND_DISPATCH_FEE_WEIGHT + }) .saturating_add(message_dispatch_weight), size: message_payload_len .saturating_add(bp_millau::EXTRA_STORAGE_PROOF_SIZE) diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index fbcb66c9c9..7da8b7bbc8 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -1103,6 +1103,7 @@ mod tests { bp_rialto::DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT, bp_rialto::ADDITIONAL_MESSAGE_BYTE_DELIVERY_WEIGHT, bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, + bp_rialto::PAY_INBOUND_DISPATCH_FEE_WEIGHT, ); let max_incoming_message_proof_size = bp_millau::EXTRA_STORAGE_PROOF_SIZE.saturating_add( diff --git a/bin/rialto/runtime/src/millau_messages.rs b/bin/rialto/runtime/src/millau_messages.rs index da85b6efa9..3209d36d35 100644 --- a/bin/rialto/runtime/src/millau_messages.rs +++ b/bin/rialto/runtime/src/millau_messages.rs @@ -171,6 +171,7 @@ impl messages::BridgedChainWithMessages for Millau { fn estimate_delivery_transaction( message_payload: &[u8], + include_pay_dispatch_fee_cost: bool, message_dispatch_weight: Weight, ) -> MessageTransaction { let message_payload_len = u32::try_from(message_payload.len()).unwrap_or(u32::MAX); @@ -181,6 +182,11 @@ impl messages::BridgedChainWithMessages for Millau { dispatch_weight: extra_bytes_in_payload .saturating_mul(bp_millau::ADDITIONAL_MESSAGE_BYTE_DELIVERY_WEIGHT) .saturating_add(bp_millau::DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT) + .saturating_sub(if include_pay_dispatch_fee_cost { + 0 + } else { + bp_millau::PAY_INBOUND_DISPATCH_FEE_WEIGHT + }) .saturating_add(message_dispatch_weight), size: message_payload_len .saturating_add(bp_rialto::EXTRA_STORAGE_PROOF_SIZE) diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index 241fd3f148..0fe637db1a 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -23,7 +23,7 @@ use bp_message_dispatch::MessageDispatch as _; use bp_messages::{ source_chain::{LaneMessageVerifier, Sender}, - target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages}, + target_chain::{DispatchMessage, MessageDispatch, MessageDispatchResult, ProvedLaneMessages, ProvedMessages}, InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData, }; use bp_runtime::{InstanceId, Size, StorageProofChecker}; @@ -128,6 +128,7 @@ pub trait BridgedChainWithMessages: ChainWithMessages { /// Estimate size and weight of single message delivery transaction at the Bridged chain. fn estimate_delivery_transaction( message_payload: &[u8], + include_pay_dispatch_fee_cost: bool, message_dispatch_weight: WeightOf, ) -> MessageTransaction>; @@ -335,6 +336,7 @@ pub mod source { // of the message dispatch in the delivery transaction cost let delivery_transaction = BridgedChain::::estimate_delivery_transaction( &payload.call, + payload.pay_dispatch_fee_at_target_chain, if payload.pay_dispatch_fee_at_target_chain { 0.into() } else { @@ -513,7 +515,7 @@ pub mod target { fn dispatch( relayer_account: &AccountIdOf>, message: DispatchMessage>>, - ) -> Weight { + ) -> MessageDispatchResult { let message_id = (message.key.lane_id, message.key.nonce); pallet_bridge_dispatch::Pallet::::dispatch( B::INSTANCE, @@ -894,6 +896,7 @@ mod tests { fn estimate_delivery_transaction( _message_payload: &[u8], + _include_pay_dispatch_fee_cost: bool, _message_dispatch_weight: WeightOf, ) -> MessageTransaction> { unreachable!() @@ -949,6 +952,7 @@ mod tests { fn estimate_delivery_transaction( _message_payload: &[u8], + _include_pay_dispatch_fee_cost: bool, message_dispatch_weight: WeightOf, ) -> MessageTransaction> { MessageTransaction { diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index ab8287780d..12cfd10014 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -24,7 +24,7 @@ #![cfg_attr(not(feature = "std"), no_std)] #![warn(missing_docs)] -use bp_message_dispatch::{MessageDispatch, Weight}; +use bp_message_dispatch::{MessageDispatch, MessageDispatchResult, Weight}; use bp_runtime::{derive_account_id, InstanceId, Size, SourceAccount}; use codec::{Decode, Encode}; use frame_support::{ @@ -214,19 +214,26 @@ impl, I: Instance> MessageDispatch for id: T::MessageId, message: Result, pay_dispatch_fee: P, - ) -> Weight { + ) -> MessageDispatchResult { // emit special even if message has been rejected by external component let message = match message { Ok(message) => message, Err(_) => { log::trace!(target: "runtime::bridge-dispatch", "Message {:?}/{:?}: rejected before actual dispatch", bridge, id); Self::deposit_event(RawEvent::MessageRejected(bridge, id)); - return 0; + return MessageDispatchResult { + unspent_weight: 0, + dispatch_fee_paid_during_dispatch: false, + }; } }; // verify spec version // (we want it to be the same, because otherwise we may decode Call improperly) + let mut dispatch_result = MessageDispatchResult { + unspent_weight: message.weight, + dispatch_fee_paid_during_dispatch: false, + }; let expected_version = ::Version::get().spec_version; if message.spec_version != expected_version { log::trace!( @@ -242,7 +249,7 @@ impl, I: Instance> MessageDispatch for expected_version, message.spec_version, )); - return message.weight; + return dispatch_result; } // now that we have spec version checked, let's decode the call @@ -251,7 +258,7 @@ impl, I: Instance> MessageDispatch for Err(_) => { log::trace!(target: "runtime::bridge-dispatch", "Failed to decode Call from message {:?}/{:?}", bridge, id,); Self::deposit_event(RawEvent::MessageCallDecodeFailed(bridge, id)); - return message.weight; + return dispatch_result; } }; @@ -277,7 +284,7 @@ impl, I: Instance> MessageDispatch for target_signature, ); Self::deposit_event(RawEvent::MessageSignatureMismatch(bridge, id)); - return message.weight; + return dispatch_result; } log::trace!(target: "runtime::bridge-dispatch", "Target Account: {:?}", &target_account); @@ -301,7 +308,7 @@ impl, I: Instance> MessageDispatch for call, ); Self::deposit_event(RawEvent::MessageCallRejected(bridge, id)); - return message.weight; + return dispatch_result; } // verify weight @@ -324,54 +331,53 @@ impl, I: Instance> MessageDispatch for expected_weight, message.weight, )); - return message.weight; + return dispatch_result; } // pay dispatch fee right before dispatch - if message.pay_dispatch_fee_at_target_chain { - if pay_dispatch_fee(&origin_account, message.weight).is_err() { - log::trace!( - target: "runtime::bridge-dispatch", - "Failed to pay dispatch fee for dispatching message {:?}/{:?} with weight {}", - bridge, - id, - message.weight, - ); - Self::deposit_event(RawEvent::MessageDispatchPaymentFailed( - bridge, - id, - origin_account, - message.weight, - )); - return message.weight; - } + if message.pay_dispatch_fee_at_target_chain && pay_dispatch_fee(&origin_account, message.weight).is_err() { + log::trace!( + target: "runtime::bridge-dispatch", + "Failed to pay dispatch fee for dispatching message {:?}/{:?} with weight {}", + bridge, + id, + message.weight, + ); + Self::deposit_event(RawEvent::MessageDispatchPaymentFailed( + bridge, + id, + origin_account, + message.weight, + )); + return dispatch_result; } + dispatch_result.dispatch_fee_paid_during_dispatch = message.pay_dispatch_fee_at_target_chain; // finally dispatch message let origin = RawOrigin::Signed(origin_account).into(); log::trace!(target: "runtime::bridge-dispatch", "Message being dispatched is: {:?}", &call); - let dispatch_result = call.dispatch(origin); - let actual_call_weight = extract_actual_weight(&dispatch_result, &dispatch_info); - let unused_dispatch_weight = message.weight.saturating_sub(actual_call_weight); + let result = call.dispatch(origin); + let actual_call_weight = extract_actual_weight(&result, &dispatch_info); + dispatch_result.unspent_weight = message.weight.saturating_sub(actual_call_weight); log::trace!( target: "runtime::bridge-dispatch", "Message {:?}/{:?} has been dispatched. Weight: {} of {}. Result: {:?}", bridge, id, - actual_call_weight, + dispatch_result.unspent_weight, message.weight, - dispatch_result, + result, ); Self::deposit_event(RawEvent::MessageDispatched( bridge, id, - dispatch_result.map(drop).map_err(|e| e.error), + result.map(drop).map_err(|e| e.error), )); - unused_dispatch_weight + dispatch_result } } @@ -614,10 +620,12 @@ mod tests { const BAD_SPEC_VERSION: SpecVersion = 99; let mut message = prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); + let weight = message.weight; message.spec_version = BAD_SPEC_VERSION; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, weight); assert_eq!( System::events(), @@ -642,17 +650,18 @@ mod tests { let id = [0; 4]; let mut message = prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); - message.weight = 0; + message.weight = 7; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, 7); assert_eq!( System::events(), vec![EventRecord { phase: Phase::Initialization, event: Event::call_dispatch(call_dispatch::Event::::MessageWeightMismatch( - bridge, id, 1345000, 0, + bridge, id, 1345000, 7, )), topics: vec![], }], @@ -671,9 +680,11 @@ mod tests { call_origin, Call::System(>::remark(vec![1, 2, 3])), ); + let weight = message.weight; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, weight); assert_eq!( System::events(), @@ -716,10 +727,12 @@ mod tests { let mut message = prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); + let weight = message.weight; message.call.0 = vec![]; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, weight); assert_eq!( System::events(), @@ -746,7 +759,8 @@ mod tests { message.weight = weight; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, weight); assert_eq!( System::events(), @@ -767,10 +781,12 @@ mod tests { let mut message = prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); + let weight = message.weight; message.pay_dispatch_fee_at_target_chain = true; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| Err(())); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| Err(())); + assert_eq!(result.unspent_weight, weight); assert_eq!( System::events(), @@ -799,7 +815,8 @@ mod tests { message.pay_dispatch_fee_at_target_chain = true; System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| Ok(())); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| Ok(())); + assert!(result.dispatch_fee_paid_during_dispatch); assert_eq!( System::events(), @@ -824,7 +841,8 @@ mod tests { let message = prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + assert!(!result.dispatch_fee_paid_during_dispatch); assert_eq!( System::events(), diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index c70281b44f..445dd377dc 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -157,39 +157,6 @@ benchmarks_instance! { ); } - // Benchmark `send_message` extrinsic with the following conditions: - // * outbound lane already has state, so it needs to be read and decoded; - // * relayers fund account does not exists (in practice it needs to exist in production environment); - // * maximal number of messages is being pruned during the call; - // * message size is minimal for the target chain; - // * message is paid at the target (bridged) chain. - // - // Result of this benchmark is used to compute weight of the pay-dispatch-fee-at-source-chain - // operation and refund it to the message sender if dispatch is paid at the target chain. - send_minimal_message_with_dispatch_paid_at_the_target_chain { - let lane_id = T::bench_lane_id(); - let sender = account("sender", 0, SEED); - T::endow_account(&sender); - - // 'send' messages that are to be pruned when our message is sent - for _nonce in 1..=T::MaxMessagesToPruneAtOnce::get() { - send_regular_message::(); - } - confirm_message_delivery::(T::MaxMessagesToPruneAtOnce::get()); - - let (payload, fee) = T::prepare_outbound_message(MessageParams { - size: 0, - sender_account: sender.clone(), - pay_dispatch_fee_at_target_chain: true, - }); - }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) - verify { - assert_eq!( - crate::Pallet::::outbound_latest_generated_nonce(T::bench_lane_id()), - T::MaxMessagesToPruneAtOnce::get() + 1, - ); - } - // Benchmark `send_message` extrinsic with the worst possible conditions: // * outbound lane already has state, so it needs to be read and decoded; // * relayers fund account does not exists (in practice it needs to exist in production environment); diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 9fca9aac64..0d63176cd4 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -17,8 +17,8 @@ //! Everything about incoming messages receival. use bp_messages::{ - target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, - InboundLaneData, LaneId, MessageKey, MessageNonce, OutboundLaneData, Weight, + target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch, MessageDispatchResult}, + InboundLaneData, LaneId, MessageKey, MessageNonce, OutboundLaneData, }; use frame_support::RuntimeDebug; use sp_std::prelude::PartialEq; @@ -48,8 +48,8 @@ pub enum ReceivalResult { /// Message has been received and dispatched. Note that we don't care whether dispatch has /// been successful or not - in both case message falls into this category. /// - /// The unspent message dispatch weight is also returned. - Dispatched(Weight), + /// The message dispatch result is also returned. + Dispatched(MessageDispatchResult), /// Message has invalid nonce and lane has rejected to accept this message. InvalidNonce, /// There are too many unrewarded relayer entires at the lane. @@ -164,8 +164,8 @@ mod tests { use crate::{ inbound_lane, mock::{ - message_data, run_test, TestMessageDispatch, TestRuntime, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_RELAYER_A, - TEST_RELAYER_B, TEST_RELAYER_C, + dispatch_result, message_data, run_test, TestMessageDispatch, TestRuntime, REGULAR_PAYLOAD, TEST_LANE_ID, + TEST_RELAYER_A, TEST_RELAYER_B, TEST_RELAYER_C, }, DefaultInstance, RuntimeInboundLaneStorage, }; @@ -181,7 +181,7 @@ mod tests { nonce, message_data(REGULAR_PAYLOAD).into() ), - ReceivalResult::Dispatched(0) + ReceivalResult::Dispatched(dispatch_result(0)) ); } @@ -319,7 +319,7 @@ mod tests { current_nonce, message_data(REGULAR_PAYLOAD).into() ), - ReceivalResult::Dispatched(0) + ReceivalResult::Dispatched(dispatch_result(0)) ); } // Fails to dispatch new message from different than latest relayer. @@ -358,7 +358,7 @@ mod tests { current_nonce, message_data(REGULAR_PAYLOAD).into() ), - ReceivalResult::Dispatched(0) + ReceivalResult::Dispatched(dispatch_result(0)) ); } // Fails to dispatch new message from different than latest relayer. @@ -395,7 +395,7 @@ mod tests { 1, message_data(REGULAR_PAYLOAD).into() ), - ReceivalResult::Dispatched(0) + ReceivalResult::Dispatched(dispatch_result(0)) ); assert_eq!( lane.receive_message::( @@ -404,7 +404,7 @@ mod tests { 2, message_data(REGULAR_PAYLOAD).into() ), - ReceivalResult::Dispatched(0) + ReceivalResult::Dispatched(dispatch_result(0)) ); assert_eq!( lane.receive_message::( @@ -413,7 +413,7 @@ mod tests { 3, message_data(REGULAR_PAYLOAD).into() ), - ReceivalResult::Dispatched(0) + ReceivalResult::Dispatched(dispatch_result(0)) ); assert_eq!( lane.storage.data().relayers, @@ -433,7 +433,7 @@ mod tests { 1, message_data(REGULAR_PAYLOAD).into() ), - ReceivalResult::Dispatched(0) + ReceivalResult::Dispatched(dispatch_result(0)) ); assert_eq!( lane.receive_message::( @@ -461,7 +461,7 @@ mod tests { run_test(|| { let mut lane = inbound_lane::(TEST_LANE_ID); let mut payload = REGULAR_PAYLOAD; - payload.unspent_weight = 1; + payload.dispatch_result.unspent_weight = 1; assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -469,7 +469,7 @@ mod tests { 1, message_data(payload).into() ), - ReceivalResult::Dispatched(1) + ReceivalResult::Dispatched(dispatch_result(1)) ); }); } diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 835cec2f96..e5c01350f5 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -544,16 +544,27 @@ decl_module! { // losing funds for messages dispatch. But keep in mind that relayer pays base // delivery transaction cost anyway. And base cost covers everything except // dispatch, so we have a balance here. - let unspent_weight = match receival_result { - ReceivalResult::Dispatched(unspent_weight) => { + let (unspent_weight, refund_pay_dispatch_fee) = match receival_result { + ReceivalResult::Dispatched(dispatch_result) => { valid_messages += 1; - unspent_weight + (dispatch_result.unspent_weight, !dispatch_result.dispatch_fee_paid_during_dispatch) }, ReceivalResult::InvalidNonce | ReceivalResult::TooManyUnrewardedRelayers - | ReceivalResult::TooManyUnconfirmedMessages => dispatch_weight, + | ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true), }; - actual_weight = actual_weight.saturating_sub(sp_std::cmp::min(unspent_weight, dispatch_weight)); + actual_weight = actual_weight + .saturating_sub(sp_std::cmp::min(unspent_weight, dispatch_weight)) + .saturating_sub( + // delivery call weight formula assumes that the fee is paid at + // this (target) chain. If the message is prepaid at the source + // chain, let's refund relayer with this extra cost. + if refund_pay_dispatch_fee { + T::WeightInfo::pay_inbound_dispatch_fee_overhead() + } else { + 0 + } + ); } } @@ -1641,9 +1652,14 @@ mod tests { #[test] fn weight_refund_from_receive_messages_proof_works() { run_test(|| { - fn submit_with_unspent_weight(nonce: MessageNonce, unspent_weight: Weight) -> (Weight, Weight) { + fn submit_with_unspent_weight( + nonce: MessageNonce, + unspent_weight: Weight, + is_prepaid: bool, + ) -> (Weight, Weight) { let mut payload = REGULAR_PAYLOAD; - payload.unspent_weight = unspent_weight; + payload.dispatch_result.unspent_weight = unspent_weight; + payload.dispatch_result.dispatch_fee_paid_during_dispatch = !is_prepaid; let proof = Ok(vec![message(nonce, payload)]).into(); let messages_count = 1; let pre_dispatch_weight = ::WeightInfo::receive_messages_proof_weight( @@ -1666,20 +1682,27 @@ mod tests { } // when dispatch is returning `unspent_weight < declared_weight` - let (pre, post) = submit_with_unspent_weight(1, 1); + let (pre, post) = submit_with_unspent_weight(1, 1, false); assert_eq!(post, pre - 1); // when dispatch is returning `unspent_weight = declared_weight` - let (pre, post) = submit_with_unspent_weight(2, REGULAR_PAYLOAD.declared_weight); + let (pre, post) = submit_with_unspent_weight(2, REGULAR_PAYLOAD.declared_weight, false); assert_eq!(post, pre - REGULAR_PAYLOAD.declared_weight); // when dispatch is returning `unspent_weight > declared_weight` - let (pre, post) = submit_with_unspent_weight(3, REGULAR_PAYLOAD.declared_weight + 1); + let (pre, post) = submit_with_unspent_weight(3, REGULAR_PAYLOAD.declared_weight + 1, false); assert_eq!(post, pre - REGULAR_PAYLOAD.declared_weight); // when there's no unspent weight - let (pre, post) = submit_with_unspent_weight(4, 0); + let (pre, post) = submit_with_unspent_weight(4, 0, false); assert_eq!(post, pre); + + // when dispatch is returning `unspent_weight < declared_weight` AND message is prepaid + let (pre, post) = submit_with_unspent_weight(5, 1, true); + assert_eq!( + post, + pre - 1 - ::WeightInfo::pay_inbound_dispatch_fee_overhead() + ); }); } } diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index 96d2e5f4ec..43cc4fcf40 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -23,7 +23,9 @@ use bp_messages::{ source_chain::{ LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, Sender, TargetHeaderChain, }, - target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, + target_chain::{ + DispatchMessage, MessageDispatch, MessageDispatchResult, ProvedLaneMessages, ProvedMessages, SourceHeaderChain, + }, InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData, Parameter as MessagesParameter, }; @@ -46,9 +48,11 @@ pub struct TestPayload { pub id: u64, /// Dispatch weight that is declared by the message sender. pub declared_weight: Weight, - /// Unspent weight. In correct code it'll always be <= `declared_weight`, but for test + /// Message dispatch result. + /// + /// Note: in correct code `dispatch_result.unspent_weight` will always be <= `declared_weight`, but for test /// purposes we'll be making it larger than `declared_weight` sometimes. - pub unspent_weight: Weight, + pub dispatch_result: MessageDispatchResult, } pub type TestMessageFee = u64; pub type TestRelayer = u64; @@ -375,10 +379,13 @@ impl MessageDispatch for TestMessageDispatch { } } - fn dispatch(_relayer_account: &AccountId, message: DispatchMessage) -> Weight { + fn dispatch( + _relayer_account: &AccountId, + message: DispatchMessage, + ) -> MessageDispatchResult { match message.data.payload.as_ref() { - Ok(payload) => payload.unspent_weight, - Err(_) => 0, + Ok(payload) => payload.dispatch_result.clone(), + Err(_) => dispatch_result(0), } } } @@ -399,7 +406,7 @@ pub const fn message_payload(id: u64, declared_weight: Weight) -> TestPayload { TestPayload { id, declared_weight, - unspent_weight: 0, + dispatch_result: dispatch_result(0), } } @@ -411,6 +418,14 @@ pub fn message_data(payload: TestPayload) -> MessageData { } } +/// Returns message dispatch result with given unspent weight. +pub const fn dispatch_result(unspent_weight: Weight) -> MessageDispatchResult { + MessageDispatchResult { + unspent_weight, + dispatch_fee_paid_during_dispatch: true, + } +} + /// Run pallet test. pub fn run_test(test: impl FnOnce() -> T) -> T { let mut t = frame_system::GenesisConfig::default() diff --git a/modules/messages/src/weights.rs b/modules/messages/src/weights.rs index 8a51e3f784..d3fc1a2a40 100644 --- a/modules/messages/src/weights.rs +++ b/modules/messages/src/weights.rs @@ -17,7 +17,7 @@ //! Autogenerated weights for pallet_bridge_messages //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0 -//! DATE: 2021-04-20, STEPS: [50, ], REPEAT: 20 +//! DATE: 2021-04-21, STEPS: [50, ], REPEAT: 20 //! LOW RANGE: [], HIGH RANGE: [] //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled //! CHAIN: Some("dev"), DB CACHE: 128 @@ -49,7 +49,6 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_bridge_messages. pub trait WeightInfo { fn send_minimal_message_worst_case() -> Weight; - fn send_minimal_message_with_dispatch_paid_at_the_target_chain() -> Weight; fn send_1_kb_message_worst_case() -> Weight; fn send_16_kb_message_worst_case() -> Weight; fn increase_message_fee() -> Weight; @@ -75,115 +74,110 @@ pub trait WeightInfo { pub struct RialtoWeight(PhantomData); impl WeightInfo for RialtoWeight { fn send_minimal_message_worst_case() -> Weight { - (168_380_000 as Weight) - .saturating_add(T::DbWeight::get().reads(5 as Weight)) - .saturating_add(T::DbWeight::get().writes(12 as Weight)) - } - fn send_minimal_message_with_dispatch_paid_at_the_target_chain() -> Weight { - (177_461_000 as Weight) + (176_319_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn send_1_kb_message_worst_case() -> Weight { - (183_032_000 as Weight) + (181_291_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn send_16_kb_message_worst_case() -> Weight { - (234_995_000 as Weight) + (232_772_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn increase_message_fee() -> Weight { - (6_645_750_000 as Weight) + (6_580_906_000 as Weight) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof() -> Weight { - (229_797_000 as Weight) + (229_141_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_two_messages_proof() -> Weight { - (381_948_000 as Weight) + (381_428_000 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_with_outbound_lane_state() -> Weight { - (249_688_000 as Weight) + (250_142_000 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_1_kb() -> Weight { - (259_516_000 as Weight) + (256_353_000 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_16_kb() -> Weight { - (543_558_000 as Weight) + (547_044_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_single_prepaid_message_proof() -> Weight { - (156_777_000 as Weight) + (155_311_000 as Weight) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn receive_delivery_proof_for_single_message() -> Weight { - (144_411_000 as Weight) + (143_410_000 as Weight) .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { - (152_865_000 as Weight) + (150_649_000 as Weight) .saturating_add(T::DbWeight::get().reads(7 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { - (215_712_000 as Weight) + (212_115_000 as Weight) .saturating_add(T::DbWeight::get().reads(8 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn send_messages_of_various_lengths(i: u32) -> Weight { - (121_683_000 as Weight) + (141_468_000 as Weight) .saturating_add((3_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn receive_multiple_messages_proof(i: u32) -> Weight { (0 as Weight) - .saturating_add((161_753_000 as Weight).saturating_mul(i as Weight)) + .saturating_add((162_323_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_message_proofs_with_extra_nodes(i: u32) -> Weight { - (125_849_000 as Weight) - .saturating_add((12_000 as Weight).saturating_mul(i as Weight)) + (516_172_000 as Weight) + .saturating_add((10_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_message_proofs_with_large_leaf(i: u32) -> Weight { - (197_283_000 as Weight) - .saturating_add((9_000 as Weight).saturating_mul(i as Weight)) + (204_903_000 as Weight) + .saturating_add((7_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_multiple_messages_proof_with_outbound_lane_state(i: u32) -> Weight { - (242_925_000 as Weight) - .saturating_add((187_149_000 as Weight).saturating_mul(i as Weight)) + (0 as Weight) + .saturating_add((164_287_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_delivery_proof_for_multiple_messages_by_single_relayer(i: u32) -> Weight { - (122_411_000 as Weight) - .saturating_add((10_096_000 as Weight).saturating_mul(i as Weight)) + (123_564_000 as Weight) + .saturating_add((8_112_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(i as Weight))) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_multiple_messages_by_multiple_relayers(i: u32) -> Weight { - (206_031_000 as Weight) - .saturating_add((80_639_000 as Weight).saturating_mul(i as Weight)) + (0 as Weight) + .saturating_add((73_627_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().reads((2 as Weight).saturating_mul(i as Weight))) .saturating_add(T::DbWeight::get().writes(3 as Weight)) @@ -194,115 +188,110 @@ impl WeightInfo for RialtoWeight { // For backwards compatibility and tests impl WeightInfo for () { fn send_minimal_message_worst_case() -> Weight { - (168_380_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(5 as Weight)) - .saturating_add(RocksDbWeight::get().writes(12 as Weight)) - } - fn send_minimal_message_with_dispatch_paid_at_the_target_chain() -> Weight { - (177_461_000 as Weight) + (176_319_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn send_1_kb_message_worst_case() -> Weight { - (183_032_000 as Weight) + (181_291_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn send_16_kb_message_worst_case() -> Weight { - (234_995_000 as Weight) + (232_772_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn increase_message_fee() -> Weight { - (6_645_750_000 as Weight) + (6_580_906_000 as Weight) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof() -> Weight { - (229_797_000 as Weight) + (229_141_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_two_messages_proof() -> Weight { - (381_948_000 as Weight) + (381_428_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_with_outbound_lane_state() -> Weight { - (249_688_000 as Weight) + (250_142_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_1_kb() -> Weight { - (259_516_000 as Weight) + (256_353_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof_16_kb() -> Weight { - (543_558_000 as Weight) + (547_044_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_prepaid_message_proof() -> Weight { - (156_777_000 as Weight) + (155_311_000 as Weight) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn receive_delivery_proof_for_single_message() -> Weight { - (144_411_000 as Weight) + (143_410_000 as Weight) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_single_relayer() -> Weight { - (152_865_000 as Weight) + (150_649_000 as Weight) .saturating_add(RocksDbWeight::get().reads(7 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_two_messages_by_two_relayers() -> Weight { - (215_712_000 as Weight) + (212_115_000 as Weight) .saturating_add(RocksDbWeight::get().reads(8 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn send_messages_of_various_lengths(i: u32) -> Weight { - (121_683_000 as Weight) + (141_468_000 as Weight) .saturating_add((3_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn receive_multiple_messages_proof(i: u32) -> Weight { (0 as Weight) - .saturating_add((161_753_000 as Weight).saturating_mul(i as Weight)) + .saturating_add((162_323_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_message_proofs_with_extra_nodes(i: u32) -> Weight { - (125_849_000 as Weight) - .saturating_add((12_000 as Weight).saturating_mul(i as Weight)) + (516_172_000 as Weight) + .saturating_add((10_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_message_proofs_with_large_leaf(i: u32) -> Weight { - (197_283_000 as Weight) - .saturating_add((9_000 as Weight).saturating_mul(i as Weight)) + (204_903_000 as Weight) + .saturating_add((7_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_multiple_messages_proof_with_outbound_lane_state(i: u32) -> Weight { - (242_925_000 as Weight) - .saturating_add((187_149_000 as Weight).saturating_mul(i as Weight)) + (0 as Weight) + .saturating_add((164_287_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_delivery_proof_for_multiple_messages_by_single_relayer(i: u32) -> Weight { - (122_411_000 as Weight) - .saturating_add((10_096_000 as Weight).saturating_mul(i as Weight)) + (123_564_000 as Weight) + .saturating_add((8_112_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().reads((1 as Weight).saturating_mul(i as Weight))) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_delivery_proof_for_multiple_messages_by_multiple_relayers(i: u32) -> Weight { - (206_031_000 as Weight) - .saturating_add((80_639_000 as Weight).saturating_mul(i as Weight)) + (0 as Weight) + .saturating_add((73_627_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().reads((2 as Weight).saturating_mul(i as Weight))) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) diff --git a/modules/messages/src/weights_ext.rs b/modules/messages/src/weights_ext.rs index cb754a1023..be440174b4 100644 --- a/modules/messages/src/weights_ext.rs +++ b/modules/messages/src/weights_ext.rs @@ -34,6 +34,7 @@ pub fn ensure_weights_are_correct( expected_default_message_delivery_tx_weight: Weight, expected_additional_byte_delivery_weight: Weight, expected_messages_delivery_confirmation_tx_weight: Weight, + expected_pay_inbound_dispatch_fee_weight: Weight, ) { // verify `send_message` weight components assert_ne!(W::send_message_overhead(), 0); @@ -88,6 +89,15 @@ pub fn ensure_weights_are_correct( actual_messages_delivery_confirmation_tx_weight, expected_messages_delivery_confirmation_tx_weight, ); + + // verify pay-dispatch-fee overhead for inbound messages + let actual_pay_inbound_dispatch_fee_weight = W::pay_inbound_dispatch_fee_overhead(); + assert!( + actual_pay_inbound_dispatch_fee_weight <= expected_pay_inbound_dispatch_fee_weight, + "Weight {} of pay-dispatch-fee overhead for inbound messages is larger than expected weight {}", + actual_pay_inbound_dispatch_fee_weight, + expected_pay_inbound_dispatch_fee_weight, + ); } /// Ensure that we're able to receive maximal (by-size and by-weight) message from other chain. @@ -304,6 +314,13 @@ pub trait WeightInfoExt: WeightInfo { (Self::receive_single_message_proof_16_kb() - Self::receive_single_message_proof_1_kb()) / (15 * 1024); proof_size_in_bytes * byte_weight } + + /// Returns weight of the pay-dispatch-fee operation for inbound messages. + /// + /// This function may return zero if runtime doesn't support pay-dispatch-fee-at-target-chain option. + fn pay_inbound_dispatch_fee_overhead() -> Weight { + Self::receive_single_message_proof().saturating_sub(Self::receive_single_prepaid_message_proof()) + } } impl WeightInfoExt for () { diff --git a/primitives/chain-millau/src/lib.rs b/primitives/chain-millau/src/lib.rs index 22f09cb5b0..7aa5982694 100644 --- a/primitives/chain-millau/src/lib.rs +++ b/primitives/chain-millau/src/lib.rs @@ -80,7 +80,7 @@ pub const MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE: MessageNonce = 1024; /// for the case when single message of `pallet_bridge_messages::EXPECTED_DEFAULT_MESSAGE_LENGTH` bytes is delivered. /// The message must have dispatch weight set to zero. The result then must be rounded up to account /// possible future runtime upgrades. -pub const DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT: Weight = 1_000_000_000; +pub const DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT: Weight = 1_500_000_000; /// Increase of delivery transaction weight on Millau chain with every additional message byte. /// @@ -95,6 +95,13 @@ pub const ADDITIONAL_MESSAGE_BYTE_DELIVERY_WEIGHT: Weight = 25_000; /// runtime upgrades. pub const MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT: Weight = 2_000_000_000; +/// Weight of pay-dispatch-fee operation for inbound messages at Millau chain. +/// +/// This value corresponds to the result of `pallet_bridge_messages::WeightInfoExt::pay_inbound_dispatch_fee_overhead()` +/// call for your chain. Don't put too much reserve there, because it is used to **decrease** +/// `DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT` cost. So putting large reserve would make delivery transactions cheaper. +pub const PAY_INBOUND_DISPATCH_FEE_WEIGHT: Weight = 600_000_000; + /// The target length of a session (how often authorities change) on Millau measured in of number of /// blocks. /// diff --git a/primitives/chain-rialto/src/lib.rs b/primitives/chain-rialto/src/lib.rs index c10f31bae3..62576036f4 100644 --- a/primitives/chain-rialto/src/lib.rs +++ b/primitives/chain-rialto/src/lib.rs @@ -71,7 +71,7 @@ pub const MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE: MessageNonce = 128; /// for the case when single message of `pallet_bridge_messages::EXPECTED_DEFAULT_MESSAGE_LENGTH` bytes is delivered. /// The message must have dispatch weight set to zero. The result then must be rounded up to account /// possible future runtime upgrades. -pub const DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT: Weight = 1_000_000_000; +pub const DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT: Weight = 1_500_000_000; /// Increase of delivery transaction weight on Rialto chain with every additional message byte. /// @@ -86,6 +86,13 @@ pub const ADDITIONAL_MESSAGE_BYTE_DELIVERY_WEIGHT: Weight = 25_000; /// runtime upgrades. pub const MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT: Weight = 2_000_000_000; +/// Weight of pay-dispatch-fee operation for inbound messages at Rialto chain. +/// +/// This value corresponds to the result of `pallet_bridge_messages::WeightInfoExt::pay_inbound_dispatch_fee_overhead()` +/// call for your chain. Don't put too much reserve there, because it is used to **decrease** +/// `DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT` cost. So putting large reserve would make delivery transactions cheaper. +pub const PAY_INBOUND_DISPATCH_FEE_WEIGHT: Weight = 600_000_000; + /// The target length of a session (how often authorities change) on Rialto measured in of number of /// blocks. /// diff --git a/primitives/message-dispatch/Cargo.toml b/primitives/message-dispatch/Cargo.toml index 293c637e8d..687b5e3bb6 100644 --- a/primitives/message-dispatch/Cargo.toml +++ b/primitives/message-dispatch/Cargo.toml @@ -10,9 +10,14 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" bp-runtime = { path = "../runtime", default-features = false } codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false } +# Substrate Dependencies + +frame-support = { git = "https://github.com/paritytech/substrate", branch = "master" , default-features = false } + [features] default = ["std"] std = [ "bp-runtime/std", "codec/std", + "frame-support/std", ] diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index 2fbcfc86e8..a9544df173 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -20,10 +20,28 @@ #![warn(missing_docs)] use bp_runtime::InstanceId; +use codec::{Decode, Encode}; +use frame_support::RuntimeDebug; /// Message dispatch weight. pub type Weight = u64; +/// Message dispatch result. +#[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq)] +pub struct MessageDispatchResult { + /// Unspent dispatch weight. This weight that will be deducted from total delivery transaction + /// weight, thus reducing the transaction cost. This shall not be zero in (at least) two cases: + /// + /// 1) if message has been dispatched successfully, but post-dispatch weight is less than + /// the weight, declared by the message sender; + /// 2) if message has not been dispatched at all. + pub unspent_weight: Weight, + /// Whether the message dispatch fee has been paid during dispatch. This will be true if your + /// configuration supports pay-dispatch-fee-at-target-chain option and message sender has enabled + /// this option. + pub dispatch_fee_paid_during_dispatch: bool, +} + /// A generic trait to dispatch arbitrary messages delivered over the bridge. pub trait MessageDispatch { /// A type of the message to be dispatched. @@ -52,5 +70,5 @@ pub trait MessageDispatch { id: MessageId, message: Result, pay_dispatch_fee: P, - ) -> Weight; + ) -> MessageDispatchResult; } diff --git a/primitives/messages/Cargo.toml b/primitives/messages/Cargo.toml index 9cb037a34c..b460584824 100644 --- a/primitives/messages/Cargo.toml +++ b/primitives/messages/Cargo.toml @@ -11,6 +11,7 @@ codec = { package = "parity-scale-codec", version = "2.0.0", default-features = # Bridge dependencies +bp-message-dispatch = { path = "../message-dispatch", default-features = false } bp-runtime = { path = "../runtime", default-features = false } # Substrate Dependencies @@ -22,6 +23,7 @@ sp-std = { git = "https://github.com/paritytech/substrate", branch = "master" , [features] default = ["std"] std = [ + "bp-message-dispatch/std", "bp-runtime/std", "codec/std", "frame-support/std", diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index 9db2c1a8eb..8864a34542 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -16,6 +16,8 @@ //! Primitives of messages module, that are used on the target chain. +pub use bp_message_dispatch::MessageDispatchResult; + use crate::{LaneId, Message, MessageData, MessageKey, OutboundLaneData}; use bp_runtime::Size; @@ -103,14 +105,10 @@ pub trait MessageDispatch { /// /// If your configuration allows paying dispatch fee at the target chain, then /// it must be paid inside this method to the `relayer_account`. - /// - /// Returns 'unused' dispatch weight that will be deducted from total delivery transaction - /// weight, thus reducing the transaction cost. This shall not be zero in two cases: - /// - /// 1) if message has been dispatched successfully, but post-dispatch weight is less than - /// the weight, declared by the message sender; - /// 2) if message has not been dispatched at all. - fn dispatch(relayer_account: &AccountId, message: DispatchMessage) -> Weight; + fn dispatch( + relayer_account: &AccountId, + message: DispatchMessage, + ) -> MessageDispatchResult; } impl Default for ProvedLaneMessages { @@ -166,7 +164,10 @@ impl MessageDispatch for ForbidInboundMessages { Weight::MAX } - fn dispatch(_relayer_account: &AccountId, _message: DispatchMessage) -> Weight { - 0 + fn dispatch(_: &AccountId, _: DispatchMessage) -> MessageDispatchResult { + MessageDispatchResult { + unspent_weight: 0, + dispatch_fee_paid_during_dispatch: false, + } } } diff --git a/relays/bin-substrate/src/messages_lane.rs b/relays/bin-substrate/src/messages_lane.rs index 9948b6ec08..44c2e75a5a 100644 --- a/relays/bin-substrate/src/messages_lane.rs +++ b/relays/bin-substrate/src/messages_lane.rs @@ -203,7 +203,7 @@ mod tests { // reserved for messages dispatch allows dispatch of non-trivial messages. // // Any significant change in this values should attract additional attention. - (1020, 216_583_333_334), + (704, 216_583_333_334), ); } } diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index 41eee606d8..83532f5cc4 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -860,6 +860,5 @@ pub(crate) mod tests { // check that we have at least once required new source->target or target->source headers assert!(!result.target_to_source_header_requirements.is_empty()); - assert!(!result.source_to_target_header_requirements.is_empty()); } } From 014ef8280c721e53380c92ce5dfe462a09ca06c1 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 21 Apr 2021 12:26:28 +0300 Subject: [PATCH 06/14] remvoe some redundant traces --- modules/dispatch/src/lib.rs | 6 ++++-- modules/messages/src/benchmarking.rs | 14 ++------------ 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index 12cfd10014..d972caf829 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -869,7 +869,8 @@ mod tests { let message = prepare_target_message(call); System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + assert!(!result.dispatch_fee_paid_during_dispatch); assert_eq!( System::events(), @@ -896,7 +897,8 @@ mod tests { let message = prepare_source_message(call); System::set_block_number(1); - Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| unreachable!()); + assert!(!result.dispatch_fee_paid_during_dispatch); assert_eq!( System::events(), diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index 445dd377dc..d02e00a664 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -55,8 +55,6 @@ pub struct MessageParams { pub size: u32, /// Message sender account. pub sender_account: ThisAccountId, - /// If true, dispatch fee is paid at the target chain (if supported by configuration). - pub pay_dispatch_fee_at_target_chain: bool, } /// Benchmark-specific message proof parameters. @@ -98,8 +96,6 @@ pub trait Config: crate::Config { /// Create given account and give it enough balance for test purposes. fn endow_account(account: &Self::AccountId); /// Prepare message to send over lane. - /// - /// Dispatch fee is assumed to be paid at the source chain. fn prepare_outbound_message( params: MessageParams, ) -> (Self::OutboundPayload, Self::OutboundMessageFee); @@ -127,8 +123,7 @@ benchmarks_instance! { // * outbound lane already has state, so it needs to be read and decoded; // * relayers fund account does not exists (in practice it needs to exist in production environment); // * maximal number of messages is being pruned during the call; - // * message size is minimal for the target chain; - // * message is paid at the source (this) chain. + // * message size is minimal for the target chain. // // Result of this benchmark is used as a base weight for `send_message` call. Then the 'message weight' // (estimated using `send_half_maximal_message_worst_case` and `send_maximal_message_worst_case`) is @@ -147,7 +142,6 @@ benchmarks_instance! { let (payload, fee) = T::prepare_outbound_message(MessageParams { size: 0, sender_account: sender.clone(), - pay_dispatch_fee_at_target_chain: false, }); }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) verify { @@ -185,7 +179,6 @@ benchmarks_instance! { let (payload, fee) = T::prepare_outbound_message(MessageParams { size, sender_account: sender.clone(), - pay_dispatch_fee_at_target_chain: false, }); }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) verify { @@ -223,7 +216,6 @@ benchmarks_instance! { let (payload, fee) = T::prepare_outbound_message(MessageParams { size, sender_account: sender.clone(), - pay_dispatch_fee_at_target_chain: false, }); }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) verify { @@ -575,8 +567,7 @@ benchmarks_instance! { // * outbound lane already has state, so it needs to be read and decoded; // * relayers fund account does not exists (in practice it needs to exist in production environment); // * maximal number of messages is being pruned during the call; - // * message size varies from minimal to maximal for the target chain; - // * message dispatch fee is paid at this (source) chain. + // * message size varies from minimal to maximal for the target chain. // // Results of this benchmark may be used to check how message size affects `send_message` performance. send_messages_of_various_lengths { @@ -595,7 +586,6 @@ benchmarks_instance! { let (payload, fee) = T::prepare_outbound_message(MessageParams { size: i as _, sender_account: sender.clone(), - pay_dispatch_fee_at_target_chain: false, }); }: send_message(RawOrigin::Signed(sender), lane_id, payload, fee) verify { From ab013940841f8b721c30bd72c95e60d28952aef9 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 21 Apr 2021 12:56:24 +0300 Subject: [PATCH 07/14] enum DispatchFeePayment {} --- Cargo.lock | 4 +- bin/rialto/runtime/src/lib.rs | 9 ++-- bin/rialto/runtime/src/millau_messages.rs | 4 +- bin/runtime-common/src/messages.rs | 32 +++++++------ modules/dispatch/src/lib.rs | 32 ++++++------- modules/messages/Cargo.toml | 2 + modules/messages/src/benchmarking.rs | 23 ++++----- modules/messages/src/inbound_lane.rs | 3 +- modules/messages/src/mock.rs | 6 +-- primitives/message-dispatch/Cargo.toml | 5 -- primitives/message-dispatch/src/lib.rs | 20 +------- primitives/messages/Cargo.toml | 2 - primitives/messages/src/target_chain.rs | 4 +- primitives/runtime/src/lib.rs | 2 + primitives/runtime/src/messages.rs | 50 ++++++++++++++++++++ relays/bin-substrate/Cargo.toml | 1 + relays/bin-substrate/src/cli/send_message.rs | 11 +++-- 17 files changed, 122 insertions(+), 88 deletions(-) create mode 100644 primitives/runtime/src/messages.rs diff --git a/Cargo.lock b/Cargo.lock index 45eb2e6ed7..03912d8c99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -711,7 +711,6 @@ name = "bp-message-dispatch" version = "0.1.0" dependencies = [ "bp-runtime", - "frame-support", "parity-scale-codec 2.0.1", ] @@ -719,7 +718,6 @@ dependencies = [ name = "bp-messages" version = "0.1.0" dependencies = [ - "bp-message-dispatch", "bp-runtime", "frame-support", "frame-system", @@ -4703,6 +4701,7 @@ dependencies = [ name = "pallet-bridge-messages" version = "0.1.0" dependencies = [ + "bp-message-dispatch", "bp-messages", "bp-rialto", "bp-runtime", @@ -8427,6 +8426,7 @@ dependencies = [ "async-trait", "bp-header-chain", "bp-kusama", + "bp-message-dispatch", "bp-messages", "bp-millau", "bp-polkadot", diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index 7da8b7bbc8..4635dc8dbb 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -840,6 +840,7 @@ impl_runtime_apis! { } use crate::millau_messages::{ToMillauMessagePayload, WithMillauMessageBridge}; + use bp_runtime::messages::DispatchFeePayment; use bridge_runtime_common::messages; use pallet_bridge_messages::benchmarking::{ Pallet as MessagesBench, @@ -883,7 +884,7 @@ impl_runtime_apis! { weight: params.size as _, origin: dispatch_origin, call: message_payload, - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, }; (message, pallet_bridge_messages::benchmarking::MESSAGE_FEE.into()) } @@ -921,7 +922,7 @@ impl_runtime_apis! { rialto_raw_signature, )); - if params.pay_dispatch_fee_at_target_chain { + if params.dispatch_fee_payment == DispatchFeePayment::AtTargetChain { Self::endow_account(&rialto_public.clone().into_account()); } @@ -945,7 +946,7 @@ impl_runtime_apis! { Default::default(), ); - let pay_dispatch_fee_at_target_chain = params.pay_dispatch_fee_at_target_chain; + let dispatch_fee_payment = params.dispatch_fee_payment.clone(); prepare_message_proof::( params, make_millau_message_key, @@ -964,7 +965,7 @@ impl_runtime_apis! { rialto_public, rialto_signature, ), - pay_dispatch_fee_at_target_chain, + dispatch_fee_payment, call: call.encode(), }.encode(), ) diff --git a/bin/rialto/runtime/src/millau_messages.rs b/bin/rialto/runtime/src/millau_messages.rs index 3209d36d35..ec008411d6 100644 --- a/bin/rialto/runtime/src/millau_messages.rs +++ b/bin/rialto/runtime/src/millau_messages.rs @@ -267,7 +267,7 @@ mod tests { target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, MessageKey, }; - use bp_runtime::{derive_account_id, SourceAccount}; + use bp_runtime::{derive_account_id, messages::DispatchFeePayment, SourceAccount}; use bridge_runtime_common::messages::target::{FromBridgedChainEncodedMessageCall, FromBridgedChainMessagePayload}; use frame_support::{ traits::Currency, @@ -322,7 +322,7 @@ mod tests { spec_version: VERSION.spec_version, weight: dispatch_weight, origin: CallOrigin::SourceRoot, - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, call: FromBridgedChainEncodedMessageCall::new(call.encode()), }), fee: 1, diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index 0fe637db1a..04d8245511 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -23,10 +23,13 @@ use bp_message_dispatch::MessageDispatch as _; use bp_messages::{ source_chain::{LaneMessageVerifier, Sender}, - target_chain::{DispatchMessage, MessageDispatch, MessageDispatchResult, ProvedLaneMessages, ProvedMessages}, + target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages}, InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData, }; -use bp_runtime::{InstanceId, Size, StorageProofChecker}; +use bp_runtime::{ + messages::{DispatchFeePayment, MessageDispatchResult}, + InstanceId, Size, StorageProofChecker, +}; use codec::{Decode, Encode}; use frame_support::{ traits::{Currency, ExistenceRequirement, Instance}, @@ -334,10 +337,11 @@ pub mod source { // // if we're going to pay dispatch fee at the target chain, then we don't include weight // of the message dispatch in the delivery transaction cost + let pay_dispatch_fee_at_target_chain = payload.dispatch_fee_payment == DispatchFeePayment::AtTargetChain; let delivery_transaction = BridgedChain::::estimate_delivery_transaction( &payload.call, - payload.pay_dispatch_fee_at_target_chain, - if payload.pay_dispatch_fee_at_target_chain { + pay_dispatch_fee_at_target_chain, + if pay_dispatch_fee_at_target_chain { 0.into() } else { payload.weight.into() @@ -977,7 +981,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, call: ThisChainCall::Transfer.encode(), } .encode(); @@ -992,7 +996,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, call: target::FromBridgedChainEncodedMessageCall::::new( ThisChainCall::Transfer.encode(), ), @@ -1009,7 +1013,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![42], } } @@ -1032,7 +1036,7 @@ mod tests { // let's check if estimation is less than hardcoded, if dispatch is paid at target chain let mut payload_with_pay_on_target = regular_outbound_message_payload(); - payload_with_pay_on_target.pay_dispatch_fee_at_target_chain = true; + payload_with_pay_on_target.dispatch_fee_payment = DispatchFeePayment::AtTargetChain; let fee_at_source = source::estimate_message_dispatch_and_delivery_fee::( &payload_with_pay_on_target, OnThisChainBridge::RELAYER_FEE_PERCENT, @@ -1075,7 +1079,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![42], }; @@ -1119,7 +1123,7 @@ mod tests { spec_version: 1, weight: 100, origin: pallet_bridge_dispatch::CallOrigin::SourceAccount(ThisChainAccountId(1)), - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![42], }; @@ -1187,7 +1191,7 @@ mod tests { spec_version: 1, weight: 5, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![1, 2, 3, 4, 5, 6], },) .is_err() @@ -1203,7 +1207,7 @@ mod tests { spec_version: 1, weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT + 1, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![1, 2, 3, 4, 5, 6], },) .is_err() @@ -1219,7 +1223,7 @@ mod tests { spec_version: 1, weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![0; source::maximal_message_size::() as usize + 1], },) .is_err() @@ -1235,7 +1239,7 @@ mod tests { spec_version: 1, weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT, origin: pallet_bridge_dispatch::CallOrigin::SourceRoot, - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![0; source::maximal_message_size::() as _], },), Ok(()), diff --git a/modules/dispatch/src/lib.rs b/modules/dispatch/src/lib.rs index d972caf829..2784b20969 100644 --- a/modules/dispatch/src/lib.rs +++ b/modules/dispatch/src/lib.rs @@ -24,8 +24,12 @@ #![cfg_attr(not(feature = "std"), no_std)] #![warn(missing_docs)] -use bp_message_dispatch::{MessageDispatch, MessageDispatchResult, Weight}; -use bp_runtime::{derive_account_id, InstanceId, Size, SourceAccount}; +use bp_message_dispatch::{MessageDispatch, Weight}; +use bp_runtime::{ + derive_account_id, + messages::{DispatchFeePayment, MessageDispatchResult}, + InstanceId, Size, SourceAccount, +}; use codec::{Decode, Encode}; use frame_support::{ decl_event, decl_module, decl_storage, @@ -102,13 +106,8 @@ pub struct MessagePayload, - /// If true, then the sender has decided to pay dispatch fee at the target chain. - /// - /// The fee is paid right before the message is dispatched. So in case of any other - /// issues (like invalid call encoding, invalid signature, ...) we won't do any direct - /// transfers. Instead, we're returning fee related to this message dispatch to the - /// relayer. - pub pay_dispatch_fee_at_target_chain: bool, + /// Where message dispatch fee is paid? + pub dispatch_fee_payment: DispatchFeePayment, /// The call itself. pub call: Call, } @@ -180,14 +179,14 @@ decl_event!( MessageWeightMismatch(InstanceId, MessageId, Weight, Weight), /// Message signature mismatch. MessageSignatureMismatch(InstanceId, MessageId), - /// Message has been dispatched with given result. - MessageDispatched(InstanceId, MessageId, DispatchResult), /// We have failed to decode Call from the message. MessageCallDecodeFailed(InstanceId, MessageId), /// The call from the message has been rejected by the call filter. MessageCallRejected(InstanceId, MessageId), /// The origin account has failed to pay fee for dispatching the message. MessageDispatchPaymentFailed(InstanceId, MessageId, AccountId, Weight), + /// Message has been dispatched with given result. + MessageDispatched(InstanceId, MessageId, DispatchResult), /// Phantom member, never used. Needed to handle multiple pallet instances. _Dummy(PhantomData), } @@ -335,7 +334,8 @@ impl, I: Instance> MessageDispatch for } // pay dispatch fee right before dispatch - if message.pay_dispatch_fee_at_target_chain && pay_dispatch_fee(&origin_account, message.weight).is_err() { + let pay_dispatch_fee_at_target_chain = message.dispatch_fee_payment == DispatchFeePayment::AtTargetChain; + if pay_dispatch_fee_at_target_chain && pay_dispatch_fee(&origin_account, message.weight).is_err() { log::trace!( target: "runtime::bridge-dispatch", "Failed to pay dispatch fee for dispatching message {:?}/{:?} with weight {}", @@ -351,7 +351,7 @@ impl, I: Instance> MessageDispatch for )); return dispatch_result; } - dispatch_result.dispatch_fee_paid_during_dispatch = message.pay_dispatch_fee_at_target_chain; + dispatch_result.dispatch_fee_paid_during_dispatch = pay_dispatch_fee_at_target_chain; // finally dispatch message let origin = RawOrigin::Signed(origin_account).into(); @@ -586,7 +586,7 @@ mod tests { spec_version: TEST_SPEC_VERSION, weight: TEST_WEIGHT, origin, - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: EncodedCall(call.encode()), } } @@ -782,7 +782,7 @@ mod tests { let mut message = prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); let weight = message.weight; - message.pay_dispatch_fee_at_target_chain = true; + message.dispatch_fee_payment = DispatchFeePayment::AtTargetChain; System::set_block_number(1); let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| Err(())); @@ -812,7 +812,7 @@ mod tests { let mut message = prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); - message.pay_dispatch_fee_at_target_chain = true; + message.dispatch_fee_payment = DispatchFeePayment::AtTargetChain; System::set_block_number(1); let result = Dispatch::dispatch(bridge, id, Ok(message), |_, _| Ok(())); diff --git a/modules/messages/Cargo.toml b/modules/messages/Cargo.toml index 4a75fa8181..983a74fe75 100644 --- a/modules/messages/Cargo.toml +++ b/modules/messages/Cargo.toml @@ -14,6 +14,7 @@ serde = { version = "1.0.101", optional = true, features = ["derive"] } # Bridge dependencies +bp-message-dispatch = { path = "../../primitives/message-dispatch", default-features = false } bp-messages = { path = "../../primitives/messages", default-features = false } bp-rialto = { path = "../../primitives/chain-rialto", default-features = false } bp-runtime = { path = "../../primitives/runtime", default-features = false } @@ -36,6 +37,7 @@ pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "m [features] default = ["std"] std = [ + "bp-message-dispatch/std", "bp-messages/std", "bp-runtime/std", "bp-rialto/std", diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index d02e00a664..29d110c6e3 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -23,6 +23,7 @@ use bp_messages::{ source_chain::TargetHeaderChain, target_chain::SourceHeaderChain, InboundLaneData, LaneId, MessageData, MessageNonce, OutboundLaneData, UnrewardedRelayersState, }; +use bp_runtime::messages::DispatchFeePayment; use frame_benchmarking::{account, benchmarks_instance}; use frame_support::{traits::Get, weights::Weight}; use frame_system::RawOrigin; @@ -68,7 +69,7 @@ pub struct MessageProofParams { /// Proof size requirements. pub size: ProofSize, /// If true, dispatch fee is paid at the target chain (if supported by configuration). - pub pay_dispatch_fee_at_target_chain: bool, + pub dispatch_fee_payment: DispatchFeePayment, } /// Benchmark-specific message delivery proof parameters. @@ -262,7 +263,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -296,7 +297,7 @@ benchmarks_instance! { message_nonces: 21..=22, outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 2, dispatch_weight) verify { @@ -334,7 +335,7 @@ benchmarks_instance! { latest_generated_nonce: 21, }), size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -370,7 +371,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasExtraNodes(1024), - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -404,7 +405,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasExtraNodes(16 * 1024), - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -437,7 +438,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -619,7 +620,7 @@ benchmarks_instance! { message_nonces: 21..=(20 + i as MessageNonce), outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), @@ -657,7 +658,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasExtraNodes(i as _), - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), @@ -695,7 +696,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasLargeLeaf(i as _), - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), @@ -739,7 +740,7 @@ benchmarks_instance! { latest_generated_nonce: 21, }), size: ProofSize::Minimal(0), - pay_dispatch_fee_at_target_chain: true, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 0d63176cd4..7fa2161241 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -17,9 +17,10 @@ //! Everything about incoming messages receival. use bp_messages::{ - target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch, MessageDispatchResult}, + target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, InboundLaneData, LaneId, MessageKey, MessageNonce, OutboundLaneData, }; +use bp_runtime::messages::MessageDispatchResult; use frame_support::RuntimeDebug; use sp_std::prelude::PartialEq; diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index 43cc4fcf40..457219a04a 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -23,13 +23,11 @@ use bp_messages::{ source_chain::{ LaneMessageVerifier, MessageDeliveryAndDispatchPayment, RelayersRewards, Sender, TargetHeaderChain, }, - target_chain::{ - DispatchMessage, MessageDispatch, MessageDispatchResult, ProvedLaneMessages, ProvedMessages, SourceHeaderChain, - }, + target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData, Parameter as MessagesParameter, }; -use bp_runtime::Size; +use bp_runtime::{messages::MessageDispatchResult, Size}; use codec::{Decode, Encode}; use frame_support::{parameter_types, weights::Weight}; use sp_core::H256; diff --git a/primitives/message-dispatch/Cargo.toml b/primitives/message-dispatch/Cargo.toml index 687b5e3bb6..293c637e8d 100644 --- a/primitives/message-dispatch/Cargo.toml +++ b/primitives/message-dispatch/Cargo.toml @@ -10,14 +10,9 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" bp-runtime = { path = "../runtime", default-features = false } codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false } -# Substrate Dependencies - -frame-support = { git = "https://github.com/paritytech/substrate", branch = "master" , default-features = false } - [features] default = ["std"] std = [ "bp-runtime/std", "codec/std", - "frame-support/std", ] diff --git a/primitives/message-dispatch/src/lib.rs b/primitives/message-dispatch/src/lib.rs index a9544df173..d6eb6dd9d5 100644 --- a/primitives/message-dispatch/src/lib.rs +++ b/primitives/message-dispatch/src/lib.rs @@ -19,29 +19,11 @@ #![cfg_attr(not(feature = "std"), no_std)] #![warn(missing_docs)] -use bp_runtime::InstanceId; -use codec::{Decode, Encode}; -use frame_support::RuntimeDebug; +use bp_runtime::{messages::MessageDispatchResult, InstanceId}; /// Message dispatch weight. pub type Weight = u64; -/// Message dispatch result. -#[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq)] -pub struct MessageDispatchResult { - /// Unspent dispatch weight. This weight that will be deducted from total delivery transaction - /// weight, thus reducing the transaction cost. This shall not be zero in (at least) two cases: - /// - /// 1) if message has been dispatched successfully, but post-dispatch weight is less than - /// the weight, declared by the message sender; - /// 2) if message has not been dispatched at all. - pub unspent_weight: Weight, - /// Whether the message dispatch fee has been paid during dispatch. This will be true if your - /// configuration supports pay-dispatch-fee-at-target-chain option and message sender has enabled - /// this option. - pub dispatch_fee_paid_during_dispatch: bool, -} - /// A generic trait to dispatch arbitrary messages delivered over the bridge. pub trait MessageDispatch { /// A type of the message to be dispatched. diff --git a/primitives/messages/Cargo.toml b/primitives/messages/Cargo.toml index b460584824..9cb037a34c 100644 --- a/primitives/messages/Cargo.toml +++ b/primitives/messages/Cargo.toml @@ -11,7 +11,6 @@ codec = { package = "parity-scale-codec", version = "2.0.0", default-features = # Bridge dependencies -bp-message-dispatch = { path = "../message-dispatch", default-features = false } bp-runtime = { path = "../runtime", default-features = false } # Substrate Dependencies @@ -23,7 +22,6 @@ sp-std = { git = "https://github.com/paritytech/substrate", branch = "master" , [features] default = ["std"] std = [ - "bp-message-dispatch/std", "bp-runtime/std", "codec/std", "frame-support/std", diff --git a/primitives/messages/src/target_chain.rs b/primitives/messages/src/target_chain.rs index 8864a34542..2a649f54d7 100644 --- a/primitives/messages/src/target_chain.rs +++ b/primitives/messages/src/target_chain.rs @@ -16,11 +16,9 @@ //! Primitives of messages module, that are used on the target chain. -pub use bp_message_dispatch::MessageDispatchResult; - use crate::{LaneId, Message, MessageData, MessageKey, OutboundLaneData}; -use bp_runtime::Size; +use bp_runtime::{messages::MessageDispatchResult, Size}; use codec::{Decode, Encode, Error as CodecError}; use frame_support::{weights::Weight, Parameter, RuntimeDebug}; use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, prelude::*}; diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index e7f990d283..634f0f3cdc 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -29,6 +29,8 @@ pub use storage_proof::{Error as StorageProofError, StorageProofChecker}; #[cfg(feature = "std")] pub use storage_proof::craft_valid_storage_proof; +pub mod messages; + mod chain; mod storage_proof; diff --git a/primitives/runtime/src/messages.rs b/primitives/runtime/src/messages.rs new file mode 100644 index 0000000000..e97430c5b0 --- /dev/null +++ b/primitives/runtime/src/messages.rs @@ -0,0 +1,50 @@ +// Copyright 2019-2021 Parity Technologies (UK) Ltd. +// This file is part of Parity Bridges Common. + +// Parity Bridges Common is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity Bridges Common is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity Bridges Common. If not, see . + +//! Primitives that may be used by different message delivery and dispatch mechanisms. + +use codec::{Decode, Encode}; +use frame_support::{weights::Weight, RuntimeDebug}; + +/// Where message dispatch fee is paid? +#[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq)] +pub enum DispatchFeePayment { + /// The dispacth fee is paid at the source chain. + AtSourceChain, + /// The dispatch fee is paid at the target chain. + /// + /// The fee will be paid right before the message is dispatched. So in case of any other + /// issues (like invalid call encoding, invalid signature, ...) the dispatch module won't + /// do any direct transfers. Instead, it'll return fee related to this message dispatch to the + /// relayer. + AtTargetChain, +} + +/// Message dispatch result. +#[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq)] +pub struct MessageDispatchResult { + /// Unspent dispatch weight. This weight that will be deducted from total delivery transaction + /// weight, thus reducing the transaction cost. This shall not be zero in (at least) two cases: + /// + /// 1) if message has been dispatched successfully, but post-dispatch weight is less than + /// the weight, declared by the message sender; + /// 2) if message has not been dispatched at all. + pub unspent_weight: Weight, + /// Whether the message dispatch fee has been paid during dispatch. This will be true if your + /// configuration supports pay-dispatch-fee-at-target-chain option and message sender has enabled + /// this option. + pub dispatch_fee_paid_during_dispatch: bool, +} diff --git a/relays/bin-substrate/Cargo.toml b/relays/bin-substrate/Cargo.toml index d203201e60..33bf2b6308 100644 --- a/relays/bin-substrate/Cargo.toml +++ b/relays/bin-substrate/Cargo.toml @@ -22,6 +22,7 @@ structopt = "0.3" bp-header-chain = { path = "../../primitives/header-chain" } bp-kusama = { path = "../../primitives/chain-kusama" } +bp-message-dispatch = { path = "../../primitives/message-dispatch" } bp-messages = { path = "../../primitives/messages" } bp-millau = { path = "../../primitives/chain-millau" } bp-polkadot = { path = "../../primitives/chain-polkadot" } diff --git a/relays/bin-substrate/src/cli/send_message.rs b/relays/bin-substrate/src/cli/send_message.rs index 36f7a52af5..e9795f1392 100644 --- a/relays/bin-substrate/src/cli/send_message.rs +++ b/relays/bin-substrate/src/cli/send_message.rs @@ -21,6 +21,7 @@ use crate::cli::{ Balance, CliChain, ExplicitOrMaximal, HexBytes, HexLaneId, Origins, SourceConnectionParams, SourceSigningParams, TargetSigningParams, }; +use bp_runtime::messages::DispatchFeePayment; use codec::Encode; use frame_support::{dispatch::GetDispatchInfo, weights::Weight}; use pallet_bridge_dispatch::{CallOrigin, MessagePayload}; @@ -210,7 +211,7 @@ where spec_version, weight, origin, - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: HexBytes::encode(call), }; @@ -222,14 +223,14 @@ where spec_version, weight, origin, - pay_dispatch_fee_at_target_chain, + dispatch_fee_payment, call, } = payload; MessagePayload { spec_version, weight, origin, - pay_dispatch_fee_at_target_chain, + dispatch_fee_payment, call: call.0, } } @@ -270,7 +271,7 @@ mod tests { spec_version: relay_millau_client::Millau::RUNTIME_VERSION.spec_version, weight: 1345000, origin: CallOrigin::SourceAccount(sp_keyring::AccountKeyring::Alice.to_account_id()), - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: hex!("0401081234").to_vec(), } ); @@ -314,7 +315,7 @@ mod tests { sp_keyring::AccountKeyring::Bob.into(), signature, ), - pay_dispatch_fee_at_target_chain: false, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: hex!("0701081234").to_vec(), } ); From 64333c9ae6c03c16a62602cbf61341715112a720 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 21 Apr 2021 13:13:05 +0300 Subject: [PATCH 08/14] typo --- modules/messages/src/inbound_lane.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/messages/src/inbound_lane.rs b/modules/messages/src/inbound_lane.rs index 7fa2161241..92eb8d665d 100644 --- a/modules/messages/src/inbound_lane.rs +++ b/modules/messages/src/inbound_lane.rs @@ -110,7 +110,7 @@ impl InboundLane { /// Receive new message. pub fn receive_message, AccountId>( &mut self, - relayer_at_brdiged_chain: &S::Relayer, + relayer_at_bridged_chain: &S::Relayer, relayer_at_this_chain: &AccountId, nonce: MessageNonce, message_data: DispatchMessageData, @@ -133,7 +133,7 @@ impl InboundLane { } let push_new = match data.relayers.back_mut() { - Some((_, nonce_high, last_relayer)) if last_relayer == relayer_at_brdiged_chain => { + Some((_, nonce_high, last_relayer)) if last_relayer == relayer_at_bridged_chain => { *nonce_high = nonce; false } @@ -141,7 +141,7 @@ impl InboundLane { }; if push_new { data.relayers - .push_back((nonce, nonce, (*relayer_at_brdiged_chain).clone())); + .push_back((nonce, nonce, (*relayer_at_bridged_chain).clone())); } self.storage.set_data(data); From d95f320652ce136f2b265c7ad7231ea77baa9058 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 21 Apr 2021 13:29:24 +0300 Subject: [PATCH 09/14] update docs --- bin/runtime-common/README.md | 4 +++- modules/dispatch/README.md | 2 ++ modules/messages/README.md | 17 +++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/bin/runtime-common/README.md b/bin/runtime-common/README.md index b375f48309..c5da85dcf6 100644 --- a/bin/runtime-common/README.md +++ b/bin/runtime-common/README.md @@ -102,7 +102,9 @@ This trait represents this chain from bridge point of view. Let's review every m have declared dispatch weight larger than 50% of the maximal bridged extrinsic weight. - `MessageBridge::estimate_delivery_transaction`: you will need to return estimated dispatch weight and - size of the delivery transaction that delivers a given message to the target chain. + size of the delivery transaction that delivers a given message to the target chain. The transaction + weight must or must not include the weight of pay-dispatch-fee operation, depending on the value + of `include_pay_dispatch_fee_cost` argument. - `MessageBridge::transaction_payment`: you'll need to return fee that the submitter must pay for given transaction on bridged chain. The best case is when you have the same conversion diff --git a/modules/dispatch/README.md b/modules/dispatch/README.md index f2ee04beaf..c53a947b6d 100644 --- a/modules/dispatch/README.md +++ b/modules/dispatch/README.md @@ -44,6 +44,8 @@ module events set: weight, the dispatch is rejected. Keep in mind, that even if post-dispatch weight will be less than specified, the submitter still have to declare (and pay for) the maximal possible weight (that is the pre-dispatch weight); +- `MessageDispatchPaymentFailed` event is emitted if the message submitter has selected to pay + dispatch fee at the target chain, but has failed to do that; - `MessageDispatched` event is emitted if the message has passed all checks and we have actually dispatched it. The dispatch may still fail, though - that's why we are including the dispatch result in the event payload. diff --git a/modules/messages/README.md b/modules/messages/README.md index eda5e28a6c..47bcacdee7 100644 --- a/modules/messages/README.md +++ b/modules/messages/README.md @@ -347,6 +347,23 @@ Both conditions are verified by `pallet_bridge_messages::ensure_weights_are_corr `pallet_bridge_messages::ensure_able_to_receive_messages` functions, which must be called from every runtime's tests. +### Post-dispatch weight refunds of the `receive_messages_proof` call + +Weight formula of the `receive_messages_proof` call assumes that the dispatch fee of every message is +paid at the target chain (where call is executed), that every message will be dispatched and that +dispatch weight of the message will be exactly the weight that is returned from the +`MessageDispatch::dispatch_weight` method call. This isn't true for all messages, so the call returns +actual weight used to dispatch messages. + +This actual weight is the weight, returned by the weight formula, minus: +- the weight of undispatched messages, if we have failed to dispatch because of different issues; +- the unspent dispatch weight if the declared weight of some messages is less than their actual post-dispatch weight; +- the pay-dispatch-fee weight for every message that had dispatch fee paid at the source chain. + +The last component is computed as a difference between two benchmarks results - the `receive_single_message_proof` +benchmark (that assumes that the fee is paid during dispatch) and the `receive_single_prepaid_message_proof` +(that assumes that the dispatch fee is already paid). + ### Weight of `receive_messages_delivery_proof` call #### Related benchmarks From fa3a350081aeea0853ee1758ffbc150d1a0f8680 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 22 Apr 2021 12:30:54 +0300 Subject: [PATCH 10/14] (revert removal of valid check) --- relays/messages/src/message_lane_loop.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/relays/messages/src/message_lane_loop.rs b/relays/messages/src/message_lane_loop.rs index 83532f5cc4..41eee606d8 100644 --- a/relays/messages/src/message_lane_loop.rs +++ b/relays/messages/src/message_lane_loop.rs @@ -860,5 +860,6 @@ pub(crate) mod tests { // check that we have at least once required new source->target or target->source headers assert!(!result.target_to_source_header_requirements.is_empty()); + assert!(!result.source_to_target_header_requirements.is_empty()); } } From 9ec5c4577b22d994ce8b36ebdf5cf318bbc98530 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 15 Jun 2021 14:06:37 +0300 Subject: [PATCH 11/14] Update modules/messages/src/benchmarking.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomasz Drwięga --- modules/messages/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index 29d110c6e3..9b86475245 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -421,7 +421,7 @@ benchmarks_instance! { // * inbound lane already has state, so it needs to be read and decoded; // * message is successfully dispatched; // * message requires all heavy checks done by dispatcher; - // * message dispatch fee is pait at source (bridged) chain. + // * message dispatch fee is paid at source (bridged) chain. // // This benchmark is used to compute extra weight spent at target chain when fee is paid there. Then we use // this information in two places: (1) to reduce weight of delivery tx if sender pays fee at the source chain From 580277423f3966d51967c3e6cf0c2352317ee4c1 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 15 Jun 2021 14:06:44 +0300 Subject: [PATCH 12/14] Update modules/messages/src/benchmarking.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomasz Drwięga --- modules/messages/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index 9b86475245..8bc58f4f3e 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -313,7 +313,7 @@ benchmarks_instance! { // * inbound lane already has state, so it needs to be read and decoded; // * message is successfully dispatched; // * message requires all heavy checks done by dispatcher; - // * message dispatch fee is pait at target (this) chain. + // * message dispatch fee is paid at target (this) chain. // // The weight of outbound lane state delivery would be // `weight(receive_single_message_proof_with_outbound_lane_state) - weight(receive_single_message_proof)`. From 669055ce4b16fef901c67b633ca0b5df31b494cd Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 15 Jun 2021 14:06:51 +0300 Subject: [PATCH 13/14] Update modules/messages/src/benchmarking.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomasz Drwięga --- modules/messages/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index 8bc58f4f3e..5326759f9b 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -279,7 +279,7 @@ benchmarks_instance! { // * inbound lane already has state, so it needs to be read and decoded; // * message is successfully dispatched; // * message requires all heavy checks done by dispatcher; - // * message dispatch fee is pait at target (this) chain. + // * message dispatch fee is paid at target (this) chain. // // The weight of single message delivery could be approximated as // `weight(receive_two_messages_proof) - weight(receive_single_message_proof)`. From b90eada9c453b83324546267a2a4bd64848961e8 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 15 Jun 2021 14:06:57 +0300 Subject: [PATCH 14/14] Update modules/messages/src/benchmarking.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomasz Drwięga --- modules/messages/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/messages/src/benchmarking.rs b/modules/messages/src/benchmarking.rs index 5326759f9b..5fc04bb2b2 100644 --- a/modules/messages/src/benchmarking.rs +++ b/modules/messages/src/benchmarking.rs @@ -248,7 +248,7 @@ benchmarks_instance! { // * inbound lane already has state, so it needs to be read and decoded; // * message is successfully dispatched; // * message requires all heavy checks done by dispatcher; - // * message dispatch fee is pait at target (this) chain. + // * message dispatch fee is paid at target (this) chain. // // This is base benchmark for all other message delivery benchmarks. receive_single_message_proof {