reth
reth copied to clipboard
paradigmxyz
refactor: re-implement `eth_feeHistory`
The existing implementation of eth_feeHistory had some issues:
- It did not limit the request size
- It did not correctly calculate percentiles, because it was comparing
header.gas_usedto the percentile threshold, but it should be comparing a cumulative gas usage of transactions to the percentile threshold - It did not include the base fee of the next block (even if it doesn't exist) as per the spec
- It always responded with at least
reward: [], but if the request does not include percentiles, then it should not respond with this at all - It always loaded and sorted transactions, even if there were no percentiles (so it would be unnecessary)
- It supported block hashes when it should only support numbers or tags
I also opted to remove the fee history cache, since it adds additional overhead, and it is hard to cache this endpoint, as the response can change significantly based on the reward percentiles.
Perhaps we can use some sort of basic cache for the headers only, or the headers and a pre-sorted list of transactions, but I don't think it would really give us much benefit.
I am going to test this on a node and compare the results to Erigon to get a sense of if the implementation is flawed or not
Codecov Report
Merging #3288 (9f1db8a) into main (49922bf) will decrease coverage by
0.15%. The diff coverage is61.38%.
@@ Coverage Diff @@
## main #3288 +/- ##
==========================================
- Coverage 69.41% 69.27% -0.15%
==========================================
Files 537 537
Lines 71991 72041 +50
==========================================
- Hits 49973 49906 -67
- Misses 22018 22135 +117
| Flag | Coverage Δ | |
|---|---|---|
| integration-tests | 16.33% <2.47%> (-0.01%) |
:arrow_down: |
| unit-tests | 64.34% <61.38%> (-0.14%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| crates/primitives/src/transaction/mod.rs | 85.23% <0.00%> (-1.57%) |
:arrow_down: |
| crates/rpc/rpc-api/src/eth.rs | 100.00% <ø> (ø) |
|
| crates/rpc/rpc/src/eth/api/mod.rs | 87.09% <ø> (-0.31%) |
:arrow_down: |
| crates/rpc/rpc/src/eth/error.rs | 7.50% <0.00%> (ø) |
|
| crates/rpc/rpc/src/eth/api/fees.rs | 48.41% <33.68%> (-38.55%) |
:arrow_down: |
| crates/rpc/rpc/src/eth/api/server.rs | 95.41% <87.12%> (-2.87%) |
:arrow_down: |
| crates/rpc/rpc-types/src/eth/fee.rs | 7.14% <100.00%> (-50.76%) |
:arrow_down: |
| crates/rpc/rpc/src/eth/gas_oracle.rs | 39.59% <100.00%> (+1.24%) |
:arrow_up: |
... and 14 files with indirect coverage changes
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
TL;DR: It matches except in the case of block count of 0, but this case is not defined in the spec..
Comparison between Erigon's impl and Reth's impl. Each example uses the same percentiles (0, 10, 25, 50, 100).
Block 17m, block count of 1
Erigon:
{
"jsonrpc":"2.0",
"id":1,
"result":{
"oldestBlock":"0x1036640",
"reward":[
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0xb376620",
"0x26df9ef958"
]
],
"baseFeePerGas":[
"0x4cad3abe1",
"0x48f2114b8"
],
"gasUsedRatio":[
0.3053592666666667
]
}
}
Reth:
{
"jsonrpc":"2.0",
"result":{
"baseFeePerGas":[
"0x4cad3abe1",
"0x48f2114b8"
],
"gasUsedRatio":[
0.3053592666666667
],
"oldestBlock":"0x1036640",
"reward":[
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0xb376620",
"0x26df9ef958"
]
]
},
"id":1
}
Block 17m, block count of 0
Erigon:
{
"jsonrpc":"2.0",
"id":1,
"result":{
"oldestBlock":"0x0",
"gasUsedRatio":null
}
}
Reth:
{
"jsonrpc":"2.0",
"result":{
"baseFeePerGas":[
],
"gasUsedRatio":[
],
"oldestBlock":"0x0",
"reward":null
},
"id":1
}
Doesn't match, but are logically similar I think
Block 17m, block count of 10
Erigon:
{
"jsonrpc":"2.0",
"id":1,
"result":{
"oldestBlock":"0x1036637",
"reward":[
[
"0x0",
"0x59a5380",
"0x5f5e100",
"0x11e1a300",
"0xcb7c3cc58"
],
[
"0x0",
"0x1f8dead",
"0x1f8dead",
"0x4a75410",
"0x2cd3a56ad"
],
[
"0x0",
"0x5f5e100",
"0x121738e4",
"0x77359400",
"0x527935f3e"
],
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0x5f5e100",
"0x8680c0212"
],
[
"0x0",
"0x5f5e100",
"0x11e1a300",
"0x3b9aca00",
"0x2b6e7d7f4"
],
[
"0x5f5e100",
"0xa1fa7e6",
"0x2d8dfc0c",
"0x2d8dfc0c",
"0x4cc25aef1"
],
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0x5f5e100",
"0x42a860c240"
],
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0x5f5e100",
"0xdb597ddd3"
],
[
"0x0",
"0x485ce38",
"0x59a5380",
"0xb376620",
"0x419b5992a"
],
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0xb376620",
"0x26df9ef958"
]
],
"baseFeePerGas":[
"0x4475fcda8",
"0x42ee95553",
"0x4b4ae5842",
"0x44d989922",
"0x45200f7f8",
"0x4035437f4",
"0x4839a89b2",
"0x4705ccab0",
"0x4f68b16aa",
"0x4cad3abe1",
"0x48f2114b8"
],
"gasUsedRatio":[
0.4106688,
0.9996475666666667,
0.1577187,
0.5160047666666666,
0.2154628,
0.9994493,
0.43339863333333334,
0.9723170666666666,
0.3623692,
0.3053592666666667
]
}
}
Reth:
{
"jsonrpc":"2.0",
"result":{
"baseFeePerGas":[
"0x4475fcda8",
"0x42ee95553",
"0x4b4ae5842",
"0x44d989922",
"0x45200f7f8",
"0x4035437f4",
"0x4839a89b2",
"0x4705ccab0",
"0x4f68b16aa",
"0x4cad3abe1",
"0x48f2114b8"
],
"gasUsedRatio":[
0.4106688,
0.9996475666666667,
0.1577187,
0.5160047666666666,
0.2154628,
0.9994493,
0.43339863333333334,
0.9723170666666666,
0.3623692,
0.3053592666666667
],
"oldestBlock":"0x1036637",
"reward":[
[
"0x0",
"0x59a5380",
"0x5f5e100",
"0x11e1a300",
"0xcb7c3cc58"
],
[
"0x0",
"0x1f8dead",
"0x1f8dead",
"0x4a75410",
"0x2cd3a56ad"
],
[
"0x0",
"0x5f5e100",
"0x121738e4",
"0x77359400",
"0x527935f3e"
],
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0x5f5e100",
"0x8680c0212"
],
[
"0x0",
"0x5f5e100",
"0x11e1a300",
"0x3b9aca00",
"0x2b6e7d7f4"
],
[
"0x5f5e100",
"0xa1fa7e6",
"0x2d8dfc0c",
"0x2d8dfc0c",
"0x4cc25aef1"
],
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0x5f5e100",
"0x42a860c240"
],
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0x5f5e100",
"0xdb597ddd3"
],
[
"0x0",
"0x485ce38",
"0x59a5380",
"0xb376620",
"0x419b5992a"
],
[
"0x0",
"0x5f5e100",
"0x5f5e100",
"0xb376620",
"0x26df9ef958"
]
]
},
"id":1
}
Pre-EIP 1559 (block 4 with a block count of 4, i.e. genesis..=4)
Erigon:
{
"jsonrpc":"2.0",
"id":1,
"result":{
"oldestBlock":"0x1",
"reward":[
[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
],
[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
],
[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
],
[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
]
],
"baseFeePerGas":[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
],
"gasUsedRatio":[
0,
0,
0,
0
]
}
}
Reth:
{
"jsonrpc":"2.0",
"result":{
"baseFeePerGas":[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
],
"gasUsedRatio":[
0.0,
0.0,
0.0,
0.0
],
"oldestBlock":"0x1",
"reward":[
[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
],
[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
],
[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
],
[
"0x0",
"0x0",
"0x0",
"0x0",
"0x0"
]
]
},
"id":1
}
reth:
"reward":null
looks like erigon/geth don't return this field if null?
but return empty
"gasUsedRatio":[
],
as null
should we mimic this as well?
reth:
"reward":null
looks like erigon/geth don't return this field if null?
but return empty
"gasUsedRatio":[ ],as null
should we mimic this as well?
It's annoying that it responds like that, but I implemented 1:1 compat in af48ee1
@mattsse I removed the 1024 limit, it seems I was mistaken and this is not actually part of the spec.
@mattsse I removed the 1024 limit, it seems I was mistaken and this is not actually part of the spec.
we definitely need to cap it,
geth caps it to 1024:
https://github.com/ethereum/go-ethereum/blob/2754b197c935ee63101cbbca2752338246384fec/eth/ethconfig/config.go#L38-L46
This is the full node config, i.e. it is capped because it will not have data for more than 1024 blocks at head afaik. Unsure what the best approach is to cap it
Edit: Ah, nvm, I see, this is separate from the regular full node config, instead it is a GPO config and these are just the defaults for full node. Do we have some sort of config mechanism for the RPC where we could add this (with a default of 1024)?
it's always capped, otherwise you can DOS the endpoint
https://github.com/ethereum/go-ethereum/blob/2754b197c935ee63101cbbca2752338246384fec/eth/gasprice/feehistory.go#L222-L225
it's configurable in geth via config though
https://github.com/ethereum/go-ethereum/blob/2754b197c935ee63101cbbca2752338246384fec/eth/gasprice/gasprice.go#L42-L50
Do we have some sort of config mechanism for the RPC where we could add this (with a default of 1024)?
we have in in this:
https://github.com/paradigmxyz/reth/blob/49922bfd897a16b3750db4c20aa0f40b073d6f05/crates/rpc/rpc/src/eth/gas_oracle.rs#L22-L33
which we should be able to query in eth_feeHistory via .gas_oracle()
https://github.com/paradigmxyz/reth/blob/49922bfd897a16b3750db4c20aa0f40b073d6f05/crates/rpc/rpc/src/eth/api/mod.rs#L287-L288