Skip to content

op-node: support shanghai L1 blockhash input verification#5144

Merged
tynes merged 1 commit intodevelopfrom
shapella-fix
Mar 15, 2023
Merged

op-node: support shanghai L1 blockhash input verification#5144
tynes merged 1 commit intodevelopfrom
shapella-fix

Conversation

@protolambda
Copy link
Contributor

@protolambda protolambda commented Mar 14, 2023

Description

The Shapella (Shanghai + Capella) L1 network upgrade introduced a breaking change in the L1 block header format the op-node input-validation was not accounting for yet.
This updates the L1 RPC decoding to pull the withdrawals-root from the json, pass it to the geth header type, and verify the block-hash in a Shanghai-compatible way.

Without this fix, the --l1.trustrpc flag is required, or the L1 RPC responses will not be accepted due to the unrecognized input data.

Implementers note: the inclusion of the withdrawals-root is not verified against the L1 time, as that would require L1 chain configuration. Instead it includes it if the data is there, and not if it is not there. It affects the block-hash, so verification is still safe even with a bad L1 RPC, as it can be distinguished from the canonical L1 chain.

Tests

Minor test framework change: Update the action-test L1-miner actor to include Shapella data into test L1 blocks.

Tests:

  • run through a L1 chain that forks into Shanghai, with a verifier and sequencer. The actors use the standard client bindings and derivation code. Both pre and post Shanghai L1 headers will be traversed through by the verifier, and the L2 chain has L1 origins pointing to pre and post Shanghai L1 blocks.
  • unit tests for pre and post-shanghai block hash verification of JSON data

Invariants

Accept post-Shanghai L1 block headers as input.

Metadata

Fix CLI-3628

@protolambda protolambda requested a review from a team as a code owner March 14, 2023 23:47
@protolambda protolambda requested a review from tynes March 14, 2023 23:47
@changeset-bot
Copy link

changeset-bot bot commented Mar 14, 2023

⚠️ No Changeset found

Latest commit: d975d8a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@netlify
Copy link

netlify bot commented Mar 14, 2023

Deploy Preview for opstack-docs canceled.

Name Link
🔨 Latest commit d975d8a
🔍 Latest deploy log https://app.netlify.com/sites/opstack-docs/deploys/64121cbe2c90be0008ea4f4c

@tynes
Copy link
Contributor

tynes commented Mar 14, 2023

If shanghai is turned on at genesis in op-geth, do we need to make any changes there?

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.

rpcHeader fix lgtm, but instead of making an action test can we make unit tests which de-serializes the json of a pre & post shapella block & then verifies the block hash?
This is a very simple deterministic calculation that is well suited to unit testing.

@protolambda protolambda force-pushed the shapella-fix branch 2 times, most recently from 9bc9643 to f86e0a7 Compare March 15, 2023 00:21
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.

lgtm

@protolambda
Copy link
Contributor Author

rebased on develop

@tynes tynes merged commit 1e0e7c0 into develop Mar 15, 2023
@tynes tynes deleted the shapella-fix branch March 15, 2023 20:33
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.

3 participants