feat: allow liquidity withdrawal from EOA or multisig (#275)

Author: nmlinaric

Diff for: contracts/TestContracts.sol

@@ -3,7 +3,7 @@
 pragma solidity 0.8.11;
 
 import "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol";
-import "./handlers/ERCHandlerHelpers.sol";
+import { ERCHandlerHelpers } from "./handlers/ERCHandlerHelpers.sol";
 import "./interfaces/IERC20Plus.sol";
 
 contract NoArgument {

Diff for: contracts/handlers/ERC1155Handler.sol

@@ -4,7 +4,7 @@ pragma solidity 0.8.11;
 
 import "../interfaces/IHandler.sol";
 import "./ERCHandlerHelpers.sol";
-import "../ERC1155Safe.sol";
+import { ERC1155Safe } from "../ERC1155Safe.sol";
 import "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
 import "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol";
 import "@openzeppelin/contracts/token/ERC1155/extensions/IERC1155MetadataURI.sol";
@@ -94,7 +94,7 @@ contract ERC1155Handler is IHandler, ERCHandlerHelpers, ERC1155Safe, ERC1155Hold
         @param data Consists of ABI-encoded {tokenAddress}, {recipient}, {tokenIDs},
         {amounts}, and {transferData} of types address, address, uint[], uint[], bytes.
      */
-    function withdraw(bytes memory data) external override onlyBridge {
+    function withdraw(bytes memory data) external override onlyAuthorized {
         address tokenAddress;
         address recipient;
         uint[] memory tokenIDs;

Diff for: contracts/handlers/ERC20Handler.sol

@@ -4,8 +4,8 @@ pragma solidity 0.8.11;
 
 import "../interfaces/IHandler.sol";
 import "./ERCHandlerHelpers.sol";
+import { ERC20Safe } from "../ERC20Safe.sol";
 import "./DepositDataHelper.sol";
-import "../ERC20Safe.sol";
 import "../utils/ExcessivelySafeCall.sol";
 
 /**
@@ -112,7 +112,7 @@ contract ERC20Handler is IHandler, ERCHandlerHelpers, DepositDataHelper, ERC20Sa
         recipient                              address     bytes  32 - 64
         amount                                 uint        bytes  64 - 96
      */
-    function withdraw(bytes memory data) external override onlyBridge {
+    function withdraw(bytes memory data) external override onlyAuthorized {
         address tokenAddress;
         address recipient;
         uint amount;

Diff for: contracts/handlers/ERC721Handler.sol

@@ -4,7 +4,7 @@ pragma solidity 0.8.11;
 
 import "../interfaces/IHandler.sol";
 import "./ERCHandlerHelpers.sol";
-import "../ERC721Safe.sol";
+import { ERC721Safe } from "../ERC721Safe.sol";
 import "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
 import "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
 
@@ -123,7 +123,7 @@ contract ERC721Handler is IHandler, ERCHandlerHelpers, ERC721Safe {
         recipient                              address     bytes  32 - 64
         tokenID                                uint        bytes  64 - 96
      */
-    function withdraw(bytes memory data) external override onlyBridge {
+    function withdraw(bytes memory data) external override onlyAuthorized {
         address tokenAddress;
         address recipient;
         uint tokenID;

Diff for: contracts/handlers/ERCHandlerHelpers.sol

@@ -3,14 +3,17 @@
 pragma solidity 0.8.11;
 
 import "../interfaces/IERCHandler.sol";
+import { AccessControl } from "../utils/AccessControl.sol";
 import "../utils/SanityChecks.sol";
 
 /**
     @title Function used across handler contracts.
     @author ChainSafe Systems.
     @notice This contract is intended to be used with the Bridge contract.
  */
-contract ERCHandlerHelpers is IERCHandler {
+contract ERCHandlerHelpers is IERCHandler, AccessControl {
+    bytes32 public constant LIQUIDITY_MANAGER_ROLE = keccak256("LIQUIDITY_MANAGER_ROLE");
+
     address public immutable _bridgeAddress;
 
     uint8 public constant defaultDecimals = 18;
@@ -28,6 +31,7 @@ contract ERCHandlerHelpers is IERCHandler {
     }
 
     error ContractAddressNotWhitelisted(address contractAddress);
+    error NotAuthorized();
 
     // resourceID => token contract address
     mapping (bytes32 => address) public _resourceIDToTokenContractAddress;
@@ -42,19 +46,30 @@ contract ERCHandlerHelpers is IERCHandler {
         _;
     }
 
+    modifier onlyAuthorized() {
+        _onlyAuthorized();
+        _;
+    }
+
     /**
         @param bridgeAddress Contract address of previously deployed Bridge.
      */
     constructor(
         address          bridgeAddress
     ) {
         _bridgeAddress = bridgeAddress;
+        _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
+        _setupRole(LIQUIDITY_MANAGER_ROLE, bridgeAddress);
     }
 
     function _onlyBridge() private view {
         require(msg.sender == _bridgeAddress, "sender must be bridge contract");
     }
 
+    function _onlyAuthorized() private view {
+        if(!hasRole(LIQUIDITY_MANAGER_ROLE, _msgSender())) revert NotAuthorized();
+    }
+
     /**
         @notice First verifies {contractAddress} is whitelisted, then sets
         {_tokenContractAddressToTokenProperties[contractAddress].isBurnable} to true.

Diff for: contracts/handlers/XC20Handler.sol

@@ -4,7 +4,7 @@ pragma solidity 0.8.11;
 
 import "../interfaces/IHandler.sol";
 import "./ERCHandlerHelpers.sol";
-import "../XC20Safe.sol";
+import { XC20Safe } from "../XC20Safe.sol";
 
 /**
     @title Handles XC20 deposits and deposit executions.

Diff for: test/contractBridge/admin.js

@@ -20,12 +20,13 @@ contract("Bridge - [admin]", async (accounts) => {
   const nonAdminAddress = accounts[1];
 
   const expectedBridgeAdmin = accounts[0];
+  const authorizedAddress = accounts[2];
   const someAddress = "0xcafecafecafecafecafecafecafecafecafecafe";
   const nullAddress = "0x0000000000000000000000000000000000000000";
   const topologyHash = "549f715f5b06809ada23145c2dc548db";
   const txHash =
     "0x59d881e01ca682130e550e3576b6de760951fb45b1d5dd81342132f57920bbfa";
-
+  const depositAmount = 10;
   const bytes32 = "0x0";
   const emptySetResourceData = "0x";
 
@@ -425,6 +426,47 @@ contract("Bridge - [admin]", async (accounts) => {
     )
   });
 
+  it("Should allow to withdraw funds if called by authorized address", async () => {
+    const tokenOwner = accounts[0];
+    const ERC20HandlerInstance = await ERC20HandlerContract.new(
+      BridgeInstance.address,
+      DefaultMessageReceiverInstance.address
+
+    );
+    const ERC20MintableInstance = await ERC20MintableContract.new(
+      "token",
+      "TOK"
+    );
+    await ERC20MintableInstance.mint(ERC20HandlerInstance.address, depositAmount)
+
+    expect(await ERC20HandlerInstance.hasRole(
+      await ERC20HandlerInstance.LIQUIDITY_MANAGER_ROLE(),
+      tokenOwner
+    )).to.be.equal(false);
+
+    await ERC20HandlerInstance.grantRole(
+      await ERC20HandlerInstance.LIQUIDITY_MANAGER_ROLE(),
+      authorizedAddress,
+      {
+        from: tokenOwner
+      }
+    );
+
+    const recipientBalanceBefore = await ERC20MintableInstance.balanceOf(tokenOwner);
+    const withdrawData = Helpers.createERCWithdrawData(
+      ERC20MintableInstance.address,
+      tokenOwner,
+      depositAmount,
+    );
+
+    await TruffleAssert.passes(ERC20HandlerInstance.withdraw(withdrawData, {from: authorizedAddress}));
+    const recipientBalanceAfter = await ERC20MintableInstance.balanceOf(tokenOwner);
+
+    expect(
+      new Ethers.BigNumber.from(depositAmount).add(recipientBalanceBefore.toString()).toString()
+    ).to.be.equal(recipientBalanceAfter.toString());
+  });
+
   // Set nonce
 
   it("Should set nonce", async () => {

Diff for: test/e2e/erc1155/sameChain.js

@@ -170,7 +170,7 @@ contract("E2E ERC1155 - Same Chain", async (accounts) => {
     );
   });
 
-  it("Handler's withdraw function can be called by only bridge", async () => {
+  it("Handler's withdraw function can be called only by authorized address", async () => {
     const withdrawData = Helpers.createERC1155WithdrawData(
       ERC1155MintableInstance.address,
       depositorAddress,
@@ -179,9 +179,9 @@ contract("E2E ERC1155 - Same Chain", async (accounts) => {
       "0x"
     );
 
-    await Helpers.reverts(
-      ERC1155HandlerInstance.withdraw(withdrawData, {from: depositorAddress}),
-      "sender must be bridge contract"
+    await Helpers.expectToRevertWithCustomError(
+      ERC1155HandlerInstance.withdraw.call(withdrawData, {from: depositorAddress}),
+      "NotAuthorized()"
     );
   });