Skip to content

improve(InventoryClient): Broaden repayment eligibility for hub-origin deposits#3528

Open
droplet-rl wants to merge 2 commits into
masterfrom
droplet/broaden-hub-origin-repayment
Open

improve(InventoryClient): Broaden repayment eligibility for hub-origin deposits#3528
droplet-rl wants to merge 2 commits into
masterfrom
droplet/broaden-hub-origin-repayment

Conversation

@droplet-rl

Copy link
Copy Markdown
Contributor

Motivation

For a deposit originating on the hub chain, the input liquidity lands on the hub — the rebalancing center. The relayer can therefore be repaid on any chain with a pool rebalance route for the token and have the hub source that rebalance cheaply. Today, hub-origin deposits are only evaluated for repayment on the origin (hub) and destination chains.

This surfaced on a USDT (Mainnet) → USDC (Base) deposit that went unfilled: the destination (Base) was over-allocated and the hub hit origin-commitment limits, so there was no acceptable repayment chain — even though the relayer was under-allocated on other chains it could have rebalanced into.

Change

  • InventoryClient.getRepaymentChainsForL1Token() — new helper enumerating every enabled L2 chain that has both inventory token config and a pool rebalance route for the L1 token.
  • determineRefundChainId() — for hub-origin deposits, append those chains to the candidate set. The existing per-chain allocation check (expectedPostRelayAllocation <= effectiveTargetPct) keeps only under-allocated chains, so broadening can only add safe targets. Destination stays on the standard path; the hub remains the fallback.
  • getPossibleRepaymentChainIds() — mirror the broadened set so batchComputeLpFees precomputes an LP fee per candidate. LP-fee differences (i.e. the cost of rebalancing to a third chain) are therefore priced into downstream profitability selection — an unprofitable third-chain repayment is filtered out.
  • Token-agnostic: keyed on the deposit's L1 token, not any specific token.

Notes

  • Cheap by construction. This reuses the in-memory allocation check only; it does not reintroduce the running-balance computation removed in fix(relayer): Drop slow withdrawal repayment considerations #3520.
  • Guard simplification. The gate is just originChainId === hubChainId. forceOriginRepayment cannot be set at that point — a hub origin is always an unmetered fast-rebalance source, so isUnmeteredFastRebalance() is true and the forced-origin path short-circuits earlier in determineRefundChainId().
  • Fixes a stale batchComputeLpFees comment that still referenced the removed slow-withdrawal chains.
  • Interaction with the fast-rebalance short-circuit: broadening engages for hub-origin deposits not already short-circuited to origin by the canFastRebalanceFill rule. Since the hub is typically absent from tokenConfig, that short-circuit doesn't fire and broadening runs (the real-world case here). If we later want broadening to also override an explicit hub token config, that's a follow-up.

Testing

yarn build + lint clean. New cases in test/InventoryClient.RefundChain.ts (all 46 in the file pass, no regressions):

  • hub-origin deposit surfaces every pool-rebalance-route chain in the possible list (and excludes a config-but-no-route chain);
  • non-hub origin is unaffected (no broadening);
  • a hub-origin fill refunds on under-allocated route chains that are neither origin nor destination.

Worth a sanity check on utilisation after enabling, in the same spirit as #3520.

🤖 Generated with Claude Code

…n deposits

For a deposit originating on the hub chain, the input liquidity lands on the
hub, so the relayer can be repaid on any chain with a pool rebalance route for
the token and have the hub source the rebalance cheaply. Enumerate those chains
and add them to the repayment-eligibility evaluation; the existing per-chain
allocation check keeps only the under-allocated ones, and LP fee differences are
priced into downstream profitability selection.

Previously, hub-origin deposits could only be repaid on the origin (hub) or
destination chain. When the destination was over-allocated and the hub hit
origin-commitment limits, such deposits were left unfilled even though the
relayer was under-allocated elsewhere. This reuses the cheap in-memory
allocation check rather than the running-balance computation removed in #3520.

The guard is simply `originChainId === hubChainId`: forceOriginRepayment cannot
be set at that point because a hub origin is always an unmetered fast-rebalance
source, so the forced-origin path short-circuits earlier. Also fixes a stale
batchComputeLpFees comment referencing the removed slow-withdrawal chains.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9eebb5ce3

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

Comment on lines +766 to +769
const broadenedCandidateChains =
originChainId === hubChainId
? this.getRepaymentChainsForL1Token(l1Token).filter((chainId) => chainId !== destinationChainId)
: [];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include destination in hub-origin swap broadening

For supported swap deposits, the standard destination path does not handle the destination chain because canTakeDestinationChainRepayment() only returns true when the input and output tokens are equivalent. This unconditional chainId !== destinationChainId filter therefore drops the destination from the broadened hub-origin candidates, so a Mainnet USDT -> Base USDC-style deposit will not evaluate Base as a USDT repayment route even when Base has a PoolRebalanceRoute and is the only under-allocated/profitable chain, causing the fill to fall back to other chains or be skipped.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

canTakeDestinationChainRepayment() does not gate on input/output equivalence — it returns true whenever the input token's L1 mapping has a pool-rebalance route on the destination chain (l2TokenEnabledForL1TokenAtBlock(l1Token, destinationChainId)), regardless of the output token. So for Mainnet USDT -> Base USDC where Base has a USDT PoolRebalanceRoute, canTakeDestinationChainRepayment is true and the standard canEvaluateDestination path (canTakeDestinationChainRepayment && _l1TokenEnabledForChain(l1Token, dest)) already evaluates Base.

That standard predicate (config AND route on the destination) is exactly the predicate getRepaymentChainsForL1Token() uses for membership, so the destination is never a member of the broadened set that the standard path skips. The chainId !== destinationChainId filter only avoids double-counting the destination in the candidate list; it cannot silently drop an otherwise-eligible route. (Conversely, if Base lacked a USDT route or config, it would be excluded from both the standard and broadened sets — correctly, since the relayer can't manage USDT inventory there.) Leaving this as-is.

Comment on lines +545 to +549
// For a hub-origin deposit the input liquidity lands on the hub, so the relayer can be repaid on any chain with
// a pool rebalance route for this token and have the hub source the rebalance cheaply. Surface them all here so
// their LP fees are precomputed; determineRefundChainId() applies the allocation check. forceOriginRepayment is
// necessarily false at this point (it returns above).
if (originChainId === this.hubPoolClient.chainId) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update repayment docs for broadened hub-origin chains

AGENTS.md requires relevant AGENTS.md/README.md files to be updated in the same change whenever runtime behavior changes. This new hub-origin branch changes the repayment eligibility/runtime flow, but the repository docs remain stale (for example, docs/repayment-eligibility.md still describes possible chains as only origin/destination/override/hub and candidate evaluation as origin/destination), so operators debugging these fills will be following incorrect rules; please update the relevant README/docs alongside this behavior change.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 5f81d4a. Updated docs/repayment-eligibility.md to document the broadened hub-origin candidate set: the new getRepaymentChainsForL1Token() bullet under getPossibleRepaymentChainIds(), the candidate-build step in the determineRefundChainId() walkthrough, and a dedicated "hub-origin broadening" entry under Subtle behaviors covering the rationale, the allocation-check guard, and the LP-fee mirroring.

Comment on lines +578 to +579
const repaymentToken = this.getRemoteTokenForL1Token(l1Token, chainId);
return isDefined(repaymentToken) && this.hubPoolClient.l2TokenHasPoolRebalanceRoute(repaymentToken, chainId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle alias repayment tokens when broadening

When the inventory config for an L1 token is an alias config, the pool-rebalance route may be on a configured alias token (for example USDC.e/USDbC) rather than the single canonical token returned here. Because this helper probes only getRemoteTokenForL1Token(), hub-origin broadening silently omits otherwise eligible under-allocated chains for those alias-token routes and their LP fees are never precomputed; this recreates the no-eligible-chain behavior for common USDC-style configurations even though getRemoteTokensForL1Token() can enumerate the configured repayment tokens.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The helper deliberately mirrors the canonical-token resolution that the downstream allocation loop in determineRefundChainId() already uses: for each ranked non-origin chain the loop resolves repaymentToken = getRemoteTokenForL1Token(l1Token, chainId) (the same singular/canonical lookup) and then asserts l2TokenHasPoolRebalanceRoute(repaymentToken, chainId).

If getRepaymentChainsForL1Token() instead enumerated alias tokens via getRemoteTokensForL1Token() and admitted a chain whose route lives only on a non-canonical alias (e.g. USDC.e/USDbC), that chain would enter the loop, the loop would still resolve the canonical token (which has no route), and the assert would throw — turning a silent omission into a hard crash. So the suggested change is unsafe unless the loop's repayment-token resolution is reworked to pick the alias that actually carries the route.

That canonical-only resolution is pre-existing across the entire origin/destination allocation path, not introduced by this PR, so broadening alias-token handling is a separate change with its own test surface. I'm keeping the helper aligned with the loop here, and have noted the alias limitation for a follow-up.

Update the InventoryClient repayment-eligibility reference to reflect the
broadened candidate set for hub-origin deposits, per the documentation
maintenance requirement in AGENTS.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@droplet-rl

Copy link
Copy Markdown
Contributor Author

Thanks @codex — reviewed all three suggestions. Summary:

1. Update repayment docs (P2) — fixed in 5f81d4a. docs/repayment-eligibility.md now documents the broadened hub-origin candidate set (the getRepaymentChainsForL1Token() helper, the candidate-build step, and a "hub-origin broadening" entry under Subtle behaviors).

2. Include destination in hub-origin swap broadening (P2) — declined (false positive). canTakeDestinationChainRepayment() doesn't gate on input/output equivalence; it returns true whenever the input token's L1 mapping has a pool-rebalance route on the destination. That's the same (config + route) predicate getRepaymentChainsForL1Token() uses, so a Mainnet USDT → Base USDC deposit with a USDT route on Base is already evaluated via the standard path. The chainId !== destinationChainId filter only prevents double-counting — it never drops an eligible route.

3. Handle alias repayment tokens (P2) — declined for this PR. The helper intentionally mirrors the canonical-token resolution the downstream allocation loop already uses (getRemoteTokenForL1Token + the l2TokenHasPoolRebalanceRoute assert). Switching to the plural alias enumeration would admit chains whose route is on a non-canonical alias, which the loop would then assert-fail on — a hard crash, not a fix. The canonical-only resolution is pre-existing across the whole origin/destination path; broadening alias handling is a separate change with its own test surface.

Per-thread replies left inline. The only diff change is the docs commit (no logic change), so the broadening behavior and existing tests are unaffected.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

  • Updated docs/repayment-eligibility.md to document hub-origin repayment broadening in the possible repayment chain set, including the getRepaymentChainsForL1Token() helper and LP-fee precomputation alignment. docs/repayment-eligibility.mdL66-L80
  • Clarified the determineRefundChainId() candidate-build step so the docs now describe standard origin/destination ordering, hub-origin PoolRebalanceRoute candidate appending, and de-duplication before allocation checks. docs/repayment-eligibility.mdL86-L104
  • Added a subtle-behavior note explaining why hub-origin broadening can evaluate chains beyond origin/destination while still relying on allocation checks and mirrored LP-fee coverage. docs/repayment-eligibility.mdL108-L118
  • Committed the documentation update as 3038288 docs: clarify hub-origin repayment broadening and created a PR via the make_pr tool.
  • Left the pre-existing unstaged yarn.lock modification untouched.

Testing

  • git diff -- docs/repayment-eligibility.md
  • git add docs/repayment-eligibility.md && git commit -m "docs: clarify hub-origin repayment broadening"
  • git status --short && nl -ba docs/repayment-eligibility.md | sed -n '66,118p'
  • ⚠️ Not run: project test suite, because this was a documentation-only change.

View task →

if (originChainId === this.hubPoolClient.chainId) {
const l1Token = this.getL1TokenAddress(inputToken, originChainId);
if (isDefined(l1Token)) {
this.getRepaymentChainsForL1Token(l1Token).forEach((chainId) => chainIds.add(chainId));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@droplet-rl Do we subsequently dedup here? It'd be nice to avoid evaluating the same chainIds repeatedly.

Comment thread src/clients/InventoryClient.ts
@droplet-rl

Copy link
Copy Markdown
Contributor Author

This docs update is already part of this PR — no separate change is needed.

I addressed the original "update repayment docs" review comment in the prior round with commit 5f81d4a (docs(repayment-eligibility): document hub-origin repayment broadening), which is the current head of #3528. It covers the same three edits described here: the getRepaymentChainsForL1Token() bullet under getPossibleRepaymentChainIds(), the hub-origin candidate-append step in the determineRefundChainId() walkthrough, and a "hub-origin broadening" entry under Subtle behaviors (rationale + allocation-check guard + mirrored LP-fee coverage).

For the record, the separate commit referenced above (3038288) and the make_pr-created PR aren't present on across-protocol/relayer — that work stayed in the sandbox and didn't push here. So there's nothing additional to merge, and if a duplicate PR was opened elsewhere it's redundant against 5f81d4a and can be closed. The docs in #3528 are already current; no code or doc changes in this round.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

// their LP fees are precomputed; determineRefundChainId() applies the allocation check. forceOriginRepayment is
// necessarily false at this point (it returns above).
if (originChainId === this.hubPoolClient.chainId) {
const l1Token = this.getL1TokenAddress(inputToken, originChainId);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the originChainId is known to be the hubPool chain, we already know that inputToken is equal to l1Token. Therefore, l1Token lookup is redundant. Agree?

@droplet-rl

Copy link
Copy Markdown
Contributor Author

Thanks for the review @pxrl. I replied inline on the getRepaymentChainsForL1Token exception question.

Short version: none of the four methods throw on real inputs. getEnabledL2Chains() and _l1TokenEnabledForChain() are pure in-memory checks. The two SDK-backed calls return undefined/false for unknown mappings rather than throwing — getRemoteTokenForL1Token() uses the non-asserting resolveAcrossToken overload (and the isDefined() guard handles undefined), and l2TokenHasPoolRebalanceRoute() just returns isDefined(getL1TokenForL2TokenAtBlock(...)). Both are already called unguarded on this same path (determineRefundChainId L778/L782, canTakeDestinationChainRepayment L654), so the helper adds no new exception surface.

No code change for now; I'd lean against a try/catch here since it'd be inconsistent with the surrounding unguarded calls and could swallow a real misconfig — but happy to add one if you'd prefer.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants