diff --git a/contracts/asset-proxy/CHANGELOG.json b/contracts/asset-proxy/CHANGELOG.json index f16a870e82..d6c173cf28 100644 --- a/contracts/asset-proxy/CHANGELOG.json +++ b/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 } ] }, diff --git a/contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol b/contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol index f1b6d8263c..2a98663897 100644 --- a/contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol +++ b/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) ); diff --git a/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol b/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol index 872d858333..3d0d596273 100644 --- a/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol +++ b/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. - 0, + // Must buy at least 1 intermediate ETH. + 1, // Expires after this block. block.timestamp, // Recipient is `to`. diff --git a/contracts/asset-proxy/contracts/test/TestKyberBridge.sol b/contracts/asset-proxy/contracts/test/TestKyberBridge.sol index 9a54bc16b8..7c6e28f0e8 100644 --- a/contracts/asset-proxy/contracts/test/TestKyberBridge.sol +++ b/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) diff --git a/contracts/asset-proxy/test/kyber_bridge.ts b/contracts/asset-proxy/test/kyber_bridge.ts index 287ffb1efc..d6fe644327 100644 --- a/contracts/asset-proxy/test/kyber_bridge.ts +++ b/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(); - await testContract.createToken().awaitTransactionSuccessAsync(); - toTokenAddress = await testContract.createToken().callAsync(); - await testContract.createToken().awaitTransactionSuccessAsync(); + fromTokenAddress = await testContract.createToken(FROM_TOKEN_DECIMALS).callAsync(); + await testContract.createToken(FROM_TOKEN_DECIMALS).awaitTransactionSuccessAsync(); + toTokenAddress = await testContract.createToken(TO_TOKEN_DECIMALS).callAsync(); + 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 { + const amount = getRandomInteger(1, TO_TOKEN_BASE.times(100)); return { fromTokenAddress, toTokenAddress, + amount, toAddress: randomAddress(), - amount: getRandomInteger(1, 10e18), - fillAmount: getRandomInteger(1, 10e18), - fromTokenBalance: getRandomInteger(1, 10e18), + fillAmount: getRandomPortion(amount), + fromTokenBalance: getRandomInteger(1, FROM_TOKEN_BASE.times(100)), ...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(opts.fromTokenBalance) + .div(toBase) + .div(opts.fromTokenBalance.div(fromBase)) + .times(KYBER_RATE_BASE) .integerValue(BigNumber.ROUND_DOWN); } diff --git a/contracts/asset-proxy/test/uniswap_bridge.ts b/contracts/asset-proxy/test/uniswap_bridge.ts index 36c096636d..1b12c6caba 100644 --- a/contracts/asset-proxy/test/uniswap_bridge.ts +++ b/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); diff --git a/packages/contract-addresses/CHANGELOG.json b/packages/contract-addresses/CHANGELOG.json index 9a3069eb13..914abe91e1 100644 --- a/packages/contract-addresses/CHANGELOG.json +++ b/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 } ] }, diff --git a/packages/contract-addresses/addresses.json b/packages/contract-addresses/addresses.json index 4c54e445ae..1742da742a 100644 --- a/packages/contract-addresses/addresses.json +++ b/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": {