EIP-4844 - pass blockchain tests

[pdobacz]

build/bin/evmone-blockchaintest ~/Downloads/fixtures_test_prague_devnet_5/blockchain_tests/cancun/eip4844_blobs/
...
[==========] 43 tests from 7 test suites ran. (4250 ms total)
[  PASSED  ] 43 tests.

🍏

NOTE: I'll squash the commits before merging.

NOTE2: opening as draft to work through CI issues. Not reviewable yet, but if I'm completely off the scent somewhere, please let know

Ready for review

[codecov]

Codecov Report

Attention: Patch coverage is 90.32258% with 6 lines in your changes missing coverage. Please review.

Project coverage is 94.27%. Comparing base (8dd19d7) to head (8dbd8c6).

Files with missing lines	Patch %	Lines
test/blockchaintest/blockchaintest_runner.cpp	75.00%	4 Missing ⚠️
test/blockchaintest/blockchaintest_loader.cpp	90.90%	1 Missing ⚠️
test/state/block.cpp	88.88%	1 Missing ⚠️

Additional details and impacted files

@@                  Coverage Diff                  @@
##           enable-inv-bloack    #1077      +/-   ##
=====================================================
- Coverage              94.29%   94.27%   -0.03%     
=====================================================
  Files                    159      159              
  Lines                  17160    17201      +41     
=====================================================
+ Hits                   16181    16216      +35     
- Misses                   979      985       +6     

Flag	Coverage Δ
eof_execution_spec_tests	23.64% <48.38%> (+0.06%)	⬆️
ethereum_tests	26.67% <58.06%> (+0.08%)	⬆️
ethereum_tests_silkpre	19.00% <16.32%> (-0.02%)	⬇️
execution_spec_tests	20.98% <56.45%> (+0.09%)	⬆️
unittests	88.96% <66.12%> (-0.08%)	⬇️

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

Files with missing lines	Coverage Δ
test/blockchaintest/blockchaintest.hpp	0.00% <ø> (ø)
test/state/block.hpp	100.00% <ø> (ø)
test/state/host.cpp	100.00% <100.00%> (ø)
test/state/state.cpp	99.59% <100.00%> (ø)
test/state/transaction.hpp	100.00% <100.00%> (ø)
test/statetest/statetest_loader.cpp	98.16% <100.00%> (ø)
test/t8n/t8n.cpp	88.09% <100.00%> (+0.07%)	⬆️
test/unittests/state_transition.cpp	98.49% <100.00%> (+0.05%)	⬆️
test/unittests/state_transition_block_test.cpp	100.00% <100.00%> (ø)
test/unittests/state_transition_tx_test.cpp	100.00% <100.00%> (ø)
... and 4 more

[chfast]

I think we should not compute this each access. Maybe we need two sets of fields in BlockInfo: official/header fields and computed values.

If big deal, we can handle this by separate PR.

[pdobacz]

Oh right, get_tx_context() is called within the instruction, yeah, better to fix right away :(. Alternatively this can be kept in Host? WDYT? if we can avoid mixing header/computed values, maybe it's better? or is Host just inappropriate for this?

[chfast]

Better to put it back to BlockInfo because this is also used in transaction validation and transition. These are also important cases.

[pdobacz]

argh, I've done it and only after I recalled from previous week, that I have already been there. The problem which arises is that it makes compute_blob_gas_price come before block validation, making problematic inputs (excess_blob_gas) come through, which previously wasn't the case. fake_exponential seems to go into infinite loop for such inputs.

I'll need to revisit this and look more closely at what exactly is happening

[chfast]

I imagine we keep the newly added field and also have blob_base_fee "after" them computed as some point...

[pdobacz]

I explored this idea, but I'm struggling to find a good point to compute that. It would sacrifice some consts and make a mess, or force us to introduce more complexity to manage that.

I have a counter-proposal. I dug deeper into fake_exponential, and it seems to me like not only it infinite loops but also easily overflows over 256 bits. If I cap the result at uint256::max on mul overflow (I can post code in a moment), things work OK. I think the tacit assumption in fake_exponential spec is that it should use arbitrary precision, and I peeked at some impls and they actually do that.

So I'd rather fix fake_exponential to not loop/overflow, and rely on current block validation, which would reject the block properly.

I see three options here:

1. switch from uint256 to big int of some sort
2. detect overflow and cap compute_blob_gas_price to uint256::max
3. detect overflow and return nullopt from compute_blob_gas_price

2 is the easiest (will push in a sec), but has the disadvantage of there being a block with potentially incorrect blob_base_fee field, hanging around until we reject it (on validation of excess/used blob gas fields).

3 seems better but not sure if the opts won't proliferate around the code base excessively. Maybe not

[pdobacz]

Trying (2.) first

Copilot wasn't able to review any files in this pull request.

Files not reviewed (14)

• test/blockchaintest/blockchaintest.hpp: Language not supported
• test/blockchaintest/blockchaintest_loader.cpp: Language not supported
• test/blockchaintest/blockchaintest_runner.cpp: Language not supported
• test/state/block.cpp: Language not supported
• test/state/block.hpp: Language not supported
• test/state/host.cpp: Language not supported
• test/state/state.cpp: Language not supported
• test/state/transaction.hpp: Language not supported
• test/statetest/statetest_loader.cpp: Language not supported
• test/t8n/t8n.cpp: Language not supported
• test/unittests/state_transition.cpp: Language not supported
• test/unittests/state_transition_block_test.cpp: Language not supported
• test/unittests/state_transition_tx_test.cpp: Language not supported
• test/unittests/state_tx_test.cpp: Language not supported