fix(gasless): reject unresolvable token pairs instead of throwing

[droplet-rl]

Problem

isAllowedGaslessPair resolves both tokens to an L1 equivalent via getInventoryEquivalentL1TokenAddress, which asserts when a token has no hub-chain address. Tokens listed only on a remote chain — a chain's wrapped native asset, for example — have no such address, so the assert fires.

The call is unguarded, so the throw escapes processDepositMessage. forEachAsync is Promise.all, so that rejects the whole poll tick (evaluateApiSignatures failed; batch skipped this tick). Two follow-on effects:

• The message never reaches a terminal state, so it recurs on every subsequent tick.
• The fill lock taken immediately before the throw is never released, so later deposits from the same authorizer on that chain spin in the lock-wait branch.

A deposit naming a token the pinned TOKEN_SYMBOLS_MAP doesn't know is ordinary user input, not an error condition — it should be rejected, not thrown.

Change

• Add tryGetInventoryEquivalentL1TokenAddress, a non-throwing variant of the existing resolver.
• isAllowedGaslessPair uses it and treats an unresolvable token as a rejected pair. Valid pairs behave exactly as before.
• Wrap the per-message state machine in try/catch/finally: an unexpected throw now marks only that message ERROR, and finally always releases the fill lock. The catch still logs at error, so genuinely unexpected failures keep alerting — they just no longer take the batch or the lock down with them.

Most of the diff in GaslessRelayer.ts is re-indentation from the try. Review with git diff -w.

Tests

Five added; three fail without this change:

• isAllowedGaslessPair: rejects rather than throws for a token with no hub-chain equivalent, and for one absent from the map; still accepts a genuine same-L1 pair.
• Relayer: a deposit naming an unknown token ends ERROR rather than throwing.
• Relayer: a throwing message is marked ERROR and releases its lock, while a second message sharing its authorizer still completes to FILLED.

Refs BOU-54.

[linear]

BOU-54

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-31T06:44:42.224987Z	0994544	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[droplet-rl]

Codex finished while I was looking at this: completed with no findings on 0994544 (it left the 👍 reaction, no review body and no inline comments). So there was nothing from it to address.

I used the pass to self-review the block that comment was triggered on instead, and found two things wrong with my own catch/finally in GaslessRelayer.ts. Pushed as e320f13e:

1. The finally released a lock it may not hold. It did an unconditional delete this.fillLock[fillKey]. A message that throws while spinning in the lock-wait branch doesn't own that lock — it belongs to another message — so releasing it there would let a second fill for the same authorizer start while the holder's fill is still in flight. That's exactly the overlap the lock exists to prevent, and it's a hazard this PR introduced by adding the finally. Now guarded on ownership:

if (this.fillLock[fillKey] === depositKey) {
  delete this.fillLock[fillKey];
}

Every normal exit path holds the lock at exit, so intended releases are unchanged — including the delete at the _findDeposit-miss site, which re-acquires on the next iteration.

2. The error log recorded the wrong state. The catch called setState(MessageState.ERROR) before logging, so the alert always said state: ERROR instead of the state the message actually died in. I widened the local log helper to accept "error" and log before the transition. That fixes the state field and picks up the authorizer/nonce/token/amount context the direct logger.error call was dropping — authorizer being the field that tells you which fill lock was involved.

Verification: test/GaslessRelayer.ts + test/GaslessUtils.ts 85 and 32 passing, 0 failing, including the 5 added here. tsc --noEmit clean, prettier clean.

One caveat worth flagging rather than hiding: I did not add a test for the ownership guard. Exercising it needs a throw from inside the lock-wait branch, which is only log + delay + continue — reachable only by mocking the logger to throw, which felt more contrived than the guard is worth. It's a strict narrowing of a delete, so it can't strand a lock: if we don't own it, the owner still releases it.

No doc change — per AGENTS.md, bug fixes normally don't clear that bar.

Since this was the one automated round: I think the change is complete as it stands, but the lock-ownership reasoning above is the part I'd most want a human to sanity-check. If you'd rather iterate further, assigning me to the PR raises the round budget.

[droplet-rl]

🔎 View trace

[pxrl]

Not keen on scoping this try statement so broadly. Narrow it to where it's actually needed.

[droplet-rl]

Agreed — narrowed it to a wrapper around the single processDepositMessage call at the forEachAsync site (9241b3ff).

That is where the concern actually lives: forEachAsync is Promise.all, so the hazard is one message's rejection taking the whole batch — a property of the batch boundary, not of any individual state transition. Wrapping the state machine to express it was the wrong altitude.

const processDepositMessageSafely = async (depositMessage: AnyGaslessDepositMessage) => {
  const depositKey = this._getDepositKeyFromMessage(depositMessage);
  const fillKey = this._getFillLockKeyFromMessage(depositMessage);
  try {
    await processDepositMessage(depositMessage);
  } catch (error) { /* mark only this message ERROR */ }
  finally { if (this.fillLock[fillKey] === depositKey) { delete this.fillLock[fillKey]; } }
};

The state machine is now byte-identical to master — no try, no re-indentation, and the success-path delete this.fillLock[fillKey] stays exactly where it was. The finally is purely the error-path net (a no-op on the success path, since the lock is already released), still guarded on ownership so a throw from the lock-wait branch can't release a lock another message holds.

Diff on this file drops from 369 lines to 42, all additive. Also extracted _getFillLockKeyFromMessage so the wrapper and the state machine derive the fill key one way rather than two.

Worth noting the two concerns are now cleanly separated, which the tests show: the actual bug is fixed at source by the non-throwing resolver, and with the wrapper temporarily bypassed the token-map test still passes while only the isolation test fails. So the wrapper is a generic net, not load-bearing for this bug.

[droplet-rl]

Thanks — good call, that try was at the wrong altitude. Addressed in 9241b3ff, replied on the thread with detail.

What changed: the isolation moved to a wrapper around the single processDepositMessage call at the forEachAsync site. That's where the concern actually lives — forEachAsync is Promise.all, so the hazard is one message's rejection taking the whole batch, which is a property of the batch boundary rather than of any state transition inside the machine.

Consequences:

• The deposit state machine is byte-identical to master again — no try, no re-indentation, and the success-path delete this.fillLock[fillKey] stays exactly where it was.
• The wrapper's finally is purely the error-path net (no-op on the success path), still guarded on lock ownership.
• Diff on GaslessRelayer.ts drops from 369 lines to 42, all additive. The "review with git diff -w" caveat in the PR description no longer applies.
• Extracted _getFillLockKeyFromMessage so the wrapper and the state machine derive the fill key one way instead of two.

Verified: 85 passing / 0 failing across test/GaslessRelayer.ts + test/GaslessUtils.ts, tsc --noEmit clean, prettier + eslint clean.

I also checked the wrapper is load-bearing rather than decorative: with it temporarily bypassed, the isolation test fails (52 passing, 1 failing) while the token-map test still passes. That's the separation I'd want — the actual bug is fixed at source by the non-throwing resolver, and the wrapper is only a generic net for unexpected throws.

One thing worth flagging: my first commit on this pass accidentally staged two unrelated pre-existing working-tree deletions (.env.example, src/monitor/.env.sample) via git add -A. I caught it before finishing, restored both from master and amended — the branch now touches only the 5 intended files. Nothing to do on your side, but noting it rather than leaving it silent in the history.

[droplet-rl]

🔎 View trace