Merge pull request #77 from bancorprotocol/deposit-permitted

lbeder · web-flow · commit 0c365d1edba4 · 2021-09-16T13:03:50.000+03:00
Add permitted deposit flows + with minor security issue in PendingWithdrawals
diff --git a/packages/v3/contracts/helpers/TestERC20Token.sol b/packages/v3/contracts/helpers/TestERC20Token.sol
@@ -2,13 +2,14 @@
 pragma solidity 0.7.6;
 
 import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+import { ERC20Permit } from "@openzeppelin/contracts/drafts/ERC20Permit.sol";
 
-contract TestERC20Token is ERC20 {
+contract TestERC20Token is ERC20Permit {
     constructor(
         string memory name,
         string memory symbol,
         uint256 totalSupply
-    ) ERC20(name, symbol) {
+    ) ERC20(name, symbol) ERC20Permit(name) {
         _mint(msg.sender, totalSupply);
     }
 
diff --git a/packages/v3/contracts/network/BancorNetwork.sol b/packages/v3/contracts/network/BancorNetwork.sol
@@ -5,6 +5,7 @@ pragma abicoder v2;
 import { Address } from "@openzeppelin/contracts/utils/Address.sol";
 import { SafeMath } from "@openzeppelin/contracts/math/SafeMath.sol";
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import { IERC20Permit } from "@openzeppelin/contracts/drafts/IERC20Permit.sol";
 import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";
 import { EnumerableSetUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/EnumerableSetUpgradeable.sol";
 
@@ -541,8 +542,8 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, OwnedUpgradeable, Reentra
         external
         payable
         override
-        validAddress(address(pool))
         validAddress(provider)
+        validAddress(address(pool))
         greaterThanZero(tokenAmount)
         nonReentrant
     {
@@ -563,15 +564,42 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, OwnedUpgradeable, Reentra
         _depositFor(msg.sender, pool, tokenAmount, msg.sender);
     }
 
+    /**
+     * @inheritdoc IBancorNetwork
+     */
+    function depositForPermitted(
+        address provider,
+        IReserveToken pool,
+        uint256 tokenAmount,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) external override validAddress(provider) validAddress(address(pool)) greaterThanZero(tokenAmount) nonReentrant {
+        _depositBaseTokenForPermitted(provider, pool, tokenAmount, msg.sender, deadline, v, r, s);
+    }
+
+    /**
+     * @inheritdoc IBancorNetwork
+     */
+    function depositPermitted(
+        IReserveToken pool,
+        uint256 tokenAmount,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) external override validAddress(address(pool)) greaterThanZero(tokenAmount) nonReentrant {
+        _depositBaseTokenForPermitted(msg.sender, pool, tokenAmount, msg.sender, deadline, v, r, s);
+    }
+
     /**
      * @inheritdoc IBancorNetwork
      */
     function withdraw(uint256 id) external override nonReentrant {
+        bytes32 contextId = _withdrawContextId(id);
         address provider = msg.sender;
 
-        // generate context ID for monitoring
-        bytes32 contextId = keccak256(abi.encodePacked(provider, _time(), id));
-
         // complete the withdrawal and claim the locked pool tokens
         CompletedWithdrawal memory completedRequest = _pendingWithdrawals.completeWithdrawal(contextId, provider, id);
 
@@ -614,6 +642,25 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, OwnedUpgradeable, Reentra
         );
     }
 
+    /**
+     * @dev generates context ID for a deposit requesst
+     */
+    function _depositContextId(
+        address provider,
+        IReserveToken pool,
+        uint256 tokenAmount,
+        address sender
+    ) private view returns (bytes32) {
+        return keccak256(abi.encodePacked(provider, _time(), pool, tokenAmount, sender));
+    }
+
+    /**
+     * @dev generates context ID for a withdraw request
+     */
+    function _withdrawContextId(uint256 id) private view returns (bytes32) {
+        return keccak256(abi.encodePacked(msg.sender, _time(), id));
+    }
+
     /**
      * @dev deposits liquidity for the specified provider from sender
      *
@@ -627,8 +674,7 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, OwnedUpgradeable, Reentra
         uint256 tokenAmount,
         address sender
     ) private {
-        // generate context ID for monitoring
-        bytes32 contextId = keccak256(abi.encodePacked(provider, _time(), pool, tokenAmount, sender));
+        bytes32 contextId = _depositContextId(provider, pool, tokenAmount, sender);
 
         if (pool == IReserveToken(address(_networkToken))) {
             _depositNetworkTokenFor(contextId, provider, tokenAmount, sender);
@@ -785,6 +831,28 @@ contract BancorNetwork is IBancorNetwork, Upgradeable, OwnedUpgradeable, Reentra
         });
     }
 
+    function _depositBaseTokenForPermitted(
+        address provider,
+        IReserveToken pool,
+        uint256 tokenAmount,
+        address sender,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) private {
+        // neither the network token nor ETH support EIP2612 permit requests
+        require(pool != IReserveToken(address(_networkToken)) && !pool.isNativeToken(), "ERR_PERMIT_UNSUPPORTED");
+
+        // permit the amount the caller is trying to deposit. Please note, that if the base token doesn't support
+        // EIP2612 permit - either this call of the inner safeTransferFrom will revert
+        IERC20Permit(address(pool)).permit(sender, address(this), tokenAmount, deadline, v, r, s);
+
+        bytes32 contextId = _depositContextId(provider, pool, tokenAmount, sender);
+
+        _depositBaseTokenFor(contextId, provider, pool, tokenAmount, sender);
+    }
+
     /**
      * @dev handles network token withdrawal
      */
diff --git a/packages/v3/contracts/network/PendingWithdrawals.sol b/packages/v3/contracts/network/PendingWithdrawals.sol
@@ -250,18 +250,17 @@ contract PendingWithdrawals is
     /**
      * @inheritdoc IPendingWithdrawals
      */
-    function initWithdrawalDelegated(
+    function initWithdrawalPermitted(
         IPoolToken poolToken,
         uint256 poolTokenAmount,
-        address provider,
         uint256 deadline,
         uint8 v,
         bytes32 r,
         bytes32 s
     ) external override validAddress(address(poolToken)) greaterThanZero(poolTokenAmount) nonReentrant {
-        poolToken.permit(provider, address(this), poolTokenAmount, deadline, v, r, s);
+        poolToken.permit(msg.sender, address(this), poolTokenAmount, deadline, v, r, s);
 
-        _initWithdrawal(provider, poolToken, poolTokenAmount);
+        _initWithdrawal(msg.sender, poolToken, poolTokenAmount);
     }
 
     /**
diff --git a/packages/v3/contracts/network/interfaces/IBancorNetwork.sol b/packages/v3/contracts/network/interfaces/IBancorNetwork.sol
@@ -128,6 +128,41 @@ interface IBancorNetwork is IUpgradeable {
      */
     function deposit(IReserveToken pool, uint256 tokenAmount) external payable;
 
+    /**
+     * @dev deposits liquidity for the specified provider by providing an EIP712 typed signature for an EIP2612 permit
+     * request
+     *
+     * requirements:
+     *
+     * - the caller must have provided a valid and unused EIP712 typed signature
+     */
+    function depositForPermitted(
+        address provider,
+        IReserveToken pool,
+        uint256 tokenAmount,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) external;
+
+    /**
+     * @dev deposits liquidity by providing an EIP712 typed signature for an EIP2612 permit
+     * request
+     *
+     * requirements:
+     *
+     * - the caller must have provided a valid and unused EIP712 typed signature
+     */
+    function depositPermitted(
+        IReserveToken pool,
+        uint256 tokenAmount,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) external;
+
     /**
      * @dev withdraws liquidity
      *
diff --git a/packages/v3/contracts/network/interfaces/IPendingWithdrawals.sol b/packages/v3/contracts/network/interfaces/IPendingWithdrawals.sol
@@ -79,10 +79,9 @@ interface IPendingWithdrawals is IUpgradeable {
      *
      * - the caller must have provided a valid and unused EIP712 typed signature
      */
-    function initWithdrawalDelegated(
+    function initWithdrawalPermitted(
         IPoolToken poolToken,
         uint256 poolTokenAmount,
-        address provider,
         uint256 deadline,
         uint8 v,
         bytes32 r,
diff --git a/packages/v3/test/helpers/Utils.ts b/packages/v3/test/helpers/Utils.ts
@@ -1,10 +1,10 @@
+import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';
+import { BaseContract, BigNumber, BigNumberish, ContractTransaction, Wallet } from 'ethers';
+import { ethers, waffle } from 'hardhat';
 import Contracts from '../../components/Contracts';
 import { TestERC20Token } from '../../typechain';
 import { NATIVE_TOKEN_ADDRESS } from './Constants';
 import { toWei } from './Types';
-import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';
-import { BigNumber, BigNumberish, ContractTransaction, BaseContract } from 'ethers';
-import { ethers } from 'hardhat';
 
 export type TokenWithAddress = TestERC20Token | { address: string };
 
@@ -66,3 +66,12 @@ export const createTokenBySymbol = async (symbol: string, networkToken: TestERC2
             throw new Error(`Unsupported type ${symbol}`);
     }
 };
+
+export const createWallet = async () => {
+    // create a random wallet, connect it to a test provider, and fund it
+    const wallet = Wallet.createRandom().connect(waffle.provider);
+    const deployer = (await ethers.getSigners())[0];
+    await deployer.sendTransaction({ value: toWei(BigNumber.from(10)), to: await wallet.getAddress() });
+
+    return wallet;
+};
diff --git a/packages/v3/test/network/BancorNetwork.ts b/packages/v3/test/network/BancorNetwork.ts
diff --git a/packages/v3/test/network/PendingWithdrawals.ts b/packages/v3/test/network/PendingWithdrawals.ts
