499 make vault retry stellar transaction submission for tx insufficie…

…nt fee error (#500)

* Run `cargo fmt --all`

* Implement resubmission with fee bump transaction

* Replace fee-bump transaction with normal transaction

* Change log level

* Change other test

* Bound tx fee to a maximum

* Update clients/wallet/src/resubmissions.rs

clients/wallet/src/horizon/horizon.rs

@@ -267,7 +267,7 @@ impl<C: HorizonClient + Clone> HorizonFetcher<C> {
 			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?;

clients/wallet/src/horizon/responses.rs

@@ -181,8 +181,8 @@ pub struct TransactionResponse {
 	pub fee_account: Vec<u8>,
 	#[serde(deserialize_with = "de_str_to_u64")]
 	pub fee_charged: u64,
-	#[serde(deserialize_with = "de_str_to_bytes")]
-	pub max_fee: Vec<u8>,
+	#[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<u8>,
@@ -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))

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<String>,
+	) -> Result<TransactionResponse, Error> {
+		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

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"))]

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::<MainnetExecutor>::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::<TestnetExecutor>::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