Skip to content
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

Jump to bottom

feat: validate payload versioned hashes #4417

Merged
merged 4 commits into from
Aug 31, 2023
Merged

feat: validate payload versioned hashes #4417

merged 4 commits into from
Aug 31, 2023

Conversation

Rjected
Copy link
Member

@Rjected Rjected commented Aug 30, 2023

This validates versioned hashes received from engine_newPayloadV3 calls according to the Engine API Cancun spec.

Fixes #4300

@Rjected Rjected added C-enhancement New feature or request A-rpc Related to the RPC implementation A-consensus Related to the consensus engine M-eip This change relates to the implementation of an EIP E-cancun Related to the Cancun network upgrade labels Aug 30, 2023
@codecov
Copy link

codecov bot commented Aug 30, 2023 •
edited
Loading

Codecov Report

Merging #4417 (afcbb51) into main (1c83740) will decrease coverage by 0.08%.
Report is 8 commits behind head on main.
The diff coverage is 50.00%.

Impacted file tree graph

Files Changed Coverage Δ
crates/rpc/rpc-types/src/eth/engine/payload.rs 60.00% <ø> (ø)
crates/consensus/beacon/src/engine/mod.rs 74.33% <45.94%> (-1.04%) ⬇️
crates/primitives/src/block.rs 77.53% <100.00%> (+0.11%) ⬆️

... and 16 files with indirect coverage changes

Flag Coverage Δ
integration-tests 16.68% <0.00%> (-0.01%) ⬇️
unit-tests 63.72% <50.00%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 29.21% <ø> (ø)
blockchain tree 83.52% <ø> (ø)
pipeline 90.25% <ø> (ø)
storage (db) 74.81% <ø> (ø)
trie 94.88% <ø> (ø)
txpool 48.11% <ø> (-0.70%) ⬇️
networking 77.47% <ø> (-0.04%) ⬇️
rpc 57.30% <ø> (-0.03%) ⬇️
consensus 63.28% <45.94%> (-0.32%) ⬇️
revm 31.74% <ø> (ø)
payload builder 6.37% <ø> (ø)
primitives 86.21% <100.00%> (+<0.01%) ⬆️

mattsse
mattsse approved these changes Aug 30, 2023
View reviewed changes
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm,
one suggestion

crates/consensus/beacon/src/engine/mod.rs Outdated Show resolved Hide resolved
Rjected and others added 2 commits August 30, 2023 16:49
@Rjected @mattsse
restrict validate_versioned_hashes visibility
d34cfac 
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
@Rjected
fix formatting
afcbb51
@mattsse mattsse enabled auto-merge August 31, 2023 00:01
@mattsse mattsse added this pull request to the merge queue Aug 31, 2023
Merged via the queue into main with commit 893f4cf Aug 31, 2023
24 checks passed
@mattsse mattsse deleted the dan/validate-versioned-hashes branch August 31, 2023 00:15
shekhirin pushed a commit that referenced this pull request Aug 31, 2023
@Rjected @mattsse @shekhirin
feat: validate payload versioned hashes (#4417)
4b4a3be 
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
shekhirin pushed a commit that referenced this pull request Aug 31, 2023
@Rjected @mattsse @shekhirin
feat: validate payload versioned hashes (#4417)
e3466dc 
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
@Rjected Rjected mentioned this pull request Sep 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse approved these changes

@rkrasiuk rkrasiuk Awaiting requested review from rkrasiuk rkrasiuk is a code owner

@gakonst gakonst Awaiting requested review from gakonst

Assignees
No one assigned
Labels
A-consensus Related to the consensus engine A-rpc Related to the RPC implementation C-enhancement New feature or request E-cancun Related to the Cancun network upgrade M-eip This change relates to the implementation of an EIP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Validate versioned hashes on engine_newPayloadV3
2 participants
@Rjected @mattsse