Merge pull request #134 from bancorprotocol/vortex-withdraw-funds

CarbonVortex - add withdraw funds

README.md

@@ -34,7 +34,7 @@ The security policy is available [here](./SECURITY.md).
 As a first step of contributing to the repo, you should install all the required dependencies via:
 
 ```sh
-yarn install
+pnpm install
 ```
 
 You will also need to create and update the `.env` file if you’d like to interact or run the unit tests against mainnet forks (see [.env.example](./.env.example))
@@ -48,13 +48,13 @@ Testing the protocol is possible via multiple approaches:
 You can run the full test suite via:
 
 ```sh
-yarn test
+pnpm test
 ```
 
 You can also run the test suite with additional stress tests via:
 
 ```sh
-yarn test:nightly
+pnpm test:nightly
 ```
 
 This suite is called “nightly” since it’s scheduled to run every day at midnight against the release and production branches (see [nightly.yml](.github/workflows/nightly.yml)).
@@ -64,7 +64,7 @@ This suite is called “nightly” since it’s scheduled to run every day at mi
 You can test new deployments (and the health of the network) against a mainnet fork via:
 
 ```sh
-yarn test:deploy
+pnpm test:deploy
 ```
 
 This will automatically be skipped on an already deployed and configured deployment scripts and will only test the additional changeset resulting by running any new/pending deployment scripts and perform an e2e test against the up to date state. This is especially useful to verify that any future deployments and upgrades, suggested by the DAO, work correctly and preserve the integrity of the system.
@@ -122,19 +122,19 @@ All files               |     99.3 |    92.55 |    99.28 |    99.28 |
 In order to audit the test coverage of the full test suite, run:
 
 ```sh
-yarn test:coverage
+pnpm test:coverage
 ```
 
 It’s also possible to audit the test coverage of the deployment unit-tests only (which is especially useful when verifying that any future deployments and upgrades are properly covered and tested before the DAO can consider to execute them):
 
 ```sh
-yarn test:coverage:deploy
+pnpm test:coverage:deploy
 ```
 
 Similarly to the regular test suite, it’s also possible to audit the test coverage of the stress test suite via:
 
 ```sh
-yarn test:coverage:nightly
+pnpm test:coverage:nightly
 ```
 
 ## Deployments
@@ -144,13 +144,13 @@ The contracts have built-in support for deployments on different chains and main
 You can deploy the fully configured Carbon protocol via:
 
 ```sh
-yarn deploy
+pnpm deploy
 ```
 
 There’s also a special deployment mode which deploys the protocol to a mainnet fork. It can be run via:
 
 ```sh
-yarn deploy:fork
+pnpm deploy:fork
 ```
 
 ## Community

contracts/token/Token.sol

@@ -4,12 +4,14 @@ pragma solidity 0.8.19;
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import { Address } from "@openzeppelin/contracts/utils/Address.sol";
 
 /**
  * @dev This type implements ERC20 and SafeERC20 utilities for both the native token and for ERC20 tokens
  */
 type Token is address;
 using SafeERC20 for IERC20;
+using Address for address payable;
 
 // the address that represents the native token reserve
 address constant NATIVE_TOKEN_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
@@ -34,7 +36,8 @@ using {
     safeTransfer,
     safeTransferFrom,
     safeApprove,
-    safeIncreaseAllowance
+    safeIncreaseAllowance,
+    unsafeTransfer
 } for Token global;
 
 /* solhint-disable func-visibility */
@@ -144,6 +147,22 @@ function safeIncreaseAllowance(Token token, address spender, uint256 amount) {
     toIERC20(token).safeIncreaseAllowance(spender, amount);
 }
 
+/**
+ * @dev transfers a specific amount of the native token/ERC20 token
+ * @dev forwards all available gas if sending native token
+ */
+function unsafeTransfer(Token token, address to, uint256 amount) {
+    if (amount == 0) {
+        return;
+    }
+
+    if (isNative(token)) {
+        payable(to).sendValue(amount);
+    } else {
+        toIERC20(token).safeTransfer(to, amount);
+    }
+}
+
 /**
  * @dev utility function that converts a token to an IERC20
  */

