diff options
author | Fabio Berger <me@fabioberger.com> | 2018-04-19 10:28:18 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-04-19 10:28:18 +0800 |
commit | 4bc65faf1ab2c753da0989088852ebf7281fd7de (patch) | |
tree | 40925c2880764b02320903d4bc61d4b5cae59530 | |
parent | 8634551f53bac9769e866b9a2f6f7d0dbd203704 (diff) | |
parent | d1d26f8bf6da4d47df026f509a51d2be62cf1800 (diff) | |
download | dexon-0x-contracts-4bc65faf1ab2c753da0989088852ebf7281fd7de.tar dexon-0x-contracts-4bc65faf1ab2c753da0989088852ebf7281fd7de.tar.gz dexon-0x-contracts-4bc65faf1ab2c753da0989088852ebf7281fd7de.tar.bz2 dexon-0x-contracts-4bc65faf1ab2c753da0989088852ebf7281fd7de.tar.lz dexon-0x-contracts-4bc65faf1ab2c753da0989088852ebf7281fd7de.tar.xz dexon-0x-contracts-4bc65faf1ab2c753da0989088852ebf7281fd7de.tar.zst dexon-0x-contracts-4bc65faf1ab2c753da0989088852ebf7281fd7de.zip |
Merge pull request #526 from 0xProject/fix/expiration-watcher
Fix expiration watcher comparator
-rw-r--r-- | packages/0x.js/CHANGELOG.json | 9 | ||||
-rw-r--r-- | packages/0x.js/package.json | 2 | ||||
-rw-r--r-- | packages/0x.js/src/order_watcher/expiration_watcher.ts | 13 | ||||
-rw-r--r-- | packages/0x.js/test/0x.js_test.ts | 7 | ||||
-rw-r--r-- | packages/0x.js/test/expiration_watcher_test.ts | 39 | ||||
-rw-r--r-- | packages/0x.js/test/global_hooks.ts | 7 | ||||
-rw-r--r-- | packages/migrations/src/migration.ts | 2 |
7 files changed, 68 insertions, 11 deletions
diff --git a/packages/0x.js/CHANGELOG.json b/packages/0x.js/CHANGELOG.json index 04d115809..ef4bb1e07 100644 --- a/packages/0x.js/CHANGELOG.json +++ b/packages/0x.js/CHANGELOG.json @@ -1,5 +1,14 @@ [ { + "version": "0.36.4", + "changes": [ + { + "note": "Fixed expiration watcher comparator to handle orders with equal expiration times", + "pr": 526 + } + ] + }, + { "version": "0.36.3", "changes": [ { diff --git a/packages/0x.js/package.json b/packages/0x.js/package.json index 4b2d92240..dd4bd3f5c 100644 --- a/packages/0x.js/package.json +++ b/packages/0x.js/package.json @@ -26,7 +26,7 @@ "build:umd:prod": "NODE_ENV=production webpack", "build:commonjs": "tsc && yarn update_artifacts && copyfiles -u 2 './src/compact_artifacts/**/*.json' ./lib/src/compact_artifacts && copyfiles -u 3 './lib/src/monorepo_scripts/**/*' ./scripts", "test:commonjs": "run-s build:commonjs run_mocha", - "run_mocha": "mocha lib/test/**/*_test.js --timeout 10000 --bail --exit", + "run_mocha": "mocha lib/test/**/*_test.js lib/test/global_hooks.js --timeout 10000 --bail --exit", "manual:postpublish": "yarn build; node ./scripts/postpublish.js", "docs:stage": "yarn build && node ./scripts/stage_docs.js", "docs:json": "typedoc --excludePrivate --excludeExternals --target ES5 --json $JSON_FILE_PATH $PROJECT_FILES", diff --git a/packages/0x.js/src/order_watcher/expiration_watcher.ts b/packages/0x.js/src/order_watcher/expiration_watcher.ts index 8b306bf3b..27ec7107d 100644 --- a/packages/0x.js/src/order_watcher/expiration_watcher.ts +++ b/packages/0x.js/src/order_watcher/expiration_watcher.ts @@ -22,8 +22,17 @@ export class ExpirationWatcher { this._expirationMarginMs = expirationMarginIfExistsMs || DEFAULT_EXPIRATION_MARGIN_MS; this._orderExpirationCheckingIntervalMs = expirationMarginIfExistsMs || DEFAULT_ORDER_EXPIRATION_CHECKING_INTERVAL_MS; - const scoreFunction = (orderHash: string) => this._expiration[orderHash].toNumber(); - const comparator = (lhs: string, rhs: string) => scoreFunction(lhs) - scoreFunction(rhs); + const comparator = (lhsOrderHash: string, rhsOrderHash: string) => { + const lhsExpiration = this._expiration[lhsOrderHash].toNumber(); + const rhsExpiration = this._expiration[rhsOrderHash].toNumber(); + if (lhsExpiration !== rhsExpiration) { + return lhsExpiration - rhsExpiration; + } else { + // HACK: If two orders have identical expirations, the order in which they are emitted by the + // ExpirationWatcher does not matter, so we emit them in alphabetical order by orderHash. + return lhsOrderHash.localeCompare(rhsOrderHash); + } + }; this._orderHashByExpirationRBTree = new RBTree(comparator); } public subscribe(callback: (orderHash: string) => void): void { diff --git a/packages/0x.js/test/0x.js_test.ts b/packages/0x.js/test/0x.js_test.ts index de5a6be58..838ee7080 100644 --- a/packages/0x.js/test/0x.js_test.ts +++ b/packages/0x.js/test/0x.js_test.ts @@ -1,9 +1,4 @@ -import { Deployer } from '@0xproject/deployer'; import { BlockchainLifecycle, devConstants, web3Factory } from '@0xproject/dev-utils'; -// HACK: This dependency is optional since it is only available when run from within -// the monorepo. tslint doesn't handle optional dependencies -// tslint:disable-next-line:no-implicit-dependencies -import { runMigrationsAsync } from '@0xproject/migrations'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import * as _ from 'lodash'; @@ -15,7 +10,6 @@ import { ApprovalContractEventArgs, LogWithDecodedArgs, Order, TokenEvents, Zero import { chaiSetup } from './utils/chai_setup'; import { constants } from './utils/constants'; -import { deployer } from './utils/deployer'; import { TokenUtils } from './utils/token_utils'; import { provider, web3Wrapper } from './utils/web3_wrapper'; @@ -28,7 +22,6 @@ const SHOULD_ADD_PERSONAL_MESSAGE_PREFIX = false; describe('ZeroEx library', () => { let zeroEx: ZeroEx; before(async () => { - await runMigrationsAsync(deployer); const config = { networkId: constants.TESTRPC_NETWORK_ID, }; diff --git a/packages/0x.js/test/expiration_watcher_test.ts b/packages/0x.js/test/expiration_watcher_test.ts index 29b111fa3..1b022539a 100644 --- a/packages/0x.js/test/expiration_watcher_test.ts +++ b/packages/0x.js/test/expiration_watcher_test.ts @@ -153,4 +153,43 @@ describe('ExpirationWatcher', () => { timer.tick(order2Lifetime * 1000); })().catch(done); }); + it('emits events in correct order when expirations are equal', (done: DoneCallback) => { + (async () => { + const order1Lifetime = 60; + const order2Lifetime = 60; + const order1ExpirationUnixTimestampSec = currentUnixTimestampSec.plus(order1Lifetime); + const order2ExpirationUnixTimestampSec = currentUnixTimestampSec.plus(order2Lifetime); + const signedOrder1 = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, + takerTokenAddress, + makerAddress, + takerAddress, + fillableAmount, + order1ExpirationUnixTimestampSec, + ); + const signedOrder2 = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, + takerTokenAddress, + makerAddress, + takerAddress, + fillableAmount, + order2ExpirationUnixTimestampSec, + ); + const orderHash1 = ZeroEx.getOrderHashHex(signedOrder1); + const orderHash2 = ZeroEx.getOrderHashHex(signedOrder2); + expirationWatcher.addOrder(orderHash1, signedOrder1.expirationUnixTimestampSec.times(1000)); + expirationWatcher.addOrder(orderHash2, signedOrder2.expirationUnixTimestampSec.times(1000)); + const expirationOrder = orderHash1 < orderHash2 ? [orderHash1, orderHash2] : [orderHash2, orderHash1]; + const expectToBeCalledOnce = false; + const callbackAsync = reportNoErrorCallbackErrors(done, expectToBeCalledOnce)((hash: string) => { + const orderHash = expirationOrder.shift(); + expect(hash).to.be.equal(orderHash); + if (_.isEmpty(expirationOrder)) { + done(); + } + }); + expirationWatcher.subscribe(callbackAsync); + timer.tick(order2Lifetime * 1000); + })().catch(done); + }); }); diff --git a/packages/0x.js/test/global_hooks.ts b/packages/0x.js/test/global_hooks.ts new file mode 100644 index 000000000..e3c986524 --- /dev/null +++ b/packages/0x.js/test/global_hooks.ts @@ -0,0 +1,7 @@ +import { runMigrationsAsync } from '@0xproject/migrations'; + +import { deployer } from './utils/deployer'; + +before('migrate contracts', async () => { + await runMigrationsAsync(deployer); +}); diff --git a/packages/migrations/src/migration.ts b/packages/migrations/src/migration.ts index 4827328fc..6313efcff 100644 --- a/packages/migrations/src/migration.ts +++ b/packages/migrations/src/migration.ts @@ -69,7 +69,7 @@ export const runMigrationsAsync = async (deployer: Deployer) => { }, ); for (const token of tokenInfo) { - const totalSupply = new BigNumber(0); + const totalSupply = new BigNumber(100000000000000000000); const args = [token.name, token.symbol, token.decimals, totalSupply]; const dummyToken = await deployer.deployAsync(ContractName.DummyToken, args); await tokenReg.addToken.sendTransactionAsync( |