Merge pull request #796 from 0xProject/fix/contracts/failingTransactionAssertions

Update expectRevertReasonOrAlwaysFailingTransactionAsync to check status codes

2 files changed, 38 insertions, 27 deletions

diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts
index e702a3200..2ba8f9fb1 100644
--- a/packages/contracts/src/utils/assertions.ts
+++ b/packages/contracts/src/utils/assertions.ts
@@ -1,8 +1,10 @@
 import { RevertReason } from '@0xproject/types';
 import * as chai from 'chai';
+import { TransactionReceipt, TransactionReceiptWithDecodedLogs } from 'ethereum-types';
 import * as _ from 'lodash';
 
 import { constants } from './constants';
+import { web3Wrapper } from './web3_wrapper';
 
 const expect = chai.expect;
 
@@ -53,18 +55,45 @@ export function expectRevertOrAlwaysFailingTransactionAsync<T>(p: Promise<T>): P
 }
 
 /**
- * Rejects if the given Promise does not reject with the given revert reason or "always
- * failing transaction" error.
+ * Rejects if at least one the following conditions is not met:
+ * 1) The given Promise rejects with the given revert reason.
+ * 2) The given Promise rejects with an error containing "always failing transaction"
+ * 3) The given Promise fulfills with a txReceipt that has a status of 0 or '0', indicating the transaction failed.
+ * 4) The given Promise fulfills with a txHash and corresponding txReceipt has a status of 0 or '0'.
  * @param p the Promise which is expected to reject
  * @param reason a specific revert reason
  * @returns a new Promise which will reject if the conditions are not met and
  * otherwise resolve with no value.
  */
-export function expectRevertReasonOrAlwaysFailingTransactionAsync<T>(
-    p: Promise<T>,
+export async function expectRevertReasonOrAlwaysFailingTransactionAsync(
+    p: Promise<string | TransactionReceiptWithDecodedLogs | TransactionReceipt>,
     reason: RevertReason,
-): PromiseLike<void> {
-    return _expectEitherErrorAsync(p, 'always failing transaction', reason);
+): Promise<void> {
+    return p
+        .then(async result => {
+            let txReceiptStatus: string | 0 | 1 | null;
+            if (typeof result === 'string') {
+                // Result is a txHash. We need to make a web3 call to get the receipt.
+                const txReceipt = await web3Wrapper.getTransactionReceiptAsync(result);
+                txReceiptStatus = txReceipt.status;
+            } else if ('status' in result) {
+                // Result is a TransactionReceiptWithDecodedLogs or TransactionReceipt
+                // and status is a field of result.
+                txReceiptStatus = result.status;
+            } else {
+                throw new Error('Unexpected result type');
+            }
+            expect(_.toString(txReceiptStatus)).to.equal(
+                '0',
+                'transactionReceipt had a non-zero status, indicating success',
+            );
+        })
+        .catch(err => {
+            expect(err.message).to.satisfy(
+                (msg: string) => _.includes(msg, reason) || _.includes(msg, 'always failing transaction'),
+                `Expected ${reason} or 'always failing transaction' but error message was ${err.message}`,
+            );
+        });
 }
 
 /**
diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts
index e23ab9851..7bdb9c385 100644
--- a/packages/contracts/test/exchange/match_orders.ts
+++ b/packages/contracts/test/exchange/match_orders.ts
@@ -651,13 +651,7 @@ describe('matchOrders', () => {
             });
             // Match orders
             return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                matchOrderTester.matchOrdersAndVerifyBalancesAsync(
-                    signedOrderLeft,
-                    signedOrderRight,
-                    takerAddress,
-                    erc20BalancesByOwner,
-                    erc721TokenIdsByOwner,
-                ),
+                exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress),
                 RevertReason.NegativeSpreadRequired,
             );
         });
@@ -680,13 +674,7 @@ describe('matchOrders', () => {
             });
             // Match orders
             return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                matchOrderTester.matchOrdersAndVerifyBalancesAsync(
-                    signedOrderLeft,
-                    signedOrderRight,
-                    takerAddress,
-                    erc20BalancesByOwner,
-                    erc721TokenIdsByOwner,
-                ),
+                exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress),
                 // We are assuming assetData fields of the right order are the
                 // reverse of the left order, rather than checking equality. This
                 // saves a bunch of gas, but as a result if the assetData fields are
@@ -715,13 +703,7 @@ describe('matchOrders', () => {
             });
             // Match orders
             return expectRevertReasonOrAlwaysFailingTransactionAsync(
-                matchOrderTester.matchOrdersAndVerifyBalancesAsync(
-                    signedOrderLeft,
-                    signedOrderRight,
-                    takerAddress,
-                    erc20BalancesByOwner,
-                    erc721TokenIdsByOwner,
-                ),
+                exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress),
                 RevertReason.InvalidOrderSignature,
             );
         });