@0x/contracts-zero-ex: Fix TransformERC20Feature reverting when selling taker's entire balance when input token is ETH (#46)

Co-authored-by: Lawrence Forman <me@merklejerk.com>

contracts/zero-ex/CHANGELOG.json

@@ -33,6 +33,10 @@
             {
                 "note": "Require RFQ orders to specify a transaction origin, and allow approved alternative addresses",
                 "pr": 47
+            },
+            {
+                "note": "Do not try to pull all tokens if selling all ETH in `TransformERC20Feature`",
+                "pr": 46
             }
         ]
     },

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

@@ -214,13 +214,19 @@ contract TransformERC20Feature is
         private
         returns (uint256 outputTokenAmount)
     {
-        // If the input token amount is -1, transform the taker's entire
+        // If the input token amount is -1 and we are not selling ETH,
-        // spendable balance.
+        // transform the taker's entire spendable balance.
         if (args.inputTokenAmount == uint256(-1)) {
-            args.inputTokenAmount = _getSpendableERC20BalanceOf(
+            if (LibERC20Transformer.isTokenETH(args.inputToken)) {
-                args.inputToken,
+                // We can't pull more ETH from the taker, so we just set the
-                args.taker
+                // input token amount to the value attached to the call.
-            );
+                args.inputTokenAmount = msg.value;
+            } else {
+                args.inputTokenAmount = _getSpendableERC20BalanceOf(
+                    args.inputToken,
+                    args.taker
+                );
+            }
         }
 
         TransformERC20PrivateState memory state;

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

@@ -60,11 +60,17 @@ contract TestMintTokenERC20Transformer is
             context.sender,
             context.taker,
             context.data,
-            data.inputToken.balanceOf(address(this)),
+            LibERC20Transformer.isTokenETH(data.inputToken)
+                ? address(this).balance
+                : data.inputToken.balanceOf(address(this)),
             address(this).balance
         );
         // "Burn" input tokens.
-        data.inputToken.transfer(address(0), data.burnAmount);
+        if (LibERC20Transformer.isTokenETH(data.inputToken)) {
+            address(0).transfer(data.burnAmount);
+        } else {
+            data.inputToken.transfer(address(0), data.burnAmount);
+        }
         // Mint output tokens.
         if (LibERC20Transformer.isTokenETH(IERC20TokenV06(address(data.outputToken)))) {
             context.taker.transfer(data.mintAmount);

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

@@ -649,6 +649,82 @@ blockchainTests.resets('TransformERC20 feature', env => {
                 const { callDataHash: actualCallDataHash } = (receipt.logs[0] as MintTokenTransformerEvent).args;
                 expect(actualCallDataHash).to.eq(NULL_BYTES32);
             });
+
+            it('can sell entire taker balance', async () => {
+                const startingInputTokenBalance = getRandomInteger(0, '100e18');
+                await inputToken.mint(taker, startingInputTokenBalance).awaitTransactionSuccessAsync();
+                const minOutputTokenAmount = getRandomInteger(1, '1e18');
+                const outputTokenMintAmount = minOutputTokenAmount;
+                const callValue = getRandomInteger(1, '1e18');
+                const callDataHash = hexUtils.random();
+                const transformation = createMintTokenTransformation({
+                    outputTokenMintAmount,
+                    inputTokenBurnAmunt: startingInputTokenBalance,
+                });
+                const receipt = await feature
+                    ._transformERC20({
+                        taker,
+                        inputToken: inputToken.address,
+                        outputToken: outputToken.address,
+                        inputTokenAmount: MAX_UINT256,
+                        minOutputTokenAmount,
+                        transformations: [transformation],
+                        callDataHash,
+                        callDataSignature: NULL_BYTES,
+                    })
+                    .awaitTransactionSuccessAsync({ value: callValue });
+                verifyEventsFromLogs(
+                    receipt.logs,
+                    [
+                        {
+                            taker,
+                            inputTokenAmount: startingInputTokenBalance,
+                            outputTokenAmount: outputTokenMintAmount,
+                            inputToken: inputToken.address,
+                            outputToken: outputToken.address,
+                        },
+                    ],
+                    TransformERC20FeatureEvents.TransformedERC20,
+                );
+            });
+
+            it('can sell entire taker balance with ETH (but not really)', async () => {
+                const ethAttchedAmount = getRandomInteger(0, '100e18');
+                await inputToken.mint(taker, ethAttchedAmount).awaitTransactionSuccessAsync();
+                const minOutputTokenAmount = getRandomInteger(1, '1e18');
+                const outputTokenMintAmount = minOutputTokenAmount;
+                const callDataHash = hexUtils.random();
+                const transformation = createMintTokenTransformation({
+                    outputTokenMintAmount,
+                    inputTokenAddress: ETH_TOKEN_ADDRESS,
+                    inputTokenBurnAmunt: ethAttchedAmount,
+                });
+                const receipt = await feature
+                    ._transformERC20({
+                        taker,
+                        inputToken: ETH_TOKEN_ADDRESS,
+                        outputToken: outputToken.address,
+                        inputTokenAmount: MAX_UINT256,
+                        minOutputTokenAmount,
+                        transformations: [transformation],
+                        callDataHash,
+                        callDataSignature: NULL_BYTES,
+                    })
+                    .awaitTransactionSuccessAsync({ value: ethAttchedAmount });
+                verifyEventsFromLogs(
+                    receipt.logs,
+                    [
+                        {
+                            taker,
+                            inputTokenAmount: ethAttchedAmount,
+                            outputTokenAmount: outputTokenMintAmount,
+                            inputToken: ETH_TOKEN_ADDRESS,
+                            outputToken: outputToken.address,
+                        },
+                    ],
+                    TransformERC20FeatureEvents.TransformedERC20,
+                );
+            });
         });
 
         describe('transformERC20()', () => {