Use real weights to compute message delivery and dispatch fee (parity…

…tech#598)

* message fee formula

* update GetDelvieryConfirmationTransactionFee

* include cost of transactions (i.e. not only dispatch cost) in delivery_and_dispatch_fee

* endow relayers fund account

* include db ops weight in max tx weight estimation

* (in bytes)

Co-authored-by: Hernando Castano <castano.ha@gmail.com>

bridges/bin/millau/node/Cargo.toml

@@ -19,6 +19,7 @@ bp-message-lane = { path = "../../../primitives/message-lane" }
 bp-millau= { path = "../../../primitives/millau" }
 bp-runtime = { path = "../../../primitives/runtime" }
 millau-runtime = { path = "../runtime" }
+pallet-message-lane = { path = "../../../modules/message-lane" }
 pallet-message-lane-rpc = { path = "../../../modules/message-lane/rpc" }
 
 # Substrate Dependencies

bridges/bin/millau/node/src/chain_spec.rs

@@ -122,6 +122,7 @@ impl Alternative {
 							get_account_id_from_seed::<sr25519::Public>("Ferdie//stash"),
 							get_account_id_from_seed::<sr25519::Public>("George//stash"),
 							get_account_id_from_seed::<sr25519::Public>("Harry//stash"),
+							pallet_message_lane::Module::<millau_runtime::Runtime, pallet_message_lane::DefaultInstance>::relayer_fund_account_id(),
 							derive_account_from_rialto_id(bp_runtime::SourceAccount::Account(
 								get_account_id_from_seed::<sr25519::Public>("Dave"),
 							)),

bridges/bin/millau/runtime/src/lib.rs

@@ -315,8 +315,9 @@ parameter_types! {
 		bp_millau::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE;
 	pub const MaxUnconfirmedMessagesAtInboundLane: bp_message_lane::MessageNonce =
 		bp_millau::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE;
-	// TODO: https://github.com/paritytech/parity-bridges-common/pull/598
-	pub GetDeliveryConfirmationTransactionFee: Balance = 0;
+	// `IdentityFee` is used by Millau => we may use weight directly
+	pub const GetDeliveryConfirmationTransactionFee: Balance =
+		bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT as _;
 	pub const RootAccountForPayments: Option<AccountId> = None;
 }
 
@@ -594,3 +595,17 @@ impl_runtime_apis! {
 		}
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn ensure_millau_message_lane_weights_are_correct() {
+		// TODO: https://github.com/paritytech/parity-bridges-common/issues/390
+		pallet_message_lane::ensure_weights_are_correct::<pallet_message_lane::weights::RialtoWeight<Runtime>>(
+			bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_TX_WEIGHT,
+			bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT,
+		);
+	}
+}

bridges/bin/millau/runtime/src/rialto_messages.rs

@@ -26,7 +26,7 @@ use bp_message_lane::{
 use bp_runtime::{InstanceId, RIALTO_BRIDGE_INSTANCE};
 use bridge_runtime_common::messages::{self, ChainWithMessageLanes, MessageBridge};
 use frame_support::{
-	weights::{Weight, WeightToFeePolynomial},
+	weights::{DispatchClass, Weight, WeightToFeePolynomial},
 	RuntimeDebug,
 };
 use sp_core::storage::StorageKey;
@@ -98,21 +98,35 @@ impl MessageBridge for WithRialtoMessageBridge {
 
 		// given Rialto chain parameters (`TransactionByteFee`, `WeightToFee`, `FeeMultiplierUpdate`),
 		// the minimal weight of the message may be computed as message.length()
-		let lower_limit = Weight::try_from(message_payload.len()).unwrap_or(Weight::MAX);
+		let lower_limit = u32::try_from(message_payload.len())
+			.map(Into::into)
+			.unwrap_or(Weight::MAX);
 
 		lower_limit..=upper_limit
 	}
 
-	fn weight_of_delivery_transaction() -> Weight {
-		0 // TODO: https://github.com/paritytech/parity-bridges-common/issues/391
+	fn weight_of_delivery_transaction(message_payload: &[u8]) -> Weight {
+		messages::transaction_weight_without_multiplier(
+			bp_millau::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic,
+			u32::try_from(message_payload.len())
+				.map(Into::into)
+				.unwrap_or(Weight::MAX)
+				.saturating_add(bp_millau::EXTRA_STORAGE_PROOF_SIZE as _),
+			bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_TX_WEIGHT,
+		)
 	}
 
 	fn weight_of_delivery_confirmation_transaction_on_this_chain() -> Weight {
-		0 // TODO: https://github.com/paritytech/parity-bridges-common/issues/391
-	}
-
-	fn weight_of_reward_confirmation_transaction_on_target_chain() -> Weight {
-		0 // TODO: https://github.com/paritytech/parity-bridges-common/issues/391
+		let inbounded_data_size: Weight =
+			InboundLaneData::<bp_rialto::AccountId>::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 {

bridges/bin/rialto/node/Cargo.toml

@@ -18,6 +18,7 @@ structopt = "0.3.21"
 bp-message-lane = { path = "../../../primitives/message-lane" }
 bp-runtime = { path = "../../../primitives/runtime" }
 bp-rialto = { path = "../../../primitives/rialto" }
+pallet-message-lane = { path = "../../../modules/message-lane" }
 pallet-message-lane-rpc = { path = "../../../modules/message-lane/rpc" }
 rialto-runtime = { path = "../runtime" }
 

bridges/bin/rialto/node/src/chain_spec.rs

@@ -122,6 +122,7 @@ impl Alternative {
 							get_account_id_from_seed::<sr25519::Public>("Ferdie//stash"),
 							get_account_id_from_seed::<sr25519::Public>("George//stash"),
 							get_account_id_from_seed::<sr25519::Public>("Harry//stash"),
+							pallet_message_lane::Module::<rialto_runtime::Runtime, pallet_message_lane::DefaultInstance>::relayer_fund_account_id(),
 							derive_account_from_millau_id(bp_runtime::SourceAccount::Account(
 								get_account_id_from_seed::<sr25519::Public>("Dave"),
 							)),

bridges/bin/rialto/runtime/src/lib.rs

@@ -419,11 +419,12 @@ impl pallet_shift_session_manager::Config for Runtime {}
 parameter_types! {
 	pub const MaxMessagesToPruneAtOnce: bp_message_lane::MessageNonce = 8;
 	pub const MaxUnrewardedRelayerEntriesAtInboundLane: bp_message_lane::MessageNonce =
-		bp_millau::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE;
+		bp_rialto::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE;
 	pub const MaxUnconfirmedMessagesAtInboundLane: bp_message_lane::MessageNonce =
 		bp_rialto::MAX_UNCONFIRMED_MESSAGES_AT_INBOUND_LANE;
-	// TODO: https://github.com/paritytech/parity-bridges-common/pull/598
-	pub GetDeliveryConfirmationTransactionFee: Balance = 0;
+	// `IdentityFee` is used by Rialto => we may use weight directly
+	pub const GetDeliveryConfirmationTransactionFee: Balance =
+		bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT as _;
 	pub const RootAccountForPayments: Option<AccountId> = None;
 }
 
@@ -1026,7 +1027,10 @@ mod tests {
 
 	#[test]
 	fn ensure_rialto_message_lane_weights_are_correct() {
-		pallet_message_lane::ensure_weights_are_correct::<pallet_message_lane::weights::RialtoWeight<Runtime>>();
+		pallet_message_lane::ensure_weights_are_correct::<pallet_message_lane::weights::RialtoWeight<Runtime>>(
+			bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_TX_WEIGHT,
+			bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT,
+		);
 	}
 
 	#[test]

bridges/bin/rialto/runtime/src/millau_messages.rs

@@ -26,7 +26,7 @@ use bp_message_lane::{
 use bp_runtime::{InstanceId, MILLAU_BRIDGE_INSTANCE};
 use bridge_runtime_common::messages::{self, ChainWithMessageLanes, MessageBridge};
 use frame_support::{
-	weights::{Weight, WeightToFeePolynomial},
+	weights::{DispatchClass, Weight, WeightToFeePolynomial},
 	RuntimeDebug,
 };
 use sp_core::storage::StorageKey;
@@ -99,21 +99,35 @@ impl MessageBridge for WithMillauMessageBridge {
 
 		// given Millau chain parameters (`TransactionByteFee`, `WeightToFee`, `FeeMultiplierUpdate`),
 		// the minimal weight of the message may be computed as message.length()
-		let lower_limit = Weight::try_from(message_payload.len()).unwrap_or(Weight::MAX);
+		let lower_limit = u32::try_from(message_payload.len())
+			.map(Into::into)
+			.unwrap_or(Weight::MAX);
 
 		lower_limit..=upper_limit
 	}
 
-	fn weight_of_delivery_transaction() -> Weight {
-		0 // TODO: https://github.com/paritytech/parity-bridges-common/issues/391
+	fn weight_of_delivery_transaction(message_payload: &[u8]) -> Weight {
+		messages::transaction_weight_without_multiplier(
+			bp_millau::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic,
+			u32::try_from(message_payload.len())
+				.map(Into::into)
+				.unwrap_or(Weight::MAX)
+				.saturating_add(bp_rialto::EXTRA_STORAGE_PROOF_SIZE as _),
+			bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_TX_WEIGHT,
+		)
 	}
 
 	fn weight_of_delivery_confirmation_transaction_on_this_chain() -> Weight {
-		0 // TODO: https://github.com/paritytech/parity-bridges-common/issues/391
-	}
-
-	fn weight_of_reward_confirmation_transaction_on_target_chain() -> Weight {
-		0 // TODO: https://github.com/paritytech/parity-bridges-common/issues/391
+		let inbounded_data_size: Weight =
+			InboundLaneData::<bp_millau::AccountId>::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 {

bridges/bin/runtime-common/src/messages.rs

@@ -28,7 +28,7 @@ use bp_message_lane::{
 };
 use bp_runtime::InstanceId;
 use codec::{Compact, Decode, Encode, Input};
-use frame_support::{traits::Instance, RuntimeDebug};
+use frame_support::{traits::Instance, weights::Weight, RuntimeDebug};
 use hash_db::Hasher;
 use pallet_substrate_bridge::StorageProofChecker;
 use sp_runtime::traits::{CheckedAdd, CheckedDiv, CheckedMul};
@@ -65,16 +65,11 @@ pub trait MessageBridge {
 	) -> RangeInclusive<WeightOf<BridgedChain<Self>>>;
 
 	/// Maximal weight of single message delivery transaction on Bridged chain.
-	fn weight_of_delivery_transaction() -> WeightOf<BridgedChain<Self>>;
+	fn weight_of_delivery_transaction(message_payload: &[u8]) -> WeightOf<BridgedChain<Self>>;
 
 	/// Maximal weight of single message delivery confirmation transaction on This chain.
 	fn weight_of_delivery_confirmation_transaction_on_this_chain() -> WeightOf<ThisChain<Self>>;
 
-	/// Weight of single message reward confirmation on the Bridged chain. This confirmation
-	/// is a part of delivery transaction, so this weight is added to the delivery
-	/// transaction weight.
-	fn weight_of_reward_confirmation_transaction_on_target_chain() -> WeightOf<BridgedChain<Self>>;
-
 	/// Convert weight of This chain to the fee (paid in Balance) of This chain.
 	fn this_weight_to_this_balance(weight: WeightOf<ThisChain<Self>>) -> BalanceOf<ThisChain<Self>>;
 
@@ -120,6 +115,31 @@ pub(crate) type BalanceOf<C> = <C as ChainWithMessageLanes>::Balance;
 pub(crate) type CallOf<C> = <C as ChainWithMessageLanes>::Call;
 pub(crate) type MessageLaneInstanceOf<C> = <C as ChainWithMessageLanes>::MessageLaneInstance;
 
+/// Compute weight 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;
+
+	// we assume that adjustable per-byte weight is always zero
+	let adjusted_per_byte_weight = 0;
+
+	// we assume that transaction tip we use is also zero
+	let transaction_tip_weight = 0;
+
+	base_weight
+		.saturating_add(per_byte_weight)
+		.saturating_add(adjusted_per_byte_weight)
+		.saturating_add(transaction_tip_weight)
+		.saturating_add(dispatch_weight)
+}
+
 /// Sub-module that is declaring types required for processing This -> Bridged chain messages.
 pub mod source {
 	use super::*;
@@ -226,10 +246,8 @@ pub mod source {
 		relayer_fee_percent: u32,
 	) -> Result<BalanceOf<ThisChain<B>>, &'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());
+		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 reward_confirmation_fee =
-			B::bridged_weight_to_bridged_balance(B::weight_of_reward_confirmation_transaction_on_target_chain());
 
 		// the fee (in This tokens) of all transactions that are made on This chain
 		let delivery_confirmation_fee =
@@ -238,7 +256,6 @@ pub mod source {
 		// minimal fee (in This tokens) is a sum of all required fees
 		let minimal_fee = delivery_fee
 			.checked_add(&dispatch_fee)
-			.and_then(|fee| fee.checked_add(&reward_confirmation_fee))
 			.map(B::bridged_balance_to_this_balance)
 			.and_then(|fee| fee.checked_add(&delivery_confirmation_fee));
 
@@ -570,7 +587,6 @@ mod tests {
 
 	const DELIVERY_TRANSACTION_WEIGHT: Weight = 100;
 	const DELIVERY_CONFIRMATION_TRANSACTION_WEIGHT: Weight = 100;
-	const REWARD_CONFIRMATION_TRANSACTION_WEIGHT: Weight = 100;
 	const THIS_CHAIN_WEIGHT_TO_BALANCE_RATE: Weight = 2;
 	const BRIDGED_CHAIN_WEIGHT_TO_BALANCE_RATE: Weight = 4;
 	const BRIDGED_CHAIN_TO_THIS_CHAIN_BALANCE_RATE: u32 = 6;
@@ -596,18 +612,14 @@ mod tests {
 			begin..=BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT
 		}
 
-		fn weight_of_delivery_transaction() -> 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 weight_of_reward_confirmation_transaction_on_target_chain() -> Weight {
-			REWARD_CONFIRMATION_TRANSACTION_WEIGHT
-		}
-
 		fn this_weight_to_this_balance(weight: Weight) -> ThisChainBalance {
 			ThisChainBalance(weight as u32 * THIS_CHAIN_WEIGHT_TO_BALANCE_RATE as u32)
 		}
@@ -639,18 +651,14 @@ mod tests {
 			unreachable!()
 		}
 
-		fn weight_of_delivery_transaction() -> Weight {
+		fn weight_of_delivery_transaction(_message_payload: &[u8]) -> Weight {
 			unreachable!()
 		}
 
 		fn weight_of_delivery_confirmation_transaction_on_this_chain() -> Weight {
 			unreachable!()
 		}
 
-		fn weight_of_reward_confirmation_transaction_on_target_chain() -> Weight {
-			unreachable!()
-		}
-
 		fn this_weight_to_this_balance(_weight: Weight) -> BridgedChainBalance {
 			unreachable!()
 		}
@@ -809,7 +817,7 @@ mod tests {
 
 	#[test]
 	fn message_fee_is_checked_by_verifier() {
-		const EXPECTED_MINIMAL_FEE: u32 = 8140;
+		const EXPECTED_MINIMAL_FEE: u32 = 5500;
 
 		// payload of the This -> Bridged chain message
 		let payload = source::FromThisChainMessagePayload::<OnThisChainBridge> {

bridges/modules/message-lane/rpc/Cargo.toml

@@ -12,6 +12,7 @@ futures = { version = "0.3.5", features = ["compat"] }
 jsonrpc-core = "15.1.0"
 jsonrpc-core-client = "15.1.0"
 jsonrpc-derive = "15.1.0"
+log = "0.4.11"
 
 # Bridge dependencies
 

bridges/modules/message-lane/rpc/src/lib.rs

@@ -116,6 +116,7 @@ where
 		} else {
 			None
 		};
+		let messages_count = if end >= begin { end - begin + 1 } else { 0 };
 		Box::new(
 			prove_keys_read(
 				self.backend.clone(),
@@ -126,7 +127,15 @@ where
 			)
 			.boxed()
 			.compat()
-			.map(serialize_storage_proof)
+			.map(move |proof| {
+				let serialized_proof = serialize_storage_proof(proof);
+				log::trace!(
+					"Generated proof of {} messages. Size: {}",
+					messages_count,
+					serialized_proof.len()
+				);
+				serialized_proof
+			})
 			.map_err(Into::into),
 		)
 	}
@@ -145,7 +154,11 @@ where
 			)
 			.boxed()
 			.compat()
-			.map(serialize_storage_proof)
+			.map(|proof| {
+				let serialized_proof = serialize_storage_proof(proof);
+				log::trace!("Generated message delivery proof. Size: {}", serialized_proof.len());
+				serialized_proof
+			})
 			.map_err(Into::into),
 		)
 	}

bridges/modules/message-lane/src/lib.rs

@@ -326,16 +326,19 @@ decl_module! {
 
 			// finally, save message in outbound storage and emit event
 			let mut lane = outbound_lane::<T, I>(lane_id);
+			let encoded_payload = payload.encode();
+			let encoded_payload_len = encoded_payload.len();
 			let nonce = lane.send_message(MessageData {
-				payload: payload.encode(),
+				payload: encoded_payload,
 				fee: delivery_and_dispatch_fee,
 			});
 			lane.prune_messages(T::MaxMessagesToPruneAtOnce::get());
 
 			frame_support::debug::trace!(
-				"Accepted message {} to lane {:?}",
+				"Accepted message {} to lane {:?}. Message size: {:?}",
 				nonce,
 				lane_id,
+				encoded_payload_len,
 			);
 
 			Self::deposit_event(RawEvent::MessageAccepted(lane_id, nonce));