Skip to content

Conversation

@r0qs
Copy link
Member

@r0qs r0qs commented Dec 26, 2023

Depends on #14743 (merged)
First part of #14740

This PR implements the blobhash opcode as specified in the EIP-4844 and add tests for assembler mode. The opcode is not exposed to Solidity in this part; that will be addressed in part 2. While it's intended to be non-breaking, additional tests may be required.

@r0qs r0qs added has dependencies The PR depends on other PRs that must be merged first feature labels Dec 26, 2023
@r0qs r0qs added this to the 0.8.24 milestone Dec 26, 2023
@r0qs r0qs self-assigned this Dec 26, 2023
@r0qs r0qs force-pushed the eip-4844-add-blobhash branch 4 times, most recently from 01f3c80 to 367ca5f Compare December 26, 2023 20:50
@r0qs r0qs marked this pull request as ready for review December 26, 2023 21:24
@nikola-matic nikola-matic force-pushed the bump-evmone-and-evmc-to-11 branch from 498252a to 9160451 Compare December 27, 2023 10:57
@r0qs r0qs force-pushed the eip-4844-add-blobhash branch 2 times, most recently from 35e367d to 873e62e Compare December 27, 2023 20:27
@r0qs r0qs requested a review from nikola-matic December 27, 2023 20:29
@r0qs r0qs mentioned this pull request Dec 27, 2023
@r0qs r0qs changed the title Implement blobhash opcode EIP 4844 - Implement blobhash opcode (part 1) Dec 27, 2023
@r0qs r0qs changed the title EIP 4844 - Implement blobhash opcode (part 1) EIP 4844 - blobhash opcode (part 1) Dec 27, 2023
@r0qs r0qs force-pushed the eip-4844-add-blobhash branch from e65d8bc to 69122e0 Compare December 27, 2023 21:43
@r0qs r0qs force-pushed the eip-4844-add-blobhash branch from 69122e0 to 66e145f Compare January 5, 2024 12:28
@cameel cameel force-pushed the bump-evmone-and-evmc-to-11 branch 3 times, most recently from 93b8bfa to d5119f7 Compare January 6, 2024 16:29
Base automatically changed from bump-evmone-and-evmc-to-11 to develop January 6, 2024 18:34
@cameel cameel force-pushed the eip-4844-add-blobhash branch from 66e145f to 9eb631b Compare January 6, 2024 19:32
Copy link
Collaborator

@cameel cameel left a comment

Choose a reason for hiding this comment

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

This is missing a changelog entry (we usually add a separate one for the opcode and for high-level support in Solidity). Also, the bits of docs from #14759 describing the opcode should really be in this PR.

Other than that I did not find any serious problems. Mostly small things, easy to tweak. I added a fixup to deal with some of it, the rest is in comments.

Since the evmone update is now merged, I rebased the PR on develop.

@r0qs r0qs force-pushed the eip-4844-add-blobhash branch 3 times, most recently from c65588e to 08bbb84 Compare January 9, 2024 16:02
@r0qs r0qs force-pushed the eip-4844-add-blobhash branch from 1d6a91f to 9a7639e Compare January 11, 2024 17:38
Copy link
Collaborator

@cameel cameel left a comment

Choose a reason for hiding this comment

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

I looked through all the uses of the already existing blockhash() to make sure we're not forgetting anything we should have for blobhash() here and found only one minor test that might need it.

Other than that, we actually should change #14757 (comment) back (I'll post details in a moment). EDIT: Actually it's fine. I wanted to note that the variable assigned to the pointer will go out of scope, but it's static so it's fine.

I'm going to look through #14759 before approving this in case something from there should still end up in this PR, but if not, we'll be done here.

@r0qs r0qs force-pushed the eip-4844-add-blobhash branch 4 times, most recently from d762c6d to 1af000d Compare January 12, 2024 17:38
@r0qs r0qs force-pushed the eip-4844-add-blobhash branch 2 times, most recently from 1f5e3b7 to eabe457 Compare January 15, 2024 14:59
Copy link
Collaborator

@nikola-matic nikola-matic left a comment

Choose a reason for hiding this comment

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

Aside from the one question I left (which is important, at least for me), looks good.

@r0qs r0qs force-pushed the eip-4844-add-blobhash branch 2 times, most recently from b00b2cd to 2150f74 Compare January 16, 2024 18:09
@r0qs r0qs force-pushed the eip-4844-add-blobhash branch 2 times, most recently from 3cb794d to 9243997 Compare January 18, 2024 15:15
Co-authored-by: Kamil Śliwak <kamil.sliwak@codepoets.it>
@r0qs r0qs force-pushed the eip-4844-add-blobhash branch from 9243997 to 269951e Compare January 18, 2024 16:03
* Introduce global ``block.blobbasefee`` for retrieving the blob base fee of the current block.
* Yul: Introduce builtin ``blobbasefee()`` for retrieving the blob base fee of the current block.

* Yul: Introduce builtin ``blobhash()`` for retrieving versioned hashes of blobs associated with the transaction.
Copy link
Collaborator

@cameel cameel Jan 18, 2024

Choose a reason for hiding this comment

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

Oh, one more thing, let's finally stop messing with the spacing in the changelog :)

Suggested change
* Yul: Introduce builtin ``blobhash()`` for retrieving versioned hashes of blobs associated with the transaction.
* Yul: Introduce builtin ``blobhash()`` for retrieving versioned hashes of blobs associated with the transaction.

I fixed it earlier in all those PRs but it keeps reappearing again and again for some reason. Don't worry about the wrong spacing of the Compiler Features: section. We'll fix it when we sort the changelog. In PRs just add an entry for your PR and don't touch other lines.

@r0qs r0qs enabled auto-merge January 18, 2024 16:37
@r0qs r0qs merged commit ec563a1 into develop Jan 18, 2024
@r0qs r0qs deleted the eip-4844-add-blobhash branch January 18, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants