Commit e81c50de authored by Blaine Malone's avatar Blaine Malone Committed by GitHub

fix: OPCM additional safety checks (#12107)

parent 06f1406e
...@@ -139,9 +139,10 @@ contract DeployOPChainInput is BaseDeployIO { ...@@ -139,9 +139,10 @@ contract DeployOPChainInput is BaseDeployIO {
return abi.encode(defaultStartingAnchorRoots); return abi.encode(defaultStartingAnchorRoots);
} }
// TODO: Check that opcm is proxied and it has an implementation. function opcmProxy() public returns (OPContractsManager) {
function opcmProxy() public view returns (OPContractsManager) {
require(address(_opcmProxy) != address(0), "DeployOPChainInput: not set"); require(address(_opcmProxy) != address(0), "DeployOPChainInput: not set");
DeployUtils.assertValidContractAddress(address(_opcmProxy));
DeployUtils.assertImplementationSet(address(_opcmProxy));
return _opcmProxy; return _opcmProxy;
} }
} }
...@@ -303,7 +304,7 @@ contract DeployOPChainOutput is BaseDeployIO { ...@@ -303,7 +304,7 @@ contract DeployOPChainOutput is BaseDeployIO {
assertValidSystemConfig(_doi); assertValidSystemConfig(_doi);
} }
function assertValidPermissionedDisputeGame(DeployOPChainInput _doi) internal view { function assertValidPermissionedDisputeGame(DeployOPChainInput _doi) internal {
PermissionedDisputeGame game = permissionedDisputeGame(); PermissionedDisputeGame game = permissionedDisputeGame();
require(GameType.unwrap(game.gameType()) == GameType.unwrap(GameTypes.PERMISSIONED_CANNON), "DPG-10"); require(GameType.unwrap(game.gameType()) == GameType.unwrap(GameTypes.PERMISSIONED_CANNON), "DPG-10");
...@@ -344,7 +345,7 @@ contract DeployOPChainOutput is BaseDeployIO { ...@@ -344,7 +345,7 @@ contract DeployOPChainOutput is BaseDeployIO {
require(address(registry.disputeGameFactory()) == address(disputeGameFactoryProxy()), "ANCHORI-10"); require(address(registry.disputeGameFactory()) == address(disputeGameFactoryProxy()), "ANCHORI-10");
} }
function assertValidSystemConfig(DeployOPChainInput _doi) internal view { function assertValidSystemConfig(DeployOPChainInput _doi) internal {
SystemConfig systemConfig = systemConfigProxy(); SystemConfig systemConfig = systemConfigProxy();
DeployUtils.assertInitialized({ _contractAddress: address(systemConfig), _slot: 0, _offset: 0 }); DeployUtils.assertInitialized({ _contractAddress: address(systemConfig), _slot: 0, _offset: 0 });
...@@ -383,7 +384,7 @@ contract DeployOPChainOutput is BaseDeployIO { ...@@ -383,7 +384,7 @@ contract DeployOPChainOutput is BaseDeployIO {
require(gasPayingToken == Constants.ETHER, "SYSCON-220"); require(gasPayingToken == Constants.ETHER, "SYSCON-220");
} }
function assertValidL1CrossDomainMessenger(DeployOPChainInput _doi) internal view { function assertValidL1CrossDomainMessenger(DeployOPChainInput _doi) internal {
L1CrossDomainMessenger messenger = l1CrossDomainMessengerProxy(); L1CrossDomainMessenger messenger = l1CrossDomainMessengerProxy();
DeployUtils.assertInitialized({ _contractAddress: address(messenger), _slot: 0, _offset: 20 }); DeployUtils.assertInitialized({ _contractAddress: address(messenger), _slot: 0, _offset: 20 });
...@@ -399,7 +400,7 @@ contract DeployOPChainOutput is BaseDeployIO { ...@@ -399,7 +400,7 @@ contract DeployOPChainOutput is BaseDeployIO {
require(address(uint160(uint256(xdmSenderSlot))) == Constants.DEFAULT_L2_SENDER, "L1xDM-60"); require(address(uint160(uint256(xdmSenderSlot))) == Constants.DEFAULT_L2_SENDER, "L1xDM-60");
} }
function assertValidL1StandardBridge(DeployOPChainInput _doi) internal view { function assertValidL1StandardBridge(DeployOPChainInput _doi) internal {
L1StandardBridge bridge = l1StandardBridgeProxy(); L1StandardBridge bridge = l1StandardBridgeProxy();
L1CrossDomainMessenger messenger = l1CrossDomainMessengerProxy(); L1CrossDomainMessenger messenger = l1CrossDomainMessengerProxy();
...@@ -421,7 +422,7 @@ contract DeployOPChainOutput is BaseDeployIO { ...@@ -421,7 +422,7 @@ contract DeployOPChainOutput is BaseDeployIO {
require(factory.bridge() == address(l1StandardBridgeProxy()), "MERC20F-20"); require(factory.bridge() == address(l1StandardBridgeProxy()), "MERC20F-20");
} }
function assertValidL1ERC721Bridge(DeployOPChainInput _doi) internal view { function assertValidL1ERC721Bridge(DeployOPChainInput _doi) internal {
L1ERC721Bridge bridge = l1ERC721BridgeProxy(); L1ERC721Bridge bridge = l1ERC721BridgeProxy();
DeployUtils.assertInitialized({ _contractAddress: address(bridge), _slot: 0, _offset: 0 }); DeployUtils.assertInitialized({ _contractAddress: address(bridge), _slot: 0, _offset: 0 });
...@@ -434,7 +435,7 @@ contract DeployOPChainOutput is BaseDeployIO { ...@@ -434,7 +435,7 @@ contract DeployOPChainOutput is BaseDeployIO {
require(address(bridge.superchainConfig()) == address(_doi.opcmProxy().superchainConfig()), "L721B-50"); require(address(bridge.superchainConfig()) == address(_doi.opcmProxy().superchainConfig()), "L721B-50");
} }
function assertValidOptimismPortal(DeployOPChainInput _doi) internal view { function assertValidOptimismPortal(DeployOPChainInput _doi) internal {
OptimismPortal2 portal = optimismPortalProxy(); OptimismPortal2 portal = optimismPortalProxy();
ISuperchainConfig superchainConfig = ISuperchainConfig(address(_doi.opcmProxy().superchainConfig())); ISuperchainConfig superchainConfig = ISuperchainConfig(address(_doi.opcmProxy().superchainConfig()));
......
...@@ -30,6 +30,7 @@ import { L1CrossDomainMessenger } from "src/L1/L1CrossDomainMessenger.sol"; ...@@ -30,6 +30,7 @@ import { L1CrossDomainMessenger } from "src/L1/L1CrossDomainMessenger.sol";
import { L1ERC721Bridge } from "src/L1/L1ERC721Bridge.sol"; import { L1ERC721Bridge } from "src/L1/L1ERC721Bridge.sol";
import { L1StandardBridge } from "src/L1/L1StandardBridge.sol"; import { L1StandardBridge } from "src/L1/L1StandardBridge.sol";
import { OptimismMintableERC20Factory } from "src/universal/OptimismMintableERC20Factory.sol"; import { OptimismMintableERC20Factory } from "src/universal/OptimismMintableERC20Factory.sol";
import { Proxy } from "src/universal/Proxy.sol";
import { GameType, GameTypes, Hash, OutputRoot } from "src/dispute/lib/Types.sol"; import { GameType, GameTypes, Hash, OutputRoot } from "src/dispute/lib/Types.sol";
...@@ -52,6 +53,15 @@ contract DeployOPChainInput_Test is Test { ...@@ -52,6 +53,15 @@ contract DeployOPChainInput_Test is Test {
doi = new DeployOPChainInput(); doi = new DeployOPChainInput();
} }
function buildOpcmProxy() public returns (Proxy opcmProxy) {
opcmProxy = new Proxy(address(0));
OPContractsManager opcmImpl = OPContractsManager(address(makeAddr("opcmImpl")));
vm.prank(address(0));
opcmProxy.upgradeTo(address(opcmImpl));
vm.etch(address(opcmProxy), address(opcmProxy).code);
vm.etch(address(opcmImpl), hex"01");
}
function test_set_succeeds() public { function test_set_succeeds() public {
doi.set(doi.opChainProxyAdminOwner.selector, opChainProxyAdminOwner); doi.set(doi.opChainProxyAdminOwner.selector, opChainProxyAdminOwner);
doi.set(doi.systemConfigOwner.selector, systemConfigOwner); doi.set(doi.systemConfigOwner.selector, systemConfigOwner);
...@@ -62,7 +72,10 @@ contract DeployOPChainInput_Test is Test { ...@@ -62,7 +72,10 @@ contract DeployOPChainInput_Test is Test {
doi.set(doi.basefeeScalar.selector, basefeeScalar); doi.set(doi.basefeeScalar.selector, basefeeScalar);
doi.set(doi.blobBaseFeeScalar.selector, blobBaseFeeScalar); doi.set(doi.blobBaseFeeScalar.selector, blobBaseFeeScalar);
doi.set(doi.l2ChainId.selector, l2ChainId); doi.set(doi.l2ChainId.selector, l2ChainId);
doi.set(doi.opcmProxy.selector, address(opcm));
(Proxy opcmProxy) = buildOpcmProxy();
doi.set(doi.opcmProxy.selector, address(opcmProxy));
// Compare the default inputs to the getter methods. // Compare the default inputs to the getter methods.
assertEq(opChainProxyAdminOwner, doi.opChainProxyAdminOwner(), "200"); assertEq(opChainProxyAdminOwner, doi.opChainProxyAdminOwner(), "200");
assertEq(systemConfigOwner, doi.systemConfigOwner(), "300"); assertEq(systemConfigOwner, doi.systemConfigOwner(), "300");
...@@ -73,7 +86,7 @@ contract DeployOPChainInput_Test is Test { ...@@ -73,7 +86,7 @@ contract DeployOPChainInput_Test is Test {
assertEq(basefeeScalar, doi.basefeeScalar(), "800"); assertEq(basefeeScalar, doi.basefeeScalar(), "800");
assertEq(blobBaseFeeScalar, doi.blobBaseFeeScalar(), "900"); assertEq(blobBaseFeeScalar, doi.blobBaseFeeScalar(), "900");
assertEq(l2ChainId, doi.l2ChainId(), "1000"); assertEq(l2ChainId, doi.l2ChainId(), "1000");
assertEq(address(opcm), address(doi.opcmProxy()), "1100"); assertEq(address(opcmProxy), address(doi.opcmProxy()), "1100");
} }
function test_getters_whenNotSet_revert() public { function test_getters_whenNotSet_revert() public {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment