Cleanup order watcher from redundant asyncs

This commit is contained in:
Leonid Logvinov
2017-11-22 12:15:31 -06:00
parent 4fe28ec53c
commit 66aaceea91
5 changed files with 34 additions and 30 deletions

View File

@@ -29,12 +29,12 @@ export class ExpirationWatcher {
const comparator = (lhs: string, rhs: string) => scoreFunction(lhs) - scoreFunction(rhs); const comparator = (lhs: string, rhs: string) => scoreFunction(lhs) - scoreFunction(rhs);
this.orderHashByExpirationRBTree = new RBTree(comparator); this.orderHashByExpirationRBTree = new RBTree(comparator);
} }
public subscribe(callbackAsync: (orderHash: string) => Promise<void>): void { public subscribe(callback: (orderHash: string) => void): void {
if (!_.isUndefined(this.orderExpirationCheckingIntervalIdIfExists)) { if (!_.isUndefined(this.orderExpirationCheckingIntervalIdIfExists)) {
throw new Error(ZeroExError.SubscriptionAlreadyPresent); throw new Error(ZeroExError.SubscriptionAlreadyPresent);
} }
this.orderExpirationCheckingIntervalIdIfExists = intervalUtils.setAsyncExcludingInterval( this.orderExpirationCheckingIntervalIdIfExists = intervalUtils.setAsyncExcludingInterval(
this.pruneExpiredOrdersAsync.bind(this, callbackAsync), this.orderExpirationCheckingIntervalMs, this.pruneExpiredOrders.bind(this, callback), this.orderExpirationCheckingIntervalMs,
); );
} }
public unsubscribe(): void { public unsubscribe(): void {
@@ -52,7 +52,7 @@ export class ExpirationWatcher {
this.orderHashByExpirationRBTree.remove(orderHash); this.orderHashByExpirationRBTree.remove(orderHash);
delete this.expiration[orderHash]; delete this.expiration[orderHash];
} }
private async pruneExpiredOrdersAsync(callbackAsync: (orderHash: string) => Promise<void>): Promise<void> { private pruneExpiredOrders(callback: (orderHash: string) => void): void {
const currentUnixTimestampMs = utils.getCurrentUnixTimestampMs(); const currentUnixTimestampMs = utils.getCurrentUnixTimestampMs();
while (true) { while (true) {
const hasTrakedOrders = this.orderHashByExpirationRBTree.size === 0; const hasTrakedOrders = this.orderHashByExpirationRBTree.size === 0;
@@ -70,7 +70,7 @@ export class ExpirationWatcher {
const orderHash = this.orderHashByExpirationRBTree.min(); const orderHash = this.orderHashByExpirationRBTree.min();
this.orderHashByExpirationRBTree.remove(orderHash); this.orderHashByExpirationRBTree.remove(orderHash);
delete this.expiration[orderHash]; delete this.expiration[orderHash];
await callbackAsync(orderHash); callback(orderHash);
} }
} }
} }

View File

@@ -94,12 +94,12 @@ export class OrderStateWatcher {
* signature is verified. * signature is verified.
* @param signedOrder The order you wish to start watching. * @param signedOrder The order you wish to start watching.
*/ */
public async addOrderAsync(signedOrder: SignedOrder): Promise<void> { public addOrder(signedOrder: SignedOrder): void {
assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema); assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema);
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
assert.isValidSignature(orderHash, signedOrder.ecSignature, signedOrder.maker); assert.isValidSignature(orderHash, signedOrder.ecSignature, signedOrder.maker);
this._orderByOrderHash[orderHash] = signedOrder; this._orderByOrderHash[orderHash] = signedOrder;
await this.addToDependentOrderHashesAsync(signedOrder, orderHash); this.addToDependentOrderHashes(signedOrder, orderHash);
const expirationUnixTimestampMs = signedOrder.expirationUnixTimestampSec.times(1000); const expirationUnixTimestampMs = signedOrder.expirationUnixTimestampSec.times(1000);
this._expirationWatcher.addOrder(orderHash, expirationUnixTimestampMs); this._expirationWatcher.addOrder(orderHash, expirationUnixTimestampMs);
} }
@@ -107,7 +107,7 @@ export class OrderStateWatcher {
* Removes an order from the orderStateWatcher * Removes an order from the orderStateWatcher
* @param orderHash The orderHash of the order you wish to stop watching. * @param orderHash The orderHash of the order you wish to stop watching.
*/ */
public async removeOrderAsync(orderHash: string): Promise<void> { public removeOrder(orderHash: string): void {
assert.doesConformToSchema('orderHash', orderHash, schemas.orderHashSchema); assert.doesConformToSchema('orderHash', orderHash, schemas.orderHashSchema);
const signedOrder = this._orderByOrderHash[orderHash]; const signedOrder = this._orderByOrderHash[orderHash];
if (_.isUndefined(signedOrder)) { if (_.isUndefined(signedOrder)) {
@@ -134,7 +134,7 @@ export class OrderStateWatcher {
} }
this._callbackIfExists = callback; this._callbackIfExists = callback;
this._eventWatcher.subscribe(this._onEventWatcherCallbackAsync.bind(this)); this._eventWatcher.subscribe(this._onEventWatcherCallbackAsync.bind(this));
this._expirationWatcher.subscribe(this._onOrderExpiredAsync.bind(this)); this._expirationWatcher.subscribe(this._onOrderExpired.bind(this));
} }
/** /**
* Ends an orderStateWatcher subscription. * Ends an orderStateWatcher subscription.
@@ -149,14 +149,14 @@ export class OrderStateWatcher {
this._eventWatcher.unsubscribe(); this._eventWatcher.unsubscribe();
this._expirationWatcher.unsubscribe(); this._expirationWatcher.unsubscribe();
} }
private async _onOrderExpiredAsync(orderHash: string): Promise<void> { private _onOrderExpired(orderHash: string): void {
const orderState: OrderState = { const orderState: OrderState = {
isValid: false, isValid: false,
orderHash, orderHash,
error: ExchangeContractErrs.OrderFillExpired, error: ExchangeContractErrs.OrderFillExpired,
}; };
if (!_.isUndefined(this._orderByOrderHash[orderHash])) { if (!_.isUndefined(this._orderByOrderHash[orderHash])) {
await this.removeOrderAsync(orderHash); this.removeOrder(orderHash);
if (!_.isUndefined(this._callbackIfExists)) { if (!_.isUndefined(this._callbackIfExists)) {
this._callbackIfExists(orderState); this._callbackIfExists(orderState);
} }
@@ -254,7 +254,7 @@ export class OrderStateWatcher {
this._callbackIfExists(orderState); this._callbackIfExists(orderState);
} }
} }
private async addToDependentOrderHashesAsync(signedOrder: SignedOrder, orderHash: string): Promise<void> { private addToDependentOrderHashes(signedOrder: SignedOrder, orderHash: string): void {
if (_.isUndefined(this._dependentOrderHashes[signedOrder.maker])) { if (_.isUndefined(this._dependentOrderHashes[signedOrder.maker])) {
this._dependentOrderHashes[signedOrder.maker] = {}; this._dependentOrderHashes[signedOrder.maker] = {};
} }

View File

@@ -7,6 +7,7 @@ import BigNumber from 'bignumber.js';
import {chaiSetup} from './utils/chai_setup'; import {chaiSetup} from './utils/chai_setup';
import {web3Factory} from './utils/web3_factory'; import {web3Factory} from './utils/web3_factory';
import {utils} from '../src/utils/utils'; import {utils} from '../src/utils/utils';
import {constants} from '../src/utils/constants';
import {Web3Wrapper} from '../src/web3_wrapper'; import {Web3Wrapper} from '../src/web3_wrapper';
import {TokenUtils} from './utils/token_utils'; import {TokenUtils} from './utils/token_utils';
import {ExpirationWatcher} from '../src/order_watcher/expiration_watcher'; import {ExpirationWatcher} from '../src/order_watcher/expiration_watcher';
@@ -41,8 +42,11 @@ describe('ExpirationWatcher', () => {
let expirationWatcher: ExpirationWatcher; let expirationWatcher: ExpirationWatcher;
before(async () => { before(async () => {
web3 = web3Factory.create(); web3 = web3Factory.create();
zeroEx = new ZeroEx(web3.currentProvider); const config = {
exchangeContractAddress = await zeroEx.exchange.getContractAddressAsync(); networkId: constants.TESTRPC_NETWORK_ID,
};
zeroEx = new ZeroEx(web3.currentProvider, config);
exchangeContractAddress = await zeroEx.exchange.getContractAddress();
userAddresses = await zeroEx.getAvailableAddressesAsync(); userAddresses = await zeroEx.getAvailableAddressesAsync();
tokens = await zeroEx.tokenRegistry.getTokensAsync(); tokens = await zeroEx.tokenRegistry.getTokensAsync();
tokenUtils = new TokenUtils(tokens); tokenUtils = new TokenUtils(tokens);

View File

@@ -78,13 +78,13 @@ describe('OrderStateWatcher', () => {
makerToken.address, takerToken.address, maker, taker, fillableAmount, makerToken.address, takerToken.address, maker, taker, fillableAmount,
); );
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
expect((zeroEx.orderStateWatcher as any)._orderByOrderHash).to.include({ expect((zeroEx.orderStateWatcher as any)._orderByOrderHash).to.include({
[orderHash]: signedOrder, [orderHash]: signedOrder,
}); });
let dependentOrderHashes = (zeroEx.orderStateWatcher as any)._dependentOrderHashes; let dependentOrderHashes = (zeroEx.orderStateWatcher as any)._dependentOrderHashes;
expect(dependentOrderHashes[signedOrder.maker][signedOrder.makerTokenAddress]).to.have.keys(orderHash); expect(dependentOrderHashes[signedOrder.maker][signedOrder.makerTokenAddress]).to.have.keys(orderHash);
await zeroEx.orderStateWatcher.removeOrderAsync(orderHash); zeroEx.orderStateWatcher.removeOrder(orderHash);
expect((zeroEx.orderStateWatcher as any)._orderByOrderHash).to.not.include({ expect((zeroEx.orderStateWatcher as any)._orderByOrderHash).to.not.include({
[orderHash]: signedOrder, [orderHash]: signedOrder,
}); });
@@ -97,7 +97,7 @@ describe('OrderStateWatcher', () => {
); );
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
const nonExistentOrderHash = `0x${orderHash.substr(2).split('').reverse().join('')}`; const nonExistentOrderHash = `0x${orderHash.substr(2).split('').reverse().join('')}`;
await zeroEx.orderStateWatcher.removeOrderAsync(nonExistentOrderHash); zeroEx.orderStateWatcher.removeOrder(nonExistentOrderHash);
}); });
}); });
describe('#subscribe', async () => { describe('#subscribe', async () => {
@@ -114,7 +114,7 @@ describe('OrderStateWatcher', () => {
afterEach(async () => { afterEach(async () => {
zeroEx.orderStateWatcher.unsubscribe(); zeroEx.orderStateWatcher.unsubscribe();
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.removeOrderAsync(orderHash); zeroEx.orderStateWatcher.removeOrder(orderHash);
}); });
it('should emit orderStateInvalid when maker allowance set to 0 for watched order', (done: DoneCallback) => { it('should emit orderStateInvalid when maker allowance set to 0 for watched order', (done: DoneCallback) => {
(async () => { (async () => {
@@ -122,7 +122,7 @@ describe('OrderStateWatcher', () => {
makerToken.address, takerToken.address, maker, taker, fillableAmount, makerToken.address, takerToken.address, maker, taker, fillableAmount,
); );
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
expect(orderState.isValid).to.be.false(); expect(orderState.isValid).to.be.false();
const invalidOrderState = orderState as OrderStateInvalid; const invalidOrderState = orderState as OrderStateInvalid;
@@ -140,7 +140,7 @@ describe('OrderStateWatcher', () => {
makerToken.address, takerToken.address, maker, taker, fillableAmount, makerToken.address, takerToken.address, maker, taker, fillableAmount,
); );
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
throw new Error('OrderState callback fired for irrelevant order'); throw new Error('OrderState callback fired for irrelevant order');
}); });
@@ -161,7 +161,7 @@ describe('OrderStateWatcher', () => {
makerToken.address, takerToken.address, maker, taker, fillableAmount, makerToken.address, takerToken.address, maker, taker, fillableAmount,
); );
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
expect(orderState.isValid).to.be.false(); expect(orderState.isValid).to.be.false();
const invalidOrderState = orderState as OrderStateInvalid; const invalidOrderState = orderState as OrderStateInvalid;
@@ -181,7 +181,7 @@ describe('OrderStateWatcher', () => {
makerToken.address, takerToken.address, maker, taker, fillableAmount, makerToken.address, takerToken.address, maker, taker, fillableAmount,
); );
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
let eventCount = 0; let eventCount = 0;
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
@@ -213,7 +213,7 @@ describe('OrderStateWatcher', () => {
const fillAmountInBaseUnits = new BigNumber(2); const fillAmountInBaseUnits = new BigNumber(2);
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
let eventCount = 0; let eventCount = 0;
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
@@ -248,7 +248,7 @@ describe('OrderStateWatcher', () => {
makerToken.address, takerToken.address, makerFee, takerFee, maker, taker, fillableAmount, makerToken.address, takerToken.address, makerFee, takerFee, maker, taker, fillableAmount,
taker); taker);
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
done(); done();
}); });
@@ -269,7 +269,7 @@ describe('OrderStateWatcher', () => {
const takerBalance = await zeroEx.token.getBalanceAsync(makerToken.address, taker); const takerBalance = await zeroEx.token.getBalanceAsync(makerToken.address, taker);
const fillAmountInBaseUnits = ZeroEx.toBaseUnitAmount(new BigNumber(2), decimals); const fillAmountInBaseUnits = ZeroEx.toBaseUnitAmount(new BigNumber(2), decimals);
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
let eventCount = 0; let eventCount = 0;
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
eventCount++; eventCount++;
@@ -301,7 +301,7 @@ describe('OrderStateWatcher', () => {
const makerBalance = await zeroEx.token.getBalanceAsync(makerToken.address, maker); const makerBalance = await zeroEx.token.getBalanceAsync(makerToken.address, maker);
const changedMakerApprovalAmount = ZeroEx.toBaseUnitAmount(new BigNumber(3), decimals); const changedMakerApprovalAmount = ZeroEx.toBaseUnitAmount(new BigNumber(3), decimals);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
const validOrderState = orderState as OrderStateValid; const validOrderState = orderState as OrderStateValid;
@@ -326,7 +326,7 @@ describe('OrderStateWatcher', () => {
const remainingAmount = ZeroEx.toBaseUnitAmount(new BigNumber(1), decimals); const remainingAmount = ZeroEx.toBaseUnitAmount(new BigNumber(1), decimals);
const transferAmount = makerBalance.sub(remainingAmount); const transferAmount = makerBalance.sub(remainingAmount);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
expect(orderState.isValid).to.be.true(); expect(orderState.isValid).to.be.true();
@@ -432,7 +432,7 @@ describe('OrderStateWatcher', () => {
makerToken.address, takerToken.address, maker, taker, fillableAmount, makerToken.address, takerToken.address, maker, taker, fillableAmount,
); );
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
expect(orderState.isValid).to.be.false(); expect(orderState.isValid).to.be.false();
@@ -454,7 +454,7 @@ describe('OrderStateWatcher', () => {
makerToken.address, takerToken.address, maker, taker, fillableAmount, makerToken.address, takerToken.address, maker, taker, fillableAmount,
); );
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
expect(orderState.isValid).to.be.false(); expect(orderState.isValid).to.be.false();
@@ -480,7 +480,7 @@ describe('OrderStateWatcher', () => {
const cancelAmountInBaseUnits = new BigNumber(2); const cancelAmountInBaseUnits = new BigNumber(2);
const orderHash = ZeroEx.getOrderHashHex(signedOrder); const orderHash = ZeroEx.getOrderHashHex(signedOrder);
await zeroEx.orderStateWatcher.addOrderAsync(signedOrder); zeroEx.orderStateWatcher.addOrder(signedOrder);
const callback = reportCallbackErrors(done)((orderState: OrderState) => { const callback = reportCallbackErrors(done)((orderState: OrderState) => {
expect(orderState.isValid).to.be.true(); expect(orderState.isValid).to.be.true();

View File

@@ -41,7 +41,7 @@ describe('OrderValidation', () => {
before(async () => { before(async () => {
web3 = web3Factory.create(); web3 = web3Factory.create();
zeroEx = new ZeroEx(web3.currentProvider, config); zeroEx = new ZeroEx(web3.currentProvider, config);
exchangeContractAddress = await zeroEx.exchange.getContractAddressAsync(); exchangeContractAddress = zeroEx.exchange.getContractAddress();
userAddresses = await zeroEx.getAvailableAddressesAsync(); userAddresses = await zeroEx.getAvailableAddressesAsync();
[coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses; [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses;
tokens = await zeroEx.tokenRegistry.getTokensAsync(); tokens = await zeroEx.tokenRegistry.getTokensAsync();