[Feature] Spender permit feature

contracts/AMMWrapper.sol

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.7.6;
+pragma abicoder v2;
 
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol";
@@ -114,30 +115,12 @@ contract AMMWrapper is IAMMWrapper, StrategyBase, ReentrancyGuard, BaseLibEIP712
      *                   External functions                      *
      *************************************************************/
     function trade(
-        address _makerAddr,
-        address _takerAssetAddr,
-        address _makerAssetAddr,
-        uint256 _takerAssetAmount,
-        uint256 _makerAssetAmount,
+        AMMLibEIP712.Order calldata _order,
         uint256 _feeFactor,
-        address _userAddr,
-        address payable _receiverAddr,
-        uint256 _salt,
-        uint256 _deadline,
-        bytes calldata _sig
+        bytes calldata _sig,
+        bytes calldata _takerAssetPermitSig
     ) external payable override nonReentrant onlyUserProxy returns (uint256) {
-        AMMLibEIP712.Order memory order = AMMLibEIP712.Order(
-            _makerAddr,
-            _takerAssetAddr,
-            _makerAssetAddr,
-            _takerAssetAmount,
-            _makerAssetAmount,
-            _userAddr,
-            _receiverAddr,
-            _salt,
-            _deadline
-        );
-        require(order.deadline >= block.timestamp, "AMMWrapper: expired order");
+        require(_order.deadline >= block.timestamp, "AMMWrapper: expired order");
 
         // These variables are copied straight from function parameters and
         // used to bypass stack too deep error.
@@ -151,36 +134,36 @@ contract AMMWrapper is IAMMWrapper, StrategyBase, ReentrancyGuard, BaseLibEIP712
         }
 
         // Assign trade vairables
-        internalTxData.fromEth = (order.takerAssetAddr == LibConstant.ZERO_ADDRESS || order.takerAssetAddr == LibConstant.ETH_ADDRESS);
-        internalTxData.toEth = (order.makerAssetAddr == LibConstant.ZERO_ADDRESS || order.makerAssetAddr == LibConstant.ETH_ADDRESS);
-        if (_isCurve(order.makerAddr)) {
+        internalTxData.fromEth = (_order.takerAssetAddr == LibConstant.ZERO_ADDRESS || _order.takerAssetAddr == LibConstant.ETH_ADDRESS);
+        internalTxData.toEth = (_order.makerAssetAddr == LibConstant.ZERO_ADDRESS || _order.makerAssetAddr == LibConstant.ETH_ADDRESS);
+        if (_isCurve(_order.makerAddr)) {
             // PermanetStorage can recognize `ETH_ADDRESS` but not `ZERO_ADDRESS`.
             // Convert it to `ETH_ADDRESS` as passed in `order.takerAssetAddr` or `order.makerAssetAddr` might be `ZERO_ADDRESS`.
-            internalTxData.takerAssetInternalAddr = internalTxData.fromEth ? LibConstant.ETH_ADDRESS : order.takerAssetAddr;
-            internalTxData.makerAssetInternalAddr = internalTxData.toEth ? LibConstant.ETH_ADDRESS : order.makerAssetAddr;
+            internalTxData.takerAssetInternalAddr = internalTxData.fromEth ? LibConstant.ETH_ADDRESS : _order.takerAssetAddr;
+            internalTxData.makerAssetInternalAddr = internalTxData.toEth ? LibConstant.ETH_ADDRESS : _order.makerAssetAddr;
         } else {
-            internalTxData.takerAssetInternalAddr = internalTxData.fromEth ? address(weth) : order.takerAssetAddr;
-            internalTxData.makerAssetInternalAddr = internalTxData.toEth ? address(weth) : order.makerAssetAddr;
+            internalTxData.takerAssetInternalAddr = internalTxData.fromEth ? address(weth) : _order.takerAssetAddr;
+            internalTxData.makerAssetInternalAddr = internalTxData.toEth ? address(weth) : _order.makerAssetAddr;
         }
 
-        txMetaData.transactionHash = _verify(order, _sig);
+        txMetaData.transactionHash = _verify(_order, _sig);
 
-        _prepare(order, internalTxData);
+        _transferTakerAssetToAMM(_order, internalTxData, _takerAssetPermitSig);
 
         {
             // Set min amount for swap = _order.makerAssetAmount * (10000 / (10000 - feeFactor))
-            uint256 swapMinOutAmount = order.makerAssetAmount.mul(LibConstant.BPS_MAX).div(LibConstant.BPS_MAX.sub(txMetaData.feeFactor));
-            (txMetaData.source, txMetaData.receivedAmount) = _swap(order, internalTxData, swapMinOutAmount);
+            uint256 swapMinOutAmount = _order.makerAssetAmount.mul(LibConstant.BPS_MAX).div(LibConstant.BPS_MAX.sub(txMetaData.feeFactor));
+            (txMetaData.source, txMetaData.receivedAmount) = _swap(_order, internalTxData, swapMinOutAmount);
 
             // Settle
             // Calculate fee using actually received from swap
             uint256 actualFee = txMetaData.receivedAmount.mul(txMetaData.feeFactor).div(LibConstant.BPS_MAX);
             txMetaData.settleAmount = txMetaData.receivedAmount.sub(actualFee);
-            require(txMetaData.settleAmount >= order.makerAssetAmount, "AMMWrapper: insufficient maker output");
-            _settle(order, internalTxData, txMetaData.settleAmount, actualFee);
+            require(txMetaData.settleAmount >= _order.makerAssetAmount, "AMMWrapper: insufficient maker output");
+            _settle(_order, internalTxData, txMetaData.settleAmount, actualFee);
         }
 
-        emitSwappedEvent(order, txMetaData);
+        emitSwappedEvent(_order, txMetaData);
         return txMetaData.settleAmount;
     }
 
@@ -229,9 +212,13 @@ contract AMMWrapper is IAMMWrapper, StrategyBase, ReentrancyGuard, BaseLibEIP712
 
     /**
      * @dev internal function of `trade`.
-     * It executes the swap on chosen AMM.
+     * It transfers assets from the taker to this contract with taker's permission.
      */
-    function _prepare(AMMLibEIP712.Order memory _order, InternalTxData memory _internalTxData) internal {
+    function _transferTakerAssetToAMM(
+        AMMLibEIP712.Order memory _order,
+        InternalTxData memory _internalTxData,
+        bytes memory _takerAssetPermitSig
+    ) internal {
         // Transfer asset from user and deposit to weth if needed
         if (_internalTxData.fromEth) {
             require(msg.value > 0, "AMMWrapper: msg.value is zero");
@@ -241,8 +228,17 @@ contract AMMWrapper is IAMMWrapper, StrategyBase, ReentrancyGuard, BaseLibEIP712
                 weth.deposit{ value: msg.value }();
             }
         } else {
-            // other ERC20 tokens
-            spender.spendFromUser(_order.userAddr, _order.takerAssetAddr, _order.takerAssetAmount);
+            // Transfer other ERC20 tokens to this contract
+            SpenderLibEIP712.SpendWithPermit memory takerAssetPermit = SpenderLibEIP712.SpendWithPermit({
+                tokenAddr: _order.takerAssetAddr,
+                requester: address(this),
+                user: _order.userAddr,
+                recipient: address(this),
+                amount: _order.takerAssetAmount,
+                actionHash: AMMLibEIP712._getOrderHash(_order),
+                expiry: uint64(_order.deadline)
+            });
+            spender.spendFromUserToWithPermit(takerAssetPermit, _takerAssetPermitSig);
         }
     }
 

contracts/AMMWrapperWithPath.sol

