From ff5fa9e5416fc70b610d314ef24a09318091fade Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Wed, 6 Nov 2024 09:36:37 -0500 Subject: [PATCH 01/11] Config: Enable Isthmus --- bedrock-devnet/devnet/__init__.py | 2 +- op-chain-ops/genesis/config.go | 13 +++++++++++++ op-chain-ops/genesis/layer_two.go | 1 + op-node/rollup/chain_spec.go | 12 ++++++++++++ op-node/rollup/types.go | 22 ++++++++++++++++++++++ op-node/rollup/types_test.go | 19 +++++++++++++++++++ 6 files changed, 68 insertions(+), 1 deletion(-) diff --git a/bedrock-devnet/devnet/__init__.py b/bedrock-devnet/devnet/__init__.py index 70243dc790c8..7bf5c6a2595c 100644 --- a/bedrock-devnet/devnet/__init__.py +++ b/bedrock-devnet/devnet/__init__.py @@ -24,7 +24,7 @@ log = logging.getLogger() # Global constants -FORKS = ["delta", "ecotone", "fjord", "granite", "holocene"] +FORKS = ["delta", "ecotone", "fjord", "granite", "holocene", "isthmus"] # Global environment variables DEVNET_NO_BUILD = os.getenv('DEVNET_NO_BUILD') == "true" diff --git a/op-chain-ops/genesis/config.go b/op-chain-ops/genesis/config.go index 7ed0a6e2f682..26a845dc2f73 100644 --- a/op-chain-ops/genesis/config.go +++ b/op-chain-ops/genesis/config.go @@ -345,6 +345,9 @@ type UpgradeScheduleDeployConfig struct { // L2GenesisHoloceneTimeOffset is the number of seconds after genesis block that the Holocene hard fork activates. // Set it to 0 to activate at genesis. Nil to disable Holocene. L2GenesisHoloceneTimeOffset *hexutil.Uint64 `json:"l2GenesisHoloceneTimeOffset,omitempty"` + // L2GenesisIsthmusTimeOffset is the number of seconds after genesis block that the Isthmus hard fork activates. + // Set it to 0 to activate at genesis. Nil to disable Isthmus. + L2GenesisIsthmusTimeOffset *hexutil.Uint64 `json:"l2GenesisIsthmusTimeOffset,omitempty"` // L2GenesisInteropTimeOffset is the number of seconds after genesis block that the Interop hard fork activates. // Set it to 0 to activate at genesis. Nil to disable Interop. L2GenesisInteropTimeOffset *hexutil.Uint64 `json:"l2GenesisInteropTimeOffset,omitempty"` @@ -385,6 +388,8 @@ func (d *UpgradeScheduleDeployConfig) ForkTimeOffset(fork rollup.ForkName) *uint return (*uint64)(d.L2GenesisGraniteTimeOffset) case rollup.Holocene: return (*uint64)(d.L2GenesisHoloceneTimeOffset) + case rollup.Isthmus: + return (*uint64)(d.L2GenesisIsthmusTimeOffset) case rollup.Interop: return (*uint64)(d.L2GenesisInteropTimeOffset) default: @@ -408,6 +413,8 @@ func (d *UpgradeScheduleDeployConfig) SetForkTimeOffset(fork rollup.ForkName, of d.L2GenesisGraniteTimeOffset = (*hexutil.Uint64)(offset) case rollup.Holocene: d.L2GenesisHoloceneTimeOffset = (*hexutil.Uint64)(offset) + case rollup.Isthmus: + d.L2GenesisIsthmusTimeOffset = (*hexutil.Uint64)(offset) case rollup.Interop: d.L2GenesisInteropTimeOffset = (*hexutil.Uint64)(offset) default: @@ -472,6 +479,10 @@ func (d *UpgradeScheduleDeployConfig) HoloceneTime(genesisTime uint64) *uint64 { return offsetToUpgradeTime(d.L2GenesisHoloceneTimeOffset, genesisTime) } +func (d *UpgradeScheduleDeployConfig) IsthmusTime(genesisTime uint64) *uint64 { + return offsetToUpgradeTime(d.L2GenesisIsthmusTimeOffset, genesisTime) +} + func (d *UpgradeScheduleDeployConfig) InteropTime(genesisTime uint64) *uint64 { return offsetToUpgradeTime(d.L2GenesisInteropTimeOffset, genesisTime) } @@ -504,6 +515,7 @@ func (d *UpgradeScheduleDeployConfig) forks() []Fork { {L2GenesisTimeOffset: d.L2GenesisFjordTimeOffset, Name: string(L2AllocsFjord)}, {L2GenesisTimeOffset: d.L2GenesisGraniteTimeOffset, Name: string(L2AllocsGranite)}, {L2GenesisTimeOffset: d.L2GenesisHoloceneTimeOffset, Name: string(L2AllocsHolocene)}, + {L2GenesisTimeOffset: d.L2GenesisIsthmusTimeOffset, Name: string(L2AllocsIsthmus)}, } } @@ -1014,6 +1026,7 @@ func (d *DeployConfig) RollupConfig(l1StartBlock *types.Header, l2GenesisBlockHa FjordTime: d.FjordTime(l1StartTime), GraniteTime: d.GraniteTime(l1StartTime), HoloceneTime: d.HoloceneTime(l1StartTime), + IsthmusTime: d.IsthmusTime(l1StartTime), InteropTime: d.InteropTime(l1StartTime), ProtocolVersionsAddress: d.ProtocolVersionsProxy, AltDAConfig: altDA, diff --git a/op-chain-ops/genesis/layer_two.go b/op-chain-ops/genesis/layer_two.go index b1c0922de1eb..50a70c42a2b9 100644 --- a/op-chain-ops/genesis/layer_two.go +++ b/op-chain-ops/genesis/layer_two.go @@ -27,6 +27,7 @@ const ( L2AllocsFjord L2AllocsMode = "fjord" L2AllocsGranite L2AllocsMode = "granite" L2AllocsHolocene L2AllocsMode = "holocene" + L2AllocsIsthmus L2AllocsMode = "isthmus" ) var ( diff --git a/op-node/rollup/chain_spec.go b/op-node/rollup/chain_spec.go index 1ddcb3190290..936bb2b17dfa 100644 --- a/op-node/rollup/chain_spec.go +++ b/op-node/rollup/chain_spec.go @@ -41,6 +41,7 @@ const ( Fjord ForkName = "fjord" Granite ForkName = "granite" Holocene ForkName = "holocene" + Isthmus ForkName = "isthmus" Interop ForkName = "interop" // ADD NEW FORKS TO AllForks BELOW! None ForkName = "none" @@ -55,6 +56,7 @@ var AllForks = []ForkName{ Fjord, Granite, Holocene, + Isthmus, Interop, // ADD NEW FORKS HERE! } @@ -114,6 +116,11 @@ func (s *ChainSpec) IsHolocene(t uint64) bool { return s.config.IsHolocene(t) } +// IsIsthmus returns true if t >= isthmus_time +func (s *ChainSpec) IsIsthmus(t uint64) bool { + return s.config.IsIsthmus(t) +} + // MaxChannelBankSize returns the maximum number of bytes the can allocated inside the channel bank // before pruning occurs at the given timestamp. func (s *ChainSpec) MaxChannelBankSize(t uint64) uint64 { @@ -185,6 +192,9 @@ func (s *ChainSpec) CheckForkActivation(log log.Logger, block eth.L2BlockRef) { if s.config.IsHolocene(block.Time) { s.currentFork = Holocene } + if s.config.IsIsthmus(block.Time) { + s.currentFork = Isthmus + } if s.config.IsInterop(block.Time) { s.currentFork = Interop } @@ -209,6 +219,8 @@ func (s *ChainSpec) CheckForkActivation(log log.Logger, block eth.L2BlockRef) { foundActivationBlock = s.config.IsGraniteActivationBlock(block.Time) case Holocene: foundActivationBlock = s.config.IsHoloceneActivationBlock(block.Time) + case Isthmus: + foundActivationBlock = s.config.IsIsthmusActivationBlock(block.Time) case Interop: foundActivationBlock = s.config.IsInteropActivationBlock(block.Time) } diff --git a/op-node/rollup/types.go b/op-node/rollup/types.go index e5e541a6f7a5..88c67d7fda2b 100644 --- a/op-node/rollup/types.go +++ b/op-node/rollup/types.go @@ -120,6 +120,10 @@ type Config struct { // Active if HoloceneTime != nil && L2 block timestamp >= *HoloceneTime, inactive otherwise. HoloceneTime *uint64 `json:"holocene_time,omitempty"` + // IsthmusTime sets the activation time of the Isthmus network upgrade. + // Active if IsthmusTime != nil && L2 block timestamp >= *IsthmusTime, inactive otherwise. + IsthmusTime *uint64 `json:"isthmus_time,omitempty"` + // InteropTime sets the activation time for an experimental feature-set, activated like a hardfork. // Active if InteropTime != nil && L2 block timestamp >= *InteropTime, inactive otherwise. InteropTime *uint64 `json:"interop_time,omitempty"` @@ -404,6 +408,11 @@ func (c *Config) IsHolocene(timestamp uint64) bool { return c.HoloceneTime != nil && timestamp >= *c.HoloceneTime } +// IsIsthmus returns true if the Isthmus hardfork is active at or past the given timestamp. +func (c *Config) IsIsthmus(timestamp uint64) bool { + return c.IsthmusTime != nil && timestamp >= *c.IsthmusTime +} + // IsInterop returns true if the Interop hardfork is active at or past the given timestamp. func (c *Config) IsInterop(timestamp uint64) bool { return c.InteropTime != nil && timestamp >= *c.InteropTime @@ -459,6 +468,14 @@ func (c *Config) IsHoloceneActivationBlock(l2BlockTime uint64) bool { !c.IsHolocene(l2BlockTime-c.BlockTime) } +// IsIsthmusActivationBlock returns whether the specified block is the first block subject to the +// Isthmus upgrade. +func (c *Config) IsIsthmusActivationBlock(l2BlockTime uint64) bool { + return c.IsIsthmus(l2BlockTime) && + l2BlockTime >= c.BlockTime && + !c.IsIsthmus(l2BlockTime-c.BlockTime) +} + func (c *Config) IsInteropActivationBlock(l2BlockTime uint64) bool { return c.IsInterop(l2BlockTime) && l2BlockTime >= c.BlockTime && @@ -482,6 +499,9 @@ func (c *Config) ActivateAtGenesis(hardfork ForkName) { case Interop: c.InteropTime = new(uint64) fallthrough + case Isthmus: + c.IsthmusTime = new(uint64) + fallthrough case Holocene: c.HoloceneTime = new(uint64) fallthrough @@ -621,6 +641,7 @@ func (c *Config) Description(l2Chains map[string]string) string { banner += fmt.Sprintf(" - Fjord: %s\n", fmtForkTimeOrUnset(c.FjordTime)) banner += fmt.Sprintf(" - Granite: %s\n", fmtForkTimeOrUnset(c.GraniteTime)) banner += fmt.Sprintf(" - Holocene: %s\n", fmtForkTimeOrUnset(c.HoloceneTime)) + banner += fmt.Sprintf(" - Isthmus: %s\n", fmtForkTimeOrUnset(c.IsthmusTime)) banner += fmt.Sprintf(" - Interop: %s\n", fmtForkTimeOrUnset(c.InteropTime)) // Report the protocol version banner += fmt.Sprintf("Node supports up to OP-Stack Protocol Version: %s\n", OPStackSupport) @@ -657,6 +678,7 @@ func (c *Config) LogDescription(log log.Logger, l2Chains map[string]string) { "fjord_time", fmtForkTimeOrUnset(c.FjordTime), "granite_time", fmtForkTimeOrUnset(c.GraniteTime), "holocene_time", fmtForkTimeOrUnset(c.HoloceneTime), + "isthmus_time", fmtForkTimeOrUnset(c.IsthmusTime), "interop_time", fmtForkTimeOrUnset(c.InteropTime), "alt_da", c.AltDAConfig != nil, ) diff --git a/op-node/rollup/types_test.go b/op-node/rollup/types_test.go index 11c4db505c96..066ad0f7ec52 100644 --- a/op-node/rollup/types_test.go +++ b/op-node/rollup/types_test.go @@ -250,6 +250,15 @@ func TestActivations(t *testing.T) { return c.IsHolocene(t) }, }, + { + name: "Isthmus", + setUpgradeTime: func(t *uint64, c *Config) { + c.IsthmusTime = t + }, + checkEnabled: func(t uint64, c *Config) bool { + return c.IsIsthmus(t) + }, + }, { name: "Interop", setUpgradeTime: func(t *uint64, c *Config) { @@ -518,10 +527,20 @@ func TestConfig_Check(t *testing.T) { canyonTime := uint64(2) deltaTime := uint64(3) ecotoneTime := uint64(4) + fjordTime := uint64(5) + graniteTime := uint64(6) + holoceneTime := uint64(7) + isthmusTime := uint64(8) + interopTime := uint64(9) cfg.RegolithTime = ®olithTime cfg.CanyonTime = &canyonTime cfg.DeltaTime = &deltaTime cfg.EcotoneTime = &ecotoneTime + cfg.FjordTime = &fjordTime + cfg.GraniteTime = &graniteTime + cfg.HoloceneTime = &holoceneTime + cfg.IsthmusTime = &isthmusTime + cfg.InteropTime = &interopTime }, expectedErr: nil, }, From 8db62541c9f9a56cf18f239d99730f9f3e027d8d Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Tue, 29 Oct 2024 18:46:37 -0400 Subject: [PATCH 02/11] Add l2 withdrawals root to exec payload, ssz & gossip * add withdrawalsRoot to ExecutionPayload. * add a `BlocksV4` topic to p2p gossip and check for non-empty withdrawalsRoot on v4 topic. * add checks for whether block is type v4 and apply relevant marshal/unmarshal for l2 withdrawals root. --- op-node/p2p/gossip.go | 37 ++++++++++++++++++++++++++++++++-- op-service/eth/ssz.go | 44 ++++++++++++++++++++++++++++++++++------- op-service/eth/types.go | 10 +++++++++- 3 files changed, 81 insertions(+), 10 deletions(-) diff --git a/op-node/p2p/gossip.go b/op-node/p2p/gossip.go index f835dfb2f163..cd75efe74adf 100644 --- a/op-node/p2p/gossip.go +++ b/op-node/p2p/gossip.go @@ -78,10 +78,19 @@ func blocksTopicV3(cfg *rollup.Config) string { return fmt.Sprintf("/optimism/%s/2/blocks", cfg.L2ChainID.String()) } +func blocksTopicV4(cfg *rollup.Config) string { + return fmt.Sprintf("/optimism/%s/3/blocks", cfg.L2ChainID.String()) +} + // BuildSubscriptionFilter builds a simple subscription filter, // to help protect against peers spamming useless subscriptions. func BuildSubscriptionFilter(cfg *rollup.Config) pubsub.SubscriptionFilter { - return pubsub.NewAllowlistSubscriptionFilter(blocksTopicV1(cfg), blocksTopicV2(cfg), blocksTopicV3(cfg)) // add more topics here in the future, if any. + return pubsub.NewAllowlistSubscriptionFilter( + blocksTopicV1(cfg), + blocksTopicV2(cfg), + blocksTopicV3(cfg), + blocksTopicV4(cfg), // add more topics here in the future, if any. + ) } var msgBufPool = sync.Pool{New: func() any { @@ -386,6 +395,10 @@ func BuildBlocksValidator(log log.Logger, cfg *rollup.Config, runCfg GossipRunti return pubsub.ValidationReject } + if blockVersion.HasWithdrawalsRoot() && payload.WithdrawalsRoot == nil { + log.Warn("payload is on v4 topic, but has nil withdrawals root", "bad_hash", payload.BlockHash.String()) + } + seen, ok := blockHeightLRU.Get(uint64(payload.BlockNumber)) if !ok { seen = new(seenBlocks) @@ -450,6 +463,7 @@ type GossipTopicInfo interface { BlocksTopicV1Peers() []peer.ID BlocksTopicV2Peers() []peer.ID BlocksTopicV3Peers() []peer.ID + BlocksTopicV4Peers() []peer.ID } type GossipOut interface { @@ -485,6 +499,7 @@ type publisher struct { blocksV1 *blockTopic blocksV2 *blockTopic blocksV3 *blockTopic + blocksV4 *blockTopic runCfg GossipRuntimeConfig } @@ -507,7 +522,12 @@ func combinePeers(allPeers ...[]peer.ID) []peer.ID { } func (p *publisher) AllBlockTopicsPeers() []peer.ID { - return combinePeers(p.BlocksTopicV1Peers(), p.BlocksTopicV2Peers(), p.BlocksTopicV3Peers()) + return combinePeers( + p.BlocksTopicV1Peers(), + p.BlocksTopicV2Peers(), + p.BlocksTopicV3Peers(), + p.BlocksTopicV4Peers(), + ) } func (p *publisher) BlocksTopicV1Peers() []peer.ID { @@ -522,6 +542,10 @@ func (p *publisher) BlocksTopicV3Peers() []peer.ID { return p.blocksV3.topic.ListPeers() } +func (p *publisher) BlocksTopicV4Peers() []peer.ID { + return p.blocksV4.topic.ListPeers() +} + func (p *publisher) PublishL2Payload(ctx context.Context, envelope *eth.ExecutionPayloadEnvelope, signer Signer) error { res := msgBufPool.Get().(*[]byte) buf := bytes.NewBuffer((*res)[:0]) @@ -597,6 +621,14 @@ func JoinGossip(self peer.ID, ps *pubsub.PubSub, log log.Logger, cfg *rollup.Con return nil, fmt.Errorf("failed to setup blocks v3 p2p: %w", err) } + v4Logger := log.New("topic", "blocksV4") + blocksV4Validator := guardGossipValidator(log, logValidationResult(self, "validated blockv4", v4Logger, BuildBlocksValidator(v4Logger, cfg, runCfg, eth.BlockV4))) + blocksV4, err := newBlockTopic(p2pCtx, blocksTopicV4(cfg), ps, v4Logger, gossipIn, blocksV4Validator) + if err != nil { + p2pCancel() + return nil, fmt.Errorf("failed to setup blocks v4 p2p: %w", err) + } + return &publisher{ log: log, cfg: cfg, @@ -604,6 +636,7 @@ func JoinGossip(self peer.ID, ps *pubsub.PubSub, log log.Logger, cfg *rollup.Con blocksV1: blocksV1, blocksV2: blocksV2, blocksV3: blocksV3, + blocksV4: blocksV4, runCfg: runCfg, }, nil } diff --git a/op-service/eth/ssz.go b/op-service/eth/ssz.go index ae3d89ec3614..c8a165cc33db 100644 --- a/op-service/eth/ssz.go +++ b/op-service/eth/ssz.go @@ -18,6 +18,7 @@ const ( // iota is reset to 0 BlockV1 BlockVersion = iota BlockV2 BlockV3 + BlockV4 ) // ExecutionPayload and ExecutionPayloadEnvelope are the only SSZ types we have to marshal/unmarshal, @@ -49,9 +50,12 @@ const ( // V1 + Withdrawals offset blockV2FixedPart = blockV1FixedPart + 4 - // V2 + BlobGasUed + ExcessBlobGas + // V2 + BlobGasUsed + ExcessBlobGas blockV3FixedPart = blockV2FixedPart + 8 + 8 + // V3 + WithdrawalsRoot + blockV4FixedPart = blockV3FixedPart + 32 + withdrawalSize = 8 + 8 + 20 + 8 // MAX_TRANSACTIONS_PER_PAYLOAD in consensus spec @@ -64,19 +68,25 @@ const ( ) func (v BlockVersion) HasBlobProperties() bool { - return v == BlockV3 + return v == BlockV3 || v == BlockV4 } func (v BlockVersion) HasWithdrawals() bool { - return v == BlockV2 || v == BlockV3 + return v == BlockV2 || v == BlockV3 || v == BlockV4 } func (v BlockVersion) HasParentBeaconBlockRoot() bool { - return v == BlockV3 + return v == BlockV3 || v == BlockV4 +} + +func (v BlockVersion) HasWithdrawalsRoot() bool { + return v == BlockV4 } func executionPayloadFixedPart(version BlockVersion) uint32 { - if version == BlockV3 { + if version == BlockV4 { + return blockV4FixedPart + } else if version == BlockV3 { return blockV3FixedPart } else if version == BlockV2 { return blockV2FixedPart @@ -86,7 +96,9 @@ func executionPayloadFixedPart(version BlockVersion) uint32 { } func (payload *ExecutionPayload) inferVersion() BlockVersion { - if payload.ExcessBlobGas != nil && payload.BlobGasUsed != nil { + if payload.WithdrawalsRoot != nil { + return BlockV4 + } else if payload.ExcessBlobGas != nil && payload.BlobGasUsed != nil { return BlockV3 } else if payload.Withdrawals != nil { return BlockV2 @@ -197,7 +209,8 @@ func (payload *ExecutionPayload) MarshalSSZ(w io.Writer) (n int, err error) { offset += 4 } - if payload.inferVersion() == BlockV3 { + payloadVersion := payload.inferVersion() + if payloadVersion == BlockV3 || payloadVersion == BlockV4 { if payload.BlobGasUsed == nil || payload.ExcessBlobGas == nil { return 0, errors.New("cannot encode ecotone payload without dencun header attributes") } @@ -207,6 +220,14 @@ func (payload *ExecutionPayload) MarshalSSZ(w io.Writer) (n int, err error) { offset += 8 } + if payloadVersion == BlockV4 { + if payload.WithdrawalsRoot == nil { + return 0, errors.New("cannot encode Isthmus payload without withdrawals root") + } + copy(buf[offset:offset+32], (*payload.WithdrawalsRoot)[:]) + offset += 32 + } + if payload.Withdrawals != nil && offset != fixedSize { panic("withdrawals - fixed part size is inconsistent") } @@ -330,7 +351,16 @@ func (payload *ExecutionPayload) UnmarshalSSZ(version BlockVersion, scope uint32 offset += 8 excessBlobGas := binary.LittleEndian.Uint64(buf[offset : offset+8]) payload.ExcessBlobGas = (*Uint64Quantity)(&excessBlobGas) + offset += 8 } + + if version == BlockV4 { + withdrawalsRoot := common.Hash{} + copy(withdrawalsRoot[:], buf[offset:offset+32]) + payload.WithdrawalsRoot = &withdrawalsRoot + offset += 32 + } + _ = offset // for future extensions: we keep the offset accurate for extensions if transactionsOffset > extraDataOffset+32 || transactionsOffset > scope { diff --git a/op-service/eth/types.go b/op-service/eth/types.go index 122e9a4df783..5e5af3d89aa2 100644 --- a/op-service/eth/types.go +++ b/op-service/eth/types.go @@ -231,6 +231,8 @@ type ExecutionPayload struct { BlobGasUsed *Uint64Quantity `json:"blobGasUsed,omitempty"` // Nil if not present (Bedrock, Canyon, Delta) ExcessBlobGas *Uint64Quantity `json:"excessBlobGas,omitempty"` + // Nil if not present (Isthmus) + WithdrawalsRoot *common.Hash `json:"withdrawalsRoot,omitempty"` } func (payload *ExecutionPayload) ID() BlockID { @@ -256,6 +258,10 @@ func (payload *ExecutionPayload) CanyonBlock() bool { return payload.Withdrawals != nil } +func (payload *ExecutionPayload) IsthmusBlock() bool { + return payload.WithdrawalsRoot != nil +} + // CheckBlockHash recomputes the block hash and returns if the embedded block hash matches. func (envelope *ExecutionPayloadEnvelope) CheckBlockHash() (actual common.Hash, ok bool) { payload := envelope.ExecutionPayload @@ -283,7 +289,9 @@ func (envelope *ExecutionPayloadEnvelope) CheckBlockHash() (actual common.Hash, ParentBeaconRoot: envelope.ParentBeaconBlockRoot, } - if payload.CanyonBlock() { + if payload.IsthmusBlock() { + header.WithdrawalsHash = payload.WithdrawalsRoot + } else if payload.CanyonBlock() { withdrawalHash := types.DeriveSha(*payload.Withdrawals, hasher) header.WithdrawalsHash = &withdrawalHash } From 86232e93c457f457a464eb537433b2210f1f8f3d Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Wed, 30 Oct 2024 10:50:38 -0400 Subject: [PATCH 03/11] Config: Add Isthmus fork config --- op-node/rollup/types.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/op-node/rollup/types.go b/op-node/rollup/types.go index 88c67d7fda2b..956911961226 100644 --- a/op-node/rollup/types.go +++ b/op-node/rollup/types.go @@ -332,6 +332,9 @@ func (cfg *Config) Check() error { if err := checkFork(cfg.GraniteTime, cfg.HoloceneTime, Granite, Holocene); err != nil { return err } + if err := checkFork(cfg.HoloceneTime, cfg.IsthmusTime, Holocene, Isthmus); err != nil { + return err + } return nil } @@ -641,7 +644,7 @@ func (c *Config) Description(l2Chains map[string]string) string { banner += fmt.Sprintf(" - Fjord: %s\n", fmtForkTimeOrUnset(c.FjordTime)) banner += fmt.Sprintf(" - Granite: %s\n", fmtForkTimeOrUnset(c.GraniteTime)) banner += fmt.Sprintf(" - Holocene: %s\n", fmtForkTimeOrUnset(c.HoloceneTime)) - banner += fmt.Sprintf(" - Isthmus: %s\n", fmtForkTimeOrUnset(c.IsthmusTime)) + banner += fmt.Sprintf(" - Isthmus: %s\n", fmtForkTimeOrUnset(c.IsthmusTime)) banner += fmt.Sprintf(" - Interop: %s\n", fmtForkTimeOrUnset(c.InteropTime)) // Report the protocol version banner += fmt.Sprintf("Node supports up to OP-Stack Protocol Version: %s\n", OPStackSupport) From 3dc861670d827909120b19d1ee659d6d0ccedb19 Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Wed, 30 Oct 2024 22:18:31 -0400 Subject: [PATCH 04/11] Update logic for l2 withdrawals root * attributes check * outputV0AtBlock api update * minor type updates --- op-node/node/api.go | 2 + .../rollup/attributes/engine_consolidate.go | 55 ++++++++++++------- op-service/eth/block_info.go | 9 +++ op-service/sources/l2_client.go | 30 ++++++---- op-service/sources/types.go | 39 +++++++------ op-service/testutils/l1info.go | 5 ++ 6 files changed, 91 insertions(+), 49 deletions(-) diff --git a/op-node/node/api.go b/op-node/node/api.go index ccd4a3b81bb3..66fea4895ede 100644 --- a/op-node/node/api.go +++ b/op-node/node/api.go @@ -135,6 +135,8 @@ func (n *nodeAPI) OutputAtBlock(ctx context.Context, number hexutil.Uint64) (*et return nil, fmt.Errorf("failed to get L2 block ref with sync status: %w", err) } + // OutputV0AtBlock uses the WithdrawalsRoot in the block header as the value for the + // output MessagePasserStorageRoot, if Isthmus hard fork has activated. output, err := n.client.OutputV0AtBlock(ctx, ref.Hash) if err != nil { return nil, fmt.Errorf("failed to get L2 output at block %s: %w", ref, err) diff --git a/op-node/rollup/attributes/engine_consolidate.go b/op-node/rollup/attributes/engine_consolidate.go index 3cd9e8931210..3b99c92df567 100644 --- a/op-node/rollup/attributes/engine_consolidate.go +++ b/op-node/rollup/attributes/engine_consolidate.go @@ -55,7 +55,7 @@ func AttributesMatchBlock(rollupCfg *rollup.Config, attrs *eth.PayloadAttributes if *attrs.GasLimit != block.GasLimit { return fmt.Errorf("gas limit does not match. expected %d. got: %d", *attrs.GasLimit, block.GasLimit) } - if withdrawalErr := checkWithdrawalsMatch(attrs.Withdrawals, block.Withdrawals); withdrawalErr != nil { + if withdrawalErr := checkWithdrawals(rollupCfg, attrs, block); withdrawalErr != nil { return withdrawalErr } if err := checkParentBeaconBlockRootMatch(attrs.ParentBeaconBlockRoot, envelope.ParentBeaconBlockRoot); err != nil { @@ -82,31 +82,44 @@ func checkParentBeaconBlockRootMatch(attrRoot, blockRoot *common.Hash) error { return nil } -func checkWithdrawalsMatch(attrWithdrawals *types.Withdrawals, blockWithdrawals *types.Withdrawals) error { - if attrWithdrawals == nil && blockWithdrawals == nil { - return nil +// checkWithdrawals checks if the withdrawals list and withdrawalsRoot are as expected in the attributes and block, +// based on the active hard fork. +func checkWithdrawals(rollupCfg *rollup.Config, attrs *eth.PayloadAttributes, block *eth.ExecutionPayload) error { + if attrs == nil || block == nil { + return fmt.Errorf("nil attributes or block") } - if attrWithdrawals == nil && blockWithdrawals != nil { - return fmt.Errorf("expected withdrawals in block to be nil, actual %v", *blockWithdrawals) - } - - if attrWithdrawals != nil && blockWithdrawals == nil { - return fmt.Errorf("expected withdrawals in block to be non-nil %v, actual nil", *attrWithdrawals) - } + attrWithdrawals := attrs.Withdrawals + blockWithdrawals := block.Withdrawals - if len(*attrWithdrawals) != len(*blockWithdrawals) { - return fmt.Errorf("expected withdrawals in block to be %d, actual %d", len(*attrWithdrawals), len(*blockWithdrawals)) - } - - for idx, expected := range *attrWithdrawals { - actual := (*blockWithdrawals)[idx] - - if *expected != *actual { - return fmt.Errorf("expected withdrawal %d to be %v, actual %v", idx, expected, actual) + // If we are pre-canyon, attributes should have nil withdrawals list and withdrawalsRoot + if !rollupCfg.IsCanyon(uint64(block.Timestamp)) { + if attrWithdrawals != nil { + return fmt.Errorf("pre-canyon: expected withdrawals in attributes to be nil, actual %v", *attrWithdrawals) + } + } else if rollupCfg.IsIsthmus(uint64(block.Timestamp)) { + // isthmus is active, we should have an empty withdrawals list and non-nil withdrawalsRoot + if !(blockWithdrawals != nil && len(*blockWithdrawals) == 0) { + return fmt.Errorf("isthmus: expected withdrawals in block to be non-nil and empty, actual %v", *blockWithdrawals) + } + if block.WithdrawalsRoot == nil { + return fmt.Errorf("isthmus: expected withdrawalsRoot in block to be non-nil") + } + if !(attrWithdrawals != nil && len(*attrWithdrawals) == 0) { + return fmt.Errorf("isthmus: expected withdrawals in attributes to be non-nil and empty, actual %v", *attrWithdrawals) + } + } else { + // pre-isthmus, post-canyon + if !(blockWithdrawals != nil && len(*blockWithdrawals) == 0) { + return fmt.Errorf("pre-isthmus: expected withdrawals in block to be non-nil and empty, actual %v", *blockWithdrawals) + } + if block.WithdrawalsRoot != nil { + return fmt.Errorf("pre-isthmus: expected withdrawalsRoot in block to be nil, actual %v", *block.WithdrawalsRoot) + } + if !(attrWithdrawals != nil && len(*attrWithdrawals) == 0) { + return fmt.Errorf("pre-isthmus: expected withdrawals in attributes to be non-nil and empty, actual %v", *attrWithdrawals) } } - return nil } diff --git a/op-service/eth/block_info.go b/op-service/eth/block_info.go index 268c6d934b6e..b53f6279f4da 100644 --- a/op-service/eth/block_info.go +++ b/op-service/eth/block_info.go @@ -26,6 +26,7 @@ type BlockInfo interface { GasUsed() uint64 GasLimit() uint64 ParentBeaconRoot() *common.Hash // Dencun extension + WithdrawalsRoot() *common.Hash // Isthmus extension // HeaderRLP returns the RLP of the block header as per consensus rules // Returns an error if the header RLP could not be written @@ -72,6 +73,10 @@ func (b blockInfo) ParentBeaconRoot() *common.Hash { return b.Block.BeaconRoot() } +func (b blockInfo) WithdrawalsRoot() *common.Hash { + return b.Header().WithdrawalsHash +} + func BlockToInfo(b *types.Block) BlockInfo { return blockInfo{b} } @@ -133,6 +138,10 @@ func (h headerBlockInfo) ParentBeaconRoot() *common.Hash { return h.Header.ParentBeaconRoot } +func (h headerBlockInfo) WithdrawalsRoot() *common.Hash { + return h.Header.WithdrawalsHash +} + func (h headerBlockInfo) HeaderRLP() ([]byte, error) { return rlp.EncodeToBytes(h.Header) } diff --git a/op-service/sources/l2_client.go b/op-service/sources/l2_client.go index 078385d40510..0e2e8569aed2 100644 --- a/op-service/sources/l2_client.go +++ b/op-service/sources/l2_client.go @@ -180,21 +180,29 @@ func (s *L2Client) OutputV0AtBlock(ctx context.Context, blockHash common.Hash) ( return nil, ethereum.NotFound } - proof, err := s.GetProof(ctx, predeploys.L2ToL1MessagePasserAddr, []common.Hash{}, blockHash.String()) - if err != nil { - return nil, fmt.Errorf("failed to get contract proof at block %s: %w", blockHash, err) - } - if proof == nil { - return nil, fmt.Errorf("proof %w", ethereum.NotFound) - } - // make sure that the proof (including storage hash) that we retrieved is correct by verifying it against the state-root - if err := proof.Verify(head.Root()); err != nil { - return nil, fmt.Errorf("invalid withdrawal root hash, state root was %s: %w", head.Root(), err) + var messagePasserStorageRoot eth.Bytes32 + if s.rollupCfg.IsIsthmus(head.Time()) { + // If Isthmus hard fork has activated, we can get the messagePasserStorageRoot directly from the header + // instead of having to compute it from the contract storage trie. + messagePasserStorageRoot = eth.Bytes32(*head.WithdrawalsRoot()) + } else { + proof, err := s.GetProof(ctx, predeploys.L2ToL1MessagePasserAddr, []common.Hash{}, blockHash.String()) + if err != nil { + return nil, fmt.Errorf("failed to get contract proof at block %s: %w", blockHash, err) + } + if proof == nil { + return nil, fmt.Errorf("proof %w", ethereum.NotFound) + } + // make sure that the proof (including storage hash) that we retrieved is correct by verifying it against the state-root + if err := proof.Verify(head.Root()); err != nil { + return nil, fmt.Errorf("invalid withdrawal root hash, state root was %s: %w", head.Root(), err) + } + messagePasserStorageRoot = eth.Bytes32(proof.StorageHash) } stateRoot := head.Root() return ð.OutputV0{ StateRoot: eth.Bytes32(stateRoot), - MessagePasserStorageRoot: eth.Bytes32(proof.StorageHash), + MessagePasserStorageRoot: eth.Bytes32(messagePasserStorageRoot), BlockHash: blockHash, }, nil } diff --git a/op-service/sources/types.go b/op-service/sources/types.go index f337beb02058..4fc73010d04b 100644 --- a/op-service/sources/types.go +++ b/op-service/sources/types.go @@ -95,6 +95,10 @@ func (h headerInfo) ParentBeaconRoot() *common.Hash { return h.Header.ParentBeaconRoot } +func (h headerInfo) WithdrawalsRoot() *common.Hash { + return h.Header.WithdrawalsHash +} + func (h headerInfo) HeaderRLP() ([]byte, error) { return rlp.EncodeToBytes(h.Header) } @@ -293,23 +297,24 @@ func (block *RPCBlock) ExecutionPayloadEnvelope(trustCache bool) (*eth.Execution } payload := ð.ExecutionPayload{ - ParentHash: block.ParentHash, - FeeRecipient: block.Coinbase, - StateRoot: eth.Bytes32(block.Root), - ReceiptsRoot: eth.Bytes32(block.ReceiptHash), - LogsBloom: block.Bloom, - PrevRandao: eth.Bytes32(block.MixDigest), // mix-digest field is used for prevRandao post-merge - BlockNumber: block.Number, - GasLimit: block.GasLimit, - GasUsed: block.GasUsed, - Timestamp: block.Time, - ExtraData: eth.BytesMax32(block.Extra), - BaseFeePerGas: eth.Uint256Quantity(baseFee), - BlockHash: block.Hash, - Transactions: opaqueTxs, - Withdrawals: block.Withdrawals, - BlobGasUsed: block.BlobGasUsed, - ExcessBlobGas: block.ExcessBlobGas, + ParentHash: block.ParentHash, + FeeRecipient: block.Coinbase, + StateRoot: eth.Bytes32(block.Root), + ReceiptsRoot: eth.Bytes32(block.ReceiptHash), + LogsBloom: block.Bloom, + PrevRandao: eth.Bytes32(block.MixDigest), // mix-digest field is used for prevRandao post-merge + BlockNumber: block.Number, + GasLimit: block.GasLimit, + GasUsed: block.GasUsed, + Timestamp: block.Time, + ExtraData: eth.BytesMax32(block.Extra), + BaseFeePerGas: eth.Uint256Quantity(baseFee), + BlockHash: block.Hash, + Transactions: opaqueTxs, + Withdrawals: block.Withdrawals, + BlobGasUsed: block.BlobGasUsed, + ExcessBlobGas: block.ExcessBlobGas, + WithdrawalsRoot: block.WithdrawalsRoot, } return ð.ExecutionPayloadEnvelope{ diff --git a/op-service/testutils/l1info.go b/op-service/testutils/l1info.go index 8f04b71fed93..ec5f75e08012 100644 --- a/op-service/testutils/l1info.go +++ b/op-service/testutils/l1info.go @@ -30,6 +30,7 @@ type MockBlockInfo struct { InfoHeaderRLP []byte InfoParentBeaconRoot *common.Hash + InfoWithdrawalsRoot *common.Hash } func (l *MockBlockInfo) Hash() common.Hash { @@ -88,6 +89,10 @@ func (l *MockBlockInfo) ParentBeaconRoot() *common.Hash { return l.InfoParentBeaconRoot } +func (l *MockBlockInfo) WithdrawalsRoot() *common.Hash { + return l.InfoWithdrawalsRoot +} + func (l *MockBlockInfo) HeaderRLP() ([]byte, error) { if l.InfoHeaderRLP == nil { return nil, errors.New("header rlp not available") From 2e3e499f14447505fcbb1b9c563caf9b48877fa5 Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Wed, 30 Oct 2024 23:14:07 -0400 Subject: [PATCH 05/11] Test for attributes check and fixes for a few failures --- op-node/rollup/attributes/attributes_test.go | 4 + .../attributes/engine_consolidate_test.go | 220 +++++++++++++++--- 2 files changed, 187 insertions(+), 37 deletions(-) diff --git a/op-node/rollup/attributes/attributes_test.go b/op-node/rollup/attributes/attributes_test.go index 05fbd21fe784..69880885a8e1 100644 --- a/op-node/rollup/attributes/attributes_test.go +++ b/op-node/rollup/attributes/attributes_test.go @@ -84,6 +84,8 @@ func TestAttributesHandler(t *testing.T) { EcotoneTime: new(uint64), } + emptyWithdrawals := make(types.Withdrawals, 0) + a1L1Info, err := derive.L1InfoDepositBytes(cfg, cfg.Genesis.SystemConfig, 1, aL1Info, refA0.Time+cfg.BlockTime) require.NoError(t, err) parentBeaconBlockRoot := testutils.RandomHash(rng) @@ -102,6 +104,7 @@ func TestAttributesHandler(t *testing.T) { BaseFeePerGas: eth.Uint256Quantity(*uint256.NewInt(7)), BlockHash: common.Hash{}, Transactions: []eth.Data{a1L1Info}, + Withdrawals: &emptyWithdrawals, }, ParentBeaconBlockRoot: &parentBeaconBlockRoot} // fix up the block-hash payloadA1.ExecutionPayload.BlockHash, _ = payloadA1.CheckBlockHash() @@ -139,6 +142,7 @@ func TestAttributesHandler(t *testing.T) { BaseFeePerGas: eth.Uint256Quantity(*uint256.NewInt(7)), BlockHash: common.Hash{}, Transactions: []eth.Data{a1L1Info}, + Withdrawals: &emptyWithdrawals, }, ParentBeaconBlockRoot: &parentBeaconBlockRoot} // fix up the block-hash payloadA1Alt.ExecutionPayload.BlockHash, _ = payloadA1Alt.CheckBlockHash() diff --git a/op-node/rollup/attributes/engine_consolidate_test.go b/op-node/rollup/attributes/engine_consolidate_test.go index 015e34f5d3d4..9baf22bab1ac 100644 --- a/op-node/rollup/attributes/engine_consolidate_test.go +++ b/op-node/rollup/attributes/engine_consolidate_test.go @@ -24,7 +24,7 @@ var ( validParentBeaconRoot = common.HexToHash("0x456") validPrevRandao = eth.Bytes32(common.HexToHash("0x789")) validGasLimit = eth.Uint64Quantity(1000) - validWithdrawals = types.Withdrawals{} + validWithdrawals = make(types.Withdrawals, 0) validFeeRecipient = predeploys.SequencerFeeVaultAddr ) @@ -43,7 +43,7 @@ func ecotoneArgs() args { Timestamp: validTimestamp, PrevRandao: validPrevRandao, GasLimit: validGasLimit, - Withdrawals: &validWithdrawals, + Withdrawals: nil, FeeRecipient: validFeeRecipient, }, }, @@ -52,7 +52,7 @@ func ecotoneArgs() args { PrevRandao: validPrevRandao, GasLimit: &validGasLimit, ParentBeaconBlockRoot: &validParentBeaconRoot, - Withdrawals: &validWithdrawals, + Withdrawals: nil, SuggestedFeeRecipient: validFeeRecipient, }, parentHash: validParentHash, @@ -145,137 +145,283 @@ func createMismatchedFeeRecipient() args { } func TestAttributesMatch(t *testing.T) { + canyonTimeInFuture := uint64(100) + canyonTimeInPast := uint64(0) + isthmusTimeInFuture := uint64(250) + + rollupCfgPreCanyonChecks := &rollup.Config{CanyonTime: &canyonTimeInFuture} + rollupCfgPreIsthmusChecks := &rollup.Config{CanyonTime: &canyonTimeInPast, IsthmusTime: &isthmusTimeInFuture} + rollupCfg := &rollup.Config{} tests := []struct { shouldMatch bool args args + rollupCfg *rollup.Config + desc string }{ { shouldMatch: true, args: ecotoneArgs(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "ecotoneArgs", }, { shouldMatch: true, args: canyonArgs(), + rollupCfg: rollupCfgPreIsthmusChecks, + desc: "canyonArgs", }, { shouldMatch: true, args: bedrockArgs(), + rollupCfg: rollupCfgPreIsthmusChecks, + desc: "bedrockArgs", }, { shouldMatch: false, args: mismatchedParentHashArgs(), + rollupCfg: rollupCfgPreIsthmusChecks, + desc: "mismatchedParentHashArgs", }, { shouldMatch: false, args: ecotoneNoParentBeaconBlockRoot(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "ecotoneNoParentBeaconBlockRoot", }, { shouldMatch: false, args: ecotoneUnexpectedParentBeaconBlockRoot(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "ecotoneUnexpectedParentBeaconBlockRoot", }, { shouldMatch: false, args: ecotoneMismatchParentBeaconBlockRoot(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "ecotoneMismatchParentBeaconBlockRoot", }, { shouldMatch: true, args: ecotoneMismatchParentBeaconBlockRootPtr(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "ecotoneMismatchParentBeaconBlockRootPtr", }, { shouldMatch: true, args: ecotoneNilParentBeaconBlockRoots(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "ecotoneNilParentBeaconBlockRoots", }, { shouldMatch: false, args: createMismatchedPrevRandao(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "createMismatchedPrevRandao", }, { shouldMatch: false, args: createMismatchedGasLimit(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "createMismatchedGasLimit", }, { shouldMatch: false, args: createNilGasLimit(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "createNilGasLimit", }, { shouldMatch: false, args: createMismatchedTimestamp(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "createMismatchedTimestamp", }, { shouldMatch: false, args: createMismatchedFeeRecipient(), + rollupCfg: rollupCfgPreCanyonChecks, + desc: "createMismatchedFeeRecipient", }, } - for i, test := range tests { + for _, test := range tests { err := AttributesMatchBlock(rollupCfg, test.args.attrs, test.args.parentHash, test.args.envelope, testlog.Logger(t, log.LevelInfo)) if test.shouldMatch { - require.NoError(t, err, "fail %d", i) + require.NoError(t, err, "fail: %s", test.desc) } else { - require.Error(t, err, "fail %d", i) + require.Error(t, err, "fail: %s", test.desc) } } } func TestWithdrawalsMatch(t *testing.T) { + canyonTimeInFuture := uint64(100) + canyonTimeInPast := uint64(0) + isthmusTimeInPast := uint64(150) + isthmusTimeInFuture := uint64(250) + + emptyWithdrawals := make(types.Withdrawals, 0) + + rollupCfgPreCanyonChecks := &rollup.Config{CanyonTime: &canyonTimeInFuture} + rollupCfgPreIsthmusChecks := &rollup.Config{CanyonTime: &canyonTimeInPast, IsthmusTime: &isthmusTimeInFuture} + rollupCfgPostIsthmusChecks := &rollup.Config{CanyonTime: &canyonTimeInPast, IsthmusTime: &isthmusTimeInPast} + tests := []struct { - attrs *types.Withdrawals - block *types.Withdrawals - shouldMatch bool + cfg *rollup.Config + attrs *eth.PayloadAttributes + block *eth.ExecutionPayload + checkFails bool + desc string }{ { - attrs: nil, - block: nil, - shouldMatch: true, + cfg: rollupCfgPreCanyonChecks, + attrs: nil, + block: nil, + checkFails: true, + desc: "nil attributes/block", }, { - attrs: &types.Withdrawals{}, - block: nil, - shouldMatch: false, + cfg: rollupCfgPreCanyonChecks, + attrs: ð.PayloadAttributes{Withdrawals: nil}, + block: ð.ExecutionPayload{Timestamp: 0}, + checkFails: false, + desc: "pre-canyon: nil attr withdrawals", }, { - attrs: nil, - block: &types.Withdrawals{}, - shouldMatch: false, + cfg: rollupCfgPreCanyonChecks, + attrs: ð.PayloadAttributes{ + Withdrawals: &types.Withdrawals{ + &types.Withdrawal{ + Index: 1, + }, + }, + }, + block: ð.ExecutionPayload{Timestamp: 0}, + checkFails: true, + desc: "pre-canyon: non-nil withdrawals", }, { - attrs: &types.Withdrawals{}, - block: &types.Withdrawals{}, - shouldMatch: true, + cfg: rollupCfgPostIsthmusChecks, + attrs: ð.PayloadAttributes{}, + block: ð.ExecutionPayload{ + Timestamp: 200, + Withdrawals: &types.Withdrawals{ + &types.Withdrawal{ + Index: 1, + }, + }, + }, + checkFails: true, + desc: "post-isthmus: non-empty block withdrawals list", }, { - attrs: &types.Withdrawals{ - { - Index: 1, + cfg: rollupCfgPostIsthmusChecks, + attrs: ð.PayloadAttributes{}, + block: ð.ExecutionPayload{ + Timestamp: 200, + WithdrawalsRoot: nil, + Withdrawals: &emptyWithdrawals, + }, + checkFails: true, + desc: "post-isthmus: nil block withdrawalsRoot", + }, + { + cfg: rollupCfgPostIsthmusChecks, + attrs: ð.PayloadAttributes{ + Withdrawals: &types.Withdrawals{ + &types.Withdrawal{ + Index: 1, + }, }, }, - block: &types.Withdrawals{}, - shouldMatch: false, + block: ð.ExecutionPayload{ + Timestamp: 200, + WithdrawalsRoot: &common.Hash{}, + Withdrawals: &emptyWithdrawals, + }, + checkFails: true, + desc: "post-isthmus: non-emtpy attr withdrawals list", + }, + { + cfg: rollupCfgPostIsthmusChecks, + attrs: ð.PayloadAttributes{ + Withdrawals: &emptyWithdrawals, + }, + block: ð.ExecutionPayload{ + Timestamp: 200, + WithdrawalsRoot: &common.Hash{}, + Withdrawals: &emptyWithdrawals, + }, + checkFails: false, + desc: "post-isthmus: non-empty block withdrawalsRoot and empty block/attr withdrawals list", }, { - attrs: &types.Withdrawals{ - { - Index: 1, + cfg: rollupCfgPreIsthmusChecks, + attrs: ð.PayloadAttributes{}, + block: ð.ExecutionPayload{ + Timestamp: 200, + Withdrawals: &types.Withdrawals{ + &types.Withdrawal{ + Index: 1, + }, }, }, - block: &types.Withdrawals{ - { - Index: 2, + checkFails: true, + desc: "pre-isthmus: non-emtpy block withdrawals list", + }, + { + cfg: rollupCfgPreIsthmusChecks, + attrs: ð.PayloadAttributes{}, + block: ð.ExecutionPayload{ + Timestamp: 200, + Withdrawals: &types.Withdrawals{}, + WithdrawalsRoot: &common.Hash{}, + }, + checkFails: true, + desc: "pre-isthmus: non-emtpy block withdrawalsRoot", + }, + { + cfg: rollupCfgPreIsthmusChecks, + attrs: ð.PayloadAttributes{ + Withdrawals: &types.Withdrawals{ + &types.Withdrawal{ + Index: 1, + }, }, }, - shouldMatch: false, + block: ð.ExecutionPayload{ + Timestamp: 200, + Withdrawals: &types.Withdrawals{}, + WithdrawalsRoot: nil, + }, + checkFails: true, + desc: "pre-isthmus: non-emtpy attr withdrawals list", + }, + { + cfg: rollupCfgPreIsthmusChecks, + attrs: ð.PayloadAttributes{ + Withdrawals: &emptyWithdrawals, + }, + block: ð.ExecutionPayload{ + Timestamp: 200, + WithdrawalsRoot: nil, + Withdrawals: &emptyWithdrawals, + }, + checkFails: false, + desc: "pre-isthmus: nil block withdrawalsRoot and empty block/attr withdrawals list", }, } for _, test := range tests { - err := checkWithdrawalsMatch(test.attrs, test.block) + t.Log(test.desc) + err := checkWithdrawals(test.cfg, test.attrs, test.block) - if test.shouldMatch { - require.NoError(t, err) + if test.checkFails { + require.Error(t, err, "test: %s", test.desc) } else { - require.Error(t, err) + require.NoError(t, err, "test: %s", test.desc) } } } From 7af03fd342ca5b576a6bf036e86139c2e35c1a9b Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Thu, 31 Oct 2024 15:33:15 -0400 Subject: [PATCH 06/11] op-program: use hdr withdrawalsRoot if isthmus is active when computing L2OutputRoot, no need to re-compute the storage root if Isthmus is active. --- op-program/client/l2/engine.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/op-program/client/l2/engine.go b/op-program/client/l2/engine.go index 872d0eb61ed1..f80f81a2dcd5 100644 --- a/op-program/client/l2/engine.go +++ b/op-program/client/l2/engine.go @@ -41,15 +41,23 @@ func (o *OracleEngine) L2OutputRoot(l2ClaimBlockNum uint64) (eth.Bytes32, error) if outBlock == nil { return eth.Bytes32{}, fmt.Errorf("failed to get L2 block at %d", l2ClaimBlockNum) } - stateDB, err := o.backend.StateAt(outBlock.Root) - if err != nil { - return eth.Bytes32{}, fmt.Errorf("failed to open L2 state db at block %s: %w", outBlock.Hash(), err) - } - withdrawalsTrie, err := stateDB.OpenStorageTrie(predeploys.L2ToL1MessagePasserAddr) - if err != nil { - return eth.Bytes32{}, fmt.Errorf("withdrawals trie unavailable at block %v: %w", outBlock.Hash(), err) - } - return rollup.ComputeL2OutputRootV0(eth.HeaderBlockInfo(outBlock), withdrawalsTrie.Hash()) + var storageRoot [32]byte + // if Isthmus is active, we don't need to compute the storage root, we can use the header + // withdrawalRoot which is the storage root for the L2ToL1MessagePasser contract + if o.rollupCfg.IsIsthmus(outBlock.Time) { + storageRoot = *outBlock.WithdrawalsHash + } else { + stateDB, err := o.backend.StateAt(outBlock.Root) + if err != nil { + return eth.Bytes32{}, fmt.Errorf("failed to open L2 state db at block %s: %w", outBlock.Hash(), err) + } + withdrawalsTrie, err := stateDB.OpenStorageTrie(predeploys.L2ToL1MessagePasserAddr) + if err != nil { + return eth.Bytes32{}, fmt.Errorf("withdrawals trie unavailable at block %v: %w", outBlock.Hash(), err) + } + storageRoot = withdrawalsTrie.Hash() + } + return rollup.ComputeL2OutputRootV0(eth.HeaderBlockInfo(outBlock), storageRoot) } func (o *OracleEngine) GetPayload(ctx context.Context, payloadInfo eth.PayloadInfo) (*eth.ExecutionPayloadEnvelope, error) { From b81182c9089d0668b4be58c2f108424e9ec7f89f Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Thu, 31 Oct 2024 16:07:23 -0400 Subject: [PATCH 07/11] p2p: publish blocks to v4 topic if on isthmus and fix lint errors --- op-node/p2p/gossip.go | 4 +++- op-node/p2p/rpc_server.go | 2 ++ op-node/rollup/attributes/engine_consolidate_test.go | 9 ++++----- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/op-node/p2p/gossip.go b/op-node/p2p/gossip.go index cd75efe74adf..7462596b76ea 100644 --- a/op-node/p2p/gossip.go +++ b/op-node/p2p/gossip.go @@ -578,7 +578,9 @@ func (p *publisher) PublishL2Payload(ctx context.Context, envelope *eth.Executio // This also copies the data, freeing up the original buffer to go back into the pool out := snappy.Encode(nil, data) - if p.cfg.IsEcotone(uint64(envelope.ExecutionPayload.Timestamp)) { + if p.cfg.IsIsthmus(uint64(envelope.ExecutionPayload.Timestamp)) { + return p.blocksV4.topic.Publish(ctx, out) + } else if p.cfg.IsEcotone(uint64(envelope.ExecutionPayload.Timestamp)) { return p.blocksV3.topic.Publish(ctx, out) } else if p.cfg.IsCanyon(uint64(envelope.ExecutionPayload.Timestamp)) { return p.blocksV2.topic.Publish(ctx, out) diff --git a/op-node/p2p/rpc_server.go b/op-node/p2p/rpc_server.go index 1aa587fcc89f..e1c70a177c90 100644 --- a/op-node/p2p/rpc_server.go +++ b/op-node/p2p/rpc_server.go @@ -206,6 +206,7 @@ type PeerStats struct { BlocksTopic uint `json:"blocksTopic"` BlocksTopicV2 uint `json:"blocksTopicV2"` BlocksTopicV3 uint `json:"blocksTopicV3"` + BlocksTopicV4 uint `json:"blocksTopicV4"` Banned uint `json:"banned"` Known uint `json:"known"` } @@ -223,6 +224,7 @@ func (s *APIBackend) PeerStats(_ context.Context) (*PeerStats, error) { BlocksTopic: uint(len(s.node.GossipOut().BlocksTopicV1Peers())), BlocksTopicV2: uint(len(s.node.GossipOut().BlocksTopicV2Peers())), BlocksTopicV3: uint(len(s.node.GossipOut().BlocksTopicV3Peers())), + BlocksTopicV4: uint(len(s.node.GossipOut().BlocksTopicV4Peers())), Banned: 0, Known: uint(len(pstore.Peers())), } diff --git a/op-node/rollup/attributes/engine_consolidate_test.go b/op-node/rollup/attributes/engine_consolidate_test.go index 9baf22bab1ac..da271e58e023 100644 --- a/op-node/rollup/attributes/engine_consolidate_test.go +++ b/op-node/rollup/attributes/engine_consolidate_test.go @@ -24,7 +24,6 @@ var ( validParentBeaconRoot = common.HexToHash("0x456") validPrevRandao = eth.Bytes32(common.HexToHash("0x789")) validGasLimit = eth.Uint64Quantity(1000) - validWithdrawals = make(types.Withdrawals, 0) validFeeRecipient = predeploys.SequencerFeeVaultAddr ) @@ -342,7 +341,7 @@ func TestWithdrawalsMatch(t *testing.T) { Withdrawals: &emptyWithdrawals, }, checkFails: true, - desc: "post-isthmus: non-emtpy attr withdrawals list", + desc: "post-isthmus: non-empty attr withdrawals list", }, { cfg: rollupCfgPostIsthmusChecks, @@ -369,7 +368,7 @@ func TestWithdrawalsMatch(t *testing.T) { }, }, checkFails: true, - desc: "pre-isthmus: non-emtpy block withdrawals list", + desc: "pre-isthmus: non-empty block withdrawals list", }, { cfg: rollupCfgPreIsthmusChecks, @@ -380,7 +379,7 @@ func TestWithdrawalsMatch(t *testing.T) { WithdrawalsRoot: &common.Hash{}, }, checkFails: true, - desc: "pre-isthmus: non-emtpy block withdrawalsRoot", + desc: "pre-isthmus: non-empty block withdrawalsRoot", }, { cfg: rollupCfgPreIsthmusChecks, @@ -397,7 +396,7 @@ func TestWithdrawalsMatch(t *testing.T) { WithdrawalsRoot: nil, }, checkFails: true, - desc: "pre-isthmus: non-emtpy attr withdrawals list", + desc: "pre-isthmus: non-empty attr withdrawals list", }, { cfg: rollupCfgPreIsthmusChecks, From 2ae30ad671d531541df1eb4b5183010637df2a31 Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Fri, 1 Nov 2024 12:45:21 -0400 Subject: [PATCH 08/11] op-service: add a rpc response validation interface * `RPCResponseCheck` is an interface that currently has a function that validates withdrawals. * There's an implementation of this interface for L1 and L2 * L1's response checker validates that the withdrawals list matches the root in the header. * L2's response checker validates that the withdrawals list is empty when the withdrawalsRoot is not nil --- op-chain-ops/cmd/check-derivation/main.go | 3 +- op-conductor/conductor/service.go | 3 +- op-service/sources/eth_client.go | 14 ++++++-- op-service/sources/eth_client_test.go | 3 +- op-service/sources/l1_client.go | 3 +- op-service/sources/l1_rpc_check.go | 39 +++++++++++++++++++++ op-service/sources/l2_client.go | 5 ++- op-service/sources/l2_rpc_check.go | 25 +++++++++++++ op-service/sources/receipts_basic_test.go | 6 ++-- op-service/sources/receipts_caching_test.go | 3 +- op-service/sources/receipts_test.go | 3 +- op-service/sources/types.go | 31 +++++----------- op-service/sources/types_test.go | 6 ++-- 13 files changed, 108 insertions(+), 36 deletions(-) create mode 100644 op-service/sources/l1_rpc_check.go create mode 100644 op-service/sources/l2_rpc_check.go diff --git a/op-chain-ops/cmd/check-derivation/main.go b/op-chain-ops/cmd/check-derivation/main.go index 499b128f8767..4769da9ac57b 100644 --- a/op-chain-ops/cmd/check-derivation/main.go +++ b/op-chain-ops/cmd/check-derivation/main.go @@ -114,7 +114,8 @@ func newClientsFromContext(cliCtx *cli.Context) (*ethclient.Client, *sources.Eth MethodResetDuration: time.Minute, } cl := ethclient.NewClient(clients.L2RpcClient) - ethCl, err := sources.NewEthClient(client.NewBaseRPCClient(clients.L2RpcClient), log.Root(), nil, ðClCfg) + l2RpcChecker := sources.NewL2RPCChecker() + ethCl, err := sources.NewEthClient(client.NewBaseRPCClient(clients.L2RpcClient), log.Root(), nil, ðClCfg, l2RpcChecker) if err != nil { return nil, nil, err } diff --git a/op-conductor/conductor/service.go b/op-conductor/conductor/service.go index 565096a9b45b..60bebe521528 100644 --- a/op-conductor/conductor/service.go +++ b/op-conductor/conductor/service.go @@ -129,8 +129,9 @@ func (c *OpConductor) initSequencerControl(ctx context.Context) error { return errors.Wrap(err, "failed to create geth rpc client") } execCfg := sources.L2ClientDefaultConfig(&c.cfg.RollupCfg, true) + l2RpcChecker := sources.NewL2RPCChecker() // TODO: Add metrics tracer here. tracked by https://github.com/ethereum-optimism/protocol-quest/issues/45 - exec, err := sources.NewEthClient(ec, c.log, nil, &execCfg.EthClientConfig) + exec, err := sources.NewEthClient(ec, c.log, nil, &execCfg.EthClientConfig, l2RpcChecker) if err != nil { return errors.Wrap(err, "failed to create geth client") } diff --git a/op-service/sources/eth_client.go b/op-service/sources/eth_client.go index 39587fd19750..f745ea852b0f 100644 --- a/op-service/sources/eth_client.go +++ b/op-service/sources/eth_client.go @@ -113,11 +113,18 @@ type EthClient struct { // cache payloads by hash // common.Hash -> *eth.ExecutionPayload payloadsCache *caching.LRUCache[common.Hash, *eth.ExecutionPayloadEnvelope] + + // any checks for if RPC response is valid + respChecker RPCRespCheck +} + +type RPCRespCheck interface { + ValidateWithdrawals(withdrawals *types.Withdrawals, withdrawalRoot *common.Hash) error } // NewEthClient returns an [EthClient], wrapping an RPC with bindings to fetch ethereum data with added error logging, // metric tracking, and caching. The [EthClient] uses a [LimitRPC] wrapper to limit the number of concurrent RPC requests. -func NewEthClient(client client.RPC, log log.Logger, metrics caching.Metrics, config *EthClientConfig) (*EthClient, error) { +func NewEthClient(client client.RPC, log log.Logger, metrics caching.Metrics, config *EthClientConfig, checker RPCRespCheck) (*EthClient, error) { if err := config.Check(); err != nil { return nil, fmt.Errorf("bad config, cannot create L1 source: %w", err) } @@ -136,6 +143,7 @@ func NewEthClient(client client.RPC, log log.Logger, metrics caching.Metrics, co transactionsCache: caching.NewLRUCache[common.Hash, types.Transactions](metrics, "txs", config.TransactionsCacheSize), headersCache: caching.NewLRUCache[common.Hash, eth.BlockInfo](metrics, "headers", config.HeadersCacheSize), payloadsCache: caching.NewLRUCache[common.Hash, *eth.ExecutionPayloadEnvelope](metrics, "payloads", config.PayloadsCacheSize), + respChecker: checker, }, nil } @@ -205,7 +213,7 @@ func (s *EthClient) blockCall(ctx context.Context, method string, id rpcBlockID) if block == nil { return nil, nil, ethereum.NotFound } - info, txs, err := block.Info(s.trustRPC, s.mustBePostMerge) + info, txs, err := block.Info(s.trustRPC, s.mustBePostMerge, s.respChecker) if err != nil { return nil, nil, err } @@ -226,7 +234,7 @@ func (s *EthClient) payloadCall(ctx context.Context, method string, id rpcBlockI if block == nil { return nil, ethereum.NotFound } - envelope, err := block.ExecutionPayloadEnvelope(s.trustRPC) + envelope, err := block.ExecutionPayloadEnvelope(s.trustRPC, s.respChecker) if err != nil { return nil, err } diff --git a/op-service/sources/eth_client_test.go b/op-service/sources/eth_client_test.go index fe2d9839f009..55e3d8a4b4f5 100644 --- a/op-service/sources/eth_client_test.go +++ b/op-service/sources/eth_client_test.go @@ -112,7 +112,8 @@ func TestEthClient_InfoByHash(t *testing.T) { "eth_getBlockByHash", []any{rhdr.Hash, false}).Run(func(args mock.Arguments) { *args[1].(**RPCHeader) = rhdr }).Return([]error{nil}) - s, err := NewEthClient(m, nil, nil, testEthClientConfig) + l1RpcChecker := NewL1RPCChecker() + s, err := NewEthClient(m, nil, nil, testEthClientConfig, l1RpcChecker) require.NoError(t, err) info, err := s.InfoByHash(ctx, rhdr.Hash) require.NoError(t, err) diff --git a/op-service/sources/l1_client.go b/op-service/sources/l1_client.go index d67ce1c87abd..f2321ce54353 100644 --- a/op-service/sources/l1_client.go +++ b/op-service/sources/l1_client.go @@ -65,7 +65,8 @@ type L1Client struct { // NewL1Client wraps a RPC with bindings to fetch L1 data, while logging errors, tracking metrics (optional), and caching. func NewL1Client(client client.RPC, log log.Logger, metrics caching.Metrics, config *L1ClientConfig) (*L1Client, error) { - ethClient, err := NewEthClient(client, log, metrics, &config.EthClientConfig) + l1RpcChecker := NewL1RPCChecker() + ethClient, err := NewEthClient(client, log, metrics, &config.EthClientConfig, l1RpcChecker) if err != nil { return nil, err } diff --git a/op-service/sources/l1_rpc_check.go b/op-service/sources/l1_rpc_check.go new file mode 100644 index 000000000000..0812de9d287f --- /dev/null +++ b/op-service/sources/l1_rpc_check.go @@ -0,0 +1,39 @@ +package sources + +import ( + "errors" + "fmt" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/trie" +) + +// Implements the RPCCheck interface for validating RPC responses +type L1RPCChecker struct { +} + +func NewL1RPCChecker() *L1RPCChecker { + return &L1RPCChecker{} +} + +func (c *L1RPCChecker) ValidateWithdrawals(withdrawals *types.Withdrawals, withdrawalsRoot *common.Hash) error { + if withdrawalsRoot != nil { + if withdrawals == nil { + return errors.New("expected withdrawals") + } + for i, w := range *withdrawals { + if w == nil { + return fmt.Errorf("block withdrawal %d is null", i) + } + } + if computed := types.DeriveSha(*withdrawals, trie.NewStackTrie(nil)); *withdrawalsRoot != computed { + return fmt.Errorf("failed to verify withdrawals list: computed %s but RPC said %s", computed, withdrawalsRoot) + } + } else { + if withdrawals != nil { + return fmt.Errorf("expected no withdrawals due to missing withdrawals-root, but got %d", len(*withdrawals)) + } + } + return nil +} diff --git a/op-service/sources/l2_client.go b/op-service/sources/l2_client.go index 0e2e8569aed2..b95822d1e8f9 100644 --- a/op-service/sources/l2_client.go +++ b/op-service/sources/l2_client.go @@ -79,7 +79,10 @@ type L2Client struct { // for fetching and caching eth.L2BlockRef values. This includes fetching an L2BlockRef by block number, label, or hash. // See: [L2BlockRefByLabel], [L2BlockRefByNumber], [L2BlockRefByHash] func NewL2Client(client client.RPC, log log.Logger, metrics caching.Metrics, config *L2ClientConfig) (*L2Client, error) { - ethClient, err := NewEthClient(client, log, metrics, &config.EthClientConfig) + + rpcChecker := NewL2RPCChecker() + + ethClient, err := NewEthClient(client, log, metrics, &config.EthClientConfig, rpcChecker) if err != nil { return nil, err } diff --git a/op-service/sources/l2_rpc_check.go b/op-service/sources/l2_rpc_check.go new file mode 100644 index 000000000000..80426a81c813 --- /dev/null +++ b/op-service/sources/l2_rpc_check.go @@ -0,0 +1,25 @@ +package sources + +import ( + "fmt" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" +) + +// Implements the RPCCheck interface for validating RPC responses +type L2RPCChecker struct { +} + +func NewL2RPCChecker() *L2RPCChecker { + return &L2RPCChecker{} +} + +func (c *L2RPCChecker) ValidateWithdrawals(withdrawals *types.Withdrawals, withdrawalsRoot *common.Hash) error { + if withdrawalsRoot != nil { + if !(withdrawals != nil && len(*withdrawals) == 0) { + return fmt.Errorf("expected empty withdrawals, but got %d", len(*withdrawals)) + } + } + return nil +} diff --git a/op-service/sources/receipts_basic_test.go b/op-service/sources/receipts_basic_test.go index ad1ed16013ad..18a3cda2000b 100644 --- a/op-service/sources/receipts_basic_test.go +++ b/op-service/sources/receipts_basic_test.go @@ -76,7 +76,8 @@ func TestBasicRPCReceiptsFetcher_Reuse(t *testing.T) { return err } - bInfo, _, _ := block.Info(true, true) + l1RpcChecker := NewL1RPCChecker() + bInfo, _, _ := block.Info(true, true, l1RpcChecker) // 1st fetching should result in errors recs, err := rp.FetchReceipts(ctx, bInfo, txHashes) @@ -161,7 +162,8 @@ func runConcurrentFetchingTest(t *testing.T, rp ReceiptsProvider, numFetchers in } fetchResults := make(chan fetchResult, numFetchers) barrier := make(chan struct{}) - bInfo, _, _ := block.Info(true, true) + l1RpcChecker := NewL1RPCChecker() + bInfo, _, _ := block.Info(true, true, l1RpcChecker) ctx, done := context.WithTimeout(context.Background(), 10*time.Second) defer done() for i := 0; i < numFetchers; i++ { diff --git a/op-service/sources/receipts_caching_test.go b/op-service/sources/receipts_caching_test.go index e7891b9e34fd..ce065c1a1412 100644 --- a/op-service/sources/receipts_caching_test.go +++ b/op-service/sources/receipts_caching_test.go @@ -36,7 +36,8 @@ func TestCachingReceiptsProvider_Caching(t *testing.T) { Return(types.Receipts(receipts), error(nil)). Once() // receipts should be cached after first fetch - bInfo, _, _ := block.Info(true, true) + l1RpcChecker := NewL1RPCChecker() + bInfo, _, _ := block.Info(true, true, l1RpcChecker) for i := 0; i < 4; i++ { gotRecs, err := rp.FetchReceipts(ctx, bInfo, txHashes) require.NoError(t, err) diff --git a/op-service/sources/receipts_test.go b/op-service/sources/receipts_test.go index 088a3d9b22cb..e4b1425bc5e0 100644 --- a/op-service/sources/receipts_test.go +++ b/op-service/sources/receipts_test.go @@ -164,7 +164,8 @@ func (tc *ReceiptsTestCase) Run(t *testing.T) { testCfg.MethodResetDuration = 0 } logger := testlog.Logger(t, log.LevelError) - ethCl, err := NewEthClient(client.NewBaseRPCClient(cl), logger, nil, testCfg) + l2RpcChecker := NewL2RPCChecker() + ethCl, err := NewEthClient(client.NewBaseRPCClient(cl), logger, nil, testCfg, l2RpcChecker) require.NoError(t, err) defer ethCl.Close() diff --git a/op-service/sources/types.go b/op-service/sources/types.go index 4fc73010d04b..cdcfbb2f14f8 100644 --- a/op-service/sources/types.go +++ b/op-service/sources/types.go @@ -1,7 +1,6 @@ package sources import ( - "errors" "fmt" "math/big" "strings" @@ -220,7 +219,7 @@ type RPCBlock struct { Withdrawals *types.Withdrawals `json:"withdrawals,omitempty"` } -func (block *RPCBlock) verify() error { +func (block *RPCBlock) verify(checker RPCRespCheck) error { if computed := block.computeBlockHash(); computed != block.Hash { return fmt.Errorf("failed to verify block hash: computed %s but RPC said %s", computed, block.Hash) } @@ -232,34 +231,22 @@ func (block *RPCBlock) verify() error { if computed := types.DeriveSha(types.Transactions(block.Transactions), trie.NewStackTrie(nil)); block.TxHash != computed { return fmt.Errorf("failed to verify transactions list: computed %s but RPC said %s", computed, block.TxHash) } - if block.WithdrawalsRoot != nil { - if block.Withdrawals == nil { - return errors.New("expected withdrawals") - } - for i, w := range *block.Withdrawals { - if w == nil { - return fmt.Errorf("block withdrawal %d is null", i) - } - } - if computed := types.DeriveSha(*block.Withdrawals, trie.NewStackTrie(nil)); *block.WithdrawalsRoot != computed { - return fmt.Errorf("failed to verify withdrawals list: computed %s but RPC said %s", computed, block.WithdrawalsRoot) - } - } else { - if block.Withdrawals != nil { - return fmt.Errorf("expected no withdrawals due to missing withdrawals-root, but got %d", len(*block.Withdrawals)) - } + + if err := checker.ValidateWithdrawals(block.Withdrawals, block.WithdrawalsRoot); err != nil { + return err } + return nil } -func (block *RPCBlock) Info(trustCache bool, mustBePostMerge bool) (eth.BlockInfo, types.Transactions, error) { +func (block *RPCBlock) Info(trustCache bool, mustBePostMerge bool, checker RPCRespCheck) (eth.BlockInfo, types.Transactions, error) { if mustBePostMerge { if err := block.checkPostMerge(); err != nil { return nil, nil, err } } if !trustCache { - if err := block.verify(); err != nil { + if err := block.verify(checker); err != nil { return nil, nil, err } } @@ -273,12 +260,12 @@ func (block *RPCBlock) Info(trustCache bool, mustBePostMerge bool) (eth.BlockInf return info, block.Transactions, nil } -func (block *RPCBlock) ExecutionPayloadEnvelope(trustCache bool) (*eth.ExecutionPayloadEnvelope, error) { +func (block *RPCBlock) ExecutionPayloadEnvelope(trustCache bool, checker RPCRespCheck) (*eth.ExecutionPayloadEnvelope, error) { if err := block.checkPostMerge(); err != nil { return nil, err } if !trustCache { - if err := block.verify(); err != nil { + if err := block.verify(checker); err != nil { return nil, err } } diff --git a/op-service/sources/types_test.go b/op-service/sources/types_test.go index 6d6bad5565bf..68c0c7cf7a66 100644 --- a/op-service/sources/types_test.go +++ b/op-service/sources/types_test.go @@ -70,7 +70,8 @@ func TestBlockJSON(t *testing.T) { var block RPCBlock readJsonTestdata(t, "testdata/data/blocks/"+strings.Replace(entry.Name(), "_metadata.json", "_data.json", 1), &block) - err := block.verify() + l2RpcChecker := NewL1RPCChecker() + err := block.verify(l2RpcChecker) if metadata.Fail { require.NotNil(t, err, "expecting verification error") require.ErrorContains(t, err, metadata.Reason, "validation failed for incorrect reason") @@ -136,7 +137,8 @@ func TestBlockToExecutionPayloadIncludesEcotoneProperties(t *testing.T) { Withdrawals: &types.Withdrawals{}, } - envelope, err := block.ExecutionPayloadEnvelope(false) + l2RpcChecker := NewL1RPCChecker() + envelope, err := block.ExecutionPayloadEnvelope(false, l2RpcChecker) require.NoError(t, err) require.NotNil(t, envelope.ParentBeaconBlockRoot) From 2778c159b9be6364c1b8c653ec9a0baba9b03d39 Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Tue, 5 Nov 2024 10:29:50 -0500 Subject: [PATCH 09/11] SSZ test, Isthmus activation at genesis action test --- op-e2e/actions/helpers/env.go | 2 +- op-e2e/actions/helpers/user_test.go | 5 ++ op-e2e/actions/upgrades/helpers/config.go | 9 +++ op-e2e/actions/upgrades/ishtmus_fork_test.go | 38 ++++++++++ op-e2e/config/init.go | 1 + op-e2e/e2eutils/setup.go | 8 +++ op-service/eth/ssz_test.go | 73 ++++++++++++++++++++ 7 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 op-e2e/actions/upgrades/ishtmus_fork_test.go diff --git a/op-e2e/actions/helpers/env.go b/op-e2e/actions/helpers/env.go index 77911aa80bf5..60d2e79c8062 100644 --- a/op-e2e/actions/helpers/env.go +++ b/op-e2e/actions/helpers/env.go @@ -52,7 +52,7 @@ func WithActiveGenesisFork(fork rollup.ForkName) EnvOpt { // DefaultFork specifies the default fork to use when setting up the action test environment. // Currently manually set to Holocene. // Replace with `var DefaultFork = func() rollup.ForkName { return rollup.AllForks[len(rollup.AllForks)-1] }()` after Interop launch. -const DefaultFork = rollup.Holocene +const DefaultFork = rollup.Isthmus // SetupEnv sets up a default action test environment. If no fork is specified, the default fork as // specified by the package variable [defaultFork] is used. diff --git a/op-e2e/actions/helpers/user_test.go b/op-e2e/actions/helpers/user_test.go index 6ac7e598c593..00b27dea1fe4 100644 --- a/op-e2e/actions/helpers/user_test.go +++ b/op-e2e/actions/helpers/user_test.go @@ -27,6 +27,7 @@ type hardforkScheduledTest struct { fjordTime *hexutil.Uint64 graniteTime *hexutil.Uint64 holoceneTime *hexutil.Uint64 + isthmusTime *hexutil.Uint64 runToFork string allocType config.AllocType } @@ -41,6 +42,8 @@ func (tc *hardforkScheduledTest) GetFork(fork string) *uint64 { func (tc *hardforkScheduledTest) fork(fork string) **hexutil.Uint64 { switch fork { + case "isthmus": + return &tc.isthmusTime case "holocene": return &tc.holoceneTime case "granite": @@ -88,6 +91,7 @@ func testCrossLayerUser(t *testing.T, allocType config.AllocType) { "fjord", "granite", "holocene", + "isthmus", } for i, fork := range forks { i := i @@ -146,6 +150,7 @@ func runCrossLayerUserTest(gt *testing.T, test hardforkScheduledTest) { dp.DeployConfig.L2GenesisFjordTimeOffset = test.fjordTime dp.DeployConfig.L2GenesisGraniteTimeOffset = test.graniteTime dp.DeployConfig.L2GenesisHoloceneTimeOffset = test.holoceneTime + dp.DeployConfig.L2GenesisIsthmusTimeOffset = test.isthmusTime if test.canyonTime != nil { require.Zero(t, uint64(*test.canyonTime)%uint64(dp.DeployConfig.L2BlockTime), "canyon fork must be aligned") diff --git a/op-e2e/actions/upgrades/helpers/config.go b/op-e2e/actions/upgrades/helpers/config.go index c09d0be48b6c..01d11fc1a30a 100644 --- a/op-e2e/actions/upgrades/helpers/config.go +++ b/op-e2e/actions/upgrades/helpers/config.go @@ -43,4 +43,13 @@ func ApplyDeltaTimeOffset(dp *e2eutils.DeployParams, deltaTimeOffset *hexutil.Ui dp.DeployConfig.L2GenesisHoloceneTimeOffset = deltaTimeOffset } } + + // configure Isthmus to not be before Delta accidentally + if dp.DeployConfig.L2GenesisIsthmusTimeOffset != nil { + if deltaTimeOffset == nil { + dp.DeployConfig.L2GenesisIsthmusTimeOffset = nil + } else if *dp.DeployConfig.L2GenesisIsthmusTimeOffset < *deltaTimeOffset { + dp.DeployConfig.L2GenesisIsthmusTimeOffset = deltaTimeOffset + } + } } diff --git a/op-e2e/actions/upgrades/ishtmus_fork_test.go b/op-e2e/actions/upgrades/ishtmus_fork_test.go new file mode 100644 index 000000000000..d8ce722d8e91 --- /dev/null +++ b/op-e2e/actions/upgrades/ishtmus_fork_test.go @@ -0,0 +1,38 @@ +package upgrades + +import ( + "testing" + + "github.com/ethereum-optimism/optimism/op-e2e/actions/helpers" + "github.com/ethereum-optimism/optimism/op-node/rollup" + "github.com/ethereum/go-ethereum/core/types" + "github.com/stretchr/testify/require" +) + +func TestIsthmusActivationAtGenesis(gt *testing.T) { + t := helpers.NewDefaultTesting(gt) + env := helpers.SetupEnv(t, helpers.WithActiveGenesisFork(rollup.Isthmus)) + + // Start op-nodes + env.Seq.ActL2PipelineFull(t) + env.Verifier.ActL2PipelineFull(t) + + // Verify Isthmus is active at genesis + l2Head := env.Seq.L2Unsafe() + require.NotZero(t, l2Head.Hash) + require.True(t, env.SetupData.RollupCfg.IsIsthmus(l2Head.Time), "Isthmus should be active at genesis") + + // build empty L1 block + env.Miner.ActEmptyBlock(t) + + // Build L2 chain and advance safe head + env.Seq.ActL1HeadSignal(t) + env.Seq.ActBuildToL1Head(t) + + block := env.VerifEngine.L2Chain().CurrentBlock() + verifyIsthmusBlock(gt, block) +} + +func verifyIsthmusBlock(gt *testing.T, header *types.Header) { + require.Equal(gt, types.EmptyWithdrawalsHash, *header.WithdrawalsHash) +} diff --git a/op-e2e/config/init.go b/op-e2e/config/init.go index 9419c1277060..93ae683e4c75 100644 --- a/op-e2e/config/init.go +++ b/op-e2e/config/init.go @@ -185,6 +185,7 @@ func initAllocType(root string, allocType AllocType) { } l2Alloc[mode] = allocs } + mustL2Allocs(genesis.L2AllocsIsthmus) mustL2Allocs(genesis.L2AllocsHolocene) mustL2Allocs(genesis.L2AllocsGranite) mustL2Allocs(genesis.L2AllocsFjord) diff --git a/op-e2e/e2eutils/setup.go b/op-e2e/e2eutils/setup.go index dbc9ecea25b9..f35a334dad31 100644 --- a/op-e2e/e2eutils/setup.go +++ b/op-e2e/e2eutils/setup.go @@ -107,6 +107,9 @@ func Ether(v uint64) *big.Int { } func GetL2AllocsMode(dc *genesis.DeployConfig, t uint64) genesis.L2AllocsMode { + if fork := dc.IsthmusTime(t); fork != nil && *fork <= 0 { + return genesis.L2AllocsIsthmus + } if fork := dc.HoloceneTime(t); fork != nil && *fork <= 0 { return genesis.L2AllocsHolocene } @@ -205,6 +208,7 @@ func Setup(t require.TestingT, deployParams *DeployParams, alloc *AllocParams) * FjordTime: deployConf.FjordTime(uint64(deployConf.L1GenesisBlockTimestamp)), GraniteTime: deployConf.GraniteTime(uint64(deployConf.L1GenesisBlockTimestamp)), HoloceneTime: deployConf.HoloceneTime(uint64(deployConf.L1GenesisBlockTimestamp)), + IsthmusTime: deployConf.IsthmusTime(uint64(deployConf.L1GenesisBlockTimestamp)), InteropTime: deployConf.InteropTime(uint64(deployConf.L1GenesisBlockTimestamp)), AltDAConfig: pcfg, } @@ -235,6 +239,7 @@ func SystemConfigFromDeployConfig(deployConfig *genesis.DeployConfig) eth.System } func ApplyDeployConfigForks(deployConfig *genesis.DeployConfig) { + isIsthmus := os.Getenv("OP_E2E_USE_ISTHMUS") == "true" isHolocene := os.Getenv("OP_E2E_USE_HOLOCENE") == "true" isGranite := isHolocene || os.Getenv("OP_E2E_USE_GRANITE") == "true" isFjord := isGranite || os.Getenv("OP_E2E_USE_FJORD") == "true" @@ -255,6 +260,9 @@ func ApplyDeployConfigForks(deployConfig *genesis.DeployConfig) { if isHolocene { deployConfig.L2GenesisHoloceneTimeOffset = new(hexutil.Uint64) } + if isIsthmus { + deployConfig.L2GenesisIsthmusTimeOffset = new(hexutil.Uint64) + } // Canyon and lower is activated by default deployConfig.L2GenesisCanyonTimeOffset = new(hexutil.Uint64) deployConfig.L2GenesisRegolithTimeOffset = new(hexutil.Uint64) diff --git a/op-service/eth/ssz_test.go b/op-service/eth/ssz_test.go index c08c5fa32040..b272aea5f4e4 100644 --- a/op-service/eth/ssz_test.go +++ b/op-service/eth/ssz_test.go @@ -249,6 +249,79 @@ func FuzzExecutionPayloadMarshalUnmarshalV3(f *testing.F) { }) } +func FuzzExecutionPayloadMarshalUnmarshalV4(f *testing.F) { + f.Fuzz(func(t *testing.T, data []byte, a, b, c, d uint64, extraData []byte, txs uint16, txsData []byte, wCount uint16, blobGasUsed, excessBlobGas uint64) { + if len(data) < 32+20+32+32+256+32+32+32+32 { + return + } + var payload ExecutionPayload + payload.ParentHash = *(*common.Hash)(data[:32]) + data = data[32:] + payload.FeeRecipient = *(*common.Address)(data[:20]) + data = data[20:] + payload.StateRoot = *(*Bytes32)(data[:32]) + data = data[32:] + payload.ReceiptsRoot = *(*Bytes32)(data[:32]) + data = data[32:] + payload.LogsBloom = *(*Bytes256)(data[:256]) + data = data[256:] + payload.PrevRandao = *(*Bytes32)(data[:32]) + data = data[32:] + payload.BlockNumber = Uint64Quantity(a) + payload.GasLimit = Uint64Quantity(a) + payload.GasUsed = Uint64Quantity(a) + payload.Timestamp = Uint64Quantity(a) + payload.BlobGasUsed = (*Uint64Quantity)(&blobGasUsed) + payload.ExcessBlobGas = (*Uint64Quantity)(&excessBlobGas) + payload.WithdrawalsRoot = (*common.Hash)(data[:32]) + if len(extraData) > 32 { + extraData = extraData[:32] + } + payload.ExtraData = extraData + (*uint256.Int)(&payload.BaseFeePerGas).SetBytes(data[:32]) + payload.BlockHash = *(*common.Hash)(data[:32]) + payload.Transactions = make([]Data, txs) + for i := 0; i < int(txs); i++ { + if len(txsData) < 2 { + payload.Transactions[i] = make(Data, 0) + continue + } + txSize := binary.LittleEndian.Uint16(txsData[:2]) + txsData = txsData[2:] + if int(txSize) > len(txsData) { + txSize = uint16(len(txsData)) + } + payload.Transactions[i] = txsData[:txSize] + txsData = txsData[txSize:] + } + + wCount = wCount % maxWithdrawalsPerPayload + withdrawals := make(types.Withdrawals, wCount) + for i := 0; i < int(wCount); i++ { + withdrawals[i] = &types.Withdrawal{ + Index: a, + Validator: b, + Address: common.BytesToAddress(data[:20]), + Amount: c, + } + } + payload.Withdrawals = &withdrawals + + var buf bytes.Buffer + if _, err := payload.MarshalSSZ(&buf); err != nil { + t.Fatalf("failed to marshal ExecutionPayload: %v", err) + } + var roundTripped ExecutionPayload + err := roundTripped.UnmarshalSSZ(BlockV4, uint32(len(buf.Bytes())), bytes.NewReader(buf.Bytes())) + if err != nil { + t.Fatalf("failed to decode previously marshalled payload: %v", err) + } + if diff := cmp.Diff(payload, roundTripped); diff != "" { + t.Fatalf("The data did not round trip correctly:\n%s", diff) + } + }) +} + func FuzzOBP01(f *testing.F) { payload := &ExecutionPayload{ ExtraData: make([]byte, 32), From cef3e0eb6fd7f001decb636f0671002003855854 Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Wed, 6 Nov 2024 09:43:47 -0500 Subject: [PATCH 10/11] Action test before hard fork --- op-e2e/actions/upgrades/ishtmus_fork_test.go | 38 ---------- op-e2e/actions/upgrades/isthmus_fork_test.go | 78 ++++++++++++++++++++ 2 files changed, 78 insertions(+), 38 deletions(-) delete mode 100644 op-e2e/actions/upgrades/ishtmus_fork_test.go create mode 100644 op-e2e/actions/upgrades/isthmus_fork_test.go diff --git a/op-e2e/actions/upgrades/ishtmus_fork_test.go b/op-e2e/actions/upgrades/ishtmus_fork_test.go deleted file mode 100644 index d8ce722d8e91..000000000000 --- a/op-e2e/actions/upgrades/ishtmus_fork_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package upgrades - -import ( - "testing" - - "github.com/ethereum-optimism/optimism/op-e2e/actions/helpers" - "github.com/ethereum-optimism/optimism/op-node/rollup" - "github.com/ethereum/go-ethereum/core/types" - "github.com/stretchr/testify/require" -) - -func TestIsthmusActivationAtGenesis(gt *testing.T) { - t := helpers.NewDefaultTesting(gt) - env := helpers.SetupEnv(t, helpers.WithActiveGenesisFork(rollup.Isthmus)) - - // Start op-nodes - env.Seq.ActL2PipelineFull(t) - env.Verifier.ActL2PipelineFull(t) - - // Verify Isthmus is active at genesis - l2Head := env.Seq.L2Unsafe() - require.NotZero(t, l2Head.Hash) - require.True(t, env.SetupData.RollupCfg.IsIsthmus(l2Head.Time), "Isthmus should be active at genesis") - - // build empty L1 block - env.Miner.ActEmptyBlock(t) - - // Build L2 chain and advance safe head - env.Seq.ActL1HeadSignal(t) - env.Seq.ActBuildToL1Head(t) - - block := env.VerifEngine.L2Chain().CurrentBlock() - verifyIsthmusBlock(gt, block) -} - -func verifyIsthmusBlock(gt *testing.T, header *types.Header) { - require.Equal(gt, types.EmptyWithdrawalsHash, *header.WithdrawalsHash) -} diff --git a/op-e2e/actions/upgrades/isthmus_fork_test.go b/op-e2e/actions/upgrades/isthmus_fork_test.go new file mode 100644 index 000000000000..9bd438246af9 --- /dev/null +++ b/op-e2e/actions/upgrades/isthmus_fork_test.go @@ -0,0 +1,78 @@ +package upgrades + +import ( + "testing" + + "github.com/ethereum-optimism/optimism/op-e2e/actions/helpers" + "github.com/ethereum-optimism/optimism/op-e2e/e2eutils" + "github.com/ethereum-optimism/optimism/op-node/rollup" + "github.com/ethereum-optimism/optimism/op-service/testlog" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/log" + "github.com/stretchr/testify/require" +) + +func TestIsthmusActivationAtGenesis(gt *testing.T) { + t := helpers.NewDefaultTesting(gt) + env := helpers.SetupEnv(t, helpers.WithActiveGenesisFork(rollup.Isthmus)) + + // Start op-nodes + env.Seq.ActL2PipelineFull(t) + env.Verifier.ActL2PipelineFull(t) + + // Verify Isthmus is active at genesis + l2Head := env.Seq.L2Unsafe() + require.NotZero(t, l2Head.Hash) + require.True(t, env.SetupData.RollupCfg.IsIsthmus(l2Head.Time), "Isthmus should be active at genesis") + + // build empty L1 block + env.Miner.ActEmptyBlock(t) + + // Build L2 chain and advance safe head + env.Seq.ActL1HeadSignal(t) + env.Seq.ActBuildToL1Head(t) + + block := env.VerifEngine.L2Chain().CurrentBlock() + verifyIsthmusBlock(gt, block) +} + +func TestWithdrawlsRootPreCanyon(gt *testing.T) { + t := helpers.NewDefaultTesting(gt) + dp := e2eutils.MakeDeployParams(t, helpers.DefaultRollupTestParams()) + genesisBlock := hexutil.Uint64(0) + canyonOffset := hexutil.Uint64(2) + + log := testlog.Logger(t, log.LvlDebug) + + dp.DeployConfig.L1CancunTimeOffset = &canyonOffset + + // Activate pre-canyon forks at genesis, and schedule Canyon the block after + dp.DeployConfig.L2GenesisRegolithTimeOffset = &genesisBlock + dp.DeployConfig.L2GenesisCanyonTimeOffset = &canyonOffset + dp.DeployConfig.L2GenesisDeltaTimeOffset = nil + dp.DeployConfig.L2GenesisEcotoneTimeOffset = nil + dp.DeployConfig.L2GenesisFjordTimeOffset = nil + dp.DeployConfig.L2GenesisGraniteTimeOffset = nil + dp.DeployConfig.L2GenesisHoloceneTimeOffset = nil + dp.DeployConfig.L2GenesisIsthmusTimeOffset = nil + require.NoError(t, dp.DeployConfig.Check(log), "must have valid config") + + sd := e2eutils.Setup(t, dp, helpers.DefaultAlloc) + _, _, _, sequencer, engine, verifier, _, _ := helpers.SetupReorgTestActors(t, dp, sd, log) + // ethCl := engine.EthClient() + + // start op-nodes + sequencer.ActL2PipelineFull(t) + verifier.ActL2PipelineFull(t) + + verifyPreIsthmusBlock(gt, engine.L2Chain().CurrentBlock()) +} + +func verifyPreIsthmusBlock(gt *testing.T, header *types.Header) { + require.Nil(gt, header.WithdrawalsHash) +} + +func verifyIsthmusBlock(gt *testing.T, header *types.Header) { + require.Equal(gt, types.EmptyWithdrawalsHash, *header.WithdrawalsHash) +} From fc6b673c59d1e0ec79e8c423f25b1f72e873c4f2 Mon Sep 17 00:00:00 2001 From: Vinod Damle Date: Wed, 6 Nov 2024 10:33:40 -0500 Subject: [PATCH 11/11] revert op-program change will move to a separate PR --- op-program/client/l2/engine.go | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/op-program/client/l2/engine.go b/op-program/client/l2/engine.go index f80f81a2dcd5..872d0eb61ed1 100644 --- a/op-program/client/l2/engine.go +++ b/op-program/client/l2/engine.go @@ -41,23 +41,15 @@ func (o *OracleEngine) L2OutputRoot(l2ClaimBlockNum uint64) (eth.Bytes32, error) if outBlock == nil { return eth.Bytes32{}, fmt.Errorf("failed to get L2 block at %d", l2ClaimBlockNum) } - var storageRoot [32]byte - // if Isthmus is active, we don't need to compute the storage root, we can use the header - // withdrawalRoot which is the storage root for the L2ToL1MessagePasser contract - if o.rollupCfg.IsIsthmus(outBlock.Time) { - storageRoot = *outBlock.WithdrawalsHash - } else { - stateDB, err := o.backend.StateAt(outBlock.Root) - if err != nil { - return eth.Bytes32{}, fmt.Errorf("failed to open L2 state db at block %s: %w", outBlock.Hash(), err) - } - withdrawalsTrie, err := stateDB.OpenStorageTrie(predeploys.L2ToL1MessagePasserAddr) - if err != nil { - return eth.Bytes32{}, fmt.Errorf("withdrawals trie unavailable at block %v: %w", outBlock.Hash(), err) - } - storageRoot = withdrawalsTrie.Hash() - } - return rollup.ComputeL2OutputRootV0(eth.HeaderBlockInfo(outBlock), storageRoot) + stateDB, err := o.backend.StateAt(outBlock.Root) + if err != nil { + return eth.Bytes32{}, fmt.Errorf("failed to open L2 state db at block %s: %w", outBlock.Hash(), err) + } + withdrawalsTrie, err := stateDB.OpenStorageTrie(predeploys.L2ToL1MessagePasserAddr) + if err != nil { + return eth.Bytes32{}, fmt.Errorf("withdrawals trie unavailable at block %v: %w", outBlock.Hash(), err) + } + return rollup.ComputeL2OutputRootV0(eth.HeaderBlockInfo(outBlock), withdrawalsTrie.Hash()) } func (o *OracleEngine) GetPayload(ctx context.Context, payloadInfo eth.PayloadInfo) (*eth.ExecutionPayloadEnvelope, error) {