feat: implement eth_feeHistory endpoint #502

ptrus · 2024-01-05T15:37:33Z

Fixes #501

Implements eth_feeHistory endpoint. The query supports returning the history for the maximum last 20 (configurable) blocks (this arbitrarily set limit is allowed as per spec: Requested range of blocks. Clients will return less than the requested range if not all blocks are available.)

To avoid fetching multiple blocks, transactions, and receipts from DB on every fee_history query (which can be expensive) the fee_history for last 20 (configurable) blocks is continuously pre-computed in the gas oracle (only the percentiles are computed on the fly).

TODO:

test

codecov · 2024-01-05T16:20:49Z

Codecov Report

Attention: Patch coverage is 84.97409% with 29 lines in your changes missing coverage. Please review.

Project coverage is 59.04%. Comparing base (8395380) to head (1ca1883).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
gas/fee_history.go	79.27%	17 Missing and 6 partials ⚠️
gas/backend.go	90.00%	2 Missing and 1 partial ⚠️
rpc/eth/api.go	86.36%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #502      +/-   ##
==========================================
+ Coverage   58.59%   59.04%   +0.44%     
==========================================
  Files          39       40       +1     
  Lines        4229     4380     +151     
==========================================
+ Hits         2478     2586     +108     
- Misses       1545     1572      +27     
- Partials      206      222      +16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

CedarMist · 2024-02-04T21:22:00Z

Metamask uses eth_feeHistory to calculate the different transaction priorities for gas price.

See metamask source:

This doesn't include the current txpool as BTC fee estimation does.

Recently we had a situation where many transactions were queued up, and we were not sure if the fee estimation was returning accurate data, or if the 'Speed Up' button in MetaMask would work correctly.

If eth_feeHistory isn't supported it will fallbcak to eth_gasPrice, in an ideal situation would be to return the average of the minimum and median for having the transaction included within the next block.

ZigaMr · 2024-11-11T13:16:44Z

tests/rpc/rpc_test.go

@@ -146,6 +146,26 @@ func TestEth_GasPrice(t *testing.T) {
 	t.Logf("gas price: %v", price)
 }

