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.

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

feat: add eip4844 tx type id #3928

Merged
merged 3 commits into from
Jul 27, 2023
Merged

feat: add eip4844 tx type id #3928

merged 3 commits into from
Jul 27, 2023

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Jul 26, 2023

ref #2893

@mattsse mattsse requested a review from gakonst as a code owner July 26, 2023 14:20
@mattsse mattsse added the M-eip This change relates to the implementation of an EIP label Jul 26, 2023
@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Merging #3928 (131c31c) into main (b5a44ae) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

Files Changed Coverage Δ
crates/primitives/src/transaction/mod.rs 82.22% <0.00%> (ø)
crates/primitives/src/transaction/tx_type.rs 70.00% <ø> (ø)

... and 10 files with indirect coverage changes

Flag Coverage Δ
integration-tests 15.55% <0.00%> (-0.01%) ⬇️
unit-tests 64.55% <0.00%> (-0.02%) ⬇️

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

Components Coverage Δ
reth binary 27.18% <ø> (ø)
blockchain tree 83.04% <ø> (ø)
pipeline 89.82% <ø> (ø)
storage (db) 74.29% <ø> (ø)
trie 94.70% <ø> (ø)
txpool 46.00% <ø> (ø)
networking 77.62% <ø> (-0.05%) ⬇️
rpc 58.84% <ø> (-0.02%) ⬇️
consensus 64.46% <ø> (ø)
revm 33.68% <ø> (ø)
payload builder 6.61% <ø> (ø)
primitives 87.92% <0.00%> (-0.02%) ⬇️

@mattsse mattsse requested a review from Rjected July 26, 2023 16:09
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -268,7 +267,9 @@ pub struct TxEip4844 {
pub blob_hashes: Vec<H256>,

/// Max fee per data gas
pub max_fee_per_blob: u128,
///
/// aka BlobFeeCap
Copy link
Member

Choose a reason for hiding this comment

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

good comment

Copy link
Collaborator

@joshieDo joshieDo Jul 26, 2023

Choose a reason for hiding this comment

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

why not maintain blob instead of data? (which is in accordance to the eip)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

very good point, renamed so it matches eip

@mattsse mattsse enabled auto-merge July 26, 2023 16:37
@onbjerg onbjerg added C-enhancement New feature or request A-devp2p Related to the Ethereum P2P protocol and removed A-devp2p Related to the Ethereum P2P protocol labels Jul 26, 2023
@mattsse mattsse added this pull request to the merge queue Jul 27, 2023
Merged via the queue into main with commit 72ab361 Jul 27, 2023
24 checks passed
@mattsse mattsse deleted the matt/add-blob-tx-type-id branch July 27, 2023 06:53
@rkrasiuk rkrasiuk added the E-cancun Related to the Cancun network upgrade label Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement New feature or request E-cancun Related to the Cancun network upgrade M-eip This change relates to the implementation of an EIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants