EIP-7742: Add target_blob_count to block header #7808
Conversation
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
| import org.junit.jupiter.params.provider.Arguments; | ||
|
|
||
| // TODO SLD | ||
| @Disabled("TODO SLD - Enable when Prague spec is finalized") |
There was a problem hiding this comment.
I propose we leave this disabled until the nearer the end of the Prague dev work. Anything that changes the block hash (genesis, block header, some engine API changes) causes a cascading change to all the JSON files and it's time consuming to update this with every PR. In the meantime, we will still have regression signal in the devnets and execution-spec/references tests.
There was a problem hiding this comment.
I think that's fine, still it's a lot of effort to update. Once the testnets are done though we should re-enable this test
| // TODO SLD Quantity or UInt64Value<UInt64> instead? | ||
| protected final UInt64 targetBlobCount; |
There was a problem hiding this comment.
Some other code for similar types use a subclass of BaseUInt256Value (which implements Quantity and UInt256Value).
Withdrawals already uses UInt64 directly.
I think UInt64 is more direct than the tuweni generic base class construct and avoids yet another wrapper class.
Would like to get more opinions about the benefits of defining our own type, i.e. TargetBlobCount extends BaseUInt256Value<TargetBlobCount> implements Quantity
There was a problem hiding this comment.
Side note: Teku using UInt64 directly, as in this PR https://github.com/Consensys/teku/blob/110ce917f1fec942e2c2664c50a1bc3bb778ee07/ethereum/executionclient/src/main/java/tech/pegasys/teku/ethereum/executionclient/schema/PayloadAttributesV4.java#L48
There was a problem hiding this comment.
Fine with using UInt64 directly. Adding the new type doesn't add much value
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
siladu
changed the title
| import org.junit.jupiter.params.provider.Arguments; | ||
|
|
||
| // TODO SLD | ||
| @Disabled("TODO SLD - Enable when Prague spec is finalized") |
There was a problem hiding this comment.
I think that's fine, still it's a lot of effort to update. Once the testnets are done though we should re-enable this test
Signed-off-by: Simon Dudley <simon.dudley@consensys.net> Signed-off-by: Marlene Marz <m.marz@kabelmail.de>
PR description
First major piece of EIP-7742: Add target_blob_count to the block header.
Still todo in future PRs:
Fixed Issue(s)
Part of #7605
Thanks for sending a pull request! Have you done the following?
doc-change-requiredlabel to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build./gradlew acceptanceTest./gradlew integrationTest./gradlew ethereum:referenceTests:referenceTests