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 <[email protected]>

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:/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:/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:/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:/paritytech/parity-bridges-common/issues/391
- }
-
- fn weight_of_reward_confirmation_transaction_on_target_chain() -> Weight {
- 0 // TODO: https:/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:/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:/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:/paritytech/parity-bridges-common/issues/391
- }
-
- fn weight_of_reward_confirmation_transaction_on_target_chain() -> Weight {
- 0 // TODO: https:/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));