feat: implement eth_feeHistory endpoint #502
Conversation
ptrus
force-pushed
the
ptrus/feature/fee-history
branch
4 times, most recently
from
09f6075
to
bbe0ba6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #502 +/- ##
==========================================
+ Coverage 62.31% 62.45% +0.13%
==========================================
Files 38 37 -1
Lines 3962 4051 +89
==========================================
+ Hits 2469 2530 +61
- Misses 1285 1307 +22
- Partials 208 214 +6 ☔ View full report in Codecov by Sentry. |
|
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 |
ptrus
force-pushed
the
ptrus/feature/fee-history
branch
3 times, most recently
from
0533998
to
7c10b2e
Compare
ptrus
force-pushed
the
ptrus/feature/fee-history
branch
11 times, most recently
from
606a775
to
c3fd2da
Compare
ptrus
requested review from
matevz and
CedarMist
and removed request for
kostko
ptrus
force-pushed
the
ptrus/feature/fee-history
branch
from
c3fd2da
to
f35d9e9
Compare
| @@ -146,6 +146,26 @@ func TestEth_GasPrice(t *testing.T) { | |||
| t.Logf("gas price: %v", price) | |||
| } | |||
|
|
|||
| func TestEth_FeeHistory(t *testing.T) { | |||
There was a problem hiding this comment.
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, },
| @@ -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. | |||
There was a problem hiding this comment.
Is there a difference testing it here compared to rpc_test?
| ) | ||
|
|
||
| // feeHistoryWindowSize is the number of recent blocks to store for the fee history query. | ||
| const feeHistoryWindowSize = 20 |
There was a problem hiding this comment.
Is this configurable? How did you come up with 20?
There was a problem hiding this comment.
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)
| // No transactions in the block. | ||
| if len(d.sortedTxs) == 0 { | ||
| for i := range rewards { | ||
| rewards[i] = (*hexutil.Big)(common.Big0) |
There was a problem hiding this comment.
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.
| // 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") | ||
|
|
There was a problem hiding this comment.
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)
Fixes #501
Implements eth_feeHistory endpoint. The query supports returning the history for the maximum last 20 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 blocks is continuously pre-computed in the gas oracle (only the percentiles are computed on the fly).
TODO: