From 1f117d68943ab2b284e41b34ede0eac0dc627df0 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Mon, 18 Jul 2022 13:19:06 +0300 Subject: [PATCH] Follow-up on #1487 (#1511) * Remove unused trait implementations Signed-off-by: Serban Iorga * Define encoded_size_hint_u32() Signed-off-by: Serban Iorga * Define TransactionEstimationParams trait Signed-off-by: Serban Iorga * Rework TransactionEstimation Signed-off-by: Serban Iorga * Docs + Renamings Signed-off-by: Serban Iorga --- .../bin/millau/runtime/src/rialto_messages.rs | 29 +++---- .../runtime/src/rialto_parachain_messages.rs | 23 +++--- .../runtime/src/millau_messages.rs | 25 +++--- .../bin/rialto/runtime/src/millau_messages.rs | 29 +++---- bridges/bin/runtime-common/src/messages.rs | 77 +++++++++++++++---- bridges/modules/messages/src/inbound_lane.rs | 8 -- bridges/modules/messages/src/outbound_lane.rs | 8 -- bridges/primitives/messages/src/lib.rs | 13 ++++ .../src/messages_source.rs | 4 +- 9 files changed, 115 insertions(+), 101 deletions(-) diff --git a/bridges/bin/millau/runtime/src/rialto_messages.rs b/bridges/bin/millau/runtime/src/rialto_messages.rs index 5d192bd56ad26..da44bcf8f8feb 100644 --- a/bridges/bin/millau/runtime/src/rialto_messages.rs +++ b/bridges/bin/millau/runtime/src/rialto_messages.rs @@ -24,7 +24,9 @@ use bp_messages::{ InboundLaneData, LaneId, Message, MessageNonce, Parameter as MessagesParameter, }; use bp_runtime::{Chain, ChainId, MILLAU_CHAIN_ID, RIALTO_CHAIN_ID}; -use bridge_runtime_common::messages::{self, MessageBridge, MessageTransaction}; +use bridge_runtime_common::messages::{ + self, BasicConfirmationTransactionEstimation, MessageBridge, MessageTransaction, +}; use codec::{Decode, Encode}; use frame_support::{ parameter_types, @@ -123,6 +125,12 @@ impl messages::ChainWithMessages for Millau { impl messages::ThisChainWithMessages for Millau { type Origin = crate::Origin; type Call = crate::Call; + type ConfirmationTransactionEstimation = BasicConfirmationTransactionEstimation< + Self::AccountId, + { bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT }, + { bp_rialto::EXTRA_STORAGE_PROOF_SIZE }, + { bp_millau::TX_EXTRA_BYTES }, + >; fn is_message_accepted(send_origin: &Self::Origin, lane: &LaneId) -> bool { let here_location = @@ -148,19 +156,6 @@ impl messages::ThisChainWithMessages for Millau { MessageNonce::MAX } - fn estimate_delivery_confirmation_transaction() -> MessageTransaction { - let inbound_data_size = InboundLaneData::::encoded_size_hint(1, 1) - .and_then(|x| u32::try_from(x).ok()) - .unwrap_or(u32::MAX); - - MessageTransaction { - 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: MessageTransaction) -> bp_millau::Balance { // `transaction` may represent transaction from the future, when multiplier value will // be larger, so let's use slightly increased value @@ -346,12 +341,10 @@ mod tests { ); let max_incoming_inbound_lane_data_proof_size = - bp_messages::InboundLaneData::<()>::encoded_size_hint( + bp_messages::InboundLaneData::<()>::encoded_size_hint_u32( bp_millau::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX as _, bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX as _, - ) - .and_then(|x| u32::try_from(x).ok()) - .unwrap_or(u32::MAX); + ); pallet_bridge_messages::ensure_able_to_receive_confirmation::( bp_millau::Millau::max_extrinsic_size(), bp_millau::Millau::max_extrinsic_weight(), diff --git a/bridges/bin/millau/runtime/src/rialto_parachain_messages.rs b/bridges/bin/millau/runtime/src/rialto_parachain_messages.rs index ceabe2b00018d..ef1dac57d8385 100644 --- a/bridges/bin/millau/runtime/src/rialto_parachain_messages.rs +++ b/bridges/bin/millau/runtime/src/rialto_parachain_messages.rs @@ -25,7 +25,9 @@ use bp_messages::{ }; use bp_polkadot_core::parachains::ParaId; use bp_runtime::{Chain, ChainId, MILLAU_CHAIN_ID, RIALTO_PARACHAIN_CHAIN_ID}; -use bridge_runtime_common::messages::{self, MessageBridge, MessageTransaction}; +use bridge_runtime_common::messages::{ + self, BasicConfirmationTransactionEstimation, MessageBridge, MessageTransaction, +}; use codec::{Decode, Encode}; use frame_support::{ parameter_types, @@ -128,6 +130,12 @@ impl messages::ChainWithMessages for Millau { impl messages::ThisChainWithMessages for Millau { type Call = crate::Call; type Origin = crate::Origin; + type ConfirmationTransactionEstimation = BasicConfirmationTransactionEstimation< + Self::AccountId, + { bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT }, + { bp_rialto_parachain::EXTRA_STORAGE_PROOF_SIZE }, + { bp_millau::TX_EXTRA_BYTES }, + >; fn is_message_accepted(_send_origin: &Self::Origin, lane: &LaneId) -> bool { *lane == [0, 0, 0, 0] || *lane == [0, 0, 0, 1] @@ -137,19 +145,6 @@ impl messages::ThisChainWithMessages for Millau { MessageNonce::MAX } - fn estimate_delivery_confirmation_transaction() -> MessageTransaction { - let inbound_data_size = InboundLaneData::::encoded_size_hint(1, 1) - .and_then(|x| u32::try_from(x).ok()) - .unwrap_or(u32::MAX); - - MessageTransaction { - dispatch_weight: bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, - size: inbound_data_size - .saturating_add(bp_rialto_parachain::EXTRA_STORAGE_PROOF_SIZE) - .saturating_add(bp_millau::TX_EXTRA_BYTES), - } - } - fn transaction_payment(transaction: MessageTransaction) -> bp_millau::Balance { // `transaction` may represent transaction from the future, when multiplier value will // be larger, so let's use slightly increased value diff --git a/bridges/bin/rialto-parachain/runtime/src/millau_messages.rs b/bridges/bin/rialto-parachain/runtime/src/millau_messages.rs index ef8e3c657ac69..248ec7b834d41 100644 --- a/bridges/bin/rialto-parachain/runtime/src/millau_messages.rs +++ b/bridges/bin/rialto-parachain/runtime/src/millau_messages.rs @@ -27,7 +27,9 @@ use bp_messages::{ InboundLaneData, LaneId, Message, MessageNonce, Parameter as MessagesParameter, }; use bp_runtime::{Chain, ChainId, MILLAU_CHAIN_ID, RIALTO_PARACHAIN_CHAIN_ID}; -use bridge_runtime_common::messages::{self, MessageBridge, MessageTransaction}; +use bridge_runtime_common::messages::{ + self, BasicConfirmationTransactionEstimation, MessageBridge, MessageTransaction, +}; use codec::{Decode, Encode}; use frame_support::{ parameter_types, @@ -127,6 +129,12 @@ impl messages::ChainWithMessages for RialtoParachain { impl messages::ThisChainWithMessages for RialtoParachain { type Call = crate::Call; type Origin = crate::Origin; + type ConfirmationTransactionEstimation = BasicConfirmationTransactionEstimation< + Self::AccountId, + { bp_rialto_parachain::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT }, + { bp_millau::EXTRA_STORAGE_PROOF_SIZE }, + { bp_rialto_parachain::TX_EXTRA_BYTES }, + >; fn is_message_accepted(send_origin: &Self::Origin, lane: &LaneId) -> bool { send_origin.linked_account().is_some() && (*lane == [0, 0, 0, 0] || *lane == [0, 0, 0, 1]) @@ -136,21 +144,6 @@ impl messages::ThisChainWithMessages for RialtoParachain { MessageNonce::MAX } - fn estimate_delivery_confirmation_transaction() -> MessageTransaction { - let inbound_data_size = - InboundLaneData::::encoded_size_hint(1, 1) - .and_then(|x| u32::try_from(x).ok()) - .unwrap_or(u32::MAX); - - MessageTransaction { - dispatch_weight: - bp_rialto_parachain::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, - size: inbound_data_size - .saturating_add(bp_millau::EXTRA_STORAGE_PROOF_SIZE) - .saturating_add(bp_rialto_parachain::TX_EXTRA_BYTES), - } - } - fn transaction_payment( transaction: MessageTransaction, ) -> bp_rialto_parachain::Balance { diff --git a/bridges/bin/rialto/runtime/src/millau_messages.rs b/bridges/bin/rialto/runtime/src/millau_messages.rs index e5e95e3d4a351..1267cbd06e29f 100644 --- a/bridges/bin/rialto/runtime/src/millau_messages.rs +++ b/bridges/bin/rialto/runtime/src/millau_messages.rs @@ -24,7 +24,9 @@ use bp_messages::{ InboundLaneData, LaneId, Message, MessageNonce, Parameter as MessagesParameter, }; use bp_runtime::{Chain, ChainId, MILLAU_CHAIN_ID, RIALTO_CHAIN_ID}; -use bridge_runtime_common::messages::{self, MessageBridge, MessageTransaction}; +use bridge_runtime_common::messages::{ + self, BasicConfirmationTransactionEstimation, MessageBridge, MessageTransaction, +}; use codec::{Decode, Encode}; use frame_support::{ parameter_types, @@ -122,6 +124,12 @@ impl messages::ChainWithMessages for Rialto { impl messages::ThisChainWithMessages for Rialto { type Origin = crate::Origin; type Call = crate::Call; + type ConfirmationTransactionEstimation = BasicConfirmationTransactionEstimation< + Self::AccountId, + { bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT }, + { bp_millau::EXTRA_STORAGE_PROOF_SIZE }, + { bp_rialto::TX_EXTRA_BYTES }, + >; fn is_message_accepted(send_origin: &Self::Origin, lane: &LaneId) -> bool { let here_location = @@ -147,19 +155,6 @@ impl messages::ThisChainWithMessages for Rialto { MessageNonce::MAX } - fn estimate_delivery_confirmation_transaction() -> MessageTransaction { - let inbound_data_size = InboundLaneData::::encoded_size_hint(1, 1) - .and_then(|x| u32::try_from(x).ok()) - .unwrap_or(u32::MAX); - - MessageTransaction { - 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: MessageTransaction) -> bp_rialto::Balance { // `transaction` may represent transaction from the future, when multiplier value will // be larger, so let's use slightly increased value @@ -343,12 +338,10 @@ mod tests { ); let max_incoming_inbound_lane_data_proof_size = - bp_messages::InboundLaneData::<()>::encoded_size_hint( + bp_messages::InboundLaneData::<()>::encoded_size_hint_u32( bp_rialto::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX as _, bp_rialto::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX as _, - ) - .and_then(|x| u32::try_from(x).ok()) - .unwrap_or(u32::MAX); + ); pallet_bridge_messages::ensure_able_to_receive_confirmation::( bp_rialto::Rialto::max_extrinsic_size(), bp_rialto::Rialto::max_extrinsic_weight(), diff --git a/bridges/bin/runtime-common/src/messages.rs b/bridges/bin/runtime-common/src/messages.rs index 173a501032f6e..8153e0840cf5c 100644 --- a/bridges/bin/runtime-common/src/messages.rs +++ b/bridges/bin/runtime-common/src/messages.rs @@ -27,7 +27,7 @@ use bp_messages::{ }; use bp_polkadot_core::parachains::{ParaHash, ParaHasher, ParaId}; use bp_runtime::{messages::MessageDispatchResult, ChainId, Size, StorageProofChecker}; -use codec::{Decode, DecodeLimit, Encode}; +use codec::{Decode, DecodeLimit, Encode, MaxEncodedLen}; use frame_support::{traits::Get, weights::Weight, RuntimeDebug}; use hash_db::Hasher; use scale_info::TypeInfo; @@ -69,7 +69,7 @@ pub trait ChainWithMessages { /// Hash used in the chain. type Hash: Decode; /// Accound id on the chain. - type AccountId: Encode + Decode; + type AccountId: Encode + Decode + MaxEncodedLen; /// Public key of the chain account that may be used to verify signatures. type Signer: Encode + Decode; /// Signature type used on the chain. @@ -99,12 +99,54 @@ pub struct MessageTransaction { pub size: u32, } +/// Helper trait for estimating the size and weight of a single message delivery confirmation +/// transaction. +pub trait ConfirmationTransactionEstimation { + // Estimate size and weight of single message delivery confirmation transaction. + fn estimate_delivery_confirmation_transaction() -> MessageTransaction; +} + +/// Default implementation for `ConfirmationTransactionEstimation`. +pub struct BasicConfirmationTransactionEstimation< + AccountId: MaxEncodedLen, + const MAX_CONFIRMATION_TX_WEIGHT: Weight, + const EXTRA_STORAGE_PROOF_SIZE: u32, + const TX_EXTRA_BYTES: u32, +>(PhantomData); + +impl< + AccountId: MaxEncodedLen, + const MAX_CONFIRMATION_TX_WEIGHT: Weight, + const EXTRA_STORAGE_PROOF_SIZE: u32, + const TX_EXTRA_BYTES: u32, + > ConfirmationTransactionEstimation + for BasicConfirmationTransactionEstimation< + AccountId, + MAX_CONFIRMATION_TX_WEIGHT, + EXTRA_STORAGE_PROOF_SIZE, + TX_EXTRA_BYTES, + > +{ + fn estimate_delivery_confirmation_transaction() -> MessageTransaction { + let inbound_data_size = InboundLaneData::::encoded_size_hint_u32(1, 1); + MessageTransaction { + dispatch_weight: MAX_CONFIRMATION_TX_WEIGHT, + size: inbound_data_size + .saturating_add(EXTRA_STORAGE_PROOF_SIZE) + .saturating_add(TX_EXTRA_BYTES), + } + } +} + /// This chain that has `pallet-bridge-messages` and `dispatch` modules. pub trait ThisChainWithMessages: ChainWithMessages { /// Call origin on the chain. type Origin; /// Call type on the chain. type Call: Encode + Decode; + /// Helper for estimating the size and weight of a single message delivery confirmation + /// transaction at this chain. + type ConfirmationTransactionEstimation: ConfirmationTransactionEstimation>; /// Do we accept message sent by given origin to given lane? fn is_message_accepted(origin: &Self::Origin, lane: &LaneId) -> bool; @@ -115,7 +157,9 @@ pub trait ThisChainWithMessages: ChainWithMessages { 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() -> MessageTransaction>; + fn estimate_delivery_confirmation_transaction() -> MessageTransaction> { + Self::ConfirmationTransactionEstimation::estimate_delivery_confirmation_transaction() + } /// Returns minimal transaction fee that must be paid for given transaction at This chain. fn transaction_payment(transaction: MessageTransaction>) -> BalanceOf; @@ -1037,7 +1081,7 @@ mod tests { } } - #[derive(Debug, PartialEq, Eq, Decode, Encode, Clone)] + #[derive(Debug, PartialEq, Eq, Decode, Encode, Clone, MaxEncodedLen)] struct ThisChainAccountId(u32); #[derive(Debug, PartialEq, Eq, Decode, Encode)] struct ThisChainSigner(u32); @@ -1063,7 +1107,7 @@ mod tests { } } - #[derive(Debug, PartialEq, Eq, Decode, Encode)] + #[derive(Debug, PartialEq, Eq, Decode, Encode, MaxEncodedLen)] struct BridgedChainAccountId(u32); #[derive(Debug, PartialEq, Eq, Decode, Encode)] struct BridgedChainSigner(u32); @@ -1162,6 +1206,12 @@ mod tests { impl ThisChainWithMessages for ThisChain { type Origin = ThisChainOrigin; type Call = ThisChainCall; + type ConfirmationTransactionEstimation = BasicConfirmationTransactionEstimation< + ::AccountId, + { DELIVERY_CONFIRMATION_TRANSACTION_WEIGHT }, + 0, + 0, + >; fn is_message_accepted(_send_origin: &Self::Origin, lane: &LaneId) -> bool { lane == TEST_LANE_ID @@ -1171,13 +1221,6 @@ mod tests { MAXIMAL_PENDING_MESSAGES_AT_TEST_LANE } - fn estimate_delivery_confirmation_transaction() -> MessageTransaction> { - MessageTransaction { - dispatch_weight: DELIVERY_CONFIRMATION_TRANSACTION_WEIGHT, - size: 0, - } - } - fn transaction_payment(transaction: MessageTransaction>) -> BalanceOf { ThisChainBalance( transaction.dispatch_weight as u32 * THIS_CHAIN_WEIGHT_TO_BALANCE_RATE as u32, @@ -1223,6 +1266,12 @@ mod tests { impl ThisChainWithMessages for BridgedChain { type Origin = BridgedChainOrigin; type Call = BridgedChainCall; + type ConfirmationTransactionEstimation = BasicConfirmationTransactionEstimation< + ::AccountId, + 0, + 0, + 0, + >; fn is_message_accepted(_send_origin: &Self::Origin, _lane: &LaneId) -> bool { unreachable!() @@ -1232,10 +1281,6 @@ mod tests { unreachable!() } - fn estimate_delivery_confirmation_transaction() -> MessageTransaction> { - unreachable!() - } - fn transaction_payment( _transaction: MessageTransaction>, ) -> BalanceOf { diff --git a/bridges/modules/messages/src/inbound_lane.rs b/bridges/modules/messages/src/inbound_lane.rs index 6624655ddc18e..cae76927045bc 100644 --- a/bridges/modules/messages/src/inbound_lane.rs +++ b/bridges/modules/messages/src/inbound_lane.rs @@ -79,14 +79,6 @@ impl, I: 'static> Default for StoredInboundLaneData { } } -impl, I: 'static> From> - for StoredInboundLaneData -{ - fn from(data: InboundLaneData) -> Self { - StoredInboundLaneData(data) - } -} - impl, I: 'static> From> for InboundLaneData { diff --git a/bridges/modules/messages/src/outbound_lane.rs b/bridges/modules/messages/src/outbound_lane.rs index 5f977b2f2e297..bdd503c811e73 100644 --- a/bridges/modules/messages/src/outbound_lane.rs +++ b/bridges/modules/messages/src/outbound_lane.rs @@ -72,14 +72,6 @@ impl, I: 'static> sp_std::ops::DerefMut for StoredMessageData } } -impl, I: 'static> From> - for StoredMessageData -{ - fn from(data: MessageData) -> Self { - StoredMessageData(data) - } -} - impl, I: 'static> From> for MessageData { diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs index ad1dbd38be901..d5b2b9c9db4f2 100644 --- a/bridges/primitives/messages/src/lib.rs +++ b/bridges/primitives/messages/src/lib.rs @@ -174,6 +174,19 @@ impl InboundLaneData { .and_then(|result| result.checked_add(dispatch_result_size)) } + /// Returns the approximate size of the struct as u32, given a number of entries in the + /// `relayers` set and the size of each entry. + /// + /// Returns `u32::MAX` if size overflows `u32` limits. + pub fn encoded_size_hint_u32(relayers_entries: usize, messages_count: usize) -> u32 + where + RelayerId: MaxEncodedLen, + { + Self::encoded_size_hint(relayers_entries, messages_count) + .and_then(|x| u32::try_from(x).ok()) + .unwrap_or(u32::MAX) + } + /// Nonce of the last message that has been delivered to this (target) chain. pub fn last_delivered_nonce(&self) -> MessageNonce { self.relayers diff --git a/bridges/relays/lib-substrate-relay/src/messages_source.rs b/bridges/relays/lib-substrate-relay/src/messages_source.rs index 2daa41fc28405..6647376d93516 100644 --- a/bridges/relays/lib-substrate-relay/src/messages_source.rs +++ b/bridges/relays/lib-substrate-relay/src/messages_source.rs @@ -468,9 +468,7 @@ where fn prepare_dummy_messages_delivery_proof( ) -> SubstrateMessagesDeliveryProof { let single_message_confirmation_size = - bp_messages::InboundLaneData::<()>::encoded_size_hint(1, 1) - .and_then(|x| u32::try_from(x).ok()) - .unwrap_or(u32::MAX); + bp_messages::InboundLaneData::<()>::encoded_size_hint_u32(1, 1); let proof_size = TC::STORAGE_PROOF_OVERHEAD.saturating_add(single_message_confirmation_size); ( UnrewardedRelayersState {