-
Notifications
You must be signed in to change notification settings - Fork 149
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
Implementing BLOBHASH
Opcode
#2693
Implementing BLOBHASH
Opcode
#2693
Conversation
…ment `BLOBHASH` operation
There is one more problem. You are matching directly on the txVersionedHashes field, but this field exists inside a cell Map and you are not specifying which transaction you are accessing the hashes of, which means that if the block contains multiple transactions, you might conceivably return the hashes associated with a past or future transaction within the block. You probably want to copy the field from the transaction into the |
- Position of `BLOBHASH` on evm.md - Deleting invalid side condition in `$effectiveGasPrice`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good. I wonder why some of the blobhash related tests still fail. You may want to take a look, but obviously the first priority is getting the proofs passing. As I said before, please timebox that to a couple days at most and let me know if it seems like it will take longer than that.
Co-authored-by: Andrei Văcaru <[email protected]>
@@ -2,19 +2,35 @@ BlockchainTests/GeneralStateTests/Cancun/stEIP4844-blobtransactions/blobhashList | |||
BlockchainTests/GeneralStateTests/Cancun/stEIP4844-blobtransactions/blobhashListBounds4.json,* | |||
BlockchainTests/GeneralStateTests/Cancun/stEIP4844-blobtransactions/blobhashListBounds5.json,* | |||
BlockchainTests/GeneralStateTests/Cancun/stEIP4844-blobtransactions/blobhashListBounds6.json,* | |||
BlockchainTests/GeneralStateTests/Cancun/stEIP4844-blobtransactions/blobhashListBounds7.json,* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a lot of green in this file. Can you make sure you're explaining what is happening here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I saw, it's mostly because these tests were supposed to fail. However, they were failing for the wrong reasons and now, they're "passing" when a failure is expected.
The reasons vary a lot, from wrong balance (lack of implementation of gas accounting) to the block validation.
This PR implements the
BLOBHASH
Opcode as part of the implementation of EIP-4844.To implement this new operation, the type of
txVersionedHashes
had to be changed toList
.