diff --git a/contracts/ForkingManager.sol b/contracts/ForkingManager.sol index a70ddfe1..0cba090a 100644 --- a/contracts/ForkingManager.sol +++ b/contracts/ForkingManager.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; import {ERC20PresetMinterPauser} from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import {AddressUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol"; import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; @@ -97,11 +98,69 @@ contract ForkingManager is IForkingManager, ForkableStructure { ); disputeData = _disputeData; + require( + verifyNewImplementations(_newImplementations), + "Invalid new implementations" + ); proposedImplementations = _newImplementations; // solhint-disable-next-line not-rely-on-time executionTimeForProposal = (block.timestamp + forkPreparationTime); } + /** + * @notice function to check that the new implementations are properly set and are actual contracts + * @param _newImplementations the addresses of the new implementations that will + * be used in the fork + * @return true if the new implementations are properly formed + */ + function verifyNewImplementations( + NewImplementations calldata _newImplementations + ) internal view returns (bool) { + // address zero is placeholder for no change and hence allowed. CreateChildren library will then read out the parent address and use it + require( + _newImplementations.bridgeImplementation == address(0) || + AddressUpgradeable.isContract( + _newImplementations.bridgeImplementation + ), + "bridge not contract" + ); + require( + _newImplementations.zkEVMImplementation == address(0) || + AddressUpgradeable.isContract( + _newImplementations.zkEVMImplementation + ), + "zkEVM not contract" + ); + require( + _newImplementations.forkonomicTokenImplementation == address(0) || + AddressUpgradeable.isContract( + _newImplementations.forkonomicTokenImplementation + ), + "forkonomic token not contract" + ); + require( + _newImplementations.forkingManagerImplementation == address(0) || + AddressUpgradeable.isContract( + _newImplementations.forkingManagerImplementation + ), + "forking manager not contract" + ); + + require( + _newImplementations.zkEVMImplementation == address(0) || + AddressUpgradeable.isContract( + _newImplementations.globalExitRootImplementation + ), + "global exit root not contract" + ); + require( + _newImplementations.verifier == address(0) || + AddressUpgradeable.isContract(_newImplementations.verifier), + "verifier not contract" + ); + return true; + } + /** * @dev function that executes a fork proposal */ diff --git a/test/ForkingManager.t.sol b/test/ForkingManager.t.sol index 6caf2a54..620da58c 100644 --- a/test/ForkingManager.t.sol +++ b/test/ForkingManager.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; /* solhint-disable not-rely-on-time */ import {Test} from "forge-std/Test.sol"; +import {VerifierRollupHelperMock} from "@RealityETH/zkevm-contracts/contracts/mocks/VerifierRollupHelperMock.sol"; import {ForkingManager} from "../contracts/ForkingManager.sol"; import {ForkableBridge} from "../contracts/ForkableBridge.sol"; import {ForkableZkEVM} from "../contracts/ForkableZkEVM.sol"; @@ -69,7 +70,7 @@ contract ForkingManagerTest is Test { address public newForkmanagerImplementation = address(new ForkingManager()); address public newZkevmImplementation = address(new ForkableZkEVM()); address public newVerifierImplementation = - address(0x1234567890123456789012345678901234567894); + address(new VerifierRollupHelperMock()); address public newGlobalExitRootImplementation = address(new ForkableGlobalExitRoot()); address public newForkonomicTokenImplementation = @@ -246,6 +247,48 @@ contract ForkingManagerTest is Test { assertFalse(forkmanager.canFork()); } + function testVerifyNewImplementationsRejectsNonContractBridgeImplementation() + public + { + forkonomicToken.approve(address(forkmanager), arbitrationFee); + vm.prank(address(this)); + forkonomicToken.mint(address(this), arbitrationFee); + IForkingManager.NewImplementations + memory implementations = IForkingManager.NewImplementations({ + bridgeImplementation: address(0x1), // non contract + zkEVMImplementation: newZkevmImplementation, + forkonomicTokenImplementation: newForkonomicTokenImplementation, + forkingManagerImplementation: newForkmanagerImplementation, + globalExitRootImplementation: newGlobalExitRootImplementation, + verifier: newVerifierImplementation, + forkID: newForkID + }); + + vm.expectRevert("bridge not contract"); + forkmanager.initiateFork(disputeData, implementations); + } + + function testVerifyNewImplementationsRejectsNonContractZkEVMImplementation() + public + { + forkonomicToken.approve(address(forkmanager), arbitrationFee); + vm.prank(address(this)); + forkonomicToken.mint(address(this), arbitrationFee); + IForkingManager.NewImplementations + memory implementations = IForkingManager.NewImplementations({ + bridgeImplementation: newBridgeImplementation, // non contract + zkEVMImplementation: address(0x234), + forkonomicTokenImplementation: newForkonomicTokenImplementation, + forkingManagerImplementation: newForkmanagerImplementation, + globalExitRootImplementation: newGlobalExitRootImplementation, + verifier: newVerifierImplementation, + forkID: newForkID + }); + + vm.expectRevert("zkEVM not contract"); + forkmanager.initiateFork(disputeData, implementations); + } + function testInitiateForkChargesFees() public { // Call the initiateFork function to create a new fork vm.expectRevert(bytes("ERC20: insufficient allowance")); diff --git a/test/L1GlobalChainInfoPublisher.t.sol b/test/L1GlobalChainInfoPublisher.t.sol index df2f6565..f0f2c477 100644 --- a/test/L1GlobalChainInfoPublisher.t.sol +++ b/test/L1GlobalChainInfoPublisher.t.sol @@ -12,6 +12,7 @@ import {Arbitrator} from "../contracts/lib/reality-eth/Arbitrator.sol"; // TODO: Replace this with whatever zkEVM or whatever platform we're on uses import {IAMB} from "../contracts/interfaces/IAMB.sol"; +import {VerifierRollupHelperMock} from "@RealityETH/zkevm-contracts/contracts/mocks/VerifierRollupHelperMock.sol"; import {IRealityETH} from "../contracts/interfaces/IRealityETH.sol"; import {IERC20} from "../contracts/interfaces/IERC20.sol"; import {ForkableRealityETH_ERC20} from "../contracts/ForkableRealityETH_ERC20.sol"; @@ -93,8 +94,9 @@ contract L1GlobalChainInfoPublisherTest is Test { address public newBridgeImplementation = address(new ForkableBridge()); address public newForkmanagerImplementation = address(new ForkingManager()); address public newZkevmImplementation = address(new ForkableZkEVM()); + address public newVerifierImplementation = - address(0x1234567890123456789012345678901234567894); + address(new VerifierRollupHelperMock()); address public newGlobalExitRootImplementation = address(new ForkableGlobalExitRoot()); address public newForkonomicTokenImplementation = diff --git a/test/L1GlobalForkRequester.t.sol b/test/L1GlobalForkRequester.t.sol index 0bbb01be..2b99e71c 100644 --- a/test/L1GlobalForkRequester.t.sol +++ b/test/L1GlobalForkRequester.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; /* solhint-disable not-rely-on-time */ import {Test} from "forge-std/Test.sol"; +import {VerifierRollupHelperMock} from "@RealityETH/zkevm-contracts/contracts/mocks/VerifierRollupHelperMock.sol"; import {ForkingManager} from "../contracts/ForkingManager.sol"; import {ForkableBridge} from "../contracts/ForkableBridge.sol"; import {ForkableZkEVM} from "../contracts/ForkableZkEVM.sol"; @@ -76,7 +77,7 @@ contract L1GlobalForkRequesterTest is Test { address public newForkmanagerImplementation = address(new ForkingManager()); address public newZkevmImplementation = address(new ForkableZkEVM()); address public newVerifierImplementation = - address(0x1234567890123456789012345678901234567894); + address(new VerifierRollupHelperMock()); address public newGlobalExitRootImplementation = address(new ForkableGlobalExitRoot()); address public newForkonomicTokenImplementation = @@ -322,7 +323,16 @@ contract L1GlobalForkRequesterTest is Test { vm.prank(address(this)); forkonomicToken.approve(address(forkmanager), fee); // Assume the data contains the questionId and pass it directly to the forkmanager in the fork request - IForkingManager.NewImplementations memory newImplementations; + IForkingManager.NewImplementations + memory newImplementations = IForkingManager.NewImplementations( + newBridgeImplementation, + newZkevmImplementation, + newForkonomicTokenImplementation, + newForkmanagerImplementation, + newGlobalExitRootImplementation, + newVerifierImplementation, + uint64(0x7) + ); IForkingManager.DisputeData memory disputeData = IForkingManager .DisputeData(false, address(this), requestId); forkmanager.initiateFork(disputeData, newImplementations);