feat: separate deposit execution logic

README.md

@@ -32,7 +32,7 @@ Requires `yarn` and `@nomicfoundation/hardhat`.
     `typechain` - Generate Typechain typings for compiled contracts <br>
     `verify` - Verifies contract on Etherscan <br>
   * custom commands: <br>
-    `yarn run test`: Runs truffle tests.
+    `yarn run test`: Runs tests.
 
 # Sygma Security Policy
 

package.json

@@ -5,6 +5,7 @@
   "author": "Sygma",
   "license": "BUSL-1.1",
   "scripts": {
+    "compile": "npx hardhat compile",
     "coverage": "npx hardhat coverage",
     "lint:typescript": "eslint 'test/**/*.ts' 'scripts/**/*.ts'",
     "lint:typescript:fix": "prettier --config .prettierrc.json --write 'scripts/**/*.ts' 'test/**/*.ts' 'hardhat.config.ts'",

scripts/utils.ts

@@ -3,57 +3,67 @@ import { resolve } from "path";
 import type { InterfaceAbi, Fragment } from "ethers";
 import { keccak256, ContractFactory } from "ethers";
 
-const BRIDGE_CONTRACT_PATH = resolve(__dirname, "../src/contracts/Bridge.sol");
-const ARTIFACTS_PATH = resolve(
-  __dirname,
-  "../artifacts/src/contracts/Bridge.sol/Bridge.json",
-);
-
-export function generateAccessControlFuncSignatures(): {
+export function generateAccessControlFuncSignatures(contracts: Array<string>): {
   function: string;
   hash: string;
 }[] {
-  const bridgeArtifacts = JSON.parse(
-    fs.readFileSync(ARTIFACTS_PATH).toString(),
-  );
-  const bridgeContractFactory = new ContractFactory(
-    bridgeArtifacts.abi as InterfaceAbi,
-    bridgeArtifacts.bytecode as string,
-  );
-  const bridgeContractMethods = bridgeContractFactory.interface.fragments
-    .map((fragment: Fragment) => {
-      if (fragment.type == "function") {
-        return fragment.format();
-      }
-    })
-    .filter((item) => item) as Array<string>;
-
-  const bridgeContract = fs.readFileSync(BRIDGE_CONTRACT_PATH);
-
-  // regex that will match all functions that have "onlyAllowed" modifier
-  const regex = RegExp(
-    "function\\s+(?:(?!_onlyAllowed|function).)+onlyAllowed",
-    "gs",
-  );
-
-  let a;
-  const b: Array<string> = [];
-  // fetch all functions that have "onlyAllowed" modifier from "Bridge.sol"
-  while ((a = regex.exec(bridgeContract.toString())) !== null) {
-    // filter out only function name from matching (onlyAllowed) functions
-    b.push(a[0].split(/[\s()]+/)[1]);
-  }
-
-  let accessControlFuncSignatures = [];
-  // filter out from Bridge ABI functions signatures with "onlyAllowed" modifier
-  accessControlFuncSignatures = bridgeContractMethods
-    .filter((el1) => b.some((el2) => el1.includes(el2)))
-    .map((func) => ({
-      function: func,
-      hash: keccak256(Buffer.from(func)).substring(0, 10),
-    }));
-
-  console.table(accessControlFuncSignatures);
-
-  return accessControlFuncSignatures;
+  const allAccessControlFuncSignatures: {
+    function: string;
+    hash: string;
+  }[] = [];
+
+  contracts.map((contractName) => {
+    const CONTRACT_PATH = resolve(
+      __dirname,
+      `../src/contracts/${contractName}.sol`,
+    );
+    const ARTIFACTS_PATH = resolve(
+      __dirname,
+      `../artifacts/src/contracts/${contractName}.sol/${contractName}.json`,
+    );
+
+    const bridgeArtifacts = JSON.parse(
+      fs.readFileSync(ARTIFACTS_PATH).toString(),
+    );
+    const contractFactory = new ContractFactory(
+      bridgeArtifacts.abi as InterfaceAbi,
+      bridgeArtifacts.bytecode as string,
+    );
+    const contractMethods = contractFactory.interface.fragments
+      .map((fragment: Fragment) => {
+        if (fragment.type == "function") {
+          return fragment.format();
+        }
+      })
+      .filter((item) => item) as Array<string>;
+
+    const contractInstance = fs.readFileSync(CONTRACT_PATH);
+
+    // regex that will match all functions that have "onlyAllowed" modifier
+    const regex = RegExp(
+      "function\\s+(?:(?!_onlyAllowed|function).)+onlyAllowed",
+      "gs",
+    );
+
+    let a;
+    const b: Array<string> = [];
+    // fetch all functions that have "onlyAllowed" modifier
+    while ((a = regex.exec(contractInstance.toString())) !== null) {
+      // filter out only function name from matching (onlyAllowed) functions
+      b.push(a[0].split(/[\s()]+/)[1]);
+    }
+
+    // filter out from Bridge ABI functions signatures with "onlyAllowed" modifier
+    const accessControlFuncSignatures = contractMethods
+      .filter((el1) => b.some((el2) => el1.includes(el2)))
+      .map((func) => ({
+        function: func,
+        hash: keccak256(Buffer.from(func)).substring(0, 10),
+      }));
+    allAccessControlFuncSignatures.push(...accessControlFuncSignatures);
+  });
+
+  console.table(allAccessControlFuncSignatures);
+
+  return allAccessControlFuncSignatures;
 }

src/contracts/Bridge.sol

@@ -25,54 +25,16 @@ contract Bridge is Pausable, Context {
 
     IAccessControlSegregator public _accessControl;
 
-    struct Proposal {
-        uint8 originDomainID;
-        uint64 depositNonce;
-        bytes32 resourceID;
-        bytes data;
-    }
-
-    // destinationDomainID => number of deposits
-    mapping(uint8 => uint64) public _depositCounts;
     // resourceID => handler address
     mapping(bytes32 => address) public _resourceIDToHandlerAddress;
-    // forwarder address => is Valid
-    mapping(address => bool) public isValidForwarder;
-    // origin domainID => nonces set => used deposit nonces
-    mapping(uint8 => mapping(uint256 => uint256)) public usedNonces;
+
 
     event FeeHandlerChanged(address newFeeHandler);
     event AccessControlChanged(address newAccessControl);
-    event Deposit(
-        uint8 destinationDomainID,
-        bytes32 resourceID,
-        uint64 depositNonce,
-        address indexed user,
-        bytes data,
-        bytes handlerResponse
-    );
-    event ProposalExecution(uint8 originDomainID, uint64 depositNonce, bytes32 dataHash, bytes handlerResponse);
-
-    event FailedHandlerExecution(bytes lowLevelData, uint8 originDomainID, uint64 depositNonce);
-
-    event StartKeygen();
-
-    event EndKeygen();
 
-    event KeyRefresh(string hash);
-
-    event Retry(string txHash);
 
     error AccessNotAllowed(address sender, bytes4 funcSig);
 
-    error ResourceIDNotMappedToHandler();
-
-    error DepositToCurrentDomain();
-
-    error EmptyProposalsArray();
-
-    error NonceDecrementsNotAllowed();
-
     modifier onlyAllowed() {
         _onlyAllowed(msg.sig, _msgSender());
         _;
@@ -82,16 +44,6 @@ contract Bridge is Pausable, Context {
         if (!_accessControl.hasAccess(sig, sender)) revert AccessNotAllowed(sender, sig);
     }
 
-    function _msgSender() internal view override returns (address) {
-        address signer = msg.sender;
-        if (msg.data.length >= 20 && isValidForwarder[signer]) {
-            assembly {
-                signer := shr(96, calldataload(sub(calldatasize(), 20)))
-            }
-        }
-        return signer;
-    }
-
     /**
         @notice Initializes Bridge, creates and grants {_msgSender()} the admin role,
         sets access control contract for bridge.
@@ -154,29 +106,6 @@ contract Bridge is Pausable, Context {
         handler.setBurnable(tokenAddress);
     }
 
-    /**
-        @notice Sets the nonce for the specific domainID.
-        @notice Only callable by address that has the right to call the specific function,
-        which is mapped in {functionAccess} in AccessControlSegregator contract.
-        @param domainID Domain ID for increasing nonce.
-        @param nonce The nonce value to be set.
-     */
-    function adminSetDepositNonce(uint8 domainID, uint64 nonce) external onlyAllowed {
-        require(nonce > _depositCounts[domainID], "Does not allow decrements of the nonce");
-        _depositCounts[domainID] = nonce;
-    }
-
-    /**
-        @notice Set a forwarder to be used.
-        @notice Only callable by address that has the right to call the specific function,
-        which is mapped in {functionAccess} in AccessControlSegregator contract.
-        @param forwarder Forwarder address to be added.
-        @param valid Decision for the specific forwarder.
-     */
-    function adminSetForwarder(address forwarder, bool valid) external onlyAllowed {
-        isValidForwarder[forwarder] = valid;
-    }
-
     /**
         @notice Changes access control contract address.
         @notice Only callable by address that has the right to call the specific function,
@@ -210,125 +139,4 @@ contract Bridge is Pausable, Context {
         IERCHandler handler = IERCHandler(handlerAddress);
         handler.withdraw(data);
     }
-
-    /**
-        @notice Initiates a transfer using a specified handler contract.
-        @notice Only callable when Bridge is not paused.
-        @param destinationDomainID ID of chain deposit will be bridged to.
-        @param resourceID ResourceID used to find address of handler to be used for deposit.
-        @param depositData Additional data to be passed to specified handler.
-        @param feeData Additional data to be passed to the fee handler.
-        @notice Emits {Deposit} event with all necessary parameters and a handler response.
-        @return depositNonce deposit nonce for the destination domain.
-        @return handlerResponse a handler response:
-        - ERC20Handler: responds with an empty data.
-        - PermissionlessGenericHandler: responds with an empty data.
-     */
-    function deposit(
-        uint8 destinationDomainID,
-        bytes32 resourceID,
-        bytes calldata depositData,
-        bytes calldata feeData
-    ) external payable whenNotPaused returns (uint64 depositNonce, bytes memory handlerResponse) {
-        if (destinationDomainID == _domainID) revert DepositToCurrentDomain();
-
-        address sender = _msgSender();
-        if (address(_feeHandler) == address(0)) {
-            require(msg.value == 0, "no FeeHandler, msg.value != 0");
-        } else {
-            // Reverts on failure
-            _feeHandler.collectFee{value: msg.value}(
-                sender,
-                _domainID,
-                destinationDomainID,
-                resourceID,
-                depositData,
-                feeData
-            );
-        }
-        address handler = _resourceIDToHandlerAddress[resourceID];
-        if (handler == address(0)) revert ResourceIDNotMappedToHandler();
-
-        depositNonce = ++_depositCounts[destinationDomainID];
-
-        IHandler depositHandler = IHandler(handler);
-        handlerResponse = depositHandler.deposit(resourceID, sender, depositData);
-
-        emit Deposit(destinationDomainID, resourceID, depositNonce, sender, depositData, handlerResponse);
-        return (depositNonce, handlerResponse);
-    }
-
-    /**
-        @notice Executes a deposit proposal using a specified handler contract
-        @notice Failed executeProposal from handler don't revert, emits {FailedHandlerExecution} event.
-        @param proposal Proposal which consists of:
-        - originDomainID ID of chain deposit originated from.
-        - resourceID ResourceID to be used when making deposits.
-        - depositNonce ID of deposit generated by origin Bridge contract.
-        - data Data originally provided when deposit was made.
-        @notice Emits {ProposalExecution} event.
-        @notice Behaviour: when execution fails, the handler will terminate the function with revert.
-     */
-    function executeProposal(Proposal memory proposal) public {
-        Proposal[] memory proposalArray = new Proposal[](1);
-        proposalArray[0] = proposal;
-
-        executeProposals(proposalArray);
-    }
-
-    /**
-        @notice Executes a batch of deposit proposals using a specified handler contract for each proposal
-        @notice If executeProposals fails it doesn't revert, emits {FailedHandlerExecution} event.
-        @param proposals Array of Proposal which consists of:
-        - originDomainID ID of chain deposit originated from.
-        - resourceID ResourceID to be used when making deposits.
-        - depositNonce ID of deposit generated by origin Bridge contract.
-        - data Data originally provided when deposit was made.
-        @notice Emits {ProposalExecution} event for each proposal in the batch.
-        @notice Behaviour: when execution fails, the handler will terminate the function with revert.
-     */
-    function executeProposals(Proposal[] memory proposals) public whenNotPaused {
-        if (proposals.length == 0) revert EmptyProposalsArray();
-
-        for (uint256 i = 0; i < proposals.length; i++) {
-            if (isProposalExecuted(proposals[i].originDomainID, proposals[i].depositNonce)) {
-                continue;
-            }
-
-            address handler = _resourceIDToHandlerAddress[proposals[i].resourceID];
-            bytes32 dataHash = keccak256(abi.encodePacked(handler, proposals[i].data));
-
-            IHandler depositHandler = IHandler(handler);
-
-            usedNonces[proposals[i].originDomainID][proposals[i].depositNonce / 256] |=
-                1 <<
-                (proposals[i].depositNonce % 256);
-
-            try depositHandler.executeProposal(proposals[i].resourceID, proposals[i].data) returns (
-                bytes memory handlerResponse
-            ) {
-                emit ProposalExecution(
-                    proposals[i].originDomainID,
-                    proposals[i].depositNonce,
-                    dataHash,
-                    handlerResponse
-                );
-            } catch (bytes memory lowLevelData) {
-                emit FailedHandlerExecution(lowLevelData, proposals[i].originDomainID, proposals[i].depositNonce);
-                usedNonces[proposals[i].originDomainID][proposals[i].depositNonce / 256] &= ~(1 <<
-                    (proposals[i].depositNonce % 256));
-                continue;
-            }
-        }
-    }
-
-    /**
-        @notice Returns a boolean value.
-        @param domainID ID of chain deposit originated from.
-        @param depositNonce ID of deposit generated by origin Bridge contract.
-        @return Boolean value depending if deposit nonce has already been used or not.
-     */
-    function isProposalExecuted(uint8 domainID, uint256 depositNonce) public view returns (bool) {
-        return usedNonces[domainID][depositNonce / 256] & (1 << (depositNonce % 256)) != 0;
-    }
 }