Extract unsigned tx from the SignParam structure (#1561)

* change sign_transaction method

* clippy

* rustup update && clippy

* remove redudnant clone

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::<Sign> {
+				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))

bridges/relays/lib-substrate-relay/src/error.rs

@@ -53,7 +53,7 @@ pub enum Error<Hash: Debug + MaybeDisplay, HeaderNumber: Debug + MaybeDisplay> {
 	#[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(

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<E: Engine<SourceChain>, SourceChain: Chain, TargetChain: Chain, F>(
+pub async fn initialize<
+	E: Engine<SourceChain>,
+	SourceChain: Chain,
+	TargetChain: Chain + TransactionSignScheme<Chain = TargetChain>,
+	F,
+>(
 	source_client: Client<SourceChain>,
 	target_client: Client<TargetChain>,
 	target_transactions_signer: TargetChain::AccountId,
+	target_signing_data: SignParam<TargetChain>,
 	prepare_initialize_transaction: F,
 ) where
-	F: FnOnce(TargetChain::Index, E::InitializationData) -> Result<Bytes, SubstrateError>
+	F: FnOnce(
+			TargetChain::Index,
+			E::InitializationData,
+		) -> Result<UnsignedTransaction<TargetChain>, SubstrateError>
 		+ Send
 		+ 'static,
 {
@@ -42,6 +52,7 @@ pub async fn initialize<E: Engine<SourceChain>, 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<E: Engine<SourceChain>, SourceChain: Chain, TargetChain:
 }
 
 /// Craft and submit initialization transaction, returning any error that may occur.
-async fn do_initialize<E: Engine<SourceChain>, SourceChain: Chain, TargetChain: Chain, F>(
+async fn do_initialize<
+	E: Engine<SourceChain>,
+	SourceChain: Chain,
+	TargetChain: Chain + TransactionSignScheme<Chain = TargetChain>,
+	F,
+>(
 	source_client: Client<SourceChain>,
 	target_client: Client<TargetChain>,
 	target_transactions_signer: TargetChain::AccountId,
+	target_signing_data: SignParam<TargetChain>,
 	prepare_initialize_transaction: F,
 ) -> Result<
 	Option<TargetChain::Hash>,
 	Error<SourceChain::Hash, <SourceChain::Header as HeaderT>::Number>,
 >
 where
-	F: FnOnce(TargetChain::Index, E::InitializationData) -> Result<Bytes, SubstrateError>
+	F: FnOnce(
+			TargetChain::Index,
+			E::InitializationData,
+		) -> Result<UnsignedTransaction<TargetChain>, 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))
 }

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,
 		},

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<P: SubstrateFinalitySyncPipeline> {
@@ -119,18 +118,15 @@ where
 		self.client
 			.submit_signed_extrinsic(
 				self.transaction_params.signer.public().into(),
+				SignParam::<P::TransactionSignScheme> {
+					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

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<TS> {

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};
 

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::<P::SourceTransactionSignScheme> {
+					spec_version,
+					transaction_version,
+					genesis_hash,
+					signer: self.transaction_params.signer.clone(),
+				},
 				move |best_block_id, transaction_nonce| {
 					make_messages_delivery_proof_transaction::<P>(
-						spec_version,
-						transaction_version,
-						&genesis_hash,
 						&transaction_params,
 						best_block_id,
 						transaction_nonce,
@@ -377,18 +380,24 @@ where
 			Err(_) => return BalanceOf::<P::SourceChain>::max_value(),
 		};
 		async {
-			let dummy_tx = make_messages_delivery_proof_transaction::<P>(
-				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::<P::SourceChain, P::TargetChain>(),
-				false,
-			)?;
+			let dummy_tx = P::SourceTransactionSignScheme::sign_transaction(
+				SignParam::<P::SourceTransactionSignScheme> {
+					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::<P>(
+					&self.transaction_params,
+					HeaderId(Default::default(), Default::default()),
+					Zero::zero(),
+					prepare_dummy_messages_delivery_proof::<P::SourceChain, P::TargetChain>(),
+					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<P: SubstrateMessageLane>(
-	spec_version: u32,
-	transaction_version: u32,
-	source_genesis_hash: &HashOf<P::SourceChain>,
 	source_transaction_params: &TransactionParams<AccountKeyPairOf<P::SourceTransactionSignScheme>>,
 	source_best_block_id: HeaderIdOf<P::SourceChain>,
 	transaction_nonce: IndexOf<P::SourceChain>,
 	proof: SubstrateMessagesDeliveryProof<P::TargetChain>,
 	trace_call: bool,
-) -> Result<Bytes, SubstrateError>
+) -> Result<UnsignedTransaction<P::SourceChain>, SubstrateError>
 where
 	P::SourceTransactionSignScheme: TransactionSignScheme<Chain = P::SourceChain>,
 {
@@ -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.

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::<P::TargetTransactionSignScheme> {
+					spec_version,
+					transaction_version,
+					genesis_hash,
+					signer: self.transaction_params.signer.clone(),
+				},
 				move |best_block_id, transaction_nonce| {
 					make_messages_delivery_transaction::<P>(
-						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::<P>(
-			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::<P::SourceChain>(
+		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::<P>(
+				&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::<P::SourceChain>(
+					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::<P>(
-				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::<P::SourceChain>(
+			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::<P>(
+					&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::<P::SourceChain>(
+						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::<P::TargetChain>(
 				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<P: SubstrateMessageLane>(
-	spec_version: u32,
-	transaction_version: u32,
-	target_genesis_hash: &HashOf<P::TargetChain>,
 	target_transaction_params: &TransactionParams<AccountKeyPairOf<P::TargetTransactionSignScheme>>,
 	target_best_block_id: HeaderIdOf<P::TargetChain>,
 	transaction_nonce: IndexOf<P::TargetChain>,
@@ -418,7 +429,7 @@ fn make_messages_delivery_transaction<P: SubstrateMessageLane>(
 	nonces: RangeInclusive<MessageNonce>,
 	proof: SubstrateMessagesProof<P::SourceChain>,
 	trace_call: bool,
-) -> Result<Bytes, SubstrateError>
+) -> Result<UnsignedTransaction<P::TargetChain>, SubstrateError>
 where
 	P::TargetTransactionSignScheme: TransactionSignScheme<Chain = P::TargetChain>,
 {
@@ -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.

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.

bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs

@@ -263,7 +263,7 @@ async fn background_task<P: SubstrateParachainsPipeline>(
 			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!(

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::<P::SourceParachain>::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::<P::TransactionSignScheme> {
+					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