diff --git a/clients/wallet/src/horizon/horizon.rs b/clients/wallet/src/horizon/horizon.rs index 25c0c3274..25a2b9319 100644 --- a/clients/wallet/src/horizon/horizon.rs +++ b/clients/wallet/src/horizon/horizon.rs @@ -267,7 +267,7 @@ impl HorizonFetcher { future::join(issue_map.read(), memos_to_issue_ids.read()).await; if issue_map.is_empty() || memos_to_issue_ids.is_empty() { - tracing::info!("fetch_horizon_and_process_new_transactions(): nothing to traverse"); + tracing::debug!("fetch_horizon_and_process_new_transactions(): nothing to traverse"); return Ok(last_cursor) } let mut txs_iter = self.fetch_transactions_iter(last_cursor).await?; diff --git a/clients/wallet/src/horizon/responses.rs b/clients/wallet/src/horizon/responses.rs index 90f13e20c..86062419f 100644 --- a/clients/wallet/src/horizon/responses.rs +++ b/clients/wallet/src/horizon/responses.rs @@ -181,8 +181,8 @@ pub struct TransactionResponse { pub fee_account: Vec, #[serde(deserialize_with = "de_str_to_u64")] pub fee_charged: u64, - #[serde(deserialize_with = "de_str_to_bytes")] - pub max_fee: Vec, + #[serde(deserialize_with = "de_str_to_u64")] + pub max_fee: u64, operation_count: u32, #[serde(deserialize_with = "de_str_to_bytes")] pub envelope_xdr: Vec, @@ -219,7 +219,7 @@ impl Debug for TransactionResponse { .field("source_account_sequence", &debug_str_or_vec_u8!(&self.source_account_sequence)) .field("fee_account", &debug_str_or_vec_u8!(&self.fee_account)) .field("fee_charged", &self.fee_charged) - .field("max_fee", &debug_str_or_vec_u8!(&self.max_fee)) + .field("max_fee", &self.max_fee) .field("operation_count", &self.operation_count) .field("envelope_xdr", &debug_str_or_vec_u8!(&self.envelope_xdr)) .field("result_xdr", &debug_str_or_vec_u8!(&self.result_xdr)) diff --git a/clients/wallet/src/resubmissions.rs b/clients/wallet/src/resubmissions.rs index f4a0f55e1..0585ef011 100644 --- a/clients/wallet/src/resubmissions.rs +++ b/clients/wallet/src/resubmissions.rs @@ -21,6 +21,8 @@ use primitives::stellar::{types::SequenceNumber, PublicKey}; use reqwest::Client; pub const RESUBMISSION_INTERVAL_IN_SECS: u64 = 1800; +// The maximum fee we want to charge for a transaction +const MAXIMUM_TX_FEE: u32 = 10_000_000; // 1 XLM #[cfg_attr(test, mockable)] impl StellarWallet { @@ -150,6 +152,8 @@ impl StellarWallet { return self.handle_tx_bad_seq_error_with_xdr(envelope_xdr).await.map(Some), "tx_internal_error" => return self.handle_tx_internal_error(envelope_xdr).await.map(Some), + "tx_insufficient_fee" => + return self.handle_tx_insufficient_fee_error(envelope_xdr).await.map(Some), _ => { if let Ok(env) = decode_to_envelope(envelope_xdr) { self.remove_tx_envelope_from_cache(&env); @@ -191,6 +195,36 @@ impl StellarWallet { self.submit_transaction(envelope).await } + + // We encountered an insufficient fee error and try submitting the transaction again with a + // higher fee. We'll bump the fee by 10x the original fee. We don't use a FeeBumpTransaction + // because this operation is not supported by the stellar-relay pallet yet. + async fn handle_tx_insufficient_fee_error( + &self, + envelope_xdr_as_str_opt: &Option, + ) -> Result { + let tx_envelope = decode_to_envelope(envelope_xdr_as_str_opt)?; + let mut tx = tx_envelope.get_transaction().ok_or(DecodeError)?; + + // Check if we already submitted this transaction + if !self.is_transaction_already_submitted(&tx).await { + // Remove original transaction. + // The same envelope will be saved again using a different sequence number + self.remove_tx_envelope_from_cache(&tx_envelope); + + // Bump the fee by 10x + tx.fee = tx.fee * 10; + if tx.fee > MAXIMUM_TX_FEE { + tx.fee = MAXIMUM_TX_FEE; + } + + return self.bump_sequence_number_and_submit(tx).await + } + + tracing::error!("handle_tx_insufficient_fee_error(): Similar transaction already submitted. Skipping {:?}", tx); + + Err(ResubmissionError("Transaction already submitted".to_string())) + } } fn is_source_account_match(public_key: &PublicKey, tx: &TransactionResponse) -> bool { @@ -680,6 +714,47 @@ mod test { wallet.remove_cache_dir(); } + #[tokio::test] + #[serial] + async fn check_handle_tx_insufficient_fee_error_with_envelope() { + let wallet = + wallet_with_storage("resources/check_handle_tx_insufficient_fee_error_with_envelope") + .expect("should return a wallet") + .clone(); + let wallet = wallet.write().await; + + // This is the fee we will bump by 10x + let base_fee = 99; + // This is the new maximum fee we expect to be charged + let bumped_fee = base_fee * 10; + + let sequence = wallet.get_sequence().await.expect("return a sequence"); + let envelope = wallet + .create_payment_envelope( + default_destination(), + StellarAsset::native(), + 10, + rand::random(), + base_fee, + sequence + 1, + ) + .expect("should return an envelope"); + + let envelope_xdr = envelope.to_base64_xdr(); + // Convert vec to string (because the HorizonSubmissionError always returns a string) + let envelope_xdr = + Some(String::from_utf8(envelope_xdr).expect("should create string from vec")); + + let result = wallet.handle_tx_insufficient_fee_error(&envelope_xdr).await; + + assert!(result.is_ok()); + let response = result.unwrap(); + assert!(response.successful); + assert_eq!(response.max_fee, bumped_fee as u64); + + wallet.remove_cache_dir(); + } + #[tokio::test] #[serial] async fn check_handle_error() { @@ -730,6 +805,48 @@ mod test { } } + // tx_insufficient_fee test + { + let envelope = wallet + .create_dummy_envelope_no_signature(19) + .await + .expect("returns an envelope"); + let envelope_xdr = envelope.to_base64_xdr(); + let envelope_xdr = String::from_utf8(envelope_xdr).ok(); + + // result is success + { + let error = Error::HorizonSubmissionError { + title: "title".to_string(), + status: 400, + reason: "tx_insufficient_fee".to_string(), + result_code_op: vec![], + envelope_xdr, + }; + + StellarWallet::is_transaction_already_submitted + .mock_safe(move |_, _| MockResult::Return(Box::pin(async move { false }))); + + assert!(wallet.handle_error(error).await.is_ok()); + } + + // result is error + { + let error = Error::HorizonSubmissionError { + title: "title".to_string(), + status: 400, + reason: "tx_insufficient_fee".to_string(), + result_code_op: vec![], + envelope_xdr: None, + }; + + match wallet.handle_error(error).await { + Err(Error::ResubmissionError(_)) => assert!(true), + other => panic!("expecting Error::ResubmissionError, found: {other:?}"), + }; + } + } + // tx_internal_error test { let envelope = wallet diff --git a/pallets/oracle/rpc/runtime-api/src/lib.rs b/pallets/oracle/rpc/runtime-api/src/lib.rs index 55ba1f93c..df9b3c541 100644 --- a/pallets/oracle/rpc/runtime-api/src/lib.rs +++ b/pallets/oracle/rpc/runtime-api/src/lib.rs @@ -2,11 +2,11 @@ #![cfg_attr(not(feature = "std"), no_std)] use codec::{Codec, Decode, Encode}; -use frame_support::{dispatch::DispatchError}; +use frame_support::dispatch::DispatchError; use primitives::UnsignedFixedPoint; +use scale_info::TypeInfo; #[cfg(feature = "std")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use scale_info::TypeInfo; #[derive(Eq, PartialEq, Encode, Decode, Default, TypeInfo)] #[cfg_attr(feature = "std", derive(Debug, Serialize, Deserialize))] #[cfg_attr(feature = "std", serde(rename_all = "camelCase"))] diff --git a/testchain/node/src/service.rs b/testchain/node/src/service.rs index 784aa06d8..98b94de78 100644 --- a/testchain/node/src/service.rs +++ b/testchain/node/src/service.rs @@ -4,7 +4,9 @@ use futures::StreamExt; use sc_client_api::BlockBackend; use sc_consensus_aura::{ImportQueueParams, SlotProportion, StartAuraParams}; use sc_consensus_grandpa::SharedVoterState; -use sc_executor::{NativeElseWasmExecutor, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY, HeapAllocStrategy}; +use sc_executor::{ + HeapAllocStrategy, NativeElseWasmExecutor, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY, +}; use sc_service::{ error::Error as ServiceError, Configuration, RpcHandlers, TFullBackend, TFullClient, TaskManager, @@ -115,7 +117,7 @@ pub fn new_partial_mainnet( .with_max_runtime_instances(config.max_runtime_instances) .with_runtime_cache_size(config.runtime_cache_size) .build(); - + let executor = NativeElseWasmExecutor::::new_with_wasm_executor(wasm); let (client, backend, keystore_container, task_manager) = @@ -219,7 +221,6 @@ pub fn new_partial_testnet( >, ServiceError, > { - let telemetry = config .telemetry_endpoints .clone() @@ -242,7 +243,7 @@ pub fn new_partial_testnet( .with_max_runtime_instances(config.max_runtime_instances) .with_runtime_cache_size(config.runtime_cache_size) .build(); - + let executor = NativeElseWasmExecutor::::new_with_wasm_executor(wasm); let (client, backend, keystore_container, task_manager) = @@ -461,8 +462,7 @@ pub fn new_full(mut config: Configuration) -> Result<(TaskManager, RpcHandlers), // if the node isn't actively participating in consensus then it doesn't // need a keystore, regardless of which protocol we use below. - let keystore = - if role.is_authority() { Some(keystore_container.keystore()) } else { None }; + let keystore = if role.is_authority() { Some(keystore_container.keystore()) } else { None }; let grandpa_config = sc_consensus_grandpa::Config { // FIXME #1578 make this available through chainspec