Merge pull request #959 from 0xProject/bug/order-utils/rounding-error-small-maker

[Order-utils] Order is valid when maker amount is very small

4 files changed, 43 insertions, 1 deletions

diff --git a/packages/contracts/test/exchange/fill_order.ts b/packages/contracts/test/exchange/fill_order.ts
index 1494fe093..e79e2239e 100644
--- a/packages/contracts/test/exchange/fill_order.ts
+++ b/packages/contracts/test/exchange/fill_order.ts
@@ -104,6 +104,17 @@ describe('FillOrder Tests', () => {
             };
             await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario);
         });
+        it('should transfer the correct amounts when makerAssetAmount < takerAssetAmount with zero decimals', async () => {
+            const fillScenario = {
+                ...defaultFillScenario,
+                orderScenario: {
+                    ...defaultFillScenario.orderScenario,
+                    makerAssetAmountScenario: OrderAssetAmountScenario.Small,
+                    makerAssetDataScenario: AssetDataScenario.ERC20ZeroDecimals,
+                },
+            };
+            await fillOrderCombinatorialUtils.testFillOrderScenarioAsync(provider, fillScenario);
+        });
         it('should transfer the correct amounts when taker is specified and order is claimed by taker', async () => {
             const fillScenario = {
                 ...defaultFillScenario,
diff --git a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts
index 284c4a2db..f18ad0dd3 100644
--- a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts
+++ b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts
@@ -81,6 +81,12 @@ export async function fillOrderCombinatorialUtilsFactoryAsync(
         erc20FiveDecimalTokenCount,
         fiveDecimals,
     );
+    const zeroDecimals = new BigNumber(0);
+    const erc20ZeroDecimalTokenCount = 2;
+    const [erc20ZeroDecimalTokenA, erc20ZeroDecimalTokenB] = await erc20Wrapper.deployDummyTokensAsync(
+        erc20ZeroDecimalTokenCount,
+        zeroDecimals,
+    );
     const erc20Proxy = await erc20Wrapper.deployProxyAsync();
     await erc20Wrapper.setBalancesAndAllowancesAsync();
 
@@ -119,6 +125,7 @@ export async function fillOrderCombinatorialUtilsFactoryAsync(
         zrxToken.address,
         [erc20EighteenDecimalTokenA.address, erc20EighteenDecimalTokenB.address],
         [erc20FiveDecimalTokenA.address, erc20FiveDecimalTokenB.address],
+        [erc20ZeroDecimalTokenA.address, erc20ZeroDecimalTokenB.address],
         erc721Token,
         erc721Balances,
         exchangeContract.address,
diff --git a/packages/contracts/test/utils/order_factory_from_scenario.ts b/packages/contracts/test/utils/order_factory_from_scenario.ts
index a908140b9..8e04db588 100644
--- a/packages/contracts/test/utils/order_factory_from_scenario.ts
+++ b/packages/contracts/test/utils/order_factory_from_scenario.ts
@@ -21,6 +21,8 @@ const POINT_ONE_UNITS_EIGHTEEN_DECIMALS = new BigNumber(100_000_000_000_000_000)
 const POINT_ZERO_FIVE_UNITS_EIGHTEEN_DECIMALS = new BigNumber(50_000_000_000_000_000);
 const TEN_UNITS_FIVE_DECIMALS = new BigNumber(1_000_000);
 const FIVE_UNITS_FIVE_DECIMALS = new BigNumber(500_000);
+const TEN_UNITS_ZERO_DECIMALS = new BigNumber(10);
+const ONE_THOUSAND_UNITS_ZERO_DECIMALS = new BigNumber(1000);
 const ONE_NFT_UNIT = new BigNumber(1);
 
 export class OrderFactoryFromScenario {
@@ -28,6 +30,7 @@ export class OrderFactoryFromScenario {
     private readonly _zrxAddress: string;
     private readonly _nonZrxERC20EighteenDecimalTokenAddresses: string[];
     private readonly _erc20FiveDecimalTokenAddresses: string[];
+    private readonly _erc20ZeroDecimalTokenAddresses: string[];
     private readonly _erc721Token: DummyERC721TokenContract;
     private readonly _erc721Balances: ERC721TokenIdsByOwner;
     private readonly _exchangeAddress: string;
@@ -36,6 +39,7 @@ export class OrderFactoryFromScenario {
         zrxAddress: string,
         nonZrxERC20EighteenDecimalTokenAddresses: string[],
         erc20FiveDecimalTokenAddresses: string[],
+        erc20ZeroDecimalTokenAddresses: string[],
         erc721Token: DummyERC721TokenContract,
         erc721Balances: ERC721TokenIdsByOwner,
         exchangeAddress: string,
@@ -44,6 +48,7 @@ export class OrderFactoryFromScenario {
         this._zrxAddress = zrxAddress;
         this._nonZrxERC20EighteenDecimalTokenAddresses = nonZrxERC20EighteenDecimalTokenAddresses;
         this._erc20FiveDecimalTokenAddresses = erc20FiveDecimalTokenAddresses;
+        this._erc20ZeroDecimalTokenAddresses = erc20ZeroDecimalTokenAddresses;
         this._erc721Token = erc721Token;
         this._erc721Balances = erc721Balances;
         this._exchangeAddress = exchangeAddress;
@@ -89,6 +94,9 @@ export class OrderFactoryFromScenario {
                     erc721MakerAssetIds[0],
                 );
                 break;
+            case AssetDataScenario.ERC20ZeroDecimals:
+                makerAssetData = assetDataUtils.encodeERC20AssetData(this._erc20ZeroDecimalTokenAddresses[0]);
+                break;
             default:
                 throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.makerAssetDataScenario);
         }
@@ -109,6 +117,9 @@ export class OrderFactoryFromScenario {
                     erc721TakerAssetIds[0],
                 );
                 break;
+            case AssetDataScenario.ERC20ZeroDecimals:
+                takerAssetData = assetDataUtils.encodeERC20AssetData(this._erc20ZeroDecimalTokenAddresses[1]);
+                break;
             default:
                 throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.takerAssetDataScenario);
         }
@@ -126,6 +137,9 @@ export class OrderFactoryFromScenario {
                     case AssetDataScenario.ERC721:
                         makerAssetAmount = ONE_NFT_UNIT;
                         break;
+                    case AssetDataScenario.ERC20ZeroDecimals:
+                        makerAssetAmount = ONE_THOUSAND_UNITS_ZERO_DECIMALS;
+                        break;
                     default:
                         throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.makerAssetDataScenario);
                 }
@@ -142,6 +156,9 @@ export class OrderFactoryFromScenario {
                     case AssetDataScenario.ERC721:
                         makerAssetAmount = ONE_NFT_UNIT;
                         break;
+                    case AssetDataScenario.ERC20ZeroDecimals:
+                        makerAssetAmount = TEN_UNITS_ZERO_DECIMALS;
+                        break;
                     default:
                         throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.makerAssetDataScenario);
                 }
@@ -166,6 +183,9 @@ export class OrderFactoryFromScenario {
                     case AssetDataScenario.ERC721:
                         takerAssetAmount = ONE_NFT_UNIT;
                         break;
+                    case AssetDataScenario.ERC20ZeroDecimals:
+                        takerAssetAmount = ONE_THOUSAND_UNITS_ZERO_DECIMALS;
+                        break;
                     default:
                         throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.takerAssetDataScenario);
                 }
@@ -182,6 +202,9 @@ export class OrderFactoryFromScenario {
                     case AssetDataScenario.ERC721:
                         takerAssetAmount = ONE_NFT_UNIT;
                         break;
+                    case AssetDataScenario.ERC20ZeroDecimals:
+                        takerAssetAmount = TEN_UNITS_ZERO_DECIMALS;
+                        break;
                     default:
                         throw errorUtils.spawnSwitchErr('AssetDataScenario', orderScenario.takerAssetDataScenario);
                 }
diff --git a/packages/contracts/test/utils/types.ts b/packages/contracts/test/utils/types.ts
index 67313b647..481ee87d6 100644
--- a/packages/contracts/test/utils/types.ts
+++ b/packages/contracts/test/utils/types.ts
@@ -177,10 +177,11 @@ export enum ExpirationTimeSecondsScenario {
 }
 
 export enum AssetDataScenario {
-    ERC721 = 'ERC721',
+    ERC20ZeroDecimals = 'ERC20_ZERO_DECIMALS',
     ZRXFeeToken = 'ZRX_FEE_TOKEN',
     ERC20FiveDecimals = 'ERC20_FIVE_DECIMALS',
     ERC20NonZRXEighteenDecimals = 'ERC20_NON_ZRX_EIGHTEEN_DECIMALS',
+    ERC721 = 'ERC721',
 }
 
 export enum TakerAssetFillAmountScenario {