diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts index e397b5d85..de256ef7f 100644 --- a/src/clients/SpokePoolClient.ts +++ b/src/clients/SpokePoolClient.ts @@ -318,6 +318,11 @@ export class SpokePoolClient extends BaseAbstractClient { }; } + public getDeposit(depositId: number): DepositWithBlock | undefined { + const depositHash = this.getDepositHash({ depositId, originChainId: this.chainId }); + return this.depositHashes[depositHash]; + } + /** * Find a corresponding deposit for a given fill. * @param fill The fill to find a corresponding deposit for. diff --git a/src/utils/DepositUtils.ts b/src/utils/DepositUtils.ts index 6509ed19d..42d91a6be 100644 --- a/src/utils/DepositUtils.ts +++ b/src/utils/DepositUtils.ts @@ -2,6 +2,7 @@ import assert from "assert"; import { SpokePoolClient } from "../clients"; import { DEFAULT_CACHING_TTL, EMPTY_MESSAGE } from "../constants"; import { CachingMechanismInterface, Deposit, DepositWithBlock, Fill } from "../interfaces"; +import { getNetworkName } from "../utils"; import { getDepositInCache, getDepositKey, setDepositInCache } from "./CachingUtils"; import { validateFillForDeposit } from "./FlowUtils"; import { getCurrentTime } from "./TimeUtils"; @@ -12,6 +13,16 @@ import { isDepositFormedCorrectly } from "./ValidatorUtils"; // This can be used by the Dataworker to determine whether to give a relayer a refund for a fill // of a deposit older or younger than its fixed lookback. +export enum InvalidFill { + DepositIdInvalid = 0, // Deposit ID seems invalid for origin SpokePool + DepositIdNotFound, // Deposit ID not found (bad RPC data?) + FillMismatch, // Fill does not match deposit parameters for deposit ID. +}; + +export type DepositSearchResult = + | { found: true; deposit: DepositWithBlock; } + | { found: false; code: InvalidFill; reason: string; }; + /** * Attempts to resolve a deposit for a fill. If the fill's deposit Id is within the spoke pool client's search range, * the deposit is returned immediately. Otherwise, the deposit is queried first from the provided cache, and if it is @@ -28,7 +39,7 @@ export async function queryHistoricalDepositForFill( spokePoolClient: SpokePoolClient, fill: Fill, cache?: CachingMechanismInterface -): Promise { +): Promise { if (fill.originChainId !== spokePoolClient.chainId) { throw new Error(`OriginChainId mismatch (${fill.originChainId} != ${spokePoolClient.chainId})`); } @@ -39,18 +50,28 @@ export async function queryHistoricalDepositForFill( throw new Error("SpokePoolClient must be updated before querying historical deposits"); } - if ( - fill.depositId < spokePoolClient.firstDepositIdForSpokePool || - fill.depositId > spokePoolClient.lastDepositIdForSpokePool - ) { - return undefined; + const { depositId } = fill; + let { firstDepositIdForSpokePool: lowId, lastDepositIdForSpokePool: highId } = spokePoolClient; + if (depositId < lowId || depositId > highId) { + return { + found: false, + code: InvalidFill.DepositIdInvalid, + reason: `Deposit ID ${depositId} is outside of SpokePool bounds [${lowId},${highId}].`, + }; } - if ( - fill.depositId >= spokePoolClient.earliestDepositIdQueried && - fill.depositId <= spokePoolClient.latestDepositIdQueried - ) { - return spokePoolClient.getDepositForFill(fill); + ({ earliestDepositIdQueried: lowId, latestDepositIdQueried: highId } = spokePoolClient); + if (depositId >= lowId && depositId <= highId) { + const deposit = spokePoolClient.getDeposit(depositId); + if (isDefined(deposit) && validateFillForDeposit(fill, deposit)) { + return { found: true, deposit }; + } + + return { + found: false, + code: InvalidFill.DepositIdNotFound, + reason: `Deposit ID ${depositId} not found in SpokePoolClient event buffer.` + }; } let deposit: DepositWithBlock, cachedDeposit: Deposit | undefined; @@ -82,7 +103,18 @@ export async function queryHistoricalDepositForFill( } } - return validateFillForDeposit(fill, deposit) ? deposit : undefined; + if (validateFillForDeposit(fill, deposit)) { + return { + found: true, + deposit, + }; + } + + return { + found: false, + code: InvalidFill.FillMismatch, + reason: `Fill is not valid for ${getNetworkName(deposit.originChainId)} deposit ${depositId}`, + }; } /** diff --git a/test/SpokePoolClient.ValidateFill.ts b/test/SpokePoolClient.ValidateFill.ts index c9f8f9d7f..f47dd17a6 100644 --- a/test/SpokePoolClient.ValidateFill.ts +++ b/test/SpokePoolClient.ValidateFill.ts @@ -28,7 +28,7 @@ import { import { SpokePoolClient } from "../src/clients"; import { MockConfigStoreClient, MockHubPoolClient, MockSpokePoolClient } from "./mocks"; -import { validateFillForDeposit, queryHistoricalDepositForFill } from "../src/utils"; +import { InvalidFill, validateFillForDeposit, queryHistoricalDepositForFill } from "../src/utils"; import { CHAIN_ID_TEST_LIST, repaymentChainId } from "./constants"; let spokePool_1: Contract, erc20_1: Contract, spokePool_2: Contract, erc20_2: Contract, hubPool: Contract; @@ -401,7 +401,10 @@ describe("SpokePoolClient: Fill Validation", function () { // Client has 0 deposits in memory so querying historical deposit sends fresh RPC requests. expect(spokePoolClient1.getDeposits().length).to.equal(0); const historicalDeposit = await queryHistoricalDepositForFill(spokePoolClient1, fill); - expect(historicalDeposit?.depositId).to.deep.equal(deposit.depositId); + expect(historicalDeposit.found).is.true; + if (historicalDeposit.found) { + expect(historicalDeposit.deposit.depositId).to.deep.equal(deposit.depositId); + } }); it("Can fetch younger deposit matching fill", async function () { @@ -424,7 +427,10 @@ describe("SpokePoolClient: Fill Validation", function () { // Client has 0 deposits in memory so querying historical deposit sends fresh RPC requests. expect(spokePoolClient1.getDeposits().length).to.equal(0); const historicalDeposit = await queryHistoricalDepositForFill(spokePoolClient1, fill); - expect(historicalDeposit?.depositId).to.deep.equal(deposit.depositId); + expect(historicalDeposit.found).is.true; + if (historicalDeposit.found) { + expect(historicalDeposit.deposit.depositId).to.deep.equal(deposit.depositId); + } }); it("Loads fills from memory with deposit ID > spoke pool client's earliest deposit ID queried", async function () { @@ -434,12 +440,12 @@ describe("SpokePoolClient: Fill Validation", function () { expect(spokePoolClient1.earliestDepositIdQueried == 0).is.true; // Client should NOT send RPC requests to fetch this deposit, instead it should load from memory. - expect((await queryHistoricalDepositForFill(spokePoolClient1, fill)) !== undefined).is.true; + expect((await queryHistoricalDepositForFill(spokePoolClient1, fill)).found).is.true; expect(lastSpyLogIncludes(spy, "updated!")).is.true; // Now override earliest deposit ID queried so that its > deposit ID and check that client sends RPC requests. spokePoolClient1.earliestDepositIdQueried = 1; - expect((await queryHistoricalDepositForFill(spokePoolClient1, fill)) !== undefined).is.true; + expect((await queryHistoricalDepositForFill(spokePoolClient1, fill)).found).is.true; expect(lastSpyLogIncludes(spy, "Located deposit outside of SpokePoolClient's search range")).is.true; }); @@ -452,12 +458,12 @@ describe("SpokePoolClient: Fill Validation", function () { spokePoolClient1.latestDepositIdQueried = 1; // Client should NOT send RPC requests to fetch this deposit, instead it should load from memory. - expect((await queryHistoricalDepositForFill(spokePoolClient1, fill)) !== undefined).is.true; + expect((await queryHistoricalDepositForFill(spokePoolClient1, fill)).found).is.true; expect(lastSpyLogIncludes(spy, "updated!")).is.true; // Now override latest deposit ID queried so that its < deposit ID and check that client sends RPC requests. spokePoolClient1.latestDepositIdQueried = -1; - expect((await queryHistoricalDepositForFill(spokePoolClient1, fill)) !== undefined).is.true; + expect((await queryHistoricalDepositForFill(spokePoolClient1, fill)).found).is.true; expect(lastSpyLogIncludes(spy, "Located deposit outside of SpokePoolClient's search range")).is.true; }); @@ -469,9 +475,13 @@ 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 = 1; + spokePoolClient1.firstDepositIdForSpokePool = deposit.depositId + 1; expect(fill.depositId < spokePoolClient1.firstDepositIdForSpokePool).is.true; - await queryHistoricalDepositForFill(spokePoolClient1, fill); + const search = await queryHistoricalDepositForFill(spokePoolClient1, fill); + expect(search.found).is.false; + if (search.found === false) { + expect(search.code).to.equal(InvalidFill.DepositIdInvalid); + } expect(lastSpyLogIncludes(spy, "Queried RPC for deposit")).is.not.true; }); @@ -494,7 +504,11 @@ describe("SpokePoolClient: Fill Validation", function () { await spokePoolClient1.update(); expect(fill.depositId > spokePoolClient1.lastDepositIdForSpokePool).is.true; - await queryHistoricalDepositForFill(spokePoolClient1, fill); + const search = await queryHistoricalDepositForFill(spokePoolClient1, fill); + expect(search.found).is.false; + if (search.found === false) { + expect(search.code).to.equal(InvalidFill.DepositIdInvalid); + } expect(lastSpyLogIncludes(spy, "Queried RPC for deposit")).is.not.true; });