contracts/vortex/CarbonVortex.sol

@@ -101,7 +101,7 @@ contract CarbonVortex is ICarbonVortex, Upgradeable, ReentrancyGuardUpgradeable,
      * @inheritdoc Upgradeable
      */
     function version() public pure override(IVersioned, Upgradeable) returns (uint16) {
-        return 2;
+        return 3;
     }
 
     /**
@@ -183,6 +183,28 @@ contract CarbonVortex is ICarbonVortex, Upgradeable, ReentrancyGuardUpgradeable,
         _allocateRewards(msg.sender, tokens, rewardAmounts);
     }
 
+    /**
+     * @dev withdraws funds held by the contract and sends them to an account
+     *
+     * requirements:
+     *
+     * - the caller must be the admin of the contract
+     */
+    function withdrawFunds(
+        Token token,
+        address payable target,
+        uint256 amount
+    ) external validAddress(target) nonReentrant onlyAdmin {
+        if (amount == 0) {
+            return;
+        }
+
+        // safe due to nonReentrant modifier (forwards all available gas in case of ETH)
+        token.unsafeTransfer(target, amount);
+
+        emit FundsWithdrawn({ token: token, caller: msg.sender, target: target, amount: amount });
+    }
+
     /**
      * @dev allocates the rewards to caller and burns the rest
      */

contracts/vortex/interfaces/ICarbonVortex.sol

@@ -22,6 +22,11 @@ interface ICarbonVortex is IUpgradeable {
      */
     event RewardsUpdated(uint256 prevRewardsPPM, uint256 newRewardsPPM);
 
+    /**
+     * @dev triggered when tokens have been withdrawn by the admin
+     */
+    event FundsWithdrawn(Token indexed token, address indexed caller, address indexed target, uint256 amount);
+
     /**
      * @dev returns the rewards percentage ppm
      */

deploy/scripts/mainnet/0012-CarbonVortex-upgrade.ts

@@ -0,0 +1,20 @@
+import { DeployedContracts, InstanceName, setDeploymentMetadata, upgradeProxy } from '../../../utils/Deploy';
+import { DeployFunction } from 'hardhat-deploy/types';
+import { HardhatRuntimeEnvironment } from 'hardhat/types';
+
+// carbon vortex withdraw funds upgrade
+const func: DeployFunction = async ({ getNamedAccounts }: HardhatRuntimeEnvironment) => {
+    const { deployer, bnt, bancorNetworkV3 } = await getNamedAccounts();
+
+    const carbonController = await DeployedContracts.CarbonController.deployed();
+
+    await upgradeProxy({
+        name: InstanceName.CarbonVortex,
+        from: deployer,
+        args: [bnt, carbonController.address, bancorNetworkV3]
+    });
+
+    return true;
+};
+
+export default setDeploymentMetadata(__filename, func);

deploy/tests/mainnet/0012-carbon-vortex-upgrade.ts

@@ -0,0 +1,44 @@
+import { CarbonController, CarbonVortex, ProxyAdmin } from '../../../components/Contracts';
+import { DeployedContracts, describeDeployment } from '../../../utils/Deploy';
+import { expect } from 'chai';
+import { ethers } from 'hardhat';
+
+describeDeployment(__filename, () => {
+    let proxyAdmin: ProxyAdmin;
+    let carbonController: CarbonController;
+    let carbonVortex: CarbonVortex;
+
+    beforeEach(async () => {
+        proxyAdmin = await DeployedContracts.ProxyAdmin.deployed();
+        carbonController = await DeployedContracts.CarbonController.deployed();
+        carbonVortex = await DeployedContracts.CarbonVortex.deployed();
+    });
+
+    it('should deploy and configure the carbon vortex contract', async () => {
+        expect(await proxyAdmin.getProxyAdmin(carbonVortex.address)).to.equal(proxyAdmin.address);
+        expect(await carbonVortex.version()).to.equal(3);
+
+        // check that the carbon vortex is the fee manager
+        const role = await carbonController.roleFeesManager();
+        const roleMembers = await carbonController.getRoleMemberCount(role);
+        const feeManagers = [];
+        for (let i = 0; i < roleMembers.toNumber(); ++i) {
+            const feeManagerAddress = await carbonController.getRoleMember(role, i);
+            feeManagers.push(feeManagerAddress);
+        }
+        expect(feeManagers.includes(carbonVortex.address)).to.be.true;
+    });
+
+    it('rewards percentage PPM should be set correctly', async () => {
+        const rewardsPPM = await carbonVortex.rewardsPPM();
+        expect(rewardsPPM).to.be.eq(20_000);
+    });
+
+    it('carbon vortex implementation should be initialized', async () => {
+        const implementationAddress = await proxyAdmin.getProxyImplementation(carbonVortex.address);
+        const carbonVortexImpl: CarbonVortex = await ethers.getContractAt('CarbonVortex', implementationAddress);
+        // hardcoding gas limit to avoid gas estimation attempts (which get rejected instead of reverted)
+        const tx = await carbonVortexImpl.initialize({ gasLimit: 6000000 });
+        await expect(tx.wait()).to.be.reverted;
+    });
+});

test/forge/CarbonVortex.t.sol

@@ -23,6 +23,7 @@ contract CarbonVortexTest is TestFixture {
 
     uint256 private constant REWARDS_PPM_DEFAULT = 100_000;
     uint256 private constant REWARDS_PPM_UPDATED = 110_000;
+    uint256 private constant MAX_WITHDRAW_AMOUNT = 100_000_000 ether;
 
     // Events
     /**
@@ -40,6 +41,11 @@ contract CarbonVortexTest is TestFixture {
      */
     event FeesWithdrawn(Token indexed token, address indexed recipient, uint256 indexed amount, address sender);
 
+    /**
+     * @dev triggered when tokens have been withdrawn by admin
+     */
+    event FundsWithdrawn(Token indexed token, address indexed caller, address indexed target, uint256 amount);
+
     /**
      * @dev Emitted when the allowance of a `spender` for an `owner` is set by
      * a call to {approve}. `value` is the new allowance.
@@ -85,7 +91,7 @@ contract CarbonVortexTest is TestFixture {
 
     function testShouldBeInitialized() public {
         uint16 version = carbonVortex.version();
-        assertEq(version, 2);
+        assertEq(version, 3);
     }
 
     function testShouldntBeAbleToReinitialize() public {
@@ -134,6 +140,100 @@ contract CarbonVortexTest is TestFixture {
         vm.stopPrank();
     }
 
+    /**
+     * @dev withdrawFunds tests
+     */
+
+    /// @dev test should revert when attempting to withdraw funds without the admin role
+    function testShouldRevertWhenAttemptingToWithdrawFundsWithoutTheAdminRole() public {
+        uint256 withdrawAmount = 1000;
+        vm.prank(user2);
+        vm.expectRevert(AccessDenied.selector);
+        carbonVortex.withdrawFunds(token1, user2, withdrawAmount);
+    }
+
+    /// @dev test should revert when attempting to withdraw funds to an invalid address
+    function testShouldRevertWhenAttemptingToWithdrawFundsToAnInvalidAddress() public {
+        uint256 withdrawAmount = 1000;
+        vm.prank(admin);
+        vm.expectRevert(InvalidAddress.selector);
+        carbonVortex.withdrawFunds(token1, payable(address(0)), withdrawAmount);
+    }
+
+    /// @dev test admin should be able to withdraw tokens
+    function testAdminShouldBeAbleToWithdrawTokens(uint256 withdrawAmount) public {
+        withdrawAmount = bound(withdrawAmount, 0, MAX_WITHDRAW_AMOUNT);
+
+        vm.startPrank(admin);
+        // transfer funds to vortex
+        token1.safeTransfer(address(carbonVortex), MAX_WITHDRAW_AMOUNT);
+
+        uint256 balanceBeforeVault = token1.balanceOf(address(carbonVortex));
+        uint256 balanceBeforeUser2 = token1.balanceOf(user2);
+
+        // withdraw token to user2
+        carbonVortex.withdrawFunds(token1, user2, withdrawAmount);
+
+        uint256 balanceAfterVault = token1.balanceOf(address(carbonVortex));
+        uint256 balanceAfterUser2 = token1.balanceOf(user2);
+
+        uint256 balanceWithdrawn = balanceBeforeVault - balanceAfterVault;
+        uint256 balanceGainUser2 = balanceAfterUser2 - balanceBeforeUser2;
+
+        assertEq(balanceWithdrawn, withdrawAmount);
+        assertEq(balanceGainUser2, withdrawAmount);
+    }
+
+    /// @dev test admin should be able to withdraw the native token
+    function testAdminShouldBeAbleToWithdrawNativeToken(uint256 withdrawAmount) public {
+        withdrawAmount = bound(withdrawAmount, 0, MAX_WITHDRAW_AMOUNT);
+
+        // transfer eth to the vortex
+        vm.deal(address(carbonVortex), MAX_WITHDRAW_AMOUNT);
+
+        uint256 balanceBeforeVault = address(carbonVortex).balance;
+        uint256 balanceBeforeUser2 = user2.balance;
+
+        vm.prank(admin);
+        // withdraw native token to user2
+        carbonVortex.withdrawFunds(NATIVE_TOKEN, user2, withdrawAmount);
+
+        uint256 balanceAfterVault = address(carbonVortex).balance;
+        uint256 balanceAfterUser2 = user2.balance;
+
+        uint256 balanceWithdrawn = balanceBeforeVault - balanceAfterVault;
+        uint256 balanceGainUser2 = balanceAfterUser2 - balanceBeforeUser2;
+
+        assertEq(balanceWithdrawn, withdrawAmount);
+        assertEq(balanceGainUser2, withdrawAmount);
+    }
+
+    /// @dev test withdrawing funds should emit event
+    function testWithdrawingFundsShouldEmitEvent() public {
+        uint256 withdrawAmount = 1000;
+
+        vm.startPrank(admin);
+        // transfer funds to vortex
+        token1.safeTransfer(address(carbonVortex), withdrawAmount);
+
+        vm.expectEmit();
+        emit FundsWithdrawn(token1, admin, user2, withdrawAmount);
+        // withdraw token to user2
+        carbonVortex.withdrawFunds(token1, user2, withdrawAmount);
+
+        vm.stopPrank();
+    }
+
+    /// @dev test withdrawing funds shouldn't emit event if withdrawing zero amount
+    function testFailWithdrawingFundsShouldnEmitEventIfWithdrawingZeroAmount() public {
+        uint256 withdrawAmount = 0;
+        vm.prank(admin);
+        vm.expectEmit();
+        emit FundsWithdrawn(token1, admin, user2, withdrawAmount);
+        // withdraw token to user2
+        carbonVortex.withdrawFunds(token1, user2, withdrawAmount);
+    }
+
     /**
      * @dev rewards distribution and bnt burn tests
      */

test/forge/POLTestCaseParser.t.sol

@@ -45,7 +45,7 @@ contract POLTestCaseParser is Test {
     function parseInitialPrice(
         string memory json,
         string memory initialParseString
-    ) private returns (ICarbonPOL.Price memory price) {
+    ) private pure returns (ICarbonPOL.Price memory price) {
         uint256 initialPriceSourceAmount = vm.parseJsonUint(
             json,
             string.concat(initialParseString, "].initialPriceSourceAmount")
@@ -66,7 +66,7 @@ contract POLTestCaseParser is Test {
     function parseTestCases(
         string memory json,
         string memory templateName
-    ) private returns (TestCase[] memory testCases) {
+    ) private pure returns (TestCase[] memory testCases) {
         string memory initialParseString = string.concat("$.", templateName);
 
         // read the test case length