Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Use timeout to allow RpcClient to retry initial transaction confirmation #18311

Merged
merged 5 commits into from
Jun 30, 2021

Conversation

CriesofCarrots
Copy link
Contributor

@CriesofCarrots CriesofCarrots commented Jun 29, 2021

Problem

In a load-balanced RPC setup with imperfect or nonexistent session affinity, an RpcClient may query a recent-blockhash and send a transaction to one node, and attempt to confirm the transaction on another. If this second node is behind or on a short-lived fork, it may not have yet seen the blockhash, causing the client to exit early here:

if self
.get_fee_calculator_for_blockhash_with_commitment(
recent_blockhash,
CommitmentConfig::processed(),
)?
.value
.is_none()

even though the transaction may have actually landed.

Summary of Changes

Add a configurable timeout for initial transaction confirmation, allowing get_signature_status retries even when the blockhash cannot be found. In a single-node RPC setup, this shouldn't add any additional time.
Also, a little cleanup in RpcClient to use the commitment() method (this is most of the churn), and refactoring in RpcClient to make this cleaner to use and make adding additional RpcClient config parameters easier in the future.

t-nelson
t-nelson previously approved these changes Jun 29, 2021
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Cleanups look great! Comments below should be taken as nits 😉

client/src/rpc_client.rs Outdated Show resolved Hide resolved
client/src/rpc_client.rs Show resolved Hide resolved
@CriesofCarrots CriesofCarrots force-pushed the rpc-client-confirm-retries branch from 6b01e26 to 7e747f0 Compare June 29, 2021 23:22
@mergify mergify bot dismissed t-nelson’s stale review June 29, 2021 23:23

Pull request has been modified.

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label Jun 29, 2021
@codecov
Copy link

codecov bot commented Jun 30, 2021

Codecov Report

Merging #18311 (7e747f0) into master (9d983a3) will increase coverage by 0.0%.
The diff coverage is 55.7%.

@@           Coverage Diff           @@
##           master   #18311   +/-   ##
=======================================
  Coverage    82.3%    82.3%           
=======================================
  Files         433      433           
  Lines      121160   121193   +33     
=======================================
+ Hits        99793    99846   +53     
+ Misses      21367    21347   -20     

@mergify mergify bot merged commit 9d4428d into solana-labs:master Jun 30, 2021
mergify bot pushed a commit that referenced this pull request Jun 30, 2021
…ion (#18311)

* Tidying: relocate function

* Use proper helper method for RpcClient commitment

* Add RpcClientConfig

* Add configurable confirm_transaction_initial_timeout

* Use default 5s timeout for initial tx confirmation

(cherry picked from commit 9d4428d)
mergify bot pushed a commit that referenced this pull request Jun 30, 2021
…ion (#18311)

* Tidying: relocate function

* Use proper helper method for RpcClient commitment

* Add RpcClientConfig

* Add configurable confirm_transaction_initial_timeout

* Use default 5s timeout for initial tx confirmation

(cherry picked from commit 9d4428d)
mergify bot added a commit that referenced this pull request Jun 30, 2021
…ion (#18311) (#18316)

* Tidying: relocate function

* Use proper helper method for RpcClient commitment

* Add RpcClientConfig

* Add configurable confirm_transaction_initial_timeout

* Use default 5s timeout for initial tx confirmation

(cherry picked from commit 9d4428d)

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
mergify bot added a commit that referenced this pull request Jun 30, 2021
…ion (backport #18311) (#18315)

* Use timeout to allow RpcClient to retry initial transaction confirmation (#18311)

* Tidying: relocate function

* Use proper helper method for RpcClient commitment

* Add RpcClientConfig

* Add configurable confirm_transaction_initial_timeout

* Use default 5s timeout for initial tx confirmation

(cherry picked from commit 9d4428d)

* Fixup deprecated method

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
Co-authored-by: Tyera Eulberg <tyera@solana.com>
@CriesofCarrots CriesofCarrots deleted the rpc-client-confirm-retries branch July 28, 2021 22:31
This was referenced Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants