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

Despite skip_preflight being true, preflight_commitment is nevertheless used #479

Closed
steveluscher opened this issue Mar 28, 2024 · 0 comments · Fixed by #483
Closed

Despite skip_preflight being true, preflight_commitment is nevertheless used #479

steveluscher opened this issue Mar 28, 2024 · 0 comments · Fixed by #483

Comments

@steveluscher
Copy link

Problem

I think that people have an expectation that skip_preflight makes it so that the value of preflight_commitment is not used. In fact it's used unconditionally to prepare the ‘preflight bank’:

https://github.com/anza-xyz/agave/blob/master/rpc/src/rpc.rs#L3650-L3653

…from which we compute the last_valid_block_height:

https://github.com/anza-xyz/agave/blob/master/rpc/src/rpc.rs#L3658-L3660

…and the address_loader for sanitize_transaction:

https://github.com/anza-xyz/agave/blob/master/rpc/src/rpc.rs#L3655

Is it possible that this could nevertheless materially change send_transaction's behaviour? @t-nelson suggested:

that commitment is used to query which bank to use for last_valid_block_height look ups. this is what rpc sts uses to give up on retries

Proposed Solution

Only use the preflight_commitment when skip_preflight is true.

@CriesofCarrots CriesofCarrots removed their assignment Mar 28, 2024
steveluscher added a commit to solana-labs/solana-web3.js that referenced this issue Apr 3, 2024
…eflight checks (#2415)

# Summary

There's a bug in the `sendTransaction` RPC method where, when bypassing preflight, we nevertheless materially use the value of `preflightCommitment` to determine how to behave when sending the transaction.

If you supply _nothing_ – as you might think appropriate when _skipping_ preflight – then the default of `finalized` will be used. Far from irrelevant, such a value can actually affect the retry behaviour of the send-transaction-service (STS). Read anza-xyz/agave#483 for more detail.

In this PR, we try to get ahead of anza-xyz/agave#483 by setting this value to `processed` in the client. Until the server PR is deployed widely, this should cover those who choose to upgrade

Addresses anza-xyz/agave#479

# Test plan

```
pnpm turbo test:unit:node test:unit:browser
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants