feat(BundleDataClient): Support refunds for pre-fills/slow-fill-reque…

…sts and duplicate deposits (#835)

Co-authored-by: bmzig <[email protected]>

package.json

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

src/clients/BundleDataClient/utils/FillUtils.ts

@@ -1,6 +1,6 @@
 import _ from "lodash";
 import { providers } from "ethers";
-import { DepositWithBlock, Fill, FillWithBlock } from "../../../interfaces";
+import { Deposit, DepositWithBlock, Fill, FillWithBlock } from "../../../interfaces";
 import { getBlockRangeForChain, isSlowFill, chainIsEvm, isValidEvmAddress, isDefined } from "../../../utils";
 import { HubPoolClient } from "../../HubPoolClient";
 
@@ -47,34 +47,51 @@ 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;
+}
+
+export function isEvmRepaymentValid(
+  fill: Fill,
+  repaymentChainId: number,
+  possibleRepaymentChainIds: number[] = []
+): 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)) {
+    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
 // to repay the `msg.sender` of the relay transaction. Otherwise, return undefined.
 export async function verifyFillRepayment(
-  fill: FillWithBlock,
+  _fill: FillWithBlock,
   destinationChainProvider: providers.Provider,
   matchedDeposit: DepositWithBlock,
-  possibleRepaymentChainIds: number[]
+  possibleRepaymentChainIds: number[] = []
 ): Promise<FillWithBlock | undefined> {
-  // Slow fills don't result in repayments so they're always valid.
-  if (isSlowFill(fill)) {
-    return fill;
-  }
-  // Lite chain deposits force repayment on origin chain.
-  const repaymentChainId = matchedDeposit.fromLiteChain ? fill.originChainId : fill.repaymentChainId;
-  // Return undefined if the requested repayment chain ID is not recognized by the hub pool.
-  if (!possibleRepaymentChainIds.includes(repaymentChainId)) {
-    return undefined;
-  }
-  const updatedFill = _.cloneDeep(fill);
+  const fill = _.cloneDeep(_fill);
 
-  // If the fill requests repayment on a chain where the repayment address is not valid, attempt to find a valid
-  // repayment address, otherwise return undefined.
+  const repaymentChainId = getRepaymentChainId(fill, matchedDeposit);
+  const validEvmRepayment = isEvmRepaymentValid(fill, repaymentChainId, possibleRepaymentChainIds);
 
-  // Case 1: repayment chain is an EVM chain but repayment address is not a valid EVM address.
-  if (chainIsEvm(repaymentChainId) && !isValidEvmAddress(updatedFill.relayer)) {
+  // Case 1: Repayment chain is EVM and repayment address is valid EVM address.
+  if (validEvmRepayment) {
+    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)) {
     // 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(updatedFill.transactionHash);
+    const fillTransaction = await destinationChainProvider.getTransaction(fill.transactionHash);
     const destinationRelayer = fillTransaction?.from;
     // Repayment chain is still an EVM chain, but the msg.sender is a bytes32 address, so the fill is invalid.
     if (!isDefined(destinationRelayer) || !isValidEvmAddress(destinationRelayer)) {
@@ -83,9 +100,11 @@ export async function verifyFillRepayment(
     // 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.
-    updatedFill.relayer = destinationRelayer;
+    fill.relayer = destinationRelayer;
+    return fill;
+  }
+  // Case 3: Repayment chain is not an EVM chain, must be invalid.
+  else {
+    return undefined;
   }
-
-  // Case 2: TODO repayment chain is an SVM chain and repayment address is not a valid SVM address.
-  return updatedFill;
 }

src/clients/SpokePoolClient.ts

@@ -67,6 +67,7 @@ export class SpokePoolClient extends BaseAbstractClient {
   protected currentTime = 0;
   protected oldestTime = 0;
   protected depositHashes: { [depositHash: string]: DepositWithBlock } = {};
+  protected duplicateDepositHashes: { [depositHash: string]: DepositWithBlock[] } = {};
   protected depositHashesToFills: { [depositHash: string]: FillWithBlock[] } = {};
   protected speedUps: { [depositorAddress: string]: { [depositId: string]: SpeedUpWithBlock[] } } = {};
   protected slowFillRequests: { [relayDataHash: string]: SlowFillRequestWithBlock } = {};
@@ -126,14 +127,42 @@ export class SpokePoolClient extends BaseAbstractClient {
   }
 
   /**
-   * Retrieves a list of deposits from the SpokePool contract destined for the given destination chain ID.
+   * Retrieves a list of unique deposits from the SpokePool contract destined for the given destination chain ID.
    * @param destinationChainId The destination chain ID.
    * @returns A list of deposits.
    */
   public getDepositsForDestinationChain(destinationChainId: number): DepositWithBlock[] {
     return Object.values(this.depositHashes).filter((deposit) => deposit.destinationChainId === destinationChainId);
   }
 
+  /**
+   * Retrieves a list of duplicate deposits matching the given deposit's deposit hash.
+   * @notice A duplicate is considered any deposit sent after the original deposit with the same deposit hash.
+   * @param deposit The deposit to find duplicates for.
+   * @returns A list of duplicate deposits. Does NOT include the original deposit
+   * unless the original deposit is a duplicate.
+   */
+  private _getDuplicateDeposits(deposit: DepositWithBlock): DepositWithBlock[] {
+    const depositHash = this.getDepositHash(deposit);
+    return this.duplicateDepositHashes[depositHash] ?? [];
+  }
+
+  /**
+   * Returns a list of all deposits including any duplicate ones. Designed only to be used in use cases where
+   * all deposits are required, regardless of duplicates. For example, the Dataworker can use this to refund
+   * expired deposits including for duplicates.
+   * @param destinationChainId
+   * @returns A list of deposits
+   */
+  public getDepositsForDestinationChainWithDuplicates(destinationChainId: number): DepositWithBlock[] {
+    const deposits = this.getDepositsForDestinationChain(destinationChainId);
+    const duplicateDeposits = deposits.reduce((acc, deposit) => {
+      const duplicates = this._getDuplicateDeposits(deposit);
+      return acc.concat(duplicates);
+    }, [] as DepositWithBlock[]);
+    return sortEventsAscendingInPlace(deposits.concat(duplicateDeposits.flat()));
+  }
+
   /**
    * Retrieves a list of deposits from the SpokePool contract that are associated with this spoke pool.
    * @returns A list of deposits.
@@ -579,6 +608,7 @@ export class SpokePoolClient extends BaseAbstractClient {
         }
 
         if (this.depositHashes[this.getDepositHash(deposit)] !== undefined) {
+          assign(this.duplicateDepositHashes, [this.getDepositHash(deposit)], [deposit]);
           continue;
         }
         assign(this.depositHashes, [this.getDepositHash(deposit)], deposit);

src/clients/mocks/MockSpokePoolClient.ts

@@ -123,7 +123,6 @@ export class MockSpokePoolClient extends SpokePoolClient {
     const { blockNumber, transactionIndex } = deposit;
     let { depositId, depositor, destinationChainId, inputToken, inputAmount, outputToken, outputAmount } = deposit;
     depositId ??= this.numberOfDeposits;
-    assert(depositId.gte(this.numberOfDeposits), `${depositId.toString()} < ${this.numberOfDeposits}`);
     this.numberOfDeposits = depositId.add(bnOne);
 
     destinationChainId ??= random(1, 42161, false);

src/constants.ts

@@ -26,9 +26,11 @@ export const SECONDS_PER_YEAR = 31557600; // 365.25 days per year.
  */
 export const HUBPOOL_CHAIN_ID = 1;
 
-// List of versions where certain UMIP features were deprecated
+// List of versions where certain UMIP features were deprecated or activated
 export const TRANSFER_THRESHOLD_MAX_CONFIG_STORE_VERSION = 1;
 
+export const PRE_FILL_MIN_CONFIG_STORE_VERSION = 5;
+
 // A hardcoded identifier used, by default, to tag all Arweave records.
 export const ARWEAVE_TAG_APP_NAME = "across-protocol";
 

src/utils/DepositUtils.ts

@@ -1,6 +1,6 @@
 import assert from "assert";
 import { SpokePoolClient } from "../clients";
-import { DEFAULT_CACHING_TTL, EMPTY_MESSAGE } from "../constants";
+import { DEFAULT_CACHING_TTL, EMPTY_MESSAGE, ZERO_BYTES } from "../constants";
 import { CachingMechanismInterface, Deposit, DepositWithBlock, Fill, SlowFillRequest } from "../interfaces";
 import { getNetworkName } from "./NetworkUtils";
 import { getDepositInCache, getDepositKey, setDepositInCache } from "./CachingUtils";
@@ -146,6 +146,10 @@ export function isZeroValueDeposit(deposit: Pick<Deposit, "inputAmount" | "messa
   return deposit.inputAmount.eq(0) && isMessageEmpty(deposit.message);
 }
 
+export function isZeroValueFillOrSlowFillRequest(e: Pick<Fill | SlowFillRequest, "inputAmount" | "message">): boolean {
+  return e.inputAmount.eq(0) && isFillOrSlowFillRequestMessageEmpty(e.message);
+}
+
 /**
  * Determines if a message is empty or not.
  * @param message The message to check.
@@ -155,6 +159,10 @@ export function isMessageEmpty(message = EMPTY_MESSAGE): boolean {
   return message === "" || message === "0x";
 }
 
+export function isFillOrSlowFillRequestMessageEmpty(message: string): boolean {
+  return isMessageEmpty(message) || message === ZERO_BYTES;
+}
+
 /**
  * Determines if a deposit was updated via a speed-up transaction.
  * @param deposit Deposit to evaluate.

src/utils/SpokeUtils.ts

@@ -1,14 +1,15 @@
 import assert from "assert";
 import { BytesLike, Contract, PopulatedTransaction, providers, utils as ethersUtils } from "ethers";
 import { CHAIN_IDs, MAX_SAFE_DEPOSIT_ID, ZERO_ADDRESS, ZERO_BYTES } from "../constants";
-import { Deposit, Fill, FillStatus, RelayData, SlowFillRequest } from "../interfaces";
+import { Deposit, Fill, FillStatus, FillWithBlock, RelayData, SlowFillRequest } from "../interfaces";
 import { SpokePoolClient } from "../clients";
 import { chunk } from "./ArrayUtils";
 import { BigNumber, toBN, bnOne, bnZero } from "./BigNumberUtils";
 import { keccak256 } from "./common";
 import { isMessageEmpty } from "./DepositUtils";
 import { isDefined } from "./TypeGuards";
 import { getNetworkName } from "./NetworkUtils";
+import { paginatedEventQuery, spreadEventWithBlockNumber } from "./EventUtils";
 
 type BlockTag = providers.BlockTag;
 
@@ -351,12 +352,11 @@ export async function findFillBlock(
 ): Promise<number | undefined> {
   const { provider } = spokePool;
   highBlockNumber ??= await provider.getBlockNumber();
-  assert(highBlockNumber > lowBlockNumber, `Block numbers out of range (${lowBlockNumber} > ${highBlockNumber})`);
+  assert(highBlockNumber > lowBlockNumber, `Block numbers out of range (${lowBlockNumber} >= ${highBlockNumber})`);
 
   // In production the chainId returned from the provider matches 1:1 with the actual chainId. Querying the provider
   // object saves an RPC query becasue the chainId is cached by StaticJsonRpcProvider instances. In hre, the SpokePool
   // may be configured with a different chainId than what is returned by the provider.
-  // @todo Sub out actual chain IDs w/ CHAIN_IDs constants
   const destinationChainId = Object.values(CHAIN_IDs).includes(relayData.originChainId)
     ? (await provider.getNetwork()).chainId
     : Number(await spokePool.chainId());
@@ -399,6 +399,39 @@ export async function findFillBlock(
   return lowBlockNumber;
 }
 
+export async function findFillEvent(
+  spokePool: Contract,
+  relayData: RelayData,
+  lowBlockNumber: number,
+  highBlockNumber?: number
+): Promise<FillWithBlock | undefined> {
+  const blockNumber = await findFillBlock(spokePool, relayData, lowBlockNumber, highBlockNumber);
+  if (!blockNumber) return undefined;
+  const query = await paginatedEventQuery(
+    spokePool,
+    spokePool.filters.FilledV3Relay(null, null, null, null, null, relayData.originChainId, relayData.depositId),
+    {
+      fromBlock: blockNumber,
+      toBlock: blockNumber,
+      maxBlockLookBack: 0, // We can hardcode this to 0 to instruct paginatedEventQuery to make a single request
+      // for the same block number.
+    }
+  );
+  if (query.length === 0) throw new Error(`Failed to find fill event at block ${blockNumber}`);
+  const event = query[0];
+  // In production the chainId returned from the provider matches 1:1 with the actual chainId. Querying the provider
+  // object saves an RPC query becasue the chainId is cached by StaticJsonRpcProvider instances. In hre, the SpokePool
+  // may be configured with a different chainId than what is returned by the provider.
+  const destinationChainId = Object.values(CHAIN_IDs).includes(relayData.originChainId)
+    ? (await spokePool.provider.getNetwork()).chainId
+    : Number(await spokePool.chainId());
+  const fill = {
+    ...spreadEventWithBlockNumber(event),
+    destinationChainId,
+  } as FillWithBlock;
+  return fill;
+}
+
 // Determines if the input address (either a bytes32 or bytes20) is the zero address.
 export function isZeroAddress(address: string): boolean {
   return address === ZERO_ADDRESS || address === ZERO_BYTES;

test/SpokePoolClient.SpeedUp.ts

@@ -215,13 +215,13 @@ describe("SpokePoolClient: SpeedUp", function () {
     // attributed to the existing deposit.
     for (const field of ["originChainId", "depositId", "depositor"]) {
       const testOriginChainId = field !== "originChainId" ? originChainId : originChainId + 1;
-      const testDepositId = field !== "depositId" ? depositId : depositId + 1;
+      const testDepositId = field !== "depositId" ? depositId : depositId.add(1);
       const testDepositor = field !== "depositor" ? depositor : (await ethers.getSigners())[0];
       assert.isTrue(field !== "depositor" || testDepositor.address !== depositor.address); // Sanity check
 
       const signature = await getUpdatedV3DepositSignature(
         testDepositor,
-        testDepositId,
+        testDepositId.toNumber(),
         testOriginChainId,
         updatedOutputAmount,
         updatedRecipient,

test/SpokePoolClient.ValidateFill.ts

@@ -615,7 +615,7 @@ describe("SpokePoolClient: Fill Validation", function () {
 
     // Override the first spoke pool deposit ID that the client thinks is available in the contract.
     await spokePoolClient1.update();
-    spokePoolClient1.firstDepositIdForSpokePool = deposit.depositId + 1;
+    spokePoolClient1.firstDepositIdForSpokePool = deposit.depositId.add(1);
     expect(fill.depositId < spokePoolClient1.firstDepositIdForSpokePool).is.true;
     const search = await queryHistoricalDepositForFill(spokePoolClient1, fill);
 
@@ -636,7 +636,7 @@ describe("SpokePoolClient: Fill Validation", function () {
     );
 
     // Override the deposit ID that we are "filling" to be > 1, the latest deposit ID in spoke pool 1.
-    await fillV3Relay(spokePool_2, { ...deposit, depositId: deposit.depositId + 1 }, relayer);
+    await fillV3Relay(spokePool_2, { ...deposit, depositId: deposit.depositId.add(1) }, relayer);
     await spokePoolClient2.update();
     const [fill] = spokePoolClient2.getFills();
 

test/SpokePoolClient.fills.ts

@@ -1,7 +1,7 @@
 import hre from "hardhat";
 import { SpokePoolClient } from "../src/clients";
 import { Deposit } from "../src/interfaces";
-import { bnOne, bnZero, findFillBlock, getNetworkName } from "../src/utils";
+import { bnOne, bnZero, findFillBlock, findFillEvent, getNetworkName } from "../src/utils";
 import { EMPTY_MESSAGE, ZERO_ADDRESS } from "../src/constants";
 import { originChainId, destinationChainId } from "./constants";
 import {
@@ -114,6 +114,24 @@ describe("SpokePoolClient: Fills", function () {
     expect(fillBlock).to.equal(targetFillBlock);
   });
 
+  it("Correctly returns the FilledV3Relay event using the relay data", async function () {
+    const targetDeposit = { ...deposit, depositId: deposit.depositId.add(1) };
+    // Submit multiple fills at the same block:
+    const startBlock = await spokePool.provider.getBlockNumber();
+    await fillV3Relay(spokePool, deposit, relayer1);
+    await fillV3Relay(spokePool, targetDeposit, relayer1);
+    await fillV3Relay(spokePool, { ...deposit, depositId: deposit.depositId.add(2) }, relayer1);
+    await hre.network.provider.send("evm_mine");
+
+    const fill = await findFillEvent(spokePool, targetDeposit, startBlock);
+    expect(fill).to.not.be.undefined;
+    expect(fill!.depositId).to.equal(targetDeposit.depositId);
+
+    // Looking for a fill can return undefined:
+    const missingFill = await findFillEvent(spokePool, { ...deposit, depositId: deposit.depositId.add(3) }, startBlock);
+    expect(missingFill).to.be.undefined;
+  });
+
   it("FilledV3Relay block search: bounds checking", async function () {
     const nBlocks = 100;
     const startBlock = await spokePool.provider.getBlockNumber();