From f2857452e3cd6a6bbfd444798212ce33d12386e8 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 13 Mar 2019 22:41:35 +0100 Subject: [PATCH 1/3] Fix error were object could be undefined given an approval event --- packages/order-watcher/CHANGELOG.json | 3 +++ .../order_watcher/dependent_order_hashes_tracker.ts | 12 +++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/order-watcher/CHANGELOG.json b/packages/order-watcher/CHANGELOG.json index 8cff90d24e..ef18c9e5ca 100644 --- a/packages/order-watcher/CHANGELOG.json +++ b/packages/order-watcher/CHANGELOG.json @@ -5,6 +5,9 @@ { "note": "Update websocket from ^1.0.25 to ^1.0.26", "pr": 1685 + }, + { + "note": "Fix issue where ERC721 Approval events could cause a lookup on undefined object" } ] }, diff --git a/packages/order-watcher/src/order_watcher/dependent_order_hashes_tracker.ts b/packages/order-watcher/src/order_watcher/dependent_order_hashes_tracker.ts index d1085014c0..b8ff897a12 100644 --- a/packages/order-watcher/src/order_watcher/dependent_order_hashes_tracker.ts +++ b/packages/order-watcher/src/order_watcher/dependent_order_hashes_tracker.ts @@ -37,9 +37,15 @@ export class DependentOrderHashesTracker { this._zrxTokenAddress = zrxTokenAddress; } public getDependentOrderHashesByERC721ByMaker(makerAddress: string, tokenAddress: string): string[] { - const orderHashSets = _.values( - this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress][tokenAddress], - ); + let orderHashSets: Array> = []; + if ( + this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress] && + this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress][tokenAddress] + ) { + orderHashSets = _.values( + this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress][tokenAddress], + ); + } const orderHashList = _.reduce( orderHashSets, (accumulator, orderHashSet) => [...accumulator, ...orderHashSet], From f57f29e4266ec11a9dc8ba3bb558abd420c00135 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Wed, 13 Mar 2019 22:43:45 +0100 Subject: [PATCH 2/3] Update changelog --- packages/order-watcher/CHANGELOG.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/order-watcher/CHANGELOG.json b/packages/order-watcher/CHANGELOG.json index ef18c9e5ca..8d4e22944a 100644 --- a/packages/order-watcher/CHANGELOG.json +++ b/packages/order-watcher/CHANGELOG.json @@ -7,7 +7,8 @@ "pr": 1685 }, { - "note": "Fix issue where ERC721 Approval events could cause a lookup on undefined object" + "note": "Fix issue where ERC721 Approval events could cause a lookup on undefined object", + "pr": 1692 } ] }, From e28c6d6f9c46da9641fcd5dda15106e6d7684d75 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Thu, 14 Mar 2019 10:45:20 +0100 Subject: [PATCH 3/3] Add a regression test for #1550 --- .../order_watcher/dependent_order_hashes_tracker.ts | 12 ++++++------ packages/order-watcher/test/order_watcher_test.ts | 13 +++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/order-watcher/src/order_watcher/dependent_order_hashes_tracker.ts b/packages/order-watcher/src/order_watcher/dependent_order_hashes_tracker.ts index b8ff897a12..4a2580fdd2 100644 --- a/packages/order-watcher/src/order_watcher/dependent_order_hashes_tracker.ts +++ b/packages/order-watcher/src/order_watcher/dependent_order_hashes_tracker.ts @@ -37,15 +37,15 @@ export class DependentOrderHashesTracker { this._zrxTokenAddress = zrxTokenAddress; } public getDependentOrderHashesByERC721ByMaker(makerAddress: string, tokenAddress: string): string[] { - let orderHashSets: Array> = []; if ( - this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress] && - this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress][tokenAddress] + _.isUndefined(this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress]) || + _.isUndefined(this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress][tokenAddress]) ) { - orderHashSets = _.values( - this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress][tokenAddress], - ); + return []; } + const orderHashSets = _.values( + this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress][tokenAddress], + ); const orderHashList = _.reduce( orderHashSets, (accumulator, orderHashSet) => [...accumulator, ...orderHashSet], diff --git a/packages/order-watcher/test/order_watcher_test.ts b/packages/order-watcher/test/order_watcher_test.ts index 28b564b325..d272c7fa45 100644 --- a/packages/order-watcher/test/order_watcher_test.ts +++ b/packages/order-watcher/test/order_watcher_test.ts @@ -90,6 +90,19 @@ describe('OrderWatcher', () => { afterEach(async () => { await blockchainLifecycle.revertAsync(); }); + describe('DependentOrderHashesTracker', async () => { + let makerErc721TokenAddress: string; + [makerErc721TokenAddress] = tokenUtils.getDummyERC721TokenAddresses(); + it('should handle lookups on unknown addresses', async () => { + // Regression test + // ApprovalForAll events on a token from an untracked address could cause + // nested lookups on undefined object + // #1550 + const dependentOrderHashesTracker = (orderWatcher as any) + ._dependentOrderHashesTracker as DependentOrderHashesTracker; + dependentOrderHashesTracker.getDependentOrderHashesByERC721ByMaker(takerAddress, makerErc721TokenAddress); + }); + }); describe('#removeOrder', async () => { it('should successfully remove existing order', async () => { signedOrder = await fillScenarios.createFillableSignedOrderAsync(