Merge pull request #2338 from 0xProject/fix/contracts/exchange/marketBuyOrdersNoThrow-rounding

Round up in `marketBuyOrdersNoThrow()`
This commit is contained in:
Amir Bandeali 2019-11-14 18:57:29 -08:00 committed by GitHub
commit 4b6501a739
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 166 additions and 9 deletions

View File

@ -9,6 +9,10 @@
{
"note": "Introduced new export ExchangeRevertErrors",
"pr": 2321
},
{
"note": "Round up in `marketBuyOrdersNoThrow()` so `marketBuyOrdersFillOrKill()` doesn't throw up.",
"pr": 2338
}
]
},

View File

@ -205,7 +205,7 @@ contract MixinWrapperFunctions is
// 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 = LibMath.getPartialAmountFloor(
uint256 remainingTakerAssetFillAmount = LibMath.getPartialAmountCeil(
orders[i].takerAssetAmount,
orders[i].makerAssetAmount,
remainingMakerAssetFillAmount

View File

@ -0,0 +1,61 @@
/*
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.5;
pragma experimental ABIEncoderV2;
import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol";
import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol";
import "../src/Exchange.sol";
// Exchange contract with settlement disabled so we can just check `_fillOrder()``
// calculations.
contract TestFillRounding is
Exchange
{
// solhint-disable no-empty-blocks
constructor ()
public
// Initialize the exchange with a fixed chainId ("test" in hex).
Exchange(0x74657374)
{}
function _settleOrder(
bytes32 orderHash,
LibOrder.Order memory order,
address takerAddress,
LibFillResults.FillResults memory fillResults
)
internal
{
// No-op.
}
function _assertFillableOrder(
LibOrder.Order memory order,
LibOrder.OrderInfo memory orderInfo,
address takerAddress,
bytes memory signature
)
internal
view
{
// No-op.
}
}

View File

@ -38,7 +38,7 @@
"config": {
"publicInterfaceContracts": "Exchange,IExchange",
"abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.",
"abis": "./test/generated-artifacts/@(Exchange|IAssetProxy|IAssetProxyDispatcher|IEIP1271Data|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|IProtocolFees|ISignatureValidator|ITransactions|ITransferSimulator|IWallet|IWrapperFunctions|IsolatedExchange|LibExchangeRichErrorDecoder|MixinAssetProxyDispatcher|MixinExchangeCore|MixinMatchOrders|MixinProtocolFees|MixinSignatureValidator|MixinTransactions|MixinTransferSimulator|MixinWrapperFunctions|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestProtocolFeeCollector|TestProtocolFees|TestProtocolFeesReceiver|TestSignatureValidator|TestTransactions|TestValidatorWallet|TestWrapperFunctions).json"
"abis": "./test/generated-artifacts/@(Exchange|IAssetProxy|IAssetProxyDispatcher|IEIP1271Data|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|IProtocolFees|ISignatureValidator|ITransactions|ITransferSimulator|IWallet|IWrapperFunctions|IsolatedExchange|LibExchangeRichErrorDecoder|MixinAssetProxyDispatcher|MixinExchangeCore|MixinMatchOrders|MixinProtocolFees|MixinSignatureValidator|MixinTransactions|MixinTransferSimulator|MixinWrapperFunctions|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestFillRounding|TestLibExchangeRichErrorDecoder|TestProtocolFeeCollector|TestProtocolFees|TestProtocolFeesReceiver|TestSignatureValidator|TestTransactions|TestValidatorWallet|TestWrapperFunctions).json"
},
"repository": {
"type": "git",

View File

@ -32,6 +32,7 @@ import * as MixinWrapperFunctions from '../test/generated-artifacts/MixinWrapper
import * as ReentrancyTester from '../test/generated-artifacts/ReentrancyTester.json';
import * as TestAssetProxyDispatcher from '../test/generated-artifacts/TestAssetProxyDispatcher.json';
import * as TestExchangeInternals from '../test/generated-artifacts/TestExchangeInternals.json';
import * as TestFillRounding from '../test/generated-artifacts/TestFillRounding.json';
import * as TestLibExchangeRichErrorDecoder from '../test/generated-artifacts/TestLibExchangeRichErrorDecoder.json';
import * as TestProtocolFeeCollector from '../test/generated-artifacts/TestProtocolFeeCollector.json';
import * as TestProtocolFees from '../test/generated-artifacts/TestProtocolFees.json';
@ -68,6 +69,7 @@ export const artifacts = {
ReentrancyTester: ReentrancyTester as ContractArtifact,
TestAssetProxyDispatcher: TestAssetProxyDispatcher as ContractArtifact,
TestExchangeInternals: TestExchangeInternals as ContractArtifact,
TestFillRounding: TestFillRounding as ContractArtifact,
TestLibExchangeRichErrorDecoder: TestLibExchangeRichErrorDecoder as ContractArtifact,
TestProtocolFeeCollector: TestProtocolFeeCollector as ContractArtifact,
TestProtocolFees: TestProtocolFees as ContractArtifact,

View File

@ -16,7 +16,7 @@ import ExchangeRevertErrors = require('../src/revert_errors');
import { artifacts } from './artifacts';
import { TestLibExchangeRichErrorDecoderContract } from './wrappers';
blockchainTests.resets.only('LibExchangeRichErrorDecoder', ({ provider, txDefaults }) => {
blockchainTests.resets('LibExchangeRichErrorDecoder', ({ provider, txDefaults }) => {
const ASSET_PROXY_ID_LENGTH = 4;
const SIGNATURE_LENGTH = 66;
const ASSET_DATA_LENGTH = 36;

View File

@ -1,6 +1,14 @@
import { ContractTxFunctionObj } from '@0x/base-contract';
import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs';
import { blockchainTests, constants, describe, expect, hexRandom, orderHashUtils } from '@0x/contracts-test-utils';
import {
blockchainTests,
constants,
describe,
expect,
getRandomPortion,
hexRandom,
orderHashUtils,
} from '@0x/contracts-test-utils';
import { ReferenceFunctions as UtilReferenceFunctions, SafeMathRevertErrors } from '@0x/contracts-utils';
import { FillResults, Order } from '@0x/types';
import { AnyRevertError, BigNumber, StringRevertError } from '@0x/utils';
@ -12,6 +20,7 @@ import ExchangeRevertErrors = require('../src/revert_errors');
import { artifacts } from './artifacts';
import {
TestFillRoundingContract,
TestWrapperFunctionsCancelOrderCalledEventArgs as CancelOrderCalledEventArgs,
TestWrapperFunctionsContract,
TestWrapperFunctionsFillOrderCalledEventArgs as FillOrderCalledEventArgs,
@ -20,7 +29,7 @@ import {
blockchainTests('Exchange wrapper functions unit tests.', env => {
const CHAIN_ID = 0x74657374;
const { ONE_ETHER, MAX_UINT256 } = constants;
const { addFillResults, getPartialAmountFloor } = LibReferenceFunctions;
const { addFillResults, getPartialAmountCeil } = LibReferenceFunctions;
const { safeSub } = UtilReferenceFunctions;
const protocolFeeMultiplier = new BigNumber(150000);
const randomAddress = () => hexRandom(constants.ADDRESS_LENGTH);
@ -939,9 +948,9 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
const signatures = _.times(orders.length, i => createOrderSignature(orders[i]));
const makerAssetFillAmount = ONE_ETHER;
const expectedError = new SafeMathRevertErrors.Uint256BinOpError(
SafeMathRevertErrors.BinOpErrorCodes.DivisionByZero,
orders[0].takerAssetAmount.times(makerAssetFillAmount),
orders[0].makerAssetAmount,
SafeMathRevertErrors.BinOpErrorCodes.SubtractionUnderflow,
constants.ZERO_AMOUNT,
new BigNumber(1),
);
const tx = getContractFn()(orders, makerAssetFillAmount, signatures).awaitTransactionSuccessAsync();
return expect(tx).to.revertWith(expectedError);
@ -989,7 +998,7 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
let fillResults = _.cloneDeep(EMPTY_FILL_RESULTS);
for (const [order, signature] of _.zip(orders, signatures) as [[Order, string]]) {
const remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, fillResults.makerAssetFilledAmount);
const remainingTakerAssetFillAmount = getPartialAmountFloor(
const remainingTakerAssetFillAmount = getPartialAmountCeil(
order.takerAssetAmount,
order.makerAssetAmount,
remainingMakerAssetFillAmount,
@ -1059,6 +1068,85 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
expect(actualResult).to.deep.eq(expectedResult);
assertFillOrderCallsFromLogs(receipt.logs, expectedCalls);
});
describe('partial fills', () => {
let roundingTestContract: TestFillRoundingContract;
// Use another test contract with `_fillOrder()` preserved so the
// correct fill results are returned and we can test partial fills.
// TODO(dorothy-zbornak): Consolidate these contracts into one.
before(async () => {
roundingTestContract = await TestFillRoundingContract.deployFrom0xArtifactAsync(
artifacts.TestFillRounding,
env.provider,
{
...env.txDefaults,
},
{},
);
});
it('small quantities', async () => {
const orders = [
randomOrder({
makerAssetAmount: new BigNumber(30000),
takerAssetAmount: new BigNumber(20000),
}),
];
const signatures = [hexRandom()];
const fillAmount = new BigNumber(10000);
const fillResults = await roundingTestContract
.marketBuyOrdersNoThrow(orders, fillAmount, signatures)
.callAsync();
expect(fillResults.makerAssetFilledAmount).to.bignumber.eq(10000);
});
it('large quantities', async () => {
const orders = [
randomOrder({
makerAssetAmount: new BigNumber('21400000000000000000'),
takerAssetAmount: new BigNumber('6300000000000000000'),
}),
];
const signatures = [hexRandom()];
const fillAmount = new BigNumber('2400000000000000000');
const fillResults = await roundingTestContract
.marketBuyOrdersNoThrow(orders, fillAmount, signatures)
.callAsync();
expect(fillResults.makerAssetFilledAmount).to.bignumber.eq('2400000000000000002');
});
it('large full precision quantities', async () => {
const orders = [
randomOrder({
makerAssetAmount: new BigNumber('942848588381848588533'),
takerAssetAmount: new BigNumber('103048102885858024121'),
}),
];
const signatures = [hexRandom()];
const fillAmount = new BigNumber('84819838457312347759');
const fillResults = await roundingTestContract
.marketBuyOrdersNoThrow(orders, fillAmount, signatures)
.callAsync();
expect(fillResults.makerAssetFilledAmount).to.bignumber.eq('84819838457312347760');
});
describe.optional('full precision fuzzing', () => {
const FUZZ_COUNT = 256;
for (const i of _.times(FUZZ_COUNT)) {
it(`${i + 1}/${FUZZ_COUNT}`, async () => {
const ordersCount = _.random(1, 10);
const orders = _.times(ordersCount, () => randomOrder());
const signatures = orders.map(() => hexRandom());
const totalMakerAssetAmount = BigNumber.sum(...orders.map(o => o.makerAssetAmount));
const fillAmount = getRandomPortion(totalMakerAssetAmount);
const fillResults = await roundingTestContract
.marketBuyOrdersNoThrow(orders, fillAmount, signatures)
.callAsync();
expect(fillResults.makerAssetFilledAmount).to.bignumber.gte(fillAmount);
});
}
});
});
});
describe('marketBuyOrdersFillOrKill', () => {

View File

@ -30,6 +30,7 @@ export * from '../test/generated-wrappers/mixin_wrapper_functions';
export * from '../test/generated-wrappers/reentrancy_tester';
export * from '../test/generated-wrappers/test_asset_proxy_dispatcher';
export * from '../test/generated-wrappers/test_exchange_internals';
export * from '../test/generated-wrappers/test_fill_rounding';
export * from '../test/generated-wrappers/test_lib_exchange_rich_error_decoder';
export * from '../test/generated-wrappers/test_protocol_fee_collector';
export * from '../test/generated-wrappers/test_protocol_fees';

View File

@ -32,6 +32,7 @@
"test/generated-artifacts/ReentrancyTester.json",
"test/generated-artifacts/TestAssetProxyDispatcher.json",
"test/generated-artifacts/TestExchangeInternals.json",
"test/generated-artifacts/TestFillRounding.json",
"test/generated-artifacts/TestLibExchangeRichErrorDecoder.json",
"test/generated-artifacts/TestProtocolFeeCollector.json",
"test/generated-artifacts/TestProtocolFees.json",