fix(finalizer): tolerate unrecognised tokens in Arbitrum withdrawal discovery

[droplet-rl]

Problem

arbStackFinalizer discovers withdrawals by querying the canonical Arbitrum gateways, then resolves each event's L1 token to an L2 address before it can build any finalization:

const l2Token = getL2TokenAddresses(e.args.l1Token)?.[chainId];
assert(isDefined(l2Token), `Missing L2 token mapping ...`);

getL2TokenAddresses returns undefined for any token absent from TOKEN_SYMBOLS_MAP, so the assert throws inside the .map() — before multicallArbitrumFinalizations is reached. The finalizer returns nothing, and finalize() catches it per-chain, so every Arbitrum → Ethereum withdrawal in that run is skipped rather than just the one unrecognised token. Because the input is recomputed each loop, the condition persists until the event ages out of the lookback window.

The try/catch a few lines below, whose catch logs "Skipping ERC20 withdrawal event for unknown token", was intended to cover this — but it wraps only object construction and an Array.push, neither of which can throw, so that path was unreachable.

Why finalize rather than skip

The token is not an input to the finalization. finalizeArbitrum builds the Outbox executeTransaction calldata entirely from the Arbitrum SDK message (getOutboxProof() plus nitroWriter.event); getUniqueLogIndex keys on txnRef. The only real consumer is getTokenInfo in multicallArbitrumFinalizations, which supplies symbol/decimals for the log line.

So an unrecognised token is a labelling gap, not a reason to withhold a withdrawal — and skipping would drop a legitimate withdrawal whenever a real token is missing from constants (removed or renamed while a withdrawal sits mid-challenge-period). These withdrawals are already destined for an address we finalize for; the gateway queries filter on to ∈ {HubPool, SpokePool, AtomicDepositor, userAddresses}.

Changes

• Discovery no longer treats token resolution as a precondition. An unmapped token falls back to the L1 address so the withdrawal stays identifiable in logs.
• One aggregated logger.warn per run listing the unrecognised withdrawals (l1Token, to, amount, txnRef), so these stay visible.
• getTokenInfo at the log-line site is wrapped in describeToken(), degrading to UNKNOWN/18. This was a second throw site — fixing only the assert would have relocated the failure, not removed it.
• Removed the dead try/catch.

zkSync.ts already carries the equivalent guard ("a single unknown token would otherwise abort finalization for every withdrawal on this chain"); arbStack was the gap. Polygon and Linea source their events from the SpokePool and sender-filtered queries respectively, so neither is affected.

Testing

yarn build, eslint and prettier pass. No automated regression test — exercising the discovery path needs Arbitrum SDK receipts, Redis and block-time mocking. Happy to add one by extracting the event → TokensBridged mapping into an exported pure helper (the way buildFinalizationBatches is exported for its test) if reviewers want it.

Note for reviewers

The warn is deliberately not routed to notificationPath: "across-error". Reasoning in the code comment; easy to flip if you disagree.

Credit

Reported by Jagdish, who also offered to help test the fix. Thanks!

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69060d51d2

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[droplet-rl]

Replied inline. One comment, no code change: _amount is a BigNumber, but the .toString() is for log legibility rather than type conversion — ethers v5 BigNumber.toJSON() renders as {"type":"BigNumber","hex":"0x…"} and only BigInt.prototype.toJSON is patched in src/utils/extensions.ts. Matches zkSync.ts:253. Say the word if you'd rather have the object in the log and I'll drop it.

[droplet-rl]

🔎 View trace

[droplet-rl]

Verified — you were right, my earlier answer was wrong. BigNumbers do log fine on their own; @risk-labs/logger stringifies them in iterativelyReplaceBigNumbers.

The different codepath you suspected is real though: that helper rebuilds via Object.fromEntries(...), and only when something changed. So an array containing a BigNumber round-trips into a plain object ({"0": …}), while a BigNumber-free array is returned untouched. Confirmed against the real logger — amount renders identically both ways, but unknownTokenWithdrawals degrades from array to object without the .toString().

Kept it, added a comment recording why (0f6d9a9). Happy to drop it if you'd rather have the object form.

[droplet-rl]

🔎 View trace

[droplet-rl]

Not required — removed in 1ff6c2e. withdrawalEvents goes straight into multicallArbitrumFinalizations, whose parameter type already pins the shape; confirmed the call site still fails on a renamed field without it. Only nuance is the local now infers amountToReturn as any (ethers Result), which doesn't escape the function. Prettier also collapses the map to one wrapper now. tsc/eslint/prettier clean.

Note I fast-forwarded onto your master merge + c5b3ac8 first, so this sits on top of the current branch head.

[droplet-rl]

🔎 View trace

[droplet-rl]

Applied in 248d608. One adjustment: as written the destructure sits inside the if and shadows the outer l1Token (EvmAddress) with the raw string — two types either side of the brace, and no no-shadow rule to catch it. Hoisted it to the top of the callback instead, which kills the shadow and retires the separate EvmAddress binding: l2TokenAddress: EvmAddress.from(l2Token ?? l1Token). isDefined is !== null && !== undefined, so that's exactly the old ternary.

Also re-verified the .toString() note against logger 1.3.14 from your master merge — still Object.fromEntries(...), array still collapses without it, so the comment holds. tsc/eslint/prettier clean.

[droplet-rl]

🔎 View trace

[pxrl]

@droplet babysit. Resolve any open comments.

[droplet-rl]

All 4 open threads addressed and resolved. One turned out to be a real bug.

Codex P2 was right, both halves. getL2TokenAddresses(l1Token) defaults l1ChainId to mainnet and returns the first TOKEN_SYMBOLS_MAP entry matching the L1 address without checking it covers the target chain. Verified:

USDC sepolia -> ArbSepolia       old=undefined  new=0x75faf114...
WETH mainnet -> Robinhood(4663)  old=undefined  new=0x0Bd7D308...
USDC/WETH mainnet -> Arbitrum    unchanged
unknown token                    undefined both

Sepolia is live (arbStackFinalizer is registered for ARBITRUM_SEPOLIA). The alias case is worse: ETH/WETH share 0xC02aaA39... and ETH sorts first but has no 4663 entry, so every Robinhood WETH withdrawal resolved to undefined — my new warn would have fired on all of them. Switched to getRemoteTokenForL1Token(..., hubPoolClient.chainId), which requires an entry on the remote chain. aca68d4.

Comment shrunk to one line, as asked.

L2 address via contract: queryable (calculateL2TokenAddress, wrapped by @arbitrum/sdk), but not statically derivable for custom gateways, needs an ABI entry we don't have, and gives no symbol/decimals — so it wouldn't remove the map lookup, just add RPCs to a log line. Resolved with no code change; reopen if you want it anyway.

Annotation thread was already handled in 1ff6c2e.

tsc/eslint/prettier clean, finalizer tests pass, CI green on the prior head. Note the last commit isn't pushed until checkpoint, so CI hasn't seen the token fix yet.

[droplet-rl]

🔎 View trace