feat: ERC20BridgeSampler Unlock Kyber collisions

0xProject · May 3, 2020 · 7e39e71 · 7e39e71
1 parent 49e4ade
commit 7e39e71
Show file tree

Hide file tree

Showing 13 changed files with 140 additions and 61 deletions.
diff --git a/contracts/erc20-bridge-sampler/contracts/src/ERC20BridgeSampler.sol b/contracts/erc20-bridge-sampler/contracts/src/ERC20BridgeSampler.sol
@@ -29,6 +29,7 @@ import "./IDevUtils.sol";
 import "./IERC20BridgeSampler.sol";
 import "./IEth2Dai.sol";
 import "./IKyberNetwork.sol";
+import "./IKyberNetworkContract.sol";
 import "./IUniswapExchangeQuotes.sol";
 import "./ICurve.sol";
 import "./ILiquidityProvider.sol";
@@ -183,33 +184,18 @@ contract ERC20BridgeSampler is
         returns (uint256[] memory makerTokenAmounts)
     {
         _assertValidPair(makerToken, takerToken);
-        address _takerToken = takerToken == _getWethAddress() ? KYBER_ETH_ADDRESS : takerToken;
-        address _makerToken = makerToken == _getWethAddress() ? KYBER_ETH_ADDRESS : makerToken;
-        uint256 takerTokenDecimals = _getTokenDecimals(takerToken);
-        uint256 makerTokenDecimals = _getTokenDecimals(makerToken);
         uint256 numSamples = takerTokenAmounts.length;
         makerTokenAmounts = new uint256[](numSamples);
+        address wethAddress = _getWethAddress();
         for (uint256 i = 0; i < numSamples; i++) {
-            (bool didSucceed, bytes memory resultData) =
-                _getKyberNetworkProxyAddress().staticcall.gas(KYBER_CALL_GAS)(
-                    abi.encodeWithSelector(
-                        IKyberNetwork(0).getExpectedRate.selector,
-                        _takerToken,
-                        _makerToken,
-                        takerTokenAmounts[i]
-                    ));
-            uint256 rate = 0;
-            if (didSucceed) {
-                rate = abi.decode(resultData, (uint256));
+            if (takerToken == wethAddress || makerToken == wethAddress) {
+                makerTokenAmounts[i] = _sampleSellFromKyberNetwork(takerToken, makerToken, takerTokenAmounts[i]);
             } else {
-                break;
+                uint256 value = _sampleSellFromKyberNetwork(takerToken, wethAddress, takerTokenAmounts[i]);
+                if (value != 0) {
+                    makerTokenAmounts[i] = _sampleSellFromKyberNetwork(wethAddress, makerToken, value);
+                }
             }
-            makerTokenAmounts[i] =
-                rate *
-                takerTokenAmounts[i] *
-                10 ** makerTokenDecimals /
-                10 ** takerTokenDecimals /
-                10 ** 18;
         }
     }
 
@@ -807,4 +793,53 @@ contract ERC20BridgeSampler is
             );
         }
     }
+
+    function _sampleSellFromKyberNetwork(
+        address takerToken,
+        address makerToken,
+        uint256 takerTokenAmount
+    )
+        private
+        view
+        returns (uint256 makerTokenAmount)
+    {
+        address _takerToken = takerToken == _getWethAddress() ? KYBER_ETH_ADDRESS : takerToken;
+        address _makerToken = makerToken == _getWethAddress() ? KYBER_ETH_ADDRESS : makerToken;
+        uint256 takerTokenDecimals = _getTokenDecimals(takerToken);
+        uint256 makerTokenDecimals = _getTokenDecimals(makerToken);
+        (bool didSucceed, bytes memory resultData) = _getKyberNetworkProxyAddress().staticcall.gas(DEFAULT_CALL_GAS)(
+            abi.encodeWithSelector(
+                IKyberNetwork(0).kyberNetworkContract.selector
+            ));
+        if (!didSucceed) {
+            return makerTokenAmount;
+        }
+        address kyberNetworkContract = abi.decode(resultData, (address));
+        (didSucceed, resultData) =
+            kyberNetworkContract.staticcall.gas(KYBER_CALL_GAS)(
+                abi.encodeWithSelector(
+                    IKyberNetworkContract(0).searchBestRate.selector,
+                    _takerToken,
+                    _makerToken,
+                    takerTokenAmount,
+                    false // usePermissionless
+                ));
+        uint256 rate = 0;
+        address reserve;
+        if (didSucceed) {
+            (reserve, rate) = abi.decode(resultData, (address, uint256));
+        } else {
+            return makerTokenAmount;
+        }
+        if (reserve != 0x31E085Afd48a1d6e51Cc193153d625e8f0514C7F || // Uniswap
+            reserve != 0x1E158c0e93c30d24e918Ef83d1e0bE23595C3c0f)   // Eth2Dai
+        {
+            makerTokenAmount =
+                rate *
+                takerTokenAmount *
+                10 ** makerTokenDecimals /
+                10 ** takerTokenDecimals /
+                10 ** 18;
+        }
+    }
 }
diff --git a/contracts/erc20-bridge-sampler/contracts/src/IKyberNetwork.sol b/contracts/erc20-bridge-sampler/contracts/src/IKyberNetwork.sol
@@ -21,6 +21,8 @@ pragma solidity ^0.5.9;
 
 interface IKyberNetwork {
 
+    function kyberNetworkContract() external view returns (address);
+
     function getExpectedRate(
         address fromToken,
         address toToken,

diff --git a/contracts/erc20-bridge-sampler/contracts/src/IKyberNetworkContract.sol b/contracts/erc20-bridge-sampler/contracts/src/IKyberNetworkContract.sol
@@ -0,0 +1,33 @@
+/*
+
+  Copyright 2019 ZeroEx Intl.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+*/
+
+pragma solidity ^0.5.9;
+
+
+interface IKyberNetworkContract {
+
+    function searchBestRate(
+        address fromToken,
+        address toToken,
+        uint256 fromAmount,
+        bool usePermissionless
+    )
+        external
+        view
+        returns (address reserve, uint256 expectedRate);
+}
diff --git a/contracts/erc20-bridge-sampler/contracts/test/TestERC20BridgeSampler.sol b/contracts/erc20-bridge-sampler/contracts/test/TestERC20BridgeSampler.sol
@@ -202,6 +202,29 @@ contract TestERC20BridgeSamplerKyberNetwork is
     bytes32 constant private SALT = 0x0ff3ca9d46195c39f9a12afb74207b4970349fb3cfb1e459bbf170298d326bc7;
     address constant public ETH_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
 
+    function kyberNetworkContract()
+        external
+        view
+        returns (address)
+    {
+        return address(this);
+    }
+
+    // IKyberNetworkContract not exposed via IKyberNetwork
+    function searchBestRate(
+        address fromToken,
+        address toToken,
+        uint256 fromAmount,
+        bool  // usePermissionless
+    )
+        external
+        view
+        returns (address reserve, uint256 expectedRate)
+    {
+        (expectedRate, ) = this.getExpectedRate(fromToken, toToken, fromAmount);
+        return (address(this), expectedRate);
+    }
+
     // Deterministic `IKyberNetwork.getExpectedRate()`.
     function getExpectedRate(
         address fromToken,

diff --git a/contracts/erc20-bridge-sampler/package.json b/contracts/erc20-bridge-sampler/package.json
@@ -38,7 +38,7 @@
     "config": {
         "publicInterfaceContracts": "ERC20BridgeSampler,IERC20BridgeSampler,ILiquidityProvider,ILiquidityProviderRegistry,DummyLiquidityProviderRegistry,DummyLiquidityProvider",
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.",
-        "abis": "./test/generated-artifacts/@(DummyLiquidityProvider|DummyLiquidityProviderRegistry|ERC20BridgeSampler|ICurve|IDevUtils|IERC20BridgeSampler|IEth2Dai|IKyberNetwork|ILiquidityProvider|ILiquidityProviderRegistry|IUniswapExchangeQuotes|TestERC20BridgeSampler).json"
+        "abis": "./test/generated-artifacts/@(DummyLiquidityProvider|DummyLiquidityProviderRegistry|ERC20BridgeSampler|ICurve|IDevUtils|IERC20BridgeSampler|IEth2Dai|IKyberNetwork|IKyberNetworkContract|ILiquidityProvider|ILiquidityProviderRegistry|IUniswapExchangeQuotes|TestERC20BridgeSampler).json"
     },
     "repository": {
         "type": "git",

diff --git a/contracts/erc20-bridge-sampler/test/artifacts.ts b/contracts/erc20-bridge-sampler/test/artifacts.ts
@@ -13,6 +13,7 @@ import * as IDevUtils from '../test/generated-artifacts/IDevUtils.json';
 import * as IERC20BridgeSampler from '../test/generated-artifacts/IERC20BridgeSampler.json';
 import * as IEth2Dai from '../test/generated-artifacts/IEth2Dai.json';
 import * as IKyberNetwork from '../test/generated-artifacts/IKyberNetwork.json';
+import * as IKyberNetworkContract from '../test/generated-artifacts/IKyberNetworkContract.json';
 import * as ILiquidityProvider from '../test/generated-artifacts/ILiquidityProvider.json';
 import * as ILiquidityProviderRegistry from '../test/generated-artifacts/ILiquidityProviderRegistry.json';
 import * as IUniswapExchangeQuotes from '../test/generated-artifacts/IUniswapExchangeQuotes.json';
@@ -26,6 +27,7 @@ export const artifacts = {
     IERC20BridgeSampler: IERC20BridgeSampler as ContractArtifact,
     IEth2Dai: IEth2Dai as ContractArtifact,
     IKyberNetwork: IKyberNetwork as ContractArtifact,
+    IKyberNetworkContract: IKyberNetworkContract as ContractArtifact,
     ILiquidityProvider: ILiquidityProvider as ContractArtifact,
     ILiquidityProviderRegistry: ILiquidityProviderRegistry as ContractArtifact,
     IUniswapExchangeQuotes: IUniswapExchangeQuotes as ContractArtifact,

diff --git a/contracts/erc20-bridge-sampler/test/erc20-bridge-sampler.ts b/contracts/erc20-bridge-sampler/test/erc20-bridge-sampler.ts
@@ -343,7 +343,8 @@ blockchainTests('erc20-bridge-sampler', env => {
 
         it('can quote token -> token', async () => {
             const sampleAmounts = getSampleAmounts(TAKER_TOKEN);
-            const [expectedQuotes] = getDeterministicSellQuotes(TAKER_TOKEN, MAKER_TOKEN, ['Kyber'], sampleAmounts);
+            const [takerToEthQuotes] = getDeterministicSellQuotes(TAKER_TOKEN, WETH_ADDRESS, ['Kyber'], sampleAmounts);
+            const [expectedQuotes] = getDeterministicSellQuotes(WETH_ADDRESS, MAKER_TOKEN, ['Kyber'], takerToEthQuotes);
             const quotes = await testContract
                 .sampleSellsFromKyberNetwork(TAKER_TOKEN, MAKER_TOKEN, sampleAmounts)
                 .callAsync();

diff --git a/contracts/erc20-bridge-sampler/test/wrappers.ts b/contracts/erc20-bridge-sampler/test/wrappers.ts
@@ -11,6 +11,7 @@ export * from '../test/generated-wrappers/i_dev_utils';
 export * from '../test/generated-wrappers/i_erc20_bridge_sampler';
 export * from '../test/generated-wrappers/i_eth2_dai';
 export * from '../test/generated-wrappers/i_kyber_network';
+export * from '../test/generated-wrappers/i_kyber_network_contract';
 export * from '../test/generated-wrappers/i_liquidity_provider';
 export * from '../test/generated-wrappers/i_liquidity_provider_registry';
 export * from '../test/generated-wrappers/i_uniswap_exchange_quotes';

diff --git a/contracts/erc20-bridge-sampler/tsconfig.json b/contracts/erc20-bridge-sampler/tsconfig.json
@@ -17,6 +17,7 @@
         "test/generated-artifacts/IERC20BridgeSampler.json",
         "test/generated-artifacts/IEth2Dai.json",
         "test/generated-artifacts/IKyberNetwork.json",
+        "test/generated-artifacts/IKyberNetworkContract.json",
         "test/generated-artifacts/ILiquidityProvider.json",
         "test/generated-artifacts/ILiquidityProviderRegistry.json",
         "test/generated-artifacts/IUniswapExchangeQuotes.json",

diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts
@@ -119,18 +119,15 @@ function dexQuotesToPaths(
     fees: { [source: string]: BigNumber },
 ): Fill[][] {
     const paths: Fill[][] = [];
-    for (const quote of dexQuotes) {
+    for (let quote of dexQuotes) {
         const path: Fill[] = [];
+        // Drop any non-zero entries. This can occur if the
+        // first few fills on Kyber were UniswapReserves
+        quote = quote.filter(q => !q.output.isZero());
         for (let i = 0; i < quote.length; i++) {
             const sample = quote[i];
             const prevSample = i === 0 ? undefined : quote[i - 1];
             const source = sample.source;
-            // Stop if the sample has zero output, which can occur if the source
-            // cannot fill the full amount.
-            // TODO(dorothy-zbornak): Sometimes Kyber will dip to zero then pick back up.
-            if (sample.output.eq(0)) {
-                break;
-            }
             const input = sample.input.minus(prevSample ? prevSample.input : 0);
             const output = sample.output.minus(prevSample ? prevSample.output : 0);
             const penalty =
@@ -206,7 +203,6 @@ export function getPathAdjustedSize(path: Fill[], targetInput: BigNumber = POSIT
 }
 
 export function isValidPath(path: Fill[], skipDuplicateCheck: boolean = false): boolean {
-    let flags = 0;
     for (let i = 0; i < path.length; ++i) {
         // Fill must immediately follow its parent.
         if (path[i].parent) {
@@ -222,10 +218,8 @@ export function isValidPath(path: Fill[], skipDuplicateCheck: boolean = false):
                 }
             }
         }
-        flags |= path[i].flags;
     }
-    const conflictFlags = FillFlags.Kyber | FillFlags.ConflictsWithKyber;
-    return (flags & conflictFlags) !== conflictFlags;
+    return true;
 }
 
 export function clipPathToInput(path: Fill[], targetInput: BigNumber = POSITIVE_INF): Fill[] {

diff --git a/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts b/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts
@@ -4,7 +4,7 @@ import { MarketOperation } from '../../types';
 
 import { ZERO_AMOUNT } from './constants';
 import { getPathSize, isValidPath } from './fills';
-import { Fill } from './types';
+import { Fill, FillFlags } from './types';
 
 // tslint:disable: prefer-for-of custom-no-magic-numbers completed-docs
 
@@ -24,6 +24,16 @@ export function findOptimalPath(
     for (const path of paths.slice(1)) {
         optimalPath = mixPaths(side, optimalPath, path, targetInput, runLimit);
     }
+    // Sort the path so that sources which conflict with Kyber are filled first
+    optimalPath.sort((a: Fill, b: Fill) => {
+        if (a.flags === FillFlags.ConflictsWithKyber && b.flags === FillFlags.Kyber) {
+            return -1;
+        }
+        if (b.flags === FillFlags.ConflictsWithKyber && a.flags === FillFlags.Kyber) {
+            return 1;
+        }
+        return 0;
+    });
     return isPathComplete(optimalPath, targetInput) ? optimalPath : undefined;
 }
 

diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts
@@ -466,29 +466,6 @@ describe('MarketOperationUtils tests', () => {
                 expect(orderSources.sort()).to.deep.eq(expectedSources.sort());
             });
 
-            it('Kyber is exclusive against Uniswap and Eth2Dai', async () => {
-                const rates: RatesBySource = {};
-                rates[ERC20BridgeSource.Native] = [0.3, 0.2, 0.1, 0.05];
-                rates[ERC20BridgeSource.Uniswap] = [0.5, 0.05, 0.05, 0.05];
-                rates[ERC20BridgeSource.Eth2Dai] = [0.6, 0.05, 0.05, 0.05];
-                rates[ERC20BridgeSource.Kyber] = [0.4, 0.05, 0.05, 0.05];
-                replaceSamplerOps({
-                    getSellQuotes: createGetMultipleSellQuotesOperationFromRates(rates),
-                });
-                const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync(
-                    createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]),
-                    FILL_AMOUNT,
-                    { ...DEFAULT_OPTS, numSamples: 4 },
-                );
-                const orderSources = improvedOrders.map(o => o.fills[0].source);
-                if (orderSources.includes(ERC20BridgeSource.Kyber)) {
-                    expect(orderSources).to.not.include(ERC20BridgeSource.Uniswap);
-                    expect(orderSources).to.not.include(ERC20BridgeSource.Eth2Dai);
-                } else {
-                    expect(orderSources).to.not.include(ERC20BridgeSource.Kyber);
-                }
-            });
-
             const ETH_TO_MAKER_RATE = 1.5;
 
             it('factors in fees for native orders', async () => {
@@ -605,7 +582,7 @@ describe('MarketOperationUtils tests', () => {
                     ERC20BridgeSource.Native,
                     ERC20BridgeSource.Uniswap,
                 ];
-                const secondSources = [ERC20BridgeSource.Eth2Dai];
+                const secondSources = [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Kyber];
                 expect(orderSources.slice(0, firstSources.length).sort()).to.deep.eq(firstSources.sort());
                 expect(orderSources.slice(firstSources.length).sort()).to.deep.eq(secondSources.sort());
             });

diff --git a/packages/contract-addresses/addresses.json b/packages/contract-addresses/addresses.json
@@ -20,7 +20,7 @@
         "devUtils": "0x74134cf88b21383713e096a5ecf59e297dc7f547",
         "erc20BridgeProxy": "0x8ed95d1746bf1e4dab58d8ed4724f1ef95b20db0",
         "uniswapBridge": "0x36691c4f426eb8f42f150ebde43069a31cb080ad",
-        "erc20BridgeSampler": "0xabee9a41f928c3b3b799b239a0f524343c7260c5",
+        "erc20BridgeSampler": "0x9e4fdb09dcd4fa2b5cc62da9faf3577f51b4d43e",
         "kyberBridge": "0x1c29670f7a77f1052d30813a0a4f632c78a02610",
         "eth2DaiBridge": "0x991c745401d5b5e469b8c3e2cb02c748f08754f1",
         "chaiBridge": "0x77c31eba23043b9a72d13470f3a3a311344d7438",