@@ -51,59 +51,53 @@ contract AMMWrapperWithPath is IAMMWrapperWithPath, AMMWrapper {
      *                   External functions                      *
      *************************************************************/
 
-    function trade(
-        AMMLibEIP712.Order calldata _order,
-        uint256 _feeFactor,
-        bytes calldata _sig,
-        bytes calldata _makerSpecificData,
-        address[] calldata _path
-    ) external payable override nonReentrant onlyUserProxy returns (uint256) {
-        require(_order.deadline >= block.timestamp, "AMMWrapper: expired order");
+    function trade(IAMMWrapperWithPath.TradeWithPathParams calldata _params) external payable override nonReentrant onlyUserProxy returns (uint256) {
+        require(_params.order.deadline >= block.timestamp, "AMMWrapper: expired order");
 
         // These variables are copied straight from function parameters and
         // used to bypass stack too deep error.
         TxMetaData memory txMetaData;
         InternalTxData memory internalTxData;
-        txMetaData.feeFactor = uint16(_feeFactor);
+        txMetaData.feeFactor = uint16(_params.feeFactor);
         txMetaData.relayed = permStorage.isRelayerValid(tx.origin);
-        internalTxData.makerSpecificData = _makerSpecificData;
-        internalTxData.path = _path;
+        internalTxData.makerSpecificData = _params.makerSpecificData;
+        internalTxData.path = _params.path;
         if (!txMetaData.relayed) {
             // overwrite feeFactor with defaultFeeFactor if not from valid relayer
             txMetaData.feeFactor = defaultFeeFactor;
         }
 
         // Assign trade vairables
-        internalTxData.fromEth = (_order.takerAssetAddr == LibConstant.ZERO_ADDRESS || _order.takerAssetAddr == LibConstant.ETH_ADDRESS);
-        internalTxData.toEth = (_order.makerAssetAddr == LibConstant.ZERO_ADDRESS || _order.makerAssetAddr == LibConstant.ETH_ADDRESS);
-        if (_isCurve(_order.makerAddr)) {
+        internalTxData.fromEth = (_params.order.takerAssetAddr == LibConstant.ZERO_ADDRESS || _params.order.takerAssetAddr == LibConstant.ETH_ADDRESS);
+        internalTxData.toEth = (_params.order.makerAssetAddr == LibConstant.ZERO_ADDRESS || _params.order.makerAssetAddr == LibConstant.ETH_ADDRESS);
+        if (_isCurve(_params.order.makerAddr)) {
             // PermanetStorage can recognize `ETH_ADDRESS` but not `ZERO_ADDRESS`.
             // Convert it to `ETH_ADDRESS` as passed in `_order.takerAssetAddr` or `_order.makerAssetAddr` might be `ZERO_ADDRESS`.
-            internalTxData.takerAssetInternalAddr = internalTxData.fromEth ? LibConstant.ETH_ADDRESS : _order.takerAssetAddr;
-            internalTxData.makerAssetInternalAddr = internalTxData.toEth ? LibConstant.ETH_ADDRESS : _order.makerAssetAddr;
+            internalTxData.takerAssetInternalAddr = internalTxData.fromEth ? LibConstant.ETH_ADDRESS : _params.order.takerAssetAddr;
+            internalTxData.makerAssetInternalAddr = internalTxData.toEth ? LibConstant.ETH_ADDRESS : _params.order.makerAssetAddr;
         } else {
-            internalTxData.takerAssetInternalAddr = internalTxData.fromEth ? address(weth) : _order.takerAssetAddr;
-            internalTxData.makerAssetInternalAddr = internalTxData.toEth ? address(weth) : _order.makerAssetAddr;
+            internalTxData.takerAssetInternalAddr = internalTxData.fromEth ? address(weth) : _params.order.takerAssetAddr;
+            internalTxData.makerAssetInternalAddr = internalTxData.toEth ? address(weth) : _params.order.makerAssetAddr;
         }
 
-        txMetaData.transactionHash = _verify(_order, _sig);
+        txMetaData.transactionHash = _verify(_params.order, _params.sig);
 
-        _prepare(_order, internalTxData);
+        _transferTakerAssetToAMM(_params.order, internalTxData, _params.takerAssetPermitSig);
 
         {
             // Set min amount for swap = _order.makerAssetAmount * (10000 / (10000 - feeFactor))
-            uint256 swapMinOutAmount = _order.makerAssetAmount.mul(LibConstant.BPS_MAX).div(LibConstant.BPS_MAX.sub(txMetaData.feeFactor));
-            (txMetaData.source, txMetaData.receivedAmount) = _swapWithPath(_order, internalTxData, swapMinOutAmount);
+            uint256 swapMinOutAmount = _params.order.makerAssetAmount.mul(LibConstant.BPS_MAX).div(LibConstant.BPS_MAX.sub(txMetaData.feeFactor));
+            (txMetaData.source, txMetaData.receivedAmount) = _swapWithPath(_params.order, internalTxData, swapMinOutAmount);
 
             // Settle
             // Calculate fee using actually received from swap
             uint256 actualFee = txMetaData.receivedAmount.mul(txMetaData.feeFactor).div(LibConstant.BPS_MAX);
             txMetaData.settleAmount = txMetaData.receivedAmount.sub(actualFee);
-            require(txMetaData.settleAmount >= _order.makerAssetAmount, "AMMWrapper: insufficient maker output");
-            _settle(_order, internalTxData, txMetaData.settleAmount, actualFee);
+            require(txMetaData.settleAmount >= _params.order.makerAssetAmount, "AMMWrapper: insufficient maker output");
+            _settle(_params.order, internalTxData, txMetaData.settleAmount, actualFee);
         }
 
-        emitSwappedEvent(_order, txMetaData);
+        emitSwappedEvent(_params.order, txMetaData);
         return txMetaData.settleAmount;
     }
 

contracts/L2Deposit.sol

@@ -11,6 +11,7 @@ import "./interfaces/IL2Deposit.sol";
 import "./interfaces/IOptimismL1StandardBridge.sol";
 import "./interfaces/IPermanentStorage.sol";
 import "./interfaces/ISpender.sol";
+import "./utils/SpenderLibEIP712.sol";
 import "./utils/StrategyBase.sol";
 import "./utils/BaseLibEIP712.sol";
 import "./utils/Ownable.sol";
@@ -49,8 +50,19 @@ contract L2Deposit is IL2Deposit, StrategyBase, ReentrancyGuard, BaseLibEIP712,
         // PermanentStorage will revert when L2 deposit hash is already seen
         permStorage.setL2DepositSeen(depositHash);
 
-        // Transfer token from sender to this contract
-        spender.spendFromUser(_params.deposit.sender, _params.deposit.l1TokenAddr, _params.deposit.amount);
+        // Create spendWithPermit for spender contract to verify
+        SpenderLibEIP712.SpendWithPermit memory l1TokenPermit = SpenderLibEIP712.SpendWithPermit({
+            tokenAddr: _params.deposit.l1TokenAddr,
+            requester: address(this),
+            user: _params.deposit.sender,
+            recipient: address(this),
+            amount: _params.deposit.amount,
+            actionHash: depositHash,
+            expiry: uint64(_params.deposit.expiry)
+        });
+
+        // Verfiy signature then transfer token from sender to this contract
+        spender.spendFromUserToWithPermit(l1TokenPermit, _params.l1TokenPermitSig);
 
         // Bypass stack too deep
         DepositInfo memory depositInfo = DepositInfo(

contracts/RFQ.sol

@@ -2,6 +2,8 @@
 pragma solidity 0.7.6;
 pragma abicoder v2;
 
+import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol";
 import "@openzeppelin/contracts/math/SafeMath.sol";
 import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
 import "@openzeppelin/contracts/utils/Address.sol";
@@ -10,6 +12,7 @@ import "./interfaces/IRFQ.sol";
 import "./utils/StrategyBase.sol";
 import "./utils/RFQLibEIP712.sol";
 import "./utils/BaseLibEIP712.sol";
+import "./utils/SpenderLibEIP712.sol";
 import "./utils/SignatureValidator.sol";
 import "./utils/LibConstant.sol";
 
@@ -18,6 +21,7 @@ import "./utils/LibConstant.sol";
 contract RFQ is IRFQ, StrategyBase, ReentrancyGuard, SignatureValidator, BaseLibEIP712 {
     using SafeMath for uint256;
     using Address for address;
+    using SafeERC20 for IERC20;
 
     // Constants do not have storage slot.
     string public constant SOURCE = "RFQ v1";
@@ -98,7 +102,9 @@ contract RFQ is IRFQ, StrategyBase, ReentrancyGuard, SignatureValidator, BaseLib
     function fill(
         RFQLibEIP712.Order calldata _order,
         bytes calldata _mmSignature,
-        bytes calldata _userSignature
+        bytes calldata _userSignature,
+        bytes calldata _makerAssetPermitSig,
+        bytes calldata _takerAssetPermitSig
     ) external payable override nonReentrant onlyUserProxy returns (uint256) {
         // check the order deadline and fee factor
         require(_order.deadline >= block.timestamp, "RFQ: expired order");
@@ -115,7 +121,7 @@ contract RFQ is IRFQ, StrategyBase, ReentrancyGuard, SignatureValidator, BaseLib
         // Set transaction as seen, PermanentStorage would throw error if transaction already seen.
         permStorage.setRFQTransactionSeen(vars.transactionHash);
 
-        return _settle(_order, vars);
+        return _settle(_order, vars, _makerAssetPermitSig, _takerAssetPermitSig);
     }
 
     function _emitFillOrder(
@@ -140,15 +146,45 @@ contract RFQ is IRFQ, StrategyBase, ReentrancyGuard, SignatureValidator, BaseLib
     }
 
     // settle
-    function _settle(RFQLibEIP712.Order memory _order, GroupedVars memory _vars) internal returns (uint256) {
+    function _settle(
+        RFQLibEIP712.Order memory _order,
+        GroupedVars memory _vars,
+        bytes memory _makerAssetPermitSig,
+        bytes memory _takerAssetPermitSig
+    ) internal returns (uint256) {
+        // Declare the 'maker sends makerAsset to this contract' SpendWithPermit struct from _order parameter
+        SpenderLibEIP712.SpendWithPermit memory makerAssetPermit = SpenderLibEIP712.SpendWithPermit(
+            _order.makerAssetAddr,
+            address(this),
+            _order.makerAddr,
+            address(this),
+            _order.makerAssetAmount,
+            _vars.orderHash,
+            uint64(_order.deadline)
+        );
+
+        // Declare the 'taker sends takerAsset to this contract' SpendWithPermit struct from _order parameter
+        SpenderLibEIP712.SpendWithPermit memory takerAssetPermit = SpenderLibEIP712.SpendWithPermit(
+            _order.takerAssetAddr,
+            address(this),
+            _order.takerAddr,
+            address(this),
+            _order.takerAssetAmount,
+            _vars.transactionHash,
+            uint64(_order.deadline)
+        );
+
         // Transfer taker asset to maker
         if (address(weth) == _order.takerAssetAddr) {
             // Deposit to WETH if taker asset is ETH
             require(msg.value == _order.takerAssetAmount, "RFQ: insufficient ETH");
             weth.deposit{ value: msg.value }();
             weth.transfer(_order.makerAddr, _order.takerAssetAmount);
         } else {
-            spender.spendFromUserTo(_order.takerAddr, _order.takerAssetAddr, _order.makerAddr, _order.takerAssetAmount);
+            // Transfer taker asset to this contract first,
+            spender.spendFromUserToWithPermit(takerAssetPermit, _takerAssetPermitSig);
+            // then transfer from this to maker.
+            IERC20(_order.takerAssetAddr).safeTransfer(_order.makerAddr, _order.takerAssetAmount);
         }
 
         // Transfer maker asset to taker, sub fee
@@ -158,18 +194,20 @@ contract RFQ is IRFQ, StrategyBase, ReentrancyGuard, SignatureValidator, BaseLib
             settleAmount = settleAmount.sub(fee);
         }
 
-        // Transfer token/Eth to receiver
+        // Transfer maker asset to this contract first
+        spender.spendFromUserToWithPermit(makerAssetPermit, _makerAssetPermitSig);
+
+        // Transfer maker asset less fee from this contract to receiver
         if (_order.makerAssetAddr == address(weth)) {
-            // Transfer from maker
-            spender.spendFromUser(_order.makerAddr, _order.makerAssetAddr, settleAmount);
             weth.withdraw(settleAmount);
             payable(_order.receiverAddr).transfer(settleAmount);
         } else {
-            spender.spendFromUserTo(_order.makerAddr, _order.makerAssetAddr, _order.receiverAddr, settleAmount);
+            IERC20(_order.makerAssetAddr).safeTransfer(_order.receiverAddr, settleAmount);
         }
-        // Collect fee
+
+        // Transfer fee from this contract to feeCollector
         if (fee > 0) {
-            spender.spendFromUserTo(_order.makerAddr, _order.makerAssetAddr, feeCollector, fee);
+            IERC20(_order.makerAssetAddr).safeTransfer(feeCollector, fee);
         }
 
         _emitFillOrder(_order, _vars, settleAmount);