From 0ff3828344654cb5a0802a18c513ccf6eb45fa45 Mon Sep 17 00:00:00 2001 From: james-a-morris Date: Wed, 4 Sep 2024 15:36:30 -0400 Subject: [PATCH 1/3] improve: reduce remove direct contract/rpc calls in bundle client Signed-off-by: james-a-morris --- .../BundleDataClient/BundleDataClient.ts | 8 ++--- src/clients/SpokePoolClient.ts | 30 ++++++++++++++++ src/utils/SpokeUtils.ts | 35 +++---------------- 3 files changed, 38 insertions(+), 35 deletions(-) diff --git a/src/clients/BundleDataClient/BundleDataClient.ts b/src/clients/BundleDataClient/BundleDataClient.ts index 2971f928..5a62d49a 100644 --- a/src/clients/BundleDataClient/BundleDataClient.ts +++ b/src/clients/BundleDataClient/BundleDataClient.ts @@ -29,7 +29,6 @@ import { getImpliedBundleBlockRanges, isSlowFill, mapAsync, - relayFillStatus, bnUint32Max, } from "../../utils"; import { BigNumber } from "ethers"; @@ -1125,8 +1124,7 @@ export class BundleDataClient { ) { // If we haven't seen a fill matching this deposit, then we need to rule out that it was filled a long time ago // by checkings its on-chain fill status. - const fillStatus = await relayFillStatus( - spokePoolClients[destinationChainId].spokePool, + const fillStatus = await spokePoolClients[destinationChainId].relayFillStatus( deposit, // We can assume that in production // the block ranges passed into this function would never contain blocks where the spoke pool client @@ -1313,8 +1311,8 @@ export class BundleDataClient { const startBlockForChain = Math.min(_startBlockForChain, spokePoolClient.latestBlockSearched); const endBlockForChain = Math.min(_endBlockForChain, spokePoolClient.latestBlockSearched); const [startTime, endTime] = [ - Number((await spokePoolClient.spokePool.provider.getBlock(startBlockForChain)).timestamp), - Number((await spokePoolClient.spokePool.provider.getBlock(endBlockForChain)).timestamp), + await spokePoolClient.getTimestampForBlock(startBlockForChain), + await spokePoolClient.getTimestampForBlock(endBlockForChain), ]; // Sanity checks: assert(endTime >= startTime, "End time should be greater than start time."); diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts index f82490c3..1ad3dcf0 100644 --- a/src/clients/SpokePoolClient.ts +++ b/src/clients/SpokePoolClient.ts @@ -25,6 +25,7 @@ import { Deposit, DepositWithBlock, Fill, + FillStatus, FillWithBlock, FilledV3RelayEvent, RealizedLpFee, @@ -883,4 +884,33 @@ export class SpokePoolClient extends BaseAbstractClient { this.configStoreClient?.isChainLiteChainAtTimestamp(deposit.destinationChainId, deposit.quoteTimestamp) ?? false ); } + + public async getTimestampForBlock(blockTag: number): Promise { + const block = await this.spokePool.provider.getBlock(blockTag); + return Number(block.timestamp); + } + + /** + * Find the amount filled for a deposit at a particular block. + * @param relayData Deposit information that is used to complete a fill. + * @param blockTag Block tag (numeric or "latest") to query at. + * @returns The amount filled for the specified deposit at the requested block (or latest). + */ + public async relayFillStatus( + relayData: RelayData, + blockTag?: number | "latest", + destinationChainId?: number + ): Promise { + destinationChainId ??= this.chainId; + const hash = getRelayDataHash(relayData, destinationChainId!); + const _fillStatus = await this.spokePool.fillStatuses(hash, { blockTag }); + const fillStatus = Number(_fillStatus); + if (![FillStatus.Unfilled, FillStatus.RequestedSlowFill, FillStatus.Filled].includes(fillStatus)) { + const { originChainId, depositId } = relayData; + throw new Error( + `relayFillStatus: Unexpected fillStatus for ${originChainId} deposit ${depositId} (${fillStatus})` + ); + } + return fillStatus; + } } diff --git a/src/utils/SpokeUtils.ts b/src/utils/SpokeUtils.ts index 554b777f..de840fc7 100644 --- a/src/utils/SpokeUtils.ts +++ b/src/utils/SpokeUtils.ts @@ -244,32 +244,6 @@ export function getRelayHashFromEvent(e: Deposit | Fill | SlowFillRequest): stri return getRelayDataHash(e, e.destinationChainId); } -/** - * Find the amount filled for a deposit at a particular block. - * @param spokePool SpokePool contract instance. - * @param relayData Deposit information that is used to complete a fill. - * @param blockTag Block tag (numeric or "latest") to query at. - * @returns The amount filled for the specified deposit at the requested block (or latest). - */ -export async function relayFillStatus( - spokePool: Contract, - relayData: RelayData, - blockTag?: number | "latest", - destinationChainId?: number -): Promise { - destinationChainId ??= await spokePool.chainId(); - const hash = getRelayDataHash(relayData, destinationChainId!); - const _fillStatus = await spokePool.fillStatuses(hash, { blockTag }); - const fillStatus = Number(_fillStatus); - - if (![FillStatus.Unfilled, FillStatus.RequestedSlowFill, FillStatus.Filled].includes(fillStatus)) { - const { originChainId, depositId } = relayData; - throw new Error(`relayFillStatus: Unexpected fillStatus for ${originChainId} deposit ${depositId} (${fillStatus})`); - } - - return fillStatus; -} - export async function fillStatusArray( spokePool: Contract, relayData: RelayData[], @@ -316,11 +290,12 @@ export async function fillStatusArray( * @returns The block number at which the relay was completed, or undefined. */ export async function findFillBlock( - spokePool: Contract, + spokePoolClient: SpokePoolClient, relayData: RelayData, lowBlockNumber: number, highBlockNumber?: number ): Promise { + const { spokePool } = spokePoolClient; const { provider } = spokePool; highBlockNumber ??= await provider.getBlockNumber(); assert(highBlockNumber > lowBlockNumber, `Block numbers out of range (${lowBlockNumber} > ${highBlockNumber})`); @@ -340,8 +315,8 @@ export async function findFillBlock( // Make sure the relay war completed within the block range supplied by the caller. const [initialFillStatus, finalFillStatus] = ( await Promise.all([ - relayFillStatus(spokePool, relayData, lowBlockNumber, destinationChainId), - relayFillStatus(spokePool, relayData, highBlockNumber, destinationChainId), + spokePoolClient.relayFillStatus(relayData, lowBlockNumber, destinationChainId), + spokePoolClient.relayFillStatus(relayData, highBlockNumber, destinationChainId), ]) ).map(Number); @@ -359,7 +334,7 @@ export async function findFillBlock( // Find the leftmost block where filledAmount equals the deposit amount. do { const midBlockNumber = Math.floor((highBlockNumber + lowBlockNumber) / 2); - const fillStatus = await relayFillStatus(spokePool, relayData, midBlockNumber, destinationChainId); + const fillStatus = await spokePoolClient.relayFillStatus(relayData, midBlockNumber, destinationChainId); if (fillStatus === FillStatus.Filled) { highBlockNumber = midBlockNumber; From 89074c2876b8cd8973b397e2a20daa0fa767a3c5 Mon Sep 17 00:00:00 2001 From: james-a-morris Date: Wed, 4 Sep 2024 17:42:47 -0400 Subject: [PATCH 2/3] improve: migrate tests Signed-off-by: james-a-morris --- test/SpokePoolClient.ValidateFill.ts | 20 ++++++++++++-------- test/SpokePoolClient.fills.ts | 15 ++++++++------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/test/SpokePoolClient.ValidateFill.ts b/test/SpokePoolClient.ValidateFill.ts index 51e21edc..b22ea625 100644 --- a/test/SpokePoolClient.ValidateFill.ts +++ b/test/SpokePoolClient.ValidateFill.ts @@ -4,9 +4,9 @@ import { bnOne, InvalidFill, fillStatusArray, - relayFillStatus, validateFillForDeposit, queryHistoricalDepositForFill, + DepositSearchResult, } from "../src/utils"; import { CHAIN_ID_TEST_LIST, originChainId, destinationChainId, repaymentChainId } from "./constants"; import { @@ -126,11 +126,11 @@ describe("SpokePoolClient: Fill Validation", function () { outputAmount ); - let filled = await relayFillStatus(spokePool_2, deposit); + let filled = await spokePoolClient2.relayFillStatus(deposit); expect(filled).to.equal(FillStatus.Unfilled); await fillV3Relay(spokePool_2, deposit, relayer); - filled = await relayFillStatus(spokePool_2, deposit); + filled = await spokePoolClient2.relayFillStatus(deposit); expect(filled).to.equal(FillStatus.Filled); }); @@ -439,7 +439,9 @@ describe("SpokePoolClient: Fill Validation", function () { const historicalDeposit = await queryHistoricalDepositForFill(spokePoolClient1, fill); assert.equal(historicalDeposit.found, true, "Test is broken"); // Help tsc to narrow the discriminated union. - expect(historicalDeposit.deposit.depositId).to.deep.equal(deposit.depositId); + expect((historicalDeposit as Extract).deposit.depositId).to.deep.equal( + deposit.depositId + ); }); it("Can fetch younger deposit matching fill", async function () { @@ -473,7 +475,9 @@ describe("SpokePoolClient: Fill Validation", function () { const historicalDeposit = await queryHistoricalDepositForFill(spokePoolClient1, fill); assert.equal(historicalDeposit.found, true, "Test is broken"); // Help tsc to narrow the discriminated union. - expect(historicalDeposit.deposit.depositId).to.deep.equal(deposit.depositId); + expect((historicalDeposit as Extract).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 () { @@ -547,7 +551,7 @@ describe("SpokePoolClient: Fill Validation", function () { const search = await queryHistoricalDepositForFill(spokePoolClient1, fill); assert.equal(search.found, false, "Test is broken"); // Help tsc to narrow the discriminated union. - expect(search.code).to.equal(InvalidFill.DepositIdInvalid); + expect((search as Extract).code).to.equal(InvalidFill.DepositIdInvalid); expect(lastSpyLogIncludes(spy, "Queried RPC for deposit")).is.not.true; }); @@ -572,7 +576,7 @@ describe("SpokePoolClient: Fill Validation", function () { const search = await queryHistoricalDepositForFill(spokePoolClient1, fill); assert.equal(search.found, false, "Test is broken"); // Help tsc to narrow the discriminated union. - expect(search.code).to.equal(InvalidFill.DepositIdInvalid); + expect((search as Extract).code).to.equal(InvalidFill.DepositIdInvalid); expect(lastSpyLogIncludes(spy, "Queried RPC for deposit")).is.not.true; }); @@ -594,7 +598,7 @@ describe("SpokePoolClient: Fill Validation", function () { const search = await queryHistoricalDepositForFill(spokePoolClient1, fill); assert.equal(search.found, false, "Test is broken"); // Help tsc to narrow the discriminated union. - expect(search.code).to.equal(InvalidFill.FillMismatch); + expect((search as Extract).code).to.equal(InvalidFill.FillMismatch); }); it("Returns sped up deposit matched with fill", async function () { diff --git a/test/SpokePoolClient.fills.ts b/test/SpokePoolClient.fills.ts index 0703a720..8db461b7 100644 --- a/test/SpokePoolClient.fills.ts +++ b/test/SpokePoolClient.fills.ts @@ -1,6 +1,6 @@ import hre from "hardhat"; import { SpokePoolClient } from "../src/clients"; -import { V3Deposit } from "../src/interfaces"; +import { Deposit } from "../src/interfaces"; import { bnOne, findFillBlock, getNetworkName } from "../src/utils"; import { EMPTY_MESSAGE, ZERO_ADDRESS } from "../src/constants"; import { originChainId, destinationChainId } from "./constants"; @@ -24,7 +24,7 @@ describe("SpokePoolClient: Fills", function () { let depositor: SignerWithAddress, relayer1: SignerWithAddress, relayer2: SignerWithAddress; let spokePoolClient: SpokePoolClient; let deploymentBlock: number; - let deposit: V3Deposit; + let deposit: Deposit; beforeEach(async function () { [, depositor, relayer1, relayer2] = await ethers.getSigners(); @@ -57,12 +57,13 @@ describe("SpokePoolClient: Fills", function () { inputAmount: outputAmount.add(bnOne), outputToken: destErc20.address, outputAmount: toBNWei("1"), - relayerFeePct: toBNWei("0.01"), quoteTimestamp: spokePoolTime - 60, message: EMPTY_MESSAGE, fillDeadline: spokePoolTime + 600, exclusivityDeadline: 0, exclusiveRelayer: ZERO_ADDRESS, + fromLiteChain: false, + toLiteChain: false, }; }); @@ -109,7 +110,7 @@ describe("SpokePoolClient: Fills", function () { await hre.network.provider.send("evm_mine"); } - const fillBlock = await findFillBlock(spokePool, deposit, startBlock); + const fillBlock = await findFillBlock(spokePoolClient, deposit, startBlock); expect(fillBlock).to.equal(targetFillBlock); }); @@ -121,7 +122,7 @@ describe("SpokePoolClient: Fills", function () { } // No fill has been made, so expect an undefined fillBlock. - const fillBlock = await findFillBlock(spokePool, deposit, startBlock); + const fillBlock = await findFillBlock(spokePoolClient, deposit, startBlock); expect(fillBlock).to.be.undefined; const { blockNumber: lateBlockNumber } = await fillV3Relay(spokePool, deposit, relayer1); @@ -130,13 +131,13 @@ describe("SpokePoolClient: Fills", function () { // Now search for the fill _after_ it was filled and expect an exception. const srcChain = getNetworkName(deposit.originChainId); await assertPromiseError( - findFillBlock(spokePool, deposit, lateBlockNumber), + findFillBlock(spokePoolClient, deposit, lateBlockNumber), `${srcChain} deposit ${deposit.depositId} filled on ` ); // Should assert if highBlock <= lowBlock. await assertPromiseError( - findFillBlock(spokePool, deposit, await spokePool.provider.getBlockNumber()), + findFillBlock(spokePoolClient, deposit, await spokePool.provider.getBlockNumber()), "Block numbers out of range" ); }); From 5df05e79c0a35139b0f9d909de138a0b63adf144 Mon Sep 17 00:00:00 2001 From: james-a-morris Date: Thu, 5 Sep 2024 10:23:41 -0400 Subject: [PATCH 3/3] improve: use utility function directly Signed-off-by: james-a-morris --- src/caching/Arweave/ArweaveClient.ts | 22 +++++++++++------ src/clients/SpokePoolClient.ts | 16 +++---------- src/utils/SpokeUtils.ts | 35 ++++++++++++++++++++++++---- test/SpokePoolClient.ValidateFill.ts | 11 ++++++++- test/SpokePoolClient.fills.ts | 8 +++---- test/common.test.ts | 2 +- 6 files changed, 63 insertions(+), 31 deletions(-) diff --git a/src/caching/Arweave/ArweaveClient.ts b/src/caching/Arweave/ArweaveClient.ts index cb2468d5..33efa21b 100644 --- a/src/caching/Arweave/ArweaveClient.ts +++ b/src/caching/Arweave/ArweaveClient.ts @@ -175,13 +175,21 @@ export class ArweaveClient { }); const results = await Promise.all( entries.map(async (edge) => { - const data = await this.get(edge.node.id, validator); - return isDefined(data) - ? { - data, - hash: edge.node.id, - } - : null; + try { + const data = await this.get(edge.node.id, validator); + return isDefined(data) + ? { + data, + hash: edge.node.id, + } + : null; + } catch (e) { + this.logger.warn({ + at: "ArweaveClient:getByTopic", + message: `Bad request for Arweave topic ${edge.node.id}: ${e}`, + }); + return null; + } }) ); return results.filter(isDefined); diff --git a/src/clients/SpokePoolClient.ts b/src/clients/SpokePoolClient.ts index 1ad3dcf0..5bccb3a5 100644 --- a/src/clients/SpokePoolClient.ts +++ b/src/clients/SpokePoolClient.ts @@ -39,7 +39,7 @@ import { } from "../interfaces"; import { SpokePool } from "../typechain"; import { getNetworkName } from "../utils/NetworkUtils"; -import { getBlockRangeForDepositId, getDepositIdAtBlock } from "../utils/SpokeUtils"; +import { getBlockRangeForDepositId, getDepositIdAtBlock, relayFillStatus } from "../utils/SpokeUtils"; import { BaseAbstractClient, isUpdateFailureReason, UpdateFailureReason } from "./BaseAbstractClient"; import { HubPoolClient } from "./HubPoolClient"; import { AcrossConfigStoreClient } from "./AcrossConfigStoreClient"; @@ -896,21 +896,11 @@ export class SpokePoolClient extends BaseAbstractClient { * @param blockTag Block tag (numeric or "latest") to query at. * @returns The amount filled for the specified deposit at the requested block (or latest). */ - public async relayFillStatus( + public relayFillStatus( relayData: RelayData, blockTag?: number | "latest", destinationChainId?: number ): Promise { - destinationChainId ??= this.chainId; - const hash = getRelayDataHash(relayData, destinationChainId!); - const _fillStatus = await this.spokePool.fillStatuses(hash, { blockTag }); - const fillStatus = Number(_fillStatus); - if (![FillStatus.Unfilled, FillStatus.RequestedSlowFill, FillStatus.Filled].includes(fillStatus)) { - const { originChainId, depositId } = relayData; - throw new Error( - `relayFillStatus: Unexpected fillStatus for ${originChainId} deposit ${depositId} (${fillStatus})` - ); - } - return fillStatus; + return relayFillStatus(this.spokePool, relayData, blockTag, destinationChainId); } } diff --git a/src/utils/SpokeUtils.ts b/src/utils/SpokeUtils.ts index de840fc7..554b777f 100644 --- a/src/utils/SpokeUtils.ts +++ b/src/utils/SpokeUtils.ts @@ -244,6 +244,32 @@ export function getRelayHashFromEvent(e: Deposit | Fill | SlowFillRequest): stri return getRelayDataHash(e, e.destinationChainId); } +/** + * Find the amount filled for a deposit at a particular block. + * @param spokePool SpokePool contract instance. + * @param relayData Deposit information that is used to complete a fill. + * @param blockTag Block tag (numeric or "latest") to query at. + * @returns The amount filled for the specified deposit at the requested block (or latest). + */ +export async function relayFillStatus( + spokePool: Contract, + relayData: RelayData, + blockTag?: number | "latest", + destinationChainId?: number +): Promise { + destinationChainId ??= await spokePool.chainId(); + const hash = getRelayDataHash(relayData, destinationChainId!); + const _fillStatus = await spokePool.fillStatuses(hash, { blockTag }); + const fillStatus = Number(_fillStatus); + + if (![FillStatus.Unfilled, FillStatus.RequestedSlowFill, FillStatus.Filled].includes(fillStatus)) { + const { originChainId, depositId } = relayData; + throw new Error(`relayFillStatus: Unexpected fillStatus for ${originChainId} deposit ${depositId} (${fillStatus})`); + } + + return fillStatus; +} + export async function fillStatusArray( spokePool: Contract, relayData: RelayData[], @@ -290,12 +316,11 @@ export async function fillStatusArray( * @returns The block number at which the relay was completed, or undefined. */ export async function findFillBlock( - spokePoolClient: SpokePoolClient, + spokePool: Contract, relayData: RelayData, lowBlockNumber: number, highBlockNumber?: number ): Promise { - const { spokePool } = spokePoolClient; const { provider } = spokePool; highBlockNumber ??= await provider.getBlockNumber(); assert(highBlockNumber > lowBlockNumber, `Block numbers out of range (${lowBlockNumber} > ${highBlockNumber})`); @@ -315,8 +340,8 @@ export async function findFillBlock( // Make sure the relay war completed within the block range supplied by the caller. const [initialFillStatus, finalFillStatus] = ( await Promise.all([ - spokePoolClient.relayFillStatus(relayData, lowBlockNumber, destinationChainId), - spokePoolClient.relayFillStatus(relayData, highBlockNumber, destinationChainId), + relayFillStatus(spokePool, relayData, lowBlockNumber, destinationChainId), + relayFillStatus(spokePool, relayData, highBlockNumber, destinationChainId), ]) ).map(Number); @@ -334,7 +359,7 @@ export async function findFillBlock( // Find the leftmost block where filledAmount equals the deposit amount. do { const midBlockNumber = Math.floor((highBlockNumber + lowBlockNumber) / 2); - const fillStatus = await spokePoolClient.relayFillStatus(relayData, midBlockNumber, destinationChainId); + const fillStatus = await relayFillStatus(spokePool, relayData, midBlockNumber, destinationChainId); if (fillStatus === FillStatus.Filled) { highBlockNumber = midBlockNumber; diff --git a/test/SpokePoolClient.ValidateFill.ts b/test/SpokePoolClient.ValidateFill.ts index b22ea625..dd8267f9 100644 --- a/test/SpokePoolClient.ValidateFill.ts +++ b/test/SpokePoolClient.ValidateFill.ts @@ -4,6 +4,7 @@ import { bnOne, InvalidFill, fillStatusArray, + relayFillStatus, validateFillForDeposit, queryHistoricalDepositForFill, DepositSearchResult, @@ -126,10 +127,18 @@ describe("SpokePoolClient: Fill Validation", function () { outputAmount ); - let filled = await spokePoolClient2.relayFillStatus(deposit); + let filled = await relayFillStatus(spokePool_2, deposit); + expect(filled).to.equal(FillStatus.Unfilled); + + // Also test spoke client variant + filled = await spokePoolClient2.relayFillStatus(deposit); expect(filled).to.equal(FillStatus.Unfilled); await fillV3Relay(spokePool_2, deposit, relayer); + filled = await relayFillStatus(spokePool_2, deposit); + expect(filled).to.equal(FillStatus.Filled); + + // Also test spoke client variant filled = await spokePoolClient2.relayFillStatus(deposit); expect(filled).to.equal(FillStatus.Filled); }); diff --git a/test/SpokePoolClient.fills.ts b/test/SpokePoolClient.fills.ts index 8db461b7..ff924a77 100644 --- a/test/SpokePoolClient.fills.ts +++ b/test/SpokePoolClient.fills.ts @@ -110,7 +110,7 @@ describe("SpokePoolClient: Fills", function () { await hre.network.provider.send("evm_mine"); } - const fillBlock = await findFillBlock(spokePoolClient, deposit, startBlock); + const fillBlock = await findFillBlock(spokePool, deposit, startBlock); expect(fillBlock).to.equal(targetFillBlock); }); @@ -122,7 +122,7 @@ describe("SpokePoolClient: Fills", function () { } // No fill has been made, so expect an undefined fillBlock. - const fillBlock = await findFillBlock(spokePoolClient, deposit, startBlock); + const fillBlock = await findFillBlock(spokePool, deposit, startBlock); expect(fillBlock).to.be.undefined; const { blockNumber: lateBlockNumber } = await fillV3Relay(spokePool, deposit, relayer1); @@ -131,13 +131,13 @@ describe("SpokePoolClient: Fills", function () { // Now search for the fill _after_ it was filled and expect an exception. const srcChain = getNetworkName(deposit.originChainId); await assertPromiseError( - findFillBlock(spokePoolClient, deposit, lateBlockNumber), + findFillBlock(spokePool, deposit, lateBlockNumber), `${srcChain} deposit ${deposit.depositId} filled on ` ); // Should assert if highBlock <= lowBlock. await assertPromiseError( - findFillBlock(spokePoolClient, deposit, await spokePool.provider.getBlockNumber()), + findFillBlock(spokePool, deposit, await spokePool.provider.getBlockNumber()), "Block numbers out of range" ); }); diff --git a/test/common.test.ts b/test/common.test.ts index a69ae92a..d02fcefb 100644 --- a/test/common.test.ts +++ b/test/common.test.ts @@ -53,7 +53,7 @@ describe("Utils test", () => { await estimateTotalGasRequiredByUnsignedTransaction(fill, relayerAddress, provider, 0.0, gasPrice); expect(toBN(refGasEstimate).eq(toBN(refGasCost).mul(gasPrice))).to.be.true; - for (let gasMarkup = -0.99; gasMarkup <= 4.0; gasMarkup += 0.33) { + for (let gasMarkup = -0.99; gasMarkup <= 1.0; gasMarkup += 0.33) { const { nativeGasCost, tokenGasCost } = await estimateTotalGasRequiredByUnsignedTransaction( fill, relayerAddress,