From 324e083cba13ea97618650702ce881a4ba2b1157 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 8 Mar 2021 21:07:20 +0300 Subject: [PATCH] Changed delivery and dispatch fee computation methods (#795) * removed weight <-> fee mess * updated documentation Co-authored-by: Hernando Castano --- bridges/bin/millau/runtime/src/lib.rs | 10 - .../bin/millau/runtime/src/rialto_messages.rs | 131 +++++---- bridges/bin/rialto/runtime/src/lib.rs | 10 - .../bin/rialto/runtime/src/millau_messages.rs | 131 +++++---- bridges/bin/runtime-common/README.md | 129 +++++---- bridges/bin/runtime-common/src/messages.rs | 257 ++++++++++-------- .../modules/message-lane/src/weights_ext.rs | 20 +- bridges/primitives/millau/src/lib.rs | 5 + bridges/primitives/rialto/src/lib.rs | 5 + bridges/relays/ethereum/src/rialto_client.rs | 4 +- bridges/relays/millau-client/src/lib.rs | 8 +- bridges/relays/rialto-client/src/lib.rs | 8 +- bridges/relays/substrate-client/src/chain.rs | 4 +- bridges/relays/substrate/Cargo.toml | 3 + bridges/relays/substrate/src/main.rs | 44 ++- .../substrate/src/millau_headers_to_rialto.rs | 5 +- .../src/millau_messages_to_rialto.rs | 6 +- .../substrate/src/rialto_headers_to_millau.rs | 5 +- .../src/rialto_messages_to_millau.rs | 6 +- 19 files changed, 437 insertions(+), 354 deletions(-) diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index 7c5d93ca2e..0add4f3235 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -667,11 +667,6 @@ mod tests { bp_millau::max_extrinsic_size(), bp_millau::max_extrinsic_weight(), max_incoming_message_proof_size, - bridge_runtime_common::messages::transaction_weight_without_multiplier( - bp_millau::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, - max_incoming_message_proof_size as _, - 0, - ), messages::target::maximal_incoming_message_dispatch_weight(bp_millau::max_extrinsic_weight()), ); @@ -686,11 +681,6 @@ mod tests { max_incoming_inbound_lane_data_proof_size, bp_rialto::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE, bp_rialto::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE, - bridge_runtime_common::messages::transaction_weight_without_multiplier( - bp_millau::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, - max_incoming_inbound_lane_data_proof_size as _, - 0, - ), ); } } diff --git a/bridges/bin/millau/runtime/src/rialto_messages.rs b/bridges/bin/millau/runtime/src/rialto_messages.rs index 9775c93d2d..dfe08b6e3d 100644 --- a/bridges/bin/millau/runtime/src/rialto_messages.rs +++ b/bridges/bin/millau/runtime/src/rialto_messages.rs @@ -24,11 +24,11 @@ use bp_message_lane::{ InboundLaneData, LaneId, Message, MessageNonce, Parameter as MessageLaneParameter, }; use bp_runtime::{InstanceId, RIALTO_BRIDGE_INSTANCE}; -use bridge_runtime_common::messages::{self, ChainWithMessageLanes, MessageBridge}; +use bridge_runtime_common::messages::{self, ChainWithMessageLanes, MessageBridge, MessageLaneTransaction}; use codec::{Decode, Encode}; use frame_support::{ parameter_types, - weights::{DispatchClass, Weight, WeightToFeePolynomial}, + weights::{DispatchClass, Weight}, RuntimeDebug, }; use sp_core::storage::StorageKey; @@ -99,59 +99,6 @@ impl MessageBridge for WithRialtoMessageBridge { type ThisChain = Millau; type BridgedChain = Rialto; - fn maximal_extrinsic_size_on_target_chain() -> u32 { - bp_rialto::max_extrinsic_size() - } - - fn weight_limits_of_message_on_bridged_chain(_message_payload: &[u8]) -> RangeInclusive { - // we don't want to relay too large messages + keep reserve for future upgrades - let upper_limit = messages::target::maximal_incoming_message_dispatch_weight(bp_rialto::max_extrinsic_weight()); - - // we're charging for payload bytes in `WithRialtoMessageBridge::weight_of_delivery_transaction` function - // - // this bridge may be used to deliver all kind of messages, so we're not making any assumptions about - // minimal dispatch weight here - - 0..=upper_limit - } - - fn weight_of_delivery_transaction(message_payload: &[u8]) -> Weight { - let message_payload_len = u32::try_from(message_payload.len()) - .map(Into::into) - .unwrap_or(Weight::MAX); - let extra_bytes_in_payload = - message_payload_len.saturating_sub(pallet_message_lane::EXPECTED_DEFAULT_MESSAGE_LENGTH.into()); - messages::transaction_weight_without_multiplier( - bp_rialto::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, - message_payload_len.saturating_add(bp_millau::EXTRA_STORAGE_PROOF_SIZE as _), - extra_bytes_in_payload - .saturating_mul(bp_rialto::ADDITIONAL_MESSAGE_BYTE_DELIVERY_WEIGHT) - .saturating_add(bp_rialto::DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT), - ) - } - - fn weight_of_delivery_confirmation_transaction_on_this_chain() -> Weight { - let inbounded_data_size: Weight = - InboundLaneData::::encoded_size_hint(bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, 1) - .map(Into::into) - .unwrap_or(Weight::MAX); - - messages::transaction_weight_without_multiplier( - bp_millau::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, - inbounded_data_size.saturating_add(bp_rialto::EXTRA_STORAGE_PROOF_SIZE as _), - bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, - ) - } - - fn this_weight_to_this_balance(weight: Weight) -> bp_millau::Balance { - ::WeightToFee::calc(&weight) - } - - fn bridged_weight_to_bridged_balance(weight: Weight) -> bp_rialto::Balance { - // we're using the same weights in both chains now - ::WeightToFee::calc(&weight) as _ - } - fn bridged_balance_to_this_balance(bridged_balance: bp_rialto::Balance) -> bp_millau::Balance { bp_millau::Balance::try_from(RialtoToMillauConversionRate::get().saturating_mul_int(bridged_balance)) .unwrap_or(bp_millau::Balance::MAX) @@ -167,7 +114,6 @@ impl messages::ChainWithMessageLanes for Millau { type AccountId = bp_millau::AccountId; type Signer = bp_millau::AccountSigner; type Signature = bp_millau::Signature; - type Call = crate::Call; type Weight = Weight; type Balance = bp_millau::Balance; @@ -175,6 +121,8 @@ impl messages::ChainWithMessageLanes for Millau { } impl messages::ThisChainWithMessageLanes for Millau { + type Call = crate::Call; + fn is_outbound_lane_enabled(lane: &LaneId) -> bool { *lane == LaneId::default() } @@ -182,6 +130,29 @@ impl messages::ThisChainWithMessageLanes for Millau { fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { MessageNonce::MAX } + + fn estimate_delivery_confirmation_transaction() -> MessageLaneTransaction { + let inbound_data_size = + InboundLaneData::::encoded_size_hint(bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, 1) + .unwrap_or(u32::MAX); + + MessageLaneTransaction { + dispatch_weight: bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, + size: inbound_data_size + .saturating_add(bp_rialto::EXTRA_STORAGE_PROOF_SIZE) + .saturating_add(bp_millau::TX_EXTRA_BYTES), + } + } + + fn transaction_payment(transaction: MessageLaneTransaction) -> bp_millau::Balance { + // in our testnets, both per-byte fee and weight-to-fee are 1:1 + messages::transaction_payment_without_multiplier( + bp_millau::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, + 1, + |weight| weight as _, + transaction, + ) + } } /// Rialto chain from message lane point of view. @@ -193,13 +164,59 @@ impl messages::ChainWithMessageLanes for Rialto { type AccountId = bp_rialto::AccountId; type Signer = bp_rialto::AccountSigner; type Signature = bp_rialto::Signature; - type Call = (); // unknown to us type Weight = Weight; type Balance = bp_rialto::Balance; type MessageLaneInstance = pallet_message_lane::DefaultInstance; } +impl messages::BridgedChainWithMessageLanes for Rialto { + fn maximal_extrinsic_size() -> u32 { + bp_rialto::max_extrinsic_size() + } + + fn message_weight_limits(_message_payload: &[u8]) -> RangeInclusive { + // we don't want to relay too large messages + keep reserve for future upgrades + let upper_limit = messages::target::maximal_incoming_message_dispatch_weight(bp_rialto::max_extrinsic_weight()); + + // we're charging for payload bytes in `WithRialtoMessageBridge::transaction_payment` function + // + // this bridge may be used to deliver all kind of messages, so we're not making any assumptions about + // minimal dispatch weight here + + 0..=upper_limit + } + + fn estimate_delivery_transaction( + message_payload: &[u8], + message_dispatch_weight: Weight, + ) -> MessageLaneTransaction { + let message_payload_len = u32::try_from(message_payload.len()).unwrap_or(u32::MAX); + let extra_bytes_in_payload = Weight::from(message_payload_len) + .saturating_sub(pallet_message_lane::EXPECTED_DEFAULT_MESSAGE_LENGTH.into()); + + MessageLaneTransaction { + 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_add(message_dispatch_weight), + size: message_payload_len + .saturating_add(bp_millau::EXTRA_STORAGE_PROOF_SIZE) + .saturating_add(bp_rialto::TX_EXTRA_BYTES), + } + } + + fn transaction_payment(transaction: MessageLaneTransaction) -> bp_rialto::Balance { + // in our testnets, both per-byte fee and weight-to-fee are 1:1 + messages::transaction_payment_without_multiplier( + bp_rialto::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, + 1, + |weight| weight as _, + transaction, + ) + } +} + impl TargetHeaderChain for Rialto { type Error = &'static str; // The proof is: diff --git a/bridges/bin/rialto/runtime/src/lib.rs b/bridges/bin/rialto/runtime/src/lib.rs index eaac1f5a61..18c88a1886 100644 --- a/bridges/bin/rialto/runtime/src/lib.rs +++ b/bridges/bin/rialto/runtime/src/lib.rs @@ -1096,11 +1096,6 @@ mod tests { bp_rialto::max_extrinsic_size(), bp_rialto::max_extrinsic_weight(), max_incoming_message_proof_size, - bridge_runtime_common::messages::transaction_weight_without_multiplier( - bp_rialto::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, - max_incoming_message_proof_size as _, - 0, - ), messages::target::maximal_incoming_message_dispatch_weight(bp_rialto::max_extrinsic_weight()), ); @@ -1115,11 +1110,6 @@ mod tests { max_incoming_inbound_lane_data_proof_size, bp_millau::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE, bp_millau::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE, - bridge_runtime_common::messages::transaction_weight_without_multiplier( - bp_rialto::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, - max_incoming_inbound_lane_data_proof_size as _, - 0, - ), ); } diff --git a/bridges/bin/rialto/runtime/src/millau_messages.rs b/bridges/bin/rialto/runtime/src/millau_messages.rs index 9fb57ee861..f2ccd71204 100644 --- a/bridges/bin/rialto/runtime/src/millau_messages.rs +++ b/bridges/bin/rialto/runtime/src/millau_messages.rs @@ -24,11 +24,11 @@ use bp_message_lane::{ InboundLaneData, LaneId, Message, MessageNonce, Parameter as MessageLaneParameter, }; use bp_runtime::{InstanceId, MILLAU_BRIDGE_INSTANCE}; -use bridge_runtime_common::messages::{self, ChainWithMessageLanes, MessageBridge}; +use bridge_runtime_common::messages::{self, ChainWithMessageLanes, MessageBridge, MessageLaneTransaction}; use codec::{Decode, Encode}; use frame_support::{ parameter_types, - weights::{DispatchClass, Weight, WeightToFeePolynomial}, + weights::{DispatchClass, Weight}, RuntimeDebug, }; use sp_core::storage::StorageKey; @@ -99,59 +99,6 @@ impl MessageBridge for WithMillauMessageBridge { type ThisChain = Rialto; type BridgedChain = Millau; - fn maximal_extrinsic_size_on_target_chain() -> u32 { - bp_millau::max_extrinsic_size() - } - - fn weight_limits_of_message_on_bridged_chain(_message_payload: &[u8]) -> RangeInclusive { - // we don't want to relay too large messages + keep reserve for future upgrades - let upper_limit = messages::target::maximal_incoming_message_dispatch_weight(bp_millau::max_extrinsic_weight()); - - // we're charging for payload bytes in `WithMillauMessageBridge::weight_of_delivery_transaction` function - // - // this bridge may be used to deliver all kind of messages, so we're not making any assumptions about - // minimal dispatch weight here - - 0..=upper_limit - } - - fn weight_of_delivery_transaction(message_payload: &[u8]) -> Weight { - let message_payload_len = u32::try_from(message_payload.len()) - .map(Into::into) - .unwrap_or(Weight::MAX); - let extra_bytes_in_payload = - message_payload_len.saturating_sub(pallet_message_lane::EXPECTED_DEFAULT_MESSAGE_LENGTH.into()); - messages::transaction_weight_without_multiplier( - bp_millau::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, - message_payload_len.saturating_add(bp_rialto::EXTRA_STORAGE_PROOF_SIZE as _), - extra_bytes_in_payload - .saturating_mul(bp_millau::ADDITIONAL_MESSAGE_BYTE_DELIVERY_WEIGHT) - .saturating_add(bp_millau::DEFAULT_MESSAGE_DELIVERY_TX_WEIGHT), - ) - } - - fn weight_of_delivery_confirmation_transaction_on_this_chain() -> Weight { - let inbounded_data_size: Weight = - InboundLaneData::::encoded_size_hint(bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, 1) - .map(Into::into) - .unwrap_or(Weight::MAX); - - messages::transaction_weight_without_multiplier( - bp_millau::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, - inbounded_data_size.saturating_add(bp_millau::EXTRA_STORAGE_PROOF_SIZE as _), - bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, - ) - } - - fn this_weight_to_this_balance(weight: Weight) -> bp_rialto::Balance { - ::WeightToFee::calc(&weight) - } - - fn bridged_weight_to_bridged_balance(weight: Weight) -> bp_millau::Balance { - // we're using the same weights in both chains now - ::WeightToFee::calc(&weight) as _ - } - fn bridged_balance_to_this_balance(bridged_balance: bp_millau::Balance) -> bp_rialto::Balance { bp_rialto::Balance::try_from(MillauToRialtoConversionRate::get().saturating_mul_int(bridged_balance)) .unwrap_or(bp_rialto::Balance::MAX) @@ -167,7 +114,6 @@ impl messages::ChainWithMessageLanes for Rialto { type AccountId = bp_rialto::AccountId; type Signer = bp_rialto::AccountSigner; type Signature = bp_rialto::Signature; - type Call = crate::Call; type Weight = Weight; type Balance = bp_rialto::Balance; @@ -175,6 +121,8 @@ impl messages::ChainWithMessageLanes for Rialto { } impl messages::ThisChainWithMessageLanes for Rialto { + type Call = crate::Call; + fn is_outbound_lane_enabled(lane: &LaneId) -> bool { *lane == LaneId::default() } @@ -182,6 +130,29 @@ impl messages::ThisChainWithMessageLanes for Rialto { fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { MessageNonce::MAX } + + fn estimate_delivery_confirmation_transaction() -> MessageLaneTransaction { + let inbound_data_size = + InboundLaneData::::encoded_size_hint(bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE, 1) + .unwrap_or(u32::MAX); + + MessageLaneTransaction { + dispatch_weight: bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, + size: inbound_data_size + .saturating_add(bp_millau::EXTRA_STORAGE_PROOF_SIZE) + .saturating_add(bp_rialto::TX_EXTRA_BYTES), + } + } + + fn transaction_payment(transaction: MessageLaneTransaction) -> bp_rialto::Balance { + // in our testnets, both per-byte fee and weight-to-fee are 1:1 + messages::transaction_payment_without_multiplier( + bp_rialto::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, + 1, + |weight| weight as _, + transaction, + ) + } } /// Millau chain from message lane point of view. @@ -193,13 +164,59 @@ impl messages::ChainWithMessageLanes for Millau { type AccountId = bp_millau::AccountId; type Signer = bp_millau::AccountSigner; type Signature = bp_millau::Signature; - type Call = (); // unknown to us type Weight = Weight; type Balance = bp_millau::Balance; type MessageLaneInstance = pallet_message_lane::DefaultInstance; } +impl messages::BridgedChainWithMessageLanes for Millau { + fn maximal_extrinsic_size() -> u32 { + bp_millau::max_extrinsic_size() + } + + fn message_weight_limits(_message_payload: &[u8]) -> RangeInclusive { + // we don't want to relay too large messages + keep reserve for future upgrades + let upper_limit = messages::target::maximal_incoming_message_dispatch_weight(bp_millau::max_extrinsic_weight()); + + // we're charging for payload bytes in `WithMillauMessageBridge::transaction_payment` function + // + // this bridge may be used to deliver all kind of messages, so we're not making any assumptions about + // minimal dispatch weight here + + 0..=upper_limit + } + + fn estimate_delivery_transaction( + message_payload: &[u8], + message_dispatch_weight: Weight, + ) -> MessageLaneTransaction { + let message_payload_len = u32::try_from(message_payload.len()).unwrap_or(u32::MAX); + let extra_bytes_in_payload = Weight::from(message_payload_len) + .saturating_sub(pallet_message_lane::EXPECTED_DEFAULT_MESSAGE_LENGTH.into()); + + MessageLaneTransaction { + 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_add(message_dispatch_weight), + size: message_payload_len + .saturating_add(bp_rialto::EXTRA_STORAGE_PROOF_SIZE) + .saturating_add(bp_millau::TX_EXTRA_BYTES), + } + } + + fn transaction_payment(transaction: MessageLaneTransaction) -> bp_millau::Balance { + // in our testnets, both per-byte fee and weight-to-fee are 1:1 + messages::transaction_payment_without_multiplier( + bp_millau::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic, + 1, + |weight| weight as _, + transaction, + ) + } +} + impl TargetHeaderChain for Millau { type Error = &'static str; // The proof is: diff --git a/bridges/bin/runtime-common/README.md b/bridges/bin/runtime-common/README.md index 58fe92c9ca..8d90bc7a8c 100644 --- a/bridges/bin/runtime-common/README.md +++ b/bridges/bin/runtime-common/README.md @@ -33,17 +33,63 @@ message lane module into your runtime. Basic prerequisites of these helpers are: ## `MessageBridge` Trait -The essence of your integration will be a struct that implements a `MessageBridge` trait. Let's -review every method and give some implementation hints here: +The essence of your integration will be a struct that implements a `MessageBridge` trait. It has +single method (`MessageBridge::bridged_balance_to_this_balance`), used to convert from bridged chain +tokens into this chain tokens. The bridge also requires two associated types to be specified - +`ThisChain` and `BridgedChain`. -- `MessageBridge::maximal_extrinsic_size_on_target_chain`: you will need to return the maximal - extrinsic size of the target chain from this function. This may be the constant that is updated - when your runtime is upgraded, or you may use the - [message lane parameters functionality](../../modules/message-lane/README.md#Non-Essential-Functionality) - to allow the pallet owner to update this value more frequently (you may also want to use this - functionality for all constants that are used in other methods described below). +Worth to say that if you're going to use hardcoded constant (conversion rate) in the +`MessageBridge::bridged_balance_to_this_balance` method (or in any other method of +`ThisChainWithMessageLanes` or `BridgedChainWithMessageLanes` traits), then you should take a +look at the +[message lane parameters functionality](../../modules/message-lane/README.md#Non-Essential-Functionality). +They allow pallet owner to update constants more frequently than runtime upgrade happens. -- `MessageBridge::weight_limits_of_message_on_bridged_chain`: you'll need to return a range of +## `ChainWithMessageLanes` Trait + +The trait is quite simple and can easily be implemented - you just need to specify types used at the +corresponding chain. There is single exception, though (it may be changed in the future): + +- `ChainWithMessageLanes::MessageLaneInstance`: this is used to compute runtime storage keys. There + may be several instances of message lane pallet, included in the Runtime. Every instance stores + messages and these messages stored under different keys. When we are verifying storage proofs from + the bridged chain, we should know which instance we're talking to. This is fine, but there's + significant inconvenience with that - this chain runtime must have the same message lane pallet + instance. This does not necessarily mean that we should use the same instance on both chains - + this instance may be used to bridge with another chain/instance, or may not be used at all. + +## `ThisChainWithMessageLanes` Trait + +This trait represents this chain from bridge point of view. Let's review every method of this trait: + +- `ThisChainWithMessageLanes::is_outbound_lane_enabled`: is used to check whether given lane accepts + outbound messages. + +- `ThisChainWithMessageLanes::maximal_pending_messages_at_outbound_lane`: you should return maximal + number of pending (undelivered) messages from this function. Returning small values would require + relayers to operate faster and could make message sending logic more complicated. On the other + hand, returning large values could lead to chain state growth. + +- `ThisChainWithMessageLanes::estimate_delivery_confirmation_transaction`: you'll need to return + estimated size and dispatch weight of the delivery confirmation transaction (that happens on + this chain) from this function. + +- `ThisChainWithMessageLanes::transaction_payment`: you'll need to return fee that the submitter + must pay for given transaction on this chain. Normally, you would use transaction payment pallet + for this. However, if your chain has non-zero fee multiplier set, this would mean that the + payment will be computed using current value of this multiplier. But since this transaction + will be submitted in the future, you may want to choose other value instead. Otherwise, + non-altruistic relayer may choose not to submit this transaction until number of transactions + will decrease. + +## `BridgedChainWithMessageLanes` Trait + +This trait represents this chain from bridge point of view. Let's review every method of this trait: + +- `BridgedChainWithMessageLanes::maximal_extrinsic_size`: you will need to return the maximal + extrinsic size of the target chain from this function. + +- `MessageBridge::message_weight_limits`: you'll need to return a range of dispatch weights that the outbound message may take at the target chain. Please keep in mind that our helpers assume that the message is an encoded call of the target chain. But we never decode this call at the source chain. So you can't simply get dispatch weight from pre-dispatch @@ -55,66 +101,13 @@ review every method and give some implementation hints here: maximal weight of extrinsic at the target chain. In our test chains, we reject all messages that have declared dispatch weight larger than 50% of the maximal bridged extrinsic weight. -- `MessageBridge::weight_of_delivery_transaction`: you will need to return the maximal weight of the - delivery transaction that delivers a given message to the target chain. There are three main - things to notice: +- `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. - 1. weight, returned from this function is then used to compute the fee that the - message sender needs to pay for the delivery transaction. So it shall not be a simple dispatch - weight of delivery call - it should be the "weight" of the transaction itself, including per-byte - "weight", "weight" of signed extras and etc. - 1. the delivery transaction brings storage proof of - the message, not the message itself. So your transaction will include extra bytes. We suggest - computing the size of single empty value storage proof at the source chain, increase this value a - bit and hardcode it in the source chain runtime code. This size then must be added to the size of - payload and included in the weight computation; - 1. before implementing this function, please take - a look at the - [weight formula of delivery transaction](../../modules/message-lane/README.md#Weight-of-receive_messages_proof-call). - It adds some extra weight for every additional byte of the proof (everything above - `pallet_message_lane::EXPECTED_DEFAULT_MESSAGE_LENGTH`), so it's not trivial. Even better, please - refer to [our implementation](../millau/runtime/src/rialto_messages.rs) for test chains for - details. - -- `MessageBridge::weight_of_delivery_confirmation_transaction_on_this_chain`: you'll need to return - the maximal weight of a single message delivery confirmation transaction on this chain. All points - from the previous paragraph are also relevant here. - -- `MessageBridge::this_weight_to_this_balance`: this function needs to convert weight units into fee - units on this chain. Most probably this can be done by calling - `pallet_transaction_payment::Config::WeightToFee::calc()` for passed weight. - -- `MessageBridge::bridged_weight_to_bridged_balance`: this function needs to convert weight units - into fee units on the target chain. The best case is when you have the same conversion formula on - both chains - then you may just call the same formula from the previous paragraph. Otherwise, - you'll need to hardcode this formula into your runtime. - -- `MessageBridge::bridged_balance_to_this_balance`: this may be the easiest method to implement and - the hardest to maintain at the same time. If you don't have any automatic methods to determine - conversion rate, then you'll probably need to maintain it by yourself (by updating conversion - rate, stored in runtime storage). This means that if you're too late with an update, then you risk - to accept messages with lower-than-expected fee. So it may be wise to have some reserve in this - conversion rate, even if that means larger delivery and dispatch fees. - -## `ChainWithMessageLanes` Trait - -Apart from its methods, `MessageBridge` also has two associated types that are implementing the -`ChainWithMessageLanes` trait. One is for this chain and the other is for the bridged chain. The -trait is quite simple and can easily be implemented - you just need to specify types used at the -corresponding chain. There are two exceptions, though. Both may be changed in the future. Here they -are: - -- `ChainWithMessageLanes::Call`: it isn't a good idea to reference bridged chain runtime from your - runtime (cyclic references + maintaining on upgrades). So you can't know the type of bridged chain - call in your runtime. This type isn't actually used at this chain, so you may use `()` instead. - -- `ChainWithMessageLanes::MessageLaneInstance`: this is used to compute runtime storage keys. There - may be several instances of message lane pallet, included in the Runtime. Every instance stores - messages and these messages stored under different keys. When we are verifying storage proofs from - the bridged chain, we should know which instance we're talking to. This is fine, but there's - significant inconvenience with that - this chain runtime must have the same message lane pallet - instance. This does not necessarily mean that we should use the same instance on both chains - - this instance may be used to bridge with another chain/instance, or may not be used at all. +- `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 + formula on both chains - then you may just reuse the `ThisChainWithMessageLanes::transaction_payment` + implementation. Otherwise, you'll need to hardcode this formula into your runtime. ## Helpers for the Source Chain diff --git a/bridges/bin/runtime-common/src/messages.rs b/bridges/bin/runtime-common/src/messages.rs index 04b2317749..755562db1f 100644 --- a/bridges/bin/runtime-common/src/messages.rs +++ b/bridges/bin/runtime-common/src/messages.rs @@ -31,7 +31,7 @@ use codec::{Decode, Encode}; use frame_support::{traits::Instance, weights::Weight, RuntimeDebug}; use hash_db::Hasher; use pallet_substrate_bridge::StorageProofChecker; -use sp_runtime::traits::{CheckedAdd, CheckedDiv, CheckedMul}; +use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedAdd, CheckedDiv, CheckedMul}; use sp_std::{cmp::PartialOrd, convert::TryFrom, fmt::Debug, marker::PhantomData, ops::RangeInclusive, vec::Vec}; use sp_trie::StorageProof; @@ -46,37 +46,9 @@ pub trait MessageBridge { /// This chain in context of message bridge. type ThisChain: ThisChainWithMessageLanes; /// Bridged chain in context of message bridge. - type BridgedChain: ChainWithMessageLanes; + type BridgedChain: BridgedChainWithMessageLanes; - /// Maximal extrinsic size on target chain. - fn maximal_extrinsic_size_on_target_chain() -> u32; - - /// Returns feasible weights range for given message payload on the target chain. - /// - /// If message is being sent with the weight that is out of this range, then it - /// should be rejected. - /// - /// Weights returned from this function shall not include transaction overhead - /// (like weight of signature and signed extensions verification), because they're - /// already accounted by the `weight_of_delivery_transaction`. So this function should - /// return pure call dispatch weights range. - fn weight_limits_of_message_on_bridged_chain( - message_payload: &[u8], - ) -> RangeInclusive>>; - - /// Maximal weight of single message delivery transaction on Bridged chain. - fn weight_of_delivery_transaction(message_payload: &[u8]) -> WeightOf>; - - /// Maximal weight of single message delivery confirmation transaction on This chain. - fn weight_of_delivery_confirmation_transaction_on_this_chain() -> WeightOf>; - - /// Convert weight of This chain to the fee (paid in Balance) of This chain. - fn this_weight_to_this_balance(weight: WeightOf>) -> BalanceOf>; - - /// Convert weight of the Bridged chain to the fee (paid in Balance) of the Bridged chain. - fn bridged_weight_to_bridged_balance(weight: WeightOf>) -> BalanceOf>; - - /// Convert Bridged chain Balance into This chain Balance. + /// Convert Bridged chain balance into This chain balance. fn bridged_balance_to_this_balance(bridged_balance: BalanceOf>) -> BalanceOf>; } @@ -90,8 +62,6 @@ pub trait ChainWithMessageLanes { type Signer: Decode; /// Signature type used on the chain. type Signature: Decode; - /// Call type on the chain. - type Call: Encode + Decode; /// Type of weight that is used on the chain. This would almost always be a regular /// `frame_support::weight::Weight`. But since the meaning of weight on different chains /// may be different, the `WeightOf<>` construct is used to avoid confusion between @@ -104,15 +74,59 @@ pub trait ChainWithMessageLanes { type MessageLaneInstance: Instance; } +/// Message-lane related transaction parameters estimation. +#[derive(RuntimeDebug)] +pub struct MessageLaneTransaction { + /// The estimated dispatch weight of the transaction. + pub dispatch_weight: Weight, + /// The estimated size of the encoded transaction. + pub size: u32, +} + /// This chain that has `message-lane` and `call-dispatch` modules. pub trait ThisChainWithMessageLanes: ChainWithMessageLanes { + /// Call type on the chain. + type Call: Encode + Decode; + /// Are we accepting any messages to the given lane? fn is_outbound_lane_enabled(lane: &LaneId) -> bool; - /// Maximal number of pending (not yet delivered) messages at this chain. + /// Maximal number of pending (not yet delivered) messages at This chain. /// /// Any messages over this limit, will be rejected. fn maximal_pending_messages_at_outbound_lane() -> MessageNonce; + + /// Estimate size and weight of single message delivery confirmation transaction at This chain. + fn estimate_delivery_confirmation_transaction() -> MessageLaneTransaction>; + + /// Returns minimal transaction fee that must be paid for given transaction at This chain. + fn transaction_payment(transaction: MessageLaneTransaction>) -> BalanceOf; +} + +/// Bridged chain that has `message-lane` and `call-dispatch` modules. +pub trait BridgedChainWithMessageLanes: ChainWithMessageLanes { + /// Maximal extrinsic size at Bridged chain. + fn maximal_extrinsic_size() -> u32; + + /// Returns feasible weights range for given message payload at the Bridged chain. + /// + /// If message is being sent with the weight that is out of this range, then it + /// should be rejected. + /// + /// Weights returned from this function shall not include transaction overhead + /// (like weight of signature and signed extensions verification), because they're + /// already accounted by the `weight_of_delivery_transaction`. So this function should + /// return pure call dispatch weights range. + fn message_weight_limits(message_payload: &[u8]) -> RangeInclusive; + + /// Estimate size and weight of single message delivery transaction at the Bridged chain. + fn estimate_delivery_transaction( + message_payload: &[u8], + message_dispatch_weight: WeightOf, + ) -> MessageLaneTransaction>; + + /// Returns minimal transaction fee that must be paid for given transaction at the Bridged chain. + fn transaction_payment(transaction: MessageLaneTransaction>) -> BalanceOf; } pub(crate) type ThisChain = ::ThisChain; @@ -123,35 +137,35 @@ pub(crate) type SignerOf = ::Signer; pub(crate) type SignatureOf = ::Signature; pub(crate) type WeightOf = ::Weight; pub(crate) type BalanceOf = ::Balance; -pub(crate) type CallOf = ::Call; pub(crate) type MessageLaneInstanceOf = ::MessageLaneInstance; +pub(crate) type CallOf = ::Call; + /// Raw storage proof type (just raw trie nodes). type RawStorageProof = Vec>; -/// Compute weight of transaction at runtime where: +/// Compute fee of transaction at runtime where: /// /// - transaction payment pallet is being used; /// - fee multiplier is zero. -pub fn transaction_weight_without_multiplier( - base_weight: Weight, - payload_size: Weight, - dispatch_weight: Weight, -) -> Weight { - // non-adjustable per-byte weight is mapped 1:1 to tx weight - let per_byte_weight = payload_size; +pub fn transaction_payment_without_multiplier( + base_extrinsic_weight: Weight, + per_byte_fee: Balance, + weight_to_fee: impl Fn(Weight) -> Balance, + transaction: MessageLaneTransaction, +) -> Balance { + // base fee is charged for every tx + let base_fee = weight_to_fee(base_extrinsic_weight); - // we assume that adjustable per-byte weight is always zero - let adjusted_per_byte_weight = 0; + // non-adjustable per-byte fee + let len_fee = per_byte_fee.saturating_mul(Balance::from(transaction.size)); - // we assume that transaction tip we use is also zero - let transaction_tip_weight = 0; + // the adjustable part of the fee + // + // here we assume that the fee multiplier is zero, so this part is also always zero + let adjusted_weight_fee = Balance::zero(); - base_weight - .saturating_add(per_byte_weight) - .saturating_add(adjusted_per_byte_weight) - .saturating_add(transaction_tip_weight) - .saturating_add(dispatch_weight) + base_fee.saturating_add(len_fee).saturating_add(adjusted_weight_fee) } /// Sub-module that is declaring types required for processing This -> Bridged chain messages. @@ -266,7 +280,7 @@ pub mod source { /// Return maximal message size of This -> Bridged chain message. pub fn maximal_message_size() -> u32 { - super::target::maximal_incoming_message_size(B::maximal_extrinsic_size_on_target_chain()) + super::target::maximal_incoming_message_size(BridgedChain::::maximal_extrinsic_size()) } /// Do basic Bridged-chain specific verification of This -> Bridged chain message. @@ -277,7 +291,7 @@ pub mod source { pub fn verify_chain_message( payload: &FromThisChainMessagePayload, ) -> Result<(), &'static str> { - let weight_limits = B::weight_limits_of_message_on_bridged_chain(&payload.call); + let weight_limits = BridgedChain::::message_weight_limits(&payload.call); if !weight_limits.contains(&payload.weight.into()) { return Err("Incorrect message weight declared"); } @@ -308,18 +322,17 @@ pub mod source { relayer_fee_percent: u32, ) -> Result>, &'static str> { // the fee (in Bridged tokens) of all transactions that are made on the Bridged chain - let delivery_fee = B::bridged_weight_to_bridged_balance(B::weight_of_delivery_transaction(&payload.call)); - let dispatch_fee = B::bridged_weight_to_bridged_balance(payload.weight.into()); + let delivery_transaction = + BridgedChain::::estimate_delivery_transaction(&payload.call, 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 - let delivery_confirmation_fee = - B::this_weight_to_this_balance(B::weight_of_delivery_confirmation_transaction_on_this_chain()); + let confirmation_transaction = ThisChain::::estimate_delivery_confirmation_transaction(); + let confirmation_transaction_fee = ThisChain::::transaction_payment(confirmation_transaction); // minimal fee (in This tokens) is a sum of all required fees - let minimal_fee = delivery_fee - .checked_add(&dispatch_fee) - .map(B::bridged_balance_to_this_balance) - .and_then(|fee| fee.checked_add(&delivery_confirmation_fee)); + let minimal_fee = + B::bridged_balance_to_this_balance(delivery_transaction_fee).checked_add(&confirmation_transaction_fee); // before returning, add extra fee that is paid to the relayer (relayer interest) minimal_fee @@ -681,31 +694,6 @@ mod tests { type ThisChain = ThisChain; type BridgedChain = BridgedChain; - fn maximal_extrinsic_size_on_target_chain() -> u32 { - BRIDGED_CHAIN_MAX_EXTRINSIC_SIZE - } - - fn weight_limits_of_message_on_bridged_chain(message_payload: &[u8]) -> RangeInclusive { - let begin = std::cmp::min(BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT, message_payload.len() as Weight); - begin..=BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT - } - - fn weight_of_delivery_transaction(_message_payload: &[u8]) -> Weight { - DELIVERY_TRANSACTION_WEIGHT - } - - fn weight_of_delivery_confirmation_transaction_on_this_chain() -> Weight { - DELIVERY_CONFIRMATION_TRANSACTION_WEIGHT - } - - fn this_weight_to_this_balance(weight: Weight) -> ThisChainBalance { - ThisChainBalance(weight as u32 * THIS_CHAIN_WEIGHT_TO_BALANCE_RATE as u32) - } - - fn bridged_weight_to_bridged_balance(weight: Weight) -> BridgedChainBalance { - BridgedChainBalance(weight as u32 * BRIDGED_CHAIN_WEIGHT_TO_BALANCE_RATE as u32) - } - fn bridged_balance_to_this_balance(bridged_balance: BridgedChainBalance) -> ThisChainBalance { ThisChainBalance(bridged_balance.0 * BRIDGED_CHAIN_TO_THIS_CHAIN_BALANCE_RATE as u32) } @@ -722,30 +710,6 @@ mod tests { type ThisChain = BridgedChain; type BridgedChain = ThisChain; - fn maximal_extrinsic_size_on_target_chain() -> u32 { - unreachable!() - } - - fn weight_limits_of_message_on_bridged_chain(_message_payload: &[u8]) -> RangeInclusive { - unreachable!() - } - - fn weight_of_delivery_transaction(_message_payload: &[u8]) -> Weight { - unreachable!() - } - - fn weight_of_delivery_confirmation_transaction_on_this_chain() -> Weight { - unreachable!() - } - - fn this_weight_to_this_balance(_weight: Weight) -> BridgedChainBalance { - unreachable!() - } - - fn bridged_weight_to_bridged_balance(_weight: Weight) -> ThisChainBalance { - unreachable!() - } - fn bridged_balance_to_this_balance(_this_balance: ThisChainBalance) -> BridgedChainBalance { unreachable!() } @@ -845,7 +809,6 @@ mod tests { type AccountId = ThisChainAccountId; type Signer = ThisChainSigner; type Signature = ThisChainSignature; - type Call = ThisChainCall; type Weight = frame_support::weights::Weight; type Balance = ThisChainBalance; @@ -853,6 +816,8 @@ mod tests { } impl ThisChainWithMessageLanes for ThisChain { + type Call = ThisChainCall; + fn is_outbound_lane_enabled(lane: &LaneId) -> bool { lane == TEST_LANE_ID } @@ -860,6 +825,38 @@ mod tests { fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { MAXIMAL_PENDING_MESSAGES_AT_TEST_LANE } + + fn estimate_delivery_confirmation_transaction() -> MessageLaneTransaction> { + MessageLaneTransaction { + dispatch_weight: DELIVERY_CONFIRMATION_TRANSACTION_WEIGHT, + size: 0, + } + } + + fn transaction_payment(transaction: MessageLaneTransaction>) -> BalanceOf { + ThisChainBalance(transaction.dispatch_weight as u32 * THIS_CHAIN_WEIGHT_TO_BALANCE_RATE as u32) + } + } + + impl BridgedChainWithMessageLanes for ThisChain { + fn maximal_extrinsic_size() -> u32 { + unreachable!() + } + + fn message_weight_limits(_message_payload: &[u8]) -> RangeInclusive { + unreachable!() + } + + fn estimate_delivery_transaction( + _message_payload: &[u8], + _message_dispatch_weight: WeightOf, + ) -> MessageLaneTransaction> { + unreachable!() + } + + fn transaction_payment(_transaction: MessageLaneTransaction>) -> BalanceOf { + unreachable!() + } } struct BridgedChain; @@ -869,7 +866,6 @@ mod tests { type AccountId = BridgedChainAccountId; type Signer = BridgedChainSigner; type Signature = BridgedChainSignature; - type Call = BridgedChainCall; type Weight = frame_support::weights::Weight; type Balance = BridgedChainBalance; @@ -877,6 +873,8 @@ mod tests { } impl ThisChainWithMessageLanes for BridgedChain { + type Call = BridgedChainCall; + fn is_outbound_lane_enabled(_lane: &LaneId) -> bool { unreachable!() } @@ -884,6 +882,39 @@ mod tests { fn maximal_pending_messages_at_outbound_lane() -> MessageNonce { unreachable!() } + + fn estimate_delivery_confirmation_transaction() -> MessageLaneTransaction> { + unreachable!() + } + + fn transaction_payment(_transaction: MessageLaneTransaction>) -> BalanceOf { + unreachable!() + } + } + + impl BridgedChainWithMessageLanes for BridgedChain { + fn maximal_extrinsic_size() -> u32 { + BRIDGED_CHAIN_MAX_EXTRINSIC_SIZE + } + + fn message_weight_limits(message_payload: &[u8]) -> RangeInclusive { + let begin = std::cmp::min(BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT, message_payload.len() as Weight); + begin..=BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT + } + + fn estimate_delivery_transaction( + _message_payload: &[u8], + message_dispatch_weight: WeightOf, + ) -> MessageLaneTransaction> { + MessageLaneTransaction { + dispatch_weight: DELIVERY_TRANSACTION_WEIGHT + message_dispatch_weight, + size: 0, + } + } + + fn transaction_payment(transaction: MessageLaneTransaction>) -> BalanceOf { + BridgedChainBalance(transaction.dispatch_weight as u32 * BRIDGED_CHAIN_WEIGHT_TO_BALANCE_RATE as u32) + } } fn test_lane_outbound_data() -> OutboundLaneData { diff --git a/bridges/modules/message-lane/src/weights_ext.rs b/bridges/modules/message-lane/src/weights_ext.rs index d99a20007d..bc75cd5ba0 100644 --- a/bridges/modules/message-lane/src/weights_ext.rs +++ b/bridges/modules/message-lane/src/weights_ext.rs @@ -95,9 +95,6 @@ pub fn ensure_able_to_receive_message( max_extrinsic_size: u32, max_extrinsic_weight: Weight, max_incoming_message_proof_size: u32, - // This is a base weight (which includes cost of tx itself, per-byte cost, adjusted per-byte cost) of single - // message delivery transaction that brings `max_incoming_message_proof_size` proof. - max_incoming_message_proof_base_weight: Weight, max_incoming_message_dispatch_weight: Weight, ) { // verify that we're able to receive proof of maximal-size message @@ -116,12 +113,9 @@ pub fn ensure_able_to_receive_message( 1, max_incoming_message_dispatch_weight, ); - let max_delivery_transaction_weight = - max_incoming_message_proof_base_weight.saturating_add(max_delivery_transaction_dispatch_weight); assert!( - max_delivery_transaction_weight <= max_extrinsic_weight, - "Weight of maximal message delivery transaction {} + {} is larger than maximal possible transaction weight {}", - max_delivery_transaction_weight, + max_delivery_transaction_dispatch_weight <= max_extrinsic_weight, + "Weight of maximal message delivery transaction + {} is larger than maximal possible transaction weight {}", max_delivery_transaction_dispatch_weight, max_extrinsic_weight, ); @@ -134,9 +128,6 @@ pub fn ensure_able_to_receive_confirmation( max_inbound_lane_data_proof_size_from_peer_chain: u32, max_unrewarded_relayer_entries_at_peer_inbound_lane: MessageNonce, max_unconfirmed_messages_at_inbound_lane: MessageNonce, - // This is a base weight (which includes cost of tx itself, per-byte cost, adjusted per-byte cost) of single - // confirmation transaction that brings `max_inbound_lane_data_proof_size_from_peer_chain` proof. - max_incoming_delivery_proof_base_weight: Weight, ) { // verify that we're able to receive confirmation of maximal-size let max_confirmation_transaction_size = @@ -158,12 +149,9 @@ pub fn ensure_able_to_receive_confirmation( ..Default::default() }, ); - let max_confirmation_transaction_weight = - max_incoming_delivery_proof_base_weight.saturating_add(max_confirmation_transaction_dispatch_weight); assert!( - max_confirmation_transaction_weight <= max_extrinsic_weight, - "Weight of maximal confirmation transaction {} + {} is larger than maximal possible transaction weight {}", - max_incoming_delivery_proof_base_weight, + max_confirmation_transaction_dispatch_weight <= max_extrinsic_weight, + "Weight of maximal confirmation transaction {} is larger than maximal possible transaction weight {}", max_confirmation_transaction_dispatch_weight, max_extrinsic_weight, ); diff --git a/bridges/primitives/millau/src/lib.rs b/bridges/primitives/millau/src/lib.rs index afaf36de32..7397fcf474 100644 --- a/bridges/primitives/millau/src/lib.rs +++ b/bridges/primitives/millau/src/lib.rs @@ -48,6 +48,11 @@ pub use millau_hash::MillauHash; /// Some reserve is reserved to account future chain growth. pub const EXTRA_STORAGE_PROOF_SIZE: u32 = 1024; +/// Number of bytes, included in the signed Millau transaction apart from the encoded call itself. +/// +/// Can be computed by subtracting encoded call size from raw transaction size. +pub const TX_EXTRA_BYTES: u32 = 103; + /// Maximal size (in bytes) of encoded (using `Encode::encode()`) account id. pub const MAXIMAL_ENCODED_ACCOUNT_ID_SIZE: u32 = 32; diff --git a/bridges/primitives/rialto/src/lib.rs b/bridges/primitives/rialto/src/lib.rs index c598b7ee83..ccb2154ee4 100644 --- a/bridges/primitives/rialto/src/lib.rs +++ b/bridges/primitives/rialto/src/lib.rs @@ -39,6 +39,11 @@ use sp_std::prelude::*; /// Some reserve is reserved to account future chain growth. pub const EXTRA_STORAGE_PROOF_SIZE: u32 = 1024; +/// Number of bytes, included in the signed Rialto transaction apart from the encoded call itself. +/// +/// Can be computed by subtracting encoded call size from raw transaction size. +pub const TX_EXTRA_BYTES: u32 = 103; + /// Maximal size (in bytes) of encoded (using `Encode::encode()`) account id. pub const MAXIMAL_ENCODED_ACCOUNT_ID_SIZE: u32 = 32; diff --git a/bridges/relays/ethereum/src/rialto_client.rs b/bridges/relays/ethereum/src/rialto_client.rs index 861ef8efeb..11eb200a1c 100644 --- a/bridges/relays/ethereum/src/rialto_client.rs +++ b/bridges/relays/ethereum/src/rialto_client.rs @@ -160,7 +160,7 @@ impl SubmitEthereumHeaders for SubstrateClient { let nonce = self.next_account_index(account_id).await?; let call = instance.build_signed_header_call(headers); - let transaction = Rialto::sign_transaction(self, ¶ms.signer, nonce, call); + let transaction = Rialto::sign_transaction(*self.genesis_hash(), ¶ms.signer, nonce, call); let _ = self.submit_extrinsic(Bytes(transaction.encode())).await?; Ok(()) @@ -256,7 +256,7 @@ impl SubmitEthereumExchangeTransactionProof for SubstrateClient { let nonce = self.next_account_index(account_id).await?; let call = instance.build_currency_exchange_call(proof); - let transaction = Rialto::sign_transaction(self, ¶ms.signer, nonce, call); + let transaction = Rialto::sign_transaction(*self.genesis_hash(), ¶ms.signer, nonce, call); let _ = self.submit_extrinsic(Bytes(transaction.encode())).await?; Ok(()) diff --git a/bridges/relays/millau-client/src/lib.rs b/bridges/relays/millau-client/src/lib.rs index c7d0405687..36444cda45 100644 --- a/bridges/relays/millau-client/src/lib.rs +++ b/bridges/relays/millau-client/src/lib.rs @@ -17,7 +17,7 @@ //! Types used to connect to the Millau-Substrate chain. use codec::Encode; -use relay_substrate_client::{Chain, ChainBase, ChainWithBalances, Client, TransactionSignScheme}; +use relay_substrate_client::{Chain, ChainBase, ChainWithBalances, TransactionSignScheme}; use sp_core::{storage::StorageKey, Pair}; use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount}; use std::time::Duration; @@ -65,7 +65,7 @@ impl TransactionSignScheme for Millau { type SignedTransaction = millau_runtime::UncheckedExtrinsic; fn sign_transaction( - client: &Client, + genesis_hash: ::Hash, signer: &Self::AccountKeyPair, signer_nonce: ::Index, call: ::Call, @@ -84,8 +84,8 @@ impl TransactionSignScheme for Millau { ( millau_runtime::VERSION.spec_version, millau_runtime::VERSION.transaction_version, - *client.genesis_hash(), - *client.genesis_hash(), + genesis_hash, + genesis_hash, (), (), (), diff --git a/bridges/relays/rialto-client/src/lib.rs b/bridges/relays/rialto-client/src/lib.rs index 9e38831d56..bac65b6427 100644 --- a/bridges/relays/rialto-client/src/lib.rs +++ b/bridges/relays/rialto-client/src/lib.rs @@ -17,7 +17,7 @@ //! Types used to connect to the Rialto-Substrate chain. use codec::Encode; -use relay_substrate_client::{Chain, ChainBase, ChainWithBalances, Client, TransactionSignScheme}; +use relay_substrate_client::{Chain, ChainBase, ChainWithBalances, TransactionSignScheme}; use sp_core::{storage::StorageKey, Pair}; use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount}; use std::time::Duration; @@ -65,7 +65,7 @@ impl TransactionSignScheme for Rialto { type SignedTransaction = rialto_runtime::UncheckedExtrinsic; fn sign_transaction( - client: &Client, + genesis_hash: ::Hash, signer: &Self::AccountKeyPair, signer_nonce: ::Index, call: ::Call, @@ -84,8 +84,8 @@ impl TransactionSignScheme for Rialto { ( rialto_runtime::VERSION.spec_version, rialto_runtime::VERSION.transaction_version, - *client.genesis_hash(), - *client.genesis_hash(), + genesis_hash, + genesis_hash, (), (), (), diff --git a/bridges/relays/substrate-client/src/chain.rs b/bridges/relays/substrate-client/src/chain.rs index 60a3cc5d17..86ad4bc8c9 100644 --- a/bridges/relays/substrate-client/src/chain.rs +++ b/bridges/relays/substrate-client/src/chain.rs @@ -14,8 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -use crate::client::Client; - use bp_runtime::Chain as ChainBase; use frame_support::Parameter; use jsonrpsee_types::jsonrpc::{DeserializeOwned, Serialize}; @@ -87,7 +85,7 @@ pub trait TransactionSignScheme { /// Create transaction for given runtime call, signed by given account. fn sign_transaction( - client: &Client, + genesis_hash: ::Hash, signer: &Self::AccountKeyPair, signer_nonce: ::Index, call: ::Call, diff --git a/bridges/relays/substrate/Cargo.toml b/bridges/relays/substrate/Cargo.toml index 08cf40c383..b56b3a3cbd 100644 --- a/bridges/relays/substrate/Cargo.toml +++ b/bridges/relays/substrate/Cargo.toml @@ -48,3 +48,6 @@ sp-core = { git = "https://github.com/paritytech/substrate.git", branch = "maste sp-finality-grandpa = { git = "https://github.com/paritytech/substrate.git", branch = "master" } sp-runtime = { git = "https://github.com/paritytech/substrate.git", branch = "master" } sp-trie = { git = "https://github.com/paritytech/substrate.git", branch = "master" } + +[dev-dependencies] +sp-keyring = { git = "https://github.com/paritytech/substrate.git", branch = "master" } diff --git a/bridges/relays/substrate/src/main.rs b/bridges/relays/substrate/src/main.rs index 722f850d89..8cd437027c 100644 --- a/bridges/relays/substrate/src/main.rs +++ b/bridges/relays/substrate/src/main.rs @@ -96,7 +96,7 @@ async fn run_init_bridge(command: cli::InitBridge) -> Result<(), String> { move |initialization_data| { Ok(Bytes( Rialto::sign_transaction( - &rialto_client, + *rialto_client.genesis_hash(), &rialto_sign.signer, rialto_signer_next_index, rialto_runtime::SudoCall::sudo(Box::new( @@ -132,7 +132,7 @@ async fn run_init_bridge(command: cli::InitBridge) -> Result<(), String> { move |initialization_data| { Ok(Bytes( Millau::sign_transaction( - &millau_client, + *millau_client.genesis_hash(), &millau_sign.signer, millau_signer_next_index, millau_runtime::SudoCall::sudo(Box::new( @@ -266,7 +266,7 @@ async fn run_send_message(command: cli::SendMessage) -> Result<(), String> { ); let signed_millau_call = Millau::sign_transaction( - &millau_client, + *millau_client.genesis_hash(), &millau_sign.signer, millau_client .next_account_index(millau_sign.signer.public().clone().into()) @@ -322,7 +322,7 @@ async fn run_send_message(command: cli::SendMessage) -> Result<(), String> { ); let signed_rialto_call = Rialto::sign_transaction( - &rialto_client, + *rialto_client.genesis_hash(), &rialto_sign.signer, rialto_client .next_account_index(rialto_sign.signer.public().clone().into()) @@ -933,4 +933,40 @@ mod tests { ); assert!(Rialto::verify_message(&payload).is_err()); } + + #[test] + fn rialto_tx_extra_bytes_constant_is_correct() { + let rialto_call = rialto_runtime::Call::System(rialto_runtime::SystemCall::remark(vec![])); + let rialto_tx = Rialto::sign_transaction( + Default::default(), + &sp_keyring::AccountKeyring::Alice.pair(), + 0, + rialto_call.clone(), + ); + let extra_bytes_in_transaction = rialto_tx.encode().len() - rialto_call.encode().len(); + assert!( + bp_rialto::TX_EXTRA_BYTES as usize >= extra_bytes_in_transaction, + "Hardcoded number of extra bytes in Rialto transaction {} is lower than actual value: {}", + bp_rialto::TX_EXTRA_BYTES, + extra_bytes_in_transaction, + ); + } + + #[test] + fn millau_tx_extra_bytes_constant_is_correct() { + let millau_call = millau_runtime::Call::System(millau_runtime::SystemCall::remark(vec![])); + let millau_tx = Millau::sign_transaction( + Default::default(), + &sp_keyring::AccountKeyring::Alice.pair(), + 0, + millau_call.clone(), + ); + let extra_bytes_in_transaction = millau_tx.encode().len() - millau_call.encode().len(); + assert!( + bp_millau::TX_EXTRA_BYTES as usize >= extra_bytes_in_transaction, + "Hardcoded number of extra bytes in Millau transaction {} is lower than actual value: {}", + bp_millau::TX_EXTRA_BYTES, + extra_bytes_in_transaction, + ); + } } diff --git a/bridges/relays/substrate/src/millau_headers_to_rialto.rs b/bridges/relays/substrate/src/millau_headers_to_rialto.rs index 7b9df18d8a..1ac12d2eeb 100644 --- a/bridges/relays/substrate/src/millau_headers_to_rialto.rs +++ b/bridges/relays/substrate/src/millau_headers_to_rialto.rs @@ -49,7 +49,10 @@ impl SubstrateFinalitySyncPipeline for MillauFinalityToRialto { (), ) .into(); - let transaction = Rialto::sign_transaction(&self.target_client, &self.target_sign.signer, nonce, call); + + let genesis_hash = *self.target_client.genesis_hash(); + let transaction = Rialto::sign_transaction(genesis_hash, &self.target_sign.signer, nonce, call); + Ok(transaction) } } diff --git a/bridges/relays/substrate/src/millau_messages_to_rialto.rs b/bridges/relays/substrate/src/millau_messages_to_rialto.rs index ecd59e146f..f3dfdadf9a 100644 --- a/bridges/relays/substrate/src/millau_messages_to_rialto.rs +++ b/bridges/relays/substrate/src/millau_messages_to_rialto.rs @@ -68,7 +68,8 @@ impl SubstrateMessageLane for MillauMessagesToRialto { let call: millau_runtime::Call = millau_runtime::MessageLaneCall::receive_messages_delivery_proof(proof, relayers_state).into(); let call_weight = call.get_dispatch_info().weight; - let transaction = Millau::sign_transaction(&self.source_client, &self.source_sign.signer, nonce, call); + let genesis_hash = *self.source_client.genesis_hash(); + let transaction = Millau::sign_transaction(genesis_hash, &self.source_sign.signer, nonce, call); log::trace!( target: "bridge", "Prepared Rialto -> Millau confirmation transaction. Weight: {}/{}, size: {}/{}", @@ -103,7 +104,8 @@ impl SubstrateMessageLane for MillauMessagesToRialto { ) .into(); let call_weight = call.get_dispatch_info().weight; - let transaction = Rialto::sign_transaction(&self.target_client, &self.target_sign.signer, nonce, call); + let genesis_hash = *self.target_client.genesis_hash(); + let transaction = Rialto::sign_transaction(genesis_hash, &self.target_sign.signer, nonce, call); log::trace!( target: "bridge", "Prepared Millau -> Rialto delivery transaction. Weight: {}/{}, size: {}/{}", diff --git a/bridges/relays/substrate/src/rialto_headers_to_millau.rs b/bridges/relays/substrate/src/rialto_headers_to_millau.rs index 6560d83c8c..27fe697aad 100644 --- a/bridges/relays/substrate/src/rialto_headers_to_millau.rs +++ b/bridges/relays/substrate/src/rialto_headers_to_millau.rs @@ -49,7 +49,10 @@ impl SubstrateFinalitySyncPipeline for RialtoFinalityToMillau { (), ) .into(); - let transaction = Millau::sign_transaction(&self.target_client, &self.target_sign.signer, nonce, call); + + let genesis_hash = *self.target_client.genesis_hash(); + let transaction = Millau::sign_transaction(genesis_hash, &self.target_sign.signer, nonce, call); + Ok(transaction) } } diff --git a/bridges/relays/substrate/src/rialto_messages_to_millau.rs b/bridges/relays/substrate/src/rialto_messages_to_millau.rs index 07af9ce828..96211bd944 100644 --- a/bridges/relays/substrate/src/rialto_messages_to_millau.rs +++ b/bridges/relays/substrate/src/rialto_messages_to_millau.rs @@ -68,7 +68,8 @@ impl SubstrateMessageLane for RialtoMessagesToMillau { let call: rialto_runtime::Call = rialto_runtime::MessageLaneCall::receive_messages_delivery_proof(proof, relayers_state).into(); let call_weight = call.get_dispatch_info().weight; - let transaction = Rialto::sign_transaction(&self.source_client, &self.source_sign.signer, nonce, call); + let genesis_hash = *self.source_client.genesis_hash(); + let transaction = Rialto::sign_transaction(genesis_hash, &self.source_sign.signer, nonce, call); log::trace!( target: "bridge", "Prepared Millau -> Rialto confirmation transaction. Weight: {}/{}, size: {}/{}", @@ -103,7 +104,8 @@ impl SubstrateMessageLane for RialtoMessagesToMillau { ) .into(); let call_weight = call.get_dispatch_info().weight; - let transaction = Millau::sign_transaction(&self.target_client, &self.target_sign.signer, nonce, call); + let genesis_hash = *self.target_client.genesis_hash(); + let transaction = Millau::sign_transaction(genesis_hash, &self.target_sign.signer, nonce, call); log::trace!( target: "bridge", "Prepared Rialto -> Millau delivery transaction. Weight: {}/{}, size: {}/{}",