improve(BundleDataClient,SpokePoolClient): Log about duplicate events and delete getLatestProposedBundleData

src/clients/BundleDataClient/BundleDataClient.ts

@@ -18,7 +18,7 @@ import {
   Deposit,
   DepositWithBlock,
 } from "../../interfaces";
-import { AcrossConfigStoreClient, SpokePoolClient } from "..";
+import { SpokePoolClient } from "..";
 import {
   BigNumber,
   bnZero,
@@ -41,18 +41,17 @@ import {
   isZeroValueFillOrSlowFillRequest,
   chainIsEvm,
   isValidEvmAddress,
+  duplicateEvent,
 } from "../../utils";
 import winston from "winston";
 import {
-  _buildPoolRebalanceRoot,
   BundleData,
   BundleDataSS,
   getEndBlockBuffers,
   getRefundInformationFromFill,
   getRefundsFromBundle,
   getWidestPossibleExpectedBlockRange,
   isChainDisabled,
-  PoolRebalanceRoot,
   prettyPrintV3SpokePoolEvents,
   V3DepositWithBlock,
   V3FillWithBlock,
@@ -240,13 +239,7 @@ export class BundleDataClient {
     );
   }
 
-  private async loadPersistedDataFromArweave(
-    blockRangesForChains: number[][]
-  ): Promise<LoadDataReturnValue | undefined> {
-    if (!isDefined(this.clients?.arweaveClient)) {
-      return undefined;
-    }
-    const start = performance.now();
+  private async getBundleDataFromArweave(blockRangesForChains: number[][]) {
     const persistedData = await this.clients.arweaveClient.getByTopic(
       this.getArweaveBundleDataClientKey(blockRangesForChains),
       BundleDataSS
@@ -256,6 +249,20 @@ export class BundleDataClient {
     if (!isDefined(persistedData) || persistedData.length < 1) {
       return undefined;
     }
+    return persistedData;
+  }
+
+  private async loadPersistedDataFromArweave(
+    blockRangesForChains: number[][]
+  ): Promise<LoadDataReturnValue | undefined> {
+    if (!isDefined(this.clients?.arweaveClient)) {
+      return undefined;
+    }
+    const start = performance.now();
+    const persistedData = await this.getBundleDataFromArweave(blockRangesForChains);
+    if (!isDefined(persistedData)) {
+      return undefined;
+    }
 
     // A converter function to account for the fact that our SuperStruct schema does not support numeric
     // keys in records. Fundamentally, this is a limitation of superstruct itself.
@@ -431,52 +438,6 @@ export class BundleDataClient {
       }, toBN(0));
   }
 
-  private async getLatestProposedBundleData(): Promise<{ bundleData: LoadDataReturnValue; blockRanges: number[][] }> {
-    const hubPoolClient = this.clients.hubPoolClient;
-    // Determine which bundle we should fetch from arweave, either the pending bundle or the latest
-    // executed one. Both should have arweave data but if for some reason the arweave data is missing,
-    // this function will have to compute the bundle data from scratch which will be slow. We have to fallback
-    // to computing the bundle from scratch since this function needs to return the full bundle data so that
-    // it can be used to get the running balance proposed using its data.
-    const bundleBlockRanges = getImpliedBundleBlockRanges(
-      hubPoolClient,
-      this.clients.configStoreClient,
-      hubPoolClient.hasPendingProposal()
-        ? hubPoolClient.getLatestProposedRootBundle()
-        : hubPoolClient.getLatestFullyExecutedRootBundle(hubPoolClient.latestBlockSearched)! // ! because we know there is a bundle
-    );
-    return {
-      blockRanges: bundleBlockRanges,
-      bundleData: await this.loadData(
-        bundleBlockRanges,
-        this.spokePoolClients,
-        true // this bundle data should have been published to arweave
-      ),
-    };
-  }
-
-  async getLatestPoolRebalanceRoot(): Promise<{ root: PoolRebalanceRoot; blockRanges: number[][] }> {
-    const { bundleData, blockRanges } = await this.getLatestProposedBundleData();
-    const hubPoolClient = this.clients.hubPoolClient;
-    const root = _buildPoolRebalanceRoot(
-      hubPoolClient.latestBlockSearched,
-      blockRanges[0][1],
-      bundleData.bundleDepositsV3,
-      bundleData.bundleFillsV3,
-      bundleData.bundleSlowFillsV3,
-      bundleData.unexecutableSlowFills,
-      bundleData.expiredDepositsToRefundV3,
-      {
-        hubPoolClient,
-        configStoreClient: hubPoolClient.configStoreClient as AcrossConfigStoreClient,
-      }
-    );
-    return {
-      root,
-      blockRanges,
-    };
-  }
-
   // @dev This function should probably be moved to the InventoryClient since it bypasses loadData completely now.
   // Return refunds from the next valid bundle. This will contain any refunds that have been sent but are not included
   // in a valid bundle with all of its leaves executed. This contains refunds from:
@@ -863,6 +824,14 @@ export class BundleDataClient {
             "Not using correct bundle deposit hash key"
           );
           if (deposit.blockNumber >= originChainBlockRange[0]) {
+            if (bundleDepositsV3?.[originChainId]?.[deposit.inputToken]?.find((d) => duplicateEvent(deposit, d))) {
+              this.logger.debug({
+                at: "BundleDataClient#loadData",
+                message: "Duplicate deposit detected",
+                deposit,
+              });
+              throw new Error("Duplicate deposit detected");
+            }
             bundleDepositHashes.push(newBundleDepositHash);
             updateBundleDepositsV3(bundleDepositsV3, deposit);
           } else if (deposit.blockNumber < originChainBlockRange[0]) {
@@ -920,64 +889,69 @@ export class BundleDataClient {
             fillCounter++;
             const relayDataHash = getRelayEventKey(fill);
             if (v3RelayHashes[relayDataHash]) {
-              if (!v3RelayHashes[relayDataHash].fill) {
-                assert(
-                  isDefined(v3RelayHashes[relayDataHash].deposits) && v3RelayHashes[relayDataHash].deposits!.length > 0,
-                  "Deposit should exist in relay hash dictionary."
+              if (v3RelayHashes[relayDataHash].fill) {
+                this.logger.debug({
+                  at: "BundleDataClient#loadData",
+                  message: "Duplicate fill detected",
+                  fill,
+                });
+                throw new Error("Duplicate fill detected");
+              }
+              assert(
+                isDefined(v3RelayHashes[relayDataHash].deposits) && v3RelayHashes[relayDataHash].deposits!.length > 0,
+                "Deposit should exist in relay hash dictionary."
+              );
+              v3RelayHashes[relayDataHash].fill = fill;
+              if (fill.blockNumber >= destinationChainBlockRange[0]) {
+                const fillToRefund = await verifyFillRepayment(
+                  fill,
+                  destinationClient.spokePool.provider,
+                  v3RelayHashes[relayDataHash].deposits![0],
+                  this.clients.hubPoolClient
                 );
-                v3RelayHashes[relayDataHash].fill = fill;
-                if (fill.blockNumber >= destinationChainBlockRange[0]) {
-                  const fillToRefund = await verifyFillRepayment(
-                    fill,
-                    destinationClient.spokePool.provider,
-                    v3RelayHashes[relayDataHash].deposits![0],
-                    this.clients.hubPoolClient
-                  );
-                  if (!isDefined(fillToRefund)) {
-                    bundleUnrepayableFillsV3.push(fill);
-                    // We don't return here yet because we still need to mark unexecutable slow fill leaves
-                    // or duplicate deposits. However, we won't issue a fast fill refund.
-                  } else {
-                    v3RelayHashes[relayDataHash].fill = fillToRefund;
-                    validatedBundleV3Fills.push({
-                      ...fillToRefund,
-                      quoteTimestamp: v3RelayHashes[relayDataHash].deposits![0].quoteTimestamp,
-                    });
-
-                    // Now that we know this deposit has been filled on-chain, identify any duplicate deposits
-                    // sent for this fill and refund them to the filler, because this value would not be paid out
-                    // otherwise. These deposits can no longer expire and get refunded as an expired deposit,
-                    // and they won't trigger a pre-fill refund because the fill is in this bundle.
-                    // Pre-fill refunds only happen when deposits are sent in this bundle and the
-                    // fill is from a prior bundle. Paying out the filler keeps the behavior consistent for how
-                    // we deal with duplicate deposits regardless if the deposit is matched with a pre-fill or
-                    // a current bundle fill.
-                    const duplicateDeposits = v3RelayHashes[relayDataHash].deposits!.slice(1);
-                    duplicateDeposits.forEach((duplicateDeposit) => {
-                      if (isSlowFill(fill)) {
-                        updateExpiredDepositsV3(expiredDepositsToRefundV3, duplicateDeposit);
-                      } else {
-                        validatedBundleV3Fills.push({
-                          ...fillToRefund,
-                          quoteTimestamp: duplicateDeposit.quoteTimestamp,
-                        });
-                      }
-                    });
-                  }
-
-                  // If fill replaced a slow fill request, then mark it as one that might have created an
-                  // unexecutable slow fill. We can't know for sure until we check the slow fill request
-                  // events.
-                  if (
-                    fill.relayExecutionInfo.fillType === FillType.ReplacedSlowFill &&
-                    _canCreateSlowFillLeaf(v3RelayHashes[relayDataHash].deposits![0])
-                  ) {
-                    fastFillsReplacingSlowFills.push(relayDataHash);
-                  }
+                if (!isDefined(fillToRefund)) {
+                  bundleUnrepayableFillsV3.push(fill);
+                  // We don't return here yet because we still need to mark unexecutable slow fill leaves
+                  // or duplicate deposits. However, we won't issue a fast fill refund.
+                } else {
+                  v3RelayHashes[relayDataHash].fill = fillToRefund;
+                  validatedBundleV3Fills.push({
+                    ...fillToRefund,
+                    quoteTimestamp: v3RelayHashes[relayDataHash].deposits![0].quoteTimestamp,
+                  });
+
+                  // Now that we know this deposit has been filled on-chain, identify any duplicate deposits
+                  // sent for this fill and refund them to the filler, because this value would not be paid out
+                  // otherwise. These deposits can no longer expire and get refunded as an expired deposit,
+                  // and they won't trigger a pre-fill refund because the fill is in this bundle.
+                  // Pre-fill refunds only happen when deposits are sent in this bundle and the
+                  // fill is from a prior bundle. Paying out the filler keeps the behavior consistent for how
+                  // we deal with duplicate deposits regardless if the deposit is matched with a pre-fill or
+                  // a current bundle fill.
+                  const duplicateDeposits = v3RelayHashes[relayDataHash].deposits!.slice(1);
+                  duplicateDeposits.forEach((duplicateDeposit) => {
+                    if (isSlowFill(fill)) {
+                      updateExpiredDepositsV3(expiredDepositsToRefundV3, duplicateDeposit);
+                    } else {
+                      validatedBundleV3Fills.push({
+                        ...fillToRefund,
+                        quoteTimestamp: duplicateDeposit.quoteTimestamp,
+                      });
+                    }
+                  });
+                }
+
+                // If fill replaced a slow fill request, then mark it as one that might have created an
+                // unexecutable slow fill. We can't know for sure until we check the slow fill request
+                // events.
+                if (
+                  fill.relayExecutionInfo.fillType === FillType.ReplacedSlowFill &&
+                  _canCreateSlowFillLeaf(v3RelayHashes[relayDataHash].deposits![0])
+                ) {
+                  fastFillsReplacingSlowFills.push(relayDataHash);
                 }
-              } else {
-                throw new Error("Duplicate fill detected");
               }
+
               return;
             }
 
@@ -1077,30 +1051,34 @@ export class BundleDataClient {
             const relayDataHash = getRelayEventKey(slowFillRequest);
 
             if (v3RelayHashes[relayDataHash]) {
-              if (!v3RelayHashes[relayDataHash].slowFillRequest) {
-                v3RelayHashes[relayDataHash].slowFillRequest = slowFillRequest;
-                if (v3RelayHashes[relayDataHash].fill) {
-                  // Exiting here assumes that slow fill requests must precede fills, so if there was a fill
-                  // following this slow fill request, then we would have already seen it. We don't need to check
-                  // for a fill older than this slow fill request.
-                  return;
-                }
-                assert(
-                  isDefined(v3RelayHashes[relayDataHash].deposits) && v3RelayHashes[relayDataHash].deposits!.length > 0,
-                  "Deposit should exist in relay hash dictionary."
-                );
-                const matchedDeposit = v3RelayHashes[relayDataHash].deposits![0];
-
-                if (
-                  slowFillRequest.blockNumber >= destinationChainBlockRange[0] &&
-                  _canCreateSlowFillLeaf(matchedDeposit) &&
-                  !_depositIsExpired(matchedDeposit)
-                ) {
-                  validatedBundleSlowFills.push(matchedDeposit);
-                }
-              } else {
+              if (v3RelayHashes[relayDataHash].slowFillRequest) {
+                this.logger.debug({
+                  at: "BundleDataClient#loadData",
+                  message: "Duplicate slow fill request detected",
+                  slowFillRequest,
+                });
                 throw new Error("Duplicate slow fill request detected.");
               }
+              v3RelayHashes[relayDataHash].slowFillRequest = slowFillRequest;
+              if (v3RelayHashes[relayDataHash].fill) {
+                // Exiting here assumes that slow fill requests must precede fills, so if there was a fill
+                // following this slow fill request, then we would have already seen it. We don't need to check
+                // for a fill older than this slow fill request.
+                return;
+              }
+              assert(
+                isDefined(v3RelayHashes[relayDataHash].deposits) && v3RelayHashes[relayDataHash].deposits!.length > 0,
+                "Deposit should exist in relay hash dictionary."
+              );
+              const matchedDeposit = v3RelayHashes[relayDataHash].deposits![0];
+
+              if (
+                slowFillRequest.blockNumber >= destinationChainBlockRange[0] &&
+                _canCreateSlowFillLeaf(matchedDeposit) &&
+                !_depositIsExpired(matchedDeposit)
+              ) {
+                validatedBundleSlowFills.push(matchedDeposit);
+              }
               return;
             }
 

src/clients/HubPoolClient.ts

@@ -776,17 +776,19 @@ export class HubPoolClient extends BaseAbstractClient {
     return endBlock > 0 ? endBlock + 1 : 0;
   }
 
-  getRunningBalanceBeforeBlockForChain(block: number, chain: number, l1Token: string): TokenRunningBalance {
+  getLatestExecutedRootBundleContainingL1Token(block: number, chain: number, l1Token: string): ExecutedRootBundle {
     // Search ExecutedRootBundles in descending block order to find the most recent event before the target block.
-    const executedRootBundle = sortEventsDescending(this.executedRootBundles).find(
-      (executedLeaf: ExecutedRootBundle) => {
-        return (
-          executedLeaf.blockNumber <= block &&
-          executedLeaf.chainId === chain &&
-          executedLeaf.l1Tokens.map((l1Token) => l1Token.toLowerCase()).includes(l1Token.toLowerCase())
-        );
-      }
-    ) as ExecutedRootBundle;
+    return sortEventsDescending(this.executedRootBundles).find((executedLeaf: ExecutedRootBundle) => {
+      return (
+        executedLeaf.blockNumber <= block &&
+        executedLeaf.chainId === chain &&
+        executedLeaf.l1Tokens.map((l1Token) => l1Token.toLowerCase()).includes(l1Token.toLowerCase())
+      );
+    }) as ExecutedRootBundle;
+  }
+
+  getRunningBalanceBeforeBlockForChain(block: number, chain: number, l1Token: string): TokenRunningBalance {
+    const executedRootBundle = this.getLatestExecutedRootBundleContainingL1Token(block, chain, l1Token);
 
     return this.getRunningBalanceForToken(l1Token, executedRootBundle);
   }