crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Round up for Market Buys in Forwarding Contract. Includes new test cases + regression testing.

Browse Source
This commit is contained in:
Greg Hysen 2018-10-15 17:52:51 -07:00 committed by Amir Bandeali
parent 1f0c7f8fbe
commit e086c7b8e6
3 changed files with 280 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol
View File

@ -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]
);

267
packages/contracts/test/extensions/forwarder.ts
View File

@ -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,271 @@ 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.
- // tx=0x3d22b18fe2615c1903408b1b969744bcd117becc30205df18d5c5c54d27d1237
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.
- // tx=0x3d22b18fe2615c1903408b1b969744bcd117becc30205df18d5c5c54d27d1237
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 () => {

9
packages/contracts/test/utils/forwarder_wrapper.ts
View File

@ -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);