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

Support blob transaction type (EIP-4844) #713

Merged
merged 5 commits into from
Nov 28, 2023
Merged

Support blob transaction type (EIP-4844) #713

merged 5 commits into from
Nov 28, 2023

Conversation

rodiazet
Copy link
Collaborator

@rodiazet rodiazet commented Sep 27, 2023 •
edited by chfast
Loading

  • Add blob tx type support in state.
  • Modify transition and validate_transaction functions to pass blob_gas_left parameter.
  • Update unit tests
  • Support blob tx in t8n
  • Compute excess_blob_gas and blob_gas_price.

@codecov
Copy link

codecov bot commented Sep 27, 2023 •
edited
Loading

Codecov Report

Merging #713 (0232170) into master (56641dc) will increase coverage by 0.08%.
The diff coverage is 99.65%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #713      +/-   ##
==========================================
+ Coverage   97.70%   97.79%   +0.08%     
==========================================
  Files         108      109       +1     
  Lines       10026    10295     +269     
==========================================
+ Hits         9796    10068     +272     
+ Misses        230      227       -3
Flag Coverage Δ
blockchaintests 60.79% <ø> (ø)
statetests 60.83% <71.83%> (+0.52%) ⬆️
statetests-silkpre 26.29% <10.04%> (-0.51%) ⬇️
unittests 95.90% <99.65%> (+0.18%) ⬆️

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

Files Coverage Δ
test/blockchaintest/blockchaintest.hpp 0.00% <ø> (ø)
test/blockchaintest/blockchaintest_loader.cpp 94.87% <100.00%> (+0.13%) ⬆️
test/state/host.cpp 93.89% <100.00%> (+0.08%) ⬆️
test/state/state.cpp 100.00% <100.00%> (ø)
test/state/state.hpp 100.00% <100.00%> (ø)
test/statetest/statetest.hpp 84.61% <ø> (ø)
test/statetest/statetest_loader.cpp 96.41% <100.00%> (+0.27%) ⬆️
test/statetest/statetest_runner.cpp 96.42% <100.00%> (+0.13%) ⬆️
test/unittests/state_block_test.cpp 100.00% <100.00%> (ø)
test/unittests/state_rlp_test.cpp 99.72% <100.00%> (+0.06%) ⬆️
... and 7 more

@rodiazet rodiazet force-pushed the blob_txs branch 2 times, most recently from 9bffaf2 to 9d80ad4 Compare September 27, 2023 10:45
@rodiazet rodiazet marked this pull request as ready for review September 27, 2023 11:05
@rodiazet rodiazet force-pushed the blob_txs branch 5 times, most recently from 66608ba to 158c1d9 Compare September 27, 2023 16:20
@rodiazet rodiazet added the Cancun Changes for Cancun Ethereum spec revision label Sep 28, 2023
gumb0
gumb0 reviewed Sep 28, 2023
View reviewed changes
test/state/state.hpp Outdated Show resolved Hide resolved
@rodiazet rodiazet force-pushed the blob_txs branch 3 times, most recently from dc5120a to e549fc8 Compare September 28, 2023 18:15
chfast
chfast reviewed Oct 2, 2023
View reviewed changes
Copy link
Member

@chfast chfast left a comment

Choose a reason for hiding this comment

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

How was this tested? I think we need to enable at least some external JSON tests on CI.

@rodiazet rodiazet force-pushed the blob_txs branch 2 times, most recently from 942d452 to 3dfcf0d Compare October 2, 2023 14:20
@rodiazet
Copy link
Collaborator Author

rodiazet commented Oct 2, 2023

How was this tested? I think we need to enable at least some external JSON tests on CI.

Done by 3dfcf0d

chfast
chfast reviewed Oct 2, 2023
View reviewed changes
test/state/state.cpp Outdated Show resolved Hide resolved
test/state/state.cpp Outdated Show resolved Hide resolved
test/state/state.cpp Outdated Show resolved Hide resolved
test/state/state.hpp Outdated Show resolved Hide resolved
test/state/state.hpp Outdated Show resolved Hide resolved
test/state/state.cpp Outdated Show resolved Hide resolved
@rodiazet rodiazet force-pushed the blob_txs branch 3 times, most recently from ab4d574 to 9e5a376 Compare October 4, 2023 10:38
@rodiazet rodiazet requested a review from gumb0 October 9, 2023 13:03
gumb0
gumb0 reviewed Oct 13, 2023
View reviewed changes
test/state/state.hpp Outdated Show resolved Hide resolved
gumb0
gumb0 reviewed Oct 13, 2023
View reviewed changes
test/state/state.hpp Outdated Show resolved Hide resolved
gumb0
gumb0 reviewed Oct 13, 2023
View reviewed changes
test/state/state.cpp Outdated Show resolved Hide resolved
pdobacz
pdobacz reviewed Nov 8, 2023
View reviewed changes
Copy link
Collaborator

@pdobacz pdobacz left a comment

Choose a reason for hiding this comment

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

I need some more time to mull this over, but two comments from me so far

test/state/state.cpp Outdated Show resolved Hide resolved
test/state/state.cpp Outdated Show resolved Hide resolved
@chfast chfast changed the title Support blob transaction type. Support blob transaction type Nov 13, 2023
chfast
chfast requested changes Nov 22, 2023
View reviewed changes
test/state/state.hpp Outdated Show resolved Hide resolved
test/state/state.hpp Outdated Show resolved Hide resolved
test/state/state.cpp Outdated Show resolved Hide resolved
test/state/state.cpp Show resolved Hide resolved
@rodiazet rodiazet force-pushed the blob_txs branch 3 times, most recently from 7c4c12d to 68e4230 Compare November 22, 2023 14:03
@chfast
Copy link
Member

chfast commented Nov 22, 2023

These tests from execution-spec-tests are failing: cancun/eip4844_blobs/excess_blob_gas.

@rodiazet rodiazet force-pushed the blob_txs branch 6 times, most recently from 6208bbb to 2c02cef Compare November 23, 2023 14:54
chfast
chfast requested changes Nov 27, 2023
View reviewed changes
test/statetest/statetest_loader.cpp Outdated Show resolved Hide resolved
@chfast chfast force-pushed the blob_txs branch 2 times, most recently from afbbfd6 to 93f5489 Compare November 28, 2023 18:41
@chfast chfast force-pushed the blob_txs branch 2 times, most recently from 8cffbfa to 7c31d1c Compare November 28, 2023 19:09
rodiazet and others added 4 commits November 28, 2023 20:50
@rodiazet @chfast
state: Compute blob gas price (EIP-4844)
bc3e2cd 
Compute blob gas price for transition tests using excess blob gas.
Provide the computed value for the BLOBBASEFEE instruction.
@chfast @rodiazet
state: Add support for blob transactions (EIP-4844)
79b9424 
Support blob transactions: validation, RLP-encoding and JSON tests.
Expose blob hashes to EVM.

Co-authored-by: rodiazet <[email protected]>
@rodiazet @chfast
t8n: Handle blob transactions and gas (EIP-4844)
645b69f
@rodiazet @chfast
ci: Run all blockchain tests EOF
0232170
@chfast chfast changed the title Support blob transaction type Support blob transaction type (EIP-4844) Nov 28, 2023
@chfast chfast merged commit f6d6ec3 into master Nov 28, 2023
25 checks passed
@chfast chfast deleted the blob_txs branch November 28, 2023 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gumb0 gumb0 gumb0 left review comments

@pdobacz pdobacz pdobacz approved these changes

@chfast chfast chfast approved these changes

Assignees
No one assigned
Labels
Cancun Changes for Cancun Ethereum spec revision
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rodiazet @chfast @gumb0 @pdobacz