fix(FillUtils): verifyFillRepayment should check that repayment chain has pool rebalance route

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@across-protocol/sdk",
   "author": "UMA Team",
-  "version": "4.0.1",
+  "version": "4.0.3",
   "license": "AGPL-3.0",
   "homepage": "https://docs.across.to/reference/sdk",
   "files": [

src/clients/BundleDataClient/BundleDataClient.ts

@@ -35,7 +35,6 @@ import {
   getRelayEventKey,
   isSlowFill,
   mapAsync,
-  filterAsync,
   bnUint32Max,
   isZeroValueDeposit,
   findFillEvent,
@@ -52,7 +51,6 @@ import {
   getRefundsFromBundle,
   getWidestPossibleExpectedBlockRange,
   isChainDisabled,
-  isEvmRepaymentValid,
   PoolRebalanceRoot,
   prettyPrintV3SpokePoolEvents,
   UNDEFINED_MESSAGE_HASH,
@@ -98,7 +96,7 @@ function updateBundleFillsV3(
 ): void {
   // We shouldn't pass any unrepayable fills into this function, so we perform an extra safety check.
   assert(
-    chainIsEvm(repaymentChainId) && isEvmRepaymentValid(fill, repaymentChainId),
+    chainIsEvm(repaymentChainId) && isValidEvmAddress(fill.relayer),
     "validatedBundleV3Fills dictionary should only contain fills with valid repayment information"
   );
   if (!dict?.[repaymentChainId]?.[repaymentToken]) {
@@ -349,7 +347,7 @@ export class BundleDataClient {
       // and then query the FillStatus on-chain, but that might slow this function down too much. For now, we
       // will live with this expected inaccuracy as it should be small. The pre-fill would have to precede the deposit
       // by more than the caller's event lookback window which is expected to be unlikely.
-      const fillsToCount = await filterAsync(this.spokePoolClients[chainId].getFills(), async (fill) => {
+      const fillsToCount = this.spokePoolClients[chainId].getFills().filter((fill) => {
         if (
           fill.blockNumber < blockRanges[chainIndex][0] ||
           fill.blockNumber > blockRanges[chainIndex][1] ||
@@ -365,26 +363,20 @@ export class BundleDataClient {
         const matchingDeposit = this.spokePoolClients[fill.originChainId].getDeposit(fill.depositId);
         const hasMatchingDeposit =
           matchingDeposit !== undefined && getRelayEventKey(fill) === getRelayEventKey(matchingDeposit);
-        if (hasMatchingDeposit) {
-          const validRepayment = await verifyFillRepayment(
-            fill,
-            this.spokePoolClients[fill.destinationChainId].spokePool.provider,
-            matchingDeposit,
-            // @dev: to get valid repayment chain ID's, get all chain IDs for the bundle block range and remove
-            // disabled block ranges.
-            this.clients.configStoreClient
-              .getChainIdIndicesForBlock(blockRanges[0][1])
-              .filter((_chainId, i) => !isChainDisabled(blockRanges[i]))
-          );
-          if (!isDefined(validRepayment)) {
-            return false;
-          }
-        }
         return hasMatchingDeposit;
       });
-      fillsToCount.forEach((fill) => {
-        const matchingDeposit = this.spokePoolClients[fill.originChainId].getDeposit(fill.depositId);
+      await forEachAsync(fillsToCount, async (_fill) => {
+        const matchingDeposit = this.spokePoolClients[_fill.originChainId].getDeposit(_fill.depositId);
         assert(isDefined(matchingDeposit), "Deposit not found for fill.");
+        const fill = await verifyFillRepayment(
+          _fill,
+          this.spokePoolClients[_fill.destinationChainId].spokePool.provider,
+          matchingDeposit!,
+          this.clients.hubPoolClient
+        );
+        if (!isDefined(fill)) {
+          return;
+        }
         const { chainToSendRefundTo, repaymentToken } = getRefundInformationFromFill(
           fill,
           this.clients.hubPoolClient,
@@ -934,7 +926,7 @@ export class BundleDataClient {
                     fill,
                     destinationClient.spokePool.provider,
                     v3RelayHashes[relayDataHash].deposits![0],
-                    allChainIds
+                    this.clients.hubPoolClient
                   );
                   if (!isDefined(fillToRefund)) {
                     bundleUnrepayableFillsV3.push(fill);
@@ -1028,7 +1020,7 @@ export class BundleDataClient {
                   fill,
                   destinationClient.spokePool.provider,
                   matchedDeposit,
-                  allChainIds
+                  this.clients.hubPoolClient
                 );
                 if (!isDefined(fillToRefund)) {
                   bundleUnrepayableFillsV3.push(fill);
@@ -1199,7 +1191,7 @@ export class BundleDataClient {
                 fill,
                 destinationClient.spokePool.provider,
                 v3RelayHashes[relayDataHash].deposits![0],
-                allChainIds
+                this.clients.hubPoolClient
               );
               if (!isDefined(fillToRefund)) {
                 bundleUnrepayableFillsV3.push(fill);
@@ -1253,7 +1245,7 @@ export class BundleDataClient {
                 prefill!,
                 destinationClient.spokePool.provider,
                 deposit,
-                allChainIds
+                this.clients.hubPoolClient
               );
               if (!isDefined(verifiedFill)) {
                 bundleUnrepayableFillsV3.push(prefill!);

src/clients/BundleDataClient/utils/FillUtils.ts

@@ -1,7 +1,7 @@
 import _ from "lodash";
 import { providers } from "ethers";
 import { Deposit, DepositWithBlock, Fill, FillWithBlock } from "../../../interfaces";
-import { getBlockRangeForChain, isSlowFill, chainIsEvm, isValidEvmAddress, isDefined } from "../../../utils";
+import { getBlockRangeForChain, isSlowFill, isValidEvmAddress, isDefined, chainIsEvm } from "../../../utils";
 import { HubPoolClient } from "../../HubPoolClient";
 
 export function getRefundInformationFromFill(
@@ -19,6 +19,7 @@ export function getRefundInformationFromFill(
   let chainToSendRefundTo = isSlowFill(fill) ? fill.destinationChainId : fill.repaymentChainId;
   // If the fill is for a deposit originating from the lite chain, the repayment chain is the origin chain
   // regardless of whether it is a slow or fast fill (we ignore slow fills but this is for posterity).
+  // @note fill.repaymentChainId should already be set to originChainId but reset it to be safe.
   if (fromLiteChain) {
     chainToSendRefundTo = fill.originChainId;
   }
@@ -49,24 +50,31 @@ export function getRefundInformationFromFill(
 
 export function getRepaymentChainId(fill: Fill, matchedDeposit: Deposit): number {
   // Lite chain deposits force repayment on origin chain.
-  return matchedDeposit.fromLiteChain ? fill.originChainId : fill.repaymentChainId;
+  return matchedDeposit.fromLiteChain ? matchedDeposit.originChainId : fill.repaymentChainId;
 }
 
-export function isEvmRepaymentValid(
-  fill: Fill,
+export function forceDestinationRepayment(
   repaymentChainId: number,
-  possibleRepaymentChainIds: number[] = []
+  matchedDeposit: Deposit & { quoteBlockNumber: number },
+  hubPoolClient: HubPoolClient
 ): boolean {
-  // Slow fills don't result in repayments so they're always valid.
-  if (isSlowFill(fill)) {
-    return true;
-  }
-  // Return undefined if the requested repayment chain ID is not in a passed in set of eligible chains. This can
-  // be used by the caller to narrow the chains to those that are not disabled in the config store.
-  if (possibleRepaymentChainIds.length > 0 && !possibleRepaymentChainIds.includes(repaymentChainId)) {
+  if (!matchedDeposit.fromLiteChain) {
+    try {
+      const l1TokenCounterpart = hubPoolClient.getL1TokenForL2TokenAtBlock(
+        matchedDeposit.inputToken,
+        matchedDeposit.originChainId,
+        matchedDeposit.quoteBlockNumber
+      );
+      hubPoolClient.getL2TokenForL1TokenAtBlock(l1TokenCounterpart, repaymentChainId, matchedDeposit.quoteBlockNumber);
+      // Repayment token could be found, this is a valid repayment chain.
+      return false;
+    } catch {
+      // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler there.
+      return true;
+    }
+  } else {
     return false;
   }
-  return chainIsEvm(repaymentChainId) && isValidEvmAddress(fill.relayer);
 }
 
 // Verify that a fill sent to an EVM chain has a 20 byte address. If the fill does not, then attempt
@@ -75,20 +83,24 @@ export async function verifyFillRepayment(
   _fill: FillWithBlock,
   destinationChainProvider: providers.Provider,
   matchedDeposit: DepositWithBlock,
-  possibleRepaymentChainIds: number[] = []
+  hubPoolClient: HubPoolClient
 ): Promise<FillWithBlock | undefined> {
   const fill = _.cloneDeep(_fill);
 
-  const repaymentChainId = getRepaymentChainId(fill, matchedDeposit);
-  const validEvmRepayment = isEvmRepaymentValid(fill, repaymentChainId, possibleRepaymentChainIds);
-
-  // Case 1: Repayment chain is EVM and repayment address is valid EVM address.
-  if (validEvmRepayment) {
+  // Slow fills don't result in repayments so they're always valid.
+  if (isSlowFill(fill)) {
     return fill;
   }
-  // Case 2: Repayment chain is EVM but repayment address is not a valid EVM address. Attempt to switch repayment
-  // address to msg.sender of relay transaction.
-  else if (chainIsEvm(repaymentChainId) && !isValidEvmAddress(fill.relayer)) {
+
+  let repaymentChainId = getRepaymentChainId(fill, matchedDeposit);
+
+  // If repayment chain doesn't have a Pool Rebalance Route for the input token, then change the repayment
+  // chain to the destination chain.
+  if (forceDestinationRepayment(repaymentChainId, matchedDeposit, hubPoolClient)) {
+    repaymentChainId = fill.destinationChainId;
+  }
+
+  if (!isValidEvmAddress(fill.relayer)) {
     // TODO: Handle case where fill was sent on non-EVM chain, in which case the following call would fail
     // or return something unexpected. We'd want to return undefined here.
     const fillTransaction = await destinationChainProvider.getTransaction(fill.transactionHash);
@@ -97,14 +109,20 @@ export async function verifyFillRepayment(
     if (!isDefined(destinationRelayer) || !isValidEvmAddress(destinationRelayer)) {
       return undefined;
     }
-    // Otherwise, assume the relayer to be repaid is the msg.sender. We don't need to modify the repayment chain since
-    // the getTransaction() call would only succeed if the fill was sent on an EVM chain and therefore the msg.sender
-    // is a valid EVM address and the repayment chain is an EVM chain.
+    if (!matchedDeposit.fromLiteChain) {
+      repaymentChainId = fill.destinationChainId;
+    } else {
+      // We can't switch repayment chain for a lite chain deposit so just check whether the repayment chain,
+      // which should be the origin chain, is an EVM chain.
+      if (!chainIsEvm(repaymentChainId)) {
+        return undefined;
+      }
+    }
     fill.relayer = destinationRelayer;
-    return fill;
-  }
-  // Case 3: Repayment chain is not an EVM chain, must be invalid.
-  else {
-    return undefined;
   }
+
+  // Repayment address is now valid and repayment chain is either origin chain for lite chain or the destination
+  // chain for cases where the repayment address was invalid. Fill should be valid now.
+  fill.repaymentChainId = repaymentChainId;
+  return fill;
 }

src/clients/SpokePoolClient.ts

@@ -15,6 +15,8 @@ import {
   bnOne,
   getMessageHash,
   isUnsafeDepositId,
+  isSlowFill,
+  isValidEvmAddress,
   isZeroAddress,
 } from "../utils";
 import {
@@ -45,7 +47,7 @@ import { getBlockRangeForDepositId, getDepositIdAtBlock, relayFillStatus } from
 import { BaseAbstractClient, isUpdateFailureReason, UpdateFailureReason } from "./BaseAbstractClient";
 import { HubPoolClient } from "./HubPoolClient";
 import { AcrossConfigStoreClient } from "./AcrossConfigStoreClient";
-import { getRepaymentChainId, isEvmRepaymentValid } from "./BundleDataClient/utils/FillUtils";
+import { getRepaymentChainId, forceDestinationRepayment } from "./BundleDataClient/utils/FillUtils";
 
 type SpokePoolUpdateSuccess = {
   success: true;
@@ -389,14 +391,22 @@ export class SpokePoolClient extends BaseAbstractClient {
       (groupedFills: { validFills: Fill[]; invalidFills: Fill[]; unrepayableFills: Fill[] }, fill: Fill) => {
         if (validateFillForDeposit(fill, deposit).valid) {
           const repaymentChainId = getRepaymentChainId(fill, deposit);
-          // The list of possible repayment chains should be the enabled chains in the config store (minus any
-          // disabled chains) as of the bundle end block containing this fill. Since we don't know which bundle
-          // this fill belongs to, and we don't want to convert the deposit quote timestamp into a block number
-          // to query the chain list at a mainnet block height (for latency purposes), we will just use the
-          // list of all chains that Across supports, which also can include some disabled chains. Worst case
-          // we don't a mark a fill as unrepayable because its chosen a disabled chain as repayment, and we miss
-          // a log.
-          if (!isEvmRepaymentValid(fill, repaymentChainId, this.configStoreClient?.getEnabledChains() ?? [])) {
+          // In order to keep this function sync, we can't call verifyFillRepayment so we'll log any fills that
+          // we'll have to overwrite repayment information for. This includes fills for lite chains where the
+          // repayment address is invalid, and fills for non-lite chains where the repayment address is valid or
+          // the repayment chain is invalid. We don't check that the origin chain is a valid EVM chain for
+          // lite chain deposits yet because only EVM chains are supported on Across...for now. This means
+          // this logic will have to be revisited when we add SVM to log properly.
+          if (
+            this.hubPoolClient &&
+            !isSlowFill(fill) &&
+            (!isValidEvmAddress(fill.relayer) ||
+              forceDestinationRepayment(
+                repaymentChainId,
+                { ...deposit, quoteBlockNumber: this.hubPoolClient!.latestBlockSearched },
+                this.hubPoolClient
+              ))
+          ) {
             groupedFills.unrepayableFills.push(fill);
           }
           // This fill is still valid and means that the deposit cannot be filled on-chain anymore, but it
@@ -427,7 +437,7 @@ export class SpokePoolClient extends BaseAbstractClient {
       this.logger.warn({
         at: "SpokePoolClient",
         chainId: this.chainId,
-        message: "Unrepayable fills found.",
+        message: "Unrepayable fills found where we need to switch repayment address and or chain",
         deposit,
         unrepayableFills: Object.fromEntries(unrepayableFillsForDeposit.map((x) => [x.relayer, x])),
         notificationPath: "across-unrepayable-fills",

src/clients/mocks/MockHubPoolClient.ts

@@ -100,6 +100,10 @@ export class MockHubPoolClient extends HubPoolClient {
     this.spokePoolTokens[l1Token][chainId] = l2Token;
   }
 
+  deleteTokenMapping(l1Token: string, chainId: number) {
+    delete this.spokePoolTokens[l1Token]?.[chainId];
+  }
+
   getL1TokenForL2TokenAtBlock(l2Token: string, chainId: number, blockNumber: number): string {
     const l1Token = Object.keys(this.spokePoolTokens).find(
       (l1Token) => this.spokePoolTokens[l1Token]?.[chainId] === l2Token