diff options
Diffstat (limited to 'packages/contracts')
| -rw-r--r-- | packages/contracts/README.md | 22 | ||||
| -rw-r--r-- | packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol | 12 | ||||
| -rw-r--r-- | packages/contracts/test/extensions/forwarder.ts | 265 | ||||
| -rw-r--r-- | packages/contracts/test/utils/forwarder_wrapper.ts | 9 |
4 files changed, 286 insertions, 22 deletions
diff --git a/packages/contracts/README.md b/packages/contracts/README.md index 11579cf47..5aedb249f 100644 --- a/packages/contracts/README.md +++ b/packages/contracts/README.md @@ -4,29 +4,23 @@ Smart contracts that implement the 0x protocol. Addresses of the deployed contra ## Usage -### 2.0.0 +Contracts that make up and interact with version 2.0.0 of the protocol can be found in the [contracts](./contracts) directory. The contents of this directory are broken down into the following subdirectories: -Contracts that make up and interact with version 2.0.0 of the protocol can be found in the `src/2.0.0` directory. The contents of this directory are broken down into the following subdirectories: - -* protocol +* [protocol](./contracts/protocol) * This directory contains the contracts that make up version 2.0.0. A full specification can be found [here](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md). -* extensions +* [extensions](./contracts/extensions) * This directory contains contracts that interact with the 2.0.0 contracts and will be used in production, such as the [Forwarder](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/forwarder-specification.md) contract. -* examples +* [examples](./contracts/examples) * This directory contains example implementations of contracts that interact with the protocol but are _not_ intended for use in production. Examples include [filter](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md#filter-contracts) contracts, a [Wallet](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md#wallet) contract, and a [Validator](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md#validator) contract, among others. -* tokens +* [tokens](./contracts/tokens) * This directory contains implementations of different tokens and token standards, including [wETH](https://weth.io/), ZRX, [ERC20](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20.md), and [ERC721](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md). -* multisig +* [multisig](./contracts/multisig) * This directory contains the [Gnosis MultiSigWallet](https://github.com/gnosis/MultiSigWallet) and a custom extension that adds a timelock to transactions within the MultiSigWallet. -* utils +* [utils](./contracts/utils) * This directory contains libraries and utils that are shared across all of the other directories. -* test +* [test](./contracts/test) * This directory contains mocks and other contracts that are used solely for testing contracts within the other directories. -### 1.0.0 - -Contracts that make up version 1.0.0 of the protocol can be found in `src/1.0.0`. These contracts are considered deprecated and will have limited support going forward. - ## Bug bounty A bug bounty for the 2.0.0 contracts is ongoing! Instructions can be found [here](https://0xproject.com/wiki#Bug-Bounty). diff --git a/packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol b/packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol index fea9a53c2..4991c0ea5 100644 --- a/packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol +++ b/packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol @@ -155,8 +155,10 @@ contract MixinExchangeWrapper is uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); // Convert the remaining amount of makerAsset to buy into remaining amount - // of takerAsset to sell, assuming entire amount can be sold in the current order - uint256 remainingTakerAssetFillAmount = getPartialAmountFloor( + // of takerAsset to sell, assuming entire amount can be sold in the current order. + // We round up because the exchange rate computed by fillOrder rounds in favor + // of the Maker. In this case we want to overestimate the amount of takerAsset. + uint256 remainingTakerAssetFillAmount = getPartialAmountCeil( orders[i].takerAssetAmount, orders[i].makerAssetAmount, remainingMakerAssetFillAmount @@ -224,7 +226,9 @@ contract MixinExchangeWrapper is // Convert the remaining amount of ZRX to buy into remaining amount // of WETH to sell, assuming entire amount can be sold in the current order. - uint256 remainingWethSellAmount = getPartialAmountFloor( + // We round up because the exchange rate computed by fillOrder rounds in favor + // of the Maker. In this case we want to overestimate the amount of takerAsset. + uint256 remainingWethSellAmount = getPartialAmountCeil( orders[i].takerAssetAmount, safeSub(orders[i].makerAssetAmount, orders[i].takerFee), // our exchange rate after fees remainingZrxBuyAmount @@ -233,7 +237,7 @@ contract MixinExchangeWrapper is // Attempt to sell the remaining amount of WETH. FillResults memory singleFillResult = fillOrderNoThrow( orders[i], - safeAdd(remainingWethSellAmount, 1), // we add 1 wei to the fill amount to make up for rounding errors + remainingWethSellAmount, signatures[i] ); diff --git a/packages/contracts/test/extensions/forwarder.ts b/packages/contracts/test/extensions/forwarder.ts index b76624fa9..c006be0fe 100644 --- a/packages/contracts/test/extensions/forwarder.ts +++ b/packages/contracts/test/extensions/forwarder.ts @@ -45,6 +45,7 @@ describe(ContractName.Forwarder, () => { let weth: DummyERC20TokenContract; let zrxToken: DummyERC20TokenContract; + let erc20TokenA: DummyERC20TokenContract; let erc721Token: DummyERC721TokenContract; let forwarderContract: ForwarderContract; let wethContract: WETH9Contract; @@ -77,7 +78,6 @@ describe(ContractName.Forwarder, () => { erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner); const numDummyErc20ToDeploy = 3; - let erc20TokenA; [erc20TokenA, zrxToken] = await erc20Wrapper.deployDummyTokensAsync( numDummyErc20ToDeploy, constants.DUMMY_TOKEN_DECIMALS, @@ -902,6 +902,269 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); + it('Should buy slightly greater MakerAsset when exchange rate is rounded', async () => { + // The 0x Protocol contracts round the exchange rate in favor of the Maker. + // In this case, the taker must round up how much they're going to spend, which + // in turn increases the amount of MakerAsset being purchased. + // Example: + // The taker wants to buy 5 units of the MakerAsset at a rate of 3M/2T. + // For every 2 units of TakerAsset, the taker will receive 3 units of MakerAsset. + // To purchase 5 units, the taker must spend 10/3 = 3.33 units of TakerAssset. + // However, the Taker can only spend whole units. + // Spending floor(10/3) = 3 units will yield a profit of Floor(3*3/2) = Floor(4.5) = 4 units of MakerAsset. + // Spending ceil(10/3) = 4 units will yield a profit of Floor(4*3/2) = 6 units of MakerAsset. + // + // The forwarding contract will opt for the second option, which overbuys, to ensure the taker + // receives at least the amount of MakerAsset they requested. + // + // Construct test case using values from example above + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('30'), + takerAssetAmount: new BigNumber('20'), + makerAssetData: assetDataUtils.encodeERC20AssetData(erc20TokenA.address), + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const desiredMakerAssetFillAmount = new BigNumber('5'); + const makerAssetFillAmount = new BigNumber('6'); + const ethValue = new BigNumber('4'); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + desiredMakerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + ); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerAssetAddress].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('Should buy slightly greater MakerAsset when exchange rate is rounded, and MakerAsset is ZRX', async () => { + // See the test case above for a detailed description of this case. + // The difference here is that the MakerAsset is ZRX. We expect the same result as above, + // but this tests a different code path. + // + // Construct test case using values from example above + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('30'), + takerAssetAmount: new BigNumber('20'), + makerAssetData: zrxAssetData, + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const desiredMakerAssetFillAmount = new BigNumber('5'); + const makerAssetFillAmount = new BigNumber('6'); + const ethValue = new BigNumber('4'); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + desiredMakerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + ); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][zrxToken.address].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[takerAddress][zrxToken.address].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('Should buy slightly greater MakerAsset when exchange rate is rounded (Regression Test)', async () => { + // Order taken from a transaction on mainnet that failed due to a rounding error. + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('268166666666666666666'), + takerAssetAmount: new BigNumber('219090625878836371'), + makerAssetData: assetDataUtils.encodeERC20AssetData(erc20TokenA.address), + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + // The taker will receive more than the desired amount of makerAsset due to rounding + const desiredMakerAssetFillAmount = new BigNumber('5000000000000000000'); + const ethValue = new BigNumber('4084971271824171'); + const makerAssetFillAmount = ethValue + .times(orderWithoutFee.makerAssetAmount) + .dividedToIntegerBy(orderWithoutFee.takerAssetAmount); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + desiredMakerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + ); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerAssetAddress].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('Should buy slightly greater MakerAsset when exchange rate is rounded, and MakerAsset is ZRX (Regression Test)', async () => { + // Order taken from a transaction on mainnet that failed due to a rounding error. + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('268166666666666666666'), + takerAssetAmount: new BigNumber('219090625878836371'), + makerAssetData: zrxAssetData, + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + // The taker will receive more than the desired amount of makerAsset due to rounding + const desiredMakerAssetFillAmount = new BigNumber('5000000000000000000'); + const ethValue = new BigNumber('4084971271824171'); + const makerAssetFillAmount = ethValue + .times(orderWithoutFee.makerAssetAmount) + .dividedToIntegerBy(orderWithoutFee.takerAssetAmount); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + desiredMakerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + ); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][zrxToken.address].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[takerAddress][zrxToken.address].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('Should buy correct MakerAsset when exchange rate is NOT rounded, and MakerAsset is ZRX (Regression Test)', async () => { + // An extra unit of TakerAsset was sent to the exchange contract to account for rounding errors, in Forwarder v1. + // Specifically, the takerFillAmount was calculated using Floor(desiredMakerAmount * exchangeRate) + 1 + // We have since changed this to be Ceil(desiredMakerAmount * exchangeRate) + // These calculations produce different results when `desiredMakerAmount * exchangeRate` is an integer. + // + // This test verifies that `ceil` is sufficient: + // Let TakerAssetAmount = MakerAssetAmount * 2 + // -> exchangeRate = TakerAssetAmount / MakerAssetAmount = (2*MakerAssetAmount)/MakerAssetAmount = 2 + // .: desiredMakerAmount * exchangeRate is an integer. + // + // Construct test case using values from example above + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('30'), + takerAssetAmount: new BigNumber('60'), + makerAssetData: zrxAssetData, + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const makerAssetFillAmount = new BigNumber('5'); + const ethValue = new BigNumber('10'); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { + value: ethValue, + from: takerAddress, + }); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][zrxToken.address].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[takerAddress][zrxToken.address].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); }); describe('marketBuyOrdersWithEth with extra fees', () => { it('should buy an asset and send fee to feeRecipient', async () => { diff --git a/packages/contracts/test/utils/forwarder_wrapper.ts b/packages/contracts/test/utils/forwarder_wrapper.ts index f1a64d47d..a0bfcfe1d 100644 --- a/packages/contracts/test/utils/forwarder_wrapper.ts +++ b/packages/contracts/test/utils/forwarder_wrapper.ts @@ -26,9 +26,12 @@ export class ForwarderWrapper { _.forEach(feeOrders, feeOrder => { const feeAvailable = feeOrder.makerAssetAmount.minus(feeOrder.takerFee); if (!remainingFeeAmount.isZero() && feeAvailable.gt(remainingFeeAmount)) { - wethAmount = wethAmount - .plus(feeOrder.takerAssetAmount.times(remainingFeeAmount).dividedToIntegerBy(feeAvailable)) - .plus(1); + wethAmount = wethAmount.plus( + feeOrder.takerAssetAmount + .times(remainingFeeAmount) + .dividedBy(feeAvailable) + .ceil(), + ); remainingFeeAmount = new BigNumber(0); } else if (!remainingFeeAmount.isZero()) { wethAmount = wethAmount.plus(feeOrder.takerAssetAmount); |