Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v1.18: RPC sendTransaction: be more liberal in determining and setting last_valid_block_height for skip_preflight case (backport of #483) #873

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Apr 17, 2024

Problem

Transactions submitted to RPC sendTransaction with skipPreflight: true frequently aren't retried by the SendTransactionService (STS). This happens when the transaction's recent blockhash is newer than the RPC node's finalized bank, because unless preflightCommitment is speficied, the RPC handler uses the default finalized bank to find a last_valid_block_height to provide to the STS. If the blockhash cannot be found (as when too new), last_valid_block_height is set to 0, and the STS only broadcasts the transaction once.

Summary of Changes

  • If skip_preflight is set, use the processed commitment Bank to search for the recent blockhash.
  • If the last_valid_block_height still cannot be determined, use the durable-nonce logic to retry the transaction for an arbitrary, fixed number of blocks (skip_preflight being an indication that a client wants their transactions to be sprayed around regardless of likelihood of success).

Fixes #479


This is an automatic backport of pull request #483 done by [Mergify](https://mergify.com).

…valid_block_height for skip_preflight case (#483)

* RPC sendTransaction: if skip_preflight, use processed-commitment Bank for last_valid_block_height and sanitization

* Use nonce retry logic for skip_preflight transactions if blockhash was not found

(cherry picked from commit 65a24d5)
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.5%. Comparing base (2444794) to head (48f347d).

Additional details and impacted files
@@            Coverage Diff            @@
##            v1.18     #873     +/-   ##
=========================================
- Coverage    81.5%    81.5%   -0.1%     
=========================================
  Files         827      827             
  Lines      224845   224848      +3     
=========================================
- Hits       183470   183446     -24     
- Misses      41375    41402     +27     

Copy link

@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.

oh yeah... forgot about this one. :shipit:

@CriesofCarrots CriesofCarrots merged commit d7eb8f9 into v1.18 Apr 18, 2024
35 checks passed
@CriesofCarrots CriesofCarrots deleted the mergify/bp/v1.18/pr-483 branch April 18, 2024 02:20
anwayde pushed a commit to firedancer-io/agave that referenced this pull request Jul 23, 2024
…g last_valid_block_height for skip_preflight case (backport of anza-xyz#483) (anza-xyz#873)

RPC sendTransaction: be more liberal in determining and setting last_valid_block_height for skip_preflight case (anza-xyz#483)

* RPC sendTransaction: if skip_preflight, use processed-commitment Bank for last_valid_block_height and sanitization

* Use nonce retry logic for skip_preflight transactions if blockhash was not found

(cherry picked from commit 65a24d5)

Co-authored-by: Tyera <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants