diff --git a/solidity/optics-core/contracts/Common.sol b/solidity/optics-core/contracts/Common.sol index 9abb7b95a..52192b07c 100644 --- a/solidity/optics-core/contracts/Common.sol +++ b/solidity/optics-core/contracts/Common.sol @@ -3,6 +3,7 @@ pragma solidity >=0.6.11; import "../libs/Message.sol"; +import {Initializable} from "@openzeppelin/contracts/proxy/Initializable.sol"; import "@openzeppelin/contracts/cryptography/ECDSA.sol"; /** @@ -10,7 +11,7 @@ import "@openzeppelin/contracts/cryptography/ECDSA.sol"; * @author Celo Labs Inc. * @notice Shared utilities between Home and Replica. **/ -abstract contract Common { +abstract contract Common is Initializable { enum States {UNINITIALIZED, ACTIVE, FAILED} /// @notice Domain of owning contract @@ -56,9 +57,7 @@ abstract contract Common { localDomain = _localDomain; } - function initialize(address _updater) public virtual { - require(state == States.UNINITIALIZED, "already initialized"); - + function initialize(address _updater) public virtual initializer { updater = _updater; state = States.ACTIVE; diff --git a/solidity/optics-core/contracts/Home.sol b/solidity/optics-core/contracts/Home.sol index aa238c955..6d32e7fa3 100644 --- a/solidity/optics-core/contracts/Home.sol +++ b/solidity/optics-core/contracts/Home.sol @@ -6,6 +6,7 @@ import "./Merkle.sol"; import "./Queue.sol"; import "../interfaces/IUpdaterManager.sol"; +import {Initializable} from "@openzeppelin/contracts/proxy/Initializable.sol"; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import "@openzeppelin/contracts/utils/Address.sol"; @@ -15,7 +16,13 @@ import "@openzeppelin/contracts/utils/Address.sol"; * @notice Contract responsible for managing production of the message tree and * holding custody of the updater bond. */ -contract Home is Ownable, MerkleTreeManager, QueueManager, Common { +contract Home is + Ownable, + Initializable, + MerkleTreeManager, + QueueManager, + Common +{ using QueueLib for QueueLib.Queue; using MerkleLib for MerkleLib.Tree; @@ -63,9 +70,7 @@ contract Home is Ownable, MerkleTreeManager, QueueManager, Common { constructor(uint32 _localDomain) Common(_localDomain) {} // solhint-disable-line no-empty-blocks - function initialize(address _updaterManager) public override { - require(state == States.UNINITIALIZED, "already initialized"); - + function initialize(address _updaterManager) public override initializer { _setUpdaterManager(_updaterManager); address _updater = updaterManager.updater(); _setUpdater(_updater); diff --git a/solidity/optics-core/contracts/Replica.sol b/solidity/optics-core/contracts/Replica.sol index 0bb359697..af268ce29 100644 --- a/solidity/optics-core/contracts/Replica.sol +++ b/solidity/optics-core/contracts/Replica.sol @@ -6,6 +6,7 @@ import "./Merkle.sol"; import "./Queue.sol"; import {IMessageRecipient} from "../interfaces/IMessageRecipient.sol"; +import {Initializable} from "@openzeppelin/contracts/proxy/Initializable.sol"; import "@summa-tx/memview-sol/contracts/TypedMemView.sol"; /** @@ -14,7 +15,7 @@ import "@summa-tx/memview-sol/contracts/TypedMemView.sol"; * @notice Contract responsible for tracking root updates on home, * and dispatching messages on Replica to end recipients. */ -contract Replica is Common, QueueManager { +contract Replica is Initializable, Common, QueueManager { using QueueLib for QueueLib.Queue; using MerkleLib for MerkleLib.Tree; using TypedMemView for bytes; @@ -54,9 +55,7 @@ contract Replica is Common, QueueManager { bytes32 _current, uint256 _optimisticSeconds, uint256 _lastProcessed - ) public { - require(state == States.UNINITIALIZED, "already initialized"); - + ) public initializer { remoteDomain = _remoteDomain; queue.initialize(); diff --git a/solidity/optics-core/contracts/governance/GovernanceRouter.sol b/solidity/optics-core/contracts/governance/GovernanceRouter.sol index f99bf0a66..7f9807a5d 100644 --- a/solidity/optics-core/contracts/governance/GovernanceRouter.sol +++ b/solidity/optics-core/contracts/governance/GovernanceRouter.sol @@ -2,6 +2,7 @@ pragma solidity >=0.6.11; pragma experimental ABIEncoderV2; +import {Initializable} from "@openzeppelin/contracts/proxy/Initializable.sol"; import {TypedMemView} from "@summa-tx/memview-sol/contracts/TypedMemView.sol"; import {Home} from "../Home.sol"; @@ -9,7 +10,7 @@ import {XAppConnectionManager, TypeCasts} from "../XAppConnectionManager.sol"; import {IMessageRecipient} from "../../interfaces/IMessageRecipient.sol"; import {GovernanceMessage} from "./GovernanceMessage.sol"; -contract GovernanceRouter is IMessageRecipient { +contract GovernanceRouter is Initializable, IMessageRecipient { using TypedMemView for bytes; using TypedMemView for bytes29; using GovernanceMessage for bytes29; @@ -39,13 +40,8 @@ contract GovernanceRouter is IMessageRecipient { localDomain = _localDomain; } - function initialize(address _xAppConnectionManager) public { + function initialize(address _xAppConnectionManager) public initializer { // initialize governor - require( - governorDomain == 0 && governor == address(0), - "governor already initialized" - ); - address _governorAddr = msg.sender; bool _isLocalGovernor = true; _transferGovernor(localDomain, _governorAddr, _isLocalGovernor); diff --git a/solidity/optics-core/lib/index.js b/solidity/optics-core/lib/index.js index 154c6cd3a..8c3b1a5c5 100644 --- a/solidity/optics-core/lib/index.js +++ b/solidity/optics-core/lib/index.js @@ -8,6 +8,7 @@ const { deployImplementation, deployUpgradeBeaconController, deployProxyWithImplementation, + getInitializeData, } = require('../scripts/deployUpgradeSetup'); const { deployOptics } = require('../scripts/deployOptics'); const HomeAbi = require('../../../abis/Home.abi.json'); @@ -181,5 +182,6 @@ extendEnvironment((hre) => { deployUpgradeSetup, deployOptics, deployProxyWithImplementation, + getInitializeData, }; }); diff --git a/solidity/optics-core/scripts/deployUpgradeSetup.js b/solidity/optics-core/scripts/deployUpgradeSetup.js index 183fb5e70..a4e87a35a 100644 --- a/solidity/optics-core/scripts/deployUpgradeSetup.js +++ b/solidity/optics-core/scripts/deployUpgradeSetup.js @@ -30,30 +30,45 @@ async function deployProxy(upgradeBeaconAddress, initializeData = '0x') { return proxy.deployed(); } +async function getInitializeData( + implementationName, + initializeArgs, + initializeIdentifier = 'initialize', +) { + if (initializeArgs.length === 0) { + return '0x'; + } + + const Implementation = await ethers.getContractFactory(implementationName); + + const initializeFunction = Implementation.interface.getFunction( + initializeIdentifier, + ); + + const initializeData = Implementation.interface.encodeFunctionData( + initializeFunction, + initializeArgs, + ); + + return initializeData; +} + async function deployProxyWithImplementation( upgradeBeaconAddress, implementationName, initializeArgs = [], initializeIdentifier = 'initialize', ) { - const Implementation = await ethers.getContractFactory(implementationName); - - let initializeData; - if (initializeArgs.length === 0) { - initializeData = '0x'; - } else { - const initializeFunction = Implementation.interface.getFunction( - initializeIdentifier, - ); - initializeData = Implementation.interface.encodeFunctionData( - initializeFunction, - initializeArgs, - ); - } + const initializeData = await getInitializeData( + implementationName, + initializeArgs, + initializeIdentifier, + ); const proxy = await deployProxy(upgradeBeaconAddress, initializeData); // instantiate proxy with Proxy Contract address + Implementation interface + const Implementation = await ethers.getContractFactory(implementationName); const [signer] = await ethers.getSigners(); const proxyWithImplementation = new ethers.Contract( proxy.address, @@ -152,4 +167,5 @@ module.exports = { deployUpgradeSetupAndProxy, deployProxy, deployProxyWithImplementation, + getInitializeData, }; diff --git a/solidity/optics-core/test/Common.test.js b/solidity/optics-core/test/Common.test.js index 16086b079..0ab7c0157 100644 --- a/solidity/optics-core/test/Common.test.js +++ b/solidity/optics-core/test/Common.test.js @@ -26,6 +26,12 @@ describe('Common', async () => { common = contracts.proxyWithImplementation; }); + it('Cannot be initialized twice', async () => { + await expect(common.initialize(signer.address)).to.be.revertedWith( + 'Initializable: contract is already initialized', + ); + }); + it('Accepts updater signature', async () => { const oldRoot = ethers.utils.formatBytes32String('old root'); const newRoot = ethers.utils.formatBytes32String('new root'); diff --git a/solidity/optics-core/test/Home.test.js b/solidity/optics-core/test/Home.test.js index bde77a14a..c3847d5b8 100644 --- a/solidity/optics-core/test/Home.test.js +++ b/solidity/optics-core/test/Home.test.js @@ -52,6 +52,12 @@ describe('Home', async () => { home = contracts.proxyWithImplementation; }); + it('Cannot be initialized twice', async () => { + await expect(home.initialize(signer.address)).to.be.revertedWith( + 'Initializable: contract is already initialized', + ); + }); + it('Halts on fail', async () => { await home.setFailed(); expect(await home.state()).to.equal(optics.State.FAILED); diff --git a/solidity/optics-core/test/Replica.test.js b/solidity/optics-core/test/Replica.test.js index 7b7258a5d..0b81f58a8 100644 --- a/solidity/optics-core/test/Replica.test.js +++ b/solidity/optics-core/test/Replica.test.js @@ -20,9 +20,12 @@ const localDomain = 2000; const optimisticSeconds = 3; const initialCurrentRoot = ethers.utils.formatBytes32String('current'); const initialLastProcessed = 0; +const replicaContractName = 'TestReplica'; +const replicaInitializeIdentifier = + 'initialize(uint32, address, bytes32, uint256, uint256)'; describe('Replica', async () => { - let replica, signer, fakeSigner, updater, fakeUpdater; + let replica, signer, fakeSigner, updater, fakeUpdater, initializeArgs; const enqueueValidUpdate = async (newRoot) => { let oldRoot; @@ -45,23 +48,40 @@ describe('Replica', async () => { beforeEach(async () => { const controller = null; + initializeArgs = [ + remoteDomain, + updater.signer.address, + initialCurrentRoot, + optimisticSeconds, + initialLastProcessed, + ]; + const { contracts } = await optics.deployUpgradeSetupAndProxy( - 'TestReplica', + replicaContractName, [localDomain], - [ - remoteDomain, - updater.signer.address, - initialCurrentRoot, - optimisticSeconds, - initialLastProcessed, - ], + initializeArgs, controller, - 'initialize(uint32, address, bytes32, uint256, uint256)', + replicaInitializeIdentifier, ); replica = contracts.proxyWithImplementation; }); + it('Cannot be initialized twice', async () => { + const initializeData = await optics.getInitializeData( + replicaContractName, + initializeArgs, + replicaInitializeIdentifier, + ); + + await expect( + signer.sendTransaction({ + to: replica.address, + data: initializeData, + }), + ).to.be.revertedWith('Initializable: contract is already initialized'); + }); + it('Halts on fail', async () => { await replica.setFailed(); expect(await replica.state()).to.equal(optics.State.FAILED);