Merge pull request #84 from argentlabs/feature/flash-approve-and-call

Enhance approveAndCall logic

contracts/modules/ApprovedTransfer.sol

@@ -18,7 +18,6 @@ import "../wallet/BaseWallet.sol";
 import "./common/BaseModule.sol";
 import "./common/RelayerModuleV2.sol";
 import "./common/BaseTransfer.sol";
-import "../../lib/utils/SafeMath.sol";
 
 /**
  * @title ApprovedTransfer
@@ -100,8 +99,7 @@ contract ApprovedTransfer is BaseModule, RelayerModuleV2, BaseTransfer {
         onlyWhenUnlocked(_wallet)
     {
         require(!_wallet.authorised(_contract) && _contract != address(_wallet), "AT: Forbidden contract");
-        doApproveToken(_wallet, _token, _spender, _amount);
-        doCallContract(_wallet, _contract, 0, _data);
+        doApproveTokenAndCallContract(_wallet, _token, _spender, _amount, _contract, _data);
     }
 
     // *************** Implementation of RelayerModule methods ********************* //

contracts/modules/CompoundManager.sol

@@ -15,7 +15,6 @@
 
 pragma solidity ^0.5.4;
 
-import "../../lib/utils/SafeMath.sol";
 import "../wallet/BaseWallet.sol";
 import "./common/BaseModule.sol";
 import "./common/RelayerModule.sol";

contracts/modules/RecoveryManager.sol

@@ -18,7 +18,6 @@ import "../wallet/BaseWallet.sol";
 import "./common/BaseModule.sol";
 import "./common/RelayerModuleV2.sol";
 import "./storage/GuardianStorage.sol";
-import "../../lib/utils/SafeMath.sol";
 
 /**
  * @title RecoveryManager

contracts/modules/TokenExchanger.sol

@@ -18,7 +18,6 @@ import "../wallet/BaseWallet.sol";
 import "./common/BaseModule.sol";
 import "./common/RelayerModule.sol";
 import "./common/OnlyOwnerModule.sol";
-import "../../lib/utils/SafeMath.sol";
 import "../../lib/other/ERC20.sol";
 import "../../lib/other/KyberNetwork.sol";
 

contracts/modules/TransferManager.sol

@@ -240,15 +240,17 @@ contract TransferManager is BaseModule, RelayerModule, OnlyOwnerModule, BaseTran
     * We assume that the contract will pull the tokens and does not require ETH.
     * @param _wallet The target wallet.
     * @param _token The token to approve.
-    * @param _contract The address of the contract.
+    * @param _spender The address to approve.
     * @param _amount The amount of ERC20 tokens to approve.
+    * @param _contract The address of the contract.
     * @param _data The encoded method data
     */
     function approveTokenAndCallContract(
         BaseWallet _wallet,
         address _token,
-        address _contract,
+        address _spender,
         uint256 _amount,
+        address _contract,
         bytes calldata _data
     )
         external
@@ -258,25 +260,14 @@ contract TransferManager is BaseModule, RelayerModule, OnlyOwnerModule, BaseTran
         // Make sure we don't call a module, the wallet itself, or a supported ERC20
         authoriseContractCall(_wallet, _contract);
 
-        if (isWhitelisted(_wallet, _contract)) {
-            doApproveToken(_wallet, _token, _contract, _amount);
-            doCallContract(_wallet, _contract, 0, _data);
-        } else {
-            // get current alowance
-            uint256 currentAllowance = ERC20(_token).allowance(address(_wallet), _contract);
-            if (_amount <= currentAllowance) {
-                // no need to approve more
-                doCallContract(_wallet, _contract, 0, _data);
-            } else {
-                // check if delta is under the limit
-                uint delta = _amount - currentAllowance;
-                uint256 deltaInEth = priceProvider.getEtherValue(delta, _token);
-                require(checkAndUpdateDailySpent(_wallet, deltaInEth), "TM: Approve above daily limit");
-                // approve if under the limit
-                doApproveToken(_wallet, _token, _contract, _amount);
-                doCallContract(_wallet, _contract, 0, _data);
-            }
+        if (!isWhitelisted(_wallet, _contract)) {
+            // check if the amount is under the daily limit
+            // check the entire amount because the currently approved amount will be restored and should still count towards the daily limit
+            uint256 valueInEth = priceProvider.getEtherValue(_amount, _token);
+            require(checkAndUpdateDailySpent(_wallet, valueInEth), "TM: Approve above daily limit");
         }
+
+        doApproveTokenAndCallContract(_wallet, _token, _spender, _amount, _contract, _data);
     }
 
     /**

contracts/modules/common/BaseModule.sol

@@ -19,6 +19,7 @@ import "../../infrastructure/ModuleRegistry.sol";
 import "../storage/GuardianStorage.sol";
 import "./Module.sol";
 import "../../../lib/other/ERC20.sol";
+import "../../../lib/utils/SafeMath.sol";
 
 /**
  * @title BaseModule

contracts/modules/common/BaseTransfer.sol

@@ -32,7 +32,15 @@ contract BaseTransfer is BaseModule {
     event Transfer(address indexed wallet, address indexed token, uint256 indexed amount, address to, bytes data);
     event Approved(address indexed wallet, address indexed token, uint256 amount, address spender);
     event CalledContract(address indexed wallet, address indexed to, uint256 amount, bytes data);
-
+    event ApprovedAndCalledContract(
+        address indexed wallet,
+        address indexed to,
+        address spender,
+        address indexed token,
+        uint256 amountApproved,
+        uint256 amountSpent,
+        bytes data
+    );
     // *************** Internal Functions ********************* //
 
     /**
@@ -77,4 +85,55 @@ contract BaseTransfer is BaseModule {
         invokeWallet(address(_wallet), _contract, _value, _data);
         emit CalledContract(address(_wallet), _contract, _value, _data);
     }
+
+    /**
+    * @dev Helper method to approve a certain amount of token and call an external contract.
+    * The address that spends the _token and the address that is called with _data can be different.
+    * @param _wallet The target wallet.
+    * @param _token The ERC20 address.
+    * @param _spender The spender address.
+    * @param _amount The amount of tokens to transfer.
+    * @param _contract The contract address.
+    * @param _data The method data.
+    */
+    function doApproveTokenAndCallContract(
+        BaseWallet _wallet,
+        address _token,
+        address _spender,
+        uint256 _amount,
+        address _contract,
+        bytes memory _data
+    )
+        internal
+    {
+        uint256 existingAllowance = ERC20(_token).allowance(address(_wallet), _spender);
+        uint256 totalAllowance = SafeMath.add(existingAllowance, _amount);
+        // Approve the desired amount plus existing amount. This logic allows for potential gas saving later
+        // when restoring the original approved amount, in cases where the _spender uses the exact approved _amount.
+        bytes memory methodData = abi.encodeWithSignature("approve(address,uint256)", _spender, totalAllowance);
+
+        invokeWallet(address(_wallet), _token, 0, methodData);
+        invokeWallet(address(_wallet), _contract, 0, _data);
+
+        // Calculate the approved amount that was spent after the call
+        uint256 unusedAllowance = ERC20(_token).allowance(address(_wallet), _spender);
+        uint256 usedAllowance = SafeMath.sub(totalAllowance, unusedAllowance);
+        // Ensure the amount spent does not exceed the amount approved for this call
+        require(usedAllowance <= _amount, "BT: insufficient amount for call");
+
+        if (unusedAllowance != existingAllowance) {
+            // Restore the original allowance amount if the amount spent was different (can be lower).
+            methodData = abi.encodeWithSignature("approve(address,uint256)", _spender, existingAllowance);
+            invokeWallet(address(_wallet), _token, 0, methodData);
+        }
+
+        emit ApprovedAndCalledContract(
+            address(_wallet),
+            _contract,
+            _spender,
+            _token,
+            _amount,
+            usedAllowance,
+            _data);
+    }
 }

contracts/modules/common/LimitManager.sol

@@ -15,7 +15,6 @@
 
 pragma solidity ^0.5.4;
 import "../../wallet/BaseWallet.sol";
-import "../../../lib/utils/SafeMath.sol";
 import "./BaseModule.sol";
 
 /**

contracts/modules/maker/MakerManager.sol

@@ -15,7 +15,6 @@
 
 pragma solidity ^0.5.4;
 
-import "../../../lib/utils/SafeMath.sol";
 import "../../wallet/BaseWallet.sol";
 import "../common/BaseModule.sol";
 import "../common/RelayerModule.sol";