+func TestEth_FeeHistory(t *testing.T) {


Just a suggestion, maybe we can test with additional arguments here.
For example
{ name: "Basic query with percentiles", blockCount: 10, lastBlock: nil, percentiles: []float64{0.25, 0.5, 0.75, 1}, expectErr: false, },
{ name: "Query with no reward percentiles", blockCount: 5, lastBlock: nil, percentiles: nil, expectErr: false, },
{ name: "Query specific block range", blockCount: 3, lastBlock: big.NewInt(5), percentiles: []float64{0.5}, expectErr: false, },
{ name: "Invalid block count", blockCount: 0, lastBlock: nil, percentiles: []float64{0.5}, expectErr: true, },
{ name: "Invalid percentile", blockCount: 5, lastBlock: nil, percentiles: []float64{1.5}, // Invalid percentile > 1 expectErr: true, },

Thanks for the suggestion, added!

ZigaMr

Looks good.

matevz · 2024-11-11T14:10:06Z

gas/backend_test.go

@@ -185,4 +190,6 @@ func TestGasPriceOracle(t *testing.T) {

 	require.EqualValues(coreClient.minGasPrice.ToBigInt(), gasPriceOracle.GasPrice(), "oracle should return gas reported by the node query")
 	gasPriceOracle.Stop()
+
+	// TODO: emit blocks with transactions, to test fee history.


Is there a difference testing it here compared to rpc_test?

This was a lettover comment, removed

matevz · 2024-11-11T14:11:06Z

gas/fee_history.go

+)
+
+// feeHistoryWindowSize is the number of recent blocks to store for the fee history query.
+const feeHistoryWindowSize = 20


Is this configurable? How did you come up with 20?

Arbitrarily chosen, mostly because computing the response is not the most effective, as you must go through all requested blocks. I can make it configurable, but I would keep the default low, unless there is a use case for a larger default.

Most API's I have looked at have a limit of 1024, but they always mention that the node can return less than the requested range if not all blocks are available. The reasonable usages of eth_feeHistory for gas estimation usually use small values:

web3-py uses 10 blocks for gas estimation

multichain uses 10 blocks for gas estimation

Infura used to have a note that only 4 blocks are guaranteed to be available (although, as I see, this note is now gone) and they support up to 1024 blocks (with the note that it could return less)

This is now also configurable via config.

matevz · 2024-11-11T14:16:32Z

gas/fee_history.go

+	// No transactions in the block.
+	if len(d.sortedTxs) == 0 {
+		for i := range rewards {
+			rewards[i] = (*hexutil.Big)(common.Big0)


Should this be the minimum required gas price for Sapphire (0.0000001 per gas unit) instead of 0? I'm not sure if there's another EVM call to fetch that info and dApps will fail otherwise on fresh chains.

This is covered by feehistory.BaseFee (not via the rewards field), and is handled correctly even if there's no transactions in recent blocks.

matevz · 2024-11-11T14:38:11Z

tests/rpc/rpc_test.go

+	// Query fee history.
+	feeHistory, err := ec.FeeHistory(context.Background(), 10, nil, []float64{0.25, 0.5, 0.75, 1})
+	require.NoError(t, err, "get fee history")
+


Check that the fees were not zero. (the minimum gas price is set to 0.0000001 in the test docker image, so the fees should also be non-zero)

Added a check to ensure the returnedfeeHistory.BaseFee's are > 0.

ptrus force-pushed the ptrus/feature/fee-history branch 4 times, most recently from 09f6075 to bbe0ba6 Compare January 5, 2024 16:16

ptrus mentioned this pull request Jan 5, 2024

eth_feeHistory not supported #501

Closed

ptrus force-pushed the ptrus/feature/fee-history branch 3 times, most recently from 0533998 to 7c10b2e Compare May 6, 2024 10:40

matevz assigned ptrus Oct 3, 2024

matevz mentioned this pull request Oct 3, 2024

runtime: Enable EIP-1559 support oasisprotocol/sapphire-paratime#425

Closed

ptrus force-pushed the ptrus/feature/fee-history branch 11 times, most recently from 606a775 to c3fd2da Compare October 29, 2024 09:28

ptrus marked this pull request as ready for review October 29, 2024 09:34

ptrus requested a review from kostko as a code owner October 29, 2024 09:34

ptrus requested review from matevz and CedarMist and removed request for kostko October 29, 2024 09:35

ptrus force-pushed the ptrus/feature/fee-history branch from c3fd2da to f35d9e9 Compare October 30, 2024 09:05

matevz requested a review from ZigaMr November 5, 2024 16:10

ZigaMr reviewed Nov 11, 2024

View reviewed changes

ZigaMr approved these changes Nov 11, 2024

View reviewed changes

matevz reviewed Nov 11, 2024

View reviewed changes

ptrus force-pushed the ptrus/feature/fee-history branch 8 times, most recently from 6e9d31b to eca135b Compare November 20, 2024 09:18

ptrus added 2 commits November 20, 2024 10:29

feat: implement eth_feeHistory endpoint

aa378da

feat: support eth_maxPriorityFeePerGas query

1ca1883

ptrus force-pushed the ptrus/feature/fee-history branch from eca135b to 1ca1883 Compare November 20, 2024 09:29

ptrus enabled auto-merge November 20, 2024 09:34

ptrus merged commit ae0573a into main Nov 20, 2024
8 checks passed

ptrus deleted the ptrus/feature/fee-history branch November 20, 2024 09:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement eth_feeHistory endpoint #502

feat: implement eth_feeHistory endpoint #502

ptrus commented Jan 5, 2024 •

edited

Loading

codecov bot commented Jan 5, 2024 •

edited

Loading

CedarMist commented Feb 4, 2024 •

edited

Loading

ZigaMr Nov 11, 2024

ptrus Nov 20, 2024

ZigaMr left a comment

matevz Nov 11, 2024

ptrus Nov 20, 2024

matevz Nov 11, 2024

ptrus Nov 13, 2024

ptrus Nov 20, 2024 •

edited

Loading

matevz Nov 11, 2024

ptrus Nov 20, 2024 •

edited

Loading

matevz Nov 11, 2024

ptrus Nov 20, 2024

feat: implement eth_feeHistory endpoint #502

feat: implement eth_feeHistory endpoint #502

Conversation

ptrus commented Jan 5, 2024 • edited Loading

codecov bot commented Jan 5, 2024 • edited Loading

Codecov Report

CedarMist commented Feb 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZigaMr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ptrus Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ptrus Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ptrus commented Jan 5, 2024 •

edited

Loading

codecov bot commented Jan 5, 2024 •

edited

Loading

CedarMist commented Feb 4, 2024 •

edited

Loading

ptrus Nov 20, 2024 •

edited

Loading

ptrus Nov 20, 2024 •

edited

Loading