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

Conversation

@CriesofCarrots
Copy link
Contributor

@CriesofCarrots CriesofCarrots commented Aug 19, 2022

Problem

#26709 caches recent prioritization-fee data, but there is no way for users to access this data to pick the fee for their own transaction.

Summary of Changes

  • Add new PrioritizationFeeCache api that returns a fee value for each slot in the cache, where the value is the max of the slot fee or write-lock fees for any accounts provided
  • Add getRecentPrioritizationFees RPC endpoint that returns all prioritization-fee data for all recent slots in the cache using that api

This needs a rebase on #26709; only the last 3 commits are relevant for now.

Open questions:

  • It feels weird to have the return data in no particular order. Should we sort the return data? Or should we use a BTreeMap in the cache? Or should we return Slots as well, and sort by that?
  • What is the max number of account pubkeys we should allow?
  • Do we have use cases for the other PrioritizationFeeCache apis: get_prioritization_fees(), get_account_prioritization_fees()

To come:

  • RPC unit test
  • JSONRPC docs
  • Update max inputs

Fixes #26561

cc @taozhu-chicago @jdavis103

@CriesofCarrots
Copy link
Contributor Author

@taozhu-chicago , can you please look at my open questions, and also a couple places I'm about to comment in the code?

@tao-stones
Copy link
Contributor

tao-stones commented Aug 20, 2022

return data in no particular order

I didn't think order matter initially - user wants average, or max, or min, or top 5% etc. Now you said it, order by slots can project trends, which is useful information, just not 100% convinced necessary at this point.

What is the max number of account pubkeys we should allow?

In query? I was thinking just one account per query, if that's easier. Otherwise, by the MAX_ACCOUNT_LIMIT per transaction (it is defined somewhere)

Do we have use cases for the other PrioritizationFeeCache apis: get_prioritization_fees(), get_account_prioritization_fees()

No, they are only for RPC queries. (I'm reusing prioriitization_fee.rs in banking_stage.)

@tao-stones tao-stones requested a review from jdavis103 August 22, 2022 14:06
@tao-stones
Copy link
Contributor

Ran this branch on GCE cluster, bench-tps sends transfer transactions with random priority fee (between 0..50); did simple test queries, looks getting correct results back:

  1. query min fee with two accounts, returned min fee from 150 blocks, within range of 0..50.
% curl -X POST -H "Content-Type: application/json" -d '{"jsonrpc": "2.0","id":1,"method":"getRecentPrioritizationFees","params":[["AodN7ggT5YMPGzgnomBhZxiXKCGvZiWatYKKhNtVQF8A", "9joYTaFnFtA5GXsi9c3pLnRmkCWw1xzSzXNMSNx7m9jn"],{"commitment":"confirmed"}]}' 34.82.0.171:8899
{"jsonrpc":"2.0","result":{"context":{"apiVersion":"1.12.0","slot":6131},"value":[0,0,0,0,0,27,0,0,13,43,42,0,0,0,0,0,0,0,0,40,0,4,0,0,0,0,0,0,0,0,2,17,0,0,0,0,0,0,17,0,27,0,0,0,0,0,0,0,0,0,0,49,0,2,0,33,0,0,0,0,0,0,0,33,0,0,0,0,0,0,0,0,0,26,0,0,0,0,42,0,0,7,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,31,0,0,0,0,0,23,0,0,0,0,0,0,0,0,0,0,0,0,0,13,0,0,23,0,0,0,29,0,19,0,0,0,0,0,26,0,34,29,0,0,0,0,0,0,0,0,0,0]},"id":1}
  1. query without specifying accounts, correctly return 150 0s
% curl -X POST -H "Content-Type: application/json" -d '{"jsonrpc": "2.0","id":1,"method":"getRecentPrioritizationFees","params":[[],{"commitment":"confirmed"}]}' 34.82.0.171:8899
{"jsonrpc":"2.0","result":{"context":{"apiVersion":"1.12.0","slot":6179},"value":[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]},"id":1}

@CriesofCarrots CriesofCarrots force-pushed the min-fee-rpc branch 9 times, most recently from 817034a to 8d44b0d Compare August 31, 2022 04:57
@CriesofCarrots CriesofCarrots marked this pull request as ready for review August 31, 2022 17:36
@CriesofCarrots
Copy link
Contributor Author

Rebased and ready for proper review

Copy link
Contributor

@tao-stones tao-stones left a comment

Choose a reason for hiding this comment

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

Thanks for wired it so neatly. Looks great, just few nits

@CriesofCarrots CriesofCarrots added the CI Pull Request is ready to enter CI label Sep 1, 2022
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Sep 1, 2022
@tao-stones tao-stones self-requested a review September 1, 2022 18:21
Copy link
Contributor

@tao-stones tao-stones left a comment

Choose a reason for hiding this comment

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

lgtm

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label Sep 1, 2022
@mergify mergify bot merged commit 9b8bed8 into solana-labs:master Sep 1, 2022
mergify bot pushed a commit that referenced this pull request Jan 5, 2023
* Plumb priority_fee_cache into rpc

* Add PrioritizationFeeCache api

* Add getRecentPrioritizationFees rpc endpoint

* Use MAX_TX_ACCOUNT_LOCKS to limit input keys

* Remove unused cache apis

* Map fee data by slot, and make rpc account inputs optional

* Add priority_fee_cache to rpc test framework, and add test

* Add endpoint to jsonrpc docs

* Update docs/src/developing/clients/jsonrpc-api.md

* Update docs/src/developing/clients/jsonrpc-api.md

(cherry picked from commit 9b8bed8)

# Conflicts:
#	core/src/validator.rs
#	rpc/src/rpc.rs
#	rpc/src/rpc_service.rs
#	runtime/src/prioritization_fee_cache.rs
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Jan 7, 2023
@mergify
Copy link
Contributor

mergify bot commented Jan 7, 2023

automerge label removed due to a CI failure

CriesofCarrots added a commit to CriesofCarrots/solana that referenced this pull request Jan 7, 2023
* Plumb priority_fee_cache into rpc

* Add PrioritizationFeeCache api

* Add getRecentPrioritizationFees rpc endpoint

* Use MAX_TX_ACCOUNT_LOCKS to limit input keys

* Remove unused cache apis

* Map fee data by slot, and make rpc account inputs optional

* Add priority_fee_cache to rpc test framework, and add test

* Add endpoint to jsonrpc docs

* Update docs/src/developing/clients/jsonrpc-api.md

* Update docs/src/developing/clients/jsonrpc-api.md
CriesofCarrots pushed a commit that referenced this pull request Jan 7, 2023
* Add getRecentPrioritizationFees RPC endpoint (#27278)

* Plumb priority_fee_cache into rpc

* Add PrioritizationFeeCache api

* Add getRecentPrioritizationFees rpc endpoint

* Use MAX_TX_ACCOUNT_LOCKS to limit input keys

* Remove unused cache apis

* Map fee data by slot, and make rpc account inputs optional

* Add priority_fee_cache to rpc test framework, and add test

* Add endpoint to jsonrpc docs

* Update docs/src/developing/clients/jsonrpc-api.md

* Update docs/src/developing/clients/jsonrpc-api.md

* Fix conflicts
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Need an RPC api to estimate the fee user should pay

4 participants