Merge pull request #1692 from 0xProject/fix/order-watcher/1550

Fix error where object could be undefined given an approval event
0xProject · Mar 14, 2019 · b378a06 · b378a06
2 parents af04c29 + e28c6d6
commit b378a06
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 0 deletions.
diff --git a/packages/order-watcher/CHANGELOG.json b/packages/order-watcher/CHANGELOG.json
@@ -5,6 +5,10 @@
             {
                 "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",
+                "pr": 1692
             }
         ]
     },

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
@@ -37,6 +37,12 @@ export class DependentOrderHashesTracker {
         this._zrxTokenAddress = zrxTokenAddress;
     }
     public getDependentOrderHashesByERC721ByMaker(makerAddress: string, tokenAddress: string): string[] {
+        if (
+            _.isUndefined(this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress]) ||
+            _.isUndefined(this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress][tokenAddress])
+        ) {
+            return [];
+        }
         const orderHashSets = _.values(
             this._orderHashesByERC721AddressByTokenIdByMakerAddress[makerAddress][tokenAddress],
         );

diff --git 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(