From 3bb7a256d221292a5a811b564bf6838387798ed1 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:04:13 -0500 Subject: [PATCH 01/23] fix(FillUtils): verifyFillRepayment should check that repayment chain has pool rebalance route We cannot refund fills whose repayment chain doesn't exist for an input token. --- .../BundleDataClient/BundleDataClient.ts | 29 +++++++++++-------- .../BundleDataClient/utils/FillUtils.ts | 28 ++++++++++-------- 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index ef325fcb4..6f31a1e97 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -18,7 +18,7 @@ import { Deposit, DepositWithBlock, } from "../../interfaces"; -import { AcrossConfigStoreClient, SpokePoolClient } from ".."; +import { AcrossConfigStoreClient, HubPoolClient, SpokePoolClient } from ".."; import { BigNumber, bnZero, @@ -91,7 +91,7 @@ function updateBundleFillsV3( lpFeePct: BigNumber, repaymentChainId: number, repaymentToken: string, - repaymentAddress: string + repaymentAddress: string, ): void { // We shouldn't pass any unrepayable fills into this function, so we perform an extra safety check. assert( @@ -355,11 +355,7 @@ export class BundleDataClient { 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])) + this.clients.hubPoolClient, ); if (!isDefined(validRepayment)) { return false; @@ -919,7 +915,7 @@ export class BundleDataClient { fill, destinationClient.spokePool.provider, v3RelayHashes[relayDataHash].deposits![0], - allChainIds + this.clients.hubPoolClient, ); if (!isDefined(fillToRefund)) { bundleUnrepayableFillsV3.push(fill); @@ -1013,7 +1009,7 @@ export class BundleDataClient { fill, destinationClient.spokePool.provider, matchedDeposit, - allChainIds + this.clients.hubPoolClient, ); if (!isDefined(fillToRefund)) { bundleUnrepayableFillsV3.push(fill); @@ -1187,7 +1183,7 @@ export class BundleDataClient { fill, destinationClient.spokePool.provider, v3RelayHashes[relayDataHash].deposits![0], - allChainIds + this.clients.hubPoolClient, ); if (!isDefined(fillToRefund)) { bundleUnrepayableFillsV3.push(fill); @@ -1241,7 +1237,7 @@ export class BundleDataClient { prefill!, destinationClient.spokePool.provider, deposit, - allChainIds + this.clients.hubPoolClient, ); if (!isDefined(verifiedFill)) { bundleUnrepayableFillsV3.push(prefill!); @@ -1415,7 +1411,16 @@ export class BundleDataClient { chainIds, associatedDeposit!.fromLiteChain ); - updateBundleFillsV3(bundleFillsV3, fill, realizedLpFeePct, chainToSendRefundTo, repaymentToken, fill.relayer); + updateBundleFillsV3( + bundleFillsV3, + fill, + realizedLpFeePct, + chainToSendRefundTo, + repaymentToken, + fill.relayer, + this.clients.hubPoolClient, + associatedDeposit.quoteBlockNumber + ); }); v3SlowFillLpFees.forEach(({ realizedLpFeePct: lpFeePct }, idx) => { const deposit = validatedBundleSlowFills[idx]; diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index 01d856233..c86405f4b 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -52,20 +52,11 @@ export function getRepaymentChainId(fill: Fill, matchedDeposit: Deposit): number return matchedDeposit.fromLiteChain ? fill.originChainId : fill.repaymentChainId; } -export function isEvmRepaymentValid( - fill: Fill, - repaymentChainId: number, - possibleRepaymentChainIds: number[] = [] -): boolean { +export function isEvmRepaymentValid(fill: Fill, repaymentChainId: 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); } @@ -75,12 +66,25 @@ export async function verifyFillRepayment( _fill: FillWithBlock, destinationChainProvider: providers.Provider, matchedDeposit: DepositWithBlock, - possibleRepaymentChainIds: number[] = [] + hubPoolClient: HubPoolClient ): Promise { const fill = _.cloneDeep(_fill); const repaymentChainId = getRepaymentChainId(fill, matchedDeposit); - const validEvmRepayment = isEvmRepaymentValid(fill, repaymentChainId, possibleRepaymentChainIds); + const validEvmRepayment = isEvmRepaymentValid(fill, repaymentChainId); + try { + const l1TokenCounterpart = hubPoolClient.getL1TokenForL2TokenAtBlock( + fill.inputToken, + fill.originChainId, + matchedDeposit.quoteBlockNumber + ); + hubPoolClient.getL2TokenForL1TokenAtBlock(l1TokenCounterpart, repaymentChainId, matchedDeposit.quoteBlockNumber); + // Repayment token could be found, this is a valid repayment chain. + } catch { + // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler. + return undefined; + } + // Case 1: Repayment chain is EVM and repayment address is valid EVM address. if (validEvmRepayment) { From 7a8872dcf52335e60f104aa27dd615261e3e2e1a Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:05:43 -0500 Subject: [PATCH 02/23] lint --- src/clients/BundleDataClient/BundleDataClient.ts | 14 +++++++------- src/clients/BundleDataClient/utils/FillUtils.ts | 1 - 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index 6f31a1e97..78f19a854 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -18,7 +18,7 @@ import { Deposit, DepositWithBlock, } from "../../interfaces"; -import { AcrossConfigStoreClient, HubPoolClient, SpokePoolClient } from ".."; +import { AcrossConfigStoreClient, SpokePoolClient } from ".."; import { BigNumber, bnZero, @@ -91,7 +91,7 @@ function updateBundleFillsV3( lpFeePct: BigNumber, repaymentChainId: number, repaymentToken: string, - repaymentAddress: string, + repaymentAddress: string ): void { // We shouldn't pass any unrepayable fills into this function, so we perform an extra safety check. assert( @@ -355,7 +355,7 @@ export class BundleDataClient { fill, this.spokePoolClients[fill.destinationChainId].spokePool.provider, matchingDeposit, - this.clients.hubPoolClient, + this.clients.hubPoolClient ); if (!isDefined(validRepayment)) { return false; @@ -915,7 +915,7 @@ export class BundleDataClient { fill, destinationClient.spokePool.provider, v3RelayHashes[relayDataHash].deposits![0], - this.clients.hubPoolClient, + this.clients.hubPoolClient ); if (!isDefined(fillToRefund)) { bundleUnrepayableFillsV3.push(fill); @@ -1009,7 +1009,7 @@ export class BundleDataClient { fill, destinationClient.spokePool.provider, matchedDeposit, - this.clients.hubPoolClient, + this.clients.hubPoolClient ); if (!isDefined(fillToRefund)) { bundleUnrepayableFillsV3.push(fill); @@ -1183,7 +1183,7 @@ export class BundleDataClient { fill, destinationClient.spokePool.provider, v3RelayHashes[relayDataHash].deposits![0], - this.clients.hubPoolClient, + this.clients.hubPoolClient ); if (!isDefined(fillToRefund)) { bundleUnrepayableFillsV3.push(fill); @@ -1237,7 +1237,7 @@ export class BundleDataClient { prefill!, destinationClient.spokePool.provider, deposit, - this.clients.hubPoolClient, + this.clients.hubPoolClient ); if (!isDefined(verifiedFill)) { bundleUnrepayableFillsV3.push(prefill!); diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index c86405f4b..7bd49d0df 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -85,7 +85,6 @@ export async function verifyFillRepayment( return undefined; } - // Case 1: Repayment chain is EVM and repayment address is valid EVM address. if (validEvmRepayment) { return fill; From fe14e5aab069803c2bb2fc499596bc65b016ab4c Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:13:28 -0500 Subject: [PATCH 03/23] Update BundleDataClient.ts --- src/clients/BundleDataClient/BundleDataClient.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index 78f19a854..3152e8cad 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -1411,16 +1411,7 @@ export class BundleDataClient { chainIds, associatedDeposit!.fromLiteChain ); - updateBundleFillsV3( - bundleFillsV3, - fill, - realizedLpFeePct, - chainToSendRefundTo, - repaymentToken, - fill.relayer, - this.clients.hubPoolClient, - associatedDeposit.quoteBlockNumber - ); + updateBundleFillsV3(bundleFillsV3, fill, realizedLpFeePct, chainToSendRefundTo, repaymentToken, fill.relayer); }); v3SlowFillLpFees.forEach(({ realizedLpFeePct: lpFeePct }, idx) => { const deposit = validatedBundleSlowFills[idx]; From 1c591a5df7f8121b504edb191d8edadf1d90e107 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:20:16 -0500 Subject: [PATCH 04/23] Add isSlowFill check --- .../BundleDataClient/utils/FillUtils.ts | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index 7bd49d0df..4482d0ce8 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -72,17 +72,19 @@ export async function verifyFillRepayment( const repaymentChainId = getRepaymentChainId(fill, matchedDeposit); const validEvmRepayment = isEvmRepaymentValid(fill, repaymentChainId); - try { - const l1TokenCounterpart = hubPoolClient.getL1TokenForL2TokenAtBlock( - fill.inputToken, - fill.originChainId, - matchedDeposit.quoteBlockNumber - ); - hubPoolClient.getL2TokenForL1TokenAtBlock(l1TokenCounterpart, repaymentChainId, matchedDeposit.quoteBlockNumber); - // Repayment token could be found, this is a valid repayment chain. - } catch { - // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler. - return undefined; + if (!isSlowFill(fill)) { + try { + const l1TokenCounterpart = hubPoolClient.getL1TokenForL2TokenAtBlock( + fill.inputToken, + fill.originChainId, + matchedDeposit.quoteBlockNumber + ); + hubPoolClient.getL2TokenForL1TokenAtBlock(l1TokenCounterpart, repaymentChainId, matchedDeposit.quoteBlockNumber); + // Repayment token could be found, this is a valid repayment chain. + } catch { + // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler. + return undefined; + } } // Case 1: Repayment chain is EVM and repayment address is valid EVM address. From 7e7ff5c2df3e77a9529ad6893029833536db090c Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:28:31 -0500 Subject: [PATCH 05/23] tests --- src/clients/mocks/MockHubPoolClient.ts | 4 +++ test/FillUtils.ts | 50 +++++++++++++++++++------- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/src/clients/mocks/MockHubPoolClient.ts b/src/clients/mocks/MockHubPoolClient.ts index 4b9248093..13f6746ac 100644 --- a/src/clients/mocks/MockHubPoolClient.ts +++ b/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 diff --git a/test/FillUtils.ts b/test/FillUtils.ts index 19615691a..f2a4af12a 100644 --- a/test/FillUtils.ts +++ b/test/FillUtils.ts @@ -2,25 +2,35 @@ import { DepositWithBlock, FillType, FillWithBlock } from "../src/interfaces"; import { bnOne, bnZero, toBN } from "../src/utils"; import { ZERO_ADDRESS } from "../src/constants"; import { originChainId, destinationChainId, repaymentChainId } from "./constants"; -import { expect, randomAddress } from "./utils"; +import { + createSpyLogger, + deployConfigStore, + expect, + hubPoolFixture, + randomAddress, + SignerWithAddress, + ethers, +} from "./utils"; import { verifyFillRepayment } from "../src/clients/BundleDataClient"; import { MockedProvider } from "../src/providers/mockProvider"; import { createRandomBytes32 } from "@across-protocol/contracts/dist/test-utils"; import { TransactionResponse } from "@ethersproject/abstract-provider"; +import { MockConfigStoreClient, MockHubPoolClient } from "./mocks"; describe("FillUtils", function () { let deposit: DepositWithBlock; let fill: FillWithBlock; let relayer: string; let spokeProvider: MockedProvider; - let validChainIds: number[]; + let hubPoolClient: MockHubPoolClient; + let owner: SignerWithAddress; const NOT_VALID_EVM_CHAIN = 9999; const INVALID_EVM_ADDRESS = createRandomBytes32(); - beforeEach(function () { - validChainIds = [originChainId, repaymentChainId]; + beforeEach(async function () { relayer = randomAddress(); + [owner] = await ethers.getSigners(); spokeProvider = new MockedProvider(bnZero, bnZero); deposit = { depositId: bnOne, @@ -57,32 +67,41 @@ describe("FillUtils", function () { relayer, repaymentChainId, }; + const { hubPool } = await hubPoolFixture(); + const { configStore } = await deployConfigStore(owner, []); + const configStoreClient = new MockConfigStoreClient(createSpyLogger().spyLogger, configStore); + hubPoolClient = new MockHubPoolClient(createSpyLogger().spyLogger, hubPool, configStoreClient); + hubPoolClient.setTokenMapping(ZERO_ADDRESS, deposit.originChainId, deposit.inputToken); }); describe("verifyFillRepayment", function () { it("Original repayment is valid", async function () { - const result = await verifyFillRepayment(fill, spokeProvider, deposit, validChainIds); + hubPoolClient.setTokenMapping(ZERO_ADDRESS, fill.repaymentChainId, ZERO_ADDRESS); + const result = await verifyFillRepayment(fill, spokeProvider, deposit, hubPoolClient); expect(result).to.not.be.undefined; }); it("SlowFill always valid", async function () { + // We don't set the repayment chain mapping for the input token because a slow fill should always be valid. const slowFill = { ...fill }; slowFill.relayExecutionInfo.fillType = FillType.SlowFill; - const result = await verifyFillRepayment(slowFill, spokeProvider, deposit, validChainIds); + const result = await verifyFillRepayment(slowFill, spokeProvider, deposit, hubPoolClient); expect(result).to.not.be.undefined; expect(slowFill.relayExecutionInfo.fillType).to.equal(FillType.SlowFill); }); it("Lite chain originChain used as repayment and origin chain is valid repayment chain", async function () { + // We don't set repayment chain mapping since repayment happens on origin chain. const liteChainDeposit = { ...deposit, fromLiteChain: true, }; - const result = await verifyFillRepayment(fill, spokeProvider, liteChainDeposit, [originChainId]); + const result = await verifyFillRepayment(fill, spokeProvider, liteChainDeposit, hubPoolClient); expect(result).to.not.be.undefined; // Repayment chain is untouched, it will be modified when computing bundle refunds. expect(result!.repaymentChainId).to.equal(repaymentChainId); }); it("Lite chain originChain used as repayment but origin chain is invalid repayment chain", async function () { + hubPoolClient.deleteTokenMapping(ZERO_ADDRESS, deposit.originChainId); const liteChainDeposit = { ...deposit, fromLiteChain: true, @@ -91,19 +110,21 @@ describe("FillUtils", function () { ...fill, originChainId: NOT_VALID_EVM_CHAIN, }; - const result = await verifyFillRepayment(liteChainFill, spokeProvider, liteChainDeposit, validChainIds); + const result = await verifyFillRepayment(liteChainFill, spokeProvider, liteChainDeposit, hubPoolClient); expect(result).to.be.undefined; }); it("Repayment chain is invalid", async function () { + hubPoolClient.deleteTokenMapping(ZERO_ADDRESS, fill.repaymentChainId); // valid chain ID's doesn't contain repayment chain. const invalidRepaymentFill = { ...fill, repaymentChainId: NOT_VALID_EVM_CHAIN, }; - const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, deposit, [repaymentChainId]); + const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, deposit, hubPoolClient); expect(result).to.be.undefined; }); it("Lite chain deposit and relayer is not valid EVM address; relayer gets overwritten to msg.sender", async function () { + // We don't set repayment chain mapping since repayment happens on origin chain. const liteChainDeposit = { ...deposit, fromLiteChain: true, @@ -115,13 +136,14 @@ describe("FillUtils", function () { spokeProvider._setTransaction(fill.transactionHash, { from: relayer, } as unknown as TransactionResponse); - const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, liteChainDeposit, [originChainId]); + const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, liteChainDeposit, hubPoolClient); expect(result).to.not.be.undefined; expect(result!.relayer).to.equal(relayer); // Repayment chain is untouched. expect(result!.repaymentChainId).to.equal(repaymentChainId); }); it("Relayer is not valid EVM address, relayer gets overwritten to msg.sender", async function () { + hubPoolClient.setTokenMapping(ZERO_ADDRESS, fill.repaymentChainId, ZERO_ADDRESS); // valid chain ID's doesn't contain repayment chain. const invalidRepaymentFill = { ...fill, @@ -130,13 +152,14 @@ describe("FillUtils", function () { spokeProvider._setTransaction(fill.transactionHash, { from: relayer, } as unknown as TransactionResponse); - const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, deposit, [repaymentChainId]); + const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, deposit, hubPoolClient); expect(result).to.not.be.undefined; expect(result!.relayer).to.equal(relayer); // Repayment chain is untouched. expect(result!.repaymentChainId).to.equal(repaymentChainId); }); it("Lite chain deposit and relayer is not valid EVM address; msg.sender is invalid", async function () { + // We don't set repayment chain mapping since repayment happens on origin chain. const liteChainDeposit = { ...deposit, fromLiteChain: true, @@ -148,10 +171,11 @@ describe("FillUtils", function () { spokeProvider._setTransaction(fill.transactionHash, { from: INVALID_EVM_ADDRESS, } as unknown as TransactionResponse); - const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, liteChainDeposit, [originChainId]); + const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, liteChainDeposit, hubPoolClient); expect(result).to.be.undefined; }); it("Relayer is not valid EVM address, and msg.sender is invalid", async function () { + hubPoolClient.setTokenMapping(ZERO_ADDRESS, fill.repaymentChainId, ZERO_ADDRESS); // Simulate what happens if the repayment chain is an EVM chain, the repayment address is not a vaid EVM address, // and the msg.sender is not a valid EVM address. This could happen if the fill was sent on Solana and the // repayment chain is Ethereum and the repayment address is an SVM address. @@ -162,7 +186,7 @@ describe("FillUtils", function () { spokeProvider._setTransaction(fill.transactionHash, { from: INVALID_EVM_ADDRESS, } as unknown as TransactionResponse); - const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, deposit, [repaymentChainId]); + const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, deposit, hubPoolClient); expect(result).to.be.undefined; }); }); From 985cbb703c7598b0865c31a704b246d8e6d60901 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:35:38 -0500 Subject: [PATCH 06/23] deafult to destination chain --- src/clients/BundleDataClient/utils/FillUtils.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index 4482d0ce8..d43393a7e 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -70,8 +70,10 @@ export async function verifyFillRepayment( ): Promise { const fill = _.cloneDeep(_fill); - const repaymentChainId = getRepaymentChainId(fill, matchedDeposit); - const validEvmRepayment = isEvmRepaymentValid(fill, repaymentChainId); + 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 (!isSlowFill(fill)) { try { const l1TokenCounterpart = hubPoolClient.getL1TokenForL2TokenAtBlock( @@ -82,10 +84,11 @@ export async function verifyFillRepayment( hubPoolClient.getL2TokenForL1TokenAtBlock(l1TokenCounterpart, repaymentChainId, matchedDeposit.quoteBlockNumber); // Repayment token could be found, this is a valid repayment chain. } catch { - // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler. - return undefined; + // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler there. + repaymentChainId = fill.destinationChainId; } } + const validEvmRepayment = isEvmRepaymentValid(fill, repaymentChainId); // Case 1: Repayment chain is EVM and repayment address is valid EVM address. if (validEvmRepayment) { @@ -102,10 +105,10 @@ 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. + // Otherwise, assume the relayer to be repaid is the msg.sender and swap repayment chain to destination chain + // where we know the new fill.relayer exists. fill.relayer = destinationRelayer; + fill.repaymentChainId = fill.destinationChainId; return fill; } // Case 3: Repayment chain is not an EVM chain, must be invalid. From 819173dab2b6bb0a189b9888dc03c1b50160a537 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:38:02 -0500 Subject: [PATCH 07/23] don't swap repayment chain for lite chain --- src/clients/BundleDataClient/utils/FillUtils.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index d43393a7e..34c662ef8 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -108,7 +108,9 @@ export async function verifyFillRepayment( // Otherwise, assume the relayer to be repaid is the msg.sender and swap repayment chain to destination chain // where we know the new fill.relayer exists. fill.relayer = destinationRelayer; - fill.repaymentChainId = fill.destinationChainId; + if (!matchedDeposit.fromLiteChain) { + fill.repaymentChainId = fill.destinationChainId; + } return fill; } // Case 3: Repayment chain is not an EVM chain, must be invalid. From 0c8223e0ab8971c94cc4d4c85ad80fa0a43b9db8 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:51:39 -0500 Subject: [PATCH 08/23] clean up case work --- .../BundleDataClient/utils/FillUtils.ts | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index 34c662ef8..6ef9ed4b3 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -94,27 +94,26 @@ export async function verifyFillRepayment( 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(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)) { - return undefined; + // Case 2: Repayment chain is not EVM or address is not a valid EVM address. Attempt to switch repayment + // address to msg.sender of relay transaction and repayment chain to destination chain. + else { + if (!chainIsEvm(repaymentChainId)) { + const newRepaymentChain = matchedDeposit.fromLiteChain ? fill.originChainId : fill.destinationChainId; + fill.repaymentChainId = newRepaymentChain; } - // Otherwise, assume the relayer to be repaid is the msg.sender and swap repayment chain to destination chain - // where we know the new fill.relayer exists. - fill.relayer = destinationRelayer; - if (!matchedDeposit.fromLiteChain) { - fill.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); + 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)) { + return undefined; + } + fill.relayer = destinationRelayer; } + return fill; } - // Case 3: Repayment chain is not an EVM chain, must be invalid. - else { - return undefined; - } } From 56396455046eafd8722d58fcf89d71eab00b6f9f Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:53:17 -0500 Subject: [PATCH 09/23] Update FillUtils.ts --- src/clients/BundleDataClient/utils/FillUtils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index 6ef9ed4b3..d0370fd8e 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -98,8 +98,9 @@ export async function verifyFillRepayment( // address to msg.sender of relay transaction and repayment chain to destination chain. else { if (!chainIsEvm(repaymentChainId)) { - const newRepaymentChain = matchedDeposit.fromLiteChain ? fill.originChainId : fill.destinationChainId; - fill.repaymentChainId = newRepaymentChain; + if (!matchedDeposit.fromLiteChain) { + fill.repaymentChainId = fill.destinationChainId; + } } if (!isValidEvmAddress(fill.relayer)) { From 49fa55dbd37accb8226bb6df781aa200e15e17a9 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 17:56:07 -0500 Subject: [PATCH 10/23] handle lite chains --- src/clients/BundleDataClient/utils/FillUtils.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index d0370fd8e..ecf8ca13f 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -74,7 +74,7 @@ export async function verifyFillRepayment( // If repayment chain doesn't have a Pool Rebalance Route for the input token, then change the repayment // chain to the destination chain. - if (!isSlowFill(fill)) { + if (!isSlowFill(fill) && !matchedDeposit.fromLiteChain) { try { const l1TokenCounterpart = hubPoolClient.getL1TokenForL2TokenAtBlock( fill.inputToken, @@ -100,6 +100,10 @@ export async function verifyFillRepayment( if (!chainIsEvm(repaymentChainId)) { if (!matchedDeposit.fromLiteChain) { fill.repaymentChainId = fill.destinationChainId; + } else { + // Since we can't switch the repayment chain for a lite chain deposit, we return undefined here + // because the origin chain is nota valid repayment chain. + return undefined; } } From f5ed7555a886829ed6e0ee3c1cadaa117857ed9b Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 18:22:25 -0500 Subject: [PATCH 11/23] Refactor and finish tests --- .../BundleDataClient/BundleDataClient.ts | 3 +- .../BundleDataClient/utils/FillUtils.ts | 63 ++++++++----------- test/FillUtils.ts | 36 +++-------- 3 files changed, 35 insertions(+), 67 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index 3152e8cad..0f1131eb6 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -50,7 +50,6 @@ import { getRefundsFromBundle, getWidestPossibleExpectedBlockRange, isChainDisabled, - isEvmRepaymentValid, PoolRebalanceRoot, prettyPrintV3SpokePoolEvents, V3DepositWithBlock, @@ -95,7 +94,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]) { diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index ecf8ca13f..b584e5922 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/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( @@ -49,15 +49,7 @@ 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): boolean { - // Slow fills don't result in repayments so they're always valid. - if (isSlowFill(fill)) { - return true; - } - return chainIsEvm(repaymentChainId) && isValidEvmAddress(fill.relayer); + return matchedDeposit.fromLiteChain ? matchedDeposit.originChainId : fill.repaymentChainId; } // Verify that a fill sent to an EVM chain has a 20 byte address. If the fill does not, then attempt @@ -70,6 +62,11 @@ export async function verifyFillRepayment( ): Promise { const fill = _.cloneDeep(_fill); + // Slow fills don't result in repayments so they're always valid. + if (isSlowFill(fill)) { + return fill; + } + let repaymentChainId = getRepaymentChainId(fill, matchedDeposit); // If repayment chain doesn't have a Pool Rebalance Route for the input token, then change the repayment @@ -88,37 +85,29 @@ export async function verifyFillRepayment( repaymentChainId = fill.destinationChainId; } } - const validEvmRepayment = isEvmRepaymentValid(fill, repaymentChainId); - // Case 1: Repayment chain is EVM and repayment address is valid EVM address. - if (validEvmRepayment) { - return fill; - } - // Case 2: Repayment chain is not EVM or address is not a valid EVM address. Attempt to switch repayment - // address to msg.sender of relay transaction and repayment chain to destination chain. - else { - if (!chainIsEvm(repaymentChainId)) { - if (!matchedDeposit.fromLiteChain) { - fill.repaymentChainId = fill.destinationChainId; - } else { - // Since we can't switch the repayment chain for a lite chain deposit, we return undefined here - // because the origin chain is nota valid repayment chain. - return undefined; - } + 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); + 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)) { + return undefined; } - - 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); - 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)) { + if (!matchedDeposit.fromLiteChain) { + fill.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; + fill.relayer = destinationRelayer; } + + // 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. + return fill; } diff --git a/test/FillUtils.ts b/test/FillUtils.ts index f2a4af12a..c1ff4ea0b 100644 --- a/test/FillUtils.ts +++ b/test/FillUtils.ts @@ -25,7 +25,6 @@ describe("FillUtils", function () { let hubPoolClient: MockHubPoolClient; let owner: SignerWithAddress; - const NOT_VALID_EVM_CHAIN = 9999; const INVALID_EVM_ADDRESS = createRandomBytes32(); beforeEach(async function () { @@ -79,6 +78,8 @@ describe("FillUtils", function () { hubPoolClient.setTokenMapping(ZERO_ADDRESS, fill.repaymentChainId, ZERO_ADDRESS); const result = await verifyFillRepayment(fill, spokeProvider, deposit, hubPoolClient); expect(result).to.not.be.undefined; + expect(result!.repaymentChainId).to.equal(fill.repaymentChainId); + expect(result!.relayer).to.equal(fill.relayer); }); it("SlowFill always valid", async function () { // We don't set the repayment chain mapping for the input token because a slow fill should always be valid. @@ -88,7 +89,7 @@ describe("FillUtils", function () { expect(result).to.not.be.undefined; expect(slowFill.relayExecutionInfo.fillType).to.equal(FillType.SlowFill); }); - it("Lite chain originChain used as repayment and origin chain is valid repayment chain", async function () { + it("Lite chain originChain used as repayment and relayer address is valid", async function () { // We don't set repayment chain mapping since repayment happens on origin chain. const liteChainDeposit = { ...deposit, @@ -96,32 +97,11 @@ describe("FillUtils", function () { }; const result = await verifyFillRepayment(fill, spokeProvider, liteChainDeposit, hubPoolClient); expect(result).to.not.be.undefined; + expect(result!.relayer).to.equal(relayer); // Repayment chain is untouched, it will be modified when computing bundle refunds. expect(result!.repaymentChainId).to.equal(repaymentChainId); - }); - it("Lite chain originChain used as repayment but origin chain is invalid repayment chain", async function () { - hubPoolClient.deleteTokenMapping(ZERO_ADDRESS, deposit.originChainId); - const liteChainDeposit = { - ...deposit, - fromLiteChain: true, - }; - const liteChainFill = { - ...fill, - originChainId: NOT_VALID_EVM_CHAIN, - }; - const result = await verifyFillRepayment(liteChainFill, spokeProvider, liteChainDeposit, hubPoolClient); - expect(result).to.be.undefined; - }); - it("Repayment chain is invalid", async function () { - hubPoolClient.deleteTokenMapping(ZERO_ADDRESS, fill.repaymentChainId); - // valid chain ID's doesn't contain repayment chain. - const invalidRepaymentFill = { - ...fill, - repaymentChainId: NOT_VALID_EVM_CHAIN, - }; - const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, deposit, hubPoolClient); - expect(result).to.be.undefined; + expect(result!.relayer).to.equal(relayer); }); it("Lite chain deposit and relayer is not valid EVM address; relayer gets overwritten to msg.sender", async function () { // We don't set repayment chain mapping since repayment happens on origin chain. @@ -139,7 +119,7 @@ describe("FillUtils", function () { const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, liteChainDeposit, hubPoolClient); expect(result).to.not.be.undefined; expect(result!.relayer).to.equal(relayer); - // Repayment chain is untouched. + // Repayment chain is untouched, will be modified when computing bundle refunds. expect(result!.repaymentChainId).to.equal(repaymentChainId); }); it("Relayer is not valid EVM address, relayer gets overwritten to msg.sender", async function () { @@ -155,8 +135,8 @@ describe("FillUtils", function () { const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, deposit, hubPoolClient); expect(result).to.not.be.undefined; expect(result!.relayer).to.equal(relayer); - // Repayment chain is untouched. - expect(result!.repaymentChainId).to.equal(repaymentChainId); + // Repayment chain gets overwritten to destination chain. + expect(result!.repaymentChainId).to.equal(destinationChainId); }); it("Lite chain deposit and relayer is not valid EVM address; msg.sender is invalid", async function () { // We don't set repayment chain mapping since repayment happens on origin chain. From c18383b4c41186e9ac28e24abc025d0af025c9e1 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 18:39:20 -0500 Subject: [PATCH 12/23] fix loadApproximateRefunds function --- .../BundleDataClient/BundleDataClient.ts | 27 +++++++++---------- .../BundleDataClient/utils/FillUtils.ts | 10 ++++++- test/FillUtils.ts | 6 ++--- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index 0f1131eb6..1d56e3a82 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -33,7 +33,6 @@ import { getImpliedBundleBlockRanges, isSlowFill, mapAsync, - filterAsync, bnUint32Max, isZeroValueDeposit, findFillEvent, @@ -332,7 +331,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] || @@ -349,22 +348,20 @@ export class BundleDataClient { const hasMatchingDeposit = matchingDeposit !== undefined && this.getRelayHashFromEvent(fill) === this.getRelayHashFromEvent(matchingDeposit); - if (hasMatchingDeposit) { - const validRepayment = await verifyFillRepayment( - fill, - this.spokePoolClients[fill.destinationChainId].spokePool.provider, - matchingDeposit, - this.clients.hubPoolClient - ); - 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, diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index b584e5922..a881bdc96 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -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; } @@ -81,6 +82,12 @@ export async function verifyFillRepayment( hubPoolClient.getL2TokenForL1TokenAtBlock(l1TokenCounterpart, repaymentChainId, matchedDeposit.quoteBlockNumber); // Repayment token could be found, this is a valid repayment chain. } catch { + hubPoolClient.logger.warn({ + at: "verifyFillRepayment", + message: + "Filler selected repayment chain that does not have a PoolRebalanceRoute for the input token, overriding to destination chain", + fill, + }); // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler there. repaymentChainId = fill.destinationChainId; } @@ -96,7 +103,7 @@ export async function verifyFillRepayment( return undefined; } if (!matchedDeposit.fromLiteChain) { - fill.repaymentChainId = fill.destinationChainId; + 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. @@ -109,5 +116,6 @@ export async function verifyFillRepayment( // 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; } diff --git a/test/FillUtils.ts b/test/FillUtils.ts index c1ff4ea0b..3adf2ca9d 100644 --- a/test/FillUtils.ts +++ b/test/FillUtils.ts @@ -99,8 +99,7 @@ describe("FillUtils", function () { expect(result).to.not.be.undefined; expect(result!.relayer).to.equal(relayer); - // Repayment chain is untouched, it will be modified when computing bundle refunds. - expect(result!.repaymentChainId).to.equal(repaymentChainId); + expect(result!.repaymentChainId).to.equal(originChainId); expect(result!.relayer).to.equal(relayer); }); it("Lite chain deposit and relayer is not valid EVM address; relayer gets overwritten to msg.sender", async function () { @@ -119,8 +118,7 @@ describe("FillUtils", function () { const result = await verifyFillRepayment(invalidRepaymentFill, spokeProvider, liteChainDeposit, hubPoolClient); expect(result).to.not.be.undefined; expect(result!.relayer).to.equal(relayer); - // Repayment chain is untouched, will be modified when computing bundle refunds. - expect(result!.repaymentChainId).to.equal(repaymentChainId); + expect(result!.repaymentChainId).to.equal(originChainId); }); it("Relayer is not valid EVM address, relayer gets overwritten to msg.sender", async function () { hubPoolClient.setTokenMapping(ZERO_ADDRESS, fill.repaymentChainId, ZERO_ADDRESS); From 4d0905d8dbb73a39c2b7fff746f07fb846346e0c Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 18:40:47 -0500 Subject: [PATCH 13/23] Update FillUtils.ts --- src/clients/BundleDataClient/utils/FillUtils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index a881bdc96..67e32fe7c 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -87,6 +87,7 @@ export async function verifyFillRepayment( message: "Filler selected repayment chain that does not have a PoolRebalanceRoute for the input token, overriding to destination chain", fill, + notificationPath: "across-unrepayable-fills", }); // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler there. repaymentChainId = fill.destinationChainId; From 9ea4ce3fcd65db5ea5ef1fea84661dece29be422 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 18:44:39 -0500 Subject: [PATCH 14/23] Increase test robustness --- test/FillUtils.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/FillUtils.ts b/test/FillUtils.ts index 3adf2ca9d..8283c3bd9 100644 --- a/test/FillUtils.ts +++ b/test/FillUtils.ts @@ -83,6 +83,7 @@ describe("FillUtils", function () { }); it("SlowFill always valid", async function () { // We don't set the repayment chain mapping for the input token because a slow fill should always be valid. + hubPoolClient.deleteTokenMapping(ZERO_ADDRESS, fill.repaymentChainId); const slowFill = { ...fill }; slowFill.relayExecutionInfo.fillType = FillType.SlowFill; const result = await verifyFillRepayment(slowFill, spokeProvider, deposit, hubPoolClient); @@ -91,6 +92,7 @@ describe("FillUtils", function () { }); it("Lite chain originChain used as repayment and relayer address is valid", async function () { // We don't set repayment chain mapping since repayment happens on origin chain. + hubPoolClient.deleteTokenMapping(ZERO_ADDRESS, fill.repaymentChainId); const liteChainDeposit = { ...deposit, fromLiteChain: true, @@ -104,6 +106,7 @@ describe("FillUtils", function () { }); it("Lite chain deposit and relayer is not valid EVM address; relayer gets overwritten to msg.sender", async function () { // We don't set repayment chain mapping since repayment happens on origin chain. + hubPoolClient.deleteTokenMapping(ZERO_ADDRESS, fill.repaymentChainId); const liteChainDeposit = { ...deposit, fromLiteChain: true, @@ -138,6 +141,7 @@ describe("FillUtils", function () { }); it("Lite chain deposit and relayer is not valid EVM address; msg.sender is invalid", async function () { // We don't set repayment chain mapping since repayment happens on origin chain. + hubPoolClient.deleteTokenMapping(ZERO_ADDRESS, fill.repaymentChainId); const liteChainDeposit = { ...deposit, fromLiteChain: true, From 307dbd3c03907469744dc894300b58500bea27e8 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 18:46:23 -0500 Subject: [PATCH 15/23] Remove log --- src/clients/BundleDataClient/utils/FillUtils.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index 67e32fe7c..d6b499f0a 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -82,13 +82,6 @@ export async function verifyFillRepayment( hubPoolClient.getL2TokenForL1TokenAtBlock(l1TokenCounterpart, repaymentChainId, matchedDeposit.quoteBlockNumber); // Repayment token could be found, this is a valid repayment chain. } catch { - hubPoolClient.logger.warn({ - at: "verifyFillRepayment", - message: - "Filler selected repayment chain that does not have a PoolRebalanceRoute for the input token, overriding to destination chain", - fill, - notificationPath: "across-unrepayable-fills", - }); // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler there. repaymentChainId = fill.destinationChainId; } From a1b1a03018507a6f06eea966184a6b76b38afaf2 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 18:46:57 -0500 Subject: [PATCH 16/23] bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 69d950e03..b98dc8ec7 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@across-protocol/sdk", "author": "UMA Team", - "version": "4.0.1", + "version": "4.0.2", "license": "AGPL-3.0", "homepage": "https://docs.across.to/reference/sdk", "files": [ From 2df9b403859bdc5d542f53631a31fe51455a2ce2 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 19:42:39 -0500 Subject: [PATCH 17/23] Fix --- .../BundleDataClient/utils/FillUtils.ts | 39 ++++++++++++------- src/clients/SpokePoolClient.ts | 29 +++++++++----- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index d6b499f0a..0208e7cf1 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -53,6 +53,30 @@ export function getRepaymentChainId(fill: Fill, matchedDeposit: Deposit): number return matchedDeposit.fromLiteChain ? matchedDeposit.originChainId : fill.repaymentChainId; } +export function overwriteRepaymentChain( + repaymentChainId: number, + matchedDeposit: Deposit & { quoteBlockNumber: number }, + hubPoolClient: HubPoolClient +): boolean { + 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; + } +} + // 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( @@ -72,19 +96,8 @@ export async function verifyFillRepayment( // If repayment chain doesn't have a Pool Rebalance Route for the input token, then change the repayment // chain to the destination chain. - if (!isSlowFill(fill) && !matchedDeposit.fromLiteChain) { - try { - const l1TokenCounterpart = hubPoolClient.getL1TokenForL2TokenAtBlock( - fill.inputToken, - fill.originChainId, - matchedDeposit.quoteBlockNumber - ); - hubPoolClient.getL2TokenForL1TokenAtBlock(l1TokenCounterpart, repaymentChainId, matchedDeposit.quoteBlockNumber); - // Repayment token could be found, this is a valid repayment chain. - } catch { - // Repayment token doesn't exist on repayment chain via PoolRebalanceRoutes, impossible to repay filler there. - repaymentChainId = fill.destinationChainId; - } + if (!isSlowFill(fill) && overwriteRepaymentChain(repaymentChainId, matchedDeposit, hubPoolClient)) { + repaymentChainId = fill.destinationChainId; } if (!isValidEvmAddress(fill.relayer)) { diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts index 9419ec3ac..2cd121b28 100644 --- a/src/clients/SpokePoolClient.ts +++ b/src/clients/SpokePoolClient.ts @@ -15,6 +15,8 @@ import { bnOne, getMessageHash, isUnsafeDepositId, + isSlowFill, + isValidEvmAddress, } from "../utils"; import { paginatedEventQuery, @@ -44,7 +46,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, overwriteRepaymentChain } from "./BundleDataClient/utils/FillUtils"; type SpokePoolUpdateSuccess = { success: true; @@ -388,14 +390,21 @@ 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. + if ( + this.hubPoolClient && + !isSlowFill(fill) && + (!isValidEvmAddress(fill.relayer) || + overwriteRepaymentChain( + 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 @@ -426,7 +435,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", From 65743400bb010403e6886a98a2812287ed762250 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 19:43:31 -0500 Subject: [PATCH 18/23] fix --- src/clients/BundleDataClient/utils/FillUtils.ts | 4 ++-- src/clients/SpokePoolClient.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index 0208e7cf1..fac06904e 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -53,7 +53,7 @@ export function getRepaymentChainId(fill: Fill, matchedDeposit: Deposit): number return matchedDeposit.fromLiteChain ? matchedDeposit.originChainId : fill.repaymentChainId; } -export function overwriteRepaymentChain( +export function willOverwriteRepaymentChain( repaymentChainId: number, matchedDeposit: Deposit & { quoteBlockNumber: number }, hubPoolClient: HubPoolClient @@ -96,7 +96,7 @@ export async function verifyFillRepayment( // If repayment chain doesn't have a Pool Rebalance Route for the input token, then change the repayment // chain to the destination chain. - if (!isSlowFill(fill) && overwriteRepaymentChain(repaymentChainId, matchedDeposit, hubPoolClient)) { + if (!isSlowFill(fill) && willOverwriteRepaymentChain(repaymentChainId, matchedDeposit, hubPoolClient)) { repaymentChainId = fill.destinationChainId; } diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts index 2cd121b28..a039e72fe 100644 --- a/src/clients/SpokePoolClient.ts +++ b/src/clients/SpokePoolClient.ts @@ -46,7 +46,7 @@ import { getBlockRangeForDepositId, getDepositIdAtBlock, relayFillStatus } from import { BaseAbstractClient, isUpdateFailureReason, UpdateFailureReason } from "./BaseAbstractClient"; import { HubPoolClient } from "./HubPoolClient"; import { AcrossConfigStoreClient } from "./AcrossConfigStoreClient"; -import { getRepaymentChainId, overwriteRepaymentChain } from "./BundleDataClient/utils/FillUtils"; +import { getRepaymentChainId, overwriteRepaymentChain, willOverwriteRepaymentChain } from "./BundleDataClient/utils/FillUtils"; type SpokePoolUpdateSuccess = { success: true; @@ -399,7 +399,7 @@ export class SpokePoolClient extends BaseAbstractClient { this.hubPoolClient && !isSlowFill(fill) && (!isValidEvmAddress(fill.relayer) || - overwriteRepaymentChain( + willOverwriteRepaymentChain( repaymentChainId, { ...deposit, quoteBlockNumber: this.hubPoolClient!.latestBlockSearched }, this.hubPoolClient From 6ce43e892de6a0f267fdd4c7519b0c0639e59685 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 19:43:56 -0500 Subject: [PATCH 19/23] Update SpokePoolClient.ts --- src/clients/SpokePoolClient.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts index a039e72fe..07e13a861 100644 --- a/src/clients/SpokePoolClient.ts +++ b/src/clients/SpokePoolClient.ts @@ -394,7 +394,8 @@ export class SpokePoolClient extends BaseAbstractClient { // 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. + // 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) && From 1a693357a24d1ea9e196569d026fa977dfff1b61 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 19:48:36 -0500 Subject: [PATCH 20/23] fix --- package.json | 2 +- src/clients/SpokePoolClient.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index b98dc8ec7..22a50c284 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@across-protocol/sdk", "author": "UMA Team", - "version": "4.0.2", + "version": "4.0.3-beta.0", "license": "AGPL-3.0", "homepage": "https://docs.across.to/reference/sdk", "files": [ diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts index 07e13a861..6fb931fcd 100644 --- a/src/clients/SpokePoolClient.ts +++ b/src/clients/SpokePoolClient.ts @@ -46,7 +46,7 @@ import { getBlockRangeForDepositId, getDepositIdAtBlock, relayFillStatus } from import { BaseAbstractClient, isUpdateFailureReason, UpdateFailureReason } from "./BaseAbstractClient"; import { HubPoolClient } from "./HubPoolClient"; import { AcrossConfigStoreClient } from "./AcrossConfigStoreClient"; -import { getRepaymentChainId, overwriteRepaymentChain, willOverwriteRepaymentChain } from "./BundleDataClient/utils/FillUtils"; +import { getRepaymentChainId, willOverwriteRepaymentChain } from "./BundleDataClient/utils/FillUtils"; type SpokePoolUpdateSuccess = { success: true; From 3c57e883642e32c27cf24e316c24a06ac9cfa723 Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 19:56:41 -0500 Subject: [PATCH 21/23] Update package.json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 22a50c284..d2ad54b29 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@across-protocol/sdk", "author": "UMA Team", - "version": "4.0.3-beta.0", + "version": "4.0.3", "license": "AGPL-3.0", "homepage": "https://docs.across.to/reference/sdk", "files": [ From 0cfceda39ca7c5929e7c1d15975d8ae73e134dfc Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Wed, 5 Feb 2025 20:20:26 -0500 Subject: [PATCH 22/23] forceDestinationRepayment --- src/clients/BundleDataClient/utils/FillUtils.ts | 4 ++-- src/clients/SpokePoolClient.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index fac06904e..c3af876b3 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -53,7 +53,7 @@ export function getRepaymentChainId(fill: Fill, matchedDeposit: Deposit): number return matchedDeposit.fromLiteChain ? matchedDeposit.originChainId : fill.repaymentChainId; } -export function willOverwriteRepaymentChain( +export function forceDestinationRepayment( repaymentChainId: number, matchedDeposit: Deposit & { quoteBlockNumber: number }, hubPoolClient: HubPoolClient @@ -96,7 +96,7 @@ export async function verifyFillRepayment( // If repayment chain doesn't have a Pool Rebalance Route for the input token, then change the repayment // chain to the destination chain. - if (!isSlowFill(fill) && willOverwriteRepaymentChain(repaymentChainId, matchedDeposit, hubPoolClient)) { + if (!isSlowFill(fill) && forceDestinationRepayment(repaymentChainId, matchedDeposit, hubPoolClient)) { repaymentChainId = fill.destinationChainId; } diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts index 6fb931fcd..ff8b4f4d2 100644 --- a/src/clients/SpokePoolClient.ts +++ b/src/clients/SpokePoolClient.ts @@ -46,7 +46,7 @@ import { getBlockRangeForDepositId, getDepositIdAtBlock, relayFillStatus } from import { BaseAbstractClient, isUpdateFailureReason, UpdateFailureReason } from "./BaseAbstractClient"; import { HubPoolClient } from "./HubPoolClient"; import { AcrossConfigStoreClient } from "./AcrossConfigStoreClient"; -import { getRepaymentChainId, willOverwriteRepaymentChain } from "./BundleDataClient/utils/FillUtils"; +import { getRepaymentChainId, forceDestinationRepayment } from "./BundleDataClient/utils/FillUtils"; type SpokePoolUpdateSuccess = { success: true; @@ -400,7 +400,7 @@ export class SpokePoolClient extends BaseAbstractClient { this.hubPoolClient && !isSlowFill(fill) && (!isValidEvmAddress(fill.relayer) || - willOverwriteRepaymentChain( + forceDestinationRepayment( repaymentChainId, { ...deposit, quoteBlockNumber: this.hubPoolClient!.latestBlockSearched }, this.hubPoolClient From 68b52fce4f3cb32350a288de1aa64e9cbd718bae Mon Sep 17 00:00:00 2001 From: nicholaspai Date: Thu, 6 Feb 2025 10:27:01 -0500 Subject: [PATCH 23/23] Update FillUtils.ts --- src/clients/BundleDataClient/utils/FillUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/clients/BundleDataClient/utils/FillUtils.ts b/src/clients/BundleDataClient/utils/FillUtils.ts index c3af876b3..9794b49dc 100644 --- a/src/clients/BundleDataClient/utils/FillUtils.ts +++ b/src/clients/BundleDataClient/utils/FillUtils.ts @@ -96,7 +96,7 @@ export async function verifyFillRepayment( // If repayment chain doesn't have a Pool Rebalance Route for the input token, then change the repayment // chain to the destination chain. - if (!isSlowFill(fill) && forceDestinationRepayment(repaymentChainId, matchedDeposit, hubPoolClient)) { + if (forceDestinationRepayment(repaymentChainId, matchedDeposit, hubPoolClient)) { repaymentChainId = fill.destinationChainId; }