feat(SvmSpokePoolClient): relayFillStatus and fillStatusArray implementation #990

melisaguevara · 2025-04-24T21:39:02Z

This PR introduces the SVM implementation of relayFillStatus, which resolves a relay's fill status using PDA data or, if necessary, PDA events.

The resolution process is:

Retrieve the fill status PDA for the relay.
If querying the current slot:
- If the PDA exists, fetch the status directly.
- If the PDA doesn't exist but the fill deadline hasn’t passed, solve the status as Unfilled. This relies on the fact that PDAs can't be closed before the fill deadline.
If the status can't be determined using the PDA, or if a specific slot is requested, fetch and sort relevant PDA events (fills and slow fill requests) to resolve it.

This PR also introduces fillStatusArray, a batched version of the same logic for resolving multiple deposits. Note that resolving status via events for large arrays is slow. If it becomes a bottleneck, we could introduce a new Unknown fill status to skip event reconstruction entirely. This should be acceptable, as event-based resolution is only needed for deposits whose fill deadline has already passed or for queries at a specific slot.

Some execution times observed during local testing:

Fetching data from 100 accounts takes ~250ms. Multiple requests can run concurrently.
Resolving statuses via events takes ~9000ms for each batch of 100 deposits.
Note: These timings were measured without caching which could reduce them further.

linear · 2025-04-24T21:39:05Z

ACX-4029 implement relayFillStatus

pxrl · 2025-04-25T08:11:38Z

src/arch/svm/SpokeUtils.ts

+  if (blockTag === "confirmed") {
+    toSlot = await provider.getSlot({ commitment: "confirmed" }).send();
+  } else {
+    toSlot = BigInt(blockTag);


Is it worth validating toSlot >= fromSlot here? (nb. this assumes we retain fromSlot).

I think we can get rid of fromSlot.

pxrl

@melisaguevara This looks great!

pxrl · 2025-04-25T08:57:39Z

src/arch/svm/SpokeUtils.ts

+  const relevantEvents = (
+    await Promise.all(
+      eventsToQuery.map((eventName) =>
+        pdaEventsClient.queryDerivedAddressEvents(eventName, BigInt(fromSlot), toSlot, { limit: 50 })


ooc, is there an increased cost by setting a low fromSlot here? Having to be explicit about fromSlot seems like a burden on the caller; in most (all?) cases I think the caller probably only cares about the upper bound (i.e. was this completed before some previous point in time, or alternatively, what's the current status?).

(If we do need to set reasonable bounds on fromSlot then I wonder if we could infer a safe lower bound based on the relayData.fillDeadline property)

From the events client's perspective, if we don't set fromSlot, it will fetch all events for the address. I think that should be fine for PDAs, since we expect them to have only a few events. The only case where they'd have many is if they were targeted with spam. So it seems safe to remove it, do you agree?

I've optimistically removed it here: 988c565. For now I'm only setting it as undefined when calling svmEventsClient.queryDerivedAddressEvents, but I think we can improve readability if we pass the search range as an object rather than individual params to the events client functions. I can follow up with that change after we merge the update function to avoid conflicts.

pxrl · 2025-04-25T09:05:24Z

src/arch/svm/SpokeUtils.ts

 ): Promise<(FillStatus | undefined)[]> {
-  throw new Error("fillStatusArray: not implemented");
+  assert(chainIsSvm(destinationChainId), "Destination chain must be an SVM chain");
+  const chunkSize = 2;


Is this intended to be 2 or 200? If I read the commit message properly then I think it hints at 200. 2 seems quite low, though we seem to lack any multicall-like read capabilities here, so going for high-parallelism adds the risk of rate-limiting as well :(

As background, the relayer can hit this function with an array of hundreds of relayData objects. If we're querying 2 at a time it's going to take quite a while to resolve that. I also wonder if we might see some nasty overheads given that relayFillStatus() instantiates a new events client for each individual query.

Yes, this was intended to be 200. I was locally testing with 2 😅

But I agree that both rate-limiting and instantiating a new events client for each query might be problematic. For the latter, I'm thinking we can use a single client instance if the pda is not a property of the class but only a param for the function that queries events. And for the former, maybe we can start with 100 parallel requests and see how that goes? Not sure there are many options for this without a multicall-like solution.

0015fb4 This would use a single events client instance.

I'm doing some testing regarding the chunk size to have an idea of how long will it take. Will post some numbers in a bit.

@pxrl The performance of this is really poor. It takes up to 20 seconds to fetch 100 deposits. So I’m looking into using the fill status PDA data when possible. There’s a method that allows querying 100 accounts at once. When checking the latest slot (which is what the relayer does), we can use it to filter out deposits that have either been filled or have a slow fill requested and still have an open PDA.

For deposits that are unfilled or filled but have a closed PDA, I'm afraid we’ll still have to reconstruct from events 😞. I'm still thinking if there's any faster approach for this case.

182cde9 Based on our discussion, now we are also relying on the fill deadline to filter unfilled deposits easily and reduce the amount of statuses we have to resolve with events.

pxrl · 2025-04-25T09:13:16Z

src/arch/svm/SpokeUtils.ts

+  programId: Address,
+  relayData: RelayData,
+  fromSlot: number,
+  blockTag: number | "confirmed" = "confirmed",


If the delta between processed and confirmed is reliably 0 - 2 slots then I wonder if we can just revert to a raw slot number for the upper bound (i.e. toSlot?: number), where toSlot would be a caller-specified slot, or otherwise resolved dynamically to the latest confirmed slot. wdyt?

I agree, I covered this below.

pxrl · 2025-04-25T09:20:31Z

src/clients/SpokePoolClient/SpokePoolClient.ts

-  public abstract relayFillStatus(relayData: RelayData, blockTag?: number | "latest"): Promise<FillStatus>;
+  public abstract relayFillStatus(
+    relayData: RelayData,
+    blockTag?: number | "latest" | "confirmed"


Exposing confirmed at this interface means that the caller needs to be aware of which chain type they're on, and we then need to validate in each chain-specific implementation that the correct tag was passed in. If we modify the type to be just an optional number then we could infer "chain-specific latest" if the input is undefined. That seems more platform-agnostic, though it means that we internally must decide on processed or confirmed for SVM. Based on @james-a-morris feedback about the delta being only 1 - 2 slots, it seems like we could probably live with confirmed for now. wdyt?

Suggested change

blockTag?: number | "latest" | "confirmed"

blockTag?: number

I like this idea! Fwiw, we aren't relying too much on this option, the only place where I see we are using "latest" explicitly is in this line of the Dataworker. For svm it seems safe to use confirmed. So I think this makes sense, wdyt @james-a-morris?

This is how it would look like d3cfa15

I wonder if we should change the name of this param too, blockTag might be confusing when used from the SVM Spoke Pool. Wdyt? It could be "atHeight" or something similar.

…ntation

md0x

Amazing work!

src/arch/svm/SpokeUtils.ts

james-a-morris · 2025-04-29T21:08:53Z

src/arch/svm/SpokeUtils.ts

+    console.warn(
+      "fillStatusArray: Querying specific slots for large arrays is slow. " +
+        "For current status, omit 'atHeight' param to use latest confirmed slot instead."
+    );


leftover log?

No, it was intentional, since reconstructing status from events is really slow I thought it would be useful to have this just in case.

Could be worth having this use a logger intstance - not sure if console goes to gcp in a nicely formatted way

Right, good call! a5b03ce

src/arch/svm/SpokeUtils.ts

james-a-morris

one comment on the logger instance being used over the console

james-a-morris · 2025-05-01T15:56:39Z

src/arch/svm/SpokeUtils.ts

+    console.warn(
+      "fillStatusArray: Querying specific slots for large arrays is slow. " +
+        "For current status, omit 'atHeight' param to use latest confirmed slot instead."
+    );


Could be worth having this use a logger intstance - not sure if console goes to gcp in a nicely formatted way

melisaguevara mentioned this pull request Apr 24, 2025

feat(svmSpokeUtils): svm relayFillStatus implementation #984

Closed

melisaguevara requested review from james-a-morris, pxrl, bmzig, md0x and Reinis-FRP April 24, 2025 21:41

pxrl reviewed Apr 25, 2025

View reviewed changes

melisaguevara added 6 commits April 28, 2025 19:34

feat(SvmSpokePoolClient): relayFillStatus and fillStatusArray impleme…

5c546fb

…ntation

move derived address from class property to function param

680f6f4

remove from slot

5103a17

Remove latest and confirmed as blocktag options

a7fe7d3

use fill status pda and fill deadline to determine fill status

182cde9

remove testing logs

7dad9b1

melisaguevara force-pushed the mguevara/acx-4029-relay-fill-status branch from bd17e8d to 7dad9b1 Compare April 28, 2025 22:41

Improve comments

4af867c

melisaguevara requested a review from pxrl April 29, 2025 02:16

add warning log when fillStatusArray is called for a specific slot

ce58553

md0x approved these changes Apr 29, 2025

View reviewed changes

james-a-morris reviewed Apr 29, 2025

View reviewed changes

melisaguevara requested a review from james-a-morris April 29, 2025 21:42

Reinis-FRP reviewed Apr 30, 2025

View reviewed changes

src/arch/svm/SpokeUtils.ts Outdated Show resolved Hide resolved

src/arch/svm/SpokeUtils.ts Outdated Show resolved Hide resolved

use onchain time for comparison with fill deadline

86f6008

melisaguevara requested a review from Reinis-FRP April 30, 2025 17:39

Reinis-FRP approved these changes May 1, 2025

View reviewed changes

james-a-morris reviewed May 1, 2025

View reviewed changes

md0x approved these changes May 2, 2025

View reviewed changes

use logger instead of console for warning

a5b03ce

melisaguevara requested a review from james-a-morris May 5, 2025 15:16

james-a-morris approved these changes May 6, 2025

View reviewed changes

melisaguevara merged commit eec28b8 into epic/svm-client May 7, 2025
4 checks passed

melisaguevara deleted the mguevara/acx-4029-relay-fill-status branch June 18, 2025 13:11

feat(SvmSpokePoolClient): relayFillStatus and fillStatusArray implementation #990

feat(SvmSpokePoolClient): relayFillStatus and fillStatusArray implementation #990

Uh oh!

Conversation

melisaguevara commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linear bot commented Apr 24, 2025

Uh oh!

pxrl Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pxrl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melisaguevara Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

md0x left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

james-a-morris left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

melisaguevara commented Apr 24, 2025 •

edited

Loading

pxrl Apr 25, 2025 •

edited

Loading

melisaguevara Apr 25, 2025 •

edited

Loading