diff --git a/contracts/erc20-bridge-sampler/contracts/src/ERC20BridgeSampler.sol b/contracts/erc20-bridge-sampler/contracts/src/ERC20BridgeSampler.sol index b11658576f..e4392434b6 100644 --- 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 index 49c60dc408..85771c9198 100644 --- 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 new file mode 100644 index 0000000000..d2a0a5f796 --- /dev/null +++ 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 index ac4949dc68..a6e6eb1571 100644 --- 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 index 6bbff417ff..8720939a79 100644 --- 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 index 153229adb0..f417dd9ca0 100644 --- 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 index faacb5c6e3..e876840fdd 100644 --- 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 index 21f8312556..61172d7f32 100644 --- 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 index f2e3c070fe..d80e22036f 100644 --- 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 index 7591f6c50f..bf1288dd64 100644 --- 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 index 6f9bd0cd62..e891ae1564 100644 --- 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 index 342ca3f420..d7522835cc 100644 --- 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 index 953ea719d8..afc28244f1 100644 --- 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",