Commit 8bc13075 authored by clabby's avatar clabby

Add init checks

parent dce98805
...@@ -468,9 +468,9 @@ OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutp ...@@ -468,9 +468,9 @@ OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutp
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 181439) OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 181439)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalNotProven_reverts() (gas: 41777) OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalNotProven_reverts() (gas: 41777)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 173449) OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 173449)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 180332) OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 180336)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 154191) OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 154191)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 218384) OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 218388)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 219614) OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 219614)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_paused_reverts() (gas: 53666) OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_paused_reverts() (gas: 53666)
OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_succeeds() (gas: 208960) OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_succeeds() (gas: 208960)
......
...@@ -11,6 +11,7 @@ import { SafeProxyFactory } from "safe-contracts/proxies/SafeProxyFactory.sol"; ...@@ -11,6 +11,7 @@ import { SafeProxyFactory } from "safe-contracts/proxies/SafeProxyFactory.sol";
import { Enum as SafeOps } from "safe-contracts/common/Enum.sol"; import { Enum as SafeOps } from "safe-contracts/common/Enum.sol";
import { Deployer } from "scripts/Deployer.sol"; import { Deployer } from "scripts/Deployer.sol";
import "scripts/Deployer.sol";
import { DeployConfig } from "scripts/DeployConfig.s.sol"; import { DeployConfig } from "scripts/DeployConfig.s.sol";
import { Safe } from "safe-contracts/Safe.sol"; import { Safe } from "safe-contracts/Safe.sol";
...@@ -383,6 +384,8 @@ contract Deploy is Deployer { ...@@ -383,6 +384,8 @@ contract Deploy is Deployer {
contracts.L1CrossDomainMessenger = address(messenger); contracts.L1CrossDomainMessenger = address(messenger);
ChainAssertions.checkL1CrossDomainMessenger(contracts, vm); ChainAssertions.checkL1CrossDomainMessenger(contracts, vm);
require(loadInitializedSlot("L1CrossDomainMessenger", false) == 1, "L1CrossDomainMessenger is not initialized");
addr_ = address(messenger); addr_ = address(messenger);
} }
...@@ -410,6 +413,8 @@ contract Deploy is Deployer { ...@@ -410,6 +413,8 @@ contract Deploy is Deployer {
contracts.OptimismPortal = address(portal); contracts.OptimismPortal = address(portal);
ChainAssertions.checkOptimismPortal(contracts, cfg, true); ChainAssertions.checkOptimismPortal(contracts, cfg, true);
require(loadInitializedSlot("OptimismPortal", false) == 1, "OptimismPortal is not initialized");
addr_ = address(portal); addr_ = address(portal);
} }
...@@ -432,6 +437,8 @@ contract Deploy is Deployer { ...@@ -432,6 +437,8 @@ contract Deploy is Deployer {
contracts.L2OutputOracle = address(oracle); contracts.L2OutputOracle = address(oracle);
ChainAssertions.checkL2OutputOracle(contracts, cfg, 0, 0); ChainAssertions.checkL2OutputOracle(contracts, cfg, 0, 0);
require(loadInitializedSlot("L2OutputOracle", false) == 1, "L2OutputOracle is not initialized");
addr_ = address(oracle); addr_ = address(oracle);
} }
...@@ -479,6 +486,8 @@ contract Deploy is Deployer { ...@@ -479,6 +486,8 @@ contract Deploy is Deployer {
contracts.ProtocolVersions = address(versions); contracts.ProtocolVersions = address(versions);
ChainAssertions.checkProtocolVersions(contracts, cfg, false); ChainAssertions.checkProtocolVersions(contracts, cfg, false);
require(loadInitializedSlot("ProtocolVersions", false) == 1, "ProtocolVersions is not initialized");
addr_ = address(versions); addr_ = address(versions);
} }
...@@ -521,6 +530,8 @@ contract Deploy is Deployer { ...@@ -521,6 +530,8 @@ contract Deploy is Deployer {
contracts.SystemConfig = address(config); contracts.SystemConfig = address(config);
ChainAssertions.checkSystemConfig(contracts, cfg, false); ChainAssertions.checkSystemConfig(contracts, cfg, false);
require(loadInitializedSlot("SystemConfig", false) == 1, "SystemConfig is not initialized");
addr_ = address(config); addr_ = address(config);
} }
...@@ -648,6 +659,8 @@ contract Deploy is Deployer { ...@@ -648,6 +659,8 @@ contract Deploy is Deployer {
console.log("SystemConfig version: %s", version); console.log("SystemConfig version: %s", version);
ChainAssertions.checkSystemConfig(_proxies(), cfg, true); ChainAssertions.checkSystemConfig(_proxies(), cfg, true);
require(loadInitializedSlot("SystemConfig", true) == 1, "SystemConfigProxy is not initialized");
} }
/// @notice Initialize the L1StandardBridge /// @notice Initialize the L1StandardBridge
...@@ -753,6 +766,10 @@ contract Deploy is Deployer { ...@@ -753,6 +766,10 @@ contract Deploy is Deployer {
console.log("L1CrossDomainMessenger version: %s", version); console.log("L1CrossDomainMessenger version: %s", version);
ChainAssertions.checkL1CrossDomainMessenger(_proxies(), vm); ChainAssertions.checkL1CrossDomainMessenger(_proxies(), vm);
require(
loadInitializedSlot("L1CrossDomainMessenger", true) == 1, "L1CrossDomainMessengerProxy is not initialized"
);
} }
/// @notice Initialize the L2OutputOracle /// @notice Initialize the L2OutputOracle
...@@ -775,6 +792,8 @@ contract Deploy is Deployer { ...@@ -775,6 +792,8 @@ contract Deploy is Deployer {
ChainAssertions.checkL2OutputOracle( ChainAssertions.checkL2OutputOracle(
_proxies(), cfg, cfg.l2OutputOracleStartingTimestamp(), cfg.l2OutputOracleStartingBlockNumber() _proxies(), cfg, cfg.l2OutputOracleStartingTimestamp(), cfg.l2OutputOracleStartingBlockNumber()
); );
require(loadInitializedSlot("L2OutputOracle", true) == 1, "L2OutputOracleProxy is not initialized");
} }
/// @notice Initialize the OptimismPortal /// @notice Initialize the OptimismPortal
...@@ -793,6 +812,8 @@ contract Deploy is Deployer { ...@@ -793,6 +812,8 @@ contract Deploy is Deployer {
console.log("OptimismPortal version: %s", version); console.log("OptimismPortal version: %s", version);
ChainAssertions.checkOptimismPortal(_proxies(), cfg, false); ChainAssertions.checkOptimismPortal(_proxies(), cfg, false);
require(loadInitializedSlot("OptimismPortal", true) == 1, "OptimismPortalProxy is not initialized");
} }
function initializeProtocolVersions() public broadcast { function initializeProtocolVersions() public broadcast {
...@@ -821,6 +842,8 @@ contract Deploy is Deployer { ...@@ -821,6 +842,8 @@ contract Deploy is Deployer {
console.log("ProtocolVersions version: %s", version); console.log("ProtocolVersions version: %s", version);
ChainAssertions.checkProtocolVersions(_proxies(), cfg, true); ChainAssertions.checkProtocolVersions(_proxies(), cfg, true);
require(loadInitializedSlot("ProtocolVersions", true) == 1, "ProtocolVersionsProxy is not initialized");
} }
/// @notice Transfer ownership of the ProxyAdmin contract to the final system owner /// @notice Transfer ownership of the ProxyAdmin contract to the final system owner
......
...@@ -458,6 +458,16 @@ abstract contract Deployer is Script { ...@@ -458,6 +458,16 @@ abstract contract Deployer is Script {
slot_ = abi.decode(rawSlot, (StorageSlot)); slot_ = abi.decode(rawSlot, (StorageSlot));
} }
/// @dev Returns the value of the internal `_initialized` storage slot for a given contract.
function loadInitializedSlot(string memory _contractName, bool _proxy) internal returns (uint8 initialized_) {
StorageSlot memory slot = getInitializedSlot(_contractName);
if (_proxy) {
_contractName = string.concat(_contractName, "Proxy");
}
bytes32 slotVal = vm.load(mustGetAddress(_contractName), bytes32(vm.parseUint(slot.slot)));
initialized_ = uint8((uint256(slotVal) >> (slot.offset * 8)) & 0xFF);
}
/// @notice Adds a deployment to the temp deployments file /// @notice Adds a deployment to the temp deployments file
function _writeTemp(string memory _name, address _deployed) internal { function _writeTemp(string memory _name, address _deployed) internal {
vm.writeJson({ json: stdJson.serialize("", _name, _deployed), path: tempDeploymentsPath }); vm.writeJson({ json: stdJson.serialize("", _name, _deployed), path: tempDeploymentsPath });
......
...@@ -21,7 +21,7 @@ contract Initializer_Test is Bridge_Initializer { ...@@ -21,7 +21,7 @@ contract Initializer_Test is Bridge_Initializer {
struct InitializeableContract { struct InitializeableContract {
address target; address target;
bytes initCalldata; bytes initCalldata;
StorageSlot initializedSlot; uint8 initializedSlotVal;
} }
/// @notice Contains the addresses of the contracts to test as well as the calldata /// @notice Contains the addresses of the contracts to test as well as the calldata
...@@ -40,7 +40,7 @@ contract Initializer_Test is Bridge_Initializer { ...@@ -40,7 +40,7 @@ contract Initializer_Test is Bridge_Initializer {
InitializeableContract({ InitializeableContract({
target: address(l1CrossDomainMessenger), target: address(l1CrossDomainMessenger),
initCalldata: abi.encodeCall(l1CrossDomainMessenger.initialize, ()), initCalldata: abi.encodeCall(l1CrossDomainMessenger.initialize, ()),
initializedSlot: getInitializedSlot("L1CrossDomainMessenger") initializedSlotVal: loadInitializedSlot("L1CrossDomainMessenger", true)
}) })
); );
// L2OutputOracle // L2OutputOracle
...@@ -48,7 +48,7 @@ contract Initializer_Test is Bridge_Initializer { ...@@ -48,7 +48,7 @@ contract Initializer_Test is Bridge_Initializer {
InitializeableContract({ InitializeableContract({
target: address(l2OutputOracle), target: address(l2OutputOracle),
initCalldata: abi.encodeCall(l2OutputOracle.initialize, (0, 0)), initCalldata: abi.encodeCall(l2OutputOracle.initialize, (0, 0)),
initializedSlot: getInitializedSlot("L2OutputOracle") initializedSlotVal: loadInitializedSlot("L2OutputOracle", true)
}) })
); );
// OptimismPortal // OptimismPortal
...@@ -56,7 +56,7 @@ contract Initializer_Test is Bridge_Initializer { ...@@ -56,7 +56,7 @@ contract Initializer_Test is Bridge_Initializer {
InitializeableContract({ InitializeableContract({
target: address(optimismPortal), target: address(optimismPortal),
initCalldata: abi.encodeCall(optimismPortal.initialize, (false)), initCalldata: abi.encodeCall(optimismPortal.initialize, (false)),
initializedSlot: getInitializedSlot("OptimismPortal") initializedSlotVal: loadInitializedSlot("OptimismPortal", true)
}) })
); );
// SystemConfig // SystemConfig
...@@ -82,7 +82,7 @@ contract Initializer_Test is Bridge_Initializer { ...@@ -82,7 +82,7 @@ contract Initializer_Test is Bridge_Initializer {
}) })
) )
), ),
initializedSlot: getInitializedSlot("SystemConfig") initializedSlotVal: loadInitializedSlot("SystemConfig", true)
}) })
); );
// ProtocolVersions // ProtocolVersions
...@@ -92,7 +92,7 @@ contract Initializer_Test is Bridge_Initializer { ...@@ -92,7 +92,7 @@ contract Initializer_Test is Bridge_Initializer {
initCalldata: abi.encodeCall( initCalldata: abi.encodeCall(
protocolVersions.initialize, (address(0), ProtocolVersion.wrap(1), ProtocolVersion.wrap(2)) protocolVersions.initialize, (address(0), ProtocolVersion.wrap(1), ProtocolVersion.wrap(2))
), ),
initializedSlot: getInitializedSlot("ProtocolVersions") initializedSlotVal: loadInitializedSlot("ProtocolVersions", true)
}) })
); );
} }
...@@ -110,15 +110,8 @@ contract Initializer_Test is Bridge_Initializer { ...@@ -110,15 +110,8 @@ contract Initializer_Test is Bridge_Initializer {
for (uint256 i; i < contracts.length; i++) { for (uint256 i; i < contracts.length; i++) {
InitializeableContract memory _contract = contracts[i]; InitializeableContract memory _contract = contracts[i];
// Load the `_initialized` slot from the storage of the target contract. // Assert that the contract is already initialized.
uint256 initSlotOffset = _contract.initializedSlot.offset; assertEq(_contract.initializedSlotVal, 1);
bytes32 initSlotVal = vm.load(_contract.target, bytes32(vm.parseUint(_contract.initializedSlot.slot)));
// Pull out the 8-bit `_initialized` flag from the storage slot. The offset in forge artifacts is
// relative to the least-significant bit and signifies the *byte offset*, so we need to shift the
// value to the right by the offset * 8 and then mask out the low-order byte to retrieve the flag.
uint8 init = uint8((uint256(initSlotVal) >> (initSlotOffset * 8)) & 0xFF);
assertEq(init, 1);
// Then, attempt to re-initialize the contract. This should fail. // Then, attempt to re-initialize the contract. This should fail.
(bool success, bytes memory returnData) = _contract.target.call(_contract.initCalldata); (bool success, bytes memory returnData) = _contract.target.call(_contract.initCalldata);
......
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