params: fix history serve window for verkle test #32127
Conversation
Thegaram
requested review from
fjl,
karalabe,
gballet,
rjl493456442 and
zsfelfoldi
as code owners
|
Is this even a protocol parameter? |
Technically yes, but it's already included in |
|
We should just remove HistoryServeWindow from params then! |
Thegaram
changed the title
Sure, done! |
fjl
changed the title
|
While this change is fine for geth as it is today, one thing to mention is that @gballet probably does rely on it in the verkle branch. |
|
@Thegaram can you resolve the conflict please? |
| BlobTxPointEvaluationPrecompileGas = 50000 // Gas price for the point evaluation precompile. | ||
| BlobBaseCost = 1 << 13 // Base execution gas cost for a blob. | ||
|
|
||
| HistoryServeWindow = 8192 // Number of blocks to serve historical block hashes for, EIP-2935. |
There was a problem hiding this comment.
I agree the value is incorrect, but this is still a protocol param and so it should stay defined here.
There was a problem hiding this comment.
Should I add it back? @fjl
There was a problem hiding this comment.
Friendly ping @fjl
There was a problem hiding this comment.
We can't really configure it differently, since it's an internal value in the contract.
There was a problem hiding this comment.
The value is defined by https://eips.ethereum.org/EIPS/eip-2935, and changing it would change the contract code and its address. So I think it's fair to say, it's not a tunable parameter.
There was a problem hiding this comment.
I think having HistoryServeWindow = 8191 in protocol_params.go is nice for being explicit about this, even if it is not used in geth (or only used in tests).
There was a problem hiding this comment.
Changed it back as per @gballet and @rjl493456442's suggestion.
Thegaram
changed the title
There was a problem hiding this comment.
I have followed the conversation on this PR. I see Felix's point on this not being a tunable parameter and sort of agree it can be moved to be local to the test. At the same time the PR is a strict improvement as it fixes a value. And if in its current form is easier for @gballet then LGTM.
s1na
changed the title
Fixes the history serve window parameter for the test function `getContractStoredBlockHash`. Fixes ethereum#32458.
* update op-geth to: merge go-ethereum v1.16.3 https://github.com/ethereum-optimism/op-geth * bump op-geth * various fixes * `params.HistoryServeWindow` changed 8192 -> 8191 ethereum/go-ethereum#32127 * Updated op-geth to v1.101602.4-0.20250922085653-2f0528ba0ed5 * update op-geth * update op-geth to v1.101603.0-rc.1 --------- Co-authored-by: geoknee <[email protected]>
EIP-2935 specifies
HISTORY_SERVE_WINDOW = 8191. Geth currently uses8192This does not break consensus since the history storage system contract is updated via EVM execution. However, the test functiongetContractStoredBlockHashcould return incorrect results.Fixes #32458.