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

Extend EIP 4844 transaction toJson data to include additional fields #3365

Merged
merged 9 commits into from
Apr 24, 2024
Merged

Extend EIP 4844 transaction toJson data to include additional fields #3365

merged 9 commits into from
Apr 24, 2024

Conversation

scorbajio
Copy link
Contributor

@scorbajio scorbajio commented Apr 24, 2024 •
edited
Loading

This change adds a new networkWrapperToJson function that extends the data already returned by the toJson function of the BlobEIP4844Transaction class to also include blobs, commitments, and proofs as hexadecimal strings.

@scorbajio scorbajio linked an issue Apr 24, 2024 that may be closed by this pull request
tx.toJSON doesn't include 4844 related data #3310
Closed
acolytec3
acolytec3 requested changes Apr 24, 2024
View reviewed changes
Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

This looks great! Left a few clarification suggestions but feel free to wordsmith

packages/tx/test/eip4844.spec.ts Outdated Show resolved Hide resolved
packages/tx/src/types.ts Outdated Show resolved Hide resolved
packages/tx/src/eip4844Transaction.ts Outdated Show resolved Hide resolved
packages/tx/src/eip4844Transaction.ts Outdated Show resolved Hide resolved
scorbajio and others added 5 commits April 23, 2024 19:02
@scorbajio @acolytec3
Update packages/tx/test/eip4844.spec.ts
9d8d711 
Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com>
@scorbajio @acolytec3
Update packages/tx/src/eip4844Transaction.ts
50a4af9 
Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com>
@scorbajio @acolytec3
Update packages/tx/src/eip4844Transaction.ts
f116a48 
Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com>
@scorbajio
Rename ExtendedJsonTx to JsonBlobTxNetworkWrapper
7e704d8
@scorbajio
Merge branch 'extend-4844-toJson-data' of github.com:ethereumjs/ether…
b3d20b6 
…eumjs-monorepo into extend-4844-toJson-data
@scorbajio scorbajio requested a review from acolytec3 April 24, 2024 03:45
acolytec3
acolytec3 approved these changes Apr 24, 2024
View reviewed changes
Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

LGTM.

g11tech
g11tech approved these changes Apr 24, 2024
View reviewed changes
Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm

@acolytec3 acolytec3 merged commit 3125eac into master Apr 24, 2024
34 checks passed
@scorbajio scorbajio deleted the extend-4844-toJson-data branch April 24, 2024 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@g11tech g11tech g11tech approved these changes

@acolytec3 acolytec3 acolytec3 approved these changes

Assignees
No one assigned
Labels
package: tx PR state: needs review type: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tx.toJSON doesn't include 4844 related data
3 participants
@scorbajio @acolytec3 @g11tech