From cdbc77bf97043425e8ad26f5520cfc234f1eca9c Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 25 Aug 2021 07:23:03 +0000 Subject: [PATCH] Add parameter to allow setting max-retries for SendTransaction rpc (backport #19387) (#19416) * 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 * Fix conflicts Co-authored-by: Tyera Eulberg Co-authored-by: Tyera Eulberg --- client/src/rpc_config.rs | 1 + docs/src/developing/clients/jsonrpc-api.md | 2 + rpc/src/rpc.rs | 4 + rpc/src/send_transaction_service.rs | 101 ++++++++++++++++++++- 4 files changed, 105 insertions(+), 3 deletions(-) diff --git a/client/src/rpc_config.rs b/client/src/rpc_config.rs index cd9e2fa11d..93d78f47ca 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/docs/src/developing/clients/jsonrpc-api.md b/docs/src/developing/clients/jsonrpc-api.md index a88d2bd40c..c975c202e8 100644 --- a/docs/src/developing/clients/jsonrpc-api.md +++ b/docs/src/developing/clients/jsonrpc-api.md @@ -3246,6 +3246,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: diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index f608b98315..915b599fc8 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -2142,6 +2142,7 @@ fn _send_transaction( wire_transaction: Vec, last_valid_block_height: u64, durable_nonce_info: Option<(Pubkey, Hash)>, + max_retries: Option, ) -> Result { if transaction.signatures.is_empty() { return Err(RpcCustomError::TransactionSignatureVerificationFailure.into()); @@ -2152,6 +2153,7 @@ fn _send_transaction( wire_transaction, last_valid_block_height, durable_nonce_info, + max_retries, ); meta.transaction_sender .lock() @@ -3007,6 +3009,7 @@ pub mod rpc_full { wire_transaction, last_valid_block_height, None, + None, ) } @@ -3100,6 +3103,7 @@ pub mod rpc_full { wire_transaction, last_valid_block_height, durable_nonce_info, + config.max_retries, ) } diff --git a/rpc/src/send_transaction_service.rs b/rpc/src/send_transaction_service.rs index cd3630ec3b..3162eab356 100644 --- a/rpc/src/send_transaction_service.rs +++ b/rpc/src/send_transaction_service.rs @@ -33,6 +33,8 @@ pub struct TransactionInfo { pub wire_transaction: Vec, pub last_valid_block_height: u64, pub durable_nonce_info: Option<(Pubkey, Hash)>, + pub max_retries: Option, + retries: usize, } impl TransactionInfo { @@ -41,12 +43,15 @@ impl TransactionInfo { wire_transaction: Vec, last_valid_block_height: u64, durable_nonce_info: Option<(Pubkey, Hash)>, + max_retries: Option, ) -> Self { Self { signature, wire_transaction, last_valid_block_height, durable_nonce_info, + max_retries, + retries: 0, } } } @@ -98,6 +103,7 @@ struct ProcessTransactionsResult { rooted: u64, expired: u64, retried: u64, + max_retries_elapsed: u64, failed: u64, retained: u64, } @@ -220,7 +226,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); } @@ -247,6 +253,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 => { @@ -254,6 +268,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() @@ -393,6 +408,7 @@ mod test { vec![], root_bank.block_height() - 1, None, + None, ), ); let result = SendTransactionService::process_transactions( @@ -416,7 +432,13 @@ mod test { info!("Rooted transactions are dropped..."); transactions.insert( rooted_signature, - TransactionInfo::new(rooted_signature, vec![], working_bank.block_height(), None), + TransactionInfo::new( + rooted_signature, + vec![], + working_bank.block_height(), + None, + None, + ), ); let result = SendTransactionService::process_transactions( &working_bank, @@ -439,7 +461,13 @@ mod test { info!("Failed transactions are dropped..."); transactions.insert( failed_signature, - TransactionInfo::new(failed_signature, vec![], working_bank.block_height(), None), + TransactionInfo::new( + failed_signature, + vec![], + working_bank.block_height(), + None, + None, + ), ); let result = SendTransactionService::process_transactions( &working_bank, @@ -467,6 +495,7 @@ mod test { vec![], working_bank.block_height(), None, + None, ), ); let result = SendTransactionService::process_transactions( @@ -496,6 +525,7 @@ mod test { vec![], working_bank.block_height(), None, + None, ), ); let result = SendTransactionService::process_transactions( @@ -515,6 +545,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.block_height(), + None, + Some(0), + ), + ); + transactions.insert( + Signature::new(&[2; 64]), + TransactionInfo::new( + Signature::default(), + vec![], + working_bank.block_height(), + 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] @@ -575,6 +663,7 @@ mod test { vec![], last_valid_block_height, Some((nonce_address, durable_nonce)), + None, ), ); let result = SendTransactionService::process_transactions( @@ -602,6 +691,7 @@ mod test { vec![], last_valid_block_height, Some((nonce_address, Hash::new_unique())), + None, ), ); let result = SendTransactionService::process_transactions( @@ -631,6 +721,7 @@ mod test { vec![], last_valid_block_height, Some((nonce_address, Hash::new_unique())), + None, ), ); let result = SendTransactionService::process_transactions( @@ -658,6 +749,7 @@ mod test { vec![], root_bank.block_height() - 1, Some((nonce_address, durable_nonce)), + None, ), ); let result = SendTransactionService::process_transactions( @@ -686,6 +778,7 @@ mod test { vec![], last_valid_block_height, Some((nonce_address, Hash::new_unique())), // runtime should advance nonce on failed transactions + None, ), ); let result = SendTransactionService::process_transactions( @@ -714,6 +807,7 @@ mod test { vec![], last_valid_block_height, Some((nonce_address, Hash::new_unique())), // runtime advances nonce when transaction lands + None, ), ); let result = SendTransactionService::process_transactions( @@ -743,6 +837,7 @@ mod test { vec![], last_valid_block_height, Some((nonce_address, durable_nonce)), + None, ), ); let result = SendTransactionService::process_transactions(