Skip to content

Relayer: fillStatus[idx] ?? Filled caches unknown status and permanently skips unfilled deposits #3619

Description

@jiapy97

Summary

In Relayer deposit filtering, on-chain fill status is loaded via fillStatusArray, then combined as:

fillStatus: fillStatus[idx] ?? FillStatus.Filled

and written into the long-lived this.fillStatus cache used by later getUnfilledDeposits(..., this.fillStatus) calls.

When the SDK returns undefined for a slot (intentional for undecodable / unresolved statuses, including some SVM cases), this code:

  1. Defaults unknown → Filled
  2. Persists that into this.fillStatus
  3. Causes the deposit to be filtered out as filled on subsequent loops — even if it is still unfilled on-chain

This is a liveness / incorrect fill-decision reliability bug (deposits may never be filled by this process), not a claim of double-spend or fund theft.

Type Reliability — liveness / false “filled” cache
Severity Medium–High (fill path; sticky for process lifetime)
Honest baseline If fillStatusArray never returns undefined in production for a given chain, impact is reduced; the default is still unsafe if undefined is possible

Environment

Item Value
Repo across-protocol/relayer
Commit checked 9bb9d79888e2371a0e616563aed1361e69fba264
File src/relayer/Relayer.ts (approx. ~989–1004)
Related @across-protocol/sdk SpokePoolClient.fillStatusArray (may return undefined)

Code (approx.)

const fillStatus = await spokePoolClients[destinationChainId].fillStatusArray(deposits);
// ...
.map((deposit, idx) => ({
  ...deposit,
  fillStatus: fillStatus[idx] ?? FillStatus.Filled, // dangerous default
}))
.filter(({ fillStatus, ...deposit }) => {
  const depositHash = /* ... */ getDepositHash(deposit);
  this.fillStatus[depositHash] = fillStatus; // long-lived cache
  return fillStatus !== FillStatus.Filled;
});

Later unfilled selection uses this.fillStatus, so a one-time undefined → Filled becomes sticky.


Expected vs actual

Expected Actual
fillStatus[idx] === undefined Treat as Unfilled, or do not cache, retry next loop Treat as Filled
Cache write on unknown Skip or store Unfilled Stores Filled
Later loops Re-query / still eligible if unfilled on-chain Deposit remains skipped for process lifetime

Why this is worse than soft-confirm / div-by-zero issues

ensureConfirmation soft success ProfitClient div0 This issue
Affects Confirmation semantics One evaluation throw Whether the relayer fills at all
Duration Per submit Per call Cached for process life
Path Tx client Profit math Core unfilled selection

Proof of concept (logic)

In-process construction of the same decision rule:

rpcStatus = [Unfilled, undefined]
deposits  = [deposit10, deposit11]

after map:
  deposit10.fillStatus = Unfilled
  deposit11.fillStatus = Filled   // from ?? Filled

filter unfilled → only deposit10

cache["11-<origin>"] = Filled

next loop: even if chain still Unfilled for deposit11,
getUnfilledDeposits(..., this.fillStatus) still drops deposit11

Illustrative snippet:

const FillStatus = { Unfilled: 0, Filled: 2 } as const;
const rpcStatus = [FillStatus.Unfilled, undefined];
const cached: Record<string, number> = {};

const mapped = ["dep-10", "dep-11"].map((id, idx) => {
  const fillStatus = rpcStatus[idx] ?? FillStatus.Filled;
  cached[id] = fillStatus;
  return { id, fillStatus };
});

const unfilled = mapped.filter((d) => d.fillStatus !== FillStatus.Filled);
// unfilled => only dep-10; cached["dep-11"] === Filled

Impact

  • A deposit whose on-chain status cannot be decoded (or is temporarily unresolved) may be marked filled in memory and never selected for fill by this relayer instance.
  • User-facing effect: delayed or missing fills if this relayer was expected to fill (or if the same logic is widespread).
  • Does not claim unauthorized mint or double-fill theft; this is skip / liveness, not “fill twice”.

Suggested fix

  1. Prefer ?? FillStatus.Unfilled (or an explicit Unknown that is treated as eligible / retry).
  2. On undefined, do not write this.fillStatus[depositHash] = Filled (only cache definitive statuses).
  3. Optionally re-fetch status next loop when last status was unknown.
  4. Unit tests:
    • fillStatusArray returns [Unfilled, undefined] → both remain fill-eligible (or second not cached as Filled).
    • After one loop, cache must not permanently block a still-unfilled deposit.

Example:

const status = fillStatus[idx];
if (status === undefined) {
  // do not poison cache; treat as unfilled for this round
  return { ...deposit, fillStatus: FillStatus.Unfilled };
}
this.fillStatus[depositHash] = status;
return { ...deposit, fillStatus: status };

Out of scope

  • TransactionClient.ensureConfirmation soft success (separate issue).
  • ProfitClient zero inputAmountUsd division (separate issue).
  • Historical iosiro speedUp double-spend (not reproduced on this commit; different root cause).

Thanks!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions