search

ethereum-optimism / optimism

Optimism is Ethereum, scaled.
https://optimism.io
MIT License
5.53k stars 3.17k forks source link

L2 Shapella (Shanghai + Capella) hardfork support #6728

Closed protolambda closed 10 months ago

protolambda commented 1 year ago

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:

Engine API

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

Shanghai testing

danyalprout commented 11 months ago

Hey :wave:

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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