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

client: Extend newPayloadV3 for blob versioned hashes checks #2716

Merged
merged 5 commits into from
May 22, 2023
Merged

client: Extend newPayloadV3 for blob versioned hashes checks #2716

merged 5 commits into from
May 22, 2023

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented May 21, 2023

Post blob tx conversion to RLP, CL's commitments check via versioned hashes is now passed over to EL with new payloads having the computed CL versioned hashes.

Implements:

This PR implements the same

Depends on

and should be merged (post rebase) after #2708 is merged

@g11tech g11tech added PR state: needs review package: client target: master Work to be done towards master branch labels May 21, 2023
@g11tech g11tech marked this pull request as draft May 21, 2023 11:22
@codecov
Copy link

codecov bot commented May 21, 2023 •
edited
Loading

Codecov Report

Merging #2716 (cd46ed5) into master (dd8afc5) will increase coverage by 0.32%.
The diff coverage is 79.22%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 90.75% <ø> (ø)
blockchain 90.72% <ø> (ø)
client 86.96% <79.22%> (?)
common 97.05% <ø> (ø)
devp2p 89.23% <ø> (-0.17%) ⬇️
ethash ∅ <ø> (∅)
evm 79.31% <ø> (ø)
rlp ∅ <ø> (∅)
statemanager 80.92% <ø> (ø)
trie 89.61% <ø> (-0.33%) ⬇️
tx 95.76% <ø> (ø)
util 81.34% <ø> (ø)
vm 81.36% <ø> (ø)

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

Base automatically changed from 4844-tx-rlp to master May 22, 2023 09:02
@g11tech g11tech marked this pull request as ready for review May 22, 2023 09:23
holgerd77
holgerd77 reviewed May 22, 2023
View reviewed changes
Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Some small comments on naming and doc strings

packages/client/test/rpc/engine/newPayloadV3VersionedHashes.spec.ts Outdated Show resolved Hide resolved
packages/client/test/rpc/engine/newPayloadV3VersionedHashes.spec.ts Outdated Show resolved Hide resolved
packages/client/test/rpc/engine/newPayloadV3VersionedHashes.spec.ts Outdated Show resolved Hide resolved
packages/client/test/rpc/engine/newPayloadV3VersionedHashes.spec.ts Outdated Show resolved Hide resolved
packages/client/lib/rpc/modules/engine.ts Show resolved Hide resolved
@g11tech g11tech mentioned this pull request May 22, 2023
Merged
holgerd77
holgerd77 approved these changes May 22, 2023
View reviewed changes
Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

LGTM

@holgerd77 holgerd77 merged commit 02f0b0b into master May 22, 2023
@holgerd77 holgerd77 deleted the new-payloadv3-vhashes branch May 22, 2023 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@holgerd77 holgerd77 holgerd77 approved these changes

Assignees
No one assigned
Labels
package: client PR state: needs review target: master Work to be done towards master branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@g11tech @holgerd77