From 3d3e767ed576f0cc3f43763b34f08e21211c2fe2 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Mon, 5 Aug 2024 20:03:01 -0300 Subject: [PATCH 01/28] feat: ban-deposits-interop first RFC draft --- op-node/rollup/derive/attributes.go | 17 +++- op-node/rollup/derive/l1_block_info.go | 80 ++++++++++++++++++- .../contracts-bedrock/src/L2/CrossL2Inbox.sol | 6 ++ packages/contracts-bedrock/src/L2/L1Block.sol | 23 +++++- 4 files changed, 119 insertions(+), 7 deletions(-) diff --git a/op-node/rollup/derive/attributes.go b/op-node/rollup/derive/attributes.go index c65198bcec73..7584ec5abba2 100644 --- a/op-node/rollup/derive/attributes.go +++ b/op-node/rollup/derive/attributes.go @@ -116,14 +116,27 @@ func (ba *FetchingAttributesBuilder) PreparePayloadAttributes(ctx context.Contex upgradeTxs = append(upgradeTxs, fjord...) } - l1InfoTx, err := L1InfoDepositBytes(ba.rollupCfg, sysConfig, seqNumber, l1Info, nextL2Time) + var hasDeposits = len(depositTxs) > 0 + l1InfoTx, err := L1InfoDepositBytes(ba.rollupCfg, sysConfig, seqNumber, l1Info, nextL2Time, hasDeposits) if err != nil { return nil, NewCriticalError(fmt.Errorf("failed to create l1InfoTx: %w", err)) } - txs := make([]hexutil.Bytes, 0, 1+len(depositTxs)+len(upgradeTxs)) + var txsLen = 1 + len(depositTxs) + len(upgradeTxs) + if hasDeposits { + txsLen += 1 // for the isDeposit resetting tx + } + txs := make([]hexutil.Bytes, 0, txsLen) txs = append(txs, l1InfoTx) txs = append(txs, depositTxs...) + if hasDeposits { + depositsCompleteTx, err := DepositsCompleteBytes(ba.rollupCfg, sysConfig, seqNumber, l1Info, nextL2Time) + if err != nil { + return nil, NewCriticalError(fmt.Errorf("failed to create depositsCompleteTx: %w", err)) + } + // after all the deposits have been processed, we need to include the isDeposit resetting tx + txs = append(txs, depositsCompleteTx) + } txs = append(txs, upgradeTxs...) var withdrawals *types.Withdrawals diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index 26f3f6711f55..a169e43e5d71 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -20,14 +20,19 @@ import ( const ( L1InfoFuncBedrockSignature = "setL1BlockValues(uint64,uint64,uint256,bytes32,uint64,bytes32,uint256,uint256)" L1InfoFuncEcotoneSignature = "setL1BlockValuesEcotone()" + DepositsCompleteSignature = "depositsComplete()" L1InfoArguments = 8 L1InfoBedrockLen = 4 + 32*L1InfoArguments L1InfoEcotoneLen = 4 + 32*5 // after Ecotone upgrade, args are packed into 5 32-byte slots + // TODO ^ we need to add space for the isDeposit flag here + // or should we create a new Len var for this? + DepositsCompleteLen = 4 // only the selector ) var ( L1InfoFuncBedrockBytes4 = crypto.Keccak256([]byte(L1InfoFuncBedrockSignature))[:4] L1InfoFuncEcotoneBytes4 = crypto.Keccak256([]byte(L1InfoFuncEcotoneSignature))[:4] + DepositsCompleteBytes4 = crypto.Keccak256([]byte(DepositsCompleteSignature))[:4] L1InfoDepositerAddress = common.HexToAddress("0xdeaddeaddeaddeaddeaddeaddeaddeaddead0001") L1BlockAddress = predeploys.L1BlockAddr ErrInvalidFormat = errors.New("invalid ecotone l1 block info format") @@ -55,6 +60,8 @@ type L1BlockInfo struct { BlobBaseFee *big.Int // added by Ecotone upgrade BaseFeeScalar uint32 // added by Ecotone upgrade BlobBaseFeeScalar uint32 // added by Ecotone upgrade + + IsDeposit bool // added by Interop upgrade } // Bedrock Binary Format @@ -160,6 +167,8 @@ func (info *L1BlockInfo) unmarshalBinaryBedrock(data []byte) error { // | 32 | BatcherHash | // +---------+--------------------------+ +// TODO: should we duplicate this function? +// or can we extend it somehow to only add the isDeposit flag at the end func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen)) if err := solabi.WriteSignature(w, L1InfoFuncEcotoneBytes4); err != nil { @@ -197,6 +206,7 @@ func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { if err := solabi.WriteAddress(w, info.BatcherAddr); err != nil { return nil, err } + // TODO: add isDeposit flag here return w.Bytes(), nil } @@ -238,6 +248,7 @@ func (info *L1BlockInfo) unmarshalBinaryEcotone(data []byte) error { if info.BatcherAddr, err = solabi.ReadAddress(r); err != nil { return err } + // TODO unmarshall isDeposit flag here if !solabi.EmptyReader(r) { return errors.New("too many bytes") } @@ -259,9 +270,32 @@ func L1BlockInfoFromBytes(rollupCfg *rollup.Config, l2BlockTime uint64, data []b return &info, info.unmarshalBinaryBedrock(data) } +func (info *L1BlockInfo) marshalDepositsComplete() ([]byte, error) { + w := bytes.NewBuffer(make([]byte, 0, DepositsCompleteLen)) + if err := solabi.WriteSignature(w, []byte(DepositsCompleteBytes4)); err != nil { + return nil, err + } + return w.Bytes(), nil +} + +func (info *L1BlockInfo) unmarshalDepositsComplete(data []byte) error { + if len(data) != DepositsCompleteLen { + return fmt.Errorf("data is unexpected length: %d", len(data)) + } + r := bytes.NewReader(data) + + if _, err := solabi.ReadAndValidateSignature(r, DepositsCompleteBytes4); err != nil { + return err + } + if !solabi.EmptyReader(r) { + return errors.New("too many bytes") + } + return nil +} + // L1InfoDeposit creates a L1 Info deposit transaction based on the L1 block, // and the L2 block-height difference with the start of the epoch. -func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64) (*types.DepositTx, error) { +func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64, isDeposit bool) (*types.DepositTx, error) { l1BlockInfo := L1BlockInfo{ Number: block.NumberU64(), Time: block.Time(), @@ -269,6 +303,7 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber BlockHash: block.Hash(), SequenceNumber: seqNumber, BatcherAddr: sysCfg.BatcherAddr, + IsDeposit: isDeposit, } var data []byte if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { @@ -323,8 +358,47 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber } // L1InfoDepositBytes returns a serialized L1-info attributes transaction. -func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, l1Info eth.BlockInfo, l2BlockTime uint64) ([]byte, error) { - dep, err := L1InfoDeposit(rollupCfg, sysCfg, seqNumber, l1Info, l2BlockTime) +func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, l1Info eth.BlockInfo, l2BlockTime uint64, isDeposit bool) ([]byte, error) { + dep, err := L1InfoDeposit(rollupCfg, sysCfg, seqNumber, l1Info, l2BlockTime, isDeposit) + if err != nil { + return nil, fmt.Errorf("failed to create L1 info tx: %w", err) + } + l1Tx := types.NewTx(dep) + opaqueL1Tx, err := l1Tx.MarshalBinary() + if err != nil { + return nil, fmt.Errorf("failed to encode L1 info tx: %w", err) + } + return opaqueL1Tx, nil +} + +func DepositsCompleteDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64) (*types.DepositTx, error) { + var data []byte + source := L1InfoDepositSource{ + L1BlockHash: block.Hash(), + SeqNumber: seqNumber, + } + // Set a normal gas limit with `IsSystemTransaction` to ensure + // that the DepositCompleted Transaction does not run out of gas. + out := &types.DepositTx{ + SourceHash: source.SourceHash(), + From: L1InfoDepositerAddress, + To: &L1BlockAddress, + Mint: nil, + Value: big.NewInt(0), + Gas: 50_000, + IsSystemTransaction: true, + Data: data, + } + // With the regolith fork we disable the IsSystemTx functionality, and allocate real gas + if rollupCfg.IsRegolith(l2BlockTime) { + out.IsSystemTransaction = false + out.Gas = RegolithSystemTxGas + } + return out, nil +} + +func DepositsCompleteBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, l1Info eth.BlockInfo, l2BlockTime uint64) ([]byte, error) { + dep, err := DepositsCompleteDeposit(rollupCfg, sysCfg, seqNumber, l1Info, l2BlockTime) if err != nil { return nil, fmt.Errorf("failed to create L1 info tx: %w", err) } diff --git a/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol b/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol index 970cb80d97a4..b85d09580ad7 100644 --- a/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol +++ b/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol @@ -29,6 +29,9 @@ error InvalidChainId(); /// @notice Thrown when trying to execute a cross chain message and the target call fails. error TargetCallFailed(); +/// @notice Thrown when trying to execute a cross chain message on a deposit transaction. +error NoExecutingDeposits(); + /// @custom:proxied /// @custom:predeploy 0x4200000000000000000000000000000000000022 /// @title CrossL2Inbox @@ -114,6 +117,9 @@ contract CrossL2Inbox is ICrossL2Inbox, ISemver, TransientReentrancyAware { payable reentrantAware { + // We need to know if this is being called on a depositTx + if (L1_BLOCK.isDeposit()) revert NoExecutingDeposits(); + if (_id.timestamp > block.timestamp) revert InvalidTimestamp(); if (!IDependencySet(Predeploys.L1_BLOCK_ATTRIBUTES).isInDependencySet(_id.chainId)) { revert InvalidChainId(); diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index 8cfbb391f0f5..59ac0de1ea60 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -57,6 +57,9 @@ contract L1Block is ISemver, IGasToken { /// @notice The latest L1 blob base fee. uint256 public blobBaseFee; + /// @notice The isDeposit flag. + bool public isDeposit; + /// @custom:semver 1.4.1-beta.1 function version() public pure virtual returns (string memory) { return "1.4.1-beta.1"; @@ -87,6 +90,11 @@ contract L1Block is ISemver, IGasToken { return token != Constants.ETHER; } + function isDeposit() returns (bool _isDeposit) external view { + require(msg.sender == CROSS_L2_INBOX, "L1Block: only the CrossL2Inbox can check if it is a deposit"); + returns isDeposit; + } + /// @custom:legacy /// @notice Updates the L1 block values. /// @param _number L1 blocknumber. @@ -97,6 +105,7 @@ contract L1Block is ISemver, IGasToken { /// @param _batcherHash Versioned hash to authenticate batcher by. /// @param _l1FeeOverhead L1 fee overhead. /// @param _l1FeeScalar L1 fee scalar. + /// @param _isDeposit isDeposit flag function setL1BlockValues( uint64 _number, uint64 _timestamp, @@ -105,7 +114,8 @@ contract L1Block is ISemver, IGasToken { uint64 _sequenceNumber, bytes32 _batcherHash, uint256 _l1FeeOverhead, - uint256 _l1FeeScalar + uint256 _l1FeeScalar, + bool _isDeposit ) external { @@ -119,6 +129,7 @@ contract L1Block is ISemver, IGasToken { batcherHash = _batcherHash; l1FeeOverhead = _l1FeeOverhead; l1FeeScalar = _l1FeeScalar; + isDeposit = _isDeposit; } /// @notice Updates the L1 block values for an Ecotone upgraded chain. @@ -133,7 +144,8 @@ contract L1Block is ISemver, IGasToken { /// 7. _blobBaseFee L1 blob base fee. /// 8. _hash L1 blockhash. /// 9. _batcherHash Versioned hash to authenticate batcher by. - function setL1BlockValuesEcotone() external { + /// 10. _isDeposit isDeposit flag + function setL1BlockValuesInterop() external { address depositor = DEPOSITOR_ACCOUNT(); assembly { // Revert if the caller is not the depositor account. @@ -149,6 +161,7 @@ contract L1Block is ISemver, IGasToken { sstore(blobBaseFee.slot, calldataload(68)) // uint256 sstore(hash.slot, calldataload(100)) // bytes32 sstore(batcherHash.slot, calldataload(132)) // bytes32 + sstore(isDeposit.slot, calldataload(133)) // boolean } } @@ -162,4 +175,10 @@ contract L1Block is ISemver, IGasToken { emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol }); } + + /// @notice Resets the isDeposit flag. + function depositsComplete() external { + if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); + isDeposit = false; + } } From 0d78e5a6a433e30d350cdaadbc018d4b803fefaf Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Mon, 5 Aug 2024 20:29:34 -0300 Subject: [PATCH 02/28] fix: missing marshalling of tx --- op-node/rollup/derive/l1_block_info.go | 29 ++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index a169e43e5d71..1cc3233fc213 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -293,6 +293,11 @@ func (info *L1BlockInfo) unmarshalDepositsComplete(data []byte) error { return nil } +func DepositCompletedFromBytes(rollupCfg *rollup.Config, l2BlockTime uint64, data []byte) (*L1BlockInfo, error) { + var info L1BlockInfo + return &info, info.unmarshalDepositsComplete(data) +} + // L1InfoDeposit creates a L1 Info deposit transaction based on the L1 block, // and the L2 block-height difference with the start of the epoch. func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64, isDeposit bool) (*types.DepositTx, error) { @@ -372,7 +377,19 @@ func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNu } func DepositsCompleteDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64) (*types.DepositTx, error) { - var data []byte + l1BlockInfo := L1BlockInfo{ + Number: block.NumberU64(), + Time: block.Time(), + BaseFee: block.BaseFee(), + BlockHash: block.Hash(), + SequenceNumber: seqNumber, + BatcherAddr: sysCfg.BatcherAddr, + IsDeposit: false, + } + data, err := l1BlockInfo.marshalDepositsComplete() + if err != nil { + return nil, fmt.Errorf("failed to marshal Bedrock l1 block info: %w", err) + } source := L1InfoDepositSource{ L1BlockHash: block.Hash(), SeqNumber: seqNumber, @@ -400,12 +417,12 @@ func DepositsCompleteDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, func DepositsCompleteBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, l1Info eth.BlockInfo, l2BlockTime uint64) ([]byte, error) { dep, err := DepositsCompleteDeposit(rollupCfg, sysCfg, seqNumber, l1Info, l2BlockTime) if err != nil { - return nil, fmt.Errorf("failed to create L1 info tx: %w", err) + return nil, fmt.Errorf("failed to create DepositsComplete tx: %w", err) } - l1Tx := types.NewTx(dep) - opaqueL1Tx, err := l1Tx.MarshalBinary() + depositsCompleteTx := types.NewTx(dep) + opaqueDepositsCompleteTx, err := depositsCompleteTx.MarshalBinary() if err != nil { - return nil, fmt.Errorf("failed to encode L1 info tx: %w", err) + return nil, fmt.Errorf("failed to encode DepositsComplete tx: %w", err) } - return opaqueL1Tx, nil + return opaqueDepositsCompleteTx, nil } From 37f17de03ea29b7adcdff6006e4e270bc25d61d1 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Tue, 6 Aug 2024 12:10:30 -0300 Subject: [PATCH 03/28] feat: code cleanup and renaming --- op-node/rollup/derive/attributes.go | 18 ++++++++---------- op-node/rollup/types.go | 1 + packages/contracts-bedrock/src/L2/L1Block.sol | 6 ++++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/op-node/rollup/derive/attributes.go b/op-node/rollup/derive/attributes.go index 7584ec5abba2..90b9aec6f6c5 100644 --- a/op-node/rollup/derive/attributes.go +++ b/op-node/rollup/derive/attributes.go @@ -122,21 +122,19 @@ func (ba *FetchingAttributesBuilder) PreparePayloadAttributes(ctx context.Contex return nil, NewCriticalError(fmt.Errorf("failed to create l1InfoTx: %w", err)) } - var txsLen = 1 + len(depositTxs) + len(upgradeTxs) - if hasDeposits { - txsLen += 1 // for the isDeposit resetting tx - } - txs := make([]hexutil.Bytes, 0, txsLen) - txs = append(txs, l1InfoTx) - txs = append(txs, depositTxs...) - if hasDeposits { + var postDeposits []hexutil.Bytes + if ba.rollupCfg.IsInteropActivationBlock(nextL2Time) && hasDeposits { depositsCompleteTx, err := DepositsCompleteBytes(ba.rollupCfg, sysConfig, seqNumber, l1Info, nextL2Time) if err != nil { return nil, NewCriticalError(fmt.Errorf("failed to create depositsCompleteTx: %w", err)) } - // after all the deposits have been processed, we need to include the isDeposit resetting tx - txs = append(txs, depositsCompleteTx) + postDeposits = append(postDeposits, depositsCompleteTx) } + + txs := make([]hexutil.Bytes, 0, 1+len(depositTxs)+len(postDeposits)+len(upgradeTxs)) + txs = append(txs, l1InfoTx) + txs = append(txs, depositTxs...) + txs = append(txs, postDeposits...) txs = append(txs, upgradeTxs...) var withdrawals *types.Withdrawals diff --git a/op-node/rollup/types.go b/op-node/rollup/types.go index 892bcfa86bc4..c4287f728d39 100644 --- a/op-node/rollup/types.go +++ b/op-node/rollup/types.go @@ -489,6 +489,7 @@ func (c *Config) IsHoloceneActivationBlock(l2BlockTime uint64) bool { !c.IsHolocene(l2BlockTime-c.BlockTime) } +// TODO rename to IsIsthmusActivationBlock (this will require quite a bit of renaming) func (c *Config) IsInteropActivationBlock(l2BlockTime uint64) bool { return c.IsInterop(l2BlockTime) && l2BlockTime >= c.BlockTime && diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index 59ac0de1ea60..827c44df08b2 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -92,7 +92,7 @@ contract L1Block is ISemver, IGasToken { function isDeposit() returns (bool _isDeposit) external view { require(msg.sender == CROSS_L2_INBOX, "L1Block: only the CrossL2Inbox can check if it is a deposit"); - returns isDeposit; + _isDeposit = isDeposit; } /// @custom:legacy @@ -145,7 +145,7 @@ contract L1Block is ISemver, IGasToken { /// 8. _hash L1 blockhash. /// 9. _batcherHash Versioned hash to authenticate batcher by. /// 10. _isDeposit isDeposit flag - function setL1BlockValuesInterop() external { + function setL1BlockValuesIsthmus() external { address depositor = DEPOSITOR_ACCOUNT(); assembly { // Revert if the caller is not the depositor account. @@ -162,6 +162,8 @@ contract L1Block is ISemver, IGasToken { sstore(hash.slot, calldataload(100)) // bytes32 sstore(batcherHash.slot, calldataload(132)) // bytes32 sstore(isDeposit.slot, calldataload(133)) // boolean + // TODO ^ not sure we can just add a boolean as the last slot + // (could not find how to client-side test this) } } From 4f9621ae0ce56edb6741eaa08c9e5c7988e3ad6f Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Tue, 6 Aug 2024 16:55:21 -0300 Subject: [PATCH 04/28] feat: cleaner implementation, always set isDeposit on --- op-node/rollup/derive/attributes.go | 5 ++--- op-node/rollup/derive/l1_block_info.go | 20 +++++-------------- packages/contracts-bedrock/src/L2/L1Block.sol | 13 +++++++----- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/op-node/rollup/derive/attributes.go b/op-node/rollup/derive/attributes.go index 90b9aec6f6c5..a4e4c614c246 100644 --- a/op-node/rollup/derive/attributes.go +++ b/op-node/rollup/derive/attributes.go @@ -116,14 +116,13 @@ func (ba *FetchingAttributesBuilder) PreparePayloadAttributes(ctx context.Contex upgradeTxs = append(upgradeTxs, fjord...) } - var hasDeposits = len(depositTxs) > 0 - l1InfoTx, err := L1InfoDepositBytes(ba.rollupCfg, sysConfig, seqNumber, l1Info, nextL2Time, hasDeposits) + l1InfoTx, err := L1InfoDepositBytes(ba.rollupCfg, sysConfig, seqNumber, l1Info, nextL2Time) if err != nil { return nil, NewCriticalError(fmt.Errorf("failed to create l1InfoTx: %w", err)) } var postDeposits []hexutil.Bytes - if ba.rollupCfg.IsInteropActivationBlock(nextL2Time) && hasDeposits { + if ba.rollupCfg.IsInterop(nextL2Time) { depositsCompleteTx, err := DepositsCompleteBytes(ba.rollupCfg, sysConfig, seqNumber, l1Info, nextL2Time) if err != nil { return nil, NewCriticalError(fmt.Errorf("failed to create depositsCompleteTx: %w", err)) diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index 1cc3233fc213..b2be8d0859d7 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -24,9 +24,7 @@ const ( L1InfoArguments = 8 L1InfoBedrockLen = 4 + 32*L1InfoArguments L1InfoEcotoneLen = 4 + 32*5 // after Ecotone upgrade, args are packed into 5 32-byte slots - // TODO ^ we need to add space for the isDeposit flag here - // or should we create a new Len var for this? - DepositsCompleteLen = 4 // only the selector + DepositsCompleteLen = 4 // only the selector ) var ( @@ -60,8 +58,6 @@ type L1BlockInfo struct { BlobBaseFee *big.Int // added by Ecotone upgrade BaseFeeScalar uint32 // added by Ecotone upgrade BlobBaseFeeScalar uint32 // added by Ecotone upgrade - - IsDeposit bool // added by Interop upgrade } // Bedrock Binary Format @@ -167,8 +163,6 @@ func (info *L1BlockInfo) unmarshalBinaryBedrock(data []byte) error { // | 32 | BatcherHash | // +---------+--------------------------+ -// TODO: should we duplicate this function? -// or can we extend it somehow to only add the isDeposit flag at the end func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen)) if err := solabi.WriteSignature(w, L1InfoFuncEcotoneBytes4); err != nil { @@ -206,7 +200,6 @@ func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { if err := solabi.WriteAddress(w, info.BatcherAddr); err != nil { return nil, err } - // TODO: add isDeposit flag here return w.Bytes(), nil } @@ -248,7 +241,6 @@ func (info *L1BlockInfo) unmarshalBinaryEcotone(data []byte) error { if info.BatcherAddr, err = solabi.ReadAddress(r); err != nil { return err } - // TODO unmarshall isDeposit flag here if !solabi.EmptyReader(r) { return errors.New("too many bytes") } @@ -300,7 +292,7 @@ func DepositCompletedFromBytes(rollupCfg *rollup.Config, l2BlockTime uint64, dat // L1InfoDeposit creates a L1 Info deposit transaction based on the L1 block, // and the L2 block-height difference with the start of the epoch. -func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64, isDeposit bool) (*types.DepositTx, error) { +func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64) (*types.DepositTx, error) { l1BlockInfo := L1BlockInfo{ Number: block.NumberU64(), Time: block.Time(), @@ -308,7 +300,6 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber BlockHash: block.Hash(), SequenceNumber: seqNumber, BatcherAddr: sysCfg.BatcherAddr, - IsDeposit: isDeposit, } var data []byte if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { @@ -363,8 +354,8 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber } // L1InfoDepositBytes returns a serialized L1-info attributes transaction. -func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, l1Info eth.BlockInfo, l2BlockTime uint64, isDeposit bool) ([]byte, error) { - dep, err := L1InfoDeposit(rollupCfg, sysCfg, seqNumber, l1Info, l2BlockTime, isDeposit) +func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, l1Info eth.BlockInfo, l2BlockTime uint64) ([]byte, error) { + dep, err := L1InfoDeposit(rollupCfg, sysCfg, seqNumber, l1Info, l2BlockTime) if err != nil { return nil, fmt.Errorf("failed to create L1 info tx: %w", err) } @@ -384,7 +375,6 @@ func DepositsCompleteDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, BlockHash: block.Hash(), SequenceNumber: seqNumber, BatcherAddr: sysCfg.BatcherAddr, - IsDeposit: false, } data, err := l1BlockInfo.marshalDepositsComplete() if err != nil { @@ -403,7 +393,7 @@ func DepositsCompleteDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, Mint: nil, Value: big.NewInt(0), Gas: 50_000, - IsSystemTransaction: true, + IsSystemTransaction: false, Data: data, } // With the regolith fork we disable the IsSystemTx functionality, and allocate real gas diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index 827c44df08b2..4f62cf582a42 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -132,6 +132,13 @@ contract L1Block is ISemver, IGasToken { isDeposit = _isDeposit; } + // TODO natspec + function setL1BlockValuesIsthmus() external { + isDeposit = true; + setL1BlockValuesEcotone(calldata); + // TODO ^ this is an example... we might need to do an assembly low-level call here + } + /// @notice Updates the L1 block values for an Ecotone upgraded chain. /// Params are packed and passed in as raw msg.data instead of ABI to reduce calldata size. /// Params are expected to be in the following order: @@ -144,8 +151,7 @@ contract L1Block is ISemver, IGasToken { /// 7. _blobBaseFee L1 blob base fee. /// 8. _hash L1 blockhash. /// 9. _batcherHash Versioned hash to authenticate batcher by. - /// 10. _isDeposit isDeposit flag - function setL1BlockValuesIsthmus() external { + function setL1BlockValuesEcotone() external { address depositor = DEPOSITOR_ACCOUNT(); assembly { // Revert if the caller is not the depositor account. @@ -161,9 +167,6 @@ contract L1Block is ISemver, IGasToken { sstore(blobBaseFee.slot, calldataload(68)) // uint256 sstore(hash.slot, calldataload(100)) // bytes32 sstore(batcherHash.slot, calldataload(132)) // bytes32 - sstore(isDeposit.slot, calldataload(133)) // boolean - // TODO ^ not sure we can just add a boolean as the last slot - // (could not find how to client-side test this) } } From 65b680416300dd11f0523f4595f8377295c98217 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Tue, 6 Aug 2024 19:50:22 -0300 Subject: [PATCH 05/28] feat: simplified deposits complete tx and added Isthmus L1Info tx --- op-node/rollup/derive/attributes.go | 2 +- op-node/rollup/derive/l1_block_info.go | 113 ++++++++++++------------- 2 files changed, 56 insertions(+), 59 deletions(-) diff --git a/op-node/rollup/derive/attributes.go b/op-node/rollup/derive/attributes.go index a4e4c614c246..a64ecf7a714c 100644 --- a/op-node/rollup/derive/attributes.go +++ b/op-node/rollup/derive/attributes.go @@ -123,7 +123,7 @@ func (ba *FetchingAttributesBuilder) PreparePayloadAttributes(ctx context.Contex var postDeposits []hexutil.Bytes if ba.rollupCfg.IsInterop(nextL2Time) { - depositsCompleteTx, err := DepositsCompleteBytes(ba.rollupCfg, sysConfig, seqNumber, l1Info, nextL2Time) + depositsCompleteTx, err := DepositsCompleteBytes(seqNumber, l1Info) if err != nil { return nil, NewCriticalError(fmt.Errorf("failed to create depositsCompleteTx: %w", err)) } diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index b2be8d0859d7..842f13a77880 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -20,6 +20,7 @@ import ( const ( L1InfoFuncBedrockSignature = "setL1BlockValues(uint64,uint64,uint256,bytes32,uint64,bytes32,uint256,uint256)" L1InfoFuncEcotoneSignature = "setL1BlockValuesEcotone()" + L1InfoFuncIsthmusSignature = "setL2BlockValuesIsthmus()" DepositsCompleteSignature = "depositsComplete()" L1InfoArguments = 8 L1InfoBedrockLen = 4 + 32*L1InfoArguments @@ -30,6 +31,7 @@ const ( var ( L1InfoFuncBedrockBytes4 = crypto.Keccak256([]byte(L1InfoFuncBedrockSignature))[:4] L1InfoFuncEcotoneBytes4 = crypto.Keccak256([]byte(L1InfoFuncEcotoneSignature))[:4] + L1InfoFuncIsthmusBytes4 = crypto.Keccak256([]byte(L1InfoFuncIsthmusSignature))[:4] DepositsCompleteBytes4 = crypto.Keccak256([]byte(DepositsCompleteSignature))[:4] L1InfoDepositerAddress = common.HexToAddress("0xdeaddeaddeaddeaddeaddeaddeaddeaddead0001") L1BlockAddress = predeploys.L1BlockAddr @@ -147,7 +149,7 @@ func (info *L1BlockInfo) unmarshalBinaryBedrock(data []byte) error { return nil } -// Ecotone Binary Format +// Isthmus & Ecotone Binary Format // +---------+--------------------------+ // | Bytes | Field | // +---------+--------------------------+ @@ -162,7 +164,17 @@ func (info *L1BlockInfo) unmarshalBinaryBedrock(data []byte) error { // | 32 | BlockHash | // | 32 | BatcherHash | // +---------+--------------------------+ +func (info *L1BlockInfo) marshalBinaryIsthmus() ([]byte, error) { + // Isthmus is the same as Ecotone + out, err := info.marshalBinaryEcotone() + if err != nil { + return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) + } + // replace the first 4 bytes of the signature with L1InfoFuncIsthmusBytes4 + copy(out[:4], L1InfoFuncIsthmusBytes4) // CHECK: is this correct? or is it too nasty? + return out, nil +} func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen)) if err := solabi.WriteSignature(w, L1InfoFuncEcotoneBytes4); err != nil { @@ -203,15 +215,29 @@ func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { return w.Bytes(), nil } +func (info *L1BlockInfo) unmarshalBinaryIsthmus(data []byte) error { + return info.unmarshalBinaryEcotoneAndIsthmus(data, true) +} func (info *L1BlockInfo) unmarshalBinaryEcotone(data []byte) error { + return info.unmarshalBinaryEcotoneAndIsthmus(data) +} + +// optional isIsthmus flag +func (info *L1BlockInfo) unmarshalBinaryEcotoneAndIsthmus(data []byte, isIsthmus ...bool) error { if len(data) != L1InfoEcotoneLen { return fmt.Errorf("data is unexpected length: %d", len(data)) } r := bytes.NewReader(data) var err error - if _, err := solabi.ReadAndValidateSignature(r, L1InfoFuncEcotoneBytes4); err != nil { - return err + if isIsthmus != nil && isIsthmus[0] { + if _, err := solabi.ReadAndValidateSignature(r, L1InfoFuncIsthmusBytes4); err != nil { + return err + } + } else { + if _, err := solabi.ReadAndValidateSignature(r, L1InfoFuncEcotoneBytes4); err != nil { + return err + } } if err := binary.Read(r, binary.BigEndian, &info.BaseFeeScalar); err != nil { return ErrInvalidFormat @@ -253,41 +279,22 @@ func isEcotoneButNotFirstBlock(rollupCfg *rollup.Config, l2BlockTime uint64) boo return rollupCfg.IsEcotone(l2BlockTime) && !rollupCfg.IsEcotoneActivationBlock(l2BlockTime) } +// isInteropButNotFirstBlock returns whether the specified block is subject to the Isthmus upgrade, +// but is not the actiation block itself. +func isInteropButNotFirstBlock(rollupCfg *rollup.Config, l2BlockTime uint64) bool { + return rollupCfg.IsInterop(l2BlockTime) && !rollupCfg.IsInteropActivationBlock(l2BlockTime) +} + // L1BlockInfoFromBytes is the inverse of L1InfoDeposit, to see where the L2 chain is derived from func L1BlockInfoFromBytes(rollupCfg *rollup.Config, l2BlockTime uint64, data []byte) (*L1BlockInfo, error) { var info L1BlockInfo if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { return &info, info.unmarshalBinaryEcotone(data) } - return &info, info.unmarshalBinaryBedrock(data) -} - -func (info *L1BlockInfo) marshalDepositsComplete() ([]byte, error) { - w := bytes.NewBuffer(make([]byte, 0, DepositsCompleteLen)) - if err := solabi.WriteSignature(w, []byte(DepositsCompleteBytes4)); err != nil { - return nil, err - } - return w.Bytes(), nil -} - -func (info *L1BlockInfo) unmarshalDepositsComplete(data []byte) error { - if len(data) != DepositsCompleteLen { - return fmt.Errorf("data is unexpected length: %d", len(data)) - } - r := bytes.NewReader(data) - - if _, err := solabi.ReadAndValidateSignature(r, DepositsCompleteBytes4); err != nil { - return err - } - if !solabi.EmptyReader(r) { - return errors.New("too many bytes") + if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) { + return &info, info.unmarshalBinaryIsthmus(data) } - return nil -} - -func DepositCompletedFromBytes(rollupCfg *rollup.Config, l2BlockTime uint64, data []byte) (*L1BlockInfo, error) { - var info L1BlockInfo - return &info, info.unmarshalDepositsComplete(data) + return &info, info.unmarshalBinaryBedrock(data) } // L1InfoDeposit creates a L1 Info deposit transaction based on the L1 block, @@ -302,6 +309,7 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber BatcherAddr: sysCfg.BatcherAddr, } var data []byte + if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { l1BlockInfo.BlobBaseFee = block.BlobBaseFee() if l1BlockInfo.BlobBaseFee == nil { @@ -314,11 +322,19 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber } l1BlockInfo.BlobBaseFeeScalar = scalars.BlobBaseFeeScalar l1BlockInfo.BaseFeeScalar = scalars.BaseFeeScalar - out, err := l1BlockInfo.marshalBinaryEcotone() - if err != nil { - return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) + if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) { + out, err := l1BlockInfo.marshalBinaryIsthmus() + if err != nil { + return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err) + } + data = out + } else { + out, err := l1BlockInfo.marshalBinaryEcotone() + if err != nil { + return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) + } + data = out } - data = out } else { l1BlockInfo.L1FeeOverhead = sysCfg.Overhead l1BlockInfo.L1FeeScalar = sysCfg.Scalar @@ -367,25 +383,11 @@ func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNu return opaqueL1Tx, nil } -func DepositsCompleteDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64) (*types.DepositTx, error) { - l1BlockInfo := L1BlockInfo{ - Number: block.NumberU64(), - Time: block.Time(), - BaseFee: block.BaseFee(), - BlockHash: block.Hash(), - SequenceNumber: seqNumber, - BatcherAddr: sysCfg.BatcherAddr, - } - data, err := l1BlockInfo.marshalDepositsComplete() - if err != nil { - return nil, fmt.Errorf("failed to marshal Bedrock l1 block info: %w", err) - } +func DepositsCompleteDeposit(seqNumber uint64, block eth.BlockInfo) (*types.DepositTx, error) { source := L1InfoDepositSource{ L1BlockHash: block.Hash(), SeqNumber: seqNumber, } - // Set a normal gas limit with `IsSystemTransaction` to ensure - // that the DepositCompleted Transaction does not run out of gas. out := &types.DepositTx{ SourceHash: source.SourceHash(), From: L1InfoDepositerAddress, @@ -394,18 +396,13 @@ func DepositsCompleteDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, Value: big.NewInt(0), Gas: 50_000, IsSystemTransaction: false, - Data: data, - } - // With the regolith fork we disable the IsSystemTx functionality, and allocate real gas - if rollupCfg.IsRegolith(l2BlockTime) { - out.IsSystemTransaction = false - out.Gas = RegolithSystemTxGas + Data: DepositsCompleteBytes4, } return out, nil } -func DepositsCompleteBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, l1Info eth.BlockInfo, l2BlockTime uint64) ([]byte, error) { - dep, err := DepositsCompleteDeposit(rollupCfg, sysCfg, seqNumber, l1Info, l2BlockTime) +func DepositsCompleteBytes(seqNumber uint64, l1Info eth.BlockInfo) ([]byte, error) { + dep, err := DepositsCompleteDeposit(seqNumber, l1Info) if err != nil { return nil, fmt.Errorf("failed to create DepositsComplete tx: %w", err) } From 70b7db0d27eda6cc59add29e4c2c18850b1eb0b2 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Wed, 7 Aug 2024 11:37:07 -0300 Subject: [PATCH 06/28] feat: L1Block make ecotone a public function --- packages/contracts-bedrock/src/L2/L1Block.sol | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index 4f62cf582a42..6c90f70738ef 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -135,8 +135,7 @@ contract L1Block is ISemver, IGasToken { // TODO natspec function setL1BlockValuesIsthmus() external { isDeposit = true; - setL1BlockValuesEcotone(calldata); - // TODO ^ this is an example... we might need to do an assembly low-level call here + setL1BlockValuesEcotone(); // Internally call the Ecotone function } /// @notice Updates the L1 block values for an Ecotone upgraded chain. @@ -151,7 +150,7 @@ contract L1Block is ISemver, IGasToken { /// 7. _blobBaseFee L1 blob base fee. /// 8. _hash L1 blockhash. /// 9. _batcherHash Versioned hash to authenticate batcher by. - function setL1BlockValuesEcotone() external { + function setL1BlockValuesEcotone() public { address depositor = DEPOSITOR_ACCOUNT(); assembly { // Revert if the caller is not the depositor account. From 24deb1c1a08210fc39488f2759591df2b98fd19e Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Wed, 7 Aug 2024 12:02:21 -0300 Subject: [PATCH 07/28] feat: re-organized shared functions --- op-node/rollup/derive/l1_block_info.go | 56 ++++++++++++++++---------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index 842f13a77880..f307c7405a7b 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -164,21 +164,33 @@ func (info *L1BlockInfo) unmarshalBinaryBedrock(data []byte) error { // | 32 | BlockHash | // | 32 | BatcherHash | // +---------+--------------------------+ + func (info *L1BlockInfo) marshalBinaryIsthmus() ([]byte, error) { - // Isthmus is the same as Ecotone - out, err := info.marshalBinaryEcotone() + out, err := marshalBinaryEcotoneOrIsthmus(info, true) if err != nil { - return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) + return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err) } - // replace the first 4 bytes of the signature with L1InfoFuncIsthmusBytes4 - copy(out[:4], L1InfoFuncIsthmusBytes4) // CHECK: is this correct? or is it too nasty? return out, nil - } + func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { - w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen)) - if err := solabi.WriteSignature(w, L1InfoFuncEcotoneBytes4); err != nil { - return nil, err + out, err := marshalBinaryEcotoneOrIsthmus(info, true) + if err != nil { + return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) + } + return out, nil +} + +func marshalBinaryEcotoneOrIsthmus(info *L1BlockInfo, isIsthmus ...bool) ([]byte, error) { + w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen)) // Ecotone and Isthmus have the same length + if isIsthmus != nil && isIsthmus[0] { + if err := solabi.WriteSignature(w, L1InfoFuncIsthmusBytes4); err != nil { + return nil, err + } + } else { + if err := solabi.WriteSignature(w, L1InfoFuncEcotoneBytes4); err != nil { + return nil, err + } } if err := binary.Write(w, binary.BigEndian, info.BaseFeeScalar); err != nil { return nil, err @@ -216,14 +228,13 @@ func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { } func (info *L1BlockInfo) unmarshalBinaryIsthmus(data []byte) error { - return info.unmarshalBinaryEcotoneAndIsthmus(data, true) + return unmarshalBinaryEcotoneAndIsthmus(info, data, true) } func (info *L1BlockInfo) unmarshalBinaryEcotone(data []byte) error { - return info.unmarshalBinaryEcotoneAndIsthmus(data) + return unmarshalBinaryEcotoneAndIsthmus(info, data) } -// optional isIsthmus flag -func (info *L1BlockInfo) unmarshalBinaryEcotoneAndIsthmus(data []byte, isIsthmus ...bool) error { +func unmarshalBinaryEcotoneAndIsthmus(info *L1BlockInfo, data []byte, isIsthmus ...bool) error { if len(data) != L1InfoEcotoneLen { return fmt.Errorf("data is unexpected length: %d", len(data)) } @@ -288,12 +299,13 @@ func isInteropButNotFirstBlock(rollupCfg *rollup.Config, l2BlockTime uint64) boo // L1BlockInfoFromBytes is the inverse of L1InfoDeposit, to see where the L2 chain is derived from func L1BlockInfoFromBytes(rollupCfg *rollup.Config, l2BlockTime uint64, data []byte) (*L1BlockInfo, error) { var info L1BlockInfo - if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { - return &info, info.unmarshalBinaryEcotone(data) - } + // Important, this should be ordered from most recent to oldest if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) { return &info, info.unmarshalBinaryIsthmus(data) } + if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { + return &info, info.unmarshalBinaryEcotone(data) + } return &info, info.unmarshalBinaryBedrock(data) } @@ -310,7 +322,7 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber } var data []byte - if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { + if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) { l1BlockInfo.BlobBaseFee = block.BlobBaseFee() if l1BlockInfo.BlobBaseFee == nil { // The L2 spec states to use the MIN_BLOB_GASPRICE from EIP-4844 if not yet active on L1. @@ -322,16 +334,16 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber } l1BlockInfo.BlobBaseFeeScalar = scalars.BlobBaseFeeScalar l1BlockInfo.BaseFeeScalar = scalars.BaseFeeScalar - if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) { - out, err := l1BlockInfo.marshalBinaryIsthmus() + if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { + out, err := l1BlockInfo.marshalBinaryEcotone() if err != nil { - return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err) + return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) } data = out } else { - out, err := l1BlockInfo.marshalBinaryEcotone() + out, err := l1BlockInfo.marshalBinaryIsthmus() if err != nil { - return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) + return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err) } data = out } From f463e8a19246f5c640240c590ebeeaf47c0cda3f Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Wed, 7 Aug 2024 14:33:01 -0300 Subject: [PATCH 08/28] feat: revamp unmarshalBinaryIsthmusAndEcotone function --- op-node/rollup/derive/l1_block_info.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index f307c7405a7b..c91f9d0255ff 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -228,27 +228,21 @@ func marshalBinaryEcotoneOrIsthmus(info *L1BlockInfo, isIsthmus ...bool) ([]byte } func (info *L1BlockInfo) unmarshalBinaryIsthmus(data []byte) error { - return unmarshalBinaryEcotoneAndIsthmus(info, data, true) + return unmarshalBinaryWithSignatureAndData(info, L1InfoFuncIsthmusBytes4, data) } func (info *L1BlockInfo) unmarshalBinaryEcotone(data []byte) error { - return unmarshalBinaryEcotoneAndIsthmus(info, data) + return unmarshalBinaryWithSignatureAndData(info, L1InfoFuncEcotoneBytes4, data) } -func unmarshalBinaryEcotoneAndIsthmus(info *L1BlockInfo, data []byte, isIsthmus ...bool) error { +func unmarshalBinaryWithSignatureAndData(info *L1BlockInfo, signature []byte, data []byte) error { if len(data) != L1InfoEcotoneLen { return fmt.Errorf("data is unexpected length: %d", len(data)) } r := bytes.NewReader(data) var err error - if isIsthmus != nil && isIsthmus[0] { - if _, err := solabi.ReadAndValidateSignature(r, L1InfoFuncIsthmusBytes4); err != nil { - return err - } - } else { - if _, err := solabi.ReadAndValidateSignature(r, L1InfoFuncEcotoneBytes4); err != nil { - return err - } + if _, err := solabi.ReadAndValidateSignature(r, signature); err != nil { + return err } if err := binary.Read(r, binary.BigEndian, &info.BaseFeeScalar); err != nil { return ErrInvalidFormat From e90db39b3bb6b7cf1a87c353eb6bd55aa98f26e1 Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Wed, 7 Aug 2024 21:10:46 -0300 Subject: [PATCH 09/28] feat: ban deposits interop (#11396) --- .../contracts-bedrock/src/L2/CrossL2Inbox.sol | 14 ++- packages/contracts-bedrock/src/L2/L1Block.sol | 43 +++++-- .../src/libraries/L1BlockErrors.sol | 3 + .../test/L2/CrossL2Inbox.t.sol | 71 ++++++++++- .../contracts-bedrock/test/L2/L1Block.t.sol | 112 +++++++++++++++++- 5 files changed, 230 insertions(+), 13 deletions(-) diff --git a/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol b/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol index b85d09580ad7..4a0dda1e9335 100644 --- a/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol +++ b/packages/contracts-bedrock/src/L2/CrossL2Inbox.sol @@ -17,6 +17,14 @@ interface IDependencySet { function isInDependencySet(uint256 _chainId) external view returns (bool); } +/// @title IL1Block +/// @notice Interface for L1Block with only `isDeposit()` method. +interface IL1Block { + /// @notice Returns whether the call was triggered from a a deposit or not. + /// @return True if the current call was triggered by a deposit transaction, and false otherwise. + function isDeposit() external view returns (bool); +} + /// @notice Thrown when a non-written transient storage slot is attempted to be read from. error NotEntered(); @@ -59,8 +67,8 @@ contract CrossL2Inbox is ICrossL2Inbox, ISemver, TransientReentrancyAware { bytes32 internal constant CHAINID_SLOT = 0x6e0446e8b5098b8c8193f964f1b567ec3a2bdaeba33d36acb85c1f1d3f92d313; /// @notice Semantic version. - /// @custom:semver 1.0.0-beta.3 - string public constant version = "1.0.0-beta.3"; + /// @custom:semver 1.1.0-beta.3 + string public constant version = "1.1.0-beta.3"; /// @notice Emitted when a cross chain message is being executed. /// @param msgHash Hash of message payload being executed. @@ -118,7 +126,7 @@ contract CrossL2Inbox is ICrossL2Inbox, ISemver, TransientReentrancyAware { reentrantAware { // We need to know if this is being called on a depositTx - if (L1_BLOCK.isDeposit()) revert NoExecutingDeposits(); + if (IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).isDeposit()) revert NoExecutingDeposits(); if (_id.timestamp > block.timestamp) revert InvalidTimestamp(); if (!IDependencySet(Predeploys.L1_BLOCK_ATTRIBUTES).isInDependencySet(_id.chainId)) { diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index 6c90f70738ef..cff6e2455139 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.15; import { ISemver } from "src/universal/ISemver.sol"; import { Constants } from "src/libraries/Constants.sol"; import { GasPayingToken, IGasToken } from "src/libraries/GasPayingToken.sol"; +import { Predeploys } from "src/libraries/Predeploys.sol"; import "src/libraries/L1BlockErrors.sol"; /// @custom:proxied @@ -57,12 +58,14 @@ contract L1Block is ISemver, IGasToken { /// @notice The latest L1 blob base fee. uint256 public blobBaseFee; - /// @notice The isDeposit flag. - bool public isDeposit; + /// @notice Storage slot that the isDeposit is stored at. + /// This is a custom slot that is not part of the standard storage layout. + /// keccak256(abi.encode(uint256(keccak256("l1Block.identifier.isDeposit")) - 1)) & ~bytes32(uint256(0xff)) + uint256 internal constant IS_DEPOSIT_SLOT = 0x921bd3a089295c6e5540e8fba8195448d253efd6f2e3e495b499b627dc36a300; - /// @custom:semver 1.4.1-beta.1 + /// @custom:semver 1.5.1-beta.1 function version() public pure virtual returns (string memory) { - return "1.4.1-beta.1"; + return "1.5.1-beta.1"; } /// @notice Returns the gas paying token, its decimals, name and symbol. @@ -90,9 +93,13 @@ contract L1Block is ISemver, IGasToken { return token != Constants.ETHER; } - function isDeposit() returns (bool _isDeposit) external view { - require(msg.sender == CROSS_L2_INBOX, "L1Block: only the CrossL2Inbox can check if it is a deposit"); - _isDeposit = isDeposit; + /// @notice Returns whether the call was triggered from a a deposit or not. + /// @notice This function is only callable by the CrossL2Inbox contract. + function isDeposit() external view returns (bool isDeposit_) { + if (msg.sender != Predeploys.CROSS_L2_INBOX) revert NotCrossL2Inbox(); + assembly { + isDeposit_ := sload(IS_DEPOSIT_SLOT) + } } /// @custom:legacy @@ -138,6 +145,17 @@ contract L1Block is ISemver, IGasToken { setL1BlockValuesEcotone(); // Internally call the Ecotone function } + /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain. + /// It updates the L1 block values through the `setL1BlockValuesEcotone` function. + function setL1BlockValuesIsthmus() external { + // Set the isDeposit flag to true. + assembly { + sstore(IS_DEPOSIT_SLOT, 1) + } + + setL1BlockValuesEcotone(); + } + /// @notice Updates the L1 block values for an Ecotone upgraded chain. /// Params are packed and passed in as raw msg.data instead of ABI to reduce calldata size. /// Params are expected to be in the following order: @@ -169,6 +187,17 @@ contract L1Block is ISemver, IGasToken { } } + /// @notice Resets the isDeposit flag. + /// Should only be called by the depositor account after the deposits are complete. + function depositsComplete() external { + if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); + + // Set the isDeposit flag to false. + assembly { + sstore(IS_DEPOSIT_SLOT, 0) + } + } + /// @notice Sets the gas paying token for the L2 system. Can only be called by the special /// depositor account. This function is not called on every L2 block but instead /// only called by specially crafted L1 deposit transactions. diff --git a/packages/contracts-bedrock/src/libraries/L1BlockErrors.sol b/packages/contracts-bedrock/src/libraries/L1BlockErrors.sol index c9ef3903aebe..44e156e158c0 100644 --- a/packages/contracts-bedrock/src/libraries/L1BlockErrors.sol +++ b/packages/contracts-bedrock/src/libraries/L1BlockErrors.sol @@ -4,6 +4,9 @@ pragma solidity ^0.8.0; /// @notice Error returns when a non-depositor account tries to set L1 block values. error NotDepositor(); +/// @notice Error when a non-cross L2 Inbox sender tries to call the `isDeposit()` method. +error NotCrossL2Inbox(); + /// @notice Error when a chain ID is not in the interop dependency set. error NotDependency(); diff --git a/packages/contracts-bedrock/test/L2/CrossL2Inbox.t.sol b/packages/contracts-bedrock/test/L2/CrossL2Inbox.t.sol index 9350e6b2fbbc..e01d5635b3ca 100644 --- a/packages/contracts-bedrock/test/L2/CrossL2Inbox.t.sol +++ b/packages/contracts-bedrock/test/L2/CrossL2Inbox.t.sol @@ -9,7 +9,15 @@ import { Predeploys } from "src/libraries/Predeploys.sol"; import { TransientContext } from "src/libraries/TransientContext.sol"; // Target contracts -import { CrossL2Inbox, NotEntered, InvalidTimestamp, InvalidChainId, TargetCallFailed } from "src/L2/CrossL2Inbox.sol"; +import { + CrossL2Inbox, + IL1Block, + NotEntered, + NoExecutingDeposits, + InvalidTimestamp, + InvalidChainId, + TargetCallFailed +} from "src/L2/CrossL2Inbox.sol"; import { ICrossL2Inbox } from "src/L2/ICrossL2Inbox.sol"; /// @title CrossL2InboxWithModifiableTransientStorage @@ -84,6 +92,13 @@ contract CrossL2InboxTest is Test { // Ensure that the target call is payable if value is sent if (_value > 0) assumePayable(_target); + // Ensure is not a deposit transaction + vm.mockCall({ + callee: Predeploys.L1_BLOCK_ATTRIBUTES, + data: abi.encodeWithSelector(IL1Block.isDeposit.selector), + returnData: abi.encode(false) + }); + // Ensure that the target call does not revert vm.mockCall({ callee: _target, msgValue: _value, data: _message, returnData: abi.encode(true) }); @@ -137,6 +152,13 @@ contract CrossL2InboxTest is Test { _id1.timestamp = bound(_id1.timestamp, 0, block.timestamp); _id2.timestamp = bound(_id2.timestamp, 0, block.timestamp); + // Ensure is not a deposit transaction + vm.mockCall({ + callee: Predeploys.L1_BLOCK_ATTRIBUTES, + data: abi.encodeWithSelector(IL1Block.isDeposit.selector), + returnData: abi.encode(false) + }); + // Ensure that id1's chain ID is in the dependency set vm.mockCall({ callee: Predeploys.L1_BLOCK_ATTRIBUTES, @@ -181,6 +203,32 @@ contract CrossL2InboxTest is Test { assertEq(crossL2Inbox.chainId(), _id2.chainId); } + /// @dev Tests that the `executeMessage` function reverts if the transaction comes from a deposit. + function testFuzz_executeMessage_isDeposit_reverts( + ICrossL2Inbox.Identifier calldata _id, + address _target, + bytes calldata _message, + uint256 _value + ) + external + { + // Ensure it is a deposit transaction + vm.mockCall({ + callee: Predeploys.L1_BLOCK_ATTRIBUTES, + data: abi.encodeWithSelector(IL1Block.isDeposit.selector), + returnData: abi.encode(true) + }); + + // Ensure that the contract has enough balance to send with value + vm.deal(address(this), _value); + + // Expect a revert with the NoExecutingDeposits selector + vm.expectRevert(NoExecutingDeposits.selector); + + // Call the executeMessage function + crossL2Inbox.executeMessage{ value: _value }({ _id: _id, _target: _target, _message: _message }); + } + /// @dev Tests that the `executeMessage` function reverts when called with an identifier with an invalid timestamp. function testFuzz_executeMessage_invalidTimestamp_reverts( ICrossL2Inbox.Identifier calldata _id, @@ -193,6 +241,13 @@ contract CrossL2InboxTest is Test { // Ensure that the id's timestamp is invalid (greater than the current block timestamp) vm.assume(_id.timestamp > block.timestamp); + // Ensure is not a deposit transaction + vm.mockCall({ + callee: Predeploys.L1_BLOCK_ATTRIBUTES, + data: abi.encodeWithSelector(IL1Block.isDeposit.selector), + returnData: abi.encode(false) + }); + // Ensure that the contract has enough balance to send with value vm.deal(address(this), _value); @@ -216,6 +271,13 @@ contract CrossL2InboxTest is Test { // Ensure that the id's timestamp is valid (less than or equal to the current block timestamp) _id.timestamp = bound(_id.timestamp, 0, block.timestamp); + // Ensure is not a deposit transaction + vm.mockCall({ + callee: Predeploys.L1_BLOCK_ATTRIBUTES, + data: abi.encodeWithSelector(IL1Block.isDeposit.selector), + returnData: abi.encode(false) + }); + // Ensure that the chain ID is NOT in the dependency set vm.mockCall({ callee: Predeploys.L1_BLOCK_ATTRIBUTES, @@ -251,6 +313,13 @@ contract CrossL2InboxTest is Test { // Ensure that the target call reverts vm.mockCallRevert({ callee: _target, msgValue: _value, data: _message, revertData: abi.encode(false) }); + // Ensure is not a deposit transaction + vm.mockCall({ + callee: Predeploys.L1_BLOCK_ATTRIBUTES, + data: abi.encodeWithSelector(IL1Block.isDeposit.selector), + returnData: abi.encode(false) + }); + // Ensure that the chain ID is in the dependency set vm.mockCall({ callee: Predeploys.L1_BLOCK_ATTRIBUTES, diff --git a/packages/contracts-bedrock/test/L2/L1Block.t.sol b/packages/contracts-bedrock/test/L2/L1Block.t.sol index ce67a6692223..4c0af9127f03 100644 --- a/packages/contracts-bedrock/test/L2/L1Block.t.sol +++ b/packages/contracts-bedrock/test/L2/L1Block.t.sol @@ -10,7 +10,8 @@ import { Constants } from "src/libraries/Constants.sol"; // Target contract import { L1Block } from "src/L2/L1Block.sol"; -import "src/libraries/L1BlockErrors.sol"; +import { NotDepositor, NotCrossL2Inbox } from "src/libraries/L1BlockErrors.sol"; +import { Predeploys } from "src/libraries/Predeploys.sol"; contract L1BlockTest is CommonTest { address depositor; @@ -34,7 +35,8 @@ contract L1BlockBedrock_Test is L1BlockTest { uint64 s, bytes32 bt, uint256 fo, - uint256 fs + uint256 fs, + bool d ) external { @@ -81,6 +83,59 @@ contract L1BlockBedrock_Test is L1BlockTest { } } +contract L1BlockSetL1BlockValuesIsthmus_Test is L1BlockTest { + /// @dev Tests that `setL1BlockValuesIsthmus` reverts if sender address is not the depositor + function test_setL1BlockValuesIsthmus_notDepositor_reverts(address _caller) external { + vm.assume(_caller != depositor); + vm.prank(_caller); + vm.expectRevert(NotDepositor.selector); + l1Block.setL1BlockValuesIsthmus(); + } + + /// @dev Tests that `setL1BlockValuesIsthmus` succeeds if sender address is the depositor + function test_setL1BlockValuesIsthmus_succeeds( + uint32 baseFeeScalar, + uint32 blobBaseFeeScalar, + uint64 sequenceNumber, + uint64 timestamp, + uint64 number, + uint256 baseFee, + uint256 blobBaseFee, + bytes32 hash, + bytes32 batcherHash + ) + external + { + // Ensure the `isDepositTransaction` flag is false before calling `setL1BlockValuesIsthmus` + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(l1Block.isDeposit(), false); + + bytes memory setValuesEcotoneCalldata = abi.encodePacked( + baseFeeScalar, blobBaseFeeScalar, sequenceNumber, timestamp, number, baseFee, blobBaseFee, hash, batcherHash + ); + + vm.prank(depositor); + (bool success,) = + address(l1Block).call(abi.encodePacked(L1Block.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata)); + assertTrue(success, "function call failed"); + + // Assert that the `isDepositTransaction` flag was properly set to true + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(l1Block.isDeposit(), true); + + // Assert `setL1BlockValuesEcotone` was properly called, forwarding the calldata to it + assertEq(l1Block.baseFeeScalar(), baseFeeScalar, "1"); + assertEq(l1Block.blobBaseFeeScalar(), blobBaseFeeScalar, "2"); + assertEq(l1Block.sequenceNumber(), sequenceNumber, "3"); + assertEq(l1Block.timestamp(), timestamp, "4"); + assertEq(l1Block.number(), number, "5"); + assertEq(l1Block.basefee(), baseFee, "6"); + assertEq(l1Block.blobBaseFee(), blobBaseFee, "7"); + assertEq(l1Block.hash(), hash, "8"); + assertEq(l1Block.batcherHash(), batcherHash, "9"); + } +} + contract L1BlockEcotone_Test is L1BlockTest { /// @dev Tests that setL1BlockValuesEcotone updates the values appropriately. function testFuzz_setL1BlockValuesEcotone_succeeds( @@ -168,6 +223,35 @@ contract L1BlockEcotone_Test is L1BlockTest { } } +contract L1BlockDepositsComplete_Test is L1BlockTest { + // @dev Tests that `depositsComplete` reverts if the caller is not the depositor. + function test_deposits_is_depositor_reverts(address _caller) external { + vm.assume(_caller != depositor); + vm.expectRevert(NotDepositor.selector); + l1Block.depositsComplete(); + } + + // @dev Tests that `depositsComplete` succeeds if the caller is the depositor. + function test_depositsComplete_succeeds() external { + // Set the `isDeposit` flag to true + vm.prank(depositor); + l1Block.setL1BlockValuesIsthmus(); + + // Assert that the `isDeposit` flag was properly set to true + vm.prank(Predeploys.CROSS_L2_INBOX); + assertTrue(l1Block.isDeposit()); + + // Call `depositsComplete` + vm.prank(depositor); + l1Block.depositsComplete(); + + // Assert that the `isDeposit` flag was properly set to false + /// @dev Assuming that `isDeposit()` wil return the proper value. That function is tested as well + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(l1Block.isDeposit(), false); + } +} + contract L1BlockCustomGasToken_Test is L1BlockTest { function testFuzz_setGasPayingToken_succeeds( address _token, @@ -205,3 +289,27 @@ contract L1BlockCustomGasToken_Test is L1BlockTest { l1Block.setGasPayingToken(address(this), 18, "Test", "TST"); } } + +contract L1BlockIsDeposit_Test is L1BlockTest { + /// @dev Tests that `isDeposit` reverts if the caller is not the cross L2 inbox. + function test_isDeposit_notCrossL2Inbox_reverts(address _caller) external { + vm.assume(_caller != Predeploys.CROSS_L2_INBOX); + vm.expectRevert(NotCrossL2Inbox.selector); + l1Block.isDeposit(); + } + + /// @dev Tests that `isDeposit` always returns the correct value. + function test_isDeposit_succeeds() external { + // Assert is false if the value is not updated + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(l1Block.isDeposit(), false); + + /// @dev Assuming that `setL1BlockValuesIsthmus` will set the proper value. That function is tested as well + vm.prank(depositor); + l1Block.setL1BlockValuesIsthmus(); + + // Assert is true if the value is updated + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(l1Block.isDeposit(), true); + } +} From 2838d6e319254063fce880c0e49a96fa442b4134 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Thu, 8 Aug 2024 12:43:01 -0300 Subject: [PATCH 10/28] fix: removed duplicated function from bad merge --- packages/contracts-bedrock/src/L2/L1Block.sol | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index cff6e2455139..af60667d66a4 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -139,14 +139,9 @@ contract L1Block is ISemver, IGasToken { isDeposit = _isDeposit; } - // TODO natspec - function setL1BlockValuesIsthmus() external { - isDeposit = true; - setL1BlockValuesEcotone(); // Internally call the Ecotone function - } - /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain. /// It updates the L1 block values through the `setL1BlockValuesEcotone` function. + /// It forwards the calldata to the internally-used `setL1BlockValuesEcotone` function. function setL1BlockValuesIsthmus() external { // Set the isDeposit flag to true. assembly { From e40d49fa131ea7fa03513d0520b553943519173d Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Thu, 8 Aug 2024 12:58:41 -0300 Subject: [PATCH 11/28] fix: relevant comments added, optional bool removed --- op-node/rollup/derive/l1_block_info.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index c91f9d0255ff..fe63e2e193b1 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -174,16 +174,16 @@ func (info *L1BlockInfo) marshalBinaryIsthmus() ([]byte, error) { } func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { - out, err := marshalBinaryEcotoneOrIsthmus(info, true) + out, err := marshalBinaryEcotoneOrIsthmus(info, false) if err != nil { return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) } return out, nil } -func marshalBinaryEcotoneOrIsthmus(info *L1BlockInfo, isIsthmus ...bool) ([]byte, error) { +func marshalBinaryEcotoneOrIsthmus(info *L1BlockInfo, isIsthmus bool) ([]byte, error) { w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen)) // Ecotone and Isthmus have the same length - if isIsthmus != nil && isIsthmus[0] { + if isIsthmus { if err := solabi.WriteSignature(w, L1InfoFuncIsthmusBytes4); err != nil { return nil, err } @@ -287,6 +287,11 @@ func isEcotoneButNotFirstBlock(rollupCfg *rollup.Config, l2BlockTime uint64) boo // isInteropButNotFirstBlock returns whether the specified block is subject to the Isthmus upgrade, // but is not the actiation block itself. func isInteropButNotFirstBlock(rollupCfg *rollup.Config, l2BlockTime uint64) bool { + // note from Proto: + // Since we use the pre-interop L1 tx one last time during the upgrade block, + // we must disallow the deposit-txs from using the CrossL2Inbox during this block. + // If the CrossL2Inbox does not exist yet, then it is safe, + // but we have to ensure that the spec and code puts any Interop upgrade-txs after the user deposits. return rollupCfg.IsInterop(l2BlockTime) && !rollupCfg.IsInteropActivationBlock(l2BlockTime) } @@ -316,7 +321,7 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber } var data []byte - if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) { + if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { l1BlockInfo.BlobBaseFee = block.BlobBaseFee() if l1BlockInfo.BlobBaseFee == nil { // The L2 spec states to use the MIN_BLOB_GASPRICE from EIP-4844 if not yet active on L1. @@ -328,16 +333,16 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber } l1BlockInfo.BlobBaseFeeScalar = scalars.BlobBaseFeeScalar l1BlockInfo.BaseFeeScalar = scalars.BaseFeeScalar - if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) { - out, err := l1BlockInfo.marshalBinaryEcotone() + if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) { + out, err := l1BlockInfo.marshalBinaryIsthmus() if err != nil { - return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) + return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err) } data = out } else { - out, err := l1BlockInfo.marshalBinaryIsthmus() + out, err := l1BlockInfo.marshalBinaryEcotone() if err != nil { - return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err) + return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) } data = out } @@ -400,7 +405,7 @@ func DepositsCompleteDeposit(seqNumber uint64, block eth.BlockInfo) (*types.Depo To: &L1BlockAddress, Mint: nil, Value: big.NewInt(0), - Gas: 50_000, + Gas: 50_000, // TODO: check how much gas is actually needed IsSystemTransaction: false, Data: DepositsCompleteBytes4, } From c89ffa41e21da8ec7506f3cba2e8c787e630fbbc Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Thu, 8 Aug 2024 13:04:28 -0300 Subject: [PATCH 12/28] fix: sstore of isDeposit --- packages/contracts-bedrock/src/L2/L1Block.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index af60667d66a4..fee97c69e81f 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -136,7 +136,9 @@ contract L1Block is ISemver, IGasToken { batcherHash = _batcherHash; l1FeeOverhead = _l1FeeOverhead; l1FeeScalar = _l1FeeScalar; - isDeposit = _isDeposit; + assembly { + sstore(IS_DEPOSIT_SLOT, _isDeposit) + } } /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain. From ebc225db724770d44bb7c144902ef2379c303b0c Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Thu, 8 Aug 2024 13:10:28 -0300 Subject: [PATCH 13/28] fix: sstore of isDeposit --- packages/contracts-bedrock/src/L2/L1Block.sol | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index fee97c69e81f..5c6bff8283a1 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -206,9 +206,4 @@ contract L1Block is ISemver, IGasToken { emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol }); } - /// @notice Resets the isDeposit flag. - function depositsComplete() external { - if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); - isDeposit = false; - } } From 198d683fabb41d82d50ea315384be36b57651cf3 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Thu, 8 Aug 2024 13:23:21 -0300 Subject: [PATCH 14/28] fix: is deposit relevant tests --- packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol | 1 + packages/contracts-bedrock/test/L2/L1Block.t.sol | 5 ++++- packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol b/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol index 21f7b98f666b..25ad350e1017 100644 --- a/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol +++ b/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol @@ -53,6 +53,7 @@ contract GasPriceOracleBedrock_Test is GasPriceOracle_Test { _batcherHash: batcherHash, _l1FeeOverhead: l1FeeOverhead, _l1FeeScalar: l1FeeScalar + _isDeposit: false }); } diff --git a/packages/contracts-bedrock/test/L2/L1Block.t.sol b/packages/contracts-bedrock/test/L2/L1Block.t.sol index 4c0af9127f03..c4df99071a5b 100644 --- a/packages/contracts-bedrock/test/L2/L1Block.t.sol +++ b/packages/contracts-bedrock/test/L2/L1Block.t.sol @@ -41,7 +41,7 @@ contract L1BlockBedrock_Test is L1BlockTest { external { vm.prank(depositor); - l1Block.setL1BlockValues(n, t, b, h, s, bt, fo, fs); + l1Block.setL1BlockValues(n, t, b, h, s, bt, fo, fs, d); assertEq(l1Block.number(), n); assertEq(l1Block.timestamp(), t); assertEq(l1Block.basefee(), b); @@ -50,6 +50,7 @@ contract L1BlockBedrock_Test is L1BlockTest { assertEq(l1Block.batcherHash(), bt); assertEq(l1Block.l1FeeOverhead(), fo); assertEq(l1Block.l1FeeScalar(), fs); + assertEq(l1Block.isDeposit(), d); } /// @dev Tests that `setL1BlockValues` can set max values. @@ -64,6 +65,7 @@ contract L1BlockBedrock_Test is L1BlockTest { _batcherHash: bytes32(type(uint256).max), _l1FeeOverhead: type(uint256).max, _l1FeeScalar: type(uint256).max + _isDeposit: type(bool).max }); } @@ -79,6 +81,7 @@ contract L1BlockBedrock_Test is L1BlockTest { _batcherHash: bytes32(type(uint256).max), _l1FeeOverhead: type(uint256).max, _l1FeeScalar: type(uint256).max + _isDeposit: type(bool).max }); } } diff --git a/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol b/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol index 3fad97699232..5c801449dbee 100644 --- a/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol +++ b/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol @@ -33,6 +33,7 @@ contract L1BlockNumberTest is Test { _batcherHash: bytes32(uint256(0)), _l1FeeOverhead: 2, _l1FeeScalar: 3 + _isDeposit: false }); } From 1434e89cc8c3fe9e803303432d9f9d2ad7c01339 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Thu, 8 Aug 2024 13:31:35 -0300 Subject: [PATCH 15/28] fix: is deposit relevant tests --- packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol | 2 +- packages/contracts-bedrock/test/L2/L1Block.t.sol | 8 ++++---- .../contracts-bedrock/test/legacy/L1BlockNumber.t.sol | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol b/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol index 25ad350e1017..a3e38bab1e2c 100644 --- a/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol +++ b/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol @@ -52,7 +52,7 @@ contract GasPriceOracleBedrock_Test is GasPriceOracle_Test { _sequenceNumber: sequenceNumber, _batcherHash: batcherHash, _l1FeeOverhead: l1FeeOverhead, - _l1FeeScalar: l1FeeScalar + _l1FeeScalar: l1FeeScalar, _isDeposit: false }); } diff --git a/packages/contracts-bedrock/test/L2/L1Block.t.sol b/packages/contracts-bedrock/test/L2/L1Block.t.sol index c4df99071a5b..ff714b538ad6 100644 --- a/packages/contracts-bedrock/test/L2/L1Block.t.sol +++ b/packages/contracts-bedrock/test/L2/L1Block.t.sol @@ -64,8 +64,8 @@ contract L1BlockBedrock_Test is L1BlockTest { _sequenceNumber: type(uint64).max, _batcherHash: bytes32(type(uint256).max), _l1FeeOverhead: type(uint256).max, - _l1FeeScalar: type(uint256).max - _isDeposit: type(bool).max + _l1FeeScalar: type(uint256).max, + _isDeposit: true }); } @@ -80,8 +80,8 @@ contract L1BlockBedrock_Test is L1BlockTest { _sequenceNumber: type(uint64).max, _batcherHash: bytes32(type(uint256).max), _l1FeeOverhead: type(uint256).max, - _l1FeeScalar: type(uint256).max - _isDeposit: type(bool).max + _l1FeeScalar: type(uint256).max, + _isDeposit: true }); } } diff --git a/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol b/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol index 5c801449dbee..4128d41c0b07 100644 --- a/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol +++ b/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol @@ -32,7 +32,7 @@ contract L1BlockNumberTest is Test { _sequenceNumber: uint64(4), _batcherHash: bytes32(uint256(0)), _l1FeeOverhead: 2, - _l1FeeScalar: 3 + _l1FeeScalar: 3, _isDeposit: false }); } From 8e93fcc72d8ce2a58c269c6b2f07cf920b7c5301 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Fri, 9 Aug 2024 11:52:31 -0300 Subject: [PATCH 16/28] fix: reorder marshal funcs --- op-node/rollup/derive/l1_block_info.go | 36 ++++++++----------- packages/contracts-bedrock/src/L2/L1Block.sol | 2 +- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index fe63e2e193b1..95b90daaad68 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -164,33 +164,25 @@ func (info *L1BlockInfo) unmarshalBinaryBedrock(data []byte) error { // | 32 | BlockHash | // | 32 | BatcherHash | // +---------+--------------------------+ - -func (info *L1BlockInfo) marshalBinaryIsthmus() ([]byte, error) { - out, err := marshalBinaryEcotoneOrIsthmus(info, true) +// Marshal Ecotone and Isthmus +func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { + out, err := marshalBinaryWithSignature(info, L1InfoFuncEcotoneBytes4) if err != nil { - return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err) + return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) } return out, nil } - -func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) { - out, err := marshalBinaryEcotoneOrIsthmus(info, false) +func (info *L1BlockInfo) marshalBinaryIsthmus() ([]byte, error) { + out, err := marshalBinaryWithSignature(info, L1InfoFuncIsthmusBytes4) if err != nil { - return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err) + return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err) } return out, nil } - -func marshalBinaryEcotoneOrIsthmus(info *L1BlockInfo, isIsthmus bool) ([]byte, error) { +func marshalBinaryWithSignature(info *L1BlockInfo, signature []byte) ([]byte, error) { w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen)) // Ecotone and Isthmus have the same length - if isIsthmus { - if err := solabi.WriteSignature(w, L1InfoFuncIsthmusBytes4); err != nil { - return nil, err - } - } else { - if err := solabi.WriteSignature(w, L1InfoFuncEcotoneBytes4); err != nil { - return nil, err - } + if err := solabi.WriteSignature(w, signature); err != nil { + return nil, err } if err := binary.Write(w, binary.BigEndian, info.BaseFeeScalar); err != nil { return nil, err @@ -227,13 +219,13 @@ func marshalBinaryEcotoneOrIsthmus(info *L1BlockInfo, isIsthmus bool) ([]byte, e return w.Bytes(), nil } -func (info *L1BlockInfo) unmarshalBinaryIsthmus(data []byte) error { - return unmarshalBinaryWithSignatureAndData(info, L1InfoFuncIsthmusBytes4, data) -} +// Unmarshal Ecotone and Isthmus func (info *L1BlockInfo) unmarshalBinaryEcotone(data []byte) error { return unmarshalBinaryWithSignatureAndData(info, L1InfoFuncEcotoneBytes4, data) } - +func (info *L1BlockInfo) unmarshalBinaryIsthmus(data []byte) error { + return unmarshalBinaryWithSignatureAndData(info, L1InfoFuncIsthmusBytes4, data) +} func unmarshalBinaryWithSignatureAndData(info *L1BlockInfo, signature []byte, data []byte) error { if len(data) != L1InfoEcotoneLen { return fmt.Errorf("data is unexpected length: %d", len(data)) diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index 5c6bff8283a1..bb103e5f869e 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -186,7 +186,7 @@ contract L1Block is ISemver, IGasToken { /// @notice Resets the isDeposit flag. /// Should only be called by the depositor account after the deposits are complete. - function depositsComplete() external { + function depositsComplete() external { // TODO modify spec or modify functionNaming if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); // Set the isDeposit flag to false. From abe646b3afab845f7937575a77d4d22645396f96 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Fri, 9 Aug 2024 12:03:45 -0300 Subject: [PATCH 17/28] feat: add DepositSource to avoid possible collitions --- op-node/rollup/derive/deposit_source.go | 18 ++++++++++++++++++ op-node/rollup/derive/l1_block_info.go | 3 +-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/op-node/rollup/derive/deposit_source.go b/op-node/rollup/derive/deposit_source.go index f7a9730ad026..87bae5ec15f5 100644 --- a/op-node/rollup/derive/deposit_source.go +++ b/op-node/rollup/derive/deposit_source.go @@ -16,6 +16,7 @@ const ( UserDepositSourceDomain = 0 L1InfoDepositSourceDomain = 1 UpgradeDepositSourceDomain = 2 + DepositSourceDomain = 3 ) func (dep *UserDepositSource) SourceHash() common.Hash { @@ -63,3 +64,20 @@ func (dep *UpgradeDepositSource) SourceHash() common.Hash { copy(domainInput[32:], intentHash[:]) return crypto.Keccak256Hash(domainInput[:]) } + +// Used for DepositsComplete/ResetDeposits post-deposits transactions. +// TODO ^ set correct naming +type DepositSource struct { + L1BlockHash common.Hash +} + +func (dep *DepositSource) SourceHash() common.Hash { + var input [32 * 2]byte + copy(input[:32], dep.L1BlockHash[:]) + depositIDHash := crypto.Keccak256Hash(input[:]) + + var domainInput [32 * 2]byte + binary.BigEndian.PutUint64(domainInput[32-8:32], DepositSourceDomain) + copy(domainInput[32:], depositIDHash[:]) + return crypto.Keccak256Hash(domainInput[:]) +} diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index 95b90daaad68..d7709d186667 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -387,9 +387,8 @@ func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNu } func DepositsCompleteDeposit(seqNumber uint64, block eth.BlockInfo) (*types.DepositTx, error) { - source := L1InfoDepositSource{ + source := DepositSource{ L1BlockHash: block.Hash(), - SeqNumber: seqNumber, } out := &types.DepositTx{ SourceHash: source.SourceHash(), From 13096e85e202342b6a69670a7c7e07ce8c6f741e Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Mon, 12 Aug 2024 11:25:12 -0300 Subject: [PATCH 18/28] feat: renames and revert isDeposit on legacy L1 set --- op-node/rollup/derive/attributes.go | 8 +- op-node/rollup/derive/deposit_source.go | 2 +- .../periphery/drippie/DrippieConfig.s.sol | 3 +- packages/contracts-bedrock/src/L2/L1Block.sol | 10 +- .../contracts-bedrock/src/cannon/MIPS2.sol | 96 ++++++++++++------- .../test/L2/GasPriceOracle.t.sol | 3 +- .../contracts-bedrock/test/L2/L1Block.t.sol | 12 +-- .../test/legacy/L1BlockNumber.t.sol | 3 +- .../test/universal/ProxyAdmin.t.sol | 3 +- 9 files changed, 79 insertions(+), 61 deletions(-) diff --git a/op-node/rollup/derive/attributes.go b/op-node/rollup/derive/attributes.go index a64ecf7a714c..cc38a31b9c54 100644 --- a/op-node/rollup/derive/attributes.go +++ b/op-node/rollup/derive/attributes.go @@ -121,19 +121,19 @@ func (ba *FetchingAttributesBuilder) PreparePayloadAttributes(ctx context.Contex return nil, NewCriticalError(fmt.Errorf("failed to create l1InfoTx: %w", err)) } - var postDeposits []hexutil.Bytes + var afterForceIncludeTxs []hexutil.Bytes if ba.rollupCfg.IsInterop(nextL2Time) { depositsCompleteTx, err := DepositsCompleteBytes(seqNumber, l1Info) if err != nil { return nil, NewCriticalError(fmt.Errorf("failed to create depositsCompleteTx: %w", err)) } - postDeposits = append(postDeposits, depositsCompleteTx) + afterForceIncludeTxs = append(afterForceIncludeTxs, depositsCompleteTx) } - txs := make([]hexutil.Bytes, 0, 1+len(depositTxs)+len(postDeposits)+len(upgradeTxs)) + txs := make([]hexutil.Bytes, 0, 1+len(depositTxs)+len(afterForceIncludeTxs)+len(upgradeTxs)) txs = append(txs, l1InfoTx) txs = append(txs, depositTxs...) - txs = append(txs, postDeposits...) + txs = append(txs, afterForceIncludeTxs...) txs = append(txs, upgradeTxs...) var withdrawals *types.Withdrawals diff --git a/op-node/rollup/derive/deposit_source.go b/op-node/rollup/derive/deposit_source.go index 87bae5ec15f5..390207769371 100644 --- a/op-node/rollup/derive/deposit_source.go +++ b/op-node/rollup/derive/deposit_source.go @@ -67,7 +67,7 @@ func (dep *UpgradeDepositSource) SourceHash() common.Hash { // Used for DepositsComplete/ResetDeposits post-deposits transactions. // TODO ^ set correct naming -type DepositSource struct { +type AfterForceIncludeSource struct { L1BlockHash common.Hash } diff --git a/packages/contracts-bedrock/scripts/periphery/drippie/DrippieConfig.s.sol b/packages/contracts-bedrock/scripts/periphery/drippie/DrippieConfig.s.sol index cb7dfa60386b..8a4ab9586875 100644 --- a/packages/contracts-bedrock/scripts/periphery/drippie/DrippieConfig.s.sol +++ b/packages/contracts-bedrock/scripts/periphery/drippie/DrippieConfig.s.sol @@ -116,7 +116,8 @@ contract DrippieConfig is Script, Artifacts { abi.decode(checkparams, (CheckSecrets.Params)); } else if (strcmp(dripcheck, "CheckTrue")) { // No parameters to decode. - } else { + } + else { console.log("ERROR: unknown drip configuration %s", dripcheck); revert UnknownDripCheck(dripcheck); } diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index bb103e5f869e..fac5c0893a6f 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -112,7 +112,6 @@ contract L1Block is ISemver, IGasToken { /// @param _batcherHash Versioned hash to authenticate batcher by. /// @param _l1FeeOverhead L1 fee overhead. /// @param _l1FeeScalar L1 fee scalar. - /// @param _isDeposit isDeposit flag function setL1BlockValues( uint64 _number, uint64 _timestamp, @@ -121,8 +120,7 @@ contract L1Block is ISemver, IGasToken { uint64 _sequenceNumber, bytes32 _batcherHash, uint256 _l1FeeOverhead, - uint256 _l1FeeScalar, - bool _isDeposit + uint256 _l1FeeScalar ) external { @@ -136,9 +134,6 @@ contract L1Block is ISemver, IGasToken { batcherHash = _batcherHash; l1FeeOverhead = _l1FeeOverhead; l1FeeScalar = _l1FeeScalar; - assembly { - sstore(IS_DEPOSIT_SLOT, _isDeposit) - } } /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain. @@ -186,7 +181,7 @@ contract L1Block is ISemver, IGasToken { /// @notice Resets the isDeposit flag. /// Should only be called by the depositor account after the deposits are complete. - function depositsComplete() external { // TODO modify spec or modify functionNaming + function depositsComplete() external { if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); // Set the isDeposit flag to false. @@ -205,5 +200,4 @@ contract L1Block is ISemver, IGasToken { emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol }); } - } diff --git a/packages/contracts-bedrock/src/cannon/MIPS2.sol b/packages/contracts-bedrock/src/cannon/MIPS2.sol index 44015ed16c97..7498a02d1387 100644 --- a/packages/contracts-bedrock/src/cannon/MIPS2.sol +++ b/packages/contracts-bedrock/src/cannon/MIPS2.sol @@ -383,71 +383,101 @@ contract MIPS2 is ISemver { v1 = sys.EBADF; } else if (syscall_no == sys.SYS_CLOCK_GETTIME) { // ignored - } else if (syscall_no == sys.SYS_GET_AFFINITY) { + } + else if (syscall_no == sys.SYS_GET_AFFINITY) { // ignored - } else if (syscall_no == sys.SYS_MADVISE) { + } + else if (syscall_no == sys.SYS_MADVISE) { // ignored - } else if (syscall_no == sys.SYS_RTSIGPROCMASK) { + } + else if (syscall_no == sys.SYS_RTSIGPROCMASK) { // ignored - } else if (syscall_no == sys.SYS_SIGALTSTACK) { + } + else if (syscall_no == sys.SYS_SIGALTSTACK) { // ignored - } else if (syscall_no == sys.SYS_RTSIGACTION) { + } + else if (syscall_no == sys.SYS_RTSIGACTION) { // ignored - } else if (syscall_no == sys.SYS_PRLIMIT64) { + } + else if (syscall_no == sys.SYS_PRLIMIT64) { // ignored - } else if (syscall_no == sys.SYS_CLOSE) { + } + else if (syscall_no == sys.SYS_CLOSE) { // ignored - } else if (syscall_no == sys.SYS_PREAD64) { + } + else if (syscall_no == sys.SYS_PREAD64) { // ignored - } else if (syscall_no == sys.SYS_FSTAT64) { + } + else if (syscall_no == sys.SYS_FSTAT64) { // ignored - } else if (syscall_no == sys.SYS_OPENAT) { + } + else if (syscall_no == sys.SYS_OPENAT) { // ignored - } else if (syscall_no == sys.SYS_READLINK) { + } + else if (syscall_no == sys.SYS_READLINK) { // ignored - } else if (syscall_no == sys.SYS_READLINKAT) { + } + else if (syscall_no == sys.SYS_READLINKAT) { // ignored - } else if (syscall_no == sys.SYS_IOCTL) { + } + else if (syscall_no == sys.SYS_IOCTL) { // ignored - } else if (syscall_no == sys.SYS_EPOLLCREATE1) { + } + else if (syscall_no == sys.SYS_EPOLLCREATE1) { // ignored - } else if (syscall_no == sys.SYS_PIPE2) { + } + else if (syscall_no == sys.SYS_PIPE2) { // ignored - } else if (syscall_no == sys.SYS_EPOLLCTL) { + } + else if (syscall_no == sys.SYS_EPOLLCTL) { // ignored - } else if (syscall_no == sys.SYS_EPOLLPWAIT) { + } + else if (syscall_no == sys.SYS_EPOLLPWAIT) { // ignored - } else if (syscall_no == sys.SYS_GETRANDOM) { + } + else if (syscall_no == sys.SYS_GETRANDOM) { // ignored - } else if (syscall_no == sys.SYS_UNAME) { + } + else if (syscall_no == sys.SYS_UNAME) { // ignored - } else if (syscall_no == sys.SYS_STAT64) { + } + else if (syscall_no == sys.SYS_STAT64) { // ignored - } else if (syscall_no == sys.SYS_GETUID) { + } + else if (syscall_no == sys.SYS_GETUID) { // ignored - } else if (syscall_no == sys.SYS_GETGID) { + } + else if (syscall_no == sys.SYS_GETGID) { // ignored - } else if (syscall_no == sys.SYS_LLSEEK) { + } + else if (syscall_no == sys.SYS_LLSEEK) { // ignored - } else if (syscall_no == sys.SYS_MINCORE) { + } + else if (syscall_no == sys.SYS_MINCORE) { // ignored - } else if (syscall_no == sys.SYS_TGKILL) { + } + else if (syscall_no == sys.SYS_TGKILL) { // ignored - } else if (syscall_no == sys.SYS_SETITIMER) { + } + else if (syscall_no == sys.SYS_SETITIMER) { // ignored - } else if (syscall_no == sys.SYS_TIMERCREATE) { + } + else if (syscall_no == sys.SYS_TIMERCREATE) { // ignored - } else if (syscall_no == sys.SYS_TIMERSETTIME) { + } + else if (syscall_no == sys.SYS_TIMERSETTIME) { // ignored - } else if (syscall_no == sys.SYS_TIMERDELETE) { + } + else if (syscall_no == sys.SYS_TIMERDELETE) { // ignored - } else if (syscall_no == sys.SYS_CLOCKGETTIME) { + } + else if (syscall_no == sys.SYS_CLOCKGETTIME) { // ignored - } else if (syscall_no == sys.SYS_MUNMAP) { + } + else if (syscall_no == sys.SYS_MUNMAP) { // ignored - } else { - revert("MIPS2: unimplemented syscall"); } + else revert("MIPS2: unimplemented syscall"); st.CpuScalars memory cpu = getCpuScalars(thread); sys.handleSyscallUpdates(cpu, thread.registers, v0, v1); diff --git a/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol b/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol index a3e38bab1e2c..21f7b98f666b 100644 --- a/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol +++ b/packages/contracts-bedrock/test/L2/GasPriceOracle.t.sol @@ -52,8 +52,7 @@ contract GasPriceOracleBedrock_Test is GasPriceOracle_Test { _sequenceNumber: sequenceNumber, _batcherHash: batcherHash, _l1FeeOverhead: l1FeeOverhead, - _l1FeeScalar: l1FeeScalar, - _isDeposit: false + _l1FeeScalar: l1FeeScalar }); } diff --git a/packages/contracts-bedrock/test/L2/L1Block.t.sol b/packages/contracts-bedrock/test/L2/L1Block.t.sol index ff714b538ad6..ffa5e0036b55 100644 --- a/packages/contracts-bedrock/test/L2/L1Block.t.sol +++ b/packages/contracts-bedrock/test/L2/L1Block.t.sol @@ -35,13 +35,12 @@ contract L1BlockBedrock_Test is L1BlockTest { uint64 s, bytes32 bt, uint256 fo, - uint256 fs, - bool d + uint256 fs ) external { vm.prank(depositor); - l1Block.setL1BlockValues(n, t, b, h, s, bt, fo, fs, d); + l1Block.setL1BlockValues(n, t, b, h, s, bt, fo, fs); assertEq(l1Block.number(), n); assertEq(l1Block.timestamp(), t); assertEq(l1Block.basefee(), b); @@ -50,7 +49,6 @@ contract L1BlockBedrock_Test is L1BlockTest { assertEq(l1Block.batcherHash(), bt); assertEq(l1Block.l1FeeOverhead(), fo); assertEq(l1Block.l1FeeScalar(), fs); - assertEq(l1Block.isDeposit(), d); } /// @dev Tests that `setL1BlockValues` can set max values. @@ -64,8 +62,7 @@ contract L1BlockBedrock_Test is L1BlockTest { _sequenceNumber: type(uint64).max, _batcherHash: bytes32(type(uint256).max), _l1FeeOverhead: type(uint256).max, - _l1FeeScalar: type(uint256).max, - _isDeposit: true + _l1FeeScalar: type(uint256).max }); } @@ -80,8 +77,7 @@ contract L1BlockBedrock_Test is L1BlockTest { _sequenceNumber: type(uint64).max, _batcherHash: bytes32(type(uint256).max), _l1FeeOverhead: type(uint256).max, - _l1FeeScalar: type(uint256).max, - _isDeposit: true + _l1FeeScalar: type(uint256).max }); } } diff --git a/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol b/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol index 4128d41c0b07..3fad97699232 100644 --- a/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol +++ b/packages/contracts-bedrock/test/legacy/L1BlockNumber.t.sol @@ -32,8 +32,7 @@ contract L1BlockNumberTest is Test { _sequenceNumber: uint64(4), _batcherHash: bytes32(uint256(0)), _l1FeeOverhead: 2, - _l1FeeScalar: 3, - _isDeposit: false + _l1FeeScalar: 3 }); } diff --git a/packages/contracts-bedrock/test/universal/ProxyAdmin.t.sol b/packages/contracts-bedrock/test/universal/ProxyAdmin.t.sol index 4de72c872572..15e806d3e523 100644 --- a/packages/contracts-bedrock/test/universal/ProxyAdmin.t.sol +++ b/packages/contracts-bedrock/test/universal/ProxyAdmin.t.sol @@ -164,9 +164,8 @@ contract ProxyAdmin_Test is Test { admin.getProxyAdmin(_proxy); } else if (proxyType == ProxyAdmin.ProxyType.RESOLVED) { // Just an empty block to show that all cases are covered - } else { - vm.expectRevert("ProxyAdmin: unknown proxy type"); } + else vm.expectRevert("ProxyAdmin: unknown proxy type"); // Call the proxy contract directly to get the admin. // Different proxy types have different interfaces. From 057fc5b13011e41af0664a0f060034b41f6d81bb Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Mon, 12 Aug 2024 11:30:39 -0300 Subject: [PATCH 19/28] feat: rename Deposit for AfterForceInclude --- op-node/rollup/derive/deposit_source.go | 13 ++++++------- op-node/rollup/derive/l1_block_info.go | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/op-node/rollup/derive/deposit_source.go b/op-node/rollup/derive/deposit_source.go index 390207769371..0e663be95abc 100644 --- a/op-node/rollup/derive/deposit_source.go +++ b/op-node/rollup/derive/deposit_source.go @@ -13,10 +13,10 @@ type UserDepositSource struct { } const ( - UserDepositSourceDomain = 0 - L1InfoDepositSourceDomain = 1 - UpgradeDepositSourceDomain = 2 - DepositSourceDomain = 3 + UserDepositSourceDomain = 0 + L1InfoDepositSourceDomain = 1 + UpgradeDepositSourceDomain = 2 + AfterForceIncludeSourceDomain = 3 ) func (dep *UserDepositSource) SourceHash() common.Hash { @@ -66,18 +66,17 @@ func (dep *UpgradeDepositSource) SourceHash() common.Hash { } // Used for DepositsComplete/ResetDeposits post-deposits transactions. -// TODO ^ set correct naming type AfterForceIncludeSource struct { L1BlockHash common.Hash } -func (dep *DepositSource) SourceHash() common.Hash { +func (dep *AfterForceIncludeSource) SourceHash() common.Hash { var input [32 * 2]byte copy(input[:32], dep.L1BlockHash[:]) depositIDHash := crypto.Keccak256Hash(input[:]) var domainInput [32 * 2]byte - binary.BigEndian.PutUint64(domainInput[32-8:32], DepositSourceDomain) + binary.BigEndian.PutUint64(domainInput[32-8:32], AfterForceIncludeSourceDomain) copy(domainInput[32:], depositIDHash[:]) return crypto.Keccak256Hash(domainInput[:]) } diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index d7709d186667..dd4824c00e02 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -387,7 +387,7 @@ func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNu } func DepositsCompleteDeposit(seqNumber uint64, block eth.BlockInfo) (*types.DepositTx, error) { - source := DepositSource{ + source := AfterForceIncludeSource{ L1BlockHash: block.Hash(), } out := &types.DepositTx{ From 5e0d64c57626123b55196a9435c8dd5bd5c60f78 Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Mon, 12 Aug 2024 16:20:53 -0300 Subject: [PATCH 20/28] feat: ban deposits interop (#11451) * fix: contracts compiler errors * feat: create some set l1 block values ithmus test * refactor: not cross l2 inbox error * chore: checkpoint debugging test * fix: contracts compiler errors * feat: create some set l1 block values ithmus test * refactor: not cross l2 inbox error * chore: checkpoint debugging test * refactor: call public function * fix: update is deposit var storage slot * chore: update l2 cross inbox tests with the new is deposit check * feat: add is deposit test * refactor: set is deposit as an unstructured storage slot var * feat: add missing natspec * feat: update semver * fix: stick to natspec standards * chore: enhance natspec * chore: underscore slot constant name * Revert "chore: underscore slot constant name" This reverts commit e162361d6fffae60ad19cfbf8fda673b09da42fc. * chore: enhance l1 block test * refactor: move the new isthmus logic to the l1 block interop contract * refactor: move the isthmus test logic to the l1 block test contract * refactor: functions order * chore: remove unused imports --- packages/contracts-bedrock/src/L2/L1Block.sol | 72 ++++-------- .../src/L2/L1BlockInterop.sol | 38 ++++++ .../contracts-bedrock/test/L2/L1Block.t.sol | 111 +----------------- .../test/L2/L1BlockInterop.t.sol | 110 ++++++++++++++++- 4 files changed, 174 insertions(+), 157 deletions(-) diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index fac5c0893a6f..2476e6862773 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/packages/contracts-bedrock/src/L2/L1Block.sol @@ -4,7 +4,6 @@ pragma solidity 0.8.15; import { ISemver } from "src/universal/ISemver.sol"; import { Constants } from "src/libraries/Constants.sol"; import { GasPayingToken, IGasToken } from "src/libraries/GasPayingToken.sol"; -import { Predeploys } from "src/libraries/Predeploys.sol"; import "src/libraries/L1BlockErrors.sol"; /// @custom:proxied @@ -58,11 +57,6 @@ contract L1Block is ISemver, IGasToken { /// @notice The latest L1 blob base fee. uint256 public blobBaseFee; - /// @notice Storage slot that the isDeposit is stored at. - /// This is a custom slot that is not part of the standard storage layout. - /// keccak256(abi.encode(uint256(keccak256("l1Block.identifier.isDeposit")) - 1)) & ~bytes32(uint256(0xff)) - uint256 internal constant IS_DEPOSIT_SLOT = 0x921bd3a089295c6e5540e8fba8195448d253efd6f2e3e495b499b627dc36a300; - /// @custom:semver 1.5.1-beta.1 function version() public pure virtual returns (string memory) { return "1.5.1-beta.1"; @@ -93,15 +87,6 @@ contract L1Block is ISemver, IGasToken { return token != Constants.ETHER; } - /// @notice Returns whether the call was triggered from a a deposit or not. - /// @notice This function is only callable by the CrossL2Inbox contract. - function isDeposit() external view returns (bool isDeposit_) { - if (msg.sender != Predeploys.CROSS_L2_INBOX) revert NotCrossL2Inbox(); - assembly { - isDeposit_ := sload(IS_DEPOSIT_SLOT) - } - } - /// @custom:legacy /// @notice Updates the L1 block values. /// @param _number L1 blocknumber. @@ -136,16 +121,31 @@ contract L1Block is ISemver, IGasToken { l1FeeScalar = _l1FeeScalar; } - /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain. - /// It updates the L1 block values through the `setL1BlockValuesEcotone` function. - /// It forwards the calldata to the internally-used `setL1BlockValuesEcotone` function. - function setL1BlockValuesIsthmus() external { - // Set the isDeposit flag to true. - assembly { - sstore(IS_DEPOSIT_SLOT, 1) - } + /// @notice Updates the L1 block values for an Ecotone upgraded chain. + /// Params are packed and passed in as raw msg.data instead of ABI to reduce calldata size. + /// Params are expected to be in the following order: + /// 1. _baseFeeScalar L1 base fee scalar + /// 2. _blobBaseFeeScalar L1 blob base fee scalar + /// 3. _sequenceNumber Number of L2 blocks since epoch start. + /// 4. _timestamp L1 timestamp. + /// 5. _number L1 blocknumber. + /// 6. _basefee L1 base fee. + /// 7. _blobBaseFee L1 blob base fee. + /// 8. _hash L1 blockhash. + /// 9. _batcherHash Versioned hash to authenticate batcher by. + function setL1BlockValuesEcotone() public { + _setL1BlockValuesEcotone(); + } + + /// @notice Sets the gas paying token for the L2 system. Can only be called by the special + /// depositor account. This function is not called on every L2 block but instead + /// only called by specially crafted L1 deposit transactions. + function setGasPayingToken(address _token, uint8 _decimals, bytes32 _name, bytes32 _symbol) external { + if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); - setL1BlockValuesEcotone(); + GasPayingToken.set({ _token: _token, _decimals: _decimals, _name: _name, _symbol: _symbol }); + + emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol }); } /// @notice Updates the L1 block values for an Ecotone upgraded chain. @@ -160,7 +160,7 @@ contract L1Block is ISemver, IGasToken { /// 7. _blobBaseFee L1 blob base fee. /// 8. _hash L1 blockhash. /// 9. _batcherHash Versioned hash to authenticate batcher by. - function setL1BlockValuesEcotone() public { + function _setL1BlockValuesEcotone() internal { address depositor = DEPOSITOR_ACCOUNT(); assembly { // Revert if the caller is not the depositor account. @@ -178,26 +178,4 @@ contract L1Block is ISemver, IGasToken { sstore(batcherHash.slot, calldataload(132)) // bytes32 } } - - /// @notice Resets the isDeposit flag. - /// Should only be called by the depositor account after the deposits are complete. - function depositsComplete() external { - if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); - - // Set the isDeposit flag to false. - assembly { - sstore(IS_DEPOSIT_SLOT, 0) - } - } - - /// @notice Sets the gas paying token for the L2 system. Can only be called by the special - /// depositor account. This function is not called on every L2 block but instead - /// only called by specially crafted L1 deposit transactions. - function setGasPayingToken(address _token, uint8 _decimals, bytes32 _name, bytes32 _symbol) external { - if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); - - GasPayingToken.set({ _token: _token, _decimals: _decimals, _name: _name, _symbol: _symbol }); - - emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol }); - } } diff --git a/packages/contracts-bedrock/src/L2/L1BlockInterop.sol b/packages/contracts-bedrock/src/L2/L1BlockInterop.sol index 8dbf986fb7e6..8777ccbcded7 100644 --- a/packages/contracts-bedrock/src/L2/L1BlockInterop.sol +++ b/packages/contracts-bedrock/src/L2/L1BlockInterop.sol @@ -5,6 +5,7 @@ import { L1Block } from "src/L2/L1Block.sol"; import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import { GasPayingToken } from "src/libraries/GasPayingToken.sol"; import { StaticConfig } from "src/libraries/StaticConfig.sol"; +import { Predeploys } from "src/libraries/Predeploys.sol"; import "src/libraries/L1BlockErrors.sol"; /// @notice Enum representing different types of configurations that can be set on L1BlockInterop. @@ -33,11 +34,25 @@ contract L1BlockInterop is L1Block { /// @notice The interop dependency set, containing the chain IDs in it. EnumerableSet.UintSet dependencySet; + /// @notice Storage slot that the isDeposit is stored at. + /// This is a custom slot that is not part of the standard storage layout. + /// keccak256(abi.encode(uint256(keccak256("l1Block.identifier.isDeposit")) - 1)) & ~bytes32(uint256(0xff)) + uint256 internal constant IS_DEPOSIT_SLOT = 0x921bd3a089295c6e5540e8fba8195448d253efd6f2e3e495b499b627dc36a300; + /// @custom:semver +interop function version() public pure override returns (string memory) { return string.concat(super.version(), "+interop"); } + /// @notice Returns whether the call was triggered from a a deposit or not. + /// @notice This function is only callable by the CrossL2Inbox contract. + function isDeposit() external view returns (bool isDeposit_) { + if (msg.sender != Predeploys.CROSS_L2_INBOX) revert NotCrossL2Inbox(); + assembly { + isDeposit_ := sload(IS_DEPOSIT_SLOT) + } + } + /// @notice Returns true if a chain ID is in the interop dependency set and false otherwise. /// The chain's chain ID is always considered to be in the dependency set. /// @param _chainId The chain ID to check. @@ -52,6 +67,29 @@ contract L1BlockInterop is L1Block { return uint8(dependencySet.length()); } + /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain. + /// It updates the L1 block values through the `setL1BlockValuesEcotone` function. + /// It forwards the calldata to the internally-used `setL1BlockValuesEcotone` function. + function setL1BlockValuesIsthmus() external { + // Set the isDeposit flag to true. + assembly { + sstore(IS_DEPOSIT_SLOT, 1) + } + + _setL1BlockValuesEcotone(); + } + + /// @notice Resets the isDeposit flag. + /// Should only be called by the depositor account after the deposits are complete. + function depositsComplete() external { + if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); + + // Set the isDeposit flag to false. + assembly { + sstore(IS_DEPOSIT_SLOT, 0) + } + } + /// @notice Sets static configuration options for the L2 system. Can only be called by the special /// depositor account. /// @param _type The type of configuration to set. diff --git a/packages/contracts-bedrock/test/L2/L1Block.t.sol b/packages/contracts-bedrock/test/L2/L1Block.t.sol index ffa5e0036b55..1c004a6d4295 100644 --- a/packages/contracts-bedrock/test/L2/L1Block.t.sol +++ b/packages/contracts-bedrock/test/L2/L1Block.t.sol @@ -8,10 +8,9 @@ import { CommonTest } from "test/setup/CommonTest.sol"; import { Encoding } from "src/libraries/Encoding.sol"; import { Constants } from "src/libraries/Constants.sol"; -// Target contract +// Target contract dependencies import { L1Block } from "src/L2/L1Block.sol"; -import { NotDepositor, NotCrossL2Inbox } from "src/libraries/L1BlockErrors.sol"; -import { Predeploys } from "src/libraries/Predeploys.sol"; +import "src/libraries/L1BlockErrors.sol"; contract L1BlockTest is CommonTest { address depositor; @@ -82,59 +81,6 @@ contract L1BlockBedrock_Test is L1BlockTest { } } -contract L1BlockSetL1BlockValuesIsthmus_Test is L1BlockTest { - /// @dev Tests that `setL1BlockValuesIsthmus` reverts if sender address is not the depositor - function test_setL1BlockValuesIsthmus_notDepositor_reverts(address _caller) external { - vm.assume(_caller != depositor); - vm.prank(_caller); - vm.expectRevert(NotDepositor.selector); - l1Block.setL1BlockValuesIsthmus(); - } - - /// @dev Tests that `setL1BlockValuesIsthmus` succeeds if sender address is the depositor - function test_setL1BlockValuesIsthmus_succeeds( - uint32 baseFeeScalar, - uint32 blobBaseFeeScalar, - uint64 sequenceNumber, - uint64 timestamp, - uint64 number, - uint256 baseFee, - uint256 blobBaseFee, - bytes32 hash, - bytes32 batcherHash - ) - external - { - // Ensure the `isDepositTransaction` flag is false before calling `setL1BlockValuesIsthmus` - vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(l1Block.isDeposit(), false); - - bytes memory setValuesEcotoneCalldata = abi.encodePacked( - baseFeeScalar, blobBaseFeeScalar, sequenceNumber, timestamp, number, baseFee, blobBaseFee, hash, batcherHash - ); - - vm.prank(depositor); - (bool success,) = - address(l1Block).call(abi.encodePacked(L1Block.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata)); - assertTrue(success, "function call failed"); - - // Assert that the `isDepositTransaction` flag was properly set to true - vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(l1Block.isDeposit(), true); - - // Assert `setL1BlockValuesEcotone` was properly called, forwarding the calldata to it - assertEq(l1Block.baseFeeScalar(), baseFeeScalar, "1"); - assertEq(l1Block.blobBaseFeeScalar(), blobBaseFeeScalar, "2"); - assertEq(l1Block.sequenceNumber(), sequenceNumber, "3"); - assertEq(l1Block.timestamp(), timestamp, "4"); - assertEq(l1Block.number(), number, "5"); - assertEq(l1Block.basefee(), baseFee, "6"); - assertEq(l1Block.blobBaseFee(), blobBaseFee, "7"); - assertEq(l1Block.hash(), hash, "8"); - assertEq(l1Block.batcherHash(), batcherHash, "9"); - } -} - contract L1BlockEcotone_Test is L1BlockTest { /// @dev Tests that setL1BlockValuesEcotone updates the values appropriately. function testFuzz_setL1BlockValuesEcotone_succeeds( @@ -222,35 +168,6 @@ contract L1BlockEcotone_Test is L1BlockTest { } } -contract L1BlockDepositsComplete_Test is L1BlockTest { - // @dev Tests that `depositsComplete` reverts if the caller is not the depositor. - function test_deposits_is_depositor_reverts(address _caller) external { - vm.assume(_caller != depositor); - vm.expectRevert(NotDepositor.selector); - l1Block.depositsComplete(); - } - - // @dev Tests that `depositsComplete` succeeds if the caller is the depositor. - function test_depositsComplete_succeeds() external { - // Set the `isDeposit` flag to true - vm.prank(depositor); - l1Block.setL1BlockValuesIsthmus(); - - // Assert that the `isDeposit` flag was properly set to true - vm.prank(Predeploys.CROSS_L2_INBOX); - assertTrue(l1Block.isDeposit()); - - // Call `depositsComplete` - vm.prank(depositor); - l1Block.depositsComplete(); - - // Assert that the `isDeposit` flag was properly set to false - /// @dev Assuming that `isDeposit()` wil return the proper value. That function is tested as well - vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(l1Block.isDeposit(), false); - } -} - contract L1BlockCustomGasToken_Test is L1BlockTest { function testFuzz_setGasPayingToken_succeeds( address _token, @@ -288,27 +205,3 @@ contract L1BlockCustomGasToken_Test is L1BlockTest { l1Block.setGasPayingToken(address(this), 18, "Test", "TST"); } } - -contract L1BlockIsDeposit_Test is L1BlockTest { - /// @dev Tests that `isDeposit` reverts if the caller is not the cross L2 inbox. - function test_isDeposit_notCrossL2Inbox_reverts(address _caller) external { - vm.assume(_caller != Predeploys.CROSS_L2_INBOX); - vm.expectRevert(NotCrossL2Inbox.selector); - l1Block.isDeposit(); - } - - /// @dev Tests that `isDeposit` always returns the correct value. - function test_isDeposit_succeeds() external { - // Assert is false if the value is not updated - vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(l1Block.isDeposit(), false); - - /// @dev Assuming that `setL1BlockValuesIsthmus` will set the proper value. That function is tested as well - vm.prank(depositor); - l1Block.setL1BlockValuesIsthmus(); - - // Assert is true if the value is updated - vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(l1Block.isDeposit(), true); - } -} diff --git a/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol b/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol index 159021e77dcc..7e96670de1fe 100644 --- a/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol +++ b/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol @@ -9,6 +9,7 @@ import { StaticConfig } from "src/libraries/StaticConfig.sol"; // Target contract dependencies import { L1BlockInterop, ConfigType } from "src/L2/L1BlockInterop.sol"; +import { Predeploys } from "src/libraries/Predeploys.sol"; import "src/libraries/L1BlockErrors.sol"; contract L1BlockInteropTest is CommonTest { @@ -17,7 +18,7 @@ contract L1BlockInteropTest is CommonTest { event DependencyRemoved(uint256 indexed chainId); modifier prankDepositor() { - vm.startPrank(l1Block.DEPOSITOR_ACCOUNT()); + vm.startPrank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); _; vm.stopPrank(); } @@ -202,3 +203,110 @@ contract L1BlockInteropTest is CommonTest { return L1BlockInterop(address(l1Block)); } } + +contract L1BlockInteropIsDeposit_Test is L1BlockInteropTest { + /// @dev Tests that `isDeposit` reverts if the caller is not the cross L2 inbox. + function test_isDeposit_notCrossL2Inbox_reverts(address _caller) external { + vm.assume(_caller != Predeploys.CROSS_L2_INBOX); + vm.expectRevert(NotCrossL2Inbox.selector); + _l1BlockInterop().isDeposit(); + } + + /// @dev Tests that `isDeposit` always returns the correct value. + function test_isDeposit_succeeds() external { + // Assert is false if the value is not updated + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockInterop().isDeposit(), false); + + /// @dev Assuming that `setL1BlockValuesIsthmus` will set the proper value. That function is tested as well + vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + _l1BlockInterop().setL1BlockValuesIsthmus(); + + // Assert is true if the value is updated + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockInterop().isDeposit(), true); + } +} + +contract L1BlockInteropSetL1BlockValuesIsthmus_Test is L1BlockInteropTest { + /// @dev Tests that `setL1BlockValuesIsthmus` reverts if sender address is not the depositor + function test_setL1BlockValuesIsthmus_notDepositor_reverts(address _caller) external { + vm.assume(_caller != _l1BlockInterop().DEPOSITOR_ACCOUNT()); + vm.prank(_caller); + vm.expectRevert(NotDepositor.selector); + _l1BlockInterop().setL1BlockValuesIsthmus(); + } + + /// @dev Tests that `setL1BlockValuesIsthmus` succeeds if sender address is the depositor + function test_setL1BlockValuesIsthmus_succeeds( + uint32 baseFeeScalar, + uint32 blobBaseFeeScalar, + uint64 sequenceNumber, + uint64 timestamp, + uint64 number, + uint256 baseFee, + uint256 blobBaseFee, + bytes32 hash, + bytes32 batcherHash + ) + external + { + // Ensure the `isDepositTransaction` flag is false before calling `setL1BlockValuesIsthmus` + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockInterop().isDeposit(), false); + + bytes memory setValuesEcotoneCalldata = abi.encodePacked( + baseFeeScalar, blobBaseFeeScalar, sequenceNumber, timestamp, number, baseFee, blobBaseFee, hash, batcherHash + ); + + vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + (bool success,) = address(l1Block).call( + abi.encodePacked(L1BlockInterop.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata) + ); + assertTrue(success, "function call failed"); + + // Assert that the `isDepositTransaction` flag was properly set to true + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockInterop().isDeposit(), true); + + // Assert `setL1BlockValuesEcotone` was properly called, forwarding the calldata to it + assertEq(_l1BlockInterop().baseFeeScalar(), baseFeeScalar, "base fee scalar not properly set"); + assertEq(_l1BlockInterop().blobBaseFeeScalar(), blobBaseFeeScalar, "blob base fee scalar not properly set"); + assertEq(_l1BlockInterop().sequenceNumber(), sequenceNumber, "sequence number not properly set"); + assertEq(_l1BlockInterop().timestamp(), timestamp, "timestamp not properly set"); + assertEq(_l1BlockInterop().number(), number, "number not properly set"); + assertEq(_l1BlockInterop().basefee(), baseFee, "base fee not properly set"); + assertEq(_l1BlockInterop().blobBaseFee(), blobBaseFee, "blob base fee not properly set"); + assertEq(_l1BlockInterop().hash(), hash, "hash not properly set"); + assertEq(_l1BlockInterop().batcherHash(), batcherHash, "batcher hash not properly set"); + } +} + +contract L1BlockDepositsComplete_Test is L1BlockInteropTest { + // @dev Tests that `depositsComplete` reverts if the caller is not the depositor. + function test_deposits_is_depositor_reverts(address _caller) external { + vm.assume(_caller != _l1BlockInterop().DEPOSITOR_ACCOUNT()); + vm.expectRevert(NotDepositor.selector); + _l1BlockInterop().depositsComplete(); + } + + // @dev Tests that `depositsComplete` succeeds if the caller is the depositor. + function test_depositsComplete_succeeds() external { + // Set the `isDeposit` flag to true + vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + _l1BlockInterop().setL1BlockValuesIsthmus(); + + // Assert that the `isDeposit` flag was properly set to true + vm.prank(Predeploys.CROSS_L2_INBOX); + assertTrue(_l1BlockInterop().isDeposit()); + + // Call `depositsComplete` + vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + _l1BlockInterop().depositsComplete(); + + // Assert that the `isDeposit` flag was properly set to false + /// @dev Assuming that `isDeposit()` wil return the proper value. That function is tested as well + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockInterop().isDeposit(), false); + } +} From 2c370f735bdc44c087bfb54827d256cbbe17ee54 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Mon, 12 Aug 2024 17:34:14 -0300 Subject: [PATCH 21/28] fix: typo --- op-node/rollup/derive/l1_block_info.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index dd4824c00e02..7e5213af2795 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -20,7 +20,7 @@ import ( const ( L1InfoFuncBedrockSignature = "setL1BlockValues(uint64,uint64,uint256,bytes32,uint64,bytes32,uint256,uint256)" L1InfoFuncEcotoneSignature = "setL1BlockValuesEcotone()" - L1InfoFuncIsthmusSignature = "setL2BlockValuesIsthmus()" + L1InfoFuncIsthmusSignature = "setL1BlockValuesIsthmus()" DepositsCompleteSignature = "depositsComplete()" L1InfoArguments = 8 L1InfoBedrockLen = 4 + 32*L1InfoArguments From da494fdf6398283a14f9d60d83973f7bda42da64 Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Tue, 13 Aug 2024 12:30:41 -0300 Subject: [PATCH 22/28] feat: ban deposits interop (#11454) * fix: contracts compiler errors * feat: create some set l1 block values ithmus test * refactor: not cross l2 inbox error * chore: checkpoint debugging test * fix: contracts compiler errors * feat: create some set l1 block values ithmus test * refactor: not cross l2 inbox error * chore: checkpoint debugging test * refactor: call public function * fix: update is deposit var storage slot * chore: update l2 cross inbox tests with the new is deposit check * feat: add is deposit test * refactor: set is deposit as an unstructured storage slot var * feat: add missing natspec * feat: update semver * fix: stick to natspec standards * chore: enhance natspec * chore: underscore slot constant name * Revert "chore: underscore slot constant name" This reverts commit e162361d6fffae60ad19cfbf8fda673b09da42fc. * chore: enhance l1 block test * refactor: move the new isthmus logic to the l1 block interop contract * refactor: move the isthmus test logic to the l1 block test contract * refactor: functions order * chore: remove unused imports * feat: benchmark set values ecotone and isthmus functions --- packages/contracts-bedrock/.gas-snapshot | 2 ++ .../test/BenchmarkTest.t.sol | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/packages/contracts-bedrock/.gas-snapshot b/packages/contracts-bedrock/.gas-snapshot index b3ea3b88545e..d8e6eccaafb5 100644 --- a/packages/contracts-bedrock/.gas-snapshot +++ b/packages/contracts-bedrock/.gas-snapshot @@ -1,3 +1,5 @@ ++GasBenchMark_L1BlockInterop:test_setL1BlockValuesEcotone_benchmark() (gas: 18858) ++GasBenchMark_L1BlockInterop:test_setL1BlockValuesIsthmus_benchmark() (gas: 40967) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369356) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967496) GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564483) diff --git a/packages/contracts-bedrock/test/BenchmarkTest.t.sol b/packages/contracts-bedrock/test/BenchmarkTest.t.sol index 4722107a314f..babe3add5a50 100644 --- a/packages/contracts-bedrock/test/BenchmarkTest.t.sol +++ b/packages/contracts-bedrock/test/BenchmarkTest.t.sol @@ -9,6 +9,7 @@ import { Bridge_Initializer } from "test/setup/Bridge_Initializer.sol"; import { CrossDomainMessenger } from "src/universal/CrossDomainMessenger.sol"; import { ResourceMetering } from "src/L1/ResourceMetering.sol"; import { Types } from "src/libraries/Types.sol"; +import { L1BlockInterop } from "src/L2/L1BlockInterop.sol"; // Free function for setting the prevBaseFee param in the OptimismPortal. function setPrevBaseFee(Vm _vm, address _op, uint128 _prevBaseFee) { @@ -209,3 +210,35 @@ contract GasBenchMark_L2OutputOracle is CommonTest { l2OutputOracle.proposeL2Output(nonZeroHash, nextBlockNumber, 0, 0); } } + +contract GasBenchMark_L1BlockInterop is CommonTest { + L1BlockInterop l1BlockInterop; + address depositor; + bytes setValuesEcotoneCalldata; + + function setUp() public override { + super.setUp(); + depositor = l1Block.DEPOSITOR_ACCOUNT(); + l1BlockInterop = new L1BlockInterop(); + setValuesEcotoneCalldata = abi.encodePacked( + type(uint32).max, + type(uint32).max, + type(uint64).max, + type(uint64).max, + type(uint64).max, + type(uint256).max, + type(uint256).max, + keccak256(abi.encode(1)), + bytes32(type(uint256).max) + ); + vm.startPrank(depositor); + } + + function test_setL1BlockValuesEcotone_benchmark() external { + address(l1BlockInterop).call(abi.encodeWithSelector(l1BlockInterop.setL1BlockValuesEcotone.selector)); + } + + function test_setL1BlockValuesIsthmus_benchmark() external { + address(l1BlockInterop).call(abi.encodeWithSelector(l1BlockInterop.setL1BlockValuesIsthmus.selector)); + } +} From 09c549c7ae4f76a6e5044023671024b2349464d3 Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Wed, 14 Aug 2024 18:59:36 -0300 Subject: [PATCH 23/28] feat: ban deposits interop (#11481) * fix: contracts compiler errors * feat: create some set l1 block values ithmus test * refactor: not cross l2 inbox error * chore: checkpoint debugging test * fix: contracts compiler errors * feat: create some set l1 block values ithmus test * refactor: not cross l2 inbox error * chore: checkpoint debugging test * refactor: call public function * fix: update is deposit var storage slot * chore: update l2 cross inbox tests with the new is deposit check * feat: add is deposit test * refactor: set is deposit as an unstructured storage slot var * feat: add missing natspec * feat: update semver * fix: stick to natspec standards * chore: enhance natspec * chore: underscore slot constant name * Revert "chore: underscore slot constant name" This reverts commit e162361d6fffae60ad19cfbf8fda673b09da42fc. * chore: enhance l1 block test * refactor: move the new isthmus logic to the l1 block interop contract * refactor: move the isthmus test logic to the l1 block test contract * refactor: functions order * chore: remove unused imports * feat: benchmark set values ecotone and isthmus functions * refactor: rename from l1 block interop to isthmus * feat: add benchmark test for deposits complete function * chore: update gas snapshot * feat: add warm and non warm benchmarks * chore: delete unused pkg --- packages/contracts-bedrock/.gas-snapshot | 8 +- .../contracts-bedrock/scripts/L2Genesis.s.sol | 2 +- .../src/L1/OptimismPortalInterop.sol | 4 +- .../src/L1/SystemConfigInterop.sol | 2 +- .../src/L2/L1BlockInterop.sol | 6 +- .../src/L2/L1BlockIsthmus.sol | 142 ++++++++ .../test/BenchmarkTest.t.sol | 83 ++++- .../test/L1/OptimismPortalInterop.t.sol | 8 +- .../test/L1/SystemConfigInterop.t.sol | 2 +- .../test/L2/L1BlockInterop.t.sol | 122 +++---- .../test/L2/L1BlockIsthmus.t.sol | 312 ++++++++++++++++++ 11 files changed, 609 insertions(+), 82 deletions(-) create mode 100644 packages/contracts-bedrock/src/L2/L1BlockIsthmus.sol create mode 100644 packages/contracts-bedrock/test/L2/L1BlockIsthmus.t.sol diff --git a/packages/contracts-bedrock/.gas-snapshot b/packages/contracts-bedrock/.gas-snapshot index d8e6eccaafb5..c34ed57a1696 100644 --- a/packages/contracts-bedrock/.gas-snapshot +++ b/packages/contracts-bedrock/.gas-snapshot @@ -1,5 +1,9 @@ -+GasBenchMark_L1BlockInterop:test_setL1BlockValuesEcotone_benchmark() (gas: 18858) -+GasBenchMark_L1BlockInterop:test_setL1BlockValuesIsthmus_benchmark() (gas: 40967) +GasBenchMark_L1BlockIsthmus_DepositsComplete:test_depositsComplete_benchmark() (gas: 7768) +GasBenchMark_L1BlockIsthmus_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5768) +GasBenchMark_L1BlockIsthmus_SetValuesIsthmus:test_setL1BlockValuesIsthmus_benchmark() (gas: 174050) +GasBenchMark_L1BlockIsthmus_SetValuesIsthmus_Warm:test_setL1BlockValuesIsthmus_benchmark() (gas: 5185) +GasBenchMark_L1Block_SetValuesEcotone:test_setL1BlockValuesEcotone_benchmark() (gas: 159033) +GasBenchMark_L1Block_SetValuesEcotone_Warm:test_setL1BlockValuesEcotone_benchmark() (gas: 7539) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369356) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967496) GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564483) diff --git a/packages/contracts-bedrock/scripts/L2Genesis.s.sol b/packages/contracts-bedrock/scripts/L2Genesis.s.sol index f68dd6232882..e3c1f43b6f08 100644 --- a/packages/contracts-bedrock/scripts/L2Genesis.s.sol +++ b/packages/contracts-bedrock/scripts/L2Genesis.s.sol @@ -346,7 +346,7 @@ contract L2Genesis is Deployer { /// @notice This predeploy is following the safety invariant #1. function setL1Block() public { if (cfg.useInterop()) { - string memory cname = "L1BlockInterop"; + string memory cname = "L1BlockIsthmus"; address impl = Predeploys.predeployToCodeNamespace(Predeploys.L1_BLOCK_ATTRIBUTES); console.log("Setting %s implementation at: %s", cname, impl); vm.etch(impl, vm.getDeployedCode(string.concat(cname, ".sol:", cname))); diff --git a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol index 630ae563722b..9523224d861f 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.15; import { OptimismPortal } from "src/L1/OptimismPortal.sol"; -import { L1BlockInterop, ConfigType } from "src/L2/L1BlockInterop.sol"; +import { L1BlockIsthmus, ConfigType } from "src/L2/L1BlockIsthmus.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; import { Constants } from "src/libraries/Constants.sol"; @@ -40,7 +40,7 @@ contract OptimismPortalInterop is OptimismPortal { uint256(0), // value uint64(SYSTEM_DEPOSIT_GAS_LIMIT), // gasLimit false, // isCreation, - abi.encodeCall(L1BlockInterop.setConfig, (_type, _value)) + abi.encodeCall(L1BlockIsthmus.setConfig, (_type, _value)) ) ); } diff --git a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol index d129520b4843..22f7e0baf779 100644 --- a/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol +++ b/packages/contracts-bedrock/src/L1/SystemConfigInterop.sol @@ -6,7 +6,7 @@ import { OptimismPortalInterop as OptimismPortal } from "src/L1/OptimismPortalIn import { GasPayingToken } from "src/libraries/GasPayingToken.sol"; import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import { SystemConfig } from "src/L1/SystemConfig.sol"; -import { ConfigType } from "src/L2/L1BlockInterop.sol"; +import { ConfigType } from "src/L2/L1BlockIsthmus.sol"; import { StaticConfig } from "src/libraries/StaticConfig.sol"; /// @title SystemConfigInterop diff --git a/packages/contracts-bedrock/src/L2/L1BlockInterop.sol b/packages/contracts-bedrock/src/L2/L1BlockInterop.sol index 8777ccbcded7..9f8d12cebf38 100644 --- a/packages/contracts-bedrock/src/L2/L1BlockInterop.sol +++ b/packages/contracts-bedrock/src/L2/L1BlockInterop.sol @@ -8,7 +8,7 @@ import { StaticConfig } from "src/libraries/StaticConfig.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; import "src/libraries/L1BlockErrors.sol"; -/// @notice Enum representing different types of configurations that can be set on L1BlockInterop. +/// @notice Enum representing different types of configurations that can be set on L1BlockIsthmus. /// @custom:value SET_GAS_PAYING_TOKEN Represents the config type for setting the gas paying token. /// @custom:value ADD_DEPENDENCY Represents the config type for adding a chain to the interop dependency set. /// @custom:value REMOVE_DEPENDENCY Represents the config type for removing a chain from the interop dependency set. @@ -20,9 +20,9 @@ enum ConfigType { /// @custom:proxied /// @custom:predeploy 0x4200000000000000000000000000000000000015 -/// @title L1BlockInterop +/// @title L1BlockIsthmus /// @notice Interop extenstions of L1Block. -contract L1BlockInterop is L1Block { +contract L1BlockIsthmus is L1Block { using EnumerableSet for EnumerableSet.UintSet; /// @notice Event emitted when a new dependency is added to the interop dependency set. diff --git a/packages/contracts-bedrock/src/L2/L1BlockIsthmus.sol b/packages/contracts-bedrock/src/L2/L1BlockIsthmus.sol new file mode 100644 index 000000000000..9f8d12cebf38 --- /dev/null +++ b/packages/contracts-bedrock/src/L2/L1BlockIsthmus.sol @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import { L1Block } from "src/L2/L1Block.sol"; +import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import { GasPayingToken } from "src/libraries/GasPayingToken.sol"; +import { StaticConfig } from "src/libraries/StaticConfig.sol"; +import { Predeploys } from "src/libraries/Predeploys.sol"; +import "src/libraries/L1BlockErrors.sol"; + +/// @notice Enum representing different types of configurations that can be set on L1BlockIsthmus. +/// @custom:value SET_GAS_PAYING_TOKEN Represents the config type for setting the gas paying token. +/// @custom:value ADD_DEPENDENCY Represents the config type for adding a chain to the interop dependency set. +/// @custom:value REMOVE_DEPENDENCY Represents the config type for removing a chain from the interop dependency set. +enum ConfigType { + SET_GAS_PAYING_TOKEN, + ADD_DEPENDENCY, + REMOVE_DEPENDENCY +} + +/// @custom:proxied +/// @custom:predeploy 0x4200000000000000000000000000000000000015 +/// @title L1BlockIsthmus +/// @notice Interop extenstions of L1Block. +contract L1BlockIsthmus is L1Block { + using EnumerableSet for EnumerableSet.UintSet; + + /// @notice Event emitted when a new dependency is added to the interop dependency set. + event DependencyAdded(uint256 indexed chainId); + + /// @notice Event emitted when a dependency is removed from the interop dependency set. + event DependencyRemoved(uint256 indexed chainId); + + /// @notice The interop dependency set, containing the chain IDs in it. + EnumerableSet.UintSet dependencySet; + + /// @notice Storage slot that the isDeposit is stored at. + /// This is a custom slot that is not part of the standard storage layout. + /// keccak256(abi.encode(uint256(keccak256("l1Block.identifier.isDeposit")) - 1)) & ~bytes32(uint256(0xff)) + uint256 internal constant IS_DEPOSIT_SLOT = 0x921bd3a089295c6e5540e8fba8195448d253efd6f2e3e495b499b627dc36a300; + + /// @custom:semver +interop + function version() public pure override returns (string memory) { + return string.concat(super.version(), "+interop"); + } + + /// @notice Returns whether the call was triggered from a a deposit or not. + /// @notice This function is only callable by the CrossL2Inbox contract. + function isDeposit() external view returns (bool isDeposit_) { + if (msg.sender != Predeploys.CROSS_L2_INBOX) revert NotCrossL2Inbox(); + assembly { + isDeposit_ := sload(IS_DEPOSIT_SLOT) + } + } + + /// @notice Returns true if a chain ID is in the interop dependency set and false otherwise. + /// The chain's chain ID is always considered to be in the dependency set. + /// @param _chainId The chain ID to check. + /// @return True if the chain ID to check is in the interop dependency set. False otherwise. + function isInDependencySet(uint256 _chainId) public view returns (bool) { + return _chainId == block.chainid || dependencySet.contains(_chainId); + } + + /// @notice Returns the size of the interop dependency set. + /// @return The size of the interop dependency set. + function dependencySetSize() external view returns (uint8) { + return uint8(dependencySet.length()); + } + + /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain. + /// It updates the L1 block values through the `setL1BlockValuesEcotone` function. + /// It forwards the calldata to the internally-used `setL1BlockValuesEcotone` function. + function setL1BlockValuesIsthmus() external { + // Set the isDeposit flag to true. + assembly { + sstore(IS_DEPOSIT_SLOT, 1) + } + + _setL1BlockValuesEcotone(); + } + + /// @notice Resets the isDeposit flag. + /// Should only be called by the depositor account after the deposits are complete. + function depositsComplete() external { + if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); + + // Set the isDeposit flag to false. + assembly { + sstore(IS_DEPOSIT_SLOT, 0) + } + } + + /// @notice Sets static configuration options for the L2 system. Can only be called by the special + /// depositor account. + /// @param _type The type of configuration to set. + /// @param _value The encoded value with which to set the configuration. + function setConfig(ConfigType _type, bytes calldata _value) external { + if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); + + if (_type == ConfigType.SET_GAS_PAYING_TOKEN) { + _setGasPayingToken(_value); + } else if (_type == ConfigType.ADD_DEPENDENCY) { + _addDependency(_value); + } else if (_type == ConfigType.REMOVE_DEPENDENCY) { + _removeDependency(_value); + } + } + + /// @notice Internal method to set the gas paying token. + /// @param _value The encoded value with which to set the gas paying token. + function _setGasPayingToken(bytes calldata _value) internal { + (address token, uint8 decimals, bytes32 name, bytes32 symbol) = StaticConfig.decodeSetGasPayingToken(_value); + + GasPayingToken.set({ _token: token, _decimals: decimals, _name: name, _symbol: symbol }); + + emit GasPayingTokenSet({ token: token, decimals: decimals, name: name, symbol: symbol }); + } + + /// @notice Internal method to add a dependency to the interop dependency set. + /// @param _value The encoded value with which to add the dependency. + function _addDependency(bytes calldata _value) internal { + uint256 chainId = StaticConfig.decodeAddDependency(_value); + + if (dependencySet.length() == type(uint8).max) revert DependencySetSizeTooLarge(); + + if (chainId == block.chainid || !dependencySet.add(chainId)) revert AlreadyDependency(); + + emit DependencyAdded(chainId); + } + + /// @notice Internal method to remove a dependency from the interop dependency set. + /// @param _value The encoded value with which to remove the dependency. + function _removeDependency(bytes calldata _value) internal { + uint256 chainId = StaticConfig.decodeRemoveDependency(_value); + + if (chainId == block.chainid) revert CantRemovedDependency(); + + if (!dependencySet.remove(chainId)) revert NotDependency(); + + emit DependencyRemoved(chainId); + } +} diff --git a/packages/contracts-bedrock/test/BenchmarkTest.t.sol b/packages/contracts-bedrock/test/BenchmarkTest.t.sol index babe3add5a50..5ae4b1d9335f 100644 --- a/packages/contracts-bedrock/test/BenchmarkTest.t.sol +++ b/packages/contracts-bedrock/test/BenchmarkTest.t.sol @@ -9,7 +9,7 @@ import { Bridge_Initializer } from "test/setup/Bridge_Initializer.sol"; import { CrossDomainMessenger } from "src/universal/CrossDomainMessenger.sol"; import { ResourceMetering } from "src/L1/ResourceMetering.sol"; import { Types } from "src/libraries/Types.sol"; -import { L1BlockInterop } from "src/L2/L1BlockInterop.sol"; +import { L1BlockIsthmus } from "src/L2/L1BlockIsthmus.sol"; // Free function for setting the prevBaseFee param in the OptimismPortal. function setPrevBaseFee(Vm _vm, address _op, uint128 _prevBaseFee) { @@ -211,16 +211,15 @@ contract GasBenchMark_L2OutputOracle is CommonTest { } } -contract GasBenchMark_L1BlockInterop is CommonTest { - L1BlockInterop l1BlockInterop; +contract GasBenchMark_L1Block is CommonTest { address depositor; bytes setValuesEcotoneCalldata; - function setUp() public override { + function setUp() public virtual override { super.setUp(); depositor = l1Block.DEPOSITOR_ACCOUNT(); - l1BlockInterop = new L1BlockInterop(); setValuesEcotoneCalldata = abi.encodePacked( + bytes4(keccak256("setL1BlockValuesEcotone()")), type(uint32).max, type(uint32).max, type(uint64).max, @@ -233,12 +232,82 @@ contract GasBenchMark_L1BlockInterop is CommonTest { ); vm.startPrank(depositor); } +} +contract GasBenchMark_L1Block_SetValuesEcotone is GasBenchMark_L1Block { function test_setL1BlockValuesEcotone_benchmark() external { - address(l1BlockInterop).call(abi.encodeWithSelector(l1BlockInterop.setL1BlockValuesEcotone.selector)); + address(l1Block).call(abi.encodePacked(setValuesEcotoneCalldata)); + } +} + +contract GasBenchMark_L1Block_SetValuesEcotone_Warm is GasBenchMark_L1Block { + function setUp() public virtual override { + address(l1Block).call(abi.encodePacked(setValuesEcotoneCalldata)); + } + + function test_setL1BlockValuesEcotone_benchmark() external { + address(l1Block).call(abi.encodePacked(setValuesEcotoneCalldata)); + } +} + +contract GasBenchMark_L1BlockIsthmus is GasBenchMark_L1Block { + L1BlockIsthmus l1BlockIsthmus; + + function setUp() public virtual override { + super.setUp(); + l1BlockIsthmus = new L1BlockIsthmus(); + setValuesEcotoneCalldata = abi.encodePacked( + type(uint32).max, + type(uint32).max, + type(uint64).max, + type(uint64).max, + type(uint64).max, + type(uint256).max, + type(uint256).max, + keccak256(abi.encode(1)), + bytes32(type(uint256).max) + ); + } +} + +contract GasBenchMark_L1BlockIsthmus_SetValuesIsthmus is GasBenchMark_L1BlockIsthmus { + function test_setL1BlockValuesIsthmus_benchmark() external { + address(l1BlockIsthmus).call( + abi.encodePacked(l1BlockIsthmus.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata) + ); + } +} + +contract GasBenchMark_L1BlockIsthmus_SetValuesIsthmus_Warm is GasBenchMark_L1BlockIsthmus { + function setUp() public virtual override { + address(l1BlockIsthmus).call( + abi.encodePacked(l1BlockIsthmus.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata) + ); } function test_setL1BlockValuesIsthmus_benchmark() external { - address(l1BlockInterop).call(abi.encodeWithSelector(l1BlockInterop.setL1BlockValuesIsthmus.selector)); + address(l1BlockIsthmus).call( + abi.encodePacked(l1BlockIsthmus.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata) + ); + } +} + +contract GasBenchMark_L1BlockIsthmus_DepositsComplete is GasBenchMark_L1BlockIsthmus { + function test_depositsComplete_benchmark() external { + address(l1BlockIsthmus).call(abi.encodeWithSelector(l1BlockIsthmus.depositsComplete.selector)); + } +} + +contract GasBenchMark_L1BlockIsthmus_DepositsComplete_Warm is GasBenchMark_L1BlockIsthmus { + function setUp() public virtual override { + super.setUp(); + // Set the isDeposit flag to true so then we can benchmark when it is reset. + address(l1BlockIsthmus).call( + abi.encodePacked(l1BlockIsthmus.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata) + ); + } + + function test_depositsComplete_benchmark() external { + address(l1BlockIsthmus).call(abi.encodeWithSelector(l1BlockIsthmus.depositsComplete.selector)); } } diff --git a/packages/contracts-bedrock/test/L1/OptimismPortalInterop.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortalInterop.t.sol index 1d9c2b01a248..6fb9a1aab6ed 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortalInterop.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortalInterop.t.sol @@ -11,7 +11,7 @@ import { Predeploys } from "src/libraries/Predeploys.sol"; // Target contract dependencies import "src/libraries/PortalErrors.sol"; import { OptimismPortalInterop } from "src/L1/OptimismPortalInterop.sol"; -import { L1BlockInterop, ConfigType } from "src/L2/L1BlockInterop.sol"; +import { L1BlockIsthmus, ConfigType } from "src/L2/L1BlockIsthmus.sol"; contract OptimismPortalInterop_Test is CommonTest { /// @notice Marked virtual to be overridden in @@ -31,7 +31,7 @@ contract OptimismPortalInterop_Test is CommonTest { _mint: 0, _gasLimit: 200_000, _isCreation: false, - _data: abi.encodeCall(L1BlockInterop.setConfig, (ConfigType.SET_GAS_PAYING_TOKEN, _value)) + _data: abi.encodeCall(L1BlockIsthmus.setConfig, (ConfigType.SET_GAS_PAYING_TOKEN, _value)) }); vm.prank(address(_optimismPortalInterop().systemConfig())); @@ -54,7 +54,7 @@ contract OptimismPortalInterop_Test is CommonTest { _mint: 0, _gasLimit: 200_000, _isCreation: false, - _data: abi.encodeCall(L1BlockInterop.setConfig, (ConfigType.ADD_DEPENDENCY, _value)) + _data: abi.encodeCall(L1BlockIsthmus.setConfig, (ConfigType.ADD_DEPENDENCY, _value)) }); vm.prank(address(_optimismPortalInterop().systemConfig())); @@ -77,7 +77,7 @@ contract OptimismPortalInterop_Test is CommonTest { _mint: 0, _gasLimit: 200_000, _isCreation: false, - _data: abi.encodeCall(L1BlockInterop.setConfig, (ConfigType.REMOVE_DEPENDENCY, _value)) + _data: abi.encodeCall(L1BlockIsthmus.setConfig, (ConfigType.REMOVE_DEPENDENCY, _value)) }); vm.prank(address(_optimismPortalInterop().systemConfig())); diff --git a/packages/contracts-bedrock/test/L1/SystemConfigInterop.t.sol b/packages/contracts-bedrock/test/L1/SystemConfigInterop.t.sol index 8c43aad40702..24b7a80bb74f 100644 --- a/packages/contracts-bedrock/test/L1/SystemConfigInterop.t.sol +++ b/packages/contracts-bedrock/test/L1/SystemConfigInterop.t.sol @@ -14,7 +14,7 @@ import { SystemConfig } from "src/L1/SystemConfig.sol"; import { SystemConfigInterop } from "src/L1/SystemConfigInterop.sol"; import { OptimismPortalInterop } from "src/L1/OptimismPortalInterop.sol"; import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; -import { ConfigType } from "src/L2/L1BlockInterop.sol"; +import { ConfigType } from "src/L2/L1BlockIsthmus.sol"; contract SystemConfigInterop_Test is CommonTest { /// @notice Marked virtual to be overridden in diff --git a/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol b/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol index 7e96670de1fe..1c2407dd73ab 100644 --- a/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol +++ b/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol @@ -8,17 +8,17 @@ import { CommonTest } from "test/setup/CommonTest.sol"; import { StaticConfig } from "src/libraries/StaticConfig.sol"; // Target contract dependencies -import { L1BlockInterop, ConfigType } from "src/L2/L1BlockInterop.sol"; +import { L1BlockIsthmus, ConfigType } from "src/L2/L1BlockIsthmus.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; import "src/libraries/L1BlockErrors.sol"; -contract L1BlockInteropTest is CommonTest { +contract L1BlockIsthmusTest is CommonTest { event GasPayingTokenSet(address indexed token, uint8 indexed decimals, bytes32 name, bytes32 symbol); event DependencyAdded(uint256 indexed chainId); event DependencyRemoved(uint256 indexed chainId); modifier prankDepositor() { - vm.startPrank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + vm.startPrank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); _; vm.stopPrank(); } @@ -34,14 +34,14 @@ contract L1BlockInteropTest is CommonTest { function testFuzz_isInDependencySet_succeeds(uint256 _chainId) public prankDepositor { vm.assume(_chainId != block.chainid); - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); - assertTrue(_l1BlockInterop().isInDependencySet(_chainId)); + assertTrue(_l1BlockIsthmus().isInDependencySet(_chainId)); } /// @dev Tests that `isInDependencySet` returns true when the chain's chain ID is passed as the input. function test_isInDependencySet_chainChainId_succeeds() public view { - assertTrue(_l1BlockInterop().isInDependencySet(block.chainid)); + assertTrue(_l1BlockIsthmus().isInDependencySet(block.chainid)); } /// @dev Tests that `isInDependencySet` reverts when the input chain ID is not in the dependency set @@ -50,16 +50,16 @@ contract L1BlockInteropTest is CommonTest { vm.assume(_chainId != block.chainid); // Check that the chain ID is not in the dependency set - assertFalse(_l1BlockInterop().isInDependencySet(_chainId)); + assertFalse(_l1BlockIsthmus().isInDependencySet(_chainId)); } /// @dev Tests that `isInDependencySet` returns false when the dependency set is empty. function testFuzz_isInDependencySet_dependencySetEmpty_succeeds(uint256 _chainId) public view { vm.assume(_chainId != block.chainid); - assertEq(_l1BlockInterop().dependencySetSize(), 0); + assertEq(_l1BlockIsthmus().dependencySetSize(), 0); - assertFalse(_l1BlockInterop().isInDependencySet(_chainId)); + assertFalse(_l1BlockIsthmus().isInDependencySet(_chainId)); } /// @dev Tests that the dependency set size is correct when adding an arbitrary number of chain IDs. @@ -70,16 +70,16 @@ contract L1BlockInteropTest is CommonTest { for (uint256 i = 0; i < _dependencySetSize; i++) { if (i == block.chainid) continue; - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(i)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(i)); uniqueCount++; } - assertEq(_l1BlockInterop().dependencySetSize(), uniqueCount); + assertEq(_l1BlockIsthmus().dependencySetSize(), uniqueCount); } /// @dev Tests that the dependency set size is correct when the dependency set is empty. function test_dependencySetSize_dependencySetEmpty_succeeds() public view { - assertEq(_l1BlockInterop().dependencySetSize(), 0); + assertEq(_l1BlockIsthmus().dependencySetSize(), 0); } /// @dev Tests that the config for setting the gas paying token succeeds. @@ -97,7 +97,7 @@ contract L1BlockInteropTest is CommonTest { vm.expectEmit(address(l1Block)); emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol }); - _l1BlockInterop().setConfig( + _l1BlockIsthmus().setConfig( ConfigType.SET_GAS_PAYING_TOKEN, StaticConfig.encodeSetGasPayingToken({ _token: _token, _decimals: _decimals, _name: _name, _symbol: _symbol }) ); @@ -115,7 +115,7 @@ contract L1BlockInteropTest is CommonTest { vm.assume(_token != address(vm)); vm.expectRevert(NotDepositor.selector); - _l1BlockInterop().setConfig( + _l1BlockIsthmus().setConfig( ConfigType.SET_GAS_PAYING_TOKEN, StaticConfig.encodeSetGasPayingToken({ _token: _token, _decimals: _decimals, _name: _name, _symbol: _symbol }) ); @@ -128,41 +128,41 @@ contract L1BlockInteropTest is CommonTest { vm.expectEmit(address(l1Block)); emit DependencyAdded(_chainId); - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); } /// @dev Tests that adding a dependency reverts if it's the chain's chain id function test_setConfig_addDependency_chainChainId_reverts() public prankDepositor { vm.expectRevert(AlreadyDependency.selector); - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(block.chainid)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(block.chainid)); } /// @dev Tests that adding a dependency already in the set reverts function test_setConfig_addDependency_alreadyDependency_reverts(uint256 _chainId) public prankDepositor { vm.assume(_chainId != block.chainid); - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); vm.expectRevert(AlreadyDependency.selector); - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); } /// @dev Tests that setting the add dependency config as not the depositor reverts. function testFuzz_setConfig_addDependency_notDepositor_reverts(uint256 _chainId) public { vm.expectRevert(NotDepositor.selector); - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); } /// @dev Tests that setting the add dependency config when the dependency set size is too large reverts. function test_setConfig_addDependency_dependencySetSizeTooLarge_reverts() public prankDepositor { for (uint256 i = 0; i < type(uint8).max; i++) { - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(i)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(i)); } - assertEq(_l1BlockInterop().dependencySetSize(), type(uint8).max); + assertEq(_l1BlockIsthmus().dependencySetSize(), type(uint8).max); vm.expectRevert(DependencySetSizeTooLarge.selector); - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(1)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(1)); } /// @dev Tests that the config for removing a dependency can be set. @@ -170,24 +170,24 @@ contract L1BlockInteropTest is CommonTest { vm.assume(_chainId != block.chainid); // Add the chain ID to the dependency set before removing it - _l1BlockInterop().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); vm.expectEmit(address(l1Block)); emit DependencyRemoved(_chainId); - _l1BlockInterop().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)); + _l1BlockIsthmus().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)); } /// @dev Tests that setting the remove dependency config as not the depositor reverts. function testFuzz_setConfig_removeDependency_notDepositor_reverts(uint256 _chainId) public { vm.expectRevert(NotDepositor.selector); - _l1BlockInterop().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)); + _l1BlockIsthmus().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)); } /// @dev Tests that setting the remove dependency config for the chain's chain ID reverts. function test_setConfig_removeDependency_chainChainId_reverts() public prankDepositor { vm.expectRevert(CantRemovedDependency.selector); - _l1BlockInterop().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(block.chainid)); + _l1BlockIsthmus().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(block.chainid)); } /// @dev Tests that setting the remove dependency config for a chain ID that is not in the dependency set reverts. @@ -195,46 +195,46 @@ contract L1BlockInteropTest is CommonTest { vm.assume(_chainId != block.chainid); vm.expectRevert(NotDependency.selector); - _l1BlockInterop().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)); + _l1BlockIsthmus().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)); } - /// @dev Returns the L1BlockInterop instance. - function _l1BlockInterop() internal view returns (L1BlockInterop) { - return L1BlockInterop(address(l1Block)); + /// @dev Returns the L1BlockIsthmus instance. + function _l1BlockIsthmus() internal view returns (L1BlockIsthmus) { + return L1BlockIsthmus(address(l1Block)); } } -contract L1BlockInteropIsDeposit_Test is L1BlockInteropTest { +contract L1BlockIsthmusIsDeposit_Test is L1BlockIsthmusTest { /// @dev Tests that `isDeposit` reverts if the caller is not the cross L2 inbox. function test_isDeposit_notCrossL2Inbox_reverts(address _caller) external { vm.assume(_caller != Predeploys.CROSS_L2_INBOX); vm.expectRevert(NotCrossL2Inbox.selector); - _l1BlockInterop().isDeposit(); + _l1BlockIsthmus().isDeposit(); } /// @dev Tests that `isDeposit` always returns the correct value. function test_isDeposit_succeeds() external { // Assert is false if the value is not updated vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(_l1BlockInterop().isDeposit(), false); + assertEq(_l1BlockIsthmus().isDeposit(), false); /// @dev Assuming that `setL1BlockValuesIsthmus` will set the proper value. That function is tested as well - vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); - _l1BlockInterop().setL1BlockValuesIsthmus(); + vm.prank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + _l1BlockIsthmus().setL1BlockValuesIsthmus(); // Assert is true if the value is updated vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(_l1BlockInterop().isDeposit(), true); + assertEq(_l1BlockIsthmus().isDeposit(), true); } } -contract L1BlockInteropSetL1BlockValuesIsthmus_Test is L1BlockInteropTest { +contract L1BlockIsthmusSetL1BlockValuesIsthmus_Test is L1BlockIsthmusTest { /// @dev Tests that `setL1BlockValuesIsthmus` reverts if sender address is not the depositor function test_setL1BlockValuesIsthmus_notDepositor_reverts(address _caller) external { - vm.assume(_caller != _l1BlockInterop().DEPOSITOR_ACCOUNT()); + vm.assume(_caller != _l1BlockIsthmus().DEPOSITOR_ACCOUNT()); vm.prank(_caller); vm.expectRevert(NotDepositor.selector); - _l1BlockInterop().setL1BlockValuesIsthmus(); + _l1BlockIsthmus().setL1BlockValuesIsthmus(); } /// @dev Tests that `setL1BlockValuesIsthmus` succeeds if sender address is the depositor @@ -253,60 +253,60 @@ contract L1BlockInteropSetL1BlockValuesIsthmus_Test is L1BlockInteropTest { { // Ensure the `isDepositTransaction` flag is false before calling `setL1BlockValuesIsthmus` vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(_l1BlockInterop().isDeposit(), false); + assertEq(_l1BlockIsthmus().isDeposit(), false); bytes memory setValuesEcotoneCalldata = abi.encodePacked( baseFeeScalar, blobBaseFeeScalar, sequenceNumber, timestamp, number, baseFee, blobBaseFee, hash, batcherHash ); - vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + vm.prank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); (bool success,) = address(l1Block).call( - abi.encodePacked(L1BlockInterop.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata) + abi.encodePacked(L1BlockIsthmus.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata) ); assertTrue(success, "function call failed"); // Assert that the `isDepositTransaction` flag was properly set to true vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(_l1BlockInterop().isDeposit(), true); + assertEq(_l1BlockIsthmus().isDeposit(), true); // Assert `setL1BlockValuesEcotone` was properly called, forwarding the calldata to it - assertEq(_l1BlockInterop().baseFeeScalar(), baseFeeScalar, "base fee scalar not properly set"); - assertEq(_l1BlockInterop().blobBaseFeeScalar(), blobBaseFeeScalar, "blob base fee scalar not properly set"); - assertEq(_l1BlockInterop().sequenceNumber(), sequenceNumber, "sequence number not properly set"); - assertEq(_l1BlockInterop().timestamp(), timestamp, "timestamp not properly set"); - assertEq(_l1BlockInterop().number(), number, "number not properly set"); - assertEq(_l1BlockInterop().basefee(), baseFee, "base fee not properly set"); - assertEq(_l1BlockInterop().blobBaseFee(), blobBaseFee, "blob base fee not properly set"); - assertEq(_l1BlockInterop().hash(), hash, "hash not properly set"); - assertEq(_l1BlockInterop().batcherHash(), batcherHash, "batcher hash not properly set"); + assertEq(_l1BlockIsthmus().baseFeeScalar(), baseFeeScalar, "base fee scalar not properly set"); + assertEq(_l1BlockIsthmus().blobBaseFeeScalar(), blobBaseFeeScalar, "blob base fee scalar not properly set"); + assertEq(_l1BlockIsthmus().sequenceNumber(), sequenceNumber, "sequence number not properly set"); + assertEq(_l1BlockIsthmus().timestamp(), timestamp, "timestamp not properly set"); + assertEq(_l1BlockIsthmus().number(), number, "number not properly set"); + assertEq(_l1BlockIsthmus().basefee(), baseFee, "base fee not properly set"); + assertEq(_l1BlockIsthmus().blobBaseFee(), blobBaseFee, "blob base fee not properly set"); + assertEq(_l1BlockIsthmus().hash(), hash, "hash not properly set"); + assertEq(_l1BlockIsthmus().batcherHash(), batcherHash, "batcher hash not properly set"); } } -contract L1BlockDepositsComplete_Test is L1BlockInteropTest { +contract L1BlockDepositsComplete_Test is L1BlockIsthmusTest { // @dev Tests that `depositsComplete` reverts if the caller is not the depositor. function test_deposits_is_depositor_reverts(address _caller) external { - vm.assume(_caller != _l1BlockInterop().DEPOSITOR_ACCOUNT()); + vm.assume(_caller != _l1BlockIsthmus().DEPOSITOR_ACCOUNT()); vm.expectRevert(NotDepositor.selector); - _l1BlockInterop().depositsComplete(); + _l1BlockIsthmus().depositsComplete(); } // @dev Tests that `depositsComplete` succeeds if the caller is the depositor. function test_depositsComplete_succeeds() external { // Set the `isDeposit` flag to true - vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); - _l1BlockInterop().setL1BlockValuesIsthmus(); + vm.prank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + _l1BlockIsthmus().setL1BlockValuesIsthmus(); // Assert that the `isDeposit` flag was properly set to true vm.prank(Predeploys.CROSS_L2_INBOX); - assertTrue(_l1BlockInterop().isDeposit()); + assertTrue(_l1BlockIsthmus().isDeposit()); // Call `depositsComplete` - vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); - _l1BlockInterop().depositsComplete(); + vm.prank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + _l1BlockIsthmus().depositsComplete(); // Assert that the `isDeposit` flag was properly set to false /// @dev Assuming that `isDeposit()` wil return the proper value. That function is tested as well vm.prank(Predeploys.CROSS_L2_INBOX); - assertEq(_l1BlockInterop().isDeposit(), false); + assertEq(_l1BlockIsthmus().isDeposit(), false); } } diff --git a/packages/contracts-bedrock/test/L2/L1BlockIsthmus.t.sol b/packages/contracts-bedrock/test/L2/L1BlockIsthmus.t.sol new file mode 100644 index 000000000000..1c2407dd73ab --- /dev/null +++ b/packages/contracts-bedrock/test/L2/L1BlockIsthmus.t.sol @@ -0,0 +1,312 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +// Testing utilities +import { CommonTest } from "test/setup/CommonTest.sol"; + +// Libraries +import { StaticConfig } from "src/libraries/StaticConfig.sol"; + +// Target contract dependencies +import { L1BlockIsthmus, ConfigType } from "src/L2/L1BlockIsthmus.sol"; +import { Predeploys } from "src/libraries/Predeploys.sol"; +import "src/libraries/L1BlockErrors.sol"; + +contract L1BlockIsthmusTest is CommonTest { + event GasPayingTokenSet(address indexed token, uint8 indexed decimals, bytes32 name, bytes32 symbol); + event DependencyAdded(uint256 indexed chainId); + event DependencyRemoved(uint256 indexed chainId); + + modifier prankDepositor() { + vm.startPrank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + _; + vm.stopPrank(); + } + + /// @notice Marked virtual to be overridden in + /// test/kontrol/deployment/DeploymentSummary.t.sol + function setUp() public virtual override { + super.enableInterop(); + super.setUp(); + } + + /// @dev Tests that an arbitrary chain ID can be added to the dependency set. + function testFuzz_isInDependencySet_succeeds(uint256 _chainId) public prankDepositor { + vm.assume(_chainId != block.chainid); + + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + + assertTrue(_l1BlockIsthmus().isInDependencySet(_chainId)); + } + + /// @dev Tests that `isInDependencySet` returns true when the chain's chain ID is passed as the input. + function test_isInDependencySet_chainChainId_succeeds() public view { + assertTrue(_l1BlockIsthmus().isInDependencySet(block.chainid)); + } + + /// @dev Tests that `isInDependencySet` reverts when the input chain ID is not in the dependency set + /// and is not the chain's chain ID. + function testFuzz_isInDependencySet_notDependency_reverts(uint256 _chainId) public view { + vm.assume(_chainId != block.chainid); + + // Check that the chain ID is not in the dependency set + assertFalse(_l1BlockIsthmus().isInDependencySet(_chainId)); + } + + /// @dev Tests that `isInDependencySet` returns false when the dependency set is empty. + function testFuzz_isInDependencySet_dependencySetEmpty_succeeds(uint256 _chainId) public view { + vm.assume(_chainId != block.chainid); + + assertEq(_l1BlockIsthmus().dependencySetSize(), 0); + + assertFalse(_l1BlockIsthmus().isInDependencySet(_chainId)); + } + + /// @dev Tests that the dependency set size is correct when adding an arbitrary number of chain IDs. + function testFuzz_dependencySetSize_succeeds(uint8 _dependencySetSize) public prankDepositor { + vm.assume(_dependencySetSize <= type(uint8).max); + + uint256 uniqueCount = 0; + + for (uint256 i = 0; i < _dependencySetSize; i++) { + if (i == block.chainid) continue; + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(i)); + uniqueCount++; + } + + assertEq(_l1BlockIsthmus().dependencySetSize(), uniqueCount); + } + + /// @dev Tests that the dependency set size is correct when the dependency set is empty. + function test_dependencySetSize_dependencySetEmpty_succeeds() public view { + assertEq(_l1BlockIsthmus().dependencySetSize(), 0); + } + + /// @dev Tests that the config for setting the gas paying token succeeds. + function testFuzz_setConfig_gasPayingToken_succeeds( + address _token, + uint8 _decimals, + bytes32 _name, + bytes32 _symbol + ) + public + prankDepositor + { + vm.assume(_token != address(vm)); + + vm.expectEmit(address(l1Block)); + emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol }); + + _l1BlockIsthmus().setConfig( + ConfigType.SET_GAS_PAYING_TOKEN, + StaticConfig.encodeSetGasPayingToken({ _token: _token, _decimals: _decimals, _name: _name, _symbol: _symbol }) + ); + } + + /// @dev Tests that setting the gas paying token config as not the depositor reverts. + function testFuzz_setConfig_gasPayingToken_notDepositor_reverts( + address _token, + uint8 _decimals, + bytes32 _name, + bytes32 _symbol + ) + public + { + vm.assume(_token != address(vm)); + + vm.expectRevert(NotDepositor.selector); + _l1BlockIsthmus().setConfig( + ConfigType.SET_GAS_PAYING_TOKEN, + StaticConfig.encodeSetGasPayingToken({ _token: _token, _decimals: _decimals, _name: _name, _symbol: _symbol }) + ); + } + + /// @dev Tests that the config for adding a dependency can be set. + function testFuzz_setConfig_addDependency_succeeds(uint256 _chainId) public prankDepositor { + vm.assume(_chainId != block.chainid); + + vm.expectEmit(address(l1Block)); + emit DependencyAdded(_chainId); + + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + } + + /// @dev Tests that adding a dependency reverts if it's the chain's chain id + function test_setConfig_addDependency_chainChainId_reverts() public prankDepositor { + vm.expectRevert(AlreadyDependency.selector); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(block.chainid)); + } + + /// @dev Tests that adding a dependency already in the set reverts + function test_setConfig_addDependency_alreadyDependency_reverts(uint256 _chainId) public prankDepositor { + vm.assume(_chainId != block.chainid); + + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + + vm.expectRevert(AlreadyDependency.selector); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + } + + /// @dev Tests that setting the add dependency config as not the depositor reverts. + function testFuzz_setConfig_addDependency_notDepositor_reverts(uint256 _chainId) public { + vm.expectRevert(NotDepositor.selector); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + } + + /// @dev Tests that setting the add dependency config when the dependency set size is too large reverts. + function test_setConfig_addDependency_dependencySetSizeTooLarge_reverts() public prankDepositor { + for (uint256 i = 0; i < type(uint8).max; i++) { + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(i)); + } + + assertEq(_l1BlockIsthmus().dependencySetSize(), type(uint8).max); + + vm.expectRevert(DependencySetSizeTooLarge.selector); + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(1)); + } + + /// @dev Tests that the config for removing a dependency can be set. + function testFuzz_setConfig_removeDependency_succeeds(uint256 _chainId) public prankDepositor { + vm.assume(_chainId != block.chainid); + + // Add the chain ID to the dependency set before removing it + _l1BlockIsthmus().setConfig(ConfigType.ADD_DEPENDENCY, StaticConfig.encodeAddDependency(_chainId)); + + vm.expectEmit(address(l1Block)); + emit DependencyRemoved(_chainId); + + _l1BlockIsthmus().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)); + } + + /// @dev Tests that setting the remove dependency config as not the depositor reverts. + function testFuzz_setConfig_removeDependency_notDepositor_reverts(uint256 _chainId) public { + vm.expectRevert(NotDepositor.selector); + _l1BlockIsthmus().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)); + } + + /// @dev Tests that setting the remove dependency config for the chain's chain ID reverts. + function test_setConfig_removeDependency_chainChainId_reverts() public prankDepositor { + vm.expectRevert(CantRemovedDependency.selector); + _l1BlockIsthmus().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(block.chainid)); + } + + /// @dev Tests that setting the remove dependency config for a chain ID that is not in the dependency set reverts. + function testFuzz_setConfig_removeDependency_notDependency_reverts(uint256 _chainId) public prankDepositor { + vm.assume(_chainId != block.chainid); + + vm.expectRevert(NotDependency.selector); + _l1BlockIsthmus().setConfig(ConfigType.REMOVE_DEPENDENCY, StaticConfig.encodeRemoveDependency(_chainId)); + } + + /// @dev Returns the L1BlockIsthmus instance. + function _l1BlockIsthmus() internal view returns (L1BlockIsthmus) { + return L1BlockIsthmus(address(l1Block)); + } +} + +contract L1BlockIsthmusIsDeposit_Test is L1BlockIsthmusTest { + /// @dev Tests that `isDeposit` reverts if the caller is not the cross L2 inbox. + function test_isDeposit_notCrossL2Inbox_reverts(address _caller) external { + vm.assume(_caller != Predeploys.CROSS_L2_INBOX); + vm.expectRevert(NotCrossL2Inbox.selector); + _l1BlockIsthmus().isDeposit(); + } + + /// @dev Tests that `isDeposit` always returns the correct value. + function test_isDeposit_succeeds() external { + // Assert is false if the value is not updated + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockIsthmus().isDeposit(), false); + + /// @dev Assuming that `setL1BlockValuesIsthmus` will set the proper value. That function is tested as well + vm.prank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + _l1BlockIsthmus().setL1BlockValuesIsthmus(); + + // Assert is true if the value is updated + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockIsthmus().isDeposit(), true); + } +} + +contract L1BlockIsthmusSetL1BlockValuesIsthmus_Test is L1BlockIsthmusTest { + /// @dev Tests that `setL1BlockValuesIsthmus` reverts if sender address is not the depositor + function test_setL1BlockValuesIsthmus_notDepositor_reverts(address _caller) external { + vm.assume(_caller != _l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + vm.prank(_caller); + vm.expectRevert(NotDepositor.selector); + _l1BlockIsthmus().setL1BlockValuesIsthmus(); + } + + /// @dev Tests that `setL1BlockValuesIsthmus` succeeds if sender address is the depositor + function test_setL1BlockValuesIsthmus_succeeds( + uint32 baseFeeScalar, + uint32 blobBaseFeeScalar, + uint64 sequenceNumber, + uint64 timestamp, + uint64 number, + uint256 baseFee, + uint256 blobBaseFee, + bytes32 hash, + bytes32 batcherHash + ) + external + { + // Ensure the `isDepositTransaction` flag is false before calling `setL1BlockValuesIsthmus` + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockIsthmus().isDeposit(), false); + + bytes memory setValuesEcotoneCalldata = abi.encodePacked( + baseFeeScalar, blobBaseFeeScalar, sequenceNumber, timestamp, number, baseFee, blobBaseFee, hash, batcherHash + ); + + vm.prank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + (bool success,) = address(l1Block).call( + abi.encodePacked(L1BlockIsthmus.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata) + ); + assertTrue(success, "function call failed"); + + // Assert that the `isDepositTransaction` flag was properly set to true + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockIsthmus().isDeposit(), true); + + // Assert `setL1BlockValuesEcotone` was properly called, forwarding the calldata to it + assertEq(_l1BlockIsthmus().baseFeeScalar(), baseFeeScalar, "base fee scalar not properly set"); + assertEq(_l1BlockIsthmus().blobBaseFeeScalar(), blobBaseFeeScalar, "blob base fee scalar not properly set"); + assertEq(_l1BlockIsthmus().sequenceNumber(), sequenceNumber, "sequence number not properly set"); + assertEq(_l1BlockIsthmus().timestamp(), timestamp, "timestamp not properly set"); + assertEq(_l1BlockIsthmus().number(), number, "number not properly set"); + assertEq(_l1BlockIsthmus().basefee(), baseFee, "base fee not properly set"); + assertEq(_l1BlockIsthmus().blobBaseFee(), blobBaseFee, "blob base fee not properly set"); + assertEq(_l1BlockIsthmus().hash(), hash, "hash not properly set"); + assertEq(_l1BlockIsthmus().batcherHash(), batcherHash, "batcher hash not properly set"); + } +} + +contract L1BlockDepositsComplete_Test is L1BlockIsthmusTest { + // @dev Tests that `depositsComplete` reverts if the caller is not the depositor. + function test_deposits_is_depositor_reverts(address _caller) external { + vm.assume(_caller != _l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + vm.expectRevert(NotDepositor.selector); + _l1BlockIsthmus().depositsComplete(); + } + + // @dev Tests that `depositsComplete` succeeds if the caller is the depositor. + function test_depositsComplete_succeeds() external { + // Set the `isDeposit` flag to true + vm.prank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + _l1BlockIsthmus().setL1BlockValuesIsthmus(); + + // Assert that the `isDeposit` flag was properly set to true + vm.prank(Predeploys.CROSS_L2_INBOX); + assertTrue(_l1BlockIsthmus().isDeposit()); + + // Call `depositsComplete` + vm.prank(_l1BlockIsthmus().DEPOSITOR_ACCOUNT()); + _l1BlockIsthmus().depositsComplete(); + + // Assert that the `isDeposit` flag was properly set to false + /// @dev Assuming that `isDeposit()` wil return the proper value. That function is tested as well + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockIsthmus().isDeposit(), false); + } +} From bc562dec6aed2e8aefee4f282341f086f5d600ff Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Wed, 14 Aug 2024 19:03:39 -0300 Subject: [PATCH 24/28] chore: some renames --- op-node/rollup/derive/attributes.go | 2 ++ op-node/rollup/derive/l1_block_info.go | 14 ++++++++------ .../contracts-bedrock/test/BenchmarkTest.t.sol | 4 ++++ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/op-node/rollup/derive/attributes.go b/op-node/rollup/derive/attributes.go index cc38a31b9c54..92ccd32cf5fb 100644 --- a/op-node/rollup/derive/attributes.go +++ b/op-node/rollup/derive/attributes.go @@ -122,6 +122,8 @@ func (ba *FetchingAttributesBuilder) PreparePayloadAttributes(ctx context.Contex } var afterForceIncludeTxs []hexutil.Bytes + // TODO I'm usure if this needs to be only done AFTER interop is enabled, but not on the same block. + // This is similar to what Proto mentioned about blocking the ExecutingMessages on the same block as the interop activation. if ba.rollupCfg.IsInterop(nextL2Time) { depositsCompleteTx, err := DepositsCompleteBytes(seqNumber, l1Info) if err != nil { diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index 7e5213af2795..22b39ff4e32b 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -391,12 +391,14 @@ func DepositsCompleteDeposit(seqNumber uint64, block eth.BlockInfo) (*types.Depo L1BlockHash: block.Hash(), } out := &types.DepositTx{ - SourceHash: source.SourceHash(), - From: L1InfoDepositerAddress, - To: &L1BlockAddress, - Mint: nil, - Value: big.NewInt(0), - Gas: 50_000, // TODO: check how much gas is actually needed + SourceHash: source.SourceHash(), + From: L1InfoDepositerAddress, + To: &L1BlockAddress, + Mint: nil, + Value: big.NewInt(0), + // We set the gas limit to 50k to ensure that the L1 Attributes Transaction does not run out of gas. + // see `test_depositsComplete_benchmark` at: `/packages/contracts-bedrock/test/BenchmarkTest.t.sol` + Gas: 50_000, IsSystemTransaction: false, Data: DepositsCompleteBytes4, } diff --git a/packages/contracts-bedrock/test/BenchmarkTest.t.sol b/packages/contracts-bedrock/test/BenchmarkTest.t.sol index 5ae4b1d9335f..efa6b0085a25 100644 --- a/packages/contracts-bedrock/test/BenchmarkTest.t.sol +++ b/packages/contracts-bedrock/test/BenchmarkTest.t.sol @@ -310,4 +310,8 @@ contract GasBenchMark_L1BlockIsthmus_DepositsComplete_Warm is GasBenchMark_L1Blo function test_depositsComplete_benchmark() external { address(l1BlockIsthmus).call(abi.encodeWithSelector(l1BlockIsthmus.depositsComplete.selector)); } + + function test_depositsComplete_benchmark() external { + address(l1BlockInterop).call(abi.encodeWithSelector(l1BlockInterop.depositsComplete.selector)); + } } From a8fd430035d901407c65a7430bc0c063de2701a7 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Thu, 15 Aug 2024 12:49:58 -0300 Subject: [PATCH 25/28] chore: update gas limit of DepositsComplete --- op-node/rollup/derive/l1_block_info.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index 22b39ff4e32b..8c8132f5eff4 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -396,9 +396,11 @@ func DepositsCompleteDeposit(seqNumber uint64, block eth.BlockInfo) (*types.Depo To: &L1BlockAddress, Mint: nil, Value: big.NewInt(0), - // We set the gas limit to 50k to ensure that the L1 Attributes Transaction does not run out of gas. + // We set the gas limit to 15k to ensure that the DepositsComplete Transaction does not run out of gas. + // GasBenchMark_L1BlockIsthmus_DepositsComplete:test_depositsComplete_benchmark() (gas: 7768) + // GasBenchMark_L1BlockIsthmus_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5768) // see `test_depositsComplete_benchmark` at: `/packages/contracts-bedrock/test/BenchmarkTest.t.sol` - Gas: 50_000, + Gas: 15_000, IsSystemTransaction: false, Data: DepositsCompleteBytes4, } From 5eaf39dda0ee5a0170b80af55013f6b5c66afb53 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Fri, 16 Aug 2024 13:18:14 -0300 Subject: [PATCH 26/28] fix: deprecate Interop L1 block --- .../src/L2/L1BlockInterop.sol | 142 ------------------ .../src/L2/L1BlockIsthmus.sol | 6 +- 2 files changed, 3 insertions(+), 145 deletions(-) delete mode 100644 packages/contracts-bedrock/src/L2/L1BlockInterop.sol diff --git a/packages/contracts-bedrock/src/L2/L1BlockInterop.sol b/packages/contracts-bedrock/src/L2/L1BlockInterop.sol deleted file mode 100644 index 9f8d12cebf38..000000000000 --- a/packages/contracts-bedrock/src/L2/L1BlockInterop.sol +++ /dev/null @@ -1,142 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.15; - -import { L1Block } from "src/L2/L1Block.sol"; -import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; -import { GasPayingToken } from "src/libraries/GasPayingToken.sol"; -import { StaticConfig } from "src/libraries/StaticConfig.sol"; -import { Predeploys } from "src/libraries/Predeploys.sol"; -import "src/libraries/L1BlockErrors.sol"; - -/// @notice Enum representing different types of configurations that can be set on L1BlockIsthmus. -/// @custom:value SET_GAS_PAYING_TOKEN Represents the config type for setting the gas paying token. -/// @custom:value ADD_DEPENDENCY Represents the config type for adding a chain to the interop dependency set. -/// @custom:value REMOVE_DEPENDENCY Represents the config type for removing a chain from the interop dependency set. -enum ConfigType { - SET_GAS_PAYING_TOKEN, - ADD_DEPENDENCY, - REMOVE_DEPENDENCY -} - -/// @custom:proxied -/// @custom:predeploy 0x4200000000000000000000000000000000000015 -/// @title L1BlockIsthmus -/// @notice Interop extenstions of L1Block. -contract L1BlockIsthmus is L1Block { - using EnumerableSet for EnumerableSet.UintSet; - - /// @notice Event emitted when a new dependency is added to the interop dependency set. - event DependencyAdded(uint256 indexed chainId); - - /// @notice Event emitted when a dependency is removed from the interop dependency set. - event DependencyRemoved(uint256 indexed chainId); - - /// @notice The interop dependency set, containing the chain IDs in it. - EnumerableSet.UintSet dependencySet; - - /// @notice Storage slot that the isDeposit is stored at. - /// This is a custom slot that is not part of the standard storage layout. - /// keccak256(abi.encode(uint256(keccak256("l1Block.identifier.isDeposit")) - 1)) & ~bytes32(uint256(0xff)) - uint256 internal constant IS_DEPOSIT_SLOT = 0x921bd3a089295c6e5540e8fba8195448d253efd6f2e3e495b499b627dc36a300; - - /// @custom:semver +interop - function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop"); - } - - /// @notice Returns whether the call was triggered from a a deposit or not. - /// @notice This function is only callable by the CrossL2Inbox contract. - function isDeposit() external view returns (bool isDeposit_) { - if (msg.sender != Predeploys.CROSS_L2_INBOX) revert NotCrossL2Inbox(); - assembly { - isDeposit_ := sload(IS_DEPOSIT_SLOT) - } - } - - /// @notice Returns true if a chain ID is in the interop dependency set and false otherwise. - /// The chain's chain ID is always considered to be in the dependency set. - /// @param _chainId The chain ID to check. - /// @return True if the chain ID to check is in the interop dependency set. False otherwise. - function isInDependencySet(uint256 _chainId) public view returns (bool) { - return _chainId == block.chainid || dependencySet.contains(_chainId); - } - - /// @notice Returns the size of the interop dependency set. - /// @return The size of the interop dependency set. - function dependencySetSize() external view returns (uint8) { - return uint8(dependencySet.length()); - } - - /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain. - /// It updates the L1 block values through the `setL1BlockValuesEcotone` function. - /// It forwards the calldata to the internally-used `setL1BlockValuesEcotone` function. - function setL1BlockValuesIsthmus() external { - // Set the isDeposit flag to true. - assembly { - sstore(IS_DEPOSIT_SLOT, 1) - } - - _setL1BlockValuesEcotone(); - } - - /// @notice Resets the isDeposit flag. - /// Should only be called by the depositor account after the deposits are complete. - function depositsComplete() external { - if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); - - // Set the isDeposit flag to false. - assembly { - sstore(IS_DEPOSIT_SLOT, 0) - } - } - - /// @notice Sets static configuration options for the L2 system. Can only be called by the special - /// depositor account. - /// @param _type The type of configuration to set. - /// @param _value The encoded value with which to set the configuration. - function setConfig(ConfigType _type, bytes calldata _value) external { - if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor(); - - if (_type == ConfigType.SET_GAS_PAYING_TOKEN) { - _setGasPayingToken(_value); - } else if (_type == ConfigType.ADD_DEPENDENCY) { - _addDependency(_value); - } else if (_type == ConfigType.REMOVE_DEPENDENCY) { - _removeDependency(_value); - } - } - - /// @notice Internal method to set the gas paying token. - /// @param _value The encoded value with which to set the gas paying token. - function _setGasPayingToken(bytes calldata _value) internal { - (address token, uint8 decimals, bytes32 name, bytes32 symbol) = StaticConfig.decodeSetGasPayingToken(_value); - - GasPayingToken.set({ _token: token, _decimals: decimals, _name: name, _symbol: symbol }); - - emit GasPayingTokenSet({ token: token, decimals: decimals, name: name, symbol: symbol }); - } - - /// @notice Internal method to add a dependency to the interop dependency set. - /// @param _value The encoded value with which to add the dependency. - function _addDependency(bytes calldata _value) internal { - uint256 chainId = StaticConfig.decodeAddDependency(_value); - - if (dependencySet.length() == type(uint8).max) revert DependencySetSizeTooLarge(); - - if (chainId == block.chainid || !dependencySet.add(chainId)) revert AlreadyDependency(); - - emit DependencyAdded(chainId); - } - - /// @notice Internal method to remove a dependency from the interop dependency set. - /// @param _value The encoded value with which to remove the dependency. - function _removeDependency(bytes calldata _value) internal { - uint256 chainId = StaticConfig.decodeRemoveDependency(_value); - - if (chainId == block.chainid) revert CantRemovedDependency(); - - if (!dependencySet.remove(chainId)) revert NotDependency(); - - emit DependencyRemoved(chainId); - } -} diff --git a/packages/contracts-bedrock/src/L2/L1BlockIsthmus.sol b/packages/contracts-bedrock/src/L2/L1BlockIsthmus.sol index 9f8d12cebf38..b64c44646aba 100644 --- a/packages/contracts-bedrock/src/L2/L1BlockIsthmus.sol +++ b/packages/contracts-bedrock/src/L2/L1BlockIsthmus.sol @@ -21,7 +21,7 @@ enum ConfigType { /// @custom:proxied /// @custom:predeploy 0x4200000000000000000000000000000000000015 /// @title L1BlockIsthmus -/// @notice Interop extenstions of L1Block. +/// @notice Isthmus extenstions of L1Block. contract L1BlockIsthmus is L1Block { using EnumerableSet for EnumerableSet.UintSet; @@ -39,9 +39,9 @@ contract L1BlockIsthmus is L1Block { /// keccak256(abi.encode(uint256(keccak256("l1Block.identifier.isDeposit")) - 1)) & ~bytes32(uint256(0xff)) uint256 internal constant IS_DEPOSIT_SLOT = 0x921bd3a089295c6e5540e8fba8195448d253efd6f2e3e495b499b627dc36a300; - /// @custom:semver +interop + /// @custom:semver +isthmus function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop"); + return string.concat(super.version(), "+isthmus"); } /// @notice Returns whether the call was triggered from a a deposit or not. From d5ba5d1bcfc2eba7c007797a5275d4911ea4a0fc Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman Date: Fri, 16 Aug 2024 16:55:23 -0300 Subject: [PATCH 27/28] fix: missed interop filename changes --- packages/contracts-bedrock/semver-lock.json | 2 +- packages/contracts-bedrock/test/BenchmarkTest.t.sol | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json index 267c161f00e2..57f6e2507a90 100644 --- a/packages/contracts-bedrock/semver-lock.json +++ b/packages/contracts-bedrock/semver-lock.json @@ -83,7 +83,7 @@ "initCodeHash": "0xfd099da051edf13b147f4382ab4bed9db546d0c48157736ba298fb7e178b20d9", "sourceCodeHash": "0x24db623574743432626ed0d7dd938bbd2149b570a00328c772debd7eb179ff1d" }, - "src/L2/L1BlockInterop.sol": { + "src/L2/L1BlockIsthmus.sol": { "initCodeHash": "0x6833a323934b3be1e5a5c7491c652b6e90bc5102416ddbb255b5f65aa6d5d4a1", "sourceCodeHash": "0xd8ec2f814690d1ffd55e5b8496bca5a179d6d1772d61f71cdf8296c9058dc2c6" }, diff --git a/packages/contracts-bedrock/test/BenchmarkTest.t.sol b/packages/contracts-bedrock/test/BenchmarkTest.t.sol index efa6b0085a25..ca1f1c623ea6 100644 --- a/packages/contracts-bedrock/test/BenchmarkTest.t.sol +++ b/packages/contracts-bedrock/test/BenchmarkTest.t.sol @@ -311,7 +311,4 @@ contract GasBenchMark_L1BlockIsthmus_DepositsComplete_Warm is GasBenchMark_L1Blo address(l1BlockIsthmus).call(abi.encodeWithSelector(l1BlockIsthmus.depositsComplete.selector)); } - function test_depositsComplete_benchmark() external { - address(l1BlockInterop).call(abi.encodeWithSelector(l1BlockInterop.depositsComplete.selector)); - } } From ccf505d3f4b6e5daaa77a53bff31d3d147bf0af7 Mon Sep 17 00:00:00 2001 From: Skeletor Spaceman <92943766+skeletor-spaceman@users.noreply.github.com> Date: Thu, 29 Aug 2024 18:42:46 -0300 Subject: [PATCH 28/28] feat: ban deposits client tests (#11504) * feat: first pass at some tests * fix: removed check * feat: added a few more tests * feat: added final tests on spec * fix: missguiding name * fix: move gas limit comment to the constant definition * feat: added qol hardfork config * fix: remove return and revamped tests * fix: missing test qol revamp --- op-node/rollup/derive/attributes_test.go | 48 ++++++++++ op-node/rollup/derive/deposit_source_test.go | 15 ++++ op-node/rollup/derive/fuzz_parsers_test.go | 17 +++- op-node/rollup/derive/l1_block_info.go | 21 ++--- op-node/rollup/derive/l1_block_info_test.go | 93 +++++++++++++++----- op-node/rollup/types.go | 38 ++++++++ 6 files changed, 198 insertions(+), 34 deletions(-) diff --git a/op-node/rollup/derive/attributes_test.go b/op-node/rollup/derive/attributes_test.go index 68c7c71aa1e1..e428d952e5b8 100644 --- a/op-node/rollup/derive/attributes_test.go +++ b/op-node/rollup/derive/attributes_test.go @@ -195,6 +195,54 @@ func TestPreparePayloadAttributes(t *testing.T) { require.Equal(t, l1InfoTx, []byte(attrs.Transactions[0])) require.True(t, attrs.NoTxPool) }) + t.Run("same origin with deposits on post-Isthmus", func(t *testing.T) { + rng := rand.New(rand.NewSource(1234)) + l1Fetcher := &testutils.MockL1Source{} + defer l1Fetcher.AssertExpectations(t) + l2Parent := testutils.RandomL2BlockRef(rng) + l1CfgFetcher := &testutils.MockL2Client{} + l1CfgFetcher.ExpectSystemConfigByL2Hash(l2Parent.Hash, testSysCfg, nil) + defer l1CfgFetcher.AssertExpectations(t) + l1Info := testutils.RandomBlockInfo(rng) + l1Info.InfoParentHash = l2Parent.L1Origin.Hash + l1Info.InfoNum = l2Parent.L1Origin.Number + 1 + + receipts, depositTxs, err := makeReceipts(rng, l1Info.InfoHash, cfg.DepositContractAddress, []receiptData{ + {goodReceipt: true, DepositLogs: []bool{true, false}}, + {goodReceipt: true, DepositLogs: []bool{true}}, + {goodReceipt: false, DepositLogs: []bool{true}}, + {goodReceipt: false, DepositLogs: []bool{false}}, + }) + require.NoError(t, err) + usedDepositTxs, err := encodeDeposits(depositTxs) + require.NoError(t, err) + + // sets config to post-interop + cfg.AfterHardfork(rollup.Interop, 2) + + epoch := l1Info.ID() + l1InfoTx, err := L1InfoDepositBytes(cfg, testSysCfg, 0, l1Info, 0) + require.NoError(t, err) + + l2Txs := append(append(make([]eth.Data, 0), l1InfoTx), usedDepositTxs...) + + l1Fetcher.ExpectFetchReceipts(epoch.Hash, l1Info, receipts, nil) + attrBuilder := NewFetchingAttributesBuilder(cfg, l1Fetcher, l1CfgFetcher) + attrs, err := attrBuilder.PreparePayloadAttributes(context.Background(), l2Parent, epoch) + require.NoError(t, err) + require.NotNil(t, attrs) + require.Equal(t, l2Parent.Time+cfg.BlockTime, uint64(attrs.Timestamp)) + require.Equal(t, eth.Bytes32(l1Info.InfoMixDigest), attrs.PrevRandao) + require.Equal(t, predeploys.SequencerFeeVaultAddr, attrs.SuggestedFeeRecipient) + require.Equal(t, len(l2Txs), len(attrs.Transactions), "Expected txs to equal l1 info tx + user deposit txs + DepositsComplete") + require.Equal(t, l2Txs, attrs.Transactions) + require.Equal(t, DepositsCompleteBytes4, attrs.Transactions[1+len(depositTxs)][:4]) + require.True(t, attrs.NoTxPool) + depositsCompleteTx, err := DepositsCompleteBytes(l2Parent.SequenceNumber, l1Info) + require.NoError(t, err) + require.Equal(t, l2Txs, append(attrs.Transactions, depositsCompleteTx)) + }) + // Test that the payload attributes builder changes the deposit format based on L2-time-based regolith activation t.Run("regolith", func(t *testing.T) { testCases := []struct { diff --git a/op-node/rollup/derive/deposit_source_test.go b/op-node/rollup/derive/deposit_source_test.go index 10fb7048a2a2..d851325e3706 100644 --- a/op-node/rollup/derive/deposit_source_test.go +++ b/op-node/rollup/derive/deposit_source_test.go @@ -3,6 +3,7 @@ package derive import ( "testing" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" ) @@ -34,3 +35,17 @@ func TestEcotone4788ContractSourceHash(t *testing.T) { assert.Equal(t, expected, actual.Hex()) } + +// TestAfterForceIncludeSourceHash +// cast keccak $(cast concat-hex 0x0000000000000000000000000000000000000000000000000000000000000003 $(cast keccak 0x01)) +// # 0x8afb1c4a581d0e71ab65334e3365ba5511fb15c13fa212776f9d4dafc6287845 +func TestAfterForceIncludeSource(t *testing.T) { + source := AfterForceIncludeSource{ + L1BlockHash: common.Hash{0x01}, + } + + actual := source.SourceHash() + expected := "0x8afb1c4a581d0e71ab65334e3365ba5511fb15c13fa212776f9d4dafc6287845" + + assert.Equal(t, expected, actual.Hex()) +} diff --git a/op-node/rollup/derive/fuzz_parsers_test.go b/op-node/rollup/derive/fuzz_parsers_test.go index 95ce94bc7cc8..4f76c4ac7420 100644 --- a/op-node/rollup/derive/fuzz_parsers_test.go +++ b/op-node/rollup/derive/fuzz_parsers_test.go @@ -83,15 +83,26 @@ func FuzzL1InfoEcotoneRoundTrip(f *testing.F) { } enc, err := in.marshalBinaryEcotone() if err != nil { - t.Fatalf("Failed to marshal binary: %v", err) + t.Fatalf("Failed to marshal Ecotone binary: %v", err) } var out L1BlockInfo err = out.unmarshalBinaryEcotone(enc) if err != nil { - t.Fatalf("Failed to unmarshal binary: %v", err) + t.Fatalf("Failed to unmarshal Ecotone binary: %v", err) } if !cmp.Equal(in, out, cmp.Comparer(testutils.BigEqual)) { - t.Fatalf("The data did not round trip correctly. in: %v. out: %v", in, out) + t.Fatalf("The Ecotone data did not round trip correctly. in: %v. out: %v", in, out) + } + enc, err = in.marshalBinaryIsthmus() + if err != nil { + t.Fatalf("Failed to marshal Isthmus binary: %v", err) + } + err = out.unmarshalBinaryIsthmus(enc) + if err != nil { + t.Fatalf("Failed to unmarshal Isthmus binary: %v", err) + } + if !cmp.Equal(in, out, cmp.Comparer(testutils.BigEqual)) { + t.Fatalf("The Isthmus data did not round trip correctly. in: %v. out: %v", in, out) } }) diff --git a/op-node/rollup/derive/l1_block_info.go b/op-node/rollup/derive/l1_block_info.go index 8c8132f5eff4..7631494e42c3 100644 --- a/op-node/rollup/derive/l1_block_info.go +++ b/op-node/rollup/derive/l1_block_info.go @@ -26,6 +26,11 @@ const ( L1InfoBedrockLen = 4 + 32*L1InfoArguments L1InfoEcotoneLen = 4 + 32*5 // after Ecotone upgrade, args are packed into 5 32-byte slots DepositsCompleteLen = 4 // only the selector + // We set the gas limit to 15k to ensure that the DepositsComplete Transaction does not run out of gas. + // GasBenchMark_L1BlockIsthmus_DepositsComplete:test_depositsComplete_benchmark() (gas: 7768) + // GasBenchMark_L1BlockIsthmus_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5768) + // see `test_depositsComplete_benchmark` at: `/packages/contracts-bedrock/test/BenchmarkTest.t.sol` + DepositsCompleteGas = uint64(15_000) ) var ( @@ -391,16 +396,12 @@ func DepositsCompleteDeposit(seqNumber uint64, block eth.BlockInfo) (*types.Depo L1BlockHash: block.Hash(), } out := &types.DepositTx{ - SourceHash: source.SourceHash(), - From: L1InfoDepositerAddress, - To: &L1BlockAddress, - Mint: nil, - Value: big.NewInt(0), - // We set the gas limit to 15k to ensure that the DepositsComplete Transaction does not run out of gas. - // GasBenchMark_L1BlockIsthmus_DepositsComplete:test_depositsComplete_benchmark() (gas: 7768) - // GasBenchMark_L1BlockIsthmus_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5768) - // see `test_depositsComplete_benchmark` at: `/packages/contracts-bedrock/test/BenchmarkTest.t.sol` - Gas: 15_000, + SourceHash: source.SourceHash(), + From: L1InfoDepositerAddress, + To: &L1BlockAddress, + Mint: nil, + Value: big.NewInt(0), + Gas: DepositsCompleteGas, IsSystemTransaction: false, Data: DepositsCompleteBytes4, } diff --git a/op-node/rollup/derive/l1_block_info_test.go b/op-node/rollup/derive/l1_block_info_test.go index e5c9253ce1c6..0f8473099957 100644 --- a/op-node/rollup/derive/l1_block_info_test.go +++ b/op-node/rollup/derive/l1_block_info_test.go @@ -41,6 +41,7 @@ func TestParseL1InfoDepositTxData(t *testing.T) { randomSeqNr := func(rng *rand.Rand) uint64 { return rng.Uint64() } + // Go 1.18 will have native fuzzing for us to use, until then, we cover just the below cases cases := []infoTest{ {"random", testutils.MakeBlockInfo(nil), randomL1Cfg, randomSeqNr}, @@ -109,10 +110,8 @@ func TestParseL1InfoDepositTxData(t *testing.T) { t.Run("regolith", func(t *testing.T) { rng := rand.New(rand.NewSource(1234)) info := testutils.MakeBlockInfo(nil)(rng) - zero := uint64(0) - rollupCfg := rollup.Config{ - RegolithTime: &zero, - } + rollupCfg := rollup.Config{} + rollupCfg.AtHardfork(rollup.Regolith) depTx, err := L1InfoDeposit(&rollupCfg, randomL1Cfg(rng, info), randomSeqNr(rng), info, 0) require.NoError(t, err) require.False(t, depTx.IsSystemTransaction) @@ -121,11 +120,8 @@ func TestParseL1InfoDepositTxData(t *testing.T) { t.Run("ecotone", func(t *testing.T) { rng := rand.New(rand.NewSource(1234)) info := testutils.MakeBlockInfo(nil)(rng) - zero := uint64(0) - rollupCfg := rollup.Config{ - RegolithTime: &zero, - EcotoneTime: &zero, - } + rollupCfg := rollup.Config{} + rollupCfg.AtHardfork(rollup.Ecotone) depTx, err := L1InfoDeposit(&rollupCfg, randomL1Cfg(rng, info), randomSeqNr(rng), info, 1) require.NoError(t, err) require.False(t, depTx.IsSystemTransaction) @@ -135,12 +131,8 @@ func TestParseL1InfoDepositTxData(t *testing.T) { t.Run("first-block ecotone", func(t *testing.T) { rng := rand.New(rand.NewSource(1234)) info := testutils.MakeBlockInfo(nil)(rng) - zero := uint64(2) - rollupCfg := rollup.Config{ - RegolithTime: &zero, - EcotoneTime: &zero, - BlockTime: 2, - } + rollupCfg := rollup.Config{} + rollupCfg.AfterHardfork(rollup.Ecotone, 2) depTx, err := L1InfoDeposit(&rollupCfg, randomL1Cfg(rng, info), randomSeqNr(rng), info, 2) require.NoError(t, err) require.False(t, depTx.IsSystemTransaction) @@ -150,16 +142,75 @@ func TestParseL1InfoDepositTxData(t *testing.T) { t.Run("genesis-block ecotone", func(t *testing.T) { rng := rand.New(rand.NewSource(1234)) info := testutils.MakeBlockInfo(nil)(rng) - zero := uint64(0) - rollupCfg := rollup.Config{ - RegolithTime: &zero, - EcotoneTime: &zero, - BlockTime: 2, - } + rollupCfg := rollup.Config{} + rollupCfg.AfterHardfork(rollup.Ecotone, 2) + depTx, err := L1InfoDeposit(&rollupCfg, randomL1Cfg(rng, info), randomSeqNr(rng), info, 0) + require.NoError(t, err) + require.False(t, depTx.IsSystemTransaction) + require.Equal(t, depTx.Gas, uint64(RegolithSystemTxGas)) + require.Equal(t, L1InfoEcotoneLen, len(depTx.Data)) + }) + t.Run("Isthmus", func(t *testing.T) { + rng := rand.New(rand.NewSource(1234)) + info := testutils.MakeBlockInfo(nil)(rng) + rollupCfg := rollup.Config{} + rollupCfg.AtHardfork(rollup.Interop) + depTx, err := L1InfoDeposit(&rollupCfg, randomL1Cfg(rng, info), randomSeqNr(rng), info, 1) + require.NoError(t, err) + require.False(t, depTx.IsSystemTransaction) + require.Equal(t, depTx.Gas, uint64(RegolithSystemTxGas)) + require.Equal(t, L1InfoEcotoneLen, len(depTx.Data)) + require.Equal(t, L1InfoFuncEcotoneBytes4, depTx.Data[:4]) + // should still send Ecotone signature since upgrade happens after deposits + }) + t.Run("first-block isthmus", func(t *testing.T) { + rng := rand.New(rand.NewSource(1234)) + info := testutils.MakeBlockInfo(nil)(rng) + rollupCfg := rollup.Config{} + rollupCfg.AfterHardfork(rollup.Interop, 2) + depTx, err := L1InfoDeposit(&rollupCfg, randomL1Cfg(rng, info), randomSeqNr(rng), info, 2) + require.NoError(t, err) + require.False(t, depTx.IsSystemTransaction) + require.Equal(t, depTx.Gas, uint64(RegolithSystemTxGas)) + require.Equal(t, L1InfoBedrockLen, len(depTx.Data)) + require.Equal(t, L1InfoFuncIsthmusBytes4, depTx.Data[:4]) + }) + t.Run("genesis-block isthmus", func(t *testing.T) { + rng := rand.New(rand.NewSource(1234)) + info := testutils.MakeBlockInfo(nil)(rng) + rollupCfg := rollup.Config{} + rollupCfg.AfterHardfork(rollup.Interop, 2) depTx, err := L1InfoDeposit(&rollupCfg, randomL1Cfg(rng, info), randomSeqNr(rng), info, 0) require.NoError(t, err) require.False(t, depTx.IsSystemTransaction) require.Equal(t, depTx.Gas, uint64(RegolithSystemTxGas)) require.Equal(t, L1InfoEcotoneLen, len(depTx.Data)) + require.Equal(t, L1InfoBedrockLen, len(depTx.Data)) + }) +} + +func TestDepositsCompleteBytes(t *testing.T) { + randomSeqNr := func(rng *rand.Rand) uint64 { + return rng.Uint64() + } + t.Run("valid return bytes", func(t *testing.T) { + rng := rand.New(rand.NewSource(1234)) + info := testutils.MakeBlockInfo(nil)(rng) + depTxByes, err := DepositsCompleteBytes(randomSeqNr(rng), info) + require.NoError(t, err) + require.Equal(t, depTxByes, DepositsCompleteBytes4) + require.Equal(t, DepositsCompleteLen, len(depTxByes)) + }) + t.Run("valid return Transaction", func(t *testing.T) { + rng := rand.New(rand.NewSource(1234)) + info := testutils.MakeBlockInfo(nil)(rng) + depTx, err := DepositsCompleteDeposit(randomSeqNr(rng), info) + require.NoError(t, err) + require.Equal(t, depTx.Data, DepositsCompleteBytes4) + require.Equal(t, DepositsCompleteLen, len(depTx.Data)) + require.Equal(t, DepositsCompleteGas, depTx.Gas) + require.False(t, depTx.IsSystemTransaction) + require.Equal(t, depTx.Value, big.NewInt(0)) + require.Equal(t, depTx.From, L1InfoDepositerAddress) }) } diff --git a/op-node/rollup/types.go b/op-node/rollup/types.go index 72c3d849625f..56cdcedfde22 100644 --- a/op-node/rollup/types.go +++ b/op-node/rollup/types.go @@ -474,6 +474,44 @@ func (c *Config) IsInteropActivationBlock(l2BlockTime uint64) bool { !c.IsInterop(l2BlockTime-c.BlockTime) } +func (c *Config) AtHardfork(hardfork ForkName) { + zero := uint64(0) + // IMPORTANT! ordered from newest to oldest + switch hardfork { + case Interop: + c.InteropTime = &zero + fallthrough + case Holocene: + c.HoloceneTime = &zero + fallthrough + case Granite: + c.GraniteTime = &zero + fallthrough + case Fjord: + c.FjordTime = &zero + fallthrough + case Ecotone: + c.EcotoneTime = &zero + fallthrough + case Delta: + c.DeltaTime = &zero + fallthrough + case Canyon: + c.CanyonTime = &zero + fallthrough + case Regolith: + c.RegolithTime = &zero + fallthrough + case None: + break + } +} +func (c *Config) AfterHardfork(hardfork ForkName, timestamp uint64) { + c.AtHardfork(hardfork) + // required after hardfork time bump + c.BlockTime = timestamp +} + // ForkchoiceUpdatedVersion returns the EngineAPIMethod suitable for the chain hard fork version. func (c *Config) ForkchoiceUpdatedVersion(attr *eth.PayloadAttributes) eth.EngineAPIMethod { if attr == nil {