Follow-up on paritytech#1487 (paritytech#1511)

* Remove unused trait implementations Signed-off-by: Serban Iorga <serban@parity.io> * Define encoded_size_hint_u32() Signed-off-by: Serban Iorga <serban@parity.io> * Define TransactionEstimationParams trait Signed-off-by: Serban Iorga <serban@parity.io> * Rework TransactionEstimation Signed-off-by: Serban Iorga <serban@parity.io> * Docs + Renamings Signed-off-by: Serban Iorga <serban@parity.io>
serban300 · Apr 10, 2024 · a6070bc · a6070bc
1 parent 592b820
commit a6070bc
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 101 deletions.
diff --git 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<Weight> {
-		let inbound_data_size = InboundLaneData::<bp_millau::AccountId>::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<Weight>) -> 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::<Weights>(
 			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
@@ -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<Weight> {
-		let inbound_data_size = InboundLaneData::<bp_millau::AccountId>::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<Weight>) -> 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
@@ -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<Weight> {
-		let inbound_data_size =
-			InboundLaneData::<bp_rialto_parachain::AccountId>::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<Weight>,
 	) -> bp_rialto_parachain::Balance {

diff --git 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<Weight> {
-		let inbound_data_size = InboundLaneData::<bp_rialto::AccountId>::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<Weight>) -> 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::<Weights>(
 			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
@@ -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<Weight> {
 	pub size: u32,
 }
 
+/// Helper trait for estimating the size and weight of a single message delivery confirmation
+/// transaction.
+pub trait ConfirmationTransactionEstimation<Weight> {
+	// Estimate size and weight of single message delivery confirmation transaction.
+	fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight>;
+}
+
+/// 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<AccountId>);
+
+impl<
+		AccountId: MaxEncodedLen,
+		const MAX_CONFIRMATION_TX_WEIGHT: Weight,
+		const EXTRA_STORAGE_PROOF_SIZE: u32,
+		const TX_EXTRA_BYTES: u32,
+	> ConfirmationTransactionEstimation<Weight>
+	for BasicConfirmationTransactionEstimation<
+		AccountId,
+		MAX_CONFIRMATION_TX_WEIGHT,
+		EXTRA_STORAGE_PROOF_SIZE,
+		TX_EXTRA_BYTES,
+	>
+{
+	fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight> {
+		let inbound_data_size = InboundLaneData::<AccountId>::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<WeightOf<Self>>;
 
 	/// 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<WeightOf<Self>>;
+	fn estimate_delivery_confirmation_transaction() -> MessageTransaction<WeightOf<Self>> {
+		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<WeightOf<Self>>) -> BalanceOf<Self>;
@@ -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<
+			<ThisChain as ChainWithMessages>::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<WeightOf<Self>> {
-			MessageTransaction {
-				dispatch_weight: DELIVERY_CONFIRMATION_TRANSACTION_WEIGHT,
-				size: 0,
-			}
-		}
-
 		fn transaction_payment(transaction: MessageTransaction<WeightOf<Self>>) -> BalanceOf<Self> {
 			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<
+			<BridgedChain as ChainWithMessages>::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<WeightOf<Self>> {
-			unreachable!()
-		}
-
 		fn transaction_payment(
 			_transaction: MessageTransaction<WeightOf<Self>>,
 		) -> BalanceOf<Self> {

diff --git a/bridges/modules/messages/src/inbound_lane.rs b/bridges/modules/messages/src/inbound_lane.rs
@@ -79,14 +79,6 @@ impl<T: Config<I>, I: 'static> Default for StoredInboundLaneData<T, I> {
 	}
 }
 
-impl<T: Config<I>, I: 'static> From<InboundLaneData<T::InboundRelayer>>
-	for StoredInboundLaneData<T, I>
-{
-	fn from(data: InboundLaneData<T::InboundRelayer>) -> Self {
-		StoredInboundLaneData(data)
-	}
-}
-
 impl<T: Config<I>, I: 'static> From<StoredInboundLaneData<T, I>>
 	for InboundLaneData<T::InboundRelayer>
 {

diff --git a/bridges/modules/messages/src/outbound_lane.rs b/bridges/modules/messages/src/outbound_lane.rs
@@ -72,14 +72,6 @@ impl<T: Config<I>, I: 'static> sp_std::ops::DerefMut for StoredMessageData<T, I>
 	}
 }
 
-impl<T: Config<I>, I: 'static> From<MessageData<T::OutboundMessageFee>>
-	for StoredMessageData<T, I>
-{
-	fn from(data: MessageData<T::OutboundMessageFee>) -> Self {
-		StoredMessageData(data)
-	}
-}
-
 impl<T: Config<I>, I: 'static> From<StoredMessageData<T, I>>
 	for MessageData<T::OutboundMessageFee>
 {