Skip to content
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

Canyon: Blocks V2 P2P & Testing #7707

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

danyalprout
Copy link
Contributor

@danyalprout danyalprout commented Oct 16, 2023

Description
P2P changes for this issue.

  • Add blocksv2 p2p layer
    • New blocksV2 topic for post Shanghai/Canyon blocks
    • SSZ encoding changes to support v2 blocks
  • Canyon testing
    • Flag to enable / OP_E2E_USE_CANYON=true to enable the tests to use Canyon
    • Op Geth testing (withdrawals + push0 opcode)
  • Set withdrawals for new payloads to []

TODO

  • Add an e2e transition test Regolith to Canyon
  • Add a build-server check where OP_E2E_USE_CANYON=true

op-service/eth/ssz_test.go Outdated Show resolved Hide resolved
@danyalprout danyalprout force-pushed the shanghai-p2p branch 8 times, most recently from baed4a9 to 12aacda Compare October 17, 2023 13:40
@danyalprout danyalprout marked this pull request as ready for review October 17, 2023 19:50
@danyalprout danyalprout requested review from a team and protolambda as code owners October 17, 2023 19:50
Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

We also need to add a check to

func AttributesMatchBlock(attrs *eth.PayloadAttributes, parentHash common.Hash, block *eth.ExecutionPayload, l log.Logger) error {
to ensure that we verify that the withdrawals from gossip are the same as we created locally (i.e. empty).

We could even completely disable withdrawals and require them to be empty (but not-nil) for the comment b/c we don't have a need for them. It kinda looks like deposits, but our deposits can do much more than withdrawals.

op-node/p2p/sync.go Outdated Show resolved Hide resolved
Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

I've reviewed everything. There's a couple things to look into, but broadly looks good.

op-service/eth/ssz.go Outdated Show resolved Hide resolved
Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

TYVM for implementing this

@trianglesphere trianglesphere added this pull request to the merge queue Oct 19, 2023
Merged via the queue into ethereum-optimism:develop with commit 601de6b Oct 19, 2023
64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants