Skip to content

Verify Versioned Hashes During Optimistic Sync#4832

Merged
mergify[bot] merged 10 commits intosigp:unstablefrom
ethDreamer:verify_versioned_hashes
Feb 18, 2024
Merged

Verify Versioned Hashes During Optimistic Sync#4832
mergify[bot] merged 10 commits intosigp:unstablefrom
ethDreamer:verify_versioned_hashes

Conversation

@ethDreamer
Copy link
Member

@ethDreamer ethDreamer commented Oct 11, 2023

Issue Addressed

Proposed Changes

I've done a bit of refactoring which I think makes the code a bit cleaner and eliminates some cloning of the execution payload.

Additional Info

I used alloy-consensus to pull the versioned hashes from the raw transactions. Perhaps we should replace other functions that rely on ethers_core with alloy crates as well? An easy place to start (that could potentially fit in this PR) is calculating the block hash.

@ethDreamer ethDreamer force-pushed the verify_versioned_hashes branch from 2a81cb8 to 05b7dd6 Compare October 12, 2023 20:11
@michaelsproul michaelsproul deleted the branch sigp:unstable October 16, 2023 23:58
@michaelsproul michaelsproul reopened this Oct 17, 2023
@michaelsproul
Copy link
Member

Please re-target to unstable

@michaelsproul michaelsproul changed the base branch from deneb-free-blobs to unstable October 17, 2023 00:01
@realbigsean
Copy link
Member

Perhaps we should replace other functions that rely on ethers_core with reth crates as well

I'm down to replace this too. The reth version pinned in this PR imports ethers_core anyways though. But reth is currently migrating from ethers-rs and to alloy-rs. In the next reth release, reth-primitives will no longer import ethers_core and the reth-rlp crate has been deprecated (they're directly using alloy-rlp). In the semi-near term I'm pretty sure they plan to migrate reth-primitives to alloy-primitives as well.

So maybe we can merge this one, and open an issue to migrate to alloy-rs, and also get rid of the ether_core dep generally.

@realbigsean realbigsean added the ready-for-review The code is ready for review label Oct 18, 2023
@realbigsean realbigsean self-requested a review October 18, 2023 15:26
@ethDreamer ethDreamer force-pushed the verify_versioned_hashes branch from 05b7dd6 to 54c7913 Compare October 18, 2023 16:37
@realbigsean
Copy link
Member

I'm gonna marked this blocked cause I think we're waiting on reth-primitives to feature gate c-kzg

@dapplion dapplion removed the ready-for-review The code is ready for review label Jan 29, 2024
@ethDreamer ethDreamer force-pushed the verify_versioned_hashes branch from 0a029b8 to 7aadcad Compare February 6, 2024 07:18
@ethDreamer ethDreamer added ready-for-review The code is ready for review and removed blocked labels Feb 6, 2024
@ethDreamer
Copy link
Member Author

Looks like the reth team was able to update alloy just in time for deneb 🎉! After updating this PR to use alloy we no longer have build conflicts over c-kzg. Looks like the tests that I previously designed to test this code are working as expected with alloy as well. I think this is ready to review now @realbigsean

@realbigsean realbigsean added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Feb 7, 2024
@michaelsproul
Copy link
Member

@Mergifyio requeue

@mergify
Copy link

mergify bot commented Feb 15, 2024

requeue

✅ This pull request will be re-embarked automatically

Details

The followup queue command will be automatically executed to re-embark the pull request

@mergify
Copy link

mergify bot commented Feb 15, 2024

queue

🛑 The pull request has been removed from the queue default

Details

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

mergify bot added a commit that referenced this pull request Feb 15, 2024
mergify bot added a commit that referenced this pull request Feb 15, 2024
mergify bot added a commit that referenced this pull request Feb 15, 2024
@michaelsproul
Copy link
Member

@Mergifyio requeue

@mergify
Copy link

mergify bot commented Feb 15, 2024

requeue

✅ This pull request will be re-embarked automatically

Details

The followup queue command will be automatically executed to re-embark the pull request

@mergify
Copy link

mergify bot commented Feb 15, 2024

queue

🛑 The pull request has been removed from the queue default

Details

Pull request #4832 has been dequeued by a dequeue command.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@michaelsproul
Copy link
Member

This is blocked on an update to the CI runner image for the MSRV

https://github.com/sigp/lighthouse/actions/runs/7915078252/job/21606147109?pr=5247

@michaelsproul
Copy link
Member

@Mergifyio unqueue

@mergify
Copy link

mergify bot commented Feb 15, 2024

unqueue

✅ The pull request has been removed from the queue default

paulhauner added a commit that referenced this pull request Feb 18, 2024
Squashed commit of the following:

commit 9cd7386
Author: Mark Mackey <mark@sigmaprime.io>
Date:   Fri Feb 9 12:57:39 2024 +0800

    Update to rust 1.75 & Pin alloy-consensus

commit 5d5b08d
Author: Mark Mackey <mark@sigmaprime.io>
Date:   Fri Feb 9 12:39:34 2024 +0800

    Faster Versioned Hash Extraction

commit 2dddb84
Author: ethDreamer <37123614+ethDreamer@users.noreply.github.com>
Date:   Fri Feb 9 11:16:28 2024 +0800

    Update beacon_node/execution_layer/src/engine_api/new_payload_request.rs

    Co-authored-by: realbigsean <seananderson33@GMAIL.com>

commit 32d3e99
Author: ethDreamer <37123614+ethDreamer@users.noreply.github.com>
Date:   Tue Feb 6 17:03:28 2024 +0800

    Update to use Alloy Instead of Reth Crates (#14)

commit 7aadcad
Author: Mark Mackey <mark@sigmaprime.io>
Date:   Wed Oct 18 14:48:05 2023 -0500

    Fix Problems Caused By  Merge

commit 04b5f26
Author: Mark Mackey <mark@sigmaprime.io>
Date:   Wed Oct 11 16:45:11 2023 -0500

    Added Moar Tests

commit 3e78411
Author: Mark Mackey <mark@sigmaprime.io>
Date:   Wed Oct 11 15:14:39 2023 -0500

    Added Tests for Version Hash Verification

commit dd18fed
Author: Mark Mackey <mark@sigmaprime.io>
Date:   Tue Oct 10 17:19:49 2023 -0500

    Verify Versioned Hashes

commit faa9ff1
Author: Mark Mackey <mark@sigmaprime.io>
Date:   Fri Oct 6 18:10:50 2023 -0500

    Refactor for Clarity

commit 27716c0
Author: Mark Mackey <mark@sigmaprime.io>
Date:   Fri Oct 6 17:56:16 2023 -0500

    Convert NewPayloadRequest to use Reference
@paulhauner paulhauner mentioned this pull request Feb 18, 2024
@realbigsean
Copy link
Member

@mergify requeue

@mergify
Copy link

mergify bot commented Feb 18, 2024

requeue

✅ This pull request will be re-embarked automatically

Details

The followup queue command will be automatically executed to re-embark the pull request

@mergify
Copy link

mergify bot commented Feb 18, 2024

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at a264afd

mergify bot added a commit that referenced this pull request Feb 18, 2024
@mergify mergify bot merged commit a264afd into sigp:unstable Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards-incompat Backwards-incompatible API change deneb ready-for-merge This PR is ready to merge. v5.0.0 Q1 2024

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants