Skip to content

params: fix history serve window for verkle test #32127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
s1na merged 5 commits into from
Aug 25, 2025
Merged

params: fix history serve window for verkle test #32127

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0f18f75
fix: use correct history serve window parameter
Thegaram Jul 1, 2025
a8edc64
remove HistoryServeWindow from protocol params
Thegaram Jul 1, 2025
aa62718
Merge branch 'master' into patch-1
Thegaram Jul 10, 2025
82a6c3d
Merge branch 'master' into patch-1
Thegaram Aug 4, 2025
f68e248
Revert "remove HistoryServeWindow from protocol params"
Thegaram Aug 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion params/protocol_params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ const (
BlobTxMaxBlobs = 6
BlobBaseCost = 1 << 13 // Base execution gas cost for a blob.

HistoryServeWindow = 8192 // Number of blocks to serve historical block hashes for, EIP-2935.
Copy link
Copy Markdown
Member

@gballet gballet Jul 12, 2025

Choose a reason for hiding this comment

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

I agree the value is incorrect, but this is still a protocol param and so it should stay defined here.

Copy link
Copy Markdown
Contributor Author

@Thegaram Thegaram Jul 13, 2025

Choose a reason for hiding this comment

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

Should I add it back? @fjl

Copy link
Copy Markdown
Contributor Author

@Thegaram Thegaram Jul 22, 2025

Choose a reason for hiding this comment

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

Friendly ping @fjl

Copy link
Copy Markdown
Member

@rjl493456442 rjl493456442 Jul 22, 2025

Choose a reason for hiding this comment

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

Yes please

Copy link
Copy Markdown
Contributor

@fjl fjl Jul 22, 2025

Choose a reason for hiding this comment

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

We can't really configure it differently, since it's an internal value in the contract.

Copy link
Copy Markdown
Contributor

@fjl fjl Jul 22, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@Thegaram Thegaram Jul 22, 2025

Choose a reason for hiding this comment

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

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).

Copy link
Copy Markdown
Contributor Author

@Thegaram Thegaram Aug 4, 2025

Choose a reason for hiding this comment

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

Changed it back as per @gballet and @rjl493456442's suggestion.

HistoryServeWindow = 8191 // Number of blocks to serve historical block hashes for, EIP-2935.

MaxBlockSize = 8_388_608 // maximum size of an RLP-encoded block
)
Expand Down