Update EIP-4844: Decouple blobs by g11tech · Pull Request #6610 · ethereum/EIPs

g11tech · 2023-03-04T11:09:11Z

Decouple the blobs as per

EIP-4844: Free the blobs consensus-specs#3244

eth-bot · 2023-03-04T11:09:48Z

✅ All reviewers have approved.

github-actions · 2023-03-04T11:14:46Z

The commit 366b28a (as a parent of bb8db28) contains errors.
Please inspect the Run Summary for details.

EIPS/eip-4844.md

Inphi · 2023-03-21T19:04:27Z

EIPS/eip-4844.md

    blobs: List[Vector[BLSFieldElement, FIELD_ELEMENTS_PER_BLOB], LIMIT_BLOBS_PER_TX]
    # KZGProof = Bytes48
-    kzg_aggregated_proof: KZGProof
+    blob_proofs: List[KZGProof, MAX_TX_WRAP_KZG_COMMITMENTS]


A bit redundant to call it blob_proofs. Let's call it proofs.

added it on the lines of blob_kzgs, should we also rename it to just kzgs?

IMO kzgs is ambiguous because it's not clear if it refers to proofs or commitments.

then renaming them to commitments is a good idea?

I think it is a good idea, but probably in a separate PR.

I was originally mistaken that you were gonna introduce another kzgs naming with this PR, but since it refers to something that already exists, please disregard my previous comment.

I don't have a strong opinion about blob_kzgs vs kzgs. But I guess if you go with proofs it also makes sense to go with kzgs.

Prefer commitments to kzgs.

updated, can rename blob_kzgs to commitments in a followup PR

Here is the followup PR build on top of this current branch: g11tech#1 (can update target to this repo once this PR merges)

I have merged the followup changes to this PR on the request from @Inphi

should i also rename blob_versioned_hashes to versioned_hashes in BlobTransactionType? @Inphi @asn-d6 @dankrad

asn-d6

LGTM

hwwhww · 2023-05-01T15:57:47Z

EIPS/eip-4844.md

-def kzg_to_versioned_hash(kzg: KZGCommitment) -> VersionedHash:
-    return BLOB_COMMITMENT_VERSION_KZG + sha256(kzg)[1:]
+def kzg_to_versioned_hash(commitment: KZGCommitment) -> VersionedHash:
+    return BLOB_COMMITMENT_VERSION_KZG + sha256(commitment)[1:]


Suggest renaming all BLOB_COMMITMENT_VERSION_KZG to VERSIONED_HASH_VERSION_KZG as CL specs.

Suggested change

return BLOB_COMMITMENT_VERSION_KZG + sha256(commitment)[1:]

return VERSIONED_HASH_VERSION_KZG + sha256(commitment)[1:]

umm ... BLOB_COMMITMENT_VERSION_KZG makes more sense to me as its the version of the commitment scheme

I think the rationale was the version of VersionedHash. I'm not sure what's the scenario of changing VERSIONED_HASH_VERSION_* but not just changing the commitment scheme though.

It should be unified to one of the names anyway.

/cc @asn-d6 @dankrad any thoughts?

And kzg_to_versioned_hash was also renamed to kzg_commitment_to_versioned_hash.

No strong opinion here. I think both variants are reasonable.

I would go with @hwwhww suggestions to make it the same with consensus-specs.

Co-authored-by: dankrad <[email protected]>

…on container

g11tech · 2023-05-12T13:19:25Z

rebase via UI @Inphi could you take a look/re-approve now that you are in authors list 😆

hwwhww · 2023-05-15T08:07:55Z

It might be off the scope of this PR, but there are some more CL specs changes that are not synced.
e.g., the type of Blob was changed to ByteVector[BYTES_PER_FIELD_ELEMENT * FIELD_ELEMENTS_PER_BLOB] in ethereum/consensus-specs#3038

asn-d6

Let's get this merged finally! Sorry for the delays.

@g11tech is there a chance you can also open another PR to further synchronize the EIP with the consensus-specs in terms of function names and type changes. I think consensus-specs is a bit ahead on this (due to the unittests) and we should get the EIP to catch up. Thanks!

eth-bot

All Reviewers Have Approved; Performing Automatic Merge...

g11tech · 2023-05-15T10:25:52Z

sounds good, will open an update for bringing naming convention uptodate

github-actions bot added c-update Modifies an existing proposal s-review This EIP is in Review t-core labels Mar 4, 2023

eth-bot added the a-review Waiting on author to review label Mar 4, 2023

g11tech mentioned this pull request Mar 4, 2023

tx: Update eip4844 txs to decoupled blobs spec ethereumjs/ethereumjs-monorepo#2567

Merged

github-actions bot added the w-ci Waiting on CI to pass label Mar 4, 2023

dankrad reviewed Mar 14, 2023

View reviewed changes

EIPS/eip-4844.md Outdated Show resolved Hide resolved

dankrad mentioned this pull request Mar 14, 2023

Deneb crypto helpers test coverage ethereum/consensus-specs#3283

Merged

g11tech marked this pull request as ready for review March 15, 2023 09:56

g11tech requested a review from eth-bot as a code owner March 15, 2023 09:56

github-actions bot removed the w-ci Waiting on CI to pass label Mar 15, 2023

dankrad reviewed Mar 15, 2023

View reviewed changes

EIPS/eip-4844.md Outdated Show resolved Hide resolved

dankrad reviewed Mar 16, 2023

View reviewed changes

EIPS/eip-4844.md Outdated Show resolved Hide resolved

poojaranjan mentioned this pull request Mar 21, 2023

EIP Editing Office Hour Meeting 13 ethcatherders/EIPIP#216

Closed

25 tasks

Inphi reviewed Mar 21, 2023

View reviewed changes

g11tech mentioned this pull request Mar 23, 2023

Add corresponding proofs to BlobsBundleV1 ethereum/execution-apis#392

Merged

g11tech requested review from Inphi, asn-d6 and dankrad and removed request for Inphi, asn-d6 and dankrad March 27, 2023 09:50

asn-d6 previously approved these changes Mar 29, 2023

View reviewed changes

Inphi previously approved these changes Apr 12, 2023

View reviewed changes

g11tech dismissed stale reviews from Inphi and asn-d6 via 32c1fe7 April 13, 2023 07:02

g11tech force-pushed the free-the-blobs branch from 32c1fe7 to ac89c71 Compare April 13, 2023 07:32

This was referenced Apr 17, 2023

EIP-4844 Implementers' Call #20 ethereum/pm#755

Closed

Merge getPayloadV3 and getBlobsBundleV1 ethereum/execution-apis#402

Merged

realbigsean mentioned this pull request Apr 20, 2023

update blob transaction sigp/lighthouse#4218

Merged

fab-10 mentioned this pull request Apr 27, 2023

Decouple Blobs hyperledger/besu#5402

Closed

hwwhww reviewed May 1, 2023

View reviewed changes

g11tech and others added 7 commits May 12, 2023 18:48

Update EIP-4844: Decouple blobs

3bc1d36

apply feedback

7d87825

update the proof function to public one

8841f61

Co-authored-by: dankrad <[email protected]>

explicity verify proofs number to match commitment and versioned hashes

55609d2

Co-authored-by: dankrad <[email protected]>

rename blob_proofs to more succinct proofs for network wrapper

d9517d7

Replace kzgs references with a more generalized term: commitments

656cb21

removing redundant blob prefix from versioned hashes in BlobTransacti…

0132e81

…on container

g11tech force-pushed the free-the-blobs branch from 6845a66 to 0132e81 Compare May 12, 2023 13:18

ghost approved these changes May 13, 2023

View reviewed changes

asn-d6 approved these changes May 15, 2023

View reviewed changes

eth-bot enabled auto-merge (squash) May 15, 2023 10:00

eth-bot approved these changes May 15, 2023

View reviewed changes

eth-bot merged commit 84f6136 into ethereum:master May 15, 2023

	return BLOB_COMMITMENT_VERSION_KZG + sha256(commitment)[1:]
	return VERSIONED_HASH_VERSION_KZG + sha256(commitment)[1:]

Conversation

g11tech commented Mar 4, 2023

Uh oh!

eth-bot commented Mar 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 4, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asn-d6 Mar 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

g11tech Mar 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

g11tech Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asn-d6 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

g11tech commented May 12, 2023

Uh oh!

hwwhww commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asn-d6 left a comment

Choose a reason for hiding this comment

Uh oh!

eth-bot left a comment

Choose a reason for hiding this comment

Uh oh!

g11tech commented May 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eth-bot commented Mar 4, 2023 •

edited

Loading

asn-d6 Mar 22, 2023 •

edited

Loading

g11tech Mar 22, 2023 •

edited

Loading

g11tech Apr 14, 2023 •

edited

Loading

hwwhww commented May 15, 2023 •

edited

Loading