Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions engine/src/state_chain_observer/client/extrinsic_api/common.rs
Original file line number Diff line number Diff line change
@@ -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>(
Expand All @@ -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)),
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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
Expand Down
29 changes: 22 additions & 7 deletions engine/src/state_chain_observer/client/extrinsic_api/unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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()),
}
Expand All @@ -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
Expand Down
24 changes: 16 additions & 8 deletions engine/src/state_chain_observer/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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(),
),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {}
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion engine/src/state_chain_observer/sc_observer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion engine/src/state_chain_observer/sc_observer/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
Loading