Fix Kyber and Uniswap ERC20Bridges (#2412)

* `@0x/contracts-asset-proxy`: Fix `UniswapBridge` token -> token transfer logic.
`@0x/contract-addresses`: Update `UniswapBridge` mainnet address.

* `@0x/asset-proxy`: Fix `KyberBridge` incorrect `minConversionRate` calculation.

* `@0x/contract-addresses`: Update `KyberBridge` mainnet address.

contracts/asset-proxy/CHANGELOG.json

@@ -5,6 +5,14 @@
             {
                 "note": "Integration tests for DydxBridge with ERC20BridgeProxy.",
                 "pr": 2401
+            },
+            {
+                "note": "Fix `UniswapBridge` token -> token transfer call.",
+                "pr": 2412
+            },
+            {
+                "note": "Fix `KyberBridge` incorrect `minConversionRate` calculation.",
+                "pr": 2412
             }
         ]
     },

contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol

@@ -24,6 +24,7 @@ import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol";
 import "@0x/contracts-erc20/contracts/src/LibERC20Token.sol";
 import "@0x/contracts-exchange-libs/contracts/src/IWallet.sol";
 import "@0x/contracts-utils/contracts/src/DeploymentConstants.sol";
+import "@0x/contracts-utils/contracts/src/LibSafeMath.sol";
 import "../interfaces/IERC20Bridge.sol";
 import "../interfaces/IKyberNetworkProxy.sol";
 
@@ -34,6 +35,8 @@ contract KyberBridge is
     IWallet,
     DeploymentConstants
 {
+    using LibSafeMath for uint256;
+
     // @dev Structure used internally to get around stack limits.
     struct TradeState {
         IKyberNetworkProxy kyber;
@@ -41,6 +44,7 @@ contract KyberBridge is
         address fromTokenAddress;
         uint256 fromTokenBalance;
         uint256 payableAmount;
+        uint256 conversionRate;
     }
 
     /// @dev Kyber ETH pseudo-address.
@@ -81,11 +85,23 @@ contract KyberBridge is
         state.weth = IEtherToken(_getWethAddress());
         // Decode the bridge data to get the `fromTokenAddress`.
         (state.fromTokenAddress) = abi.decode(bridgeData, (address));
+        // Query the balance of "from" tokens.
         state.fromTokenBalance = IERC20Token(state.fromTokenAddress).balanceOf(address(this));
         if (state.fromTokenBalance == 0) {
             // Return failure if no input tokens.
             return BRIDGE_FAILED;
         }
+        // Compute the conversion rate, expressed in 18 decimals.
+        // The sequential notation is to get around stack limits.
+        state.conversionRate = KYBER_RATE_BASE;
+        state.conversionRate = state.conversionRate.safeMul(amount);
+        state.conversionRate = state.conversionRate.safeMul(
+            10 ** uint256(LibERC20Token.decimals(state.fromTokenAddress))
+        );
+        state.conversionRate = state.conversionRate.safeDiv(state.fromTokenBalance);
+        state.conversionRate = state.conversionRate.safeDiv(
+            10 ** uint256(LibERC20Token.decimals(toTokenAddress))
+        );
         if (state.fromTokenAddress == toTokenAddress) {
             // Just transfer the tokens if they're the same.
             LibERC20Token.transfer(state.fromTokenAddress, to, state.fromTokenBalance);
@@ -118,7 +134,7 @@ contract KyberBridge is
             uint256(-1),
             // Compute the minimum conversion rate, which is expressed in units with
             // 18 decimal places.
-            (KYBER_RATE_BASE * amount) / state.fromTokenBalance,
+            state.conversionRate,
             // No affiliate address.
             address(0)
         );

contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol

@@ -134,8 +134,8 @@ contract UniswapBridge is
                 state.fromTokenBalance,
                 // Minimum buy amount.
                 amount,
-                // No minimum intermediate ETH buy amount.
+                // Must buy at least 1 intermediate ETH.
-                0,
+                1,
                 // Expires after this block.
                 block.timestamp,
                 // Recipient is `to`.

contracts/asset-proxy/contracts/test/TestKyberBridge.sol

@@ -67,9 +67,11 @@ interface ITestContract {
 /// @dev A minimalist ERC20/WETH token.
 contract TestToken {
 
+    uint8 public decimals;
     ITestContract private _testContract;
 
-    constructor() public {
+    constructor(uint8 decimals_) public {
+        decimals = decimals_;
         _testContract = ITestContract(msg.sender);
     }
 
@@ -165,7 +167,7 @@ contract TestKyberBridge is
     uint256 private _nextFillAmount;
 
     constructor() public {
-        weth = IEtherToken(address(new TestToken()));
+        weth = IEtherToken(address(new TestToken(18)));
     }
 
     /// @dev Implementation of `IKyberNetworkProxy.trade()`
@@ -195,11 +197,11 @@ contract TestKyberBridge is
         return _nextFillAmount;
     }
 
-    function createToken()
+    function createToken(uint8 decimals)
         external
         returns (address tokenAddress)
     {
-        return address(new TestToken());
+        return address(new TestToken(decimals));
     }
 
     function setNextFillAmount(uint256 amount)

contracts/asset-proxy/test/kyber_bridge.ts

@@ -3,6 +3,7 @@ import {
     constants,
     expect,
     getRandomInteger,
+    getRandomPortion,
     randomAddress,
     verifyEventsFromLogs,
 } from '@0x/contracts-test-utils';
@@ -17,6 +18,12 @@ import { TestKyberBridgeContract, TestKyberBridgeEvents } from './wrappers';
 
 blockchainTests.resets('KyberBridge unit tests', env => {
     const KYBER_ETH_ADDRESS = '0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee';
+    const FROM_TOKEN_DECIMALS = 6;
+    const TO_TOKEN_DECIMALS = 18;
+    const FROM_TOKEN_BASE = new BigNumber(10).pow(FROM_TOKEN_DECIMALS);
+    const TO_TOKEN_BASE = new BigNumber(10).pow(TO_TOKEN_DECIMALS);
+    const WETH_BASE = new BigNumber(10).pow(18);
+    const KYBER_RATE_BASE = WETH_BASE;
     let testContract: TestKyberBridgeContract;
 
     before(async () => {
@@ -45,10 +52,10 @@ blockchainTests.resets('KyberBridge unit tests', env => {
 
         before(async () => {
             wethAddress = await testContract.weth().callAsync();
-            fromTokenAddress = await testContract.createToken().callAsync();
+            fromTokenAddress = await testContract.createToken(FROM_TOKEN_DECIMALS).callAsync();
-            await testContract.createToken().awaitTransactionSuccessAsync();
+            await testContract.createToken(FROM_TOKEN_DECIMALS).awaitTransactionSuccessAsync();
-            toTokenAddress = await testContract.createToken().callAsync();
+            toTokenAddress = await testContract.createToken(TO_TOKEN_DECIMALS).callAsync();
-            await testContract.createToken().awaitTransactionSuccessAsync();
+            await testContract.createToken(TO_TOKEN_DECIMALS).awaitTransactionSuccessAsync();
         });
 
         const STATIC_KYBER_TRADE_ARGS = {
@@ -75,13 +82,14 @@ blockchainTests.resets('KyberBridge unit tests', env => {
         }
 
         function createTransferFromOpts(opts?: Partial<TransferFromOpts>): TransferFromOpts {
+            const amount = getRandomInteger(1, TO_TOKEN_BASE.times(100));
             return {
                 fromTokenAddress,
                 toTokenAddress,
+                amount,
                 toAddress: randomAddress(),
-                amount: getRandomInteger(1, 10e18),
+                fillAmount: getRandomPortion(amount),
-                fillAmount: getRandomInteger(1, 10e18),
+                fromTokenBalance: getRandomInteger(1, FROM_TOKEN_BASE.times(100)),
-                fromTokenBalance: getRandomInteger(1, 10e18),
                 ...opts,
             };
         }
@@ -119,9 +127,12 @@ blockchainTests.resets('KyberBridge unit tests', env => {
         }
 
         function getMinimumConversionRate(opts: TransferFromOpts): BigNumber {
+            const fromBase = opts.fromTokenAddress === wethAddress ? WETH_BASE : FROM_TOKEN_BASE;
+            const toBase = opts.toTokenAddress === wethAddress ? WETH_BASE : TO_TOKEN_BASE;
             return opts.amount
-                .times(constants.ONE_ETHER)
+                .div(toBase)
-                .div(opts.fromTokenBalance)
+                .div(opts.fromTokenBalance.div(fromBase))
+                .times(KYBER_RATE_BASE)
                 .integerValue(BigNumber.ROUND_DOWN);
         }
 

contracts/asset-proxy/test/uniswap_bridge.ts

@@ -176,7 +176,7 @@ blockchainTests.resets('UniswapBridge unit tests', env => {
                 expect(calls[0].exchange).to.eq(exchangeAddress);
                 expect(calls[0].tokensSold).to.bignumber.eq(opts.fromTokenBalance);
                 expect(calls[0].minTokensBought).to.bignumber.eq(opts.amount);
-                expect(calls[0].minEthBought).to.bignumber.eq(0);
+                expect(calls[0].minEthBought).to.bignumber.eq(1);
                 expect(calls[0].deadline).to.bignumber.eq(blockTime);
                 expect(calls[0].recipient).to.eq(opts.toAddress);
                 expect(calls[0].toTokenAddress).to.eq(opts.toTokenAddress);

packages/contract-addresses/CHANGELOG.json

@@ -13,6 +13,14 @@
             {
                 "note": "Added DydxBridge Contract to contract-addresses",
                 "pr": 2401
+            },
+            {
+                "note": "Update `UniswapBridge` mainnet address.",
+                "pr": 2412
+            },
+            {
+                "note": "Update `KyberBridge` mainnet address.",
+                "pr": 2412
             }
         ]
     },

packages/contract-addresses/addresses.json

@@ -21,10 +21,10 @@
         "stakingProxy": "0xa26e80e7dea86279c6d778d702cc413e6cffa777",
         "devUtils": "0xc7612135356ba8f75dbf517b55d88a91977492dc",
         "erc20BridgeProxy": "0x8ed95d1746bf1e4dab58d8ed4724f1ef95b20db0",
-        "uniswapBridge": "0xa6baaed2053058a3c8f11e0c7a9716304454b09e",
+        "uniswapBridge": "0xb0dc61047847732a013ce27341228228a38655a0",
         "eth2DaiBridge": "0x0ac2d6f5f5afc669d3ca38f830dad2b4f238ad3f",
         "erc20BridgeSampler": "0x1b402fdb5ee87f989c11e3963557e89cc313b6c0",
-        "kyberBridge": "0xe64660275c40c16c491c2dabf50afaded20f858f",
+        "kyberBridge": "0x7253a80c1d3a3175283bad9ed04b2cecad0fe0d3",
         "dydxBridge": "0x96ddba19b69d6ea2549f6a12d005595167414744"
     },
     "3": {