From 2825f82bee41f55e4c70bf2d278e3b01d6e7f50a Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 25 Aug 2021 16:47:47 +0000 Subject: [PATCH] Add parameter to allow setting max-retries for SendTransaction rpc (backport #19387) (#19415) * Add parameter to allow setting max-retries for SendTransaction rpc (#19387) * Add parameter to cap rpc send retries for a tx * Add parameter to docs (cherry picked from commit 7482861f4b94df81a26d5061626f925c667b70ea) # Conflicts: # banks-server/src/banks_server.rs # core/src/rpc.rs # core/src/send_transaction_service.rs * Fix conflicts Co-authored-by: Tyera Eulberg Co-authored-by: Tyera Eulberg --- client/src/rpc_config.rs | 1 + core/src/rpc.rs | 12 ++- core/src/send_transaction_service.rs | 110 +++++++++++++++++++-- docs/src/developing/clients/jsonrpc-api.md | 2 + 4 files changed, 118 insertions(+), 7 deletions(-) diff --git a/client/src/rpc_config.rs b/client/src/rpc_config.rs index 2ab983379b..92c03d3ddf 100644 --- a/client/src/rpc_config.rs +++ b/client/src/rpc_config.rs @@ -21,6 +21,7 @@ pub struct RpcSendTransactionConfig { pub skip_preflight: bool, pub preflight_commitment: Option, pub encoding: Option, + pub max_retries: Option, } #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] diff --git a/core/src/rpc.rs b/core/src/rpc.rs index 2e248a20af..359b9a4e8d 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -2163,6 +2163,7 @@ fn _send_transaction( wire_transaction: Vec, last_valid_slot: Slot, durable_nonce_info: Option<(Pubkey, Hash)>, + max_retries: Option, ) -> Result { if transaction.signatures.is_empty() { return Err(RpcCustomError::TransactionSignatureVerificationFailure.into()); @@ -2173,6 +2174,7 @@ fn _send_transaction( wire_transaction, last_valid_slot, durable_nonce_info, + max_retries, ); meta.transaction_sender .lock() @@ -3112,7 +3114,14 @@ pub mod rpc_full { Error::internal_error() })?; - _send_transaction(meta, transaction, wire_transaction, last_valid_slot, None) + _send_transaction( + meta, + transaction, + wire_transaction, + last_valid_slot, + None, + None, + ) } fn send_transaction( @@ -3204,6 +3213,7 @@ pub mod rpc_full { wire_transaction, last_valid_slot, durable_nonce_info, + config.max_retries, ) } diff --git a/core/src/send_transaction_service.rs b/core/src/send_transaction_service.rs index 782f197af9..41a2e06e7a 100644 --- a/core/src/send_transaction_service.rs +++ b/core/src/send_transaction_service.rs @@ -35,6 +35,8 @@ pub struct TransactionInfo { pub wire_transaction: Vec, pub last_valid_slot: Slot, pub durable_nonce_info: Option<(Pubkey, Hash)>, + pub max_retries: Option, + retries: usize, } impl TransactionInfo { @@ -43,12 +45,15 @@ impl TransactionInfo { wire_transaction: Vec, last_valid_slot: Slot, durable_nonce_info: Option<(Pubkey, Hash)>, + max_retries: Option, ) -> Self { Self { signature, wire_transaction, last_valid_slot, durable_nonce_info, + max_retries, + retries: 0, } } } @@ -100,6 +105,7 @@ struct ProcessTransactionsResult { rooted: u64, expired: u64, retried: u64, + max_retries_elapsed: u64, failed: u64, retained: u64, } @@ -222,7 +228,7 @@ impl SendTransactionService { ) -> ProcessTransactionsResult { let mut result = ProcessTransactionsResult::default(); - transactions.retain(|signature, transaction_info| { + transactions.retain(|signature, mut transaction_info| { if transaction_info.durable_nonce_info.is_some() { inc_new_counter_info!("send_transaction_service-nonced", 1); } @@ -249,6 +255,14 @@ impl SendTransactionService { inc_new_counter_info!("send_transaction_service-expired", 1); return false; } + if let Some(max_retries) = transaction_info.max_retries { + if transaction_info.retries >= max_retries { + info!("Dropping transaction due to max retries: {}", signature); + result.max_retries_elapsed += 1; + inc_new_counter_info!("send_transaction_service-max_retries", 1); + return false; + } + } match working_bank.get_signature_status_slot(signature) { None => { @@ -256,6 +270,7 @@ impl SendTransactionService { // dropped or landed in another fork. Re-send it info!("Retrying transaction: {}", signature); result.retried += 1; + transaction_info.retries += 1; inc_new_counter_info!("send_transaction_service-retry", 1); let addresses = leader_info .as_ref() @@ -387,7 +402,13 @@ mod test { info!("Expired transactions are dropped..."); transactions.insert( Signature::default(), - TransactionInfo::new(Signature::default(), vec![], root_bank.slot() - 1, None), + TransactionInfo::new( + Signature::default(), + vec![], + root_bank.slot() - 1, + None, + None, + ), ); let result = SendTransactionService::process_transactions( &working_bank, @@ -410,7 +431,7 @@ mod test { info!("Rooted transactions are dropped..."); transactions.insert( rooted_signature, - TransactionInfo::new(rooted_signature, vec![], working_bank.slot(), None), + TransactionInfo::new(rooted_signature, vec![], working_bank.slot(), None, None), ); let result = SendTransactionService::process_transactions( &working_bank, @@ -433,7 +454,7 @@ mod test { info!("Failed transactions are dropped..."); transactions.insert( failed_signature, - TransactionInfo::new(failed_signature, vec![], working_bank.slot(), None), + TransactionInfo::new(failed_signature, vec![], working_bank.slot(), None, None), ); let result = SendTransactionService::process_transactions( &working_bank, @@ -456,7 +477,13 @@ mod test { info!("Non-rooted transactions are kept..."); transactions.insert( non_rooted_signature, - TransactionInfo::new(non_rooted_signature, vec![], working_bank.slot(), None), + TransactionInfo::new( + non_rooted_signature, + vec![], + working_bank.slot(), + None, + None, + ), ); let result = SendTransactionService::process_transactions( &working_bank, @@ -480,7 +507,13 @@ mod test { info!("Unknown transactions are retried..."); transactions.insert( Signature::default(), - TransactionInfo::new(Signature::default(), vec![], working_bank.slot(), None), + TransactionInfo::new( + Signature::default(), + vec![], + working_bank.slot(), + None, + None, + ), ); let result = SendTransactionService::process_transactions( &working_bank, @@ -499,6 +532,64 @@ mod test { ..ProcessTransactionsResult::default() } ); + transactions.clear(); + + info!("Transactions are only retried until max_retries"); + transactions.insert( + Signature::new(&[1; 64]), + TransactionInfo::new( + Signature::default(), + vec![], + working_bank.slot(), + None, + Some(0), + ), + ); + transactions.insert( + Signature::new(&[2; 64]), + TransactionInfo::new( + Signature::default(), + vec![], + working_bank.slot(), + None, + Some(1), + ), + ); + let result = SendTransactionService::process_transactions( + &working_bank, + &root_bank, + &send_socket, + &tpu_address, + &mut transactions, + &None, + leader_forward_count, + ); + assert_eq!(transactions.len(), 1); + assert_eq!( + result, + ProcessTransactionsResult { + retried: 1, + max_retries_elapsed: 1, + ..ProcessTransactionsResult::default() + } + ); + let result = SendTransactionService::process_transactions( + &working_bank, + &root_bank, + &send_socket, + &tpu_address, + &mut transactions, + &None, + leader_forward_count, + ); + assert!(transactions.is_empty()); + assert_eq!( + result, + ProcessTransactionsResult { + max_retries_elapsed: 1, + ..ProcessTransactionsResult::default() + } + ); } #[test] @@ -559,6 +650,7 @@ mod test { vec![], last_valid_slot, Some((nonce_address, durable_nonce)), + None, ), ); let result = SendTransactionService::process_transactions( @@ -586,6 +678,7 @@ mod test { vec![], last_valid_slot, Some((nonce_address, Hash::new_unique())), + None, ), ); let result = SendTransactionService::process_transactions( @@ -615,6 +708,7 @@ mod test { vec![], last_valid_slot, Some((nonce_address, Hash::new_unique())), + None, ), ); let result = SendTransactionService::process_transactions( @@ -642,6 +736,7 @@ mod test { vec![], root_bank.slot() - 1, Some((nonce_address, durable_nonce)), + None, ), ); let result = SendTransactionService::process_transactions( @@ -670,6 +765,7 @@ mod test { vec![], last_valid_slot, Some((nonce_address, Hash::new_unique())), // runtime should advance nonce on failed transactions + None, ), ); let result = SendTransactionService::process_transactions( @@ -698,6 +794,7 @@ mod test { vec![], last_valid_slot, Some((nonce_address, Hash::new_unique())), // runtime advances nonce when transaction lands + None, ), ); let result = SendTransactionService::process_transactions( @@ -727,6 +824,7 @@ mod test { vec![], last_valid_slot, Some((nonce_address, durable_nonce)), + None, ), ); let result = SendTransactionService::process_transactions( diff --git a/docs/src/developing/clients/jsonrpc-api.md b/docs/src/developing/clients/jsonrpc-api.md index 33e3275606..c504d580f3 100644 --- a/docs/src/developing/clients/jsonrpc-api.md +++ b/docs/src/developing/clients/jsonrpc-api.md @@ -3241,6 +3241,8 @@ submission. - `skipPreflight: ` - if true, skip the preflight transaction checks (default: false) - `preflightCommitment: ` - (optional) [Commitment](jsonrpc-api.md#configuring-state-commitment) level to use for preflight (default: `"finalized"`). - `encoding: ` - (optional) Encoding used for the transaction data. Either `"base58"` (*slow*, **DEPRECATED**), or `"base64"`. (default: `"base58"`). + - `maxRetries: ` - (optional) Maximum number of times for the RPC node to retry sending the transaction to the leader. + If this parameter not provided, the RPC node will retry the transaction until it is finalized or until the blockhash expires. #### Results: