From 47bc8a6af77f030fef66cd13a41d72d2bd798e76 Mon Sep 17 00:00:00 2001
From: nicholaspai <9457025+nicholaspai@users.noreply.github.com>
Date: Thu, 3 Aug 2023 18:09:52 -0400
Subject: [PATCH] improve(ProviderUtils): Add filtered key to eth_getLogs
 (#850)

* improve(ProviderUtils): Add filtered key to eth_getLogs

We've seen QuickNode add in a `transactionLogIndex` field which isn't in the spec and is causing quorum agreement issues

* Refactor into ObjectUtils

* Update ObjectUtils.ts
---
 src/utils/ObjectUtils.ts   | 34 ++++++++++++++++++++++++++++++++++
 src/utils/ProviderUtils.ts | 34 +++++++---------------------------
 2 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/src/utils/ObjectUtils.ts b/src/utils/ObjectUtils.ts
index 2e7918721..f340dcc21 100644
--- a/src/utils/ObjectUtils.ts
+++ b/src/utils/ObjectUtils.ts
@@ -2,6 +2,10 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 // Append value along the keyPath to object. For example assign(deposits, ['1337', '31337'], [{depositId:1}]) will create
 // deposits = {1337:{31337:[{depositId:1}]}}. Note that if the path into the object exists then this will append. This
+
+import lodash from "lodash";
+import { isDefined } from "./TypeGuards";
+
 // function respects the destination type; if it is an object then deep merge and if an array effectively will push.
 export function assign(obj: any, keyPath: any[], value: any): void {
   const lastKeyIndex = keyPath.length - 1;
@@ -66,3 +70,33 @@ export function groupObjectCountsByProp(objects: any[], getProp: (obj: any) => s
     return result;
   }, {});
 }
+
+/**
+ * Deletes keys from an object and returns new copy of object without ignored keys
+ * @param ignoredKeys
+ * @param obj
+ * @returns Objects with ignored keys removed
+ */
+function deleteIgnoredKeys(ignoredKeys: string[], obj: any) {
+  if (!isDefined(obj)) {
+    return;
+  }
+  const newObj = { ...obj };
+  for (const key of ignoredKeys) {
+    delete newObj[key];
+  }
+  return newObj;
+}
+
+export function compareResultsAndFilterIgnoredKeys(ignoredKeys: string[], _objA: any, _objB: any): boolean {
+  // Deep copy objects so we don't mutate original inputs.
+  const objA = { ..._objA };
+  const objB = { ..._objB };
+
+  // Remove ignored keys from objects and store them in ignoredMappings.
+  const newObjA = deleteIgnoredKeys(ignoredKeys, objA);
+  const newObjB = deleteIgnoredKeys(ignoredKeys, objB);
+
+  // Compare objects without the ignored keys.
+  return lodash.isEqual(newObjA, newObjB);
+}
diff --git a/src/utils/ProviderUtils.ts b/src/utils/ProviderUtils.ts
index 39fa5011c..14c0b0224 100644
--- a/src/utils/ProviderUtils.ts
+++ b/src/utils/ProviderUtils.ts
@@ -12,6 +12,7 @@ import {
   BLOCK_NUMBER_TTL,
 } from "../common";
 import { delay, Logger } from "./";
+import { compareResultsAndFilterIgnoredKeys } from "./ObjectUtils";
 
 const logger = Logger;
 
@@ -81,36 +82,15 @@ function createSendErrorWithMessage(message: string, sendError: any) {
 }
 
 function compareRpcResults(method: string, rpcResultA: any, rpcResultB: any): boolean {
-  // This function mutates rpcResults and deletes the ignored keys. It returns the deleted keys that we can re-add
-  // back after we do the comparison with unignored keys. This is a faster algorithm than cloning an object but it has
-  // some side effects such as the order of keys in the rpcResults object changing.
-  const deleteIgnoredKeys = (ignoredKeys: string[], rpcResults: any) => {
-    if (!rpcResults) {
-      return;
-    }
-    const ignoredMappings = {};
-    for (const key of ignoredKeys) {
-      ignoredMappings[key] = rpcResults[key];
-      delete rpcResults[key];
-    }
-    return ignoredMappings;
-  };
-  const addIgnoredFilteredKeys = (ignoredMappings: any, rpcResults: any) => {
-    for (const [key, value] of Object.entries(ignoredMappings)) {
-      rpcResults[key] = value;
-    }
-  };
-
   if (method === "eth_getBlockByNumber") {
     // We've seen RPC's disagree on the miner field, for example when Polygon nodes updated software that
     // led alchemy and quicknode to disagree on the the miner field's value.
-    const ignoredKeys = ["miner"];
-    const ignoredMappingsA = deleteIgnoredKeys(ignoredKeys, rpcResultA);
-    const ignoredMappingsB = deleteIgnoredKeys(ignoredKeys, rpcResultB);
-    const result = lodash.isEqual(rpcResultA, rpcResultB);
-    addIgnoredFilteredKeys(ignoredMappingsA, rpcResultA);
-    addIgnoredFilteredKeys(ignoredMappingsB, rpcResultB);
-    return result;
+    return compareResultsAndFilterIgnoredKeys(["miner"], rpcResultA, rpcResultB);
+  } else if (method === "eth_getLogs") {
+    // We've seen some RPC's like QuickNode add in transactionLogIndex which isn't in the
+    // JSON RPC spec: https://ethereum.org/en/developers/docs/apis/json-rpc/#eth_getfilterchanges
+    // Additional reference: https://github.com/ethers-io/ethers.js/issues/1721
+    return compareResultsAndFilterIgnoredKeys(["transactionLogIndex"], rpcResultA, rpcResultB);
   } else {
     return lodash.isEqual(rpcResultA, rpcResultB);
   }