From 6b3ba67798fc94f2253b1ed9cffbdb3abbdf4f21 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 22 Feb 2023 14:16:02 +0300 Subject: [PATCH] get rid of ChainWithMessages::WeightInfo, because we can't have exact weights for "external chains" (#1899) --- bridges/modules/messages/src/lib.rs | 1 + .../client-bridge-hub-rococo/src/lib.rs | 3 - .../client-bridge-hub-wococo/src/lib.rs | 3 - bridges/relays/client-millau/src/lib.rs | 1 - .../relays/client-rialto-parachain/src/lib.rs | 1 - bridges/relays/client-rialto/src/lib.rs | 1 - bridges/relays/client-substrate/src/chain.rs | 3 - bridges/relays/client-substrate/src/client.rs | 50 +++--- .../lib-substrate-relay/src/messages_lane.rs | 147 ++++++++++++------ 9 files changed, 121 insertions(+), 89 deletions(-) diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index cc9033d105812..68af302444561 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -43,6 +43,7 @@ pub use weights::WeightInfo; pub use weights_ext::{ ensure_able_to_receive_confirmation, ensure_able_to_receive_message, ensure_weights_are_correct, WeightInfoExt, EXPECTED_DEFAULT_MESSAGE_LENGTH, + EXTRA_STORAGE_PROOF_SIZE, }; use crate::{ diff --git a/bridges/relays/client-bridge-hub-rococo/src/lib.rs b/bridges/relays/client-bridge-hub-rococo/src/lib.rs index b6b844021e0c0..c8963f968d002 100644 --- a/bridges/relays/client-bridge-hub-rococo/src/lib.rs +++ b/bridges/relays/client-bridge-hub-rococo/src/lib.rs @@ -132,9 +132,6 @@ impl ChainWithMessages for BridgeHubRococo { bp_bridge_hub_rococo::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX; const MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX: MessageNonce = bp_bridge_hub_rococo::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX; - - // TODO: fix (https://github.com/paritytech/parity-bridges-common/issues/1640) - type WeightInfo = (); } #[cfg(test)] diff --git a/bridges/relays/client-bridge-hub-wococo/src/lib.rs b/bridges/relays/client-bridge-hub-wococo/src/lib.rs index a466df3549510..7f23c0d93d2af 100644 --- a/bridges/relays/client-bridge-hub-wococo/src/lib.rs +++ b/bridges/relays/client-bridge-hub-wococo/src/lib.rs @@ -132,9 +132,6 @@ impl ChainWithMessages for BridgeHubWococo { bp_bridge_hub_wococo::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX; const MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX: MessageNonce = bp_bridge_hub_wococo::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX; - - // TODO: fix (https://github.com/paritytech/parity-bridges-common/issues/1640) - type WeightInfo = (); } #[cfg(test)] diff --git a/bridges/relays/client-millau/src/lib.rs b/bridges/relays/client-millau/src/lib.rs index 4c4c1370a6a56..9df21b1d503e6 100644 --- a/bridges/relays/client-millau/src/lib.rs +++ b/bridges/relays/client-millau/src/lib.rs @@ -51,7 +51,6 @@ impl ChainWithMessages for Millau { bp_millau::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX; const MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX: MessageNonce = bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX; - type WeightInfo = (); } impl Chain for Millau { diff --git a/bridges/relays/client-rialto-parachain/src/lib.rs b/bridges/relays/client-rialto-parachain/src/lib.rs index 2c3792725c389..654848da2a736 100644 --- a/bridges/relays/client-rialto-parachain/src/lib.rs +++ b/bridges/relays/client-rialto-parachain/src/lib.rs @@ -79,7 +79,6 @@ impl ChainWithMessages for RialtoParachain { bp_rialto_parachain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX; const MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX: MessageNonce = bp_rialto_parachain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX; - type WeightInfo = (); } impl ChainWithTransactions for RialtoParachain { diff --git a/bridges/relays/client-rialto/src/lib.rs b/bridges/relays/client-rialto/src/lib.rs index 39cc2721ddbf5..b822dfe7d89f3 100644 --- a/bridges/relays/client-rialto/src/lib.rs +++ b/bridges/relays/client-rialto/src/lib.rs @@ -69,7 +69,6 @@ impl ChainWithMessages for Rialto { bp_rialto::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX; const MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX: MessageNonce = bp_rialto::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX; - type WeightInfo = (); } impl ChainWithBalances for Rialto { diff --git a/bridges/relays/client-substrate/src/chain.rs b/bridges/relays/client-substrate/src/chain.rs index ebd9e7172d895..b55ae71cbf87f 100644 --- a/bridges/relays/client-substrate/src/chain.rs +++ b/bridges/relays/client-substrate/src/chain.rs @@ -136,9 +136,6 @@ pub trait ChainWithMessages: Chain { /// Maximal number of unconfirmed messages in a single confirmation transaction at this /// `ChainWithMessages`. const MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX: MessageNonce; - - /// Weights of message pallet calls. - type WeightInfo: pallet_bridge_messages::WeightInfoExt; } /// Call type used by the chain. diff --git a/bridges/relays/client-substrate/src/client.rs b/bridges/relays/client-substrate/src/client.rs index 1a4aeb583cd41..bddd53b39f8c4 100644 --- a/bridges/relays/client-substrate/src/client.rs +++ b/bridges/relays/client-substrate/src/client.rs @@ -21,7 +21,6 @@ use crate::{ rpc::{ SubstrateAuthorClient, SubstrateChainClient, SubstrateFinalityClient, SubstrateFrameSystemClient, SubstrateStateClient, SubstrateSystemClient, - SubstrateTransactionPaymentClient, }, transaction_stall_timeout, AccountKeyPairOf, ConnectionParams, Error, HashOf, HeaderIdOf, Result, SignParam, TransactionTracker, UnsignedTransaction, @@ -31,15 +30,16 @@ use async_std::sync::{Arc, Mutex}; use async_trait::async_trait; use bp_runtime::{HeaderIdProvider, StorageDoubleMapKeyProvider, StorageMapKeyProvider}; use codec::{Decode, Encode}; +use frame_support::weights::Weight; use frame_system::AccountInfo; use futures::{SinkExt, StreamExt}; use jsonrpsee::{ core::DeserializeOwned, ws_client::{WsClient as RpcClient, WsClientBuilder as RpcClientBuilder}, }; -use num_traits::{Bounded, Saturating, Zero}; +use num_traits::{Saturating, Zero}; use pallet_balances::AccountData; -use pallet_transaction_payment::InclusionFee; +use pallet_transaction_payment::RuntimeDispatchInfo; use relay_utils::{relay_loop::RECONNECT_DELAY, STALL_TIMEOUT}; use sp_core::{ storage::{StorageData, StorageKey}, @@ -51,10 +51,11 @@ use sp_runtime::{ }; use sp_trie::StorageProof; use sp_version::RuntimeVersion; -use std::{convert::TryFrom, future::Future}; +use std::future::Future; const SUB_API_GRANDPA_AUTHORITIES: &str = "GrandpaApi_grandpa_authorities"; const SUB_API_TXPOOL_VALIDATE_TRANSACTION: &str = "TaggedTransactionQueue_validate_transaction"; +const SUB_API_TX_PAYMENT_QUERY_INFO: &str = "TransactionPaymentApi_query_info"; const MAX_SUBSCRIPTION_CAPACITY: usize = 4096; /// The difference between best block number and number of its ancestor, that is enough @@ -591,33 +592,24 @@ impl Client { .await } - /// Estimate fee that will be spent on given extrinsic. - pub async fn estimate_extrinsic_fee( + /// Returns weight of the given transaction. + pub async fn extimate_extrinsic_weight( &self, - transaction: Bytes, - ) -> Result> { + transaction: SignedTransaction, + ) -> Result { self.jsonrpsee_execute(move |client| async move { - let fee_details = - SubstrateTransactionPaymentClient::::fee_details(&*client, transaction, None) - .await?; - let inclusion_fee = fee_details - .inclusion_fee - .map(|inclusion_fee| InclusionFee { - base_fee: C::Balance::try_from(inclusion_fee.base_fee.into_u256()) - .unwrap_or_else(|_| C::Balance::max_value()), - len_fee: C::Balance::try_from(inclusion_fee.len_fee.into_u256()) - .unwrap_or_else(|_| C::Balance::max_value()), - adjusted_weight_fee: C::Balance::try_from( - inclusion_fee.adjusted_weight_fee.into_u256(), - ) - .unwrap_or_else(|_| C::Balance::max_value()), - }) - .unwrap_or_else(|| InclusionFee { - base_fee: Zero::zero(), - len_fee: Zero::zero(), - adjusted_weight_fee: Zero::zero(), - }); - Ok(inclusion_fee) + let transaction_len = transaction.encoded_size() as u32; + + let call = SUB_API_TX_PAYMENT_QUERY_INFO.to_string(); + let data = Bytes((transaction, transaction_len).encode()); + + let encoded_response = + SubstrateStateClient::::call(&*client, call, data, None).await?; + let dispatch_info = + RuntimeDispatchInfo::::decode(&mut &encoded_response.0[..]) + .map_err(Error::ResponseParseFailed)?; + + Ok(dispatch_info.weight) }) .await } diff --git a/bridges/relays/lib-substrate-relay/src/messages_lane.rs b/bridges/relays/lib-substrate-relay/src/messages_lane.rs index 1a4f65ff51e10..0a7a3566d20f1 100644 --- a/bridges/relays/lib-substrate-relay/src/messages_lane.rs +++ b/bridges/relays/lib-substrate-relay/src/messages_lane.rs @@ -25,7 +25,9 @@ use crate::{ use async_std::sync::Arc; use bp_messages::{LaneId, MessageNonce}; -use bp_runtime::{AccountIdOf, Chain as _, HeaderIdOf, WeightExtraOps}; +use bp_runtime::{ + AccountIdOf, Chain as _, EncodedOrDecodedCall, HeaderIdOf, TransactionEra, WeightExtraOps, +}; use bridge_runtime_common::messages::{ source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof, }; @@ -35,13 +37,15 @@ use messages_relay::{message_lane::MessageLane, message_lane_loop::BatchTransact use pallet_bridge_messages::{Call as BridgeMessagesCall, Config as BridgeMessagesConfig}; use relay_substrate_client::{ transaction_stall_timeout, AccountKeyPairOf, BalanceOf, BlockNumberOf, CallOf, Chain, - ChainWithMessages, ChainWithTransactions, Client, Error as SubstrateError, HashOf, + ChainWithMessages, ChainWithTransactions, Client, Error as SubstrateError, HashOf, SignParam, + UnsignedTransaction, }; use relay_utils::{ metrics::{GlobalMetrics, MetricsParams, StandaloneMetric}, STALL_TIMEOUT, }; use sp_core::Pair; +use sp_runtime::traits::Zero; use std::{convert::TryFrom, fmt::Debug, marker::PhantomData}; /// Substrate -> Substrate messages synchronization pipeline. @@ -159,25 +163,25 @@ where AccountIdOf: From< as Pair>::Public>, BalanceOf: TryFrom>, { - let source_client = params.source_client; - let target_client = params.target_client; - let relayer_id_at_source: AccountIdOf = - params.source_transaction_params.signer.public().into(); - // 2/3 is reserved for proofs and tx overhead let max_messages_size_in_single_batch = P::TargetChain::max_extrinsic_size() / 3; // we don't know exact weights of the Polkadot runtime. So to guess weights we'll be using // weights from Rialto and then simply dividing it by x2. let (max_messages_in_single_batch, max_messages_weight_in_single_batch) = - crate::messages_lane::select_delivery_transaction_limits::< - ::WeightInfo, - >( + select_delivery_transaction_limits_rpc::

( + ¶ms, P::TargetChain::max_extrinsic_weight(), P::SourceChain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX, - ); + ) + .await?; let (max_messages_in_single_batch, max_messages_weight_in_single_batch) = (max_messages_in_single_batch / 2, max_messages_weight_in_single_batch / 2); + let source_client = params.source_client; + let target_client = params.target_client; + let relayer_id_at_source: AccountIdOf = + params.source_transaction_params.signer.public().into(); + log::info!( target: "bridge", "Starting {} -> {} messages relay.\n\t\ @@ -437,12 +441,15 @@ macro_rules! generate_receive_message_delivery_proof_call_builder { }; } -/// Returns maximal number of messages and their maximal cumulative dispatch weight, based -/// on given chain parameters. -pub fn select_delivery_transaction_limits( +/// Returns maximal number of messages and their maximal cumulative dispatch weight. +async fn select_delivery_transaction_limits_rpc( + params: &MessagesRelayParams

, max_extrinsic_weight: Weight, max_unconfirmed_messages_at_inbound_lane: MessageNonce, -) -> (MessageNonce, Weight) { +) -> anyhow::Result<(MessageNonce, Weight)> +where + AccountIdOf: From< as Pair>::Public>, +{ // We may try to guess accurate value, based on maximal number of messages and per-message // weight overhead, but the relay loop isn't using this info in a super-accurate way anyway. // So just a rough guess: let's say 1/3 of max tx weight is for tx itself and the rest is @@ -455,13 +462,35 @@ pub fn select_delivery_transaction_limits(params, 0)?; + let delivery_tx_with_zero_messages_weight = params + .target_client + .extimate_extrinsic_weight(delivery_tx_with_zero_messages) + .await + .map_err(|e| { + anyhow::format_err!("Failed to estimate delivery extrinsic weight: {:?}", e) + })?; + + // weight of single message delivery with outbound lane state + let delivery_tx_with_one_message = dummy_messages_delivery_transaction::

(params, 1)?; + let delivery_tx_with_one_message_weight = params + .target_client + .extimate_extrinsic_weight(delivery_tx_with_one_message) + .await + .map_err(|e| { + anyhow::format_err!("Failed to estimate delivery extrinsic weight: {:?}", e) + })?; + + // message overhead is roughly `delivery_tx_with_one_message_weight - + // delivery_tx_with_zero_messages_weight` + let delivery_tx_weight_rest = weight_for_delivery_tx - delivery_tx_with_zero_messages_weight; + let delivery_tx_message_overhead = + delivery_tx_with_one_message_weight.saturating_sub(delivery_tx_with_zero_messages_weight); let max_number_of_messages = std::cmp::min( delivery_tx_weight_rest - .min_components_checked_div(W::receive_messages_proof_messages_overhead(1)) + .min_components_checked_div(delivery_tx_message_overhead) .unwrap_or(u64::MAX), max_unconfirmed_messages_at_inbound_lane, ); @@ -475,36 +504,58 @@ pub fn select_delivery_transaction_limits; - - #[test] - fn select_delivery_transaction_limits_is_sane() { - // we want to check the `proof_size` component here too. But for Rialto and Millau - // it is set to `u64::MAX` (as for Polkadot and other relay/standalone chains). - // So let's use RialtoParachain limits here - it has `proof_size` limit as all - // Cumulus-based parachains do. - let (max_count, max_weight) = - select_delivery_transaction_limits::( - bp_rialto_parachain::RialtoParachain::max_extrinsic_weight(), - bp_rialto::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX, - ); - assert_eq!( - (max_count, max_weight), - // We don't actually care about these values, so feel free to update them whenever test - // fails. The only thing to do before that is to ensure that new values looks sane: - // i.e. weight reserved for messages dispatch allows dispatch of non-trivial messages. - // - // Any significant change in this values should attract additional attention. - (1024, Weight::from_parts(866_600_106_667, 2_271_915)), +/// Returns dummy message delivery transaction with zero messages and `1kb` proof. +fn dummy_messages_delivery_transaction( + params: &MessagesRelayParams

, + messages: u32, +) -> anyhow::Result<::SignedTransaction> +where + AccountIdOf: From< as Pair>::Public>, +{ + // we don't care about any call values here, because all that the estimation RPC does + // is calls `GetDispatchInfo::get_dispatch_info` for the wrapped call. So we only are + // interested in values that affect call weight - e.g. number of messages and the + // storage proof size + + let dummy_messages_delivery_call = + P::ReceiveMessagesProofCallBuilder::build_receive_messages_proof_call( + params.source_transaction_params.signer.public().into(), + ( + Weight::zero(), + FromBridgedChainMessagesProof { + bridged_header_hash: Default::default(), + // we may use per-chain `EXTRA_STORAGE_PROOF_SIZE`, but since we don't need + // exact values, this global estimation is fine + storage_proof: vec![vec![ + 42u8; + pallet_bridge_messages::EXTRA_STORAGE_PROOF_SIZE + as usize + ]], + lane: Default::default(), + nonces_start: 1, + nonces_end: messages as u64, + }, + ), + messages, + Weight::zero(), + false, ); - } + P::TargetChain::sign_transaction( + SignParam { + spec_version: 0, + transaction_version: 0, + genesis_hash: Default::default(), + signer: params.target_transaction_params.signer.clone(), + }, + UnsignedTransaction { + call: EncodedOrDecodedCall::Decoded(dummy_messages_delivery_call), + nonce: Zero::zero(), + tip: Zero::zero(), + era: TransactionEra::Immortal, + }, + ) + .map_err(Into::into) }