From af742841fe006f33656531eef612c6d0724e07cc Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 14 May 2025 10:16:40 -0400 Subject: [PATCH] feat: update StandardValidator for U16 --- op-validator/pkg/validations/codes.go | 30 +++- .../interfaces/L1/IStandardValidator.sol | 3 + .../deploy/DeployStandardValidator.s.sol | 10 ++ .../snapshots/abi/StandardValidator.json | 31 ++++ .../storageLayout/StandardValidator.json | 23 ++- .../src/L1/StandardValidator.sol | 87 ++++++--- .../test/L1/StandardValidator.t.sol | 165 ++++++++++++++++-- .../test/universal/Specs.t.sol | 2 + 8 files changed, 299 insertions(+), 52 deletions(-) diff --git a/op-validator/pkg/validations/codes.go b/op-validator/pkg/validations/codes.go index c75fd63c4aeb9..731366ee5f722 100644 --- a/op-validator/pkg/validations/codes.go +++ b/op-validator/pkg/validations/codes.go @@ -18,6 +18,10 @@ var descriptions = map[string]string{ "SYSCON-80": "SystemConfig systemTxMaxGas is not set to 1,000,000", "SYSCON-90": "SystemConfig minimumBaseFee is not set to 1 gwei", "SYSCON-100": "SystemConfig maximumBaseFee is not set to max uint128", + "SYSCON-110": "SystemConfig operatorFeeScalar is not set to 0", + "SYSCON-120": "SystemConfig operatorFeeConstant is not set to 0", + "SYSCON-130": "SystemConfig proxyAdmin is invalid", + "SYSCON-140": "SystemConfig superchainConfig is invalid", // L1 Cross Domain Messenger validations "L1xDM-10": "L1CrossDomainMessenger version mismatch", @@ -26,7 +30,8 @@ var descriptions = map[string]string{ "L1xDM-40": "L1CrossDomainMessenger otherMessenger address mismatch", "L1xDM-50": "L1CrossDomainMessenger PORTAL address mismatch", "L1xDM-60": "L1CrossDomainMessenger portal address mismatch", - "L1xDM-70": "L1CrossDomainMessenger superchainConfig address mismatch", + "L1xDM-70": "L1CrossDomainMessenger systemConfig address mismatch", + "L1xDM-80": "L1CrossDomainMessenger proxyAdmin is invalid", // L1 Standard Bridge validations "L1SB-10": "L1StandardBridge version mismatch", @@ -35,7 +40,8 @@ var descriptions = map[string]string{ "L1SB-40": "L1StandardBridge messenger address mismatch", "L1SB-50": "L1StandardBridge OTHER_BRIDGE address mismatch", "L1SB-60": "L1StandardBridge otherBridge address mismatch", - "L1SB-70": "L1StandardBridge superchainConfig address mismatch", + "L1SB-70": "L1StandardBridge systemConfig address mismatch", + "L1SB-80": "L1StandardBridge proxyAdmin is invalid", // Optimism Mintable ERC20 Factory validations "MERC20F-10": "OptimismMintableERC20Factory version mismatch", @@ -50,21 +56,29 @@ var descriptions = map[string]string{ "L721B-40": "L1ERC721Bridge otherBridge address mismatch", "L721B-50": "L1ERC721Bridge MESSENGER address mismatch", "L721B-60": "L1ERC721Bridge messenger address mismatch", - "L721B-70": "L1ERC721Bridge superchainConfig address mismatch", + "L721B-70": "L1ERC721Bridge systemConfig address mismatch", + "L721B-80": "L1ERC721Bridge proxyAdmin is invalid", // Optimism Portal validations "PORTAL-10": "OptimismPortal version mismatch", "PORTAL-20": "OptimismPortal implementation address mismatch", "PORTAL-30": "OptimismPortal disputeGameFactory address mismatch", "PORTAL-40": "OptimismPortal systemConfig address mismatch", - "PORTAL-50": "OptimismPortal superchainConfig address mismatch", - "PORTAL-60": "OptimismPortal guardian address mismatch", "PORTAL-80": "OptimismPortal l2Sender not set to default value", + "PORTAL-90": "OptimismPortal proxyAdmin is invalid", // Dispute Factory validations "DF-10": "DisputeGameFactory version mismatch", "DF-20": "DisputeGameFactory implementation address mismatch", "DF-30": "DisputeGameFactory owner is not set to L1 PAO multisig", + "DF-40": "DisputeGameFactory proxyAdmin is invalid", + + // ETHLockbox validations + "LOCKBOX-10": "ETHLockbox version mismatch", + "LOCKBOX-20": "ETHLockbox implementation address mismatch", + "LOCKBOX-30": "ETHLockbox proxyAdmin is invalid", + "LOCKBOX-40": "ETHLockbox systemConfig address mismatch", + "LOCKBOX-50": "ETHLockbox authorizedPortals mismatch", // Permissioned Dispute Game validations "PDDG-10": "Permissioned dispute game implementation not found", @@ -98,10 +112,14 @@ var descriptions = map[string]string{ "PDDG-DWETH-20": "Permissioned dispute game delayed WETH implementation address mismatch", "PDDG-DWETH-30": "Permissioned dispute game delayed WETH owner mismatch", "PDDG-DWETH-40": "Permissioned dispute game delayed WETH delay not set to 1 week", + "PDDG-DWETH-50": "Permissioned dispute game delayed WETH system config address mismatch", + "PDDG-DWETH-60": "Permissioned dispute game delayed WETH proxy admin mismatch", "PLDG-DWETH-10": "Permissionless dispute game delayed WETH version mismatch", "PLDG-DWETH-20": "Permissionless dispute game delayed WETH implementation address mismatch", "PLDG-DWETH-30": "Permissionless dispute game delayed WETH owner mismatch", "PLDG-DWETH-40": "Permissionless dispute game delayed WETH delay not set to 1 week", + "PLDG-DWETH-50": "Permissionless dispute game delayed WETH system config address mismatch", + "PLDG-DWETH-60": "Permissionless dispute game delayed WETH proxy admin mismatch", // Anchor State Registry validations (for both PDDG and PLDG) "PDDG-ANCHORP-10": "Permissioned dispute game anchor state registry version mismatch", @@ -109,11 +127,13 @@ var descriptions = map[string]string{ "PDDG-ANCHORP-30": "Permissioned dispute game anchor state registry dispute game factory address mismatch", "PDDG-ANCHORP-40": "Permissioned dispute game anchor state registry root hash mismatch", "PDDG-ANCHORP-50": "Permissioned dispute game anchor state registry superchain config address mismatch", + "PDDG-ANCHORP-60": "Permissioned dispute game anchor state registry retirement timestamp is not set", "PLDG-ANCHORP-10": "Permissionless dispute game anchor state registry version mismatch", "PLDG-ANCHORP-20": "Permissionless dispute game anchor state registry implementation address mismatch", "PLDG-ANCHORP-30": "Permissionless dispute game anchor state registry dispute game factory address mismatch", "PLDG-ANCHORP-40": "Permissionless dispute game anchor state registry root hash mismatch", "PLDG-ANCHORP-50": "Permissionless dispute game anchor state registry superchain config address mismatch", + "PLDG-ANCHORP-60": "Permissionless dispute game anchor state registry retirement timestamp is not set", // Preimage Oracle validations (for both PDDG and PLDG) "PDDG-PIMGO-10": "Permissioned dispute game preimage oracle version mismatch", diff --git a/packages/contracts-bedrock/interfaces/L1/IStandardValidator.sol b/packages/contracts-bedrock/interfaces/L1/IStandardValidator.sol index 3e7951b895052..8705a4cfe52de 100644 --- a/packages/contracts-bedrock/interfaces/L1/IStandardValidator.sol +++ b/packages/contracts-bedrock/interfaces/L1/IStandardValidator.sol @@ -10,6 +10,7 @@ interface IStandardValidator { struct Implementations { address l1ERC721BridgeImpl; address optimismPortalImpl; + address ethLockboxImpl; address systemConfigImpl; address optimismMintableERC20FactoryImpl; address l1CrossDomainMessengerImpl; @@ -52,6 +53,8 @@ interface IStandardValidator { function optimismMintableERC20FactoryVersion() external pure returns (string memory); function optimismPortalImpl() external view returns (address); function optimismPortalVersion() external pure returns (string memory); + function ethLockboxImpl() external view returns (address); + function ethLockboxVersion() external pure returns (string memory); function permissionedDisputeGameVersion() external pure returns (string memory); function preimageOracleVersion() external pure returns (string memory); function superchainConfig() external view returns (ISuperchainConfig); diff --git a/packages/contracts-bedrock/scripts/deploy/DeployStandardValidator.s.sol b/packages/contracts-bedrock/scripts/deploy/DeployStandardValidator.s.sol index bcffe5b32ce6f..a3e95a2927e92 100644 --- a/packages/contracts-bedrock/scripts/deploy/DeployStandardValidator.s.sol +++ b/packages/contracts-bedrock/scripts/deploy/DeployStandardValidator.s.sol @@ -29,6 +29,7 @@ contract DeployStandardValidatorInput is BaseDeployIO { address internal _protocolVersionsImpl; address internal _l1ERC721BridgeImpl; address internal _optimismPortalImpl; + address internal _ethLockboxImpl; address internal _systemConfigImpl; address internal _optimismMintableERC20FactoryImpl; address internal _l1CrossDomainMessengerImpl; @@ -60,6 +61,9 @@ contract DeployStandardValidatorInput is BaseDeployIO { } else if (_sel == this.optimismPortalImpl.selector) { require(_value != address(0), "DeployStandardValidator: optimismPortalImpl cannot be empty"); _optimismPortalImpl = _value; + } else if (_sel == this.ethLockboxImpl.selector) { + require(_value != address(0), "DeployStandardValidator: ethLockboxImpl cannot be empty"); + _ethLockboxImpl = _value; } else if (_sel == this.systemConfigImpl.selector) { require(_value != address(0), "DeployStandardValidator: systemConfigImpl cannot be empty"); _systemConfigImpl = _value; @@ -146,6 +150,11 @@ contract DeployStandardValidatorInput is BaseDeployIO { return _optimismPortalImpl; } + function ethLockboxImpl() public view returns (address) { + require(_ethLockboxImpl != address(0), "DeployStandardValidator: ethLockboxImpl not set"); + return _ethLockboxImpl; + } + function systemConfigImpl() public view returns (address) { require(_systemConfigImpl != address(0), "DeployStandardValidator: systemConfigImpl not set"); return _systemConfigImpl; @@ -231,6 +240,7 @@ contract DeployStandardValidator is Script { return IStandardValidator.Implementations({ l1ERC721BridgeImpl: _si.l1ERC721BridgeImpl(), optimismPortalImpl: _si.optimismPortalImpl(), + ethLockboxImpl: _si.ethLockboxImpl(), systemConfigImpl: _si.systemConfigImpl(), optimismMintableERC20FactoryImpl: _si.optimismMintableERC20FactoryImpl(), l1CrossDomainMessengerImpl: _si.l1CrossDomainMessengerImpl(), diff --git a/packages/contracts-bedrock/snapshots/abi/StandardValidator.json b/packages/contracts-bedrock/snapshots/abi/StandardValidator.json index 627202995855b..4db63429cbf02 100644 --- a/packages/contracts-bedrock/snapshots/abi/StandardValidator.json +++ b/packages/contracts-bedrock/snapshots/abi/StandardValidator.json @@ -13,6 +13,11 @@ "name": "optimismPortalImpl", "type": "address" }, + { + "internalType": "address", + "name": "ethLockboxImpl", + "type": "address" + }, { "internalType": "address", "name": "systemConfigImpl", @@ -173,6 +178,32 @@ "stateMutability": "pure", "type": "function" }, + { + "inputs": [], + "name": "ethLockboxImpl", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "ethLockboxVersion", + "outputs": [ + { + "internalType": "string", + "name": "", + "type": "string" + } + ], + "stateMutability": "pure", + "type": "function" + }, { "inputs": [], "name": "l1CrossDomainMessengerImpl", diff --git a/packages/contracts-bedrock/snapshots/storageLayout/StandardValidator.json b/packages/contracts-bedrock/snapshots/storageLayout/StandardValidator.json index 92946dc3b4d30..6ac032e39f1b9 100644 --- a/packages/contracts-bedrock/snapshots/storageLayout/StandardValidator.json +++ b/packages/contracts-bedrock/snapshots/storageLayout/StandardValidator.json @@ -43,58 +43,65 @@ }, { "bytes": "20", - "label": "systemConfigImpl", + "label": "ethLockboxImpl", "offset": 0, "slot": "6", "type": "address" }, { "bytes": "20", - "label": "optimismMintableERC20FactoryImpl", + "label": "systemConfigImpl", "offset": 0, "slot": "7", "type": "address" }, { "bytes": "20", - "label": "l1CrossDomainMessengerImpl", + "label": "optimismMintableERC20FactoryImpl", "offset": 0, "slot": "8", "type": "address" }, { "bytes": "20", - "label": "l1StandardBridgeImpl", + "label": "l1CrossDomainMessengerImpl", "offset": 0, "slot": "9", "type": "address" }, { "bytes": "20", - "label": "disputeGameFactoryImpl", + "label": "l1StandardBridgeImpl", "offset": 0, "slot": "10", "type": "address" }, { "bytes": "20", - "label": "anchorStateRegistryImpl", + "label": "disputeGameFactoryImpl", "offset": 0, "slot": "11", "type": "address" }, { "bytes": "20", - "label": "delayedWETHImpl", + "label": "anchorStateRegistryImpl", "offset": 0, "slot": "12", "type": "address" }, { "bytes": "20", - "label": "mipsImpl", + "label": "delayedWETHImpl", "offset": 0, "slot": "13", "type": "address" + }, + { + "bytes": "20", + "label": "mipsImpl", + "offset": 0, + "slot": "14", + "type": "address" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L1/StandardValidator.sol b/packages/contracts-bedrock/src/L1/StandardValidator.sol index e899b548c3854..ee15d123485be 100644 --- a/packages/contracts-bedrock/src/L1/StandardValidator.sol +++ b/packages/contracts-bedrock/src/L1/StandardValidator.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.15; // Libraries +import { LibString } from "@solady/utils/LibString.sol"; import { GameType, Claim, GameTypes } from "src/dispute/lib/Types.sol"; import { Duration } from "src/dispute/lib/LibUDT.sol"; import { Predeploys } from "src/libraries/Predeploys.sol"; @@ -16,6 +17,7 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { IOptimismMintableERC20Factory } from "interfaces/universal/IOptimismMintableERC20Factory.sol"; import { IL1StandardBridge } from "interfaces/L1/IL1StandardBridge.sol"; import { IL1ERC721Bridge } from "interfaces/L1/IL1ERC721Bridge.sol"; +import { IETHLockbox } from "interfaces/L1/IETHLockbox.sol"; import { IPermissionedDisputeGame } from "interfaces/dispute/IPermissionedDisputeGame.sol"; import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol"; import { IDelayedWETH } from "interfaces/dispute/IDelayedWETH.sol"; @@ -33,6 +35,7 @@ contract StandardValidator { // Implementation addresses as state variables address public l1ERC721BridgeImpl; address public optimismPortalImpl; + address public ethLockboxImpl; address public systemConfigImpl; address public optimismMintableERC20FactoryImpl; address public l1CrossDomainMessengerImpl; @@ -45,6 +48,7 @@ contract StandardValidator { struct Implementations { address l1ERC721BridgeImpl; address optimismPortalImpl; + address ethLockboxImpl; address systemConfigImpl; address optimismMintableERC20FactoryImpl; address l1CrossDomainMessengerImpl; @@ -82,6 +86,7 @@ contract StandardValidator { // Set implementation addresses from struct l1ERC721BridgeImpl = _implementations.l1ERC721BridgeImpl; optimismPortalImpl = _implementations.optimismPortalImpl; + ethLockboxImpl = _implementations.ethLockboxImpl; systemConfigImpl = _implementations.systemConfigImpl; optimismMintableERC20FactoryImpl = _implementations.optimismMintableERC20FactoryImpl; l1CrossDomainMessengerImpl = _implementations.l1CrossDomainMessengerImpl; @@ -169,6 +174,10 @@ contract StandardValidator { return "1.1.4"; } + function ethLockboxVersion() public pure returns (string memory) { + return "1.2.0"; + } + function assertValidSuperchainConfig(string memory _errors) internal view returns (string memory) { _errors = internalRequire(!superchainConfig.paused(address(0)), "SPRCFG-10", _errors); return _errors; @@ -199,7 +208,7 @@ contract StandardValidator { returns (string memory) { ISemver _semver = ISemver(address(_sysCfg)); - _errors = internalRequire(stringEq(_semver.version(), systemConfigVersion()), "SYSCON-10", _errors); + _errors = internalRequire(LibString.eq(_semver.version(), systemConfigVersion()), "SYSCON-10", _errors); _errors = internalRequire(_sysCfg.gasLimit() <= uint64(200_000_000), "SYSCON-20", _errors); _errors = internalRequire(_sysCfg.scalar() != 0, "SYSCON-30", _errors); _errors = @@ -212,9 +221,10 @@ contract StandardValidator { _errors = internalRequire(outputConfig.systemTxMaxGas == 1_000_000, "SYSCON-80", _errors); _errors = internalRequire(outputConfig.minimumBaseFee == 1 gwei, "SYSCON-90", _errors); _errors = internalRequire(outputConfig.maximumBaseFee == type(uint128).max, "SYSCON-100", _errors); - _errors = internalRequire(_sysCfg.superchainConfig() == superchainConfig, "SYSCON-110", _errors); _errors = internalRequire(_sysCfg.operatorFeeScalar() == 0, "SYSCON-110", _errors); _errors = internalRequire(_sysCfg.operatorFeeConstant() == 0, "SYSCON-120", _errors); + _errors = internalRequire(_sysCfg.proxyAdmin() == _admin, "SYSCON-130", _errors); + _errors = internalRequire(_sysCfg.superchainConfig() == superchainConfig, "SYSCON-140", _errors); return _errors; } @@ -228,7 +238,8 @@ contract StandardValidator { returns (string memory) { IL1CrossDomainMessenger _messenger = IL1CrossDomainMessenger(_sysCfg.l1CrossDomainMessenger()); - _errors = internalRequire(stringEq(_messenger.version(), l1CrossDomainMessengerVersion()), "L1xDM-10", _errors); + _errors = + internalRequire(LibString.eq(_messenger.version(), l1CrossDomainMessengerVersion()), "L1xDM-10", _errors); _errors = internalRequire( _admin.getProxyImplementation(address(_messenger)) == l1CrossDomainMessengerImpl, "L1xDM-20", _errors ); @@ -244,6 +255,7 @@ contract StandardValidator { _errors = internalRequire(address(_messenger.PORTAL()) == address(_portal), "L1xDM-50", _errors); _errors = internalRequire(address(_messenger.portal()) == address(_portal), "L1xDM-60", _errors); _errors = internalRequire(address(_messenger.systemConfig()) == address(_sysCfg), "L1xDM-70", _errors); + _errors = internalRequire(_messenger.proxyAdmin() == _admin, "L1xDM-80", _errors); return _errors; } @@ -257,7 +269,7 @@ contract StandardValidator { returns (string memory) { IL1StandardBridge _bridge = IL1StandardBridge(payable(_sysCfg.l1StandardBridge())); - _errors = internalRequire(stringEq(_bridge.version(), l1StandardBridgeVersion()), "L1SB-10", _errors); + _errors = internalRequire(LibString.eq(_bridge.version(), l1StandardBridgeVersion()), "L1SB-10", _errors); _errors = internalRequire(_admin.getProxyImplementation(address(_bridge)) == l1StandardBridgeImpl, "L1SB-20", _errors); @@ -268,6 +280,7 @@ contract StandardValidator { _errors = internalRequire(address(_bridge.OTHER_BRIDGE()) == Predeploys.L2_STANDARD_BRIDGE, "L1SB-50", _errors); _errors = internalRequire(address(_bridge.otherBridge()) == Predeploys.L2_STANDARD_BRIDGE, "L1SB-60", _errors); _errors = internalRequire(address(_bridge.systemConfig()) == address(_sysCfg), "L1SB-70", _errors); + _errors = internalRequire(_bridge.proxyAdmin() == _admin, "L1SB-80", _errors); return _errors; } @@ -281,8 +294,9 @@ contract StandardValidator { returns (string memory) { IOptimismMintableERC20Factory _factory = IOptimismMintableERC20Factory(_sysCfg.optimismMintableERC20Factory()); - _errors = - internalRequire(stringEq(_factory.version(), optimismMintableERC20FactoryVersion()), "MERC20F-10", _errors); + _errors = internalRequire( + LibString.eq(_factory.version(), optimismMintableERC20FactoryVersion()), "MERC20F-10", _errors + ); _errors = internalRequire( _admin.getProxyImplementation(address(_factory)) == optimismMintableERC20FactoryImpl, "MERC20F-20", _errors ); @@ -303,7 +317,7 @@ contract StandardValidator { returns (string memory) { IL1ERC721Bridge _bridge = IL1ERC721Bridge(_sysCfg.l1ERC721Bridge()); - _errors = internalRequire(stringEq(_bridge.version(), l1ERC721BridgeVersion()), "L721B-10", _errors); + _errors = internalRequire(LibString.eq(_bridge.version(), l1ERC721BridgeVersion()), "L721B-10", _errors); _errors = internalRequire(_admin.getProxyImplementation(address(_bridge)) == l1ERC721BridgeImpl, "L721B-20", _errors); @@ -313,6 +327,7 @@ contract StandardValidator { _errors = internalRequire(address(_bridge.MESSENGER()) == address(_l1XDM), "L721B-50", _errors); _errors = internalRequire(address(_bridge.messenger()) == address(_l1XDM), "L721B-60", _errors); _errors = internalRequire(address(_bridge.systemConfig()) == address(_sysCfg), "L721B-70", _errors); + _errors = internalRequire(_bridge.proxyAdmin() == _admin, "L721B-80", _errors); return _errors; } @@ -326,7 +341,7 @@ contract StandardValidator { returns (string memory) { IOptimismPortal2 _portal = IOptimismPortal2(payable(_sysCfg.optimismPortal())); - _errors = internalRequire(stringEq(_portal.version(), optimismPortalVersion()), "PORTAL-10", _errors); + _errors = internalRequire(LibString.eq(_portal.version(), optimismPortalVersion()), "PORTAL-10", _errors); _errors = internalRequire(_admin.getProxyImplementation(address(_portal)) == optimismPortalImpl, "PORTAL-20", _errors); @@ -334,6 +349,28 @@ contract StandardValidator { _errors = internalRequire(address(_portal.disputeGameFactory()) == address(_dgf), "PORTAL-30", _errors); _errors = internalRequire(address(_portal.systemConfig()) == address(_sysCfg), "PORTAL-40", _errors); _errors = internalRequire(_portal.l2Sender() == Constants.DEFAULT_L2_SENDER, "PORTAL-80", _errors); + _errors = internalRequire(_portal.proxyAdmin() == _admin, "PORTAL-90", _errors); + return _errors; + } + + function assertValidETHLockbox( + string memory _errors, + ISystemConfig _sysCfg, + IProxyAdmin _admin + ) + internal + view + returns (string memory) + { + IOptimismPortal2 _portal = IOptimismPortal2(payable(_sysCfg.optimismPortal())); + IETHLockbox _lockbox = IETHLockbox(_portal.ethLockbox()); + + _errors = internalRequire(LibString.eq(_lockbox.version(), ethLockboxVersion()), "LOCKBOX-10", _errors); + _errors = + internalRequire(_admin.getProxyImplementation(address(_lockbox)) == ethLockboxImpl, "LOCKBOX-20", _errors); + _errors = internalRequire(_lockbox.proxyAdmin() == _admin, "LOCKBOX-30", _errors); + _errors = internalRequire(_lockbox.systemConfig() == _sysCfg, "LOCKBOX-40", _errors); + _errors = internalRequire(_lockbox.authorizedPortals(_portal), "LOCKBOX-50", _errors); return _errors; } @@ -347,14 +384,14 @@ contract StandardValidator { view returns (string memory) { + address _l1PAOMultisig = expectedL1PAOMultisig(_overrides); IDisputeGameFactory _factory = IDisputeGameFactory(_sysCfg.disputeGameFactory()); - _errors = internalRequire(stringEq(_factory.version(), disputeGameFactoryVersion()), "DF-10", _errors); + _errors = internalRequire(LibString.eq(_factory.version(), disputeGameFactoryVersion()), "DF-10", _errors); _errors = internalRequire( _admin.getProxyImplementation(address(_factory)) == disputeGameFactoryImpl, "DF-20", _errors ); - - address _l1PAOMultisig = expectedL1PAOMultisig(_overrides); _errors = internalRequire(_factory.owner() == _l1PAOMultisig, "DF-30", _errors); + _errors = internalRequire(_factory.proxyAdmin() == _admin, "DF-40", _errors); return _errors; } @@ -456,7 +493,7 @@ contract StandardValidator { bool validGameVM = address(_game.vm()) == address(mipsImpl); _errors = internalRequire( - stringEq(_game.version(), permissionedDisputeGameVersion()), string.concat(_errorPrefix, "-20"), _errors + LibString.eq(_game.version(), permissionedDisputeGameVersion()), string.concat(_errorPrefix, "-20"), _errors ); IAnchorStateRegistry _asr = _game.anchorStateRegistry(); _errors = internalRequire( @@ -503,7 +540,7 @@ contract StandardValidator { { _errorPrefix = string.concat(_errorPrefix, "-DWETH"); _errors = internalRequire( - stringEq(_weth.version(), delayedWETHVersion()), string.concat(_errorPrefix, "-10"), _errors + LibString.eq(_weth.version(), delayedWETHVersion()), string.concat(_errorPrefix, "-10"), _errors ); _errors = internalRequire( _admin.getProxyImplementation(address(_weth)) == delayedWETHImpl, @@ -515,6 +552,7 @@ contract StandardValidator { internalRequire(_weth.proxyAdminOwner() == _l1PAOMultisig, string.concat(_errorPrefix, "-30"), _errors); _errors = internalRequire(_weth.delay() == withdrawalDelaySeconds, string.concat(_errorPrefix, "-40"), _errors); _errors = internalRequire(_weth.systemConfig() == _sysCfg, string.concat(_errorPrefix, "-50"), _errors); + _errors = internalRequire(_weth.proxyAdmin() == _admin, string.concat(_errorPrefix, "-60"), _errors); return _errors; } @@ -533,21 +571,19 @@ contract StandardValidator { { _errorPrefix = string.concat(_errorPrefix, "-ANCHORP"); _errors = internalRequire( - stringEq(_asr.version(), anchorStateRegistryVersion()), string.concat(_errorPrefix, "-10"), _errors - ); - _errors = internalRequire( - address(_asr.disputeGameFactory()) == address(_dgf), string.concat(_errorPrefix, "-30"), _errors - ); - - _errors = internalRequire(_asr.systemConfig() == _sysCfg, string.concat(_errorPrefix, "-50"), _errors); - _errors = internalRequire( - address(_asr.superchainConfig()) == address(superchainConfig), string.concat(_errorPrefix, "-50"), _errors + LibString.eq(_asr.version(), anchorStateRegistryVersion()), string.concat(_errorPrefix, "-10"), _errors ); _errors = internalRequire( _admin.getProxyImplementation(address(_asr)) == anchorStateRegistryImpl, string.concat(_errorPrefix, "-20"), _errors ); + _errors = internalRequire( + address(_asr.disputeGameFactory()) == address(_dgf), string.concat(_errorPrefix, "-30"), _errors + ); + _errors = internalRequire(_asr.systemConfig() == _sysCfg, string.concat(_errorPrefix, "-40"), _errors); + _errors = internalRequire(_asr.proxyAdmin() == _admin, string.concat(_errorPrefix, "-50"), _errors); + _errors = internalRequire(_asr.retirementTimestamp() > 0, string.concat(_errorPrefix, "-60"), _errors); return _errors; } @@ -563,7 +599,7 @@ contract StandardValidator { _errorPrefix = string.concat(_errorPrefix, "-PIMGO"); // The preimage oracle's address is correct if the MIPS address is correct. _errors = internalRequire( - stringEq(_oracle.version(), preimageOracleVersion()), string.concat(_errorPrefix, "-10"), _errors + LibString.eq(_oracle.version(), preimageOracleVersion()), string.concat(_errorPrefix, "-10"), _errors ); _errors = internalRequire(_oracle.challengePeriod() == 86400, string.concat(_errorPrefix, "-20"), _errors); _errors = internalRequire(_oracle.minProposalSize() == 126000, string.concat(_errorPrefix, "-30"), _errors); @@ -590,10 +626,6 @@ contract StandardValidator { return _errors; } - function stringEq(string memory _a, string memory _b) internal pure returns (bool) { - return keccak256(bytes(_a)) == keccak256(bytes(_b)); - } - function validate(ValidationInput memory _input, bool _allowFailure) external view returns (string memory) { return validate(_input, _allowFailure, ValidationOverrides({ l1PAOMultisig: address(0), challenger: address(0) })); @@ -625,6 +657,7 @@ contract StandardValidator { _errors = assertValidPermissionlessDisputeGame( _errors, _input.sysCfg, _input.absolutePrestate, _input.l2ChainID, _input.proxyAdmin, _overrides ); + _errors = assertValidETHLockbox(_errors, _input.sysCfg, _input.proxyAdmin); string memory overridesString = getOverridesString(_overrides); string memory finalErrors = _errors; diff --git a/packages/contracts-bedrock/test/L1/StandardValidator.t.sol b/packages/contracts-bedrock/test/L1/StandardValidator.t.sol index c42d13375c152..0fba5ab3d8628 100644 --- a/packages/contracts-bedrock/test/L1/StandardValidator.t.sol +++ b/packages/contracts-bedrock/test/L1/StandardValidator.t.sol @@ -20,6 +20,7 @@ import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol"; import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol"; import { ISemver } from "interfaces/universal/ISemver.sol"; import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol"; +import { IETHLockbox } from "interfaces/L1/IETHLockbox.sol"; import { IResourceMetering } from "interfaces/L1/IResourceMetering.sol"; import { ICrossDomainMessenger } from "interfaces/universal/ICrossDomainMessenger.sol"; import { IL1CrossDomainMessenger } from "interfaces/L1/IL1CrossDomainMessenger.sol"; @@ -141,6 +142,7 @@ contract StandardValidator_TestInit is CommonTest { IStandardValidator.Implementations({ systemConfigImpl: impls.systemConfigImpl, optimismPortalImpl: impls.optimismPortalImpl, + ethLockboxImpl: impls.ethLockboxImpl, l1CrossDomainMessengerImpl: impls.l1CrossDomainMessengerImpl, l1StandardBridgeImpl: impls.l1StandardBridgeImpl, l1ERC721BridgeImpl: impls.l1ERC721BridgeImpl, @@ -397,12 +399,22 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { assertEq("SYSCON-120", _validate(true)); } - /// @notice Tests that the validate function successfully returns the right error when both - /// SystemConfig operatorFeeScalar and operatorFeeConstant are invalid. - function test_validate_systemConfigInvalidOperatorFeeScalarAndConstant_succeeds() public { - vm.mockCall(address(systemConfig), abi.encodeCall(ISystemConfig.operatorFeeScalar, ()), abi.encode(1)); - vm.mockCall(address(systemConfig), abi.encodeCall(ISystemConfig.operatorFeeConstant, ()), abi.encode(1)); - assertEq("SYSCON-110,SYSCON-120", _validate(true)); + /// @notice Tests that the validate function successfully returns the right error when the + /// SystemConfig proxyAdmin is invalid. + function test_validate_systemConfigInvalidProxyAdmin_succeeds() public { + vm.mockCall( + address(systemConfig), abi.encodeCall(IProxyAdminOwnedBase.proxyAdmin, ()), abi.encode(address(0xbad)) + ); + assertEq("SYSCON-130", _validate(true)); + } + + /// @notice Tests that the validate function successfully returns the right error when the + /// SystemConfig superchainConfig is invalid. + function test_validate_systemConfigInvalidSuperchainConfig_succeeds() public { + vm.mockCall( + address(systemConfig), abi.encodeCall(ISystemConfig.superchainConfig, ()), abi.encode(address(0xbad)) + ); + assertEq("SYSCON-140", _validate(true)); } /// @notice Tests that the validate function successfully returns the right error when the @@ -478,6 +490,17 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { assertEq("L1xDM-70", _validate(true)); } + /// @notice Tests that the validate function successfully returns the right error when the + /// L1CrossDomainMessenger proxyAdmin is invalid. + function test_validate_l1CrossDomainMessengerInvalidProxyAdmin_succeeds() public { + vm.mockCall( + address(l1CrossDomainMessenger), + abi.encodeCall(IProxyAdminOwnedBase.proxyAdmin, ()), + abi.encode(address(0xbad)) + ); + assertEq("L1xDM-80", _validate(true)); + } + /// @notice Tests that the validate function successfully returns the right error when the /// OptimismMintableERC20Factory version is invalid. function test_validate_optimismMintableERC20FactoryInvalidVersion_succeeds() public { @@ -573,6 +596,15 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { assertEq("L721B-70", _validate(true)); } + /// @notice Tests that the validate function successfully returns the right error when the + /// L1ERC721Bridge proxyAdmin is invalid. + function test_validate_l1ERC721BridgeInvalidProxyAdmin_succeeds() public { + vm.mockCall( + address(l1ERC721Bridge), abi.encodeCall(IProxyAdminOwnedBase.proxyAdmin, ()), abi.encode(address(0xbad)) + ); + assertEq("L721B-80", _validate(true)); + } + /// @notice Tests that the validate function successfully returns the right error when the /// OptimismPortal version is invalid. function test_validate_optimismPortalInvalidVersion_succeeds() public { @@ -618,6 +650,58 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { assertEq("PORTAL-80", _validate(true)); } + /// @notice Tests that the validate function successfully returns the right error when the + /// OptimismPortal proxyAdmin is invalid. + function test_validate_optimismPortalInvalidProxyAdmin_succeeds() public { + vm.mockCall( + address(optimismPortal2), abi.encodeCall(IProxyAdminOwnedBase.proxyAdmin, ()), abi.encode(address(0xbad)) + ); + assertEq("PORTAL-90", _validate(true)); + } + + /// @notice Tests that the validate function successfully returns the right error when the + /// ETHLockbox version is invalid. + function test_validate_ethLockboxInvalidVersion_succeeds() public { + vm.mockCall(address(ethLockbox), abi.encodeCall(ISemver.version, ()), abi.encode("0.0.0")); + assertEq("LOCKBOX-10", _validate(true)); + } + + /// @notice Tests that the validate function successfully returns the right error when the + /// ETHLockbox implementation is invalid. + function test_validate_ethLockboxInvalidImplementation_succeeds() public { + vm.mockCall( + address(proxyAdmin), + abi.encodeCall(IProxyAdmin.getProxyImplementation, (address(ethLockbox))), + abi.encode(address(0xbad)) + ); + assertEq("LOCKBOX-20", _validate(true)); + } + + /// @notice Tests that the validate function successfully returns the right error when the + /// ETHLockbox proxyAdmin is invalid. + function test_validate_ethLockboxInvalidProxyAdmin_succeeds() public { + vm.mockCall( + address(ethLockbox), abi.encodeCall(IProxyAdminOwnedBase.proxyAdmin, ()), abi.encode(address(0xbad)) + ); + assertEq("LOCKBOX-30", _validate(true)); + } + + /// @notice Tests that the validate function successfully returns the right error when the + /// ETHLockbox systemConfig is invalid. + function test_validate_ethLockboxInvalidSystemConfig_succeeds() public { + vm.mockCall(address(ethLockbox), abi.encodeCall(IETHLockbox.systemConfig, ()), abi.encode(address(0xbad))); + assertEq("LOCKBOX-40", _validate(true)); + } + + /// @notice Tests that the validate function successfully returns the right error when the + /// ETHLockbox does not have the OptimismPortal as an authorized portal. + function test_validate_ethLockboxPortalUnauthorized_succeeds() public { + vm.mockCall( + address(ethLockbox), abi.encodeCall(IETHLockbox.authorizedPortals, (optimismPortal2)), abi.encode(false) + ); + assertEq("LOCKBOX-50", _validate(true)); + } + /// @notice Tests that the validate function successfully returns the right error when the /// DisputeGameFactory version is invalid. function test_validate_disputeGameFactoryInvalidVersion_succeeds() public { @@ -787,6 +871,37 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { assertEq("PDDG-ANCHORP-30,PLDG-ANCHORP-30", _validate(true)); } + /// @notice Tests that the validate function successfully returns the right error when the + /// AnchorStateRegistry systemConfig is invalid. + function test_validate_anchorStateRegistryInvalidSystemConfig_succeeds() public { + vm.mockCall( + address(anchorStateRegistry), + abi.encodeCall(IAnchorStateRegistry.systemConfig, ()), + abi.encode(address(0xbad)) + ); + assertEq("PDDG-ANCHORP-40,PLDG-ANCHORP-40", _validate(true)); + } + + /// @notice Tests that the validate function successfully returns the right error when the + /// AnchorStateRegistry proxyAdmin is invalid. + function test_validate_anchorStateRegistryInvalidProxyAdmin_succeeds() public { + vm.mockCall( + address(anchorStateRegistry), + abi.encodeCall(IProxyAdminOwnedBase.proxyAdmin, ()), + abi.encode(address(0xbad)) + ); + assertEq("PDDG-ANCHORP-50,PLDG-ANCHORP-50", _validate(true)); + } + + /// @notice Tests that the validate function successfully returns the right error when the + /// AnchorStateRegistry retirementTimestamp is invalid. + function test_validate_anchorStateRegistryInvalidRetirementTimestamp_succeeds() public { + vm.mockCall( + address(anchorStateRegistry), abi.encodeCall(IAnchorStateRegistry.retirementTimestamp, ()), abi.encode(0) + ); + assertEq("PDDG-ANCHORP-60,PLDG-ANCHORP-60", _validate(true)); + } + /// @notice Tests that the validate function successfully returns the right error when the /// DelayedWETH version is invalid. function test_validate_delayedWETHInvalidVersion_succeeds() public { @@ -856,6 +971,20 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { } } + /// @notice Tests that the validate function successfully returns the right error when the + /// DelayedWETH proxyAdmin is invalid. + function test_validate_delayedWETHInvalidProxyAdmin_succeeds() public { + vm.mockCall( + address(delayedWeth), abi.encodeCall(IProxyAdminOwnedBase.proxyAdmin, ()), abi.encode(address(0xbad)) + ); + + if (isForkTest()) { + assertEq("PDDG-DWETH-60", _validate(true)); + } else { + assertEq("PLDG-DWETH-60", _validate(true)); + } + } + /// @notice Tests that the validate function successfully returns the right error when the /// PreimageOracle version is invalid. function test_validate_preimageOracleInvalidVersion_succeeds() public { @@ -1017,8 +1146,17 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { assertEq("L1SB-70", _validate(true)); } - /// @notice Tests that the validate function (with the L1PAOMultisig and Challenger overriden) successfully returns - /// the right error when both are invalid. + /// @notice Tests that the validate function successfully returns the right error when the + /// L1StandardBridge proxyAdmin is invalid. + function test_validate_l1StandardBridgeInvalidProxyAdmin_succeeds() public { + vm.mockCall( + address(l1StandardBridge), abi.encodeCall(IProxyAdminOwnedBase.proxyAdmin, ()), abi.encode(address(0xbad)) + ); + assertEq("L1SB-80", _validate(true)); + } + + /// @notice Tests that the validate function (with the L1PAOMultisig and Challenger overriden) + /// successfully returns the right error when both are invalid. function test_validateL1PAOMultisigAndChallengerOverrides_succeeds() public view { IStandardValidator.ValidationOverrides memory overrides = _defaultValidationOverrides(); overrides.l1PAOMultisig = address(0xace); @@ -1030,7 +1168,8 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { } /// @notice Tests that the validate function (with the L1PAOMultisig and Challenger overriden) - /// successfully returns no error when there is none. That is, it never returns the overriden strings alone. + /// successfully returns no error when there is none. That is, it never returns the + /// overriden strings alone. function test_validateOverrides_noErrors_succeeds() public { IStandardValidator.ValidationOverrides memory overrides = IStandardValidator.ValidationOverrides({ l1PAOMultisig: address(0xbad), challenger: address(0xc0ffee) }); @@ -1047,8 +1186,8 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { assertEq("OVERRIDES-L1PAOMULTISIG,OVERRIDES-CHALLENGER", _validate(true, overrides)); } - /// @notice Tests that the validate function (with overrides)and allow failure set to false, returns the errors with - /// the overrides prepended. + /// @notice Tests that the validate function (with overrides) and allow failure set to false, + /// returns the errors with the overrides prepended. function test_validateOverrides_notAllowFailurePrependsOverrides_succeeds() public { IStandardValidator.ValidationOverrides memory overrides = IStandardValidator.ValidationOverrides({ l1PAOMultisig: address(0xbad), challenger: address(0xc0ffee) }); @@ -1061,7 +1200,8 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { _validate(false, overrides); } - /// @notice Tests that the version getter functions on StandardValidator return non-empty strings. + /// @notice Tests that the version getter functions on StandardValidator return non-empty + /// strings. function test_versions_succeeds() public view { assertTrue(bytes(validator.systemConfigVersion()).length > 0, "systemConfigVersion empty"); assertTrue(bytes(validator.optimismPortalVersion()).length > 0, "optimismPortalVersion empty"); @@ -1078,5 +1218,6 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit { assertTrue(bytes(validator.delayedWETHVersion()).length > 0, "delayedWETHVersion empty"); assertTrue(bytes(validator.permissionedDisputeGameVersion()).length > 0, "permissionedDisputeGameVersion empty"); assertTrue(bytes(validator.preimageOracleVersion()).length > 0, "preimageOracleVersion empty"); + assertTrue(bytes(validator.ethLockboxVersion()).length > 0, "ethLockboxVersion empty"); } } diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 0be2bbeb4acbb..5c4d20c27de62 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -908,6 +908,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "StandardValidator", _sel: _getSel("challenger()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("l1ERC721BridgeImpl()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("optimismPortalImpl()") }); + _addSpec({ _name: "StandardValidator", _sel: _getSel("ethLockboxImpl()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("systemConfigImpl()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("optimismMintableERC20FactoryImpl()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("l1CrossDomainMessengerImpl()") }); @@ -920,6 +921,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "StandardValidator", _sel: _getSel("permissionedDisputeGameVersion()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("mipsVersion()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("optimismPortalVersion()") }); + _addSpec({ _name: "StandardValidator", _sel: _getSel("ethLockboxVersion()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("anchorStateRegistryVersion()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("delayedWETHVersion()") }); _addSpec({ _name: "StandardValidator", _sel: _getSel("disputeGameFactoryVersion()") });