Add checkAllowance flag to LibTokenSpender.spendERC20Tokens (#39)

contracts/zero-ex/CHANGELOG.json

@@ -1,4 +1,17 @@
 [
+    {
+        "version": "0.10.0",
+        "changes": [
+            {
+                "note": "Add `checkAllowance` flag to LibTokenSpender.spendERC20Tokens",
+                "pr": 39
+            },
+            {
+                "note": "Use new `checkAllowance` flag in LiquidityProviderFeature, TransformERC20Feature, and MetaTransactionsFeature",
+                "pr": 39
+            }
+        ]
+    },
     {
         "version": "0.9.0",
         "changes": [

contracts/zero-ex/contracts/src/features/LiquidityProviderFeature.sol

@@ -43,7 +43,7 @@ contract LiquidityProviderFeature is
     /// @dev Name of this feature.
     string public constant override FEATURE_NAME = "LiquidityProviderFeature";
     /// @dev Version of this feature.
-    uint256 public immutable override FEATURE_VERSION = _encodeVersion(1, 0, 0);
+    uint256 public immutable override FEATURE_VERSION = _encodeVersion(1, 0, 1);
 
     /// @dev ETH pseudo-token address.
     address constant internal ETH_TOKEN_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
@@ -60,11 +60,11 @@ contract LiquidityProviderFeature is
         address recipient
     );
 
-    constructor(address zeroEx)
+    constructor(LiquidityProviderSandbox sandbox_)
         public
         FixinCommon()
     {
-        sandbox = new LiquidityProviderSandbox(zeroEx);
+        sandbox = sandbox_;
     }
 
     /// @dev Initialize and register this feature.
@@ -116,7 +116,8 @@ contract LiquidityProviderFeature is
                 IERC20TokenV06(inputToken),
                 msg.sender,
                 provider,
-                sellAmount
+                sellAmount,
+                true
             );
         }
 

contracts/zero-ex/contracts/src/features/MetaTransactionsFeature.sol

@@ -78,7 +78,7 @@ contract MetaTransactionsFeature is
     /// @dev Name of this feature.
     string public constant override FEATURE_NAME = "MetaTransactions";
     /// @dev Version of this feature.
-    uint256 public immutable override FEATURE_VERSION = _encodeVersion(1, 0, 0);
+    uint256 public immutable override FEATURE_VERSION = _encodeVersion(1, 1, 0);
     /// @dev EIP712 typehash of the `MetaTransactionData` struct.
     bytes32 public immutable MTX_EIP712_TYPEHASH = keccak256(
         "MetaTransactionData("
@@ -283,7 +283,8 @@ contract MetaTransactionsFeature is
                 mtx.feeToken,
                 mtx.signer,
                 sender,
-                mtx.feeAmount
+                mtx.feeAmount,
+                true
             );
         }
 

contracts/zero-ex/contracts/src/features/NativeOrdersFeature.sol

@@ -815,7 +815,8 @@ contract NativeOrdersFeature is
                 params.order.takerToken,
                 params.taker,
                 params.order.feeRecipient,
-                uint256(results.takerTokenFeeFilledAmount)
+                uint256(results.takerTokenFeeFilledAmount),
+                false
             );
         }
 
@@ -951,7 +952,8 @@ contract NativeOrdersFeature is
             settleInfo.takerToken,
             settleInfo.taker,
             settleInfo.maker,
-            takerTokenFilledAmount
+            takerTokenFilledAmount,
+            false
         );
 
         // Transfer maker -> taker.
@@ -959,7 +961,8 @@ contract NativeOrdersFeature is
             settleInfo.makerToken,
             settleInfo.maker,
             settleInfo.taker,
-            makerTokenFilledAmount
+            makerTokenFilledAmount,
+            false
         );
     }
 

contracts/zero-ex/contracts/src/features/TransformERC20Feature.sol

@@ -58,7 +58,7 @@ contract TransformERC20Feature is
     /// @dev Name of this feature.
     string public constant override FEATURE_NAME = "TransformERC20";
     /// @dev Version of this feature.
-    uint256 public immutable override FEATURE_VERSION = _encodeVersion(1, 3, 0);
+    uint256 public immutable override FEATURE_VERSION = _encodeVersion(1, 3, 1);
 
     /// @dev Initialize and register this feature.
     ///      Should be delegatecalled by `Migrate.migrate()`.
@@ -317,7 +317,13 @@ contract TransformERC20Feature is
         // Transfer input tokens.
         if (!LibERC20Transformer.isTokenETH(inputToken)) {
             // Token is not ETH, so pull ERC20 tokens.
-            LibTokenSpender.spendERC20Tokens(inputToken, from, to, amount);
+            LibTokenSpender.spendERC20Tokens(
+                inputToken,
+                from,
+                to,
+                amount,
+                true
+            );
         } else if (msg.value < amount) {
              // Token is ETH, so the caller must attach enough ETH to the call.
             LibTransformERC20RichErrors.InsufficientEthAttachedError(

contracts/zero-ex/contracts/src/features/libs/LibTokenSpender.sol

@@ -35,11 +35,14 @@ library LibTokenSpender {
     /// @param owner The owner of the tokens.
     /// @param to The recipient of the tokens.
     /// @param amount The amount of `token` to transfer.
+    /// @param checkAllowance Whether or not to check the owner's allowance
+    ///        prior to attempting the transfer.
     function spendERC20Tokens(
         IERC20TokenV06 token,
         address owner,
         address to,
-        uint256 amount
+        uint256 amount,
+        bool checkAllowance
     )
         internal
     {
@@ -48,6 +51,19 @@ library LibTokenSpender {
 
         require(address(token) != address(this), "LibTokenSpender/CANNOT_INVOKE_SELF");
 
+        if (checkAllowance) {
+            // If the owner doesn't have a sufficient allowance set on `address(this)`,
+            // try the old AllowanceTarget.
+            if (token.allowance(owner, address(this)) < amount) {
+                return ITokenSpenderFeature(address(this))._spendERC20Tokens(
+                    token,
+                    owner,
+                    to,
+                    amount
+                );
+            }
+        }
+
         assembly {
             let ptr := mload(0x40) // free memory pointer
 

contracts/zero-ex/contracts/test/TestLibTokenSpender.sol

@@ -34,7 +34,13 @@ contract TestLibTokenSpender {
     )
         external
     {
-        LibTokenSpender.spendERC20Tokens(token, owner, to, amount);
+        LibTokenSpender.spendERC20Tokens(
+            token,
+            owner,
+            to,
+            amount,
+            false
+        );
     }
 
     event FallbackCalled(

contracts/zero-ex/test/features/liquidity_provider_test.ts

@@ -51,17 +51,19 @@ blockchainTests('LiquidityProvider feature', env => {
             .awaitTransactionSuccessAsync({ from: taker });
 
         feature = new LiquidityProviderFeatureContract(zeroEx.address, env.provider, env.txDefaults, abis);
-        const featureImpl = await LiquidityProviderFeatureContract.deployFrom0xArtifactAsync(
+        sandbox = await LiquidityProviderSandboxContract.deployFrom0xArtifactAsync(
-            artifacts.LiquidityProviderFeature,
+            artifacts.LiquidityProviderSandbox,
             env.provider,
             env.txDefaults,
             artifacts,
             zeroEx.address,
         );
-        sandbox = new LiquidityProviderSandboxContract(
+        const featureImpl = await LiquidityProviderFeatureContract.deployFrom0xArtifactAsync(
-            await featureImpl.sandbox().callAsync(),
+            artifacts.LiquidityProviderFeature,
             env.provider,
             env.txDefaults,
+            artifacts,
+            sandbox.address,
         );
         await new IOwnableFeatureContract(zeroEx.address, env.provider, env.txDefaults, abis)
             .migrate(featureImpl.address, featureImpl.migrate().getABIEncodedTransactionData(), owner)