Fix bug where we hard-coded using pendingBlock for fetching the orderState. Moved numConfirmations to become a global orderStateWatcher config

5 files changed, 36 insertions, 20 deletions

diff --git a/src/order_watcher/event_watcher.ts b/src/order_watcher/event_watcher.ts
index ce471b58d..f71b14afb 100644
--- a/src/order_watcher/event_watcher.ts
+++ b/src/order_watcher/event_watcher.ts
@@ -13,16 +13,18 @@ export class EventWatcher {
     private _intervalId: NodeJS.Timer;
     private _lastEvents: Web3.LogEntry[] = [];
     private _callbackIfExistsAsync?: EventWatcherCallback;
-    constructor(web3Wrapper: Web3Wrapper, pollingIntervalMs: undefined|number) {
+    private _numConfirmations: number;
+    constructor(web3Wrapper: Web3Wrapper, pollingIntervalMs: undefined|number, numConfirmations: number) {
         this._web3Wrapper = web3Wrapper;
+        this._numConfirmations = numConfirmations;
         this._pollingIntervalMs = _.isUndefined(pollingIntervalMs) ?
                                   DEFAULT_EVENT_POLLING_INTERVAL :
                                   pollingIntervalMs;
     }
-    public subscribe(callback: EventWatcherCallback, numConfirmations: number): void {
+    public subscribe(callback: EventWatcherCallback): void {
         this._callbackIfExistsAsync = callback;
         this._intervalId = intervalUtils.setAsyncExcludingInterval(
-            this._pollForMempoolEventsAsync.bind(this, numConfirmations), this._pollingIntervalMs,
+            this._pollForMempoolEventsAsync.bind(this), this._pollingIntervalMs,
         );
     }
     public unsubscribe(): void {
@@ -30,8 +32,8 @@ export class EventWatcher {
         this._lastEvents = [];
         intervalUtils.clearAsyncExcludingInterval(this._intervalId);
     }
-    private async _pollForMempoolEventsAsync(numConfirmations: number): Promise<void> {
-        const pendingEvents = await this._getEventsAsync(numConfirmations);
+    private async _pollForMempoolEventsAsync(): Promise<void> {
+        const pendingEvents = await this._getEventsAsync();
         if (pendingEvents.length === 0) {
             // HACK: Sometimes when node rebuilds the pending block we get back the empty result.
             // We don't want to emit a lot of removal events and bring them back after a couple of miliseconds,
@@ -46,16 +48,16 @@ export class EventWatcher {
         await this._emitDifferencesAsync(newEvents, isRemoved);
         this._lastEvents = pendingEvents;
     }
-    private async _getEventsAsync(numConfirmations: number): Promise<Web3.LogEntry[]> {
+    private async _getEventsAsync(): Promise<Web3.LogEntry[]> {
         let fromBlock: BlockParamLiteral|number;
         let toBlock: BlockParamLiteral|number;
-        if (numConfirmations === 0) {
+        if (this._numConfirmations === 0) {
             fromBlock = BlockParamLiteral.Pending;
-            toBlock = BlockParamLiteral.Pending;
+            toBlock = fromBlock;
         } else {
             const currentBlock = await this._web3Wrapper.getBlockNumberAsync();
-            toBlock = currentBlock - numConfirmations;
-            fromBlock = currentBlock - numConfirmations;
+            toBlock = currentBlock - this._numConfirmations;
+            fromBlock = toBlock;
         }
         const eventFilter = {
             fromBlock,
diff --git a/src/order_watcher/order_state_watcher.ts b/src/order_watcher/order_state_watcher.ts
index 000e92773..fa91d1f44 100644
--- a/src/order_watcher/order_state_watcher.ts
+++ b/src/order_watcher/order_state_watcher.ts
@@ -22,6 +22,8 @@ import {
 } from '../types';
 import {Web3Wrapper} from '../web3_wrapper';
 
+const DEFAULT_NUM_CONFIRMATIONS = 0;
+
 interface DependentOrderHashes {
     [makerAddress: string]: {
         [makerToken: string]: Set<string>,
@@ -40,6 +42,7 @@ export class OrderStateWatcher {
     private _eventWatcher: EventWatcher;
     private _abiDecoder: AbiDecoder;
     private _orderStateUtils: OrderStateUtils;
+    private _numConfirmations: number;
     constructor(
         web3Wrapper: Web3Wrapper, abiDecoder: AbiDecoder, orderStateUtils: OrderStateUtils,
         config?: OrderStateWatcherConfig,
@@ -48,8 +51,11 @@ export class OrderStateWatcher {
         this._orders = {};
         this._dependentOrderHashes = {};
         const eventPollingIntervalMs = _.isUndefined(config) ? undefined : config.pollingIntervalMs;
+        this._numConfirmations = _.isUndefined(config) ?
+                                    DEFAULT_NUM_CONFIRMATIONS
+                                    : config.numConfirmations;
         this._eventWatcher = new EventWatcher(
-            this._web3Wrapper, eventPollingIntervalMs,
+            this._web3Wrapper, eventPollingIntervalMs, this._numConfirmations,
         );
         this._abiDecoder = abiDecoder;
         this._orderStateUtils = orderStateUtils;
@@ -88,13 +94,13 @@ export class OrderStateWatcher {
      *                              is 0 will watch the backing node's mempool, 3 will emit events when blockchain
      *                              state relevant to a watched order changed 3 blocks ago.
      */
-    public subscribe(callback: OnOrderStateChangeCallback, numConfirmations: number): void {
+    public subscribe(callback: OnOrderStateChangeCallback): void {
         assert.isFunction('callback', callback);
         if (!_.isUndefined(this._callbackIfExistsAsync)) {
             throw new Error(ZeroExError.SubscriptionAlreadyPresent);
         }
         this._callbackIfExistsAsync = callback;
-        this._eventWatcher.subscribe(this._onEventWatcherCallbackAsync.bind(this), numConfirmations);
+        this._eventWatcher.subscribe(this._onEventWatcherCallbackAsync.bind(this));
     }
     /**
      * Ends an orderStateWatcher subscription.
@@ -151,8 +157,11 @@ export class OrderStateWatcher {
         }
     }
     private async _emitRevalidateOrdersAsync(orderHashes: string[]): Promise<void> {
+        const defaultBlock = this._numConfirmations === 0 ?
+                                BlockParamLiteral.Pending :
+                                this._numConfirmations;
         const methodOpts = {
-            defaultBlock: BlockParamLiteral.Pending,
+            defaultBlock,
         };
 
         for (const orderHash of orderHashes) {
diff --git a/src/schemas/zero_ex_config_schema.ts b/src/schemas/zero_ex_config_schema.ts
index 5a2afeaa2..6d4b3ed27 100644
--- a/src/schemas/zero_ex_config_schema.ts
+++ b/src/schemas/zero_ex_config_schema.ts
@@ -12,6 +12,10 @@ export const zeroExConfigSchema = {
                     type: 'number',
                     minimum: 0,
                 },
+                numConfirmations: {
+                    type: 'number',
+                    minimum: 0,
+                },
             },
         },
     },
diff --git a/src/types.ts b/src/types.ts
index 79def4560..352818bc2 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -400,6 +400,7 @@ export interface JSONRPCPayload {
  */
 export interface OrderStateWatcherConfig {
     pollingIntervalMs?: number;
+    numConfirmations: number;
 }
 
 /*
diff --git a/test/order_state_watcher_test.ts b/test/order_state_watcher_test.ts
index 6060d64c5..8eb19dcef 100644
--- a/test/order_state_watcher_test.ts
+++ b/test/order_state_watcher_test.ts
@@ -88,9 +88,9 @@ describe('OrderStateWatcher', () => {
             zeroEx.orderStateWatcher.unsubscribe();
         });
         it('should fail when trying to subscribe twice', (done: DoneCallback) => {
-            zeroEx.orderStateWatcher.subscribe(_.noop, numConfirmations);
+            zeroEx.orderStateWatcher.subscribe(_.noop);
             try {
-                zeroEx.orderStateWatcher.subscribe(_.noop, numConfirmations);
+                zeroEx.orderStateWatcher.subscribe(_.noop);
                 done(new Error('Expected the second subscription to fail'));
             } catch (err) {
                 done();
@@ -117,7 +117,7 @@ describe('OrderStateWatcher', () => {
                     expect(invalidOrderState.error).to.be.equal(ExchangeContractErrs.InsufficientMakerAllowance);
                     done();
                 };
-                zeroEx.orderStateWatcher.subscribe(callback, numConfirmations);
+                zeroEx.orderStateWatcher.subscribe(callback);
                 await zeroEx.token.setProxyAllowanceAsync(makerToken.address, maker, new BigNumber(0));
             })().catch(done);
         });
@@ -135,7 +135,7 @@ describe('OrderStateWatcher', () => {
                     expect(invalidOrderState.error).to.be.equal(ExchangeContractErrs.InsufficientMakerBalance);
                     done();
                 };
-                zeroEx.orderStateWatcher.subscribe(callback, numConfirmations);
+                zeroEx.orderStateWatcher.subscribe(callback);
                 const anyRecipient = taker;
                 const makerBalance = await zeroEx.token.getBalanceAsync(makerToken.address, maker);
                 await zeroEx.token.transferAsync(makerToken.address, maker, anyRecipient, makerBalance);
@@ -160,7 +160,7 @@ describe('OrderStateWatcher', () => {
                         done();
                     }
                 };
-                zeroEx.orderStateWatcher.subscribe(callback, numConfirmations);
+                zeroEx.orderStateWatcher.subscribe(callback);
 
                 const shouldThrowOnInsufficientBalanceOrAllowance = true;
                 await zeroEx.exchange.fillOrderAsync(
@@ -194,7 +194,7 @@ describe('OrderStateWatcher', () => {
                         done();
                     }
                 };
-                zeroEx.orderStateWatcher.subscribe(callback, numConfirmations);
+                zeroEx.orderStateWatcher.subscribe(callback);
                 const shouldThrowOnInsufficientBalanceOrAllowance = true;
                 await zeroEx.exchange.fillOrderAsync(
                     signedOrder, fillAmountInBaseUnits, shouldThrowOnInsufficientBalanceOrAllowance, taker,