evm: Fix the availability of versioned hashes in contract calls

[g11tech]

In EIP4844 devnet5, there was a block in which blob transactions were doing contract executions and the versioned hashes were being accessed via DATAHASH.
however ethereumjs was only passing versioned hashes only on callDelegate, but then the contract was being executed via callStatic. This caused a block execution to fail
ay-08 14:28:15.879[chain] error: Block error slot=87475 code=BLOCK_ERROR_EXECUTION_ERROR, execStatus=INVALID, errorMessage=Error verifying block while running: Error: invalid receiptTrie (vm hf=cancun -> block number=62757 hash=fb640e94f3c2250ab223c41f07c3d72d5d0a589226ce09aab1827933d9a72548 hf=cancun baseFeePerGas=9 txs=4 uncles=0)
with gas consumption not matching with the block:

and eventually caused a fork in devnet5 since ethereumjs based peers forked off the geth based peers.

This PR fixes the issue and leads to successful execution of the block with matching execution with geth

[codecov]

Codecov Report

Merging #2694 (4aed5f8) into master (cf2b55d) will decrease coverage by 1.07%.
The diff coverage is 36.00%.

Additional details and impacted files

Flag	Coverage Δ
block	90.30% <ø> (ø)
blockchain	90.55% <ø> (ø)
client	86.89% <ø> (-0.02%)	⬇️
common	96.06% <ø> (ø)
devp2p	91.90% <ø> (?)
ethash	∅ <ø> (∅)
evm	79.37% <36.00%> (?)
rlp	?
statemanager	80.92% <ø> (ø)
trie	90.06% <ø> (-0.17%)	⬇️
tx	95.50% <ø> (ø)
util	81.33% <ø> (ø)
vm	81.36% <ø> (ø)

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

[acolytec3]

It looks good as far as it goes. Can we add a test for this? Maybe just take the actual contract/transaction that highlighted this issue and run that transaction through the EVM and verify the datahash is available in the lower level?

[g11tech]

It looks good as far as it goes. Can we add a test for this? Maybe just take the actual contract/transaction that highlighted this issue and run that transaction through the EVM and verify the datahash is available in the lower level?

sounds good will add tomorrow

[jochem-brouwer]

I reviewed the tests and they look good for me for CALL, CALLCODE, DELEGATECALL, STATTICCALL (AUTHCALL is untested but this is rather hard to setup so I am ok with this).

However, I wonder: does this work if we perform a CREATE or CREATE2 operation inside the datahash tx and then inside this frame we somewhere access the data hash? It also makes me think that these constants (these datahashes are fed with the tx, right, and are thus constant?) should be moved into some global place. For instance, there are at least two other constants in a tx currently: the origin and the block we run in (uses in opcodes like ORIGIN, NUMBER, TIMESTAMP for instance). I think for datahashes we should do this as well. I think if you do:

• Call into a contract
• This contract performs a CREATE or CREATE2 operation
• Inside this initcode, store or return DATAHASH. I think this will be zero all times.

I will try to setup a test which tests this to verify.

[g11tech]

I reviewed the tests and they look good for me for CALL, CALLCODE, DELEGATECALL, STATTICCALL (AUTHCALL is untested but this is rather hard to setup so I am ok with this).

However, I wonder: does this work if we perform a CREATE or CREATE2 operation inside the datahash tx and then inside this frame we somewhere access the data hash? It also makes me think that these constants (these datahashes are fed with the tx, right, and are thus constant?) should be moved into some global place. For instance, there are at least two other constants in a tx currently: the origin and the block we run in (uses in opcodes like ORIGIN, NUMBER, TIMESTAMP for instance). I think for datahashes we should do this as well. I think if you do:

* Call into a contract

* This contract performs a `CREATE` or `CREATE2` operation

* Inside this initcode, store or return `DATAHASH`. I think this will be zero all times.

I will try to setup a test which tests this to verify.

isn't running with just call data and no to create equivantent? thats what first test does (the one before the loop)

UPDATE: ahh so contract creates? i think this should be more or less working since normal create works and versioned hashes are available inside contract call now

[jochem-brouwer]

I checked, it doesnt work, see latest commit (if you remove the line from interpreter.ts it fails). It's fixed though, with an added test.

(However I do think we need to move it to some global space since it is constant, we can do this in another PR, this constant versionedHashes being input to the message each time though it never changes in a tx feels a bit weird)

[g11tech]

I checked, it doesnt work, see latest commit (if you remove the line from interpreter.ts it fails). It's fixed though, with an added test.

(However I do think we need to move it to some global space since it is constant, we can do this in another PR, this constant versionedHashes being input to the message each time though it never changes in a tx feels a bit weird)

aha good catch 👍 , agreed a global env should take care of it

[jochem-brouwer]

Will approve (small nit that AUTHCALL is not tested, but if we move to a global env for this, then this is/should not be a problem anymore)

[acolytec3]

LGTM! These fixes address a number of edge case bugs related to EIP 4844

[acolytec3]

This PR also fixes a couple of edge case logic bugs noted below:

• Rounding errors in the dataGasFee computation related to converting from bigint to number and back
• Not handling error conditions when validating kzg proofs in the new point evaluation precompile