feat: add access control to pushCertificate (#98)

Signed-off-by: Jawad Tariq <[email protected]>
topos-protocol · Aug 17, 2023 · 6988d8f · 6988d8f
1 parent da41ebb
commit 6988d8f
Show file tree

Hide file tree

Showing 8 changed files with 203 additions and 180 deletions.
diff --git a/contracts/interfaces/IToposCore.sol b/contracts/interfaces/IToposCore.sol
@@ -34,13 +34,13 @@ interface IToposCore {
 
     function emitCrossSubnetMessage(SubnetId targetSubnetId) external;
 
+    function initialize(address[] memory adminAddresses, uint256 newAdminThreshold) external;
+
     function pushCertificate(bytes calldata certBytes, uint256 position) external;
 
     function setNetworkSubnetId(SubnetId _networkSubnetId) external;
 
-    function setup(bytes calldata params) external;
-
-    function upgrade(address newImplementation, bytes32 newImplementationCodeHash, bytes calldata setupParams) external;
+    function upgrade(address newImplementation, bytes32 newImplementationCodeHash) external;
 
     function adminEpoch() external view returns (uint256);
 

diff --git a/contracts/topos-core/ToposCore.sol b/contracts/topos-core/ToposCore.sol
@@ -4,9 +4,11 @@ pragma solidity ^0.8.9;
 import "./AdminMultisigBase.sol";
 import "./Bytes32Sets.sol";
 
+import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
+
 import "./../interfaces/IToposCore.sol";
 
-contract ToposCore is IToposCore, AdminMultisigBase {
+contract ToposCore is IToposCore, AdminMultisigBase, Initializable {
     using Bytes32SetsLib for Bytes32SetsLib.Set;
 
     /// @dev Storage slot with the address of the current implementation. `keccak256('eip1967.proxy.implementation') - 1`
@@ -31,7 +33,10 @@ contract ToposCore is IToposCore, AdminMultisigBase {
     /// @notice Mapping of transactions root to the certificate ID
     mapping(bytes32 => CertificateId) public txRootToCertId;
 
-    constructor() {}
+    /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor() {
+        _disableInitializers();
+    }
 
     /// @notice Sets the subnet ID
     /// @param _networkSubnetId The subnet ID of the subnet this contract is to be deployed on
@@ -43,31 +48,16 @@ contract ToposCore is IToposCore, AdminMultisigBase {
     /// @dev Need to pass the setup parameters to set the admins again
     /// @param newImplementation The address of the new implementation
     /// @param newImplementationCodeHash The code hash of the new implementation
-    /// @param setupParams The setup parameters for the new implementation
-    function upgrade(
-        address newImplementation,
-        bytes32 newImplementationCodeHash,
-        bytes calldata setupParams
-    ) external override onlyAdmin {
+    function upgrade(address newImplementation, bytes32 newImplementationCodeHash) external override onlyAdmin {
         if (newImplementationCodeHash != newImplementation.codehash) revert InvalidCodeHash();
         _setImplementation(newImplementation);
-        // AUDIT: If `newImplementation.setup` performs `selfdestruct`, it will result in the loss of _this_ implementation (thereby losing the ToposCore)
-        //        if `upgrade` is entered within the context of _this_ implementation itself.
-        if (setupParams.length != 0) {
-            // solhint-disable-next-line avoid-low-level-calls
-            (bool success, ) = newImplementation.delegatecall(
-                abi.encodeWithSelector(IToposCore.setup.selector, setupParams)
-            );
-
-            if (!success) revert SetupFailed();
-        }
         emit Upgraded(newImplementation);
     }
 
     /// @notice Push the certificate on-chain
     /// @param certBytes The certificate in byte
     /// @param position The position of the certificate
-    function pushCertificate(bytes memory certBytes, uint256 position) external override {
+    function pushCertificate(bytes memory certBytes, uint256 position) external override onlyAdmin {
         (
             CertificateId prevId,
             SubnetId sourceSubnetId,
@@ -107,20 +97,6 @@ contract ToposCore is IToposCore, AdminMultisigBase {
         emit CertStored(certId, txRoot);
     }
 
-    /// @notice Sets the admin threshold and admin addresses
-    /// @dev This function can only be called by the proxy contract
-    /// @param params Admin threshold and admin addresses
-    function setup(bytes calldata params) external override {
-        (address[] memory adminAddresses, uint256 newAdminThreshold) = abi.decode(params, (address[], uint256));
-        // Prevent setup from being called on a non-proxy (the implementation).
-        if (implementation() == address(0)) revert NotProxy();
-
-        // NOTE: Admin epoch is incremented to easily invalidate current admin-related state.
-        uint256 newAdminEpoch = _adminEpoch() + uint256(1);
-        _setAdminEpoch(newAdminEpoch);
-        _setAdmins(newAdminEpoch, adminAddresses, newAdminThreshold);
-    }
-
     /// @notice Emits an event to signal a cross subnet message has been sent
     /// @param targetSubnetId The subnet ID of the target subnet
     function emitCrossSubnetMessage(SubnetId targetSubnetId) external {
@@ -147,6 +123,17 @@ contract ToposCore is IToposCore, AdminMultisigBase {
         }
     }
 
+    /// @notice Contract initializer
+    /// @dev Can only be called once
+    /// @param adminAddresses list of admins
+    /// @param newAdminThreshold number of admins required to approve a call
+    function initialize(address[] memory adminAddresses, uint256 newAdminThreshold) public initializer {
+        // NOTE: Admin epoch is incremented to easily invalidate current admin-related state.
+        uint256 newAdminEpoch = _adminEpoch() + uint256(1);
+        _setAdminEpoch(newAdminEpoch);
+        _setAdmins(newAdminEpoch, adminAddresses, newAdminThreshold);
+    }
+
     /// @notice Checks if a certificate exists on the ToposCore contract
     /// @param certId The Certificate ID
     function certificateExists(CertificateId certId) public view returns (bool) {

diff --git a/contracts/topos-core/ToposCoreProxy.sol b/contracts/topos-core/ToposCoreProxy.sol
@@ -10,16 +10,11 @@ contract ToposCoreProxy is EternalStorage {
         bytes32(0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc);
 
     error InvalidImplementation();
-    error SetupFailed();
     error NativeCurrencyNotAccepted();
 
-    constructor(address tccImplementation, bytes memory params) {
+    constructor(address tccImplementation) {
         if (tccImplementation.code.length == 0) revert InvalidImplementation();
         _setAddress(KEY_IMPLEMENTATION, tccImplementation);
-
-        // solhint-disable-next-line avoid-low-level-calls
-        (bool success, ) = tccImplementation.delegatecall(abi.encodeWithSelector(IToposCore.setup.selector, params));
-        if (!success) revert SetupFailed();
     }
 
     receive() external payable {

diff --git a/scripts/deploy-topos-msg-protocol-dynamic.ts b/scripts/deploy-topos-msg-protocol-dynamic.ts
@@ -74,23 +74,27 @@ const main = async function (...args: string[]) {
   console.log(`Topos Core contract deployed to ${ToposCore.address}`)
 
   // Deploy ToposCoreProxy
-  const toposCoreProxyParams = utils.defaultAbiCoder.encode(
-    ['address[]', 'uint256'],
-    [[wallet.address], 1] // TODO: Use a different admin address than ToposDeployer
-  )
   const ToposCoreProxyFactory = new ContractFactory(
     toposCoreProxyJSON.abi,
     toposCoreProxyJSON.bytecode,
     wallet
   )
-  const ToposCoreProxy = await ToposCoreProxyFactory.deploy(
-    ToposCore.address,
-    toposCoreProxyParams,
-    { gasLimit: 5_000_000 }
-  )
+  const ToposCoreProxy = await ToposCoreProxyFactory.deploy(ToposCore.address, {
+    gasLimit: 5_000_000,
+  })
   await ToposCoreProxy.deployed()
   console.log(`Topos Core Proxy contract deployed to ${ToposCoreProxy.address}`)
 
+  console.info(`Initializing Topos Core Contract`)
+  const sequencerWallet = new Wallet(sequencerPrivateKey, provider)
+  const toposCoreInterface = new Contract(
+    ToposCoreProxy.address,
+    toposCoreInterfaceJSON.abi,
+    sequencerWallet
+  )
+  const adminThreshold = 1
+  await initialize(toposCoreInterface, sequencerWallet, adminThreshold)
+
   // Deploy ERC20Messaging
   const ERC20MessagingFactory = new ContractFactory(
     erc20MessagingJSON.abi,
@@ -105,12 +109,7 @@ const main = async function (...args: string[]) {
   await ERC20Messaging.deployed()
   console.log(`ERC20 Messaging contract deployed to ${ERC20Messaging.address}`)
 
-  console.info(`\nSetting subnetId on ToposCore via proxy`)
-  const toposCoreInterface = new Contract(
-    ToposCoreProxy.address,
-    toposCoreInterfaceJSON.abi,
-    wallet
-  )
+  console.info(`Setting subnetId on ToposCore via proxy`)
   await toposCoreInterface
     .setNetworkSubnetId(subnetId, { gasLimit: 4_000_000 })
     .then(async (tx: ContractTransaction) => {
@@ -130,7 +129,7 @@ const main = async function (...args: string[]) {
       process.exit(1)
     })
 
-  console.info(`\nReading subnet id`)
+  console.info(`Reading subnet id`)
   const networkSubnetId = await toposCoreInterface.networkSubnetId()
 
   console.info(
@@ -142,5 +141,26 @@ const sanitizeHexString = function (hexString: string) {
   return hexString.startsWith('0x') ? hexString : `0x${hexString}`
 }
 
+async function initialize(
+  toposCoreInterface: Contract,
+  wallet: Wallet,
+  adminThreshold: number
+) {
+  await toposCoreInterface
+    .initialize([wallet.address], adminThreshold, { gasLimit: 4_000_000 })
+    .then(async (tx: ContractTransaction) => {
+      await tx.wait().catch((error) => {
+        console.error(`Error: Failed (wait) to initialize ToposCore via proxy!`)
+        console.error(error)
+        process.exit(1)
+      })
+    })
+    .catch((error: Error) => {
+      console.error(`Error: Failed to initialize ToposCore via proxy!`)
+      console.error(error)
+      process.exit(1)
+    })
+}
+
 const args = process.argv.slice(2)
 main(...args)
diff --git a/scripts/deploy-topos-msg-protocol.ts b/scripts/deploy-topos-msg-protocol.ts
@@ -73,18 +73,23 @@ const main = async function (...args: string[]) {
     4_000_000
   )
 
-  const toposCoreProxyParams = utils.defaultAbiCoder.encode(
-    ['address[]', 'uint256'],
-    [[wallet.address], 1] // TODO: Use a different admin address than ToposDeployer
-  )
   const toposCoreProxyAddress = await processContract(
     wallet,
     toposCoreProxyJSON,
     toposCoreProxySalt!,
-    [toposCoreAddress, toposCoreProxyParams],
+    [toposCoreAddress],
     4_000_000
   )
 
+  const sequencerWallet = new Wallet(sequencerPrivateKey, provider)
+  const toposCoreInterface = new Contract(
+    toposCoreProxyAddress,
+    toposCoreInterfaceJSON.abi,
+    sequencerWallet
+  )
+  const adminThreshold = 1
+  await initialize(toposCoreInterface, sequencerWallet, adminThreshold)
+
   const erc20MessagingAddresss = await processContract(
     wallet,
     erc20MessagingJSON,
@@ -93,7 +98,7 @@ const main = async function (...args: string[]) {
     4_000_000
   )
 
-  setSubnetId(toposCoreProxyAddress, wallet, subnetId)
+  setSubnetId(toposCoreInterface, subnetId)
 
   console.log(`
 export TOPOS_CORE_CONTRACT_ADDRESS=${toposCoreAddress}
@@ -164,16 +169,9 @@ const processContract = async function (
 }
 
 const setSubnetId = async function (
-  toposCoreProxyAddress: string,
-  wallet: Wallet,
+  toposCoreInterface: Contract,
   subnetId: string
 ) {
-  const toposCoreInterface = new Contract(
-    toposCoreProxyAddress,
-    toposCoreInterfaceJSON.abi,
-    wallet
-  )
-
   await toposCoreInterface
     .setNetworkSubnetId(subnetId, { gasLimit: 4_000_000 })
     .then(async (tx: ContractTransaction) => {
@@ -196,5 +194,26 @@ const setSubnetId = async function (
   await toposCoreInterface.networkSubnetId()
 }
 
+async function initialize(
+  toposCoreInterface: Contract,
+  wallet: Wallet,
+  adminThreshold: number
+) {
+  await toposCoreInterface
+    .initialize([wallet.address], adminThreshold, { gasLimit: 4_000_000 })
+    .then(async (tx: ContractTransaction) => {
+      await tx.wait().catch((error) => {
+        console.error(`Error: Failed (wait) to initialize ToposCore via proxy!`)
+        console.error(error)
+        process.exit(1)
+      })
+    })
+    .catch((error: Error) => {
+      console.error(`Error: Failed to initialize ToposCore via proxy!`)
+      console.error(error)
+      process.exit(1)
+    })
+}
+
 const args = process.argv.slice(2)
 main(...args)