fix: stale error handling for unsigned extrinsics (PRO-804) (#4100)

engine/src/state_chain_observer/client/extrinsic_api/common.rs

@@ -1,3 +1,4 @@
+use sp_runtime::transaction_validity::InvalidTransaction;
 use tokio::sync::{mpsc, oneshot};
 
 pub(super) async fn send_request<Request, F: FnOnce(oneshot::Sender<Result>) -> Request, Result>(
@@ -10,3 +11,13 @@ pub(super) async fn send_request<Request, F: FnOnce(oneshot::Sender<Result>) ->
 	let _result = request_sender.send(into_request(result_sender)).await;
 	result_receiver
 }
+
+pub(super) fn invalid_err_obj(
+	invalid_reason: InvalidTransaction,
+) -> jsonrpsee::types::ErrorObjectOwned {
+	jsonrpsee::types::ErrorObject::owned(
+		1010,
+		"Invalid Transaction",
+		Some(<&'static str>::from(invalid_reason)),
+	)
+}

engine/src/state_chain_observer/client/extrinsic_api/signed/submission_watcher.rs

@@ -22,6 +22,7 @@ use utilities::{
 use crate::state_chain_observer::client::{
 	base_rpc_api,
 	error_decoder::{DispatchError, ErrorDecoder},
+	extrinsic_api::common::invalid_err_obj,
 	storage_api::StorageApi,
 	SUBSTRATE_BEHAVIOUR,
 };
@@ -210,16 +211,6 @@ impl<'a, 'env, BaseRpcClient: base_rpc_api::BaseRpcApi + Send + Sync + 'static>
 					break Ok(Ok(tx_hash))
 				},
 				Err(rpc_err) => {
-					fn invalid_err_obj(
-						invalid_reason: InvalidTransaction,
-					) -> jsonrpsee::types::ErrorObjectOwned {
-						jsonrpsee::types::ErrorObject::owned(
-							1010,
-							"Invalid Transaction",
-							Some(<&'static str>::from(invalid_reason)),
-						)
-					}
-
 					match rpc_err {
 						// This occurs when a transaction with the same nonce is in the
 						// transaction pool (and the priority is <= priority of that

engine/src/state_chain_observer/client/extrinsic_api/unsigned.rs

@@ -2,19 +2,25 @@ use std::sync::Arc;
 
 use async_trait::async_trait;
 use sp_core::H256;
-use sp_runtime::traits::Hash;
+use sp_runtime::{traits::Hash, transaction_validity::InvalidTransaction};
 use tokio::sync::{mpsc, oneshot};
 use utilities::task_scope::{Scope, ScopedJoinHandle, OR_CANCEL};
 
+use crate::state_chain_observer::client::extrinsic_api::common::invalid_err_obj;
+
 use super::{
 	super::{base_rpc_api, SUBSTRATE_BEHAVIOUR},
 	common::send_request,
 };
 
+pub enum ExtrinsicError {
+	Stale,
+}
+
 // Note 'static on the generics in this trait are only required for mockall to mock it
 #[async_trait]
 pub trait UnsignedExtrinsicApi {
-	async fn submit_unsigned_extrinsic<Call>(&self, call: Call) -> H256
+	async fn submit_unsigned_extrinsic<Call>(&self, call: Call) -> Result<H256, ExtrinsicError>
 	where
 		Call: Into<state_chain_runtime::RuntimeCall>
 			+ Clone
@@ -25,7 +31,10 @@ pub trait UnsignedExtrinsicApi {
 }
 
 pub struct UnsignedExtrinsicClient {
-	request_sender: mpsc::Sender<(state_chain_runtime::RuntimeCall, oneshot::Sender<H256>)>,
+	request_sender: mpsc::Sender<(
+		state_chain_runtime::RuntimeCall,
+		oneshot::Sender<Result<H256, ExtrinsicError>>,
+	)>,
 	_task_handle: ScopedJoinHandle<()>,
 }
 impl UnsignedExtrinsicClient {
@@ -48,7 +57,7 @@ impl UnsignedExtrinsicClient {
 						match base_rpc_client.submit_extrinsic(extrinsic).await {
 							Ok(tx_hash) => {
 								assert_eq!(tx_hash, expected_hash, "{SUBSTRATE_BEHAVIOUR}");
-								tx_hash
+								Ok(tx_hash)
 							},
 							Err(rpc_err) => {
 								match rpc_err {
@@ -66,7 +75,7 @@ impl UnsignedExtrinsicClient {
 										tracing::debug!(
 											"Already in pool with tx_hash: {expected_hash:#x}."
 										);
-										expected_hash
+										Ok(expected_hash)
 									},
 									// POOL_TEMPORARILY_BANNED error is not entirely understood, we
 									// believe it has a similiar meaning to POOL_ALREADY_IMPORTED,
@@ -78,7 +87,13 @@ impl UnsignedExtrinsicClient {
 										tracing::debug!(
 											"Transaction is temporarily banned with tx_hash: {expected_hash:#x}."
 										);
-										expected_hash
+										Ok(expected_hash)
+									},
+									jsonrpsee::core::Error::Call(
+										jsonrpsee::types::error::CallError::Custom(ref obj),
+									) if obj == &invalid_err_obj(InvalidTransaction::Stale) => {
+										tracing::debug!("Submission failed as the transaction is stale: {obj:?}");
+										Err(ExtrinsicError::Stale)
 									},
 									_ => return Err(rpc_err.into()),
 								}
@@ -95,7 +110,7 @@ impl UnsignedExtrinsicClient {
 
 #[async_trait]
 impl UnsignedExtrinsicApi for UnsignedExtrinsicClient {
-	async fn submit_unsigned_extrinsic<Call>(&self, call: Call) -> H256
+	async fn submit_unsigned_extrinsic<Call>(&self, call: Call) -> Result<H256, ExtrinsicError>
 	where
 		Call: Into<state_chain_runtime::RuntimeCall>
 			+ Clone

engine/src/state_chain_observer/client/mod.rs

@@ -24,7 +24,10 @@ use utilities::{
 use self::{
 	base_rpc_api::BaseRpcClient,
 	chain_api::ChainApi,
-	extrinsic_api::signed::{signer, SignedExtrinsicApi},
+	extrinsic_api::{
+		signed::{signer, SignedExtrinsicApi},
+		unsigned,
+	},
 };
 
 /// For expressing an expectation regarding substrate's behaviour (Not our chain though)
@@ -81,7 +84,7 @@ pub struct StateChainClient<
 > {
 	genesis_hash: state_chain_runtime::Hash,
 	signed_extrinsic_client: SignedExtrinsicClient,
-	unsigned_extrinsic_client: extrinsic_api::unsigned::UnsignedExtrinsicClient,
+	unsigned_extrinsic_client: unsigned::UnsignedExtrinsicClient,
 	_block_producer: ScopedJoinHandle<()>,
 	pub base_rpc_client: Arc<BaseRpcClient>,
 	latest_block_hash_watcher: tokio::sync::watch::Receiver<state_chain_runtime::Hash>,
@@ -326,7 +329,7 @@ impl<BaseRpcClient: base_rpc_api::BaseRpcApi + Send + Sync + 'static, SignedExtr
 			signed_extrinsic_client: signed_extrinsic_client_builder
 				.build(scope, base_rpc_client.clone(), genesis_hash, &mut state_chain_stream)
 				.await?,
-			unsigned_extrinsic_client: extrinsic_api::unsigned::UnsignedExtrinsicClient::new(
+			unsigned_extrinsic_client: unsigned::UnsignedExtrinsicClient::new(
 				scope,
 				base_rpc_client.clone(),
 			),
@@ -479,11 +482,13 @@ impl<
 impl<
 		BaseRpcApi: base_rpc_api::BaseRpcApi + Send + Sync + 'static,
 		SignedExtrinsicClient: Send + Sync + 'static,
-	> extrinsic_api::unsigned::UnsignedExtrinsicApi
-	for StateChainClient<SignedExtrinsicClient, BaseRpcApi>
+	> unsigned::UnsignedExtrinsicApi for StateChainClient<SignedExtrinsicClient, BaseRpcApi>
 {
 	/// Submit an unsigned extrinsic.
-	async fn submit_unsigned_extrinsic<Call>(&self, call: Call) -> H256
+	async fn submit_unsigned_extrinsic<Call>(
+		&self,
+		call: Call,
+	) -> Result<H256, unsigned::ExtrinsicError>
 	where
 		Call: Into<state_chain_runtime::RuntimeCall>
 			+ std::fmt::Debug
@@ -521,7 +526,10 @@ pub mod mocks {
 	use sp_core::{storage::StorageKey, H256};
 	use state_chain_runtime::AccountId;
 
-	use super::{extrinsic_api, storage_api};
+	use super::{
+		extrinsic_api::{self, unsigned},
+		storage_api,
+	};
 
 	mock! {
 		pub StateChainClient {}
@@ -555,7 +563,7 @@ pub mod mocks {
 			async fn submit_unsigned_extrinsic<Call>(
 				&self,
 				call: Call,
-			) -> H256
+			) -> Result<H256, unsigned::ExtrinsicError>
 			where
 				Call: Into<state_chain_runtime::RuntimeCall> + Clone + std::fmt::Debug + Send + Sync + 'static;
 		}

engine/src/state_chain_observer/sc_observer/mod.rs

@@ -164,7 +164,7 @@ async fn handle_signing_request<'a, StateChainClient, MultisigClient, C, I>(
 		scope.spawn(async move {
 			match signing_result_future.await {
 				Ok(signatures) => {
-					state_chain_client
+					let _result = state_chain_client
 						.submit_unsigned_extrinsic(pallet_cf_threshold_signature::Call::<
 							state_chain_runtime::Runtime,
 							I,

engine/src/state_chain_observer/sc_observer/tests.rs

@@ -242,7 +242,7 @@ ChainCrypto>::ThresholdSignature: std::convert::From<<C as CryptoScheme>::Signat
 			signature: signatures.to_threshold_signature(),
 		}))
 		.once()
-		.return_once(|_: pallet_cf_threshold_signature::Call<Runtime, I>| H256::default());
+		.return_once(|_: pallet_cf_threshold_signature::Call<Runtime, I>| Ok(H256::default()));
 
 	let state_chain_client = Arc::new(state_chain_client);
 	task_scope(|scope| {