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

EIP-4844 V6 fixes and improvements #5745

Merged
merged 8 commits into from
Jun 12, 2023
Merged

EIP-4844 V6 fixes and improvements #5745

merged 8 commits into from
Jun 12, 2023

Conversation

flcl42
Copy link
Contributor

@flcl42 flcl42 commented May 31, 2023 •
edited
Loading

Contains fixes and changes according to the v6 scope.

Changes

Every commit is a separate item

  • Make reorgs working for blobs, we cannot have transactions in mempool form to provide for reorg, so we just do not include them
  • Datahash is renamed, because data prefix is reserved for other features
  • CL now passes blob versioned hashes to confirm they are the same as in the transactions. CL can't into RLP, but can calculate the hashes from the commitments that EL sends to CL in previous response. So it sends hashes to confirm that they are the same as in transactions' RLP. See Engine API: validate blob versioned hashes ethereum/execution-apis#407
  • To field is mandatory for Shard Blob txs
  • Improve transaction type check
  • A test case for cross-client Shard Blob tx encoding testing

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

@flcl42 flcl42 marked this pull request as draft May 31, 2023 15:44
@flcl42 flcl42 force-pushed the feature/eip-4844-v6-batch branch from 1dcd9de to 56665de Compare June 2, 2023 11:20
@flcl42 flcl42 marked this pull request as ready for review June 2, 2023 11:43
@flcl42 flcl42 force-pushed the feature/eip-4844-v6-batch branch 2 times, most recently from 0e464b5 to 9b3aa6f Compare June 2, 2023 14:23
LukaszRozmej
LukaszRozmej requested changes Jun 2, 2023
View reviewed changes
src/Nethermind/Nethermind.Blockchain.Test/Validators/TxValidatorTests.cs Show resolved Hide resolved
src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs Show resolved Hide resolved
src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.TxPool/TxPool.cs Show resolved Hide resolved
@flcl42 flcl42 marked this pull request as draft June 6, 2023 16:24
@flcl42 flcl42 force-pushed the feature/eip-4844-v6-batch branch from 9b3aa6f to 05a62ad Compare June 7, 2023 09:05
@flcl42 flcl42 marked this pull request as ready for review June 7, 2023 09:28
LukaszRozmej and others added 2 commits June 9, 2023 10:48
@LukaszRozmej @flcl42
Feature/eip 4844 v6.1 (#5784)
6e3ac40 
* Added ExecutionPayload._transactions to avoid multiple RLP decoding

* Simplify and refactor validation

* Add test

* Uglify to comply

---------

Co-authored-by: Alexey Osipov <me@flcl.me>
@flcl42
Merge remote-tracking branch 'origin/master' into feature/eip-4844-v6…
2b312f4 
…-batch
@LukaszRozmej LukaszRozmej merged commit 19614b8 into master Jun 12, 2023
@LukaszRozmej LukaszRozmej deleted the feature/eip-4844-v6-batch branch June 12, 2023 10:26
jmederosalvarado pushed a commit that referenced this pull request Jun 13, 2023
@flcl42 @LukaszRozmej @jmederosalvarado
EIP-4844 V6 fixes and improvements (#5745)
fca85e3 
* Workaround for reorgs

* Change DATAHASH to BLOBHASH

* Add blob hashes to newpayloadv3
(CL can't verify them in RLP, sends to EL)

* `To` should not be empty for blobs

* Fix tx type detection

* Add non-zero blobs test case to strengthen blob verification test coverage

* Feature/eip 4844 v6.1 (#5784)

* Added ExecutionPayload._transactions to avoid multiple RLP decoding

* Simplify and refactor validation

* Add test

* Uglify to comply

---------

Co-authored-by: Alexey Osipov <me@flcl.me>

---------

Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcindsobczak marcindsobczak marcindsobczak approved these changes

@LukaszRozmej LukaszRozmej LukaszRozmej approved these changes

@MarekM25 MarekM25 Awaiting requested review from MarekM25

@deffrian deffrian Awaiting requested review from deffrian

Assignees
No one assigned
Labels
eip-4844
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@flcl42 @LukaszRozmej @marcindsobczak