aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFabio Berger <me@fabioberger.com>2018-07-02 18:24:57 +0800
committerGitHub <noreply@github.com>2018-07-02 18:24:57 +0800
commit590033bcb27c9e8299c1cb56b75f9a64d674affb (patch)
treeea50d633171cc78ca70146c7e883251c23ced0d1
parentb9b00e10d39c3c84bc72892ef37f1313e904414d (diff)
parentaf1e4574a8ed97416ce27b1d9b532ee7ed02a746 (diff)
downloaddexon-sol-tools-590033bcb27c9e8299c1cb56b75f9a64d674affb.tar
dexon-sol-tools-590033bcb27c9e8299c1cb56b75f9a64d674affb.tar.gz
dexon-sol-tools-590033bcb27c9e8299c1cb56b75f9a64d674affb.tar.bz2
dexon-sol-tools-590033bcb27c9e8299c1cb56b75f9a64d674affb.tar.lz
dexon-sol-tools-590033bcb27c9e8299c1cb56b75f9a64d674affb.tar.xz
dexon-sol-tools-590033bcb27c9e8299c1cb56b75f9a64d674affb.tar.zst
dexon-sol-tools-590033bcb27c9e8299c1cb56b75f9a64d674affb.zip
Merge pull request #802 from 0xProject/fix/five_decimal_scenario
FillOrder Combinatorial Testing Fixes
-rw-r--r--.gitignore3
-rw-r--r--packages/contracts/test/exchange/fill_order.ts9
-rw-r--r--packages/contracts/test/utils/asset_wrapper.ts11
-rw-r--r--packages/contracts/test/utils/core_combinatorial_utils.ts130
-rw-r--r--packages/order-utils/src/order_validation_utils.ts6
5 files changed, 119 insertions, 40 deletions
diff --git a/.gitignore b/.gitignore
index 350e12995..feab4f31f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -67,6 +67,9 @@ generated_docs/
TODO.md
+# VSCode file
+.vscode
+
packages/website/public/bundle*
packages/react-docs/example/public/bundle*
diff --git a/packages/contracts/test/exchange/fill_order.ts b/packages/contracts/test/exchange/fill_order.ts
index 22eb401d9..029bd66e2 100644
--- a/packages/contracts/test/exchange/fill_order.ts
+++ b/packages/contracts/test/exchange/fill_order.ts
@@ -65,14 +65,7 @@ describe('FillOrder Tests', () => {
describe('fillOrder', () => {
const test = (fillScenarios: FillScenario[]) => {
_.forEach(fillScenarios, fillScenario => {
- const orderScenario = fillScenario.orderScenario;
- const description = `Combinatorial OrderFill: ${orderScenario.feeRecipientScenario} ${
- orderScenario.makerAssetAmountScenario
- } ${orderScenario.takerAssetAmountScenario} ${orderScenario.makerFeeScenario} ${
- orderScenario.takerFeeScenario
- } ${orderScenario.expirationTimeSecondsScenario} ${orderScenario.makerAssetDataScenario} ${
- orderScenario.takerAssetDataScenario
- }`;
+ const description = `Combinatorial OrderFill: ${JSON.stringify(fillScenario)}`;
it(description, async () => {
await coreCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario);
});
diff --git a/packages/contracts/test/utils/asset_wrapper.ts b/packages/contracts/test/utils/asset_wrapper.ts
index 402a7ab28..f291170a2 100644
--- a/packages/contracts/test/utils/asset_wrapper.ts
+++ b/packages/contracts/test/utils/asset_wrapper.ts
@@ -84,8 +84,15 @@ export class AssetWrapper {
userAddress,
);
} else if (tokenOwner === userAddress && desiredBalance.eq(0)) {
- // Burn token
- await erc721Wrapper.burnAsync(assetProxyData.tokenAddress, assetProxyData.tokenId, userAddress);
+ // Transfer token to someone else
+ const userAddresses = await (erc721Wrapper as any)._web3Wrapper.getAvailableAddressesAsync();
+ const nonOwner = _.find(userAddresses, a => a !== userAddress);
+ await erc721Wrapper.transferFromAsync(
+ assetProxyData.tokenAddress,
+ assetProxyData.tokenId,
+ tokenOwner,
+ nonOwner,
+ );
return;
} else if (
(userAddress !== tokenOwner && desiredBalance.eq(0)) ||
diff --git a/packages/contracts/test/utils/core_combinatorial_utils.ts b/packages/contracts/test/utils/core_combinatorial_utils.ts
index 718be17e0..03b09521d 100644
--- a/packages/contracts/test/utils/core_combinatorial_utils.ts
+++ b/packages/contracts/test/utils/core_combinatorial_utils.ts
@@ -146,13 +146,39 @@ export class CoreCombinatorialUtils {
public exchangeWrapper: ExchangeWrapper;
public assetWrapper: AssetWrapper;
public static generateFillOrderCombinations(): FillScenario[] {
- const takerScenarios = [TakerScenario.Unspecified];
- const feeRecipientScenarios = [FeeRecipientAddressScenario.EthUserAddress];
- const makerAssetAmountScenario = [OrderAssetAmountScenario.Large];
- const takerAssetAmountScenario = [OrderAssetAmountScenario.Large];
- const makerFeeScenario = [OrderAssetAmountScenario.Large];
- const takerFeeScenario = [OrderAssetAmountScenario.Large];
- const expirationTimeSecondsScenario = [ExpirationTimeSecondsScenario.InFuture];
+ const takerScenarios = [
+ TakerScenario.Unspecified,
+ // TakerScenario.CorrectlySpecified,
+ // TakerScenario.IncorrectlySpecified,
+ ];
+ const feeRecipientScenarios = [
+ FeeRecipientAddressScenario.EthUserAddress,
+ // FeeRecipientAddressScenario.BurnAddress,
+ ];
+ const makerAssetAmountScenario = [
+ OrderAssetAmountScenario.Large,
+ // OrderAssetAmountScenario.Zero,
+ // OrderAssetAmountScenario.Small,
+ ];
+ const takerAssetAmountScenario = [
+ OrderAssetAmountScenario.Large,
+ // OrderAssetAmountScenario.Zero,
+ // OrderAssetAmountScenario.Small,
+ ];
+ const makerFeeScenario = [
+ OrderAssetAmountScenario.Large,
+ // OrderAssetAmountScenario.Small,
+ // OrderAssetAmountScenario.Zero,
+ ];
+ const takerFeeScenario = [
+ OrderAssetAmountScenario.Large,
+ // OrderAssetAmountScenario.Small,
+ // OrderAssetAmountScenario.Zero,
+ ];
+ const expirationTimeSecondsScenario = [
+ ExpirationTimeSecondsScenario.InFuture,
+ ExpirationTimeSecondsScenario.InPast,
+ ];
const makerAssetDataScenario = [
AssetDataScenario.ERC20FiveDecimals,
AssetDataScenario.ERC20NonZRXEighteenDecimals,
@@ -165,7 +191,55 @@ export class CoreCombinatorialUtils {
AssetDataScenario.ERC721,
AssetDataScenario.ZRXFeeToken,
];
- const takerAssetFillAmountScenario = [TakerAssetFillAmountScenario.ExactlyRemainingFillableTakerAssetAmount];
+ const takerAssetFillAmountScenario = [
+ TakerAssetFillAmountScenario.ExactlyRemainingFillableTakerAssetAmount,
+ // TakerAssetFillAmountScenario.GreaterThanRemainingFillableTakerAssetAmount,
+ // TakerAssetFillAmountScenario.LessThanRemainingFillableTakerAssetAmount,
+ ];
+ const makerAssetBalanceScenario = [
+ BalanceAmountScenario.Higher,
+ // BalanceAmountScenario.Exact,
+ // BalanceAmountScenario.TooLow,
+ ];
+ const makerAssetAllowanceScenario = [
+ AllowanceAmountScenario.Higher,
+ // AllowanceAmountScenario.Exact,
+ // AllowanceAmountScenario.TooLow,
+ // AllowanceAmountScenario.Unlimited,
+ ];
+ const makerZRXBalanceScenario = [
+ BalanceAmountScenario.Higher,
+ // BalanceAmountScenario.Exact,
+ // BalanceAmountScenario.TooLow,
+ ];
+ const makerZRXAllowanceScenario = [
+ AllowanceAmountScenario.Higher,
+ // AllowanceAmountScenario.Exact,
+ // AllowanceAmountScenario.TooLow,
+ // AllowanceAmountScenario.Unlimited,
+ ];
+ const takerAssetBalanceScenario = [
+ BalanceAmountScenario.Higher,
+ // BalanceAmountScenario.Exact,
+ // BalanceAmountScenario.TooLow,
+ ];
+ const takerAssetAllowanceScenario = [
+ AllowanceAmountScenario.Higher,
+ // AllowanceAmountScenario.Exact,
+ // AllowanceAmountScenario.TooLow,
+ // AllowanceAmountScenario.Unlimited,
+ ];
+ const takerZRXBalanceScenario = [
+ BalanceAmountScenario.Higher,
+ // BalanceAmountScenario.Exact,
+ // BalanceAmountScenario.TooLow,
+ ];
+ const takerZRXAllowanceScenario = [
+ AllowanceAmountScenario.Higher,
+ // AllowanceAmountScenario.Exact,
+ // AllowanceAmountScenario.TooLow,
+ // AllowanceAmountScenario.Unlimited,
+ ];
const fillScenarioArrays = CoreCombinatorialUtils._getAllCombinations([
takerScenarios,
feeRecipientScenarios,
@@ -177,9 +251,18 @@ export class CoreCombinatorialUtils {
makerAssetDataScenario,
takerAssetDataScenario,
takerAssetFillAmountScenario,
+ makerAssetBalanceScenario,
+ makerAssetAllowanceScenario,
+ makerZRXBalanceScenario,
+ makerZRXAllowanceScenario,
+ takerAssetBalanceScenario,
+ takerAssetAllowanceScenario,
+ takerZRXBalanceScenario,
+ takerZRXAllowanceScenario,
]);
const fillScenarios = _.map(fillScenarioArrays, fillScenarioArray => {
+ // tslint:disable:custom-no-magic-numbers
const fillScenario: FillScenario = {
orderScenario: {
takerScenario: fillScenarioArray[0] as TakerScenario,
@@ -194,18 +277,19 @@ export class CoreCombinatorialUtils {
},
takerAssetFillAmountScenario: fillScenarioArray[9] as TakerAssetFillAmountScenario,
makerStateScenario: {
- traderAssetBalance: BalanceAmountScenario.Higher,
- traderAssetAllowance: AllowanceAmountScenario.Higher,
- zrxFeeBalance: BalanceAmountScenario.Higher,
- zrxFeeAllowance: AllowanceAmountScenario.Higher,
+ traderAssetBalance: fillScenarioArray[10] as BalanceAmountScenario,
+ traderAssetAllowance: fillScenarioArray[11] as AllowanceAmountScenario,
+ zrxFeeBalance: fillScenarioArray[12] as BalanceAmountScenario,
+ zrxFeeAllowance: fillScenarioArray[13] as AllowanceAmountScenario,
},
takerStateScenario: {
- traderAssetBalance: BalanceAmountScenario.Higher,
- traderAssetAllowance: AllowanceAmountScenario.Higher,
- zrxFeeBalance: BalanceAmountScenario.Higher,
- zrxFeeAllowance: AllowanceAmountScenario.Higher,
+ traderAssetBalance: fillScenarioArray[14] as BalanceAmountScenario,
+ traderAssetAllowance: fillScenarioArray[15] as AllowanceAmountScenario,
+ zrxFeeBalance: fillScenarioArray[16] as BalanceAmountScenario,
+ zrxFeeAllowance: fillScenarioArray[17] as AllowanceAmountScenario,
},
};
+ // tslint:enable:custom-no-magic-numbers
return fillScenario;
});
@@ -768,25 +852,17 @@ export class CoreCombinatorialUtils {
case AllowanceAmountScenario.TooLow:
const tooLowAllowance = takerFee.minus(1);
- await this.assetWrapper.setProxyAllowanceAsync(
- signedOrder.takerAddress,
- this.zrxAssetData,
- tooLowAllowance,
- );
+ await this.assetWrapper.setProxyAllowanceAsync(this.takerAddress, this.zrxAssetData, tooLowAllowance);
break;
case AllowanceAmountScenario.Exact:
const exactAllowance = takerFee;
- await this.assetWrapper.setProxyAllowanceAsync(
- signedOrder.takerAddress,
- this.zrxAssetData,
- exactAllowance,
- );
+ await this.assetWrapper.setProxyAllowanceAsync(this.takerAddress, this.zrxAssetData, exactAllowance);
break;
case AllowanceAmountScenario.Unlimited:
await this.assetWrapper.setProxyAllowanceAsync(
- signedOrder.takerAddress,
+ this.takerAddress,
this.zrxAssetData,
constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS,
);
diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts
index 54428f77d..94df3ef82 100644
--- a/packages/order-utils/src/order_validation_utils.ts
+++ b/packages/order-utils/src/order_validation_utils.ts
@@ -140,12 +140,12 @@ export class OrderValidationUtils {
takerAddress: string,
zrxAssetData: string,
): Promise<BigNumber> {
- if (fillTakerAssetAmount.eq(0)) {
- throw new Error(RevertReason.InvalidTakerAmount);
- }
if (signedOrder.makerAssetAmount.eq(0) || signedOrder.takerAssetAmount.eq(0)) {
throw new Error(RevertReason.OrderUnfillable);
}
+ if (fillTakerAssetAmount.eq(0)) {
+ throw new Error(RevertReason.InvalidTakerAmount);
+ }
const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
const isValid = await isValidSignatureAsync(
provider,