Feat/v3.5 (#64)

* Add BridgeProxy contract and state variables * add deposit endpoint * add logic for deposit funds in Bridge contract * moved structs to SharedStructs * add counter for transactions * add execute endpoint * fixed BridgeMock bug * add executeCallback private method * add finishExecuteGracefully private method * add refund method and view functions * removed token payments mapping and refactored * removed token payment * fixed logic after removing token payments * add setter for Bridge address * refactored tests for Bridge, MintBurnERC20 and ERC20Safe contracts * refactored deposit endpoint * removed state variable bridgeAddress * small gas optimization for getPendingTransaction view function * lowestIndexId and calldata encoding fixes * add unit tests for BridgeProxy contract * refactored execute endpoint * add logic for deleting pending txns * add test for txns with endpoints with no args * made BridgeProxy contract upgradable * test fixes after merge * add test for BridgeProxy contract upgrade * add isPendingTransaction private function * temporar fixes * fixes after review * renamed BridgeProxy to BridgeExecutor * more fixes * refactor execute function * renamed variables * - fixes after merge --------- Co-authored-by: cosmatudor <[email protected]> Co-authored-by: Rebegea Dragos-Alexandru <[email protected]> Co-authored-by: Cosma Tudor-Mihai <[email protected]>
multiversx · Sep 18, 2024 · 06bc10c · 06bc10c
1 parent ac815b3
commit 06bc10c
Show file tree

Hide file tree

Showing 13 changed files with 922 additions and 330 deletions.
diff --git a/contracts/Bridge.sol b/contracts/Bridge.sol
@@ -8,6 +8,7 @@ import "./SharedStructs.sol";
 import "./ERC20Safe.sol";
 import "./access/RelayerRole.sol";
 import "./lib/Pausable.sol";
+import "./BridgeExecutor.sol";
 
 /**
 @title Bridge
@@ -40,6 +41,7 @@ contract Bridge is Initializable, RelayerRole, Pausable {
     uint256[10] private __gap;
 
     ERC20Safe internal safe;
+    BridgeExecutor internal bridgeExecutor;
 
     mapping(uint256 => bool) public executedBatches;
     mapping(uint256 => CrossTransferStatus) public crossTransferStatuses;
@@ -53,19 +55,30 @@ contract Bridge is Initializable, RelayerRole, Pausable {
      *   - add/remove relayers
      *   - add/remove tokens that can be bridged
      */
-    function initialize(address[] memory board, uint256 initialQuorum, ERC20Safe erc20Safe) public virtual initializer {
+    function initialize(
+        address[] memory board,
+        uint256 initialQuorum,
+        ERC20Safe erc20Safe,
+        BridgeExecutor _bridgeExecutor
+    ) public virtual initializer {
         __RelayerRole_init();
-        __Bridge__init_unchained(board, initialQuorum, erc20Safe);
+        __Bridge__init_unchained(board, initialQuorum, erc20Safe, _bridgeExecutor);
     }
 
-    function __Bridge__init_unchained(address[] memory board, uint256 initialQuorum, ERC20Safe erc20Safe) internal onlyInitializing {
+    function __Bridge__init_unchained(
+        address[] memory board,
+        uint256 initialQuorum,
+        ERC20Safe erc20Safe,
+        BridgeExecutor _bridgeExecutor
+    ) internal onlyInitializing {
         require(initialQuorum >= minimumQuorum, "Quorum is too low.");
         require(board.length >= initialQuorum, "The board should be at least the quorum size.");
 
         _addRelayers(board);
 
         quorum = initialQuorum;
         safe = erc20Safe;
+        bridgeExecutor = _bridgeExecutor;
 
         batchSettleBlockCount = 40;
     }
@@ -109,20 +122,19 @@ contract Bridge is Initializable, RelayerRole, Pausable {
     }
 
     /**
-        @notice Executes transfers that were signed by the relayers.
-        @dev This is for the MultiversX to Ethereum flow
-        @dev Arrays here try to mimmick the structure of a batch. A batch represents the values from the same index in all the arrays.
-        @param tokens Array containing all the token addresses that the batch interacts with. Can even contain duplicates.
-        @param recipients Array containing all the destinations from the batch. Can be duplicates.
-        @param amounts Array containing all the amounts that will be transfered.
-        @param batchNonceMvx Nonce for the batch. This identifies a batch created on the MultiversX chain that bridges tokens from MultiversX to Ethereum
-        @param signatures Signatures from all the relayers for the execution. This mimics a delegated multisig contract. For the execution to take place, there must be enough valid signatures to achieve quorum.
+        @notice Executes a batch of transfers
+        @param mvxTransactions List of transactions from MultiversX side. Each transaction consists of:
+        - token address
+        - sender
+        - recipient
+        - amount
+        - deposit nonce
+        - call data
+        @param batchNonceMvx Nonce for the batch
+        @param signatures List of signatures from the relayers
     */
     function executeTransfer(
-        address[] calldata tokens,
-        address[] calldata recipients,
-        uint256[] calldata amounts,
-        uint256[] calldata depositNonces,
+        MvxTransaction[] calldata mvxTransactions,
         uint256 batchNonceMvx,
         bytes[] calldata signatures
     ) public whenNotPaused onlyRelayer {
@@ -132,16 +144,12 @@ contract Bridge is Initializable, RelayerRole, Pausable {
 
         _validateQuorum(
             signatures,
-            _getHashedDepositData(
-                abi.encode(recipients, tokens, amounts, depositNonces, batchNonceMvx, executeTransferAction)
-            )
+            _getHashedDepositData(abi.encode(mvxTransactions, batchNonceMvx, executeTransferAction))
         );
 
-        DepositStatus[] memory statuses = new DepositStatus[](tokens.length);
-        for (uint256 j = 0; j < tokens.length; j++) {
-            statuses[j] = safe.transfer(tokens[j], amounts[j], recipients[j])
-                ? DepositStatus.Executed
-                : DepositStatus.Rejected;
+        DepositStatus[] memory statuses = new DepositStatus[](mvxTransactions.length);
+        for (uint256 j = 0; j < mvxTransactions.length; j++) {
+            statuses[j] = _processDeposit(mvxTransactions[j]);
         }
 
         CrossTransferStatus storage crossStatus = crossTransferStatuses[batchNonceMvx];
@@ -154,7 +162,9 @@ contract Bridge is Initializable, RelayerRole, Pausable {
         @param batchNonceMvx Nonce for the batch
         @return a list of statuses for each transfer in the batch provided and a boolean that indicates if the information is final
      */
-    function getStatusesAfterExecution(uint256 batchNonceMvx) external view returns (DepositStatus[] memory, bool isFinal) {
+    function getStatusesAfterExecution(
+        uint256 batchNonceMvx
+    ) external view returns (DepositStatus[] memory, bool isFinal) {
         CrossTransferStatus memory crossStatus = crossTransferStatuses[batchNonceMvx];
         return (crossStatus.statuses, _isMvxBatchFinal(crossStatus.createdBlockNumber));
     }
@@ -176,6 +186,40 @@ contract Bridge is Initializable, RelayerRole, Pausable {
         return keccak256(abi.encodePacked(prefix, keccak256(encodedData)));
     }
 
+    function _processDeposit(MvxTransaction calldata mvxTransaction) private returns (DepositStatus) {
+        address recipient;
+        bool isScCall = _isScCall(mvxTransaction.callData);
+
+        if (isScCall) {
+            recipient = address(bridgeExecutor);
+        } else {
+            recipient = mvxTransaction.recipient;
+        }
+
+        if (mvxTransaction.amount == 0) {
+            return DepositStatus.Rejected;
+        }
+
+        bool transferSuccess = safe.transfer(mvxTransaction.token, mvxTransaction.amount, recipient);
+        if (!transferSuccess) {
+            return DepositStatus.Rejected;
+        }
+
+        // If the recipient is a smart contract, attempt to deposit the funds in bridgeExecutor
+        if (isScCall) {
+            bool depositSuccess = bridgeExecutor.deposit(mvxTransaction);
+            if (!depositSuccess) {
+                return DepositStatus.Rejected;
+            }
+        }
+
+        return DepositStatus.Executed;
+    }
+
+    function _isScCall(bytes calldata _data) private pure returns (bool) {
+        return _data.length > 0;
+    }
+
     function _validateQuorum(bytes[] memory signatures, bytes32 data) private view {
         uint256 signersCount;
         address[] memory validSigners = new address[](signatures.length);

diff --git a/contracts/BridgeExecutor.sol b/contracts/BridgeExecutor.sol
@@ -0,0 +1,139 @@
+//SPDX-License-Identifier: UNLICENSED
+
+pragma solidity ^0.8.20;
+
+import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
+import "./lib/Pausable.sol";
+import "./SharedStructs.sol";
+import "./lib/BoolTokenTransfer.sol";
+import "./access/AdminRole.sol";
+import "./access/BridgeRole.sol";
+
+contract BridgeExecutor is Initializable, Pausable, BridgeRole {
+    using BoolTokenTransfer for IERC20;
+
+    /*========================= CONTRACT STATE =========================*/
+    uint128 public constant MIN_GAS_LIMIT_FOR_SC_CALL = 10_000_000;
+    uint128 public constant DEFAULT_GAS_LIMIT_FOR_REFUND_CALLBACK = 20_000_000;
+    uint256 private lowestTxId;
+    uint256 private currentTxId;
+
+    mapping(uint256 => MvxTransaction) private pendingTransactions;
+
+    /*========================= PUBLIC API =========================*/
+    function initialize() public initializer {
+        __BridgeRole_init();
+        __Pausable_init();
+    }
+
+    function __BridgeExecutor__init_unchained() internal onlyInitializing {
+        lowestTxId = 0;
+        currentTxId = 0;
+    }
+
+    function deposit(MvxTransaction calldata txn) external payable whenNotPaused onlyBridge returns (bool) {
+        pendingTransactions[currentTxId] = txn;
+        currentTxId++;
+
+        IERC20 token = IERC20(txn.token);
+        bool approvalSuccess = token.approve(txn.recipient, txn.amount);
+
+        return approvalSuccess;
+    }
+
+    function execute(uint256 txId) external whenNotPaused {
+        require(txId < currentTxId, "BridgeExecutor: Invalid transaction ID");
+        MvxTransaction memory txn = pendingTransactions[txId];
+
+        require(txn.recipient != address(0), "BridgeExecutor: Transaction does not exist");
+
+        if (txn.callData.length == 0) {
+            _refundAndDeleteTxn(txId);
+            return;
+        }
+
+        (bytes memory selector, uint256 gasLimit, bytes memory args) = abi.decode(
+            txn.callData,
+            (bytes, uint256, bytes)
+        );
+
+        if (selector.length == 0 || gasLimit == 0 || gasLimit < MIN_GAS_LIMIT_FOR_SC_CALL) {
+            _refundAndDeleteTxn(txId);
+            return;
+        }
+
+        bytes memory data;
+        if (args.length > 0) {
+            data = abi.encodePacked(selector, args);
+        } else {
+            data = selector;
+        }
+
+        _updateLowestTxId();
+
+        delete pendingTransactions[txId];
+
+        (bool success, ) = txn.recipient.call{ gas: gasLimit }(data);
+
+        if (!success) {
+            _refundTransaction(txn.token, txn.amount);
+            return;
+        }
+    }
+
+    /*========================= PRIVATE API =========================*/
+    function _refundAndDeleteTxn(uint256 txId) private {
+        MvxTransaction memory txn = pendingTransactions[txId];
+        _refundTransaction(txn.token, txn.amount);
+
+        _updateLowestTxId();
+
+        delete pendingTransactions[txId];
+    }
+
+    function _refundTransaction(address token, uint256 amount) private {
+        IERC20 erc20 = IERC20(token);
+        bool transferExecuted = erc20.boolTransfer(this.bridge(), amount);
+        require(transferExecuted, "BridgeExecutor: Refund failed");
+    }
+
+    function _updateLowestTxId() private {
+        uint256 newLowestTxId = lowestTxId;
+
+        while (newLowestTxId < currentTxId && pendingTransactions[newLowestTxId].amount == 0) {
+            newLowestTxId++;
+        }
+
+        lowestTxId = newLowestTxId;
+    }
+
+    function _isPendingTransaction(uint256 txId) private view returns (bool) {
+        return pendingTransactions[txId].amount != 0;
+    }
+
+    /*========================= VIEW FUNCTIONS =========================*/
+    function getPendingTransactionById(uint256 txId) public view returns (MvxTransaction memory) {
+        return pendingTransactions[txId];
+    }
+
+    function getPendingTransactions() public view returns (MvxTransaction[] memory) {
+        uint256 pendingTransactionsCount = currentTxId - lowestTxId;
+        MvxTransaction[] memory txns = new MvxTransaction[](pendingTransactionsCount);
+        uint256 index = 0;
+
+        for (uint256 i = lowestTxId; i < currentTxId; i++) {
+            if (_isPendingTransaction(i)) {
+                txns[index] = pendingTransactions[i];
+                index++;
+            }
+        }
+
+        // Resize the array to the actual number of pending transactions
+        assembly {
+            mstore(txns, index)
+        }
+
+        return txns;
+    }
+}
diff --git a/contracts/SharedStructs.sol b/contracts/SharedStructs.sol
@@ -34,3 +34,12 @@ struct Batch {
 struct DepositSCExtension {
     string depositData;
 }
+
+struct MvxTransaction {
+    address token;
+    bytes32 sender;
+    address recipient;
+    uint256 amount;
+    uint256 depositNonce;
+    bytes callData;
+}
diff --git a/contracts/test/BridgeExecutorTestContract.sol b/contracts/test/BridgeExecutorTestContract.sol
@@ -0,0 +1,27 @@
+//SPDX-License-Identifier: UNLICENSED
+
+pragma solidity ^0.8.20;
+
+interface IERC20 {
+    function transferFrom(address sender, address recipient, uint256 amount) external returns (bool);
+}
+
+contract BridgeExecutorTestContract {
+    uint256 public count;
+    address public bridgeExecutor;
+
+    constructor(address _bridgeExecutor) {
+        count = 0;
+        bridgeExecutor = _bridgeExecutor;
+    }
+
+    function increment() public {
+        count += 1;
+    }
+
+    function withdraw(address tokenAddress, uint256 amount) external {
+        IERC20 token = IERC20(tokenAddress);
+        // The test contract should be approved to spend the tokens on behalf of the bridgeExecutor
+        require(token.transferFrom(bridgeExecutor, address(this), amount), "TransferFrom failed");
+    }
+}
diff --git a/contracts/test/BridgeExecutorUpgrade.sol b/contracts/test/BridgeExecutorUpgrade.sol
@@ -0,0 +1,19 @@
+//SPDX-License-Identifier: UNLICENSED
+
+pragma solidity ^0.8.20;
+
+import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
+
+import "../BridgeExecutor.sol";
+
+contract BridgeExecutorUpgrade is Initializable, BridgeExecutor {
+    uint256 public someValue;
+    // New initialization function for the upgrade
+    function initializeV2(uint256 val) public reinitializer(2) {
+        someValue = val;
+    }
+
+    function afterUpgrade() public view returns (uint256) {
+        return someValue;
+    }
+}
diff --git a/contracts/test/BridgeMock.sol b/contracts/test/BridgeMock.sol
@@ -5,8 +5,13 @@ pragma solidity ^0.8.20;
 import "../Bridge.sol";
 
 contract BridgeMock is Bridge {
-    function initialize(address[] memory board, uint256 initialQuorum, ERC20Safe erc20Safe) public override initializer {
-        Bridge.initialize(board, initialQuorum, erc20Safe);
+    function initialize(
+        address[] memory board,
+        uint256 initialQuorum,
+        ERC20Safe erc20Safe,
+        BridgeExecutor bridgeExecutor
+    ) public override initializer {
+        Bridge.initialize(board, initialQuorum, erc20Safe, bridgeExecutor);
     }
 
     function proxyTransfer(address tokenAddress, uint256 amount, address recipientAddress) external returns (bool) {

diff --git a/tasks/deploy/evil-erc.ts b/tasks/deploy/evil-erc.ts
@@ -16,5 +16,5 @@ task("deploy-evil-erc", "Deploys EvilERC20 contract to use to test the bridge").
 
   //whitelist tokens in safe
   console.log("Whitelisting token ", usdcContract.target);
-  await safe.whitelistToken(usdcContract.target, "25000000", "100000000000", false, true);
+  await safe.whitelistToken(usdcContract.target, "25000000", "100000000000", false, true, 0, 0, 0);
 });