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

L2 Shapella (Shanghai + Capella) hardfork support #6728

Closed
Tracked by #7452
protolambda opened this issue Aug 11, 2023 · 4 comments
Closed
Tracked by #7452

L2 Shapella (Shanghai + Capella) hardfork support #6728

protolambda opened this issue Aug 11, 2023 · 4 comments
Assignees
Labels
M-community Meta: community help wanted

Comments

@protolambda
Copy link
Contributor

This issue tracks the necessary changes to support the Shapella L1 upgrade in the OP-Stack. Starting with specs, op-node and op-geth changes.

These changes should be stacked together as PRs before merging, since end-to-end testing largely depends on various op-geth & engine API shapella features all being activated.

Introduced L1 EIPs

Source: https://blog.ethereum.org/2023/03/28/shapella-mainnet-announcement

Upgrading

The shanghai hardfork affects both op-geth and op-node: the fork affects the Engine API.

We need to add a shanghaiTime rollup-config attribute, and can use the existing geth shanghai upgrade functionality.

op-node changes

Rollup Config shanghaiTime attribute

The rollup-config needs to be made aware of the shanghai hardfork time to enable some of the below changes to apply based on the hardfork timestamp: the Engine API usage is dependent on the hardfork timestamp.

This needs to be documented in the network-upgrades spec: https://github.com/ethereum-optimism/optimism/blob/develop/specs/network-upgrades.md

SSZ ExecutionPayload encoding

The SSZ encoding is used in P2P gossip: we need to support the withdrawals list attribute here (even if empty, since empty is different than non-existent as in pre-shanghai blocks).

This change needs to be documented in the P2P spec: https://github.com/ethereum-optimism/optimism/blob/develop/specs/rollup-node-p2p.md

And requires a gossip-topic upgrade (the topic identifier is already versioned).

L2 Eth-RPC client bindings

When retrieving an execution-payload/block, we need to respect the withdrawals-list. We have missed this in the L1 side of these bindings before (pre-bedrock lauch, durign shanghai upgrade of Goerli).

Engine API

The Engine API changed with Shanghai to support the new withdrawals attribute, and we must migrate over:

https://github.com/ethereum/execution-apis/blob/main/src/engine/shanghai.md

The methods are a hard-requirement for blocks that are past the shanghai timestamp. The methods are backwards compatible with pre-shanghai. We need the V2 methods in op-geth to support Optimism first before we can start using them as the op-node.

The optionality depends on the timestamp of the content:

engine_newPayloadV2: optionality between ExecutionPayloadV1 and ExecutionPayloadV2

engine_forkchoiceUpdatedV2: optionality between PayloadAttributesV2 and PayloadAttributesV2

engine_getPayloadV2: optionality between ExecutionPayloadV1 and ExecutionPayloadV2

Based on the timestamp the verifier and sequencer codepaths (already unified to behind the derive.EngineControl interface)

These engine-API changes need to be documented in the execution-engine spec: https://github.com/ethereum-optimism/optimism/blob/develop/specs/exec-engine.md

op-geth changes

Hardfork time config update

We need the hardfork to be scheduled for Goerli and Mainnet similar to how Regolith was scheduled:

  • Provide a hardfork override CLI option
  • Mutate the chain-config with the shanghai time upon startup, if the chain ID matches

Engine API

Review that all the V2 methods are covered by the Optimism diff.

Shanghai testing

  • Update op-geth testing in the geth Engine API tests in op-e2e, covering the V2 methods one by one
  • Implement an op-e2e test like the regolith test
  • Implement action-tests that run through the Shanghai HF like the Regolith tests
  • Enable shanghai by default in the CI-devnet
  • Schedule and execute Shanghai on the internal devnet
@protolambda protolambda added the go label Aug 11, 2023
@sebastianst sebastianst added the M-community Meta: community help wanted label Aug 11, 2023
@refcell refcell removed the go label Aug 16, 2023
@danyalprout
Copy link
Contributor

Hey 👋

I’ve got a POC of this, have a couple of things I would like to sanity check.

The rollup-config needs to be made aware of the shanghai hardfork time to enable some of the below changes to apply based on the hardfork timestamp

Do we want to enable Shanghai separately from Canyon or tie them both to the Canyon timestamp? Currently I’ve tied Shanghai enablement to the Canyon timestamp in op-node, but happy to decouple if you think that makes more sense.

we need to support the withdrawals list attribute here (even if empty, since empty is different than non-existent as in pre-shanghai blocks

As withdrawals will always be nil pre-Shanghai and always empty post Shanghai. Do we want to make gossip changes here? Or can we just ignore withdrawals at the op-node p2p layer and infer whether it should be empty or nil based on the execution payload timestamp?

optionality between ExecutionPayloadV1 and ExecutionPayloadV2

Would you rather we have multiple versions of the ExecutionPayload struct? Right now I’ve extended the existing one with optional withdrawals.

@tynes
Copy link
Contributor

tynes commented Oct 10, 2023

re: the gossip changes with the withdrawals, it feels a bit cleaner to gossip them even though they will always be empty. Longer term I have been thinking about making ether deposits into L2 be represented as withdrawals, this could greatly reduce the cost of simple ether deposits but it would not work for any sort of cross domain calls, the cross domain calls would still operate the way that they do now. Note that there is currently no consensus on this change but I personally think it would be good to reduce costs.

@tynes
Copy link
Contributor

tynes commented Oct 10, 2023

Do we want to enable Shanghai separately from Canyon or tie them both to the Canyon timestamp? Currently I’ve tied Shanghai enablement to the Canyon timestamp in op-node, but happy to decouple if you think that makes more sense.

Perhaps having devnet targets would help with this decision, ie canyon-devnet-0 has these features, then devnet-1 has an additional set of features. cc @trianglesphere

@trianglesphere
Copy link
Contributor

Do we want to enable Shanghai separately from Canyon or tie them both to the Canyon timestamp? Currently I’ve tied Shanghai enablement to the Canyon timestamp in op-node, but happy to decouple if you think that makes more sense.

Tieing them together in op-node is fine.

optionality between ExecutionPayloadV1 and ExecutionPayloadV2

Would you rather we have multiple versions of the ExecutionPayload struct? Right now I’ve extended the existing one with optional withdrawals.

It looks like geth is using a single version of the execution payload and performing checks on the fields. https://github.com/ethereum/go-ethereum/blob/2c007cfed7db238ba038b8748ce2aabd108ac874/eth/catalyst/api.go#L450-L471

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-community Meta: community help wanted
Projects
Status: Done
Development

No branches or pull requests

6 participants