From 35443b558fd2407b884953c5500b2120e22ae8ec Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 30 Aug 2022 13:58:37 +0300 Subject: [PATCH] Extract unsigned tx from the SignParam structure (#1561) * change sign_transaction method * clippy * rustup update && clippy * remove redudnant clone --- .../rialto-parachain/node/src/chain_spec.rs | 10 +- bridges/primitives/runtime/src/lib.rs | 10 +- bridges/primitives/test-utils/src/keyring.rs | 2 +- .../relays/bin-substrate/src/chains/mod.rs | 34 +++--- .../bin-substrate/src/cli/init_bridge.rs | 29 ++--- bridges/relays/bin-substrate/src/cli/mod.rs | 7 -- .../src/cli/register_parachain.rs | 78 ++++++------- .../src/cli/resubmit_transactions.rs | 36 +++--- .../bin-substrate/src/cli/send_message.rs | 79 +++++++------ bridges/relays/client-millau/src/lib.rs | 51 +++++---- .../relays/client-rialto-parachain/src/lib.rs | 17 +-- bridges/relays/client-rialto/src/lib.rs | 51 +++++---- bridges/relays/client-substrate/src/chain.rs | 28 +++-- bridges/relays/client-substrate/src/client.rs | 54 +++++---- .../finality/src/finality_loop_tests.rs | 10 +- .../src/conversion_rate_update.rs | 29 +++-- .../relays/lib-substrate-relay/src/error.rs | 2 +- .../src/finality/initialize.rs | 43 ++++++-- .../lib-substrate-relay/src/finality/mod.rs | 2 +- .../src/finality/target.rs | 22 ++-- bridges/relays/lib-substrate-relay/src/lib.rs | 11 -- .../lib-substrate-relay/src/messages_lane.rs | 4 +- .../src/messages_source.rs | 56 +++++----- .../src/messages_target.rs | 104 +++++++++--------- .../src/on_demand/headers.rs | 3 +- .../src/on_demand/parachains.rs | 2 +- .../src/parachains/target.rs | 23 ++-- .../relays/messages/src/message_lane_loop.rs | 30 ++--- bridges/relays/utils/src/lib.rs | 9 ++ 29 files changed, 434 insertions(+), 402 deletions(-) diff --git a/bridges/bin/rialto-parachain/node/src/chain_spec.rs b/bridges/bin/rialto-parachain/node/src/chain_spec.rs index 4a7865b18542e..a0306514c0391 100644 --- a/bridges/bin/rialto-parachain/node/src/chain_spec.rs +++ b/bridges/bin/rialto-parachain/node/src/chain_spec.rs @@ -122,10 +122,7 @@ pub fn development_config(id: ParaId) -> ChainSpec { move || { testnet_genesis( get_account_id_from_seed::(SUDO_ACCOUNT), - DEV_AUTHORITIES_ACCOUNTS - .into_iter() - .map(|x| get_from_seed::(x)) - .collect(), + DEV_AUTHORITIES_ACCOUNTS.into_iter().map(get_from_seed::).collect(), endowed_accounts(), id, ) @@ -157,10 +154,7 @@ pub fn local_testnet_config(id: ParaId) -> ChainSpec { move || { testnet_genesis( get_account_id_from_seed::(SUDO_ACCOUNT), - LOCAL_AUTHORITIES_ACCOUNTS - .into_iter() - .map(|x| get_from_seed::(x)) - .collect(), + LOCAL_AUTHORITIES_ACCOUNTS.into_iter().map(get_from_seed::).collect(), endowed_accounts(), id, ) diff --git a/bridges/primitives/runtime/src/lib.rs b/bridges/primitives/runtime/src/lib.rs index 7589e70d703ae..7ca17fbd5a178 100644 --- a/bridges/primitives/runtime/src/lib.rs +++ b/bridges/primitives/runtime/src/lib.rs @@ -183,7 +183,7 @@ impl Size for PreComputedSize { } /// Era of specific transaction. -#[derive(RuntimeDebug, Clone, Copy)] +#[derive(RuntimeDebug, Clone, Copy, PartialEq)] pub enum TransactionEra { /// Transaction is immortal. Immortal, @@ -207,6 +207,14 @@ impl, BlockHash: Copy> TransactionEra Option { + match *self { + TransactionEra::Immortal => None, + TransactionEra::Mortal(_, period) => Some(period), + } + } + /// Returns era that is used by FRAME-based runtimes. pub fn frame_era(&self) -> sp_runtime::generic::Era { match *self { diff --git a/bridges/primitives/test-utils/src/keyring.rs b/bridges/primitives/test-utils/src/keyring.rs index 2436d79339236..f827c729434ec 100644 --- a/bridges/primitives/test-utils/src/keyring.rs +++ b/bridges/primitives/test-utils/src/keyring.rs @@ -43,7 +43,7 @@ impl Account { pub fn secret(&self) -> SecretKey { let data = self.0.encode(); let mut bytes = [0_u8; 32]; - bytes[0..data.len()].copy_from_slice(&*data); + bytes[0..data.len()].copy_from_slice(&data); SecretKey::from_bytes(&bytes) .expect("A static array of the correct length is a known good.") } diff --git a/bridges/relays/bin-substrate/src/chains/mod.rs b/bridges/relays/bin-substrate/src/chains/mod.rs index d3c1290d98dc4..c1e963a63ec7c 100644 --- a/bridges/relays/bin-substrate/src/chains/mod.rs +++ b/bridges/relays/bin-substrate/src/chains/mod.rs @@ -71,14 +71,15 @@ mod tests { fn rialto_tx_extra_bytes_constant_is_correct() { let rialto_call = rialto_runtime::Call::System(rialto_runtime::SystemCall::remark { remark: vec![] }); - let rialto_tx = Rialto::sign_transaction(SignParam { - spec_version: 1, - transaction_version: 1, - genesis_hash: Default::default(), - signer: sp_keyring::AccountKeyring::Alice.pair(), - era: relay_substrate_client::TransactionEra::immortal(), - unsigned: UnsignedTransaction::new(rialto_call.clone().into(), 0), - }) + let rialto_tx = Rialto::sign_transaction( + SignParam { + spec_version: 1, + transaction_version: 1, + genesis_hash: Default::default(), + signer: sp_keyring::AccountKeyring::Alice.pair(), + }, + UnsignedTransaction::new(rialto_call.clone().into(), 0), + ) .unwrap(); let extra_bytes_in_transaction = rialto_tx.encode().len() - rialto_call.encode().len(); assert!( @@ -93,14 +94,15 @@ mod tests { fn millau_tx_extra_bytes_constant_is_correct() { let millau_call = millau_runtime::Call::System(millau_runtime::SystemCall::remark { remark: vec![] }); - let millau_tx = Millau::sign_transaction(SignParam { - spec_version: 0, - transaction_version: 0, - genesis_hash: Default::default(), - signer: sp_keyring::AccountKeyring::Alice.pair(), - era: relay_substrate_client::TransactionEra::immortal(), - unsigned: UnsignedTransaction::new(millau_call.clone().into(), 0), - }) + let millau_tx = Millau::sign_transaction( + SignParam { + spec_version: 0, + transaction_version: 0, + genesis_hash: Default::default(), + signer: sp_keyring::AccountKeyring::Alice.pair(), + }, + UnsignedTransaction::new(millau_call.clone().into(), 0), + ) .unwrap(); let extra_bytes_in_transaction = millau_tx.encode().len() - millau_call.encode().len(); assert!( diff --git a/bridges/relays/bin-substrate/src/cli/init_bridge.rs b/bridges/relays/bin-substrate/src/cli/init_bridge.rs index 150c55d9c6c67..218130dbe562e 100644 --- a/bridges/relays/bin-substrate/src/cli/init_bridge.rs +++ b/bridges/relays/bin-substrate/src/cli/init_bridge.rs @@ -26,11 +26,8 @@ use crate::{ cli::{bridge::CliBridgeBase, chain_schema::*}, }; use bp_runtime::Chain as ChainBase; -use codec::Encode; -use relay_substrate_client::{ - AccountKeyPairOf, Chain, SignParam, TransactionSignScheme, UnsignedTransaction, -}; -use sp_core::{Bytes, Pair}; +use relay_substrate_client::{AccountKeyPairOf, Chain, SignParam, UnsignedTransaction}; +use sp_core::Pair; use structopt::StructOpt; use strum::{EnumString, EnumVariantNames, VariantNames}; use substrate_relay_helper::finality::engine::{Engine, Grandpa as GrandpaFinalityEngine}; @@ -82,20 +79,16 @@ where source_client, target_client.clone(), target_sign.public().into(), + SignParam { + spec_version, + transaction_version, + genesis_hash: *target_client.genesis_hash(), + signer: target_sign, + }, move |transaction_nonce, initialization_data| { - Ok(Bytes( - Self::Target::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash: *target_client.genesis_hash(), - signer: target_sign, - era: relay_substrate_client::TransactionEra::immortal(), - unsigned: UnsignedTransaction::new( - Self::encode_init_bridge(initialization_data).into(), - transaction_nonce, - ), - })? - .encode(), + Ok(UnsignedTransaction::new( + Self::encode_init_bridge(initialization_data).into(), + transaction_nonce, )) }, ) diff --git a/bridges/relays/bin-substrate/src/cli/mod.rs b/bridges/relays/bin-substrate/src/cli/mod.rs index e35548c347c42..44bfce9f384ec 100644 --- a/bridges/relays/bin-substrate/src/cli/mod.rs +++ b/bridges/relays/bin-substrate/src/cli/mod.rs @@ -223,13 +223,6 @@ impl std::fmt::Display for HexBytes { } } -impl HexBytes { - /// Encode given object and wrap into nicely formatted bytes. - pub fn encode(t: &T) -> Self { - Self(t.encode()) - } -} - /// Prometheus metrics params. #[derive(Clone, Debug, PartialEq, StructOpt)] pub struct PrometheusParams { diff --git a/bridges/relays/bin-substrate/src/cli/register_parachain.rs b/bridges/relays/bin-substrate/src/cli/register_parachain.rs index 9573cde30319f..890436537795c 100644 --- a/bridges/relays/bin-substrate/src/cli/register_parachain.rs +++ b/bridges/relays/bin-substrate/src/cli/register_parachain.rs @@ -27,13 +27,13 @@ use polkadot_runtime_common::{ }; use polkadot_runtime_parachains::paras::ParaLifecycle; use relay_substrate_client::{ - AccountIdOf, CallOf, Chain, Client, HashOf, SignParam, Subscription, TransactionSignScheme, - TransactionStatusOf, UnsignedTransaction, + AccountIdOf, CallOf, Chain, Client, HashOf, SignParam, Subscription, TransactionStatusOf, + UnsignedTransaction, }; use rialto_runtime::SudoCall; use sp_core::{ storage::{well_known_keys::CODE, StorageKey}, - Bytes, Pair, + Pair, }; use structopt::StructOpt; use strum::{EnumString, EnumVariantNames, VariantNames}; @@ -120,20 +120,16 @@ impl RegisterParachain { relay_client .submit_and_watch_signed_extrinsic( relay_sudo_account.clone(), + SignParam:: { + spec_version, + transaction_version, + genesis_hash: relay_genesis_hash, + signer: reserve_parachain_signer, + }, move |_, transaction_nonce| { - Ok(Bytes( - Relaychain::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash: relay_genesis_hash, - signer: reserve_parachain_signer, - era: relay_substrate_client::TransactionEra::immortal(), - unsigned: UnsignedTransaction::new( - reserve_parachain_id_call.into(), - transaction_nonce, - ), - })? - .encode(), + Ok(UnsignedTransaction::new( + reserve_parachain_id_call.into(), + transaction_nonce, )) }, ) @@ -169,20 +165,16 @@ impl RegisterParachain { relay_client .submit_and_watch_signed_extrinsic( relay_sudo_account.clone(), + SignParam:: { + spec_version, + transaction_version, + genesis_hash: relay_genesis_hash, + signer: register_parathread_signer, + }, move |_, transaction_nonce| { - Ok(Bytes( - Relaychain::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash: relay_genesis_hash, - signer: register_parathread_signer, - era: relay_substrate_client::TransactionEra::immortal(), - unsigned: UnsignedTransaction::new( - register_parathread_call.into(), - transaction_nonce, - ), - })? - .encode(), + Ok(UnsignedTransaction::new( + register_parathread_call.into(), + transaction_nonce, )) }, ) @@ -231,22 +223,18 @@ impl RegisterParachain { .into(); let force_lease_signer = relay_sign.clone(); relay_client - .submit_signed_extrinsic(relay_sudo_account.clone(), move |_, transaction_nonce| { - Ok(Bytes( - Relaychain::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash: relay_genesis_hash, - signer: force_lease_signer, - era: relay_substrate_client::TransactionEra::immortal(), - unsigned: UnsignedTransaction::new( - force_lease_call.into(), - transaction_nonce, - ), - })? - .encode(), - )) - }) + .submit_signed_extrinsic( + relay_sudo_account, + SignParam:: { + spec_version, + transaction_version, + genesis_hash: relay_genesis_hash, + signer: force_lease_signer, + }, + move |_, transaction_nonce| { + Ok(UnsignedTransaction::new(force_lease_call.into(), transaction_nonce)) + }, + ) .await?; log::info!(target: "bridge", "Registered parachain leases: {:?}. Waiting for onboarding", para_id); diff --git a/bridges/relays/bin-substrate/src/cli/resubmit_transactions.rs b/bridges/relays/bin-substrate/src/cli/resubmit_transactions.rs index dad1afbf146d1..ac479986ed8fb 100644 --- a/bridges/relays/bin-substrate/src/cli/resubmit_transactions.rs +++ b/bridges/relays/bin-substrate/src/cli/resubmit_transactions.rs @@ -425,14 +425,15 @@ async fn update_transaction_tip>( current_priority = client .validate_transaction( at_block.1, - S::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash: *client.genesis_hash(), - signer: transaction_params.signer.clone(), - era: relay_substrate_client::TransactionEra::immortal(), - unsigned: unsigned_tx.clone(), - })?, + S::sign_transaction( + SignParam { + spec_version, + transaction_version, + genesis_hash: *client.genesis_hash(), + signer: transaction_params.signer.clone(), + }, + unsigned_tx.clone(), + )?, ) .await?? .priority; @@ -448,17 +449,18 @@ async fn update_transaction_tip>( Ok(( old_tip != unsigned_tx.tip, - S::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash: *client.genesis_hash(), - signer: transaction_params.signer.clone(), - era: relay_substrate_client::TransactionEra::new( + S::sign_transaction( + SignParam { + spec_version, + transaction_version, + genesis_hash: *client.genesis_hash(), + signer: transaction_params.signer.clone(), + }, + unsigned_tx.era(relay_substrate_client::TransactionEra::new( at_block, transaction_params.mortality, - ), - unsigned: unsigned_tx, - })?, + )), + )?, )) } diff --git a/bridges/relays/bin-substrate/src/cli/send_message.rs b/bridges/relays/bin-substrate/src/cli/send_message.rs index a45235f0bec28..79d1efee63879 100644 --- a/bridges/relays/bin-substrate/src/cli/send_message.rs +++ b/bridges/relays/bin-substrate/src/cli/send_message.rs @@ -26,7 +26,7 @@ use crate::{ chain_schema::*, encode_message::{self, CliEncodeMessage, RawMessage}, estimate_fee::{estimate_message_delivery_and_dispatch_fee, ConversionRateOverride}, - Balance, CliChain, HexBytes, HexLaneId, + Balance, CliChain, HexLaneId, }, }; use async_trait::async_trait; @@ -146,55 +146,50 @@ where let (spec_version, transaction_version) = source_client.simple_runtime_version().await?; let estimated_transaction_fee = source_client .estimate_extrinsic_fee(Bytes( - Self::Source::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash: source_genesis_hash, - signer: source_sign.clone(), - era: relay_substrate_client::TransactionEra::immortal(), - unsigned: UnsignedTransaction::new(send_message_call.clone(), 0), - })? + Self::Source::sign_transaction( + SignParam { + spec_version, + transaction_version, + genesis_hash: source_genesis_hash, + signer: source_sign.clone(), + }, + UnsignedTransaction::new(send_message_call.clone(), 0), + )? .encode(), )) .await?; source_client - .submit_signed_extrinsic(source_sign.public().into(), move |_, transaction_nonce| { - let signed_source_call = Self::Source::sign_transaction(SignParam { + .submit_signed_extrinsic( + source_sign.public().into(), + SignParam:: { spec_version, transaction_version, genesis_hash: source_genesis_hash, signer: source_sign.clone(), - era: relay_substrate_client::TransactionEra::immortal(), - unsigned: UnsignedTransaction::new(send_message_call, transaction_nonce), - })? - .encode(); - - log::info!( - target: "bridge", - "Sending message to {}. Lane: {:?}. Size: {}. Fee: {}", - Self::Target::NAME, - lane, - payload_len, - fee, - ); - log::info!( - target: "bridge", - "The source account ({:?}) balance will be reduced by (at most) {} (message fee) - + {} (tx fee ) = {} {} tokens", AccountId32::from(source_sign.public()), - fee.0, - estimated_transaction_fee.inclusion_fee(), - fee.0.saturating_add(estimated_transaction_fee.inclusion_fee().into()), - Self::Source::NAME, - ); - log::info!( - target: "bridge", - "Signed {} Call: {:?}", - Self::Source::NAME, - HexBytes::encode(&signed_source_call) - ); + }, + move |_, transaction_nonce| { + let unsigned = UnsignedTransaction::new(send_message_call, transaction_nonce); + log::info!( + target: "bridge", + "Sending message to {}. Lane: {:?}. Size: {}. Fee: {}", + Self::Target::NAME, + lane, + payload_len, + fee, + ); + log::info!( + target: "bridge", + "The source account ({:?}) balance will be reduced by (at most) {} (message fee) + + {} (tx fee ) = {} {} tokens", AccountId32::from(source_sign.public()), + fee.0, + estimated_transaction_fee.inclusion_fee(), + fee.0.saturating_add(estimated_transaction_fee.inclusion_fee().into()), + Self::Source::NAME, + ); - Ok(Bytes(signed_source_call)) - }) + Ok(unsigned) + }, + ) .await?; Ok(()) @@ -230,7 +225,7 @@ fn decode_xcm(message: RawMessage) -> anyhow::Result> { #[cfg(test)] mod tests { use super::*; - use crate::cli::ExplicitOrMaximal; + use crate::cli::{ExplicitOrMaximal, HexBytes}; #[test] fn send_raw_rialto_to_millau() { diff --git a/bridges/relays/client-millau/src/lib.rs b/bridges/relays/client-millau/src/lib.rs index 36b7cb792b436..7bab3669c4d02 100644 --- a/bridges/relays/client-millau/src/lib.rs +++ b/bridges/relays/client-millau/src/lib.rs @@ -102,18 +102,21 @@ impl TransactionSignScheme for Millau { type AccountKeyPair = sp_core::sr25519::Pair; type SignedTransaction = millau_runtime::UncheckedExtrinsic; - fn sign_transaction(param: SignParam) -> Result { + fn sign_transaction( + param: SignParam, + unsigned: UnsignedTransaction, + ) -> Result { let raw_payload = SignedPayload::from_raw( - param.unsigned.call, + unsigned.call.clone(), ( frame_system::CheckNonZeroSender::::new(), frame_system::CheckSpecVersion::::new(), frame_system::CheckTxVersion::::new(), frame_system::CheckGenesis::::new(), - frame_system::CheckEra::::from(param.era.frame_era()), - frame_system::CheckNonce::::from(param.unsigned.nonce), + frame_system::CheckEra::::from(unsigned.era.frame_era()), + frame_system::CheckNonce::::from(unsigned.nonce), frame_system::CheckWeight::::new(), - pallet_transaction_payment::ChargeTransactionPayment::::from(param.unsigned.tip), + pallet_transaction_payment::ChargeTransactionPayment::::from(unsigned.tip), millau_runtime::BridgeRejectObsoleteHeadersAndMessages, ), ( @@ -121,7 +124,7 @@ impl TransactionSignScheme for Millau { param.spec_version, param.transaction_version, param.genesis_hash, - param.era.signed_payload(param.genesis_hash), + unsigned.era.signed_payload(param.genesis_hash), (), (), (), @@ -155,13 +158,17 @@ impl TransactionSignScheme for Millau { fn parse_transaction(tx: Self::SignedTransaction) -> Option> { let extra = &tx.signature.as_ref()?.2; - Some(UnsignedTransaction { - call: tx.function.into(), - nonce: Compact::>::decode(&mut &extra.5.encode()[..]).ok()?.into(), - tip: Compact::>::decode(&mut &extra.7.encode()[..]) - .ok()? - .into(), - }) + Some( + UnsignedTransaction::new( + tx.function.into(), + Compact::>::decode(&mut &extra.5.encode()[..]).ok()?.into(), + ) + .tip( + Compact::>::decode(&mut &extra.7.encode()[..]) + .ok()? + .into(), + ), + ) } } @@ -185,15 +192,17 @@ mod tests { .into(), nonce: 777, tip: 888, - }; - let signed_transaction = Millau::sign_transaction(SignParam { - spec_version: 42, - transaction_version: 50000, - genesis_hash: [42u8; 64].into(), - signer: sp_core::sr25519::Pair::from_seed_slice(&[1u8; 32]).unwrap(), era: TransactionEra::immortal(), - unsigned: unsigned.clone(), - }) + }; + let signed_transaction = Millau::sign_transaction( + SignParam { + spec_version: 42, + transaction_version: 50000, + genesis_hash: [42u8; 64].into(), + signer: sp_core::sr25519::Pair::from_seed_slice(&[1u8; 32]).unwrap(), + }, + unsigned.clone(), + ) .unwrap(); let parsed_transaction = Millau::parse_transaction(signed_transaction).unwrap(); assert_eq!(parsed_transaction, unsigned); diff --git a/bridges/relays/client-rialto-parachain/src/lib.rs b/bridges/relays/client-rialto-parachain/src/lib.rs index a6f34201ca5f5..c678a21420fd4 100644 --- a/bridges/relays/client-rialto-parachain/src/lib.rs +++ b/bridges/relays/client-rialto-parachain/src/lib.rs @@ -101,31 +101,32 @@ impl TransactionSignScheme for RialtoParachain { type AccountKeyPair = sp_core::sr25519::Pair; type SignedTransaction = rialto_parachain_runtime::UncheckedExtrinsic; - fn sign_transaction(param: SignParam) -> Result { + fn sign_transaction( + param: SignParam, + unsigned: UnsignedTransaction, + ) -> Result { let raw_payload = SignedPayload::from_raw( - param.unsigned.call, + unsigned.call, ( frame_system::CheckNonZeroSender::::new(), frame_system::CheckSpecVersion::::new(), frame_system::CheckTxVersion::::new(), frame_system::CheckGenesis::::new(), frame_system::CheckEra::::from( - param.era.frame_era(), - ), - frame_system::CheckNonce::::from( - param.unsigned.nonce, + unsigned.era.frame_era(), ), + frame_system::CheckNonce::::from(unsigned.nonce), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::< rialto_parachain_runtime::Runtime, - >::from(param.unsigned.tip), + >::from(unsigned.tip), ), ( (), param.spec_version, param.transaction_version, param.genesis_hash, - param.era.signed_payload(param.genesis_hash), + unsigned.era.signed_payload(param.genesis_hash), (), (), (), diff --git a/bridges/relays/client-rialto/src/lib.rs b/bridges/relays/client-rialto/src/lib.rs index 733ca9d146570..b2984336cd265 100644 --- a/bridges/relays/client-rialto/src/lib.rs +++ b/bridges/relays/client-rialto/src/lib.rs @@ -109,25 +109,28 @@ impl TransactionSignScheme for Rialto { type AccountKeyPair = sp_core::sr25519::Pair; type SignedTransaction = rialto_runtime::UncheckedExtrinsic; - fn sign_transaction(param: SignParam) -> Result { + fn sign_transaction( + param: SignParam, + unsigned: UnsignedTransaction, + ) -> Result { let raw_payload = SignedPayload::from_raw( - param.unsigned.call, + unsigned.call.clone(), ( frame_system::CheckNonZeroSender::::new(), frame_system::CheckSpecVersion::::new(), frame_system::CheckTxVersion::::new(), frame_system::CheckGenesis::::new(), - frame_system::CheckEra::::from(param.era.frame_era()), - frame_system::CheckNonce::::from(param.unsigned.nonce), + frame_system::CheckEra::::from(unsigned.era.frame_era()), + frame_system::CheckNonce::::from(unsigned.nonce), frame_system::CheckWeight::::new(), - pallet_transaction_payment::ChargeTransactionPayment::::from(param.unsigned.tip), + pallet_transaction_payment::ChargeTransactionPayment::::from(unsigned.tip), ), ( (), param.spec_version, param.transaction_version, param.genesis_hash, - param.era.signed_payload(param.genesis_hash), + unsigned.era.signed_payload(param.genesis_hash), (), (), (), @@ -158,13 +161,17 @@ impl TransactionSignScheme for Rialto { fn parse_transaction(tx: Self::SignedTransaction) -> Option> { let extra = &tx.signature.as_ref()?.2; - Some(UnsignedTransaction { - call: tx.function.into(), - nonce: Compact::>::decode(&mut &extra.5.encode()[..]).ok()?.into(), - tip: Compact::>::decode(&mut &extra.7.encode()[..]) - .ok()? - .into(), - }) + Some( + UnsignedTransaction::new( + tx.function.into(), + Compact::>::decode(&mut &extra.5.encode()[..]).ok()?.into(), + ) + .tip( + Compact::>::decode(&mut &extra.7.encode()[..]) + .ok()? + .into(), + ), + ) } } @@ -188,15 +195,17 @@ mod tests { .into(), nonce: 777, tip: 888, - }; - let signed_transaction = Rialto::sign_transaction(SignParam { - spec_version: 42, - transaction_version: 50000, - genesis_hash: [42u8; 32].into(), - signer: sp_core::sr25519::Pair::from_seed_slice(&[1u8; 32]).unwrap(), era: TransactionEra::immortal(), - unsigned: unsigned.clone(), - }) + }; + let signed_transaction = Rialto::sign_transaction( + SignParam { + spec_version: 42, + transaction_version: 50000, + genesis_hash: [42u8; 32].into(), + signer: sp_core::sr25519::Pair::from_seed_slice(&[1u8; 32]).unwrap(), + }, + unsigned.clone(), + ) .unwrap(); let parsed_transaction = Rialto::parse_transaction(signed_transaction).unwrap(); assert_eq!(parsed_transaction, unsigned); diff --git a/bridges/relays/client-substrate/src/chain.rs b/bridges/relays/client-substrate/src/chain.rs index adfecff4eed18..132b86ad0b7bd 100644 --- a/bridges/relays/client-substrate/src/chain.rs +++ b/bridges/relays/client-substrate/src/chain.rs @@ -15,7 +15,9 @@ // along with Parity Bridges Common. If not, see . use bp_messages::MessageNonce; -use bp_runtime::{Chain as ChainBase, EncodedOrDecodedCall, HashOf, TransactionEraOf}; +use bp_runtime::{ + Chain as ChainBase, EncodedOrDecodedCall, HashOf, TransactionEra, TransactionEraOf, +}; use codec::{Codec, Encode}; use frame_support::weights::{Weight, WeightToFeePolynomial}; use jsonrpsee::core::{DeserializeOwned, Serialize}; @@ -158,12 +160,14 @@ pub struct UnsignedTransaction { pub nonce: C::Index, /// Tip included into transaction. pub tip: C::Balance, + /// Transaction era used by the chain. + pub era: TransactionEraOf, } impl UnsignedTransaction { - /// Create new unsigned transaction with given call, nonce and zero tip. + /// Create new unsigned transaction with given call, nonce, era and zero tip. pub fn new(call: EncodedOrDecodedCall, nonce: C::Index) -> Self { - Self { call, nonce, tip: Zero::zero() } + Self { call, nonce, era: TransactionEra::Immortal, tip: Zero::zero() } } /// Set transaction tip. @@ -172,13 +176,20 @@ impl UnsignedTransaction { self.tip = tip; self } + + /// Set transaction era. + #[must_use] + pub fn era(mut self, era: TransactionEraOf) -> Self { + self.era = era; + self + } } /// Account key pair used by transactions signing scheme. pub type AccountKeyPairOf = ::AccountKeyPair; /// Substrate-based chain transactions signing scheme. -pub trait TransactionSignScheme { +pub trait TransactionSignScheme: 'static { /// Chain that this scheme is to be used. type Chain: Chain; /// Type of key pairs used to sign transactions. @@ -187,7 +198,10 @@ pub trait TransactionSignScheme { type SignedTransaction: Clone + Debug + Codec + Send + 'static; /// Create transaction for given runtime call, signed by given account. - fn sign_transaction(param: SignParam) -> Result + fn sign_transaction( + param: SignParam, + unsigned: UnsignedTransaction, + ) -> Result where Self: Sized; @@ -213,10 +227,6 @@ pub struct SignParam { pub genesis_hash: ::Hash, /// Signer account pub signer: T::AccountKeyPair, - /// Transaction era used by the chain. - pub era: TransactionEraOf, - /// Transaction before it is signed. - pub unsigned: UnsignedTransaction, } impl BlockWithJustification for SignedBlock { diff --git a/bridges/relays/client-substrate/src/client.rs b/bridges/relays/client-substrate/src/client.rs index c6fd07c5ae599..38e2a8b106c66 100644 --- a/bridges/relays/client-substrate/src/client.rs +++ b/bridges/relays/client-substrate/src/client.rs @@ -17,13 +17,14 @@ //! Substrate node client. use crate::{ - chain::{Chain, ChainWithBalances, TransactionStatusOf}, + chain::{Chain, ChainWithBalances}, rpc::{ SubstrateAuthorClient, SubstrateChainClient, SubstrateFrameSystemClient, SubstrateGrandpaClient, SubstrateStateClient, SubstrateSystemClient, SubstrateTransactionPaymentClient, }, - ConnectionParams, Error, HashOf, HeaderIdOf, Result, + ConnectionParams, Error, HashOf, HeaderIdOf, Result, SignParam, TransactionSignScheme, + TransactionStatusOf, UnsignedTransaction, }; use async_std::sync::{Arc, Mutex}; @@ -403,10 +404,13 @@ impl Client { /// if all client instances are clones of the same initial `Client`. /// /// Note: The given transaction needs to be SCALE encoded beforehand. - pub async fn submit_signed_extrinsic( + pub async fn submit_signed_extrinsic + 'static>( &self, extrinsic_signer: C::AccountId, - prepare_extrinsic: impl FnOnce(HeaderIdOf, C::Index) -> Result + Send + 'static, + signing_data: SignParam, + prepare_extrinsic: impl FnOnce(HeaderIdOf, C::Index) -> Result> + + Send + + 'static, ) -> Result { let _guard = self.submit_signed_extrinsic_lock.lock().await; let transaction_nonce = self.next_account_index(extrinsic_signer).await?; @@ -421,12 +425,14 @@ impl Client { self.jsonrpsee_execute(move |client| async move { let extrinsic = prepare_extrinsic(best_header_id, transaction_nonce)?; - let tx_hash = SubstrateAuthorClient::::submit_extrinsic(&*client, extrinsic) - .await - .map_err(|e| { - log::error!(target: "bridge", "Failed to send transaction to {} node: {:?}", C::NAME, e); - e - })?; + let signed_extrinsic = S::sign_transaction(signing_data, extrinsic)?.encode(); + let tx_hash = + SubstrateAuthorClient::::submit_extrinsic(&*client, Bytes(signed_extrinsic)) + .await + .map_err(|e| { + log::error!(target: "bridge", "Failed to send transaction to {} node: {:?}", C::NAME, e); + e + })?; log::trace!(target: "bridge", "Sent transaction to {} node: {:?}", C::NAME, tx_hash); Ok(tx_hash) }) @@ -435,10 +441,15 @@ impl Client { /// Does exactly the same as `submit_signed_extrinsic`, but keeps watching for extrinsic status /// after submission. - pub async fn submit_and_watch_signed_extrinsic( + pub async fn submit_and_watch_signed_extrinsic< + S: TransactionSignScheme + 'static, + >( &self, extrinsic_signer: C::AccountId, - prepare_extrinsic: impl FnOnce(HeaderIdOf, C::Index) -> Result + Send + 'static, + signing_data: SignParam, + prepare_extrinsic: impl FnOnce(HeaderIdOf, C::Index) -> Result> + + Send + + 'static, ) -> Result>> { let _guard = self.submit_signed_extrinsic_lock.lock().await; let transaction_nonce = self.next_account_index(extrinsic_signer).await?; @@ -447,14 +458,17 @@ impl Client { let subscription = self .jsonrpsee_execute(move |client| async move { let extrinsic = prepare_extrinsic(best_header_id, transaction_nonce)?; - let tx_hash = C::Hasher::hash(&extrinsic.0); - let subscription = - SubstrateAuthorClient::::submit_and_watch_extrinsic(&*client, extrinsic) - .await - .map_err(|e| { - log::error!(target: "bridge", "Failed to send transaction to {} node: {:?}", C::NAME, e); - e - })?; + let signed_extrinsic = S::sign_transaction(signing_data, extrinsic)?.encode(); + let tx_hash = C::Hasher::hash(&signed_extrinsic); + let subscription = SubstrateAuthorClient::::submit_and_watch_extrinsic( + &*client, + Bytes(signed_extrinsic), + ) + .await + .map_err(|e| { + log::error!(target: "bridge", "Failed to send transaction to {} node: {:?}", C::NAME, e); + e + })?; log::trace!(target: "bridge", "Sent transaction to {} node: {:?}", C::NAME, tx_hash); Ok(subscription) }) diff --git a/bridges/relays/finality/src/finality_loop_tests.rs b/bridges/relays/finality/src/finality_loop_tests.rs index 330d166098ca3..b7f7bc80029fa 100644 --- a/bridges/relays/finality/src/finality_loop_tests.rs +++ b/bridges/relays/finality/src/finality_loop_tests.rs @@ -127,7 +127,7 @@ impl SourceClient for TestSourceClient { async fn best_finalized_block_number(&self) -> Result { let mut data = self.data.lock(); - (self.on_method_call)(&mut *data); + (self.on_method_call)(&mut data); Ok(data.source_best_block_number) } @@ -136,13 +136,13 @@ impl SourceClient for TestSourceClient { number: TestNumber, ) -> Result<(TestSourceHeader, Option), TestError> { let mut data = self.data.lock(); - (self.on_method_call)(&mut *data); + (self.on_method_call)(&mut data); data.source_headers.get(&number).cloned().ok_or(TestError::NonConnection) } async fn finality_proofs(&self) -> Result { let mut data = self.data.lock(); - (self.on_method_call)(&mut *data); + (self.on_method_call)(&mut data); Ok(futures::stream::iter(data.source_proofs.clone()).boxed()) } } @@ -168,7 +168,7 @@ impl TargetClient for TestTargetClient { &self, ) -> Result, TestError> { let mut data = self.data.lock(); - (self.on_method_call)(&mut *data); + (self.on_method_call)(&mut data); Ok(data.target_best_block_id) } @@ -178,7 +178,7 @@ impl TargetClient for TestTargetClient { proof: TestFinalityProof, ) -> Result<(), TestError> { let mut data = self.data.lock(); - (self.on_method_call)(&mut *data); + (self.on_method_call)(&mut data); data.target_best_block_id = HeaderId(header.number(), header.hash()); data.target_headers.push((header, proof)); Ok(()) diff --git a/bridges/relays/lib-substrate-relay/src/conversion_rate_update.rs b/bridges/relays/lib-substrate-relay/src/conversion_rate_update.rs index c7e241626945d..ed13f4e5b35ff 100644 --- a/bridges/relays/lib-substrate-relay/src/conversion_rate_update.rs +++ b/bridges/relays/lib-substrate-relay/src/conversion_rate_update.rs @@ -18,13 +18,12 @@ use crate::{messages_lane::SubstrateMessageLane, TransactionParams}; -use codec::Encode; use relay_substrate_client::{ transaction_stall_timeout, AccountIdOf, AccountKeyPairOf, CallOf, Chain, Client, SignParam, TransactionEra, TransactionSignScheme, UnsignedTransaction, }; use relay_utils::metrics::F64SharedRef; -use sp_core::{Bytes, Pair}; +use sp_core::Pair; use std::time::{Duration, Instant}; /// Duration between updater iterations. @@ -272,19 +271,19 @@ where updated_rate, )?; client - .submit_signed_extrinsic(signer_id, move |best_block_id, transaction_nonce| { - Ok(Bytes( - Sign::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash, - signer: transaction_params.signer, - era: TransactionEra::new(best_block_id, transaction_params.mortality), - unsigned: UnsignedTransaction::new(call.into(), transaction_nonce), - })? - .encode(), - )) - }) + .submit_signed_extrinsic( + signer_id, + SignParam:: { + spec_version, + transaction_version, + genesis_hash, + signer: transaction_params.signer, + }, + move |best_block_id, transaction_nonce| { + Ok(UnsignedTransaction::new(call.into(), transaction_nonce) + .era(TransactionEra::new(best_block_id, transaction_params.mortality))) + }, + ) .await .map(drop) .map_err(|err| anyhow::format_err!("{:?}", err)) diff --git a/bridges/relays/lib-substrate-relay/src/error.rs b/bridges/relays/lib-substrate-relay/src/error.rs index 64ed3a4fd49c6..b41870a181d93 100644 --- a/bridges/relays/lib-substrate-relay/src/error.rs +++ b/bridges/relays/lib-substrate-relay/src/error.rs @@ -53,7 +53,7 @@ pub enum Error { #[error("Failed to decode {0} GRANDPA authorities set at header {1}: {2:?}")] DecodeAuthorities(&'static str, Hash, codec::Error), /// Failed to retrieve header by the hash from the source chain. - #[error("Failed to retrieve {0} header with hash {1}: {:?}")] + #[error("Failed to retrieve {0} header with hash {1}: {2:?}")] RetrieveHeader(&'static str, Hash, client::Error), /// Failed to submit signed extrinsic from to the target chain. #[error( diff --git a/bridges/relays/lib-substrate-relay/src/finality/initialize.rs b/bridges/relays/lib-substrate-relay/src/finality/initialize.rs index be1719b4ae033..e25743180ee9e 100644 --- a/bridges/relays/lib-substrate-relay/src/finality/initialize.rs +++ b/bridges/relays/lib-substrate-relay/src/finality/initialize.rs @@ -23,18 +23,28 @@ use crate::{error::Error, finality::engine::Engine}; -use relay_substrate_client::{Chain, Client, Error as SubstrateError}; -use sp_core::Bytes; +use relay_substrate_client::{ + Chain, Client, Error as SubstrateError, SignParam, TransactionSignScheme, UnsignedTransaction, +}; use sp_runtime::traits::Header as HeaderT; /// Submit headers-bridge initialization transaction. -pub async fn initialize, SourceChain: Chain, TargetChain: Chain, F>( +pub async fn initialize< + E: Engine, + SourceChain: Chain, + TargetChain: Chain + TransactionSignScheme, + F, +>( source_client: Client, target_client: Client, target_transactions_signer: TargetChain::AccountId, + target_signing_data: SignParam, prepare_initialize_transaction: F, ) where - F: FnOnce(TargetChain::Index, E::InitializationData) -> Result + F: FnOnce( + TargetChain::Index, + E::InitializationData, + ) -> Result, SubstrateError> + Send + 'static, { @@ -42,6 +52,7 @@ pub async fn initialize, SourceChain: Chain, TargetChain: source_client, target_client, target_transactions_signer, + target_signing_data, prepare_initialize_transaction, ) .await; @@ -66,17 +77,26 @@ pub async fn initialize, SourceChain: Chain, TargetChain: } /// Craft and submit initialization transaction, returning any error that may occur. -async fn do_initialize, SourceChain: Chain, TargetChain: Chain, F>( +async fn do_initialize< + E: Engine, + SourceChain: Chain, + TargetChain: Chain + TransactionSignScheme, + F, +>( source_client: Client, target_client: Client, target_transactions_signer: TargetChain::AccountId, + target_signing_data: SignParam, prepare_initialize_transaction: F, ) -> Result< Option, Error::Number>, > where - F: FnOnce(TargetChain::Index, E::InitializationData) -> Result + F: FnOnce( + TargetChain::Index, + E::InitializationData, + ) -> Result, SubstrateError> + Send + 'static, { @@ -103,10 +123,15 @@ where ); let initialization_tx_hash = target_client - .submit_signed_extrinsic(target_transactions_signer, move |_, transaction_nonce| { - prepare_initialize_transaction(transaction_nonce, initialization_data) - }) + .submit_signed_extrinsic( + target_transactions_signer, + target_signing_data, + move |_, transaction_nonce| { + prepare_initialize_transaction(transaction_nonce, initialization_data) + }, + ) .await .map_err(|err| Error::SubmitTransaction(TargetChain::NAME, err))?; + Ok(Some(initialization_tx_hash)) } diff --git a/bridges/relays/lib-substrate-relay/src/finality/mod.rs b/bridges/relays/lib-substrate-relay/src/finality/mod.rs index 736937ba0724d..3144a1016ea5c 100644 --- a/bridges/relays/lib-substrate-relay/src/finality/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/finality/mod.rs @@ -194,7 +194,7 @@ where stall_timeout: transaction_stall_timeout( transaction_params.mortality, P::TargetChain::AVERAGE_BLOCK_INTERVAL, - crate::STALL_TIMEOUT, + relay_utils::STALL_TIMEOUT, ), only_mandatory_headers, }, diff --git a/bridges/relays/lib-substrate-relay/src/finality/target.rs b/bridges/relays/lib-substrate-relay/src/finality/target.rs index f9859937ff342..351f21cec80a9 100644 --- a/bridges/relays/lib-substrate-relay/src/finality/target.rs +++ b/bridges/relays/lib-substrate-relay/src/finality/target.rs @@ -25,14 +25,13 @@ use crate::{ }; use async_trait::async_trait; -use codec::Encode; use finality_relay::TargetClient; use relay_substrate_client::{ AccountIdOf, AccountKeyPairOf, Chain, Client, Error, HeaderIdOf, HeaderOf, SignParam, SyncHeader, TransactionEra, TransactionSignScheme, UnsignedTransaction, }; use relay_utils::relay_loop::Client as RelayClient; -use sp_core::{Bytes, Pair}; +use sp_core::Pair; /// Substrate client as Substrate finality target. pub struct SubstrateFinalityTarget { @@ -119,18 +118,15 @@ where self.client .submit_signed_extrinsic( self.transaction_params.signer.public().into(), + SignParam:: { + spec_version, + transaction_version, + genesis_hash, + signer: transaction_params.signer.clone(), + }, move |best_block_id, transaction_nonce| { - Ok(Bytes( - P::TransactionSignScheme::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash, - signer: transaction_params.signer.clone(), - era: TransactionEra::new(best_block_id, transaction_params.mortality), - unsigned: UnsignedTransaction::new(call.into(), transaction_nonce), - })? - .encode(), - )) + Ok(UnsignedTransaction::new(call.into(), transaction_nonce) + .era(TransactionEra::new(best_block_id, transaction_params.mortality))) }, ) .await diff --git a/bridges/relays/lib-substrate-relay/src/lib.rs b/bridges/relays/lib-substrate-relay/src/lib.rs index 1af4f67a7caf5..f4f1aae9d29d1 100644 --- a/bridges/relays/lib-substrate-relay/src/lib.rs +++ b/bridges/relays/lib-substrate-relay/src/lib.rs @@ -18,8 +18,6 @@ #![warn(missing_docs)] -use std::time::Duration; - pub mod conversion_rate_update; pub mod error; pub mod finality; @@ -31,15 +29,6 @@ pub mod messages_target; pub mod on_demand; pub mod parachains; -/// Default relay loop stall timeout. If transactions generated by relay are immortal, then -/// this timeout is used. -/// -/// There are no any strict requirements on block time in Substrate. But we assume here that all -/// Substrate-based chains will be designed to produce relatively fast (compared to the slowest -/// blockchains) blocks. So 1 hour seems to be a good guess for (even congested) chains to mine -/// transaction, or remove it from the pool. -pub const STALL_TIMEOUT: Duration = Duration::from_secs(60 * 60); - /// Transaction creation parameters. #[derive(Clone, Debug)] pub struct TransactionParams { diff --git a/bridges/relays/lib-substrate-relay/src/messages_lane.rs b/bridges/relays/lib-substrate-relay/src/messages_lane.rs index 050fa3ee1ae38..18e150e3617dd 100644 --- a/bridges/relays/lib-substrate-relay/src/messages_lane.rs +++ b/bridges/relays/lib-substrate-relay/src/messages_lane.rs @@ -22,7 +22,7 @@ use crate::{ messages_source::{SubstrateMessagesProof, SubstrateMessagesSource}, messages_target::{SubstrateMessagesDeliveryProof, SubstrateMessagesTarget}, on_demand::OnDemandRelay, - TransactionParams, STALL_TIMEOUT, + TransactionParams, }; use async_std::sync::Arc; @@ -39,7 +39,7 @@ use relay_substrate_client::{ transaction_stall_timeout, AccountKeyPairOf, BalanceOf, BlockNumberOf, CallOf, Chain, ChainWithMessages, Client, HashOf, TransactionSignScheme, }; -use relay_utils::metrics::MetricsParams; +use relay_utils::{metrics::MetricsParams, STALL_TIMEOUT}; use sp_core::Pair; use std::{convert::TryFrom, fmt::Debug, marker::PhantomData}; diff --git a/bridges/relays/lib-substrate-relay/src/messages_source.rs b/bridges/relays/lib-substrate-relay/src/messages_source.rs index f032ec3c0c082..5de9e30dd0ea2 100644 --- a/bridges/relays/lib-substrate-relay/src/messages_source.rs +++ b/bridges/relays/lib-substrate-relay/src/messages_source.rs @@ -346,11 +346,14 @@ where self.source_client .submit_signed_extrinsic( self.transaction_params.signer.public().into(), + SignParam:: { + spec_version, + transaction_version, + genesis_hash, + signer: self.transaction_params.signer.clone(), + }, move |best_block_id, transaction_nonce| { make_messages_delivery_proof_transaction::

( - spec_version, - transaction_version, - &genesis_hash, &transaction_params, best_block_id, transaction_nonce, @@ -377,18 +380,24 @@ where Err(_) => return BalanceOf::::max_value(), }; async { - let dummy_tx = make_messages_delivery_proof_transaction::

( - runtime_version.spec_version, - runtime_version.transaction_version, - self.source_client.genesis_hash(), - &self.transaction_params, - HeaderId(Default::default(), Default::default()), - Zero::zero(), - prepare_dummy_messages_delivery_proof::(), - false, - )?; + let dummy_tx = P::SourceTransactionSignScheme::sign_transaction( + SignParam:: { + spec_version: runtime_version.spec_version, + transaction_version: runtime_version.transaction_version, + genesis_hash: *self.source_client.genesis_hash(), + signer: self.transaction_params.signer.clone(), + }, + make_messages_delivery_proof_transaction::

( + &self.transaction_params, + HeaderId(Default::default(), Default::default()), + Zero::zero(), + prepare_dummy_messages_delivery_proof::(), + false, + )?, + )? + .encode(); self.source_client - .estimate_extrinsic_fee(dummy_tx) + .estimate_extrinsic_fee(Bytes(dummy_tx)) .await .map(|fee| fee.inclusion_fee()) } @@ -418,17 +427,13 @@ where } /// Make messages delivery proof transaction from given proof. -#[allow(clippy::too_many_arguments)] fn make_messages_delivery_proof_transaction( - spec_version: u32, - transaction_version: u32, - source_genesis_hash: &HashOf, source_transaction_params: &TransactionParams>, source_best_block_id: HeaderIdOf, transaction_nonce: IndexOf, proof: SubstrateMessagesDeliveryProof, trace_call: bool, -) -> Result +) -> Result, SubstrateError> where P::SourceTransactionSignScheme: TransactionSignScheme, { @@ -436,17 +441,8 @@ where P::ReceiveMessagesDeliveryProofCallBuilder::build_receive_messages_delivery_proof_call( proof, trace_call, ); - Ok(Bytes( - P::SourceTransactionSignScheme::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash: *source_genesis_hash, - signer: source_transaction_params.signer.clone(), - era: TransactionEra::new(source_best_block_id, source_transaction_params.mortality), - unsigned: UnsignedTransaction::new(call.into(), transaction_nonce), - })? - .encode(), - )) + Ok(UnsignedTransaction::new(call.into(), transaction_nonce) + .era(TransactionEra::new(source_best_block_id, source_transaction_params.mortality))) } /// Prepare 'dummy' messages delivery proof that will compose the delivery confirmation transaction. diff --git a/bridges/relays/lib-substrate-relay/src/messages_target.rs b/bridges/relays/lib-substrate-relay/src/messages_target.rs index b315dfed4bb50..d88b0539153dd 100644 --- a/bridges/relays/lib-substrate-relay/src/messages_target.rs +++ b/bridges/relays/lib-substrate-relay/src/messages_target.rs @@ -255,11 +255,14 @@ where self.target_client .submit_signed_extrinsic( self.transaction_params.signer.public().into(), + SignParam:: { + spec_version, + transaction_version, + genesis_hash, + signer: self.transaction_params.signer.clone(), + }, move |best_block_id, transaction_nonce| { make_messages_delivery_transaction::

( - spec_version, - transaction_version, - &genesis_hash, &transaction_params, best_block_id, transaction_nonce, @@ -299,23 +302,29 @@ where let (spec_version, transaction_version) = self.target_client.simple_runtime_version().await?; // Prepare 'dummy' delivery transaction - we only care about its length and dispatch weight. - let delivery_tx = make_messages_delivery_transaction::

( - spec_version, - transaction_version, - self.target_client.genesis_hash(), - &self.transaction_params, - HeaderId(Default::default(), Default::default()), - Zero::zero(), - self.relayer_id_at_source.clone(), - nonces.clone(), - prepare_dummy_messages_proof::( + let delivery_tx = P::TargetTransactionSignScheme::sign_transaction( + SignParam { + spec_version, + transaction_version, + genesis_hash: Default::default(), + signer: self.transaction_params.signer.clone(), + }, + make_messages_delivery_transaction::

( + &self.transaction_params, + HeaderId(Default::default(), Default::default()), + Zero::zero(), + self.relayer_id_at_source.clone(), nonces.clone(), - total_dispatch_weight, - total_size, - ), - false, - )?; - let delivery_tx_fee = self.target_client.estimate_extrinsic_fee(delivery_tx).await?; + prepare_dummy_messages_proof::( + nonces.clone(), + total_dispatch_weight, + total_size, + ), + false, + )?, + )? + .encode(); + let delivery_tx_fee = self.target_client.estimate_extrinsic_fee(Bytes(delivery_tx)).await?; let inclusion_fee_in_target_tokens = delivery_tx_fee.inclusion_fee(); // The pre-dispatch cost of delivery transaction includes additional fee to cover dispatch @@ -340,24 +349,30 @@ where let (spec_version, transaction_version) = self.target_client.simple_runtime_version().await?; let larger_dispatch_weight = total_dispatch_weight.saturating_add(WEIGHT_DIFFERENCE); - let dummy_tx = make_messages_delivery_transaction::

( - spec_version, - transaction_version, - self.target_client.genesis_hash(), - &self.transaction_params, - HeaderId(Default::default(), Default::default()), - Zero::zero(), - self.relayer_id_at_source.clone(), - nonces.clone(), - prepare_dummy_messages_proof::( + let dummy_tx = P::TargetTransactionSignScheme::sign_transaction( + SignParam { + spec_version, + transaction_version, + genesis_hash: Default::default(), + signer: self.transaction_params.signer.clone(), + }, + make_messages_delivery_transaction::

( + &self.transaction_params, + HeaderId(Default::default(), Default::default()), + Zero::zero(), + self.relayer_id_at_source.clone(), nonces.clone(), - larger_dispatch_weight, - total_size, - ), - false, - )?; + prepare_dummy_messages_proof::( + nonces.clone(), + larger_dispatch_weight, + total_size, + ), + false, + )?, + )? + .encode(); let larger_delivery_tx_fee = - self.target_client.estimate_extrinsic_fee(dummy_tx).await?; + self.target_client.estimate_extrinsic_fee(Bytes(dummy_tx)).await?; compute_prepaid_messages_refund::( total_prepaid_nonces, @@ -406,11 +421,7 @@ where } /// Make messages delivery transaction from given proof. -#[allow(clippy::too_many_arguments)] fn make_messages_delivery_transaction( - spec_version: u32, - transaction_version: u32, - target_genesis_hash: &HashOf, target_transaction_params: &TransactionParams>, target_best_block_id: HeaderIdOf, transaction_nonce: IndexOf, @@ -418,7 +429,7 @@ fn make_messages_delivery_transaction( nonces: RangeInclusive, proof: SubstrateMessagesProof, trace_call: bool, -) -> Result +) -> Result, SubstrateError> where P::TargetTransactionSignScheme: TransactionSignScheme, { @@ -431,17 +442,8 @@ where dispatch_weight, trace_call, ); - Ok(Bytes( - P::TargetTransactionSignScheme::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash: *target_genesis_hash, - signer: target_transaction_params.signer.clone(), - era: TransactionEra::new(target_best_block_id, target_transaction_params.mortality), - unsigned: UnsignedTransaction::new(call.into(), transaction_nonce), - })? - .encode(), - )) + Ok(UnsignedTransaction::new(call.into(), transaction_nonce) + .era(TransactionEra::new(target_best_block_id, target_transaction_params.mortality))) } /// Prepare 'dummy' messages proof that will compose the delivery transaction. diff --git a/bridges/relays/lib-substrate-relay/src/on_demand/headers.rs b/bridges/relays/lib-substrate-relay/src/on_demand/headers.rs index 749205ef9b3cd..09e7a41a0c725 100644 --- a/bridges/relays/lib-substrate-relay/src/on_demand/headers.rs +++ b/bridges/relays/lib-substrate-relay/src/on_demand/headers.rs @@ -28,6 +28,7 @@ use relay_substrate_client::{ }; use relay_utils::{ metrics::MetricsParams, relay_loop::Client as RelayClient, FailedClient, MaybeConnectionError, + STALL_TIMEOUT, }; use crate::{ @@ -37,7 +38,7 @@ use crate::{ SubstrateFinalitySyncPipeline, RECENT_FINALITY_PROOFS_LIMIT, }, on_demand::OnDemandRelay, - TransactionParams, STALL_TIMEOUT, + TransactionParams, }; /// On-demand Substrate <-> Substrate header finality relay. diff --git a/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs b/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs index dc400115502eb..577312e16c109 100644 --- a/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs +++ b/bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs @@ -263,7 +263,7 @@ async fn background_task( let stall_timeout = relay_substrate_client::transaction_stall_timeout( target_transactions_mortality, P::TargetChain::AVERAGE_BLOCK_INTERVAL, - crate::STALL_TIMEOUT, + relay_utils::STALL_TIMEOUT, ); log::info!( diff --git a/bridges/relays/lib-substrate-relay/src/parachains/target.rs b/bridges/relays/lib-substrate-relay/src/parachains/target.rs index a96c0ba0ab692..355dd394c2188 100644 --- a/bridges/relays/lib-substrate-relay/src/parachains/target.rs +++ b/bridges/relays/lib-substrate-relay/src/parachains/target.rs @@ -29,7 +29,7 @@ use bp_parachains::{ }; use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId}; use bp_runtime::HeaderIdProvider; -use codec::{Decode, Encode}; +use codec::Decode; use parachains_relay::{ parachains_loop::TargetClient, parachains_loop_metrics::ParachainsLoopMetrics, }; @@ -141,7 +141,7 @@ where .and_then(|maybe_encoded_head| match maybe_encoded_head { Some(encoded_head) => HeaderOf::::decode(&mut &encoded_head.0[..]) - .map(|head| Some(head)) + .map(Some) .map_err(Self::Error::ResponseParseFailed), None => Ok(None), }) @@ -182,18 +182,15 @@ where self.client .submit_signed_extrinsic( self.transaction_params.signer.public().into(), + SignParam:: { + spec_version, + transaction_version, + genesis_hash, + signer: transaction_params.signer, + }, move |best_block_id, transaction_nonce| { - Ok(Bytes( - P::TransactionSignScheme::sign_transaction(SignParam { - spec_version, - transaction_version, - genesis_hash, - signer: transaction_params.signer, - era: TransactionEra::new(best_block_id, transaction_params.mortality), - unsigned: UnsignedTransaction::new(call.into(), transaction_nonce), - })? - .encode(), - )) + Ok(UnsignedTransaction::new(call.into(), transaction_nonce) + .era(TransactionEra::new(best_block_id, transaction_params.mortality))) }, ) .await diff --git a/bridges/relays/messages/src/message_lane_loop.rs b/bridges/relays/messages/src/message_lane_loop.rs index 2e700deb9a6bf..bd7a7de8290b9 100644 --- a/bridges/relays/messages/src/message_lane_loop.rs +++ b/bridges/relays/messages/src/message_lane_loop.rs @@ -94,7 +94,7 @@ pub struct MessageDeliveryParams { } /// Message details. -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct MessageDetails { /// Message dispatch weight. pub dispatch_weight: Weight, @@ -224,7 +224,7 @@ pub trait TargetClient: RelayClient { } /// State of the client. -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct ClientState { /// The best header id of this chain. pub best_self: SelfHeaderId, @@ -560,7 +560,7 @@ pub(crate) mod tests { async fn reconnect(&mut self) -> Result<(), TestError> { { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); data.is_source_reconnected = true; } Ok(()) @@ -571,7 +571,7 @@ pub(crate) mod tests { impl SourceClient for TestSourceClient { async fn state(&self) -> Result, TestError> { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); if data.is_source_fails { return Err(TestError) } @@ -583,7 +583,7 @@ pub(crate) mod tests { id: SourceHeaderIdOf, ) -> Result<(SourceHeaderIdOf, MessageNonce), TestError> { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); if data.is_source_fails { return Err(TestError) } @@ -595,7 +595,7 @@ pub(crate) mod tests { id: SourceHeaderIdOf, ) -> Result<(SourceHeaderIdOf, MessageNonce), TestError> { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); Ok((id, data.source_latest_confirmed_received_nonce)) } @@ -629,7 +629,7 @@ pub(crate) mod tests { TestError, > { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); Ok(( id, nonces.clone(), @@ -650,7 +650,7 @@ pub(crate) mod tests { proof: TestMessagesReceivingProof, ) -> Result<(), TestError> { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); data.source_state.best_self = HeaderId(data.source_state.best_self.0 + 1, data.source_state.best_self.1 + 1); data.source_state.best_finalized_self = data.source_state.best_self; @@ -663,7 +663,7 @@ pub(crate) mod tests { let mut data = self.data.lock(); data.target_to_source_header_required = Some(id); data.target_to_source_header_requirements.push(id); - (self.tick)(&mut *data); + (self.tick)(&mut data); } async fn estimate_confirmation_transaction(&self) -> TestSourceChainBalance { @@ -693,7 +693,7 @@ pub(crate) mod tests { async fn reconnect(&mut self) -> Result<(), TestError> { { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); data.is_target_reconnected = true; } Ok(()) @@ -704,7 +704,7 @@ pub(crate) mod tests { impl TargetClient for TestTargetClient { async fn state(&self) -> Result, TestError> { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); if data.is_target_fails { return Err(TestError) } @@ -716,7 +716,7 @@ pub(crate) mod tests { id: TargetHeaderIdOf, ) -> Result<(TargetHeaderIdOf, MessageNonce), TestError> { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); if data.is_target_fails { return Err(TestError) } @@ -743,7 +743,7 @@ pub(crate) mod tests { id: TargetHeaderIdOf, ) -> Result<(TargetHeaderIdOf, MessageNonce), TestError> { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); if data.is_target_fails { return Err(TestError) } @@ -764,7 +764,7 @@ pub(crate) mod tests { proof: TestMessagesProof, ) -> Result, TestError> { let mut data = self.data.lock(); - (self.tick)(&mut *data); + (self.tick)(&mut data); if data.is_target_fails { return Err(TestError) } @@ -784,7 +784,7 @@ pub(crate) mod tests { let mut data = self.data.lock(); data.source_to_target_header_required = Some(id); data.source_to_target_header_requirements.push(id); - (self.tick)(&mut *data); + (self.tick)(&mut data); } async fn estimate_delivery_transaction_in_source_tokens( diff --git a/bridges/relays/utils/src/lib.rs b/bridges/relays/utils/src/lib.rs index 603011819bc6f..8e8870ac188f6 100644 --- a/bridges/relays/utils/src/lib.rs +++ b/bridges/relays/utils/src/lib.rs @@ -25,6 +25,15 @@ use futures::future::FutureExt; use std::time::Duration; use thiserror::Error; +/// Default relay loop stall timeout. If transactions generated by relay are immortal, then +/// this timeout is used. +/// +/// There are no any strict requirements on block time in Substrate. But we assume here that all +/// Substrate-based chains will be designed to produce relatively fast (compared to the slowest +/// blockchains) blocks. So 1 hour seems to be a good guess for (even congested) chains to mine +/// transaction, or remove it from the pool. +pub const STALL_TIMEOUT: Duration = Duration::from_secs(60 * 60); + /// Max delay after connection-unrelated error happened before we'll try the /// same request again. pub const MAX_BACKOFF_INTERVAL: Duration = Duration::from_secs(60);