From 20b7f341e1ae46c35f84a62501ddc708929775b0 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 18 Jun 2021 12:35:15 +0300 Subject: [PATCH] pay dispatch fee at target chain (#911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * pay dispatch fee at target chain * refund unspent dispatch weight to messages relayer * test that transfer actually happens * pay-at-target-cchain benchmarks + fix previous benchmarks (invalid signature) * include/exclude pay-dispatch-fee weight from delivery_and_dispatch_fee/delivery tx cost * remvoe some redundant traces * enum DispatchFeePayment {} * typo * update docs * (revert removal of valid check) * Update modules/messages/src/benchmarking.rs Co-authored-by: Tomasz Drwięga * Update modules/messages/src/benchmarking.rs Co-authored-by: Tomasz Drwięga * Update modules/messages/src/benchmarking.rs Co-authored-by: Tomasz Drwięga * Update modules/messages/src/benchmarking.rs Co-authored-by: Tomasz Drwięga Co-authored-by: Tomasz Drwięga Co-authored-by: Tomasz Drwięga --- bridges/bin/millau/runtime/src/lib.rs | 1 + .../bin/millau/runtime/src/rialto_messages.rs | 7 + bridges/bin/rialto/runtime/src/lib.rs | 26 +- .../bin/rialto/runtime/src/millau_messages.rs | 91 ++++++ bridges/bin/runtime-common/Cargo.toml | 2 + bridges/bin/runtime-common/README.md | 4 +- bridges/bin/runtime-common/src/messages.rs | 118 ++++++-- .../src/messages_benchmarking.rs | 19 +- bridges/modules/dispatch/README.md | 2 + bridges/modules/dispatch/src/lib.rs | 179 +++++++++--- bridges/modules/messages/Cargo.toml | 2 + bridges/modules/messages/README.md | 17 ++ bridges/modules/messages/src/benchmarking.rs | 61 ++++- bridges/modules/messages/src/inbound_lane.rs | 259 ++++++++++++------ bridges/modules/messages/src/lib.rs | 181 +++++++++--- bridges/modules/messages/src/mock.rs | 49 +++- bridges/modules/messages/src/weights.rs | 173 ++++++------ bridges/modules/messages/src/weights_ext.rs | 17 ++ bridges/primitives/chain-millau/src/lib.rs | 9 +- bridges/primitives/chain-rialto/src/lib.rs | 9 +- .../primitives/message-dispatch/src/lib.rs | 19 +- .../primitives/messages/src/target_chain.rs | 21 +- bridges/primitives/runtime/src/lib.rs | 2 + bridges/primitives/runtime/src/messages.rs | 50 ++++ .../bin-substrate/src/cli/encode_message.rs | 4 +- .../bin-substrate/src/cli/send_message.rs | 6 + .../relays/bin-substrate/src/messages_lane.rs | 2 +- 27 files changed, 1052 insertions(+), 278 deletions(-) create mode 100644 bridges/primitives/runtime/src/messages.rs diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index dd13afb064..cbbe7c92ee 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -686,6 +686,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/bridges/bin/millau/runtime/src/rialto_messages.rs b/bridges/bin/millau/runtime/src/rialto_messages.rs index 6641dc01b7..b2754dcbf7 100644 --- a/bridges/bin/millau/runtime/src/rialto_messages.rs +++ b/bridges/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, >; @@ -172,6 +173,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); @@ -182,6 +184,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/bridges/bin/rialto/runtime/src/lib.rs b/bridges/bin/rialto/runtime/src/lib.rs index dff373afb5..a2e9578c1d 100644 --- a/bridges/bin/rialto/runtime/src/lib.rs +++ b/bridges/bin/rialto/runtime/src/lib.rs @@ -862,6 +862,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, @@ -905,6 +906,7 @@ impl_runtime_apis! { weight: params.size as _, origin: dispatch_origin, call: message_payload, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, }; (message, pallet_bridge_messages::benchmarking::MESSAGE_FEE.into()) } @@ -921,7 +923,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 _], @@ -934,12 +936,19 @@ impl_runtime_apis! { let (rialto_raw_public, rialto_raw_signature) = ed25519_sign( &call, &millau_account_id, + VERSION.spec_version, + bp_runtime::MILLAU_CHAIN_ID, + bp_runtime::RIALTO_CHAIN_ID, ); 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.dispatch_fee_payment == DispatchFeePayment::AtTargetChain { + Self::endow_account(&rialto_public.clone().into_account()); + } + let make_millau_message_key = |message_key: MessageKey| storage_keys::message_key::< Runtime, ::MessagesInstance, @@ -960,6 +969,7 @@ impl_runtime_apis! { Default::default(), ); + let dispatch_fee_payment = params.dispatch_fee_payment.clone(); prepare_message_proof::( params, make_millau_message_key, @@ -978,6 +988,7 @@ impl_runtime_apis! { rialto_public, rialto_signature, ), + dispatch_fee_payment, call: call.encode(), }.encode(), ) @@ -1010,6 +1021,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!( @@ -1105,6 +1128,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/bridges/bin/rialto/runtime/src/millau_messages.rs b/bridges/bin/rialto/runtime/src/millau_messages.rs index 5c1779a7fe..4945d2a7df 100644 --- a/bridges/bin/rialto/runtime/src/millau_messages.rs +++ b/bridges/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, >; @@ -172,6 +173,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); @@ -182,6 +184,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) @@ -258,3 +265,87 @@ impl MessagesParameter for RialtoToMillauMessagesParameter { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{AccountId, Call, ExistentialDeposit, Runtime, SystemCall, SystemConfig, VERSION}; + use bp_message_dispatch::CallOrigin; + use bp_messages::{ + target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, + MessageKey, + }; + use bp_runtime::{derive_account_id, messages::DispatchFeePayment, SourceAccount}; + use bridge_runtime_common::messages::target::{FromBridgedChainEncodedMessageCall, FromBridgedChainMessagePayload}; + use frame_support::{ + traits::Currency, + weights::{GetDispatchInfo, WeightToFeePolynomial}, + }; + 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_CHAIN_ID; + 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, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, + 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/bridges/bin/runtime-common/Cargo.toml b/bridges/bin/runtime-common/Cargo.toml index 83803d06de..07fe8910c2 100644 --- a/bridges/bin/runtime-common/Cargo.toml +++ b/bridges/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/bridges/bin/runtime-common/README.md b/bridges/bin/runtime-common/README.md index a7322af973..38a47bfdcc 100644 --- a/bridges/bin/runtime-common/README.md +++ b/bridges/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/bridges/bin/runtime-common/src/messages.rs b/bridges/bin/runtime-common/src/messages.rs index 52623c42c6..5628977480 100644 --- a/bridges/bin/runtime-common/src/messages.rs +++ b/bridges/bin/runtime-common/src/messages.rs @@ -26,9 +26,16 @@ use bp_messages::{ target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages}, InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData, }; -use bp_runtime::{ChainId, Size, StorageProofChecker}; +use bp_runtime::{ + messages::{DispatchFeePayment, MessageDispatchResult}, + ChainId, 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}, @@ -124,6 +131,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>; @@ -326,8 +334,19 @@ 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 pay_dispatch_fee_at_target_chain = payload.dispatch_fee_payment == DispatchFeePayment::AtTargetChain; + let delivery_transaction = BridgedChain::::estimate_delivery_transaction( + &payload.call, + pay_dispatch_fee_at_target_chain, + if 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 @@ -445,8 +464,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>, ()> { @@ -457,20 +486,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; @@ -480,14 +517,26 @@ 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>>, + ) -> MessageDispatchResult { let message_id = (message.key.lane_id, message.key.nonce); pallet_bridge_dispatch::Pallet::::dispatch( B::BridgedChain::ID, B::ThisChain::ID, 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) + }, + ) } } @@ -854,6 +903,7 @@ mod tests { fn estimate_delivery_transaction( _message_payload: &[u8], + _include_pay_dispatch_fee_cost: bool, _message_dispatch_weight: WeightOf, ) -> MessageTransaction> { unreachable!() @@ -911,6 +961,7 @@ mod tests { fn estimate_delivery_transaction( _message_payload: &[u8], + _include_pay_dispatch_fee_cost: bool, message_dispatch_weight: WeightOf, ) -> MessageTransaction> { MessageTransaction { @@ -935,6 +986,7 @@ mod tests { spec_version: 1, weight: 100, origin: bp_message_dispatch::CallOrigin::SourceRoot, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, call: ThisChainCall::Transfer.encode(), } .encode(); @@ -949,10 +1001,10 @@ mod tests { spec_version: 1, weight: 100, origin: bp_message_dispatch::CallOrigin::SourceRoot, - call: target::FromBridgedChainEncodedMessageCall:: { - encoded_call: ThisChainCall::Transfer.encode(), - _marker: PhantomData::default(), - }, + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, + call: target::FromBridgedChainEncodedMessageCall::::new( + ThisChainCall::Transfer.encode(), + ), } ); assert_eq!(Ok(ThisChainCall::Transfer), message_on_this_chain.call.into()); @@ -966,6 +1018,7 @@ mod tests { spec_version: 1, weight: 100, origin: bp_message_dispatch::CallOrigin::SourceRoot, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![42], } } @@ -986,6 +1039,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.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, + ) + .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( @@ -1016,6 +1084,7 @@ mod tests { spec_version: 1, weight: 100, origin: bp_message_dispatch::CallOrigin::SourceRoot, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![42], }; @@ -1059,6 +1128,7 @@ mod tests { spec_version: 1, weight: 100, origin: bp_message_dispatch::CallOrigin::SourceAccount(ThisChainAccountId(1)), + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![42], }; @@ -1126,6 +1196,7 @@ mod tests { spec_version: 1, weight: 5, origin: bp_message_dispatch::CallOrigin::SourceRoot, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![1, 2, 3, 4, 5, 6], },) .is_err() @@ -1141,6 +1212,7 @@ mod tests { spec_version: 1, weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT + 1, origin: bp_message_dispatch::CallOrigin::SourceRoot, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![1, 2, 3, 4, 5, 6], },) .is_err() @@ -1156,6 +1228,7 @@ mod tests { spec_version: 1, weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT, origin: bp_message_dispatch::CallOrigin::SourceRoot, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![0; source::maximal_message_size::() as usize + 1], },) .is_err() @@ -1171,6 +1244,7 @@ mod tests { spec_version: 1, weight: BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT, origin: bp_message_dispatch::CallOrigin::SourceRoot, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: vec![0; source::maximal_message_size::() as _], },), Ok(()), diff --git a/bridges/bin/runtime-common/src/messages_benchmarking.rs b/bridges/bin/runtime-common/src/messages_benchmarking.rs index 639e5f6c50..3785f4a460 100644 --- a/bridges/bin/runtime-common/src/messages_benchmarking.rs +++ b/bridges/bin/runtime-common/src/messages_benchmarking.rs @@ -25,6 +25,7 @@ use crate::messages::{ }; use bp_messages::{LaneId, MessageData, MessageKey, MessagePayload}; +use bp_runtime::ChainId; use codec::Encode; use ed25519_dalek::{PublicKey, SecretKey, Signer, KEYPAIR_LENGTH, SECRET_KEY_LENGTH}; use frame_support::weights::Weight; @@ -37,7 +38,13 @@ 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, + source_chain_id: ChainId, + target_chain_id: ChainId, +) -> ([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 +58,13 @@ 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, + source_chain_id, + target_chain_id, + ); let target_origin_signature = target_pair .try_sign(&signature_message) .expect("Ed25519 try_sign should not fail in benchmarks"); diff --git a/bridges/modules/dispatch/README.md b/bridges/modules/dispatch/README.md index c4e703c402..068ff1167f 100644 --- a/bridges/modules/dispatch/README.md +++ b/bridges/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/bridges/modules/dispatch/src/lib.rs b/bridges/modules/dispatch/src/lib.rs index 42749e075e..eeb5d5c17d 100644 --- a/bridges/modules/dispatch/src/lib.rs +++ b/bridges/modules/dispatch/src/lib.rs @@ -27,7 +27,11 @@ #![allow(clippy::unused_unit)] use bp_message_dispatch::{CallOrigin, MessageDispatch, MessagePayload, SpecVersion, Weight}; -use bp_runtime::{derive_account_id, ChainId, SourceAccount}; +use bp_runtime::{ + derive_account_id, + messages::{DispatchFeePayment, MessageDispatchResult}, + ChainId, SourceAccount, +}; use codec::{Decode, Encode}; use frame_support::{ decl_event, decl_module, decl_storage, @@ -89,7 +93,8 @@ decl_storage! { decl_event!( pub enum Event where - >::MessageId + >::MessageId, + AccountId = ::AccountId, { /// Message has been rejected before reaching dispatch. MessageRejected(ChainId, MessageId), @@ -101,12 +106,14 @@ decl_event!( MessageWeightMismatch(ChainId, MessageId, Weight, Weight), /// Message signature mismatch. MessageSignatureMismatch(ChainId, MessageId), - /// Message has been dispatched with given result. - MessageDispatched(ChainId, MessageId, DispatchResult), /// We have failed to decode Call from the message. MessageCallDecodeFailed(ChainId, MessageId), /// The call from the message has been rejected by the call filter. MessageCallRejected(ChainId, MessageId), + /// The origin account has failed to pay fee for dispatching the message. + MessageDispatchPaymentFailed(ChainId, MessageId, AccountId, Weight), + /// Message has been dispatched with given result. + MessageDispatched(ChainId, MessageId, DispatchResult), /// Phantom member, never used. Needed to handle multiple pallet instances. _Dummy(PhantomData), } @@ -120,7 +127,7 @@ decl_module! { } } -impl, I: Instance> MessageDispatch for Pallet { +impl, I: Instance> MessageDispatch for Pallet { type Message = MessagePayload; @@ -128,7 +135,13 @@ impl, I: Instance> MessageDispatch for Pallet { message.weight } - fn dispatch(source_chain: ChainId, target_chain: ChainId, id: T::MessageId, message: Result) { + fn dispatch Result<(), ()>>( + source_chain: ChainId, + target_chain: ChainId, + id: T::MessageId, + message: Result, + pay_dispatch_fee: P, + ) -> MessageDispatchResult { // emit special even if message has been rejected by external component let message = match message { Ok(message) => message, @@ -140,12 +153,19 @@ impl, I: Instance> MessageDispatch for Pallet { id, ); Self::deposit_event(RawEvent::MessageRejected(source_chain, id)); - return; + 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!( @@ -161,7 +181,7 @@ impl, I: Instance> MessageDispatch for Pallet { expected_version, message.spec_version, )); - return; + return dispatch_result; } // now that we have spec version checked, let's decode the call @@ -175,7 +195,7 @@ impl, I: Instance> MessageDispatch for Pallet { id, ); Self::deposit_event(RawEvent::MessageCallDecodeFailed(source_chain, id)); - return; + return dispatch_result; } }; @@ -207,7 +227,7 @@ impl, I: Instance> MessageDispatch for Pallet { target_signature, ); Self::deposit_event(RawEvent::MessageSignatureMismatch(source_chain, id)); - return; + return dispatch_result; } log::trace!(target: "runtime::bridge-dispatch", "Target Account: {:?}", &target_account); @@ -231,7 +251,7 @@ impl, I: Instance> MessageDispatch for Pallet { call, ); Self::deposit_event(RawEvent::MessageCallRejected(source_chain, id)); - return; + return dispatch_result; } // verify weight @@ -254,21 +274,43 @@ impl, I: Instance> MessageDispatch for Pallet { expected_weight, message.weight, )); - return; + return dispatch_result; } + // pay dispatch fee right before dispatch + 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 {}", + source_chain, + id, + message.weight, + ); + Self::deposit_event(RawEvent::MessageDispatchPaymentFailed( + source_chain, + id, + origin_account, + message.weight, + )); + return dispatch_result; + } + dispatch_result.dispatch_fee_paid_during_dispatch = 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: {:.4096?}", &call); - let dispatch_result = call.dispatch(origin); - let actual_call_weight = extract_actual_weight(&dispatch_result, &dispatch_info); + 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: {:?}", source_chain, id, - actual_call_weight, + dispatch_result.unspent_weight, message.weight, dispatch_result, ); @@ -276,8 +318,10 @@ impl, I: Instance> MessageDispatch for Pallet { Self::deposit_event(RawEvent::MessageDispatched( source_chain, id, - dispatch_result.map(drop).map_err(|e| e.error), + result.map(drop).map_err(|e| e.error), )); + + dispatch_result } } @@ -485,31 +529,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, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, 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) } @@ -522,10 +567,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(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message)); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, weight); assert_eq!( System::events(), @@ -549,10 +596,11 @@ 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(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message)); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, 7); assert_eq!( System::events(), @@ -562,7 +610,7 @@ mod tests { SOURCE_CHAIN_ID, id, 1345000, - 0, + 7, )), topics: vec![], }], @@ -580,9 +628,11 @@ mod tests { call_origin, Call::System(>::remark(vec![1, 2, 3])), ); + let weight = message.weight; System::set_block_number(1); - Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message)); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, weight); assert_eq!( System::events(), @@ -604,7 +654,7 @@ mod tests { let id = [0; 4]; System::set_block_number(1); - Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Err(())); + Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Err(()), |_, _| unreachable!()); assert_eq!( System::events(), @@ -627,10 +677,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(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message)); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, weight); assert_eq!( System::events(), @@ -657,7 +709,8 @@ mod tests { message.weight = weight; System::set_block_number(1); - Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message)); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); + assert_eq!(result.unspent_weight, weight); assert_eq!( System::events(), @@ -673,6 +726,67 @@ mod tests { }); } + #[test] + fn should_emit_event_for_unpaid_calls() { + new_test_ext().execute_with(|| { + let id = [0; 4]; + + let mut message = + prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); + let weight = message.weight; + message.dispatch_fee_payment = DispatchFeePayment::AtTargetChain; + + System::set_block_number(1); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| Err(())); + assert_eq!(result.unspent_weight, weight); + + assert_eq!( + System::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: Event::call_dispatch(call_dispatch::Event::::MessageDispatchPaymentFailed( + SOURCE_CHAIN_ID, + id, + AccountIdConverter::convert(derive_account_id::( + SOURCE_CHAIN_ID, + SourceAccount::Root + )), + TEST_WEIGHT, + )), + topics: vec![], + }], + ); + }); + } + + #[test] + fn should_dispatch_calls_paid_at_target_chain() { + new_test_ext().execute_with(|| { + let id = [0; 4]; + + let mut message = + prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); + message.dispatch_fee_payment = DispatchFeePayment::AtTargetChain; + + System::set_block_number(1); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| Ok(())); + assert!(result.dispatch_fee_paid_during_dispatch); + + assert_eq!( + System::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: Event::call_dispatch(call_dispatch::Event::::MessageDispatched( + SOURCE_CHAIN_ID, + id, + Ok(()) + )), + topics: vec![], + }], + ); + }); + } + #[test] fn should_dispatch_bridge_message_from_root_origin() { new_test_ext().execute_with(|| { @@ -680,7 +794,8 @@ mod tests { let message = prepare_root_message(Call::System(>::remark(vec![1, 2, 3]))); System::set_block_number(1); - Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message)); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); + assert!(!result.dispatch_fee_paid_during_dispatch); assert_eq!( System::events(), @@ -706,7 +821,8 @@ mod tests { let message = prepare_target_message(call); System::set_block_number(1); - Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message)); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); + assert!(!result.dispatch_fee_paid_during_dispatch); assert_eq!( System::events(), @@ -732,7 +848,8 @@ mod tests { let message = prepare_source_message(call); System::set_block_number(1); - Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message)); + let result = Dispatch::dispatch(SOURCE_CHAIN_ID, TARGET_CHAIN_ID, id, Ok(message), |_, _| unreachable!()); + assert!(!result.dispatch_fee_paid_during_dispatch); assert_eq!( System::events(), diff --git a/bridges/modules/messages/Cargo.toml b/bridges/modules/messages/Cargo.toml index 4a75fa8181..983a74fe75 100644 --- a/bridges/modules/messages/Cargo.toml +++ b/bridges/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/bridges/modules/messages/README.md b/bridges/modules/messages/README.md index eda5e28a6c..47bcacdee7 100644 --- a/bridges/modules/messages/README.md +++ b/bridges/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 diff --git a/bridges/modules/messages/src/benchmarking.rs b/bridges/modules/messages/src/benchmarking.rs index d1ecf77500..5fc04bb2b2 100644 --- a/bridges/modules/messages/src/benchmarking.rs +++ b/bridges/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; @@ -67,6 +68,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 dispatch_fee_payment: DispatchFeePayment, } /// Benchmark-specific message delivery proof parameters. @@ -108,6 +111,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! { @@ -242,7 +247,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 paid at target (this) chain. // // This is base benchmark for all other message delivery benchmarks. receive_single_message_proof { @@ -257,6 +263,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -264,13 +271,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 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)`. @@ -288,6 +297,7 @@ benchmarks_instance! { message_nonces: 21..=22, outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 2, dispatch_weight) verify { @@ -295,13 +305,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 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)`. @@ -323,6 +335,7 @@ benchmarks_instance! { latest_generated_nonce: 21, }), size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -334,6 +347,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 +371,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasExtraNodes(1024), + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -364,6 +379,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 +405,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasExtraNodes(16 * 1024), + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof(RawOrigin::Signed(relayer_id_on_target), relayer_id_on_source, proof, 1, dispatch_weight) verify { @@ -396,6 +413,40 @@ 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: + // * 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 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 + // 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), + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, + }); + }: 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_delivery_proof` extrinsic with following conditions: @@ -569,6 +620,7 @@ benchmarks_instance! { message_nonces: 21..=(20 + i as MessageNonce), outbound_lane_data: None, size: ProofSize::Minimal(EXPECTED_DEFAULT_MESSAGE_LENGTH), + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), @@ -606,6 +658,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasExtraNodes(i as _), + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), @@ -643,6 +696,7 @@ benchmarks_instance! { message_nonces: 21..=21, outbound_lane_data: None, size: ProofSize::HasLargeLeaf(i as _), + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), @@ -686,6 +740,7 @@ benchmarks_instance! { latest_generated_nonce: 21, }), size: ProofSize::Minimal(0), + dispatch_fee_payment: DispatchFeePayment::AtTargetChain, }); }: receive_messages_proof( RawOrigin::Signed(relayer_id_on_target), diff --git a/bridges/modules/messages/src/inbound_lane.rs b/bridges/modules/messages/src/inbound_lane.rs index f0655f8c95..92eb8d665d 100644 --- a/bridges/modules/messages/src/inbound_lane.rs +++ b/bridges/modules/messages/src/inbound_lane.rs @@ -20,6 +20,8 @@ use bp_messages::{ target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch}, InboundLaneData, LaneId, MessageKey, MessageNonce, OutboundLaneData, }; +use bp_runtime::messages::MessageDispatchResult; +use frame_support::RuntimeDebug; use sp_std::prelude::PartialEq; /// Inbound lane storage. @@ -27,7 +29,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; @@ -41,6 +43,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 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. + TooManyUnrewardedRelayers, + /// There are too many unconfirmed messages at the lane. + TooManyUnconfirmedMessages, +} + /// Inbound messages lane. pub struct InboundLane { storage: S, @@ -90,51 +108,54 @@ impl InboundLane { } /// Receive new message. - pub fn receive_message>( + pub fn receive_message, AccountId>( &mut self, - relayer: S::Relayer, + relayer_at_bridged_chain: &S::Relayer, + 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() { - Some((_, nonce_high, last_relayer)) if last_relayer == &relayer => { + Some((_, nonce_high, last_relayer)) if last_relayer == relayer_at_bridged_chain => { *nonce_high = nonce; false } _ => true, }; if push_new { - data.relayers.push_back((nonce, nonce, relayer)); + data.relayers + .push_back((nonce, nonce, (*relayer_at_bridged_chain).clone())); } self.storage.set_data(data); - P::dispatch(DispatchMessage { - key: MessageKey { - lane_id: self.storage.id(), - nonce, + ReceivalResult::Dispatched(P::dispatch( + relayer_at_this_chain, + DispatchMessage { + key: MessageKey { + lane_id: self.storage.id(), + nonce, + }, + data: message_data, }, - data: message_data, - }); - - true + )) } } @@ -144,8 +165,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, }; @@ -154,11 +175,15 @@ mod tests { lane: &mut InboundLane>, nonce: MessageNonce, ) { - assert!(lane.receive_message::( - 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(dispatch_result(0)) + ); } #[test] @@ -269,11 +294,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, - 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); }); } @@ -284,24 +313,36 @@ 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, - 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(dispatch_result(0)) + ); } // Fails to dispatch new message from different than latest relayer. - assert!(!lane.receive_message::( - TEST_RELAYER_A + max_nonce + 1, - max_nonce + 1, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + 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!(!lane.receive_message::( - TEST_RELAYER_A + max_nonce, - max_nonce + 1, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + lane.receive_message::( + &(TEST_RELAYER_A + max_nonce), + &(TEST_RELAYER_A + max_nonce), + max_nonce + 1, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::TooManyUnrewardedRelayers, + ); }); } @@ -311,24 +352,36 @@ 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, - 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(dispatch_result(0)) + ); } // Fails to dispatch new message from different than latest relayer. - assert!(!lane.receive_message::( - TEST_RELAYER_B, - max_nonce + 1, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + 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!(!lane.receive_message::( - TEST_RELAYER_A, - max_nonce + 1, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + max_nonce + 1, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::TooManyUnconfirmedMessages, + ); }); } @@ -336,21 +389,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, - 1, - message_data(REGULAR_PAYLOAD).into() - )); - assert!(lane.receive_message::( - TEST_RELAYER_B, - 2, - message_data(REGULAR_PAYLOAD).into() - )); - assert!(lane.receive_message::( - 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(dispatch_result(0)) + ); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_B, + &TEST_RELAYER_B, + 2, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(dispatch_result(0)) + ); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + 3, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(dispatch_result(0)) + ); assert_eq!( lane.storage.data().relayers, vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B), (3, 3, TEST_RELAYER_A)] @@ -362,16 +427,24 @@ 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, - 1, - message_data(REGULAR_PAYLOAD).into() - )); - assert!(!lane.receive_message::( - TEST_RELAYER_B, - 1, - message_data(REGULAR_PAYLOAD).into() - )); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + 1, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::Dispatched(dispatch_result(0)) + ); + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_B, + &TEST_RELAYER_B, + 1, + message_data(REGULAR_PAYLOAD).into() + ), + ReceivalResult::InvalidNonce, + ); }); } @@ -383,4 +456,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.dispatch_result.unspent_weight = 1; + assert_eq!( + lane.receive_message::( + &TEST_RELAYER_A, + &TEST_RELAYER_A, + 1, + message_data(payload).into() + ), + ReceivalResult::Dispatched(dispatch_result(1)) + ); + }); + } } diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index e8a004b19a..5b0c609d85 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -42,7 +42,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; @@ -55,9 +55,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}; @@ -150,7 +152,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. @@ -438,13 +444,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 { + ) -> DispatchResultWithPostInfo { ensure_not_halted::()?; - let _ = ensure_signed(origin)?; + let relayer_id_at_this_chain = ensure_signed(origin)?; // reject transactions that are declaring too many messages ensure!( @@ -452,6 +458,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,20 +534,57 @@ 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) { - valid_messages += 1; - } + 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, + ); + + // 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, refund_pay_dispatch_fee) = match receival_result { + ReceivalResult::Dispatched(dispatch_result) => { + valid_messages += 1; + (dispatch_result.unspent_weight, !dispatch_result.dispatch_fee_paid_during_dispatch) + }, + ReceivalResult::InvalidNonce + | ReceivalResult::TooManyUnrewardedRelayers + | ReceivalResult::TooManyUnconfirmedMessages => (dispatch_weight, true), + }; + 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 + } + ); } } 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. @@ -860,10 +920,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}; @@ -883,7 +942,7 @@ mod tests { Origin::signed(1), TEST_LANE_ID, REGULAR_PAYLOAD, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, )); // check event with assigned nonce @@ -897,7 +956,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() { @@ -1113,7 +1175,7 @@ mod tests { Origin::signed(1), TEST_LANE_ID, REGULAR_PAYLOAD, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, ), Error::::Halted, ); @@ -1129,7 +1191,7 @@ mod tests { TEST_RELAYER_A, Ok(vec![message(2, REGULAR_PAYLOAD)]).into(), 1, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, ), Error::::Halted, ); @@ -1164,7 +1226,7 @@ mod tests { Origin::signed(1), TEST_LANE_ID, REGULAR_PAYLOAD, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, ), Error::::Halted, ); @@ -1181,7 +1243,7 @@ mod tests { TEST_RELAYER_A, Ok(vec![message(1, REGULAR_PAYLOAD)]).into(), 1, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, ),); assert_ok!(Pallet::::receive_messages_delivery_proof( @@ -1214,7 +1276,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, ); @@ -1241,7 +1303,7 @@ mod tests { Origin::signed(1), TEST_LANE_ID, REGULAR_PAYLOAD, - REGULAR_PAYLOAD.1 + REGULAR_PAYLOAD.declared_weight ), Error::::FailedToWithdrawMessageFee, ); @@ -1256,7 +1318,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); @@ -1297,7 +1359,7 @@ mod tests { TEST_RELAYER_A, message_proof, 1, - REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight, )); assert_eq!( @@ -1329,7 +1391,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, ); @@ -1551,7 +1613,7 @@ mod tests { ]) .into(), 3, - REGULAR_PAYLOAD.1 + REGULAR_PAYLOAD.1, + REGULAR_PAYLOAD.declared_weight + REGULAR_PAYLOAD.declared_weight, ),); assert_eq!( @@ -1603,9 +1665,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( @@ -1672,4 +1734,61 @@ 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, + is_prepaid: bool, + ) -> (Weight, Weight) { + let mut payload = REGULAR_PAYLOAD; + 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( + &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, 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, 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, false); + assert_eq!(post, pre - REGULAR_PAYLOAD.declared_weight); + + // when there's no unspent weight + 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/bridges/modules/messages/src/mock.rs b/bridges/modules/messages/src/mock.rs index 460c001842..823ec13dd7 100644 --- a/bridges/modules/messages/src/mock.rs +++ b/bridges/modules/messages/src/mock.rs @@ -27,7 +27,7 @@ use bp_messages::{ 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; @@ -41,7 +41,17 @@ 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, + /// 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 dispatch_result: MessageDispatchResult, +} pub type TestMessageFee = u64; pub type TestRelayer = u64; @@ -189,10 +199,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>)>; @@ -359,17 +369,25 @@ 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 { match message.data.payload.as_ref() { - Ok(payload) => payload.1, + Ok(payload) => payload.declared_weight, Err(_) => 0, } } - fn dispatch(_message: DispatchMessage) {} + fn dispatch( + _relayer_account: &AccountId, + message: DispatchMessage, + ) -> MessageDispatchResult { + match message.data.payload.as_ref() { + Ok(payload) => payload.dispatch_result.clone(), + Err(_) => dispatch_result(0), + } + } } /// Return test lane message with given nonce and payload. @@ -383,6 +401,15 @@ pub fn message(nonce: MessageNonce, payload: TestPayload) -> Message TestPayload { + TestPayload { + id, + declared_weight, + dispatch_result: dispatch_result(0), + } +} + /// Return message data with valid fee for given payload. pub fn message_data(payload: TestPayload) -> MessageData { MessageData { @@ -391,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/bridges/modules/messages/src/weights.rs b/bridges/modules/messages/src/weights.rs index aea0e44239..ffe89c58bd 100644 --- a/bridges/modules/messages/src/weights.rs +++ b/bridges/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-06-03, STEPS: [50, ], REPEAT: 20 +//! DATE: 2021-06-15, STEPS: [50, ], REPEAT: 20 //! LOW RANGE: [], HIGH RANGE: [] //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled //! CHAIN: Some("dev"), DB CACHE: 128 @@ -57,6 +57,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 +74,110 @@ pub trait WeightInfo { pub struct RialtoWeight(PhantomData); impl WeightInfo for RialtoWeight { fn send_minimal_message_worst_case() -> Weight { - (140_457_000 as Weight) + (154_371_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 { - (138_097_000 as Weight) + (157_479_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 { - (196_192_000 as Weight) + (186_840_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_244_063_000 as Weight) - .saturating_add(T::DbWeight::get().reads(4 as Weight)) + (4_377_567_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() -> Weight { - (135_633_000 as Weight) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (205_350_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 { - (229_415_000 as Weight) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (337_102_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 { - (147_408_000 as Weight) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (218_825_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 { - (160_092_000 as Weight) - .saturating_add(T::DbWeight::get().reads(3 as Weight)) - .saturating_add(T::DbWeight::get().writes(1 as Weight)) + (230_759_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 { - (452_140_000 as Weight) + (490_522_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 { + (136_550_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 { - (123_704_000 as Weight) + (131_397_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 { - (127_844_000 as Weight) + (137_946_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 { - (183_119_000 as Weight) + (194_448_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 { - (162_249_000 as Weight) - .saturating_add((3_000 as Weight).saturating_mul(i as Weight)) + (142_576_000 as Weight) + .saturating_add((2_000 as Weight).saturating_mul(i as Weight)) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(12 as Weight)) } fn receive_multiple_messages_proof(i: u32) -> Weight { (0 as Weight) - .saturating_add((107_235_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((138_341_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 { - (450_232_000 as Weight) + (472_752_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)) + .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 { - (181_851_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)) + (175_300_000 as Weight) + .saturating_add((6_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 { (0 as Weight) - .saturating_add((114_622_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((142_176_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 { - (103_133_000 as Weight) - .saturating_add((6_676_000 as Weight).saturating_mul(i as Weight)) + (127_372_000 as Weight) + .saturating_add((7_927_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 { - (100_321_000 as Weight) - .saturating_add((59_736_000 as Weight).saturating_mul(i as Weight)) + (99_781_000 as Weight) + .saturating_add((64_001_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 +188,110 @@ impl WeightInfo for RialtoWeight { // For backwards compatibility and tests impl WeightInfo for () { fn send_minimal_message_worst_case() -> Weight { - (140_457_000 as Weight) + (154_371_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 { - (138_097_000 as Weight) + (157_479_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 { - (196_192_000 as Weight) + (186_840_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_244_063_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(4 as Weight)) + (4_377_567_000 as Weight) + .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn receive_single_message_proof() -> Weight { - (135_633_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (205_350_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 { - (229_415_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (337_102_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 { - (147_408_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (218_825_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 { - (160_092_000 as Weight) - .saturating_add(RocksDbWeight::get().reads(3 as Weight)) - .saturating_add(RocksDbWeight::get().writes(1 as Weight)) + (230_759_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 { - (452_140_000 as Weight) + (490_522_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 { + (136_550_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 { - (123_704_000 as Weight) + (131_397_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 { - (127_844_000 as Weight) + (137_946_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 { - (183_119_000 as Weight) + (194_448_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 { - (162_249_000 as Weight) - .saturating_add((3_000 as Weight).saturating_mul(i as Weight)) + (142_576_000 as Weight) + .saturating_add((2_000 as Weight).saturating_mul(i as Weight)) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(12 as Weight)) } fn receive_multiple_messages_proof(i: u32) -> Weight { (0 as Weight) - .saturating_add((107_235_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((138_341_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 { - (450_232_000 as Weight) + (472_752_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)) + .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 { - (181_851_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)) + (175_300_000 as Weight) + .saturating_add((6_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 { (0 as Weight) - .saturating_add((114_622_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((142_176_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 { - (103_133_000 as Weight) - .saturating_add((6_676_000 as Weight).saturating_mul(i as Weight)) + (127_372_000 as Weight) + .saturating_add((7_927_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 { - (100_321_000 as Weight) - .saturating_add((59_736_000 as Weight).saturating_mul(i as Weight)) + (99_781_000 as Weight) + .saturating_add((64_001_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/bridges/modules/messages/src/weights_ext.rs b/bridges/modules/messages/src/weights_ext.rs index cb754a1023..be440174b4 100644 --- a/bridges/modules/messages/src/weights_ext.rs +++ b/bridges/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/bridges/primitives/chain-millau/src/lib.rs b/bridges/primitives/chain-millau/src/lib.rs index 36c179ca5f..0efc54e96e 100644 --- a/bridges/primitives/chain-millau/src/lib.rs +++ b/bridges/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/bridges/primitives/chain-rialto/src/lib.rs b/bridges/primitives/chain-rialto/src/lib.rs index cc7c8e8c08..8139372959 100644 --- a/bridges/primitives/chain-rialto/src/lib.rs +++ b/bridges/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/bridges/primitives/message-dispatch/src/lib.rs b/bridges/primitives/message-dispatch/src/lib.rs index a7b07abc98..7a677dba42 100644 --- a/bridges/primitives/message-dispatch/src/lib.rs +++ b/bridges/primitives/message-dispatch/src/lib.rs @@ -19,7 +19,10 @@ #![cfg_attr(not(feature = "std"), no_std)] #![warn(missing_docs)] -use bp_runtime::{ChainId, Size}; +use bp_runtime::{ + messages::{DispatchFeePayment, MessageDispatchResult}, + ChainId, Size, +}; use codec::{Decode, Encode}; use frame_support::RuntimeDebug; use sp_std::prelude::*; @@ -31,7 +34,7 @@ pub type Weight = u64; pub type SpecVersion = u32; /// 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; @@ -52,7 +55,15 @@ 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(source_chain: ChainId, target_chain: ChainId, id: MessageId, message: Result); + /// + /// Returns unspent dispatch weight. + fn dispatch Result<(), ()>>( + source_chain: ChainId, + target_chain: ChainId, + id: MessageId, + message: Result, + pay_dispatch_fee: P, + ) -> MessageDispatchResult; } /// Origin of a Call when it is dispatched on the target chain. @@ -110,6 +121,8 @@ pub struct MessagePayload, + /// Where message dispatch fee is paid? + pub dispatch_fee_payment: DispatchFeePayment, /// The call itself. pub call: Call, } diff --git a/bridges/primitives/messages/src/target_chain.rs b/bridges/primitives/messages/src/target_chain.rs index 676e919bc6..2a649f54d7 100644 --- a/bridges/primitives/messages/src/target_chain.rs +++ b/bridges/primitives/messages/src/target_chain.rs @@ -18,7 +18,7 @@ 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::*}; @@ -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,13 @@ 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`. + fn dispatch( + relayer_account: &AccountId, + message: DispatchMessage, + ) -> MessageDispatchResult; } impl Default for ProvedLaneMessages { @@ -149,12 +155,17 @@ 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(_: &AccountId, _: DispatchMessage) -> MessageDispatchResult { + MessageDispatchResult { + unspent_weight: 0, + dispatch_fee_paid_during_dispatch: false, + } + } } diff --git a/bridges/primitives/runtime/src/lib.rs b/bridges/primitives/runtime/src/lib.rs index 80f644a588..7b17a2947f 100644 --- a/bridges/primitives/runtime/src/lib.rs +++ b/bridges/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/bridges/primitives/runtime/src/messages.rs b/bridges/primitives/runtime/src/messages.rs new file mode 100644 index 0000000000..e97430c5b0 --- /dev/null +++ b/bridges/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/bridges/relays/bin-substrate/src/cli/encode_message.rs b/bridges/relays/bin-substrate/src/cli/encode_message.rs index a29aa8597d..b5a38ec3de 100644 --- a/bridges/relays/bin-substrate/src/cli/encode_message.rs +++ b/bridges/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/bridges/relays/bin-substrate/src/cli/send_message.rs b/bridges/relays/bin-substrate/src/cli/send_message.rs index f6338dea65..7ff96912d4 100644 --- a/bridges/relays/bin-substrate/src/cli/send_message.rs +++ b/bridges/relays/bin-substrate/src/cli/send_message.rs @@ -22,6 +22,7 @@ use crate::cli::{ TargetSigningParams, }; use bp_message_dispatch::{CallOrigin, MessagePayload}; +use bp_runtime::messages::DispatchFeePayment; use codec::Encode; use frame_support::{dispatch::GetDispatchInfo, weights::Weight}; use relay_substrate_client::{Chain, TransactionSignScheme}; @@ -211,6 +212,7 @@ where spec_version, weight, origin, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: HexBytes::encode(call), }; @@ -222,12 +224,14 @@ where spec_version, weight, origin, + dispatch_fee_payment, call, } = payload; MessagePayload { spec_version, weight, origin, + dispatch_fee_payment, call: call.0, } } @@ -268,6 +272,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()), + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: hex!("0401081234").to_vec(), } ); @@ -311,6 +316,7 @@ mod tests { sp_keyring::AccountKeyring::Bob.into(), signature, ), + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, call: hex!("0701081234").to_vec(), } ); diff --git a/bridges/relays/bin-substrate/src/messages_lane.rs b/bridges/relays/bin-substrate/src/messages_lane.rs index 7468c16706..5bb64ed077 100644 --- a/bridges/relays/bin-substrate/src/messages_lane.rs +++ b/bridges/relays/bin-substrate/src/messages_lane.rs @@ -204,7 +204,7 @@ mod tests { // reserved for messages dispatch allows dispatch of non-trivial messages. // // Any significant change in this values should attract additional attention. - (1024, 216_583_333_334), + (814, 216_583_333_334), ); } }