Add ability to sweep funds from Bulker (#413)

This PR adds an admin to the Bulker contract and allows the admin to sweep ERC20 tokens and ETH out of the contract to handle accidentally sent funds. This addresses part of #376's concern

This PR also removes the getAmount helper used in the Bulker to calculate the max transfer/withdraw values, as max transfer/withdraw are now supported natively in the protocol.

Author: kevincheng96

contracts/Bulker.sol

@@ -2,12 +2,14 @@
 pragma solidity 0.8.13;
 
 import "./CometInterface.sol";
+import "./ERC20.sol";
 import "./IWETH9.sol";
 
 contract Bulker {
     /** General configuration constants **/
-    address payable public immutable weth;
+    address public immutable admin;
     address public immutable comet;
+    address payable public immutable weth;
     address public immutable baseToken;
 
     /** Actions **/
@@ -20,8 +22,10 @@ contract Bulker {
     /** Custom errors **/
     error InvalidArgument();
     error FailedToSendEther();
+    error Unauthorized();
 
-    constructor(address comet_, address payable weth_) {
+    constructor(address admin_, address comet_, address payable weth_) {
+        admin = admin_;
         comet = comet_;
         weth = weth_;
         baseToken = CometInterface(comet_).baseToken();
@@ -32,6 +36,30 @@ contract Bulker {
      */
     receive() external payable {}
 
+    /**
+     * @notice A public function to sweep accidental ERC-20 transfers to this contract. Tokens are sent to admin (Timelock)
+     * @param recipient The address that will receive the swept funds
+     * @param asset The address of the ERC-20 token to sweep
+     */
+    function sweepToken(address recipient, ERC20 asset) external {
+        if (msg.sender != admin) revert Unauthorized();
+
+        uint256 balance = asset.balanceOf(address(this));
+        asset.transfer(recipient, balance);
+    }
+
+    /**
+     * @notice A public function to sweep accidental ETH transfers to this contract. Tokens are sent to admin (Timelock)
+     * @param recipient The address that will receive the swept funds
+     */
+    function sweepEth(address recipient) external {
+        if (msg.sender != admin) revert Unauthorized();
+
+        uint256 balance = address(this).balance;
+        (bool success, ) = recipient.call{ value: balance }("");
+        if (!success) revert FailedToSendEther();
+    }
+
     /**
      * @notice Executes a list of actions in order
      * @param actions The list of actions to execute in order
@@ -90,40 +118,23 @@ contract Bulker {
      * @notice Transfers an asset to a user in Comet
      */
     function transferTo(address to, address asset, uint amount) internal {
-        amount = getAmount(asset, amount);
         CometInterface(comet).transferAssetFrom(msg.sender, to, asset, amount);
     }
 
     /**
      * @notice Withdraws an asset to a user in Comet
      */
     function withdrawTo(address to, address asset, uint amount) internal {
-        amount = getAmount(asset, amount);
         CometInterface(comet).withdrawFrom(msg.sender, to, asset, amount);
     }
 
     /**
      * @notice Withdraws WETH from Comet to a user after unwrapping it to ETH
      */
     function withdrawEthTo(address to, uint amount) internal {
-        amount = getAmount(weth, amount);
         CometInterface(comet).withdrawFrom(msg.sender, address(this), weth, amount);
         IWETH9(weth).withdraw(amount);
         (bool success, ) = to.call{ value: amount }("");
         if (!success) revert FailedToSendEther();
     }
-
-    /**
-     * @notice Handles the max transfer/withdraw case so that no dust is left in the protocol.
-     */
-    function getAmount(address asset, uint amount) internal view returns (uint) {
-        if (amount == type(uint256).max) {
-            if (asset == baseToken) {
-                return CometInterface(comet).balanceOf(msg.sender);
-            } else {
-                return CometInterface(comet).collateralBalanceOf(msg.sender, asset);
-            }
-        }
-        return amount;
-    }
 }

deployments/kovan/migrations/1651257129_bulker_and_rewards.ts

@@ -16,9 +16,9 @@ migration<Vars>('1651257129_bulker_and_rewards', {
     const timelock = await deploymentManager.contract('timelock') as SimpleTimelock;
 
     // Deploy new Bulker and Rewards contracts
-    const newBulker = await deploymentManager.deploy<Bulker, Bulker__factory, [string, string]>(
+    const newBulker = await deploymentManager.deploy<Bulker, Bulker__factory, [string, string, string]>(
       'Bulker.sol',
-      [comet.address, weth.address]
+      [timelock.address, comet.address, weth.address]
     );
 
     const newRewards = await deploymentManager.deploy<CometRewards, CometRewards__factory, [string]>(

test/bulker-test.ts

@@ -329,6 +329,77 @@ describe('bulker', function () {
     await expect(bulker.connect(alice).invoke([await bulker.ACTION_WITHDRAW_ETH()], [withdrawEthCalldata]))
       .to.be.reverted; // Should revert with "custom error 'Unauthorized()'"
   });
+
+  describe('admin functions', function () {
+    it('sweep ERC20 token', async () => {
+      const protocol = await makeProtocol({});
+      const { comet, governor, tokens: { USDC, WETH }, users: [alice] } = protocol;
+      const bulkerInfo = await makeBulker({ admin: governor, comet: comet.address, weth: WETH.address });
+      const { bulker } = bulkerInfo;
+
+      // Alice "accidentally" sends 10 USDC to the Bulker
+      const transferAmount = exp(10, 6);
+      await USDC.allocateTo(alice.address, transferAmount);
+      await USDC.connect(alice).transfer(bulker.address, transferAmount);
+
+      const oldBulkerBalance = await USDC.balanceOf(bulker.address);
+      const oldGovBalance = await USDC.balanceOf(governor.address);
+
+      // Governor sweeps tokens
+      await bulker.connect(governor).sweepToken(governor.address, USDC.address);
+
+      const newBulkerBalance = await USDC.balanceOf(bulker.address);
+      const newGovBalance = await USDC.balanceOf(governor.address);
+
+      expect(newBulkerBalance.sub(oldBulkerBalance)).to.be.equal(-transferAmount);
+      expect(newGovBalance.sub(oldGovBalance)).to.be.equal(transferAmount);
+    });
+
+    it('sweep ETH', async () => {
+      const protocol = await makeProtocol({});
+      const { comet, governor, tokens: { WETH }, users: [alice] } = protocol;
+      const bulkerInfo = await makeBulker({ admin: governor, comet: comet.address, weth: WETH.address });
+      const { bulker } = bulkerInfo;
+
+      // Alice "accidentally" sends 1 ETH to the Bulker
+      const transferAmount = exp(1, 18);
+      await alice.sendTransaction({ to: bulker.address, value: transferAmount });
+
+      const oldBulkerBalance = await ethers.provider.getBalance(bulker.address);
+      const oldGovBalance = await ethers.provider.getBalance(governor.address);
+
+      // Governor sweeps ETH
+      const txn = await wait(bulker.connect(governor).sweepEth(governor.address));
+
+      const newBulkerBalance = await ethers.provider.getBalance(bulker.address);
+      const newGovBalance = await ethers.provider.getBalance(governor.address);
+
+      expect(newBulkerBalance.sub(oldBulkerBalance)).to.be.equal(-transferAmount);
+      expect(newGovBalance.sub(oldGovBalance)).to.be.equal(transferAmount - getGasUsed(txn));
+    });
+
+    it('reverts if sweepToken is called by non-admin', async () => {
+      const protocol = await makeProtocol({});
+      const { comet, governor, tokens: { USDC, WETH }, users: [alice] } = protocol;
+      const bulkerInfo = await makeBulker({ admin: governor, comet: comet.address, weth: WETH.address });
+      const { bulker } = bulkerInfo;
+
+      // Alice sweeps tokens
+      await expect(bulker.connect(alice).sweepToken(governor.address, USDC.address))
+        .to.be.revertedWith("custom error 'Unauthorized()'");
+    });
+
+    it('reverts if sweepEth is called by non-admin', async () => {
+      const protocol = await makeProtocol({});
+      const { comet, governor, tokens: { WETH }, users: [alice] } = protocol;
+      const bulkerInfo = await makeBulker({ admin: governor, comet: comet.address, weth: WETH.address });
+      const { bulker } = bulkerInfo;
+
+      // Alice sweeps ETH
+      await expect(bulker.connect(alice).sweepEth(governor.address))
+        .to.be.revertedWith("custom error 'Unauthorized()'");
+    });
+  });
 });
 
 describe('bulker multiple actions', function () {

test/helpers.ts

@@ -120,8 +120,9 @@ export type Rewards = {
 };
 
 export type BulkerOpts = {
-  comet: string;
-  weth: string;
+  admin?: SignerWithAddress;
+  comet?: string;
+  weth?: string;
 };
 
 export type BulkerInfo = {
@@ -466,11 +467,14 @@ export async function makeRewards(opts: RewardsOpts = {}): Promise<Rewards> {
 }
 
 export async function makeBulker(opts: BulkerOpts): Promise<BulkerInfo> {
+  const signers = await ethers.getSigners();
+
+  const admin = opts.admin || signers[0];
   const comet = opts.comet;
   const weth = opts.weth;
 
   const BulkerFactory = (await ethers.getContractFactory('Bulker')) as Bulker__factory;
-  const bulker = await BulkerFactory.deploy(comet, weth);
+  const bulker = await BulkerFactory.deploy(admin.address, comet, weth);
   await bulker.deployed();
 
   return {