Skip to content

fillStatus defaults to FillStatus.Filled for missing array entries — partial RPC response permanently skips valid deposits #3432

Description

@Nexory

Summary

In src/relayer/Relayer.ts, fillStatusArray() is called to check whether each deposit on a destination chain still needs to be filled. When the returned array is shorter than the input deposits array (e.g. due to a partial or truncated RPC response), the nullish-coalescing default ?? FillStatus.Filled causes out-of-bounds entries to be treated as already filled. Because the resulting fill status is written unconditionally into the persistent this.fillStatus cache on the very next line, those deposits are permanently blacklisted from the relayer's fill loop — with no log warning and no recovery path short of a process restart.

What I observed

src/relayer/Relayer.ts, lines 949–964:

const fillStatus = await spokePoolClients[destinationChainId].fillStatusArray(deposits);
// ...
const unfilledDeposits = deposits
  .map((deposit, idx) => ({ ...deposit, fillStatus: fillStatus[idx] ?? FillStatus.Filled }))
  //                                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^
  //  If fillStatusArray returns N < deposits.length elements,
  //  every deposit at index >= N gets FillStatus.Filled here.
  .filter(({ fillStatus, ...deposit }) => {
    const depositHash = spokePoolClients[deposit.destinationChainId].getDepositHash(deposit);
    this.fillStatus[depositHash] = fillStatus;   // <-- cached unconditionally
    return fillStatus !== FillStatus.Filled;
  })

There is no length-mismatch guard before the .map(), and the cache write inside .filter() happens regardless of whether the fill status came from a real RPC result or the fallback default.

Impact

A short or truncated response from fillStatusArray (network hiccup, provider-side truncation, gas limit, ABI decode edge-case) causes every deposit beyond the truncation point to be silently recorded as FillStatus.Filled in this.fillStatus. On the next loop those deposits are filtered out by _getUnfilledDeposits() before they ever reach fillStatusArray again. The relayer will not fill them, not warn about them, and not recover until the process restarts and the in-memory cache is cleared. For the depositor this looks like an indefinitely hung fill.

Suggested fix

The safest minimal fix is to skip the cache write (and treat the deposit as potentially unfilled) whenever fillStatusArray did not return a value for that index:

-  .map((deposit, idx) => ({ ...deposit, fillStatus: fillStatus[idx] ?? FillStatus.Filled }))
+  .map((deposit, idx) => ({ ...deposit, fillStatus: fillStatus[idx] ?? FillStatus.Unfilled }))

A stricter alternative is to add an explicit length guard before the map and log a warning + skip the batch if fillStatus.length < deposits.length, so operators are alerted to RPC degradation:

if (fillStatus.length < deposits.length) {
  this.logger.warn({ at, message: `fillStatusArray returned ${fillStatus.length}/${deposits.length} entries — skipping batch to avoid false-filled cache` });
  return;
}

Either approach prevents the irreversible cache poisoning. The ?? FillStatus.Unfilled change is the lowest-risk one-liner; the length guard gives better observability.

Notes

This was identified during a static audit of the relayer fill loop. No related open issues or PRs were found (fillStatusArray, FillStatus.Filled default, deposits skipped searches all returned empty). The fix does not require changes outside this function.

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