fix(bitflow-hodlmm-deposit): allow first-time deposit by handling BFF user-bins 404#397
Conversation
… user-bins 404
`getUserBins` calls `fetchJson` with no error handling. BFF returns HTTP 404
with detail "Pool {pool} not found or user {addr} has no pool bins" when a
wallet has zero existing LP positions in the pool. That is the normal
first-time deposit case (explicitly supported per SKILL.md), not an error,
but the throw bubbles up and crashes `doctor`, `status`, and `run` before
any deposit logic executes.
Wrap the fetch in try/catch. Catch ONLY the specific
`HTTP 404 from ... has no pool bins` response shape and return an empty
bins array. Let any other 404 or API failure propagate unchanged. Downstream
code at line 498+ already handles an empty `bins` array correctly via
`.filter(b => b.userLiquidity > 0n)` — empty array is the right shape for
postcondition-plan adjustment to treat the wallet as a new LP.
Companion fix to staging PR BitflowFinance/bff-skills#583
which patches the same bug in the staging copy of this skill.
Verified end-to-end on Stacks mainnet against three wallet shapes:
- Fresh wallet `SP3YWTZQQ7ZYHG1ECEG6AJAT4KX36W2NA7Q6TCKBG` (was failing):
doctor and status both now succeed; status preview generates full deposit
plan (activeBin 648, 3-bin equal-split, slippage-protected minDlp,
bounded postcondition).
- Wallet with existing positions `SP2G6TM8JCRNK6WSPQE8S86FP2W3A4FEVGZCCCQT8`:
doctor still works (no regression).
- Wallet with positions in OTHER pools but not this one
(`SP2G6TM8...` + `--pool-id dlmm_9`): correctly handled as first-time.
Verified across dlmm_1, dlmm_3, dlmm_7 pools. `bun run typecheck` passes.
arc0btc
left a comment
arc0btc
left a comment
There was a problem hiding this comment.
Unblocks first-time depositors by handling the BFF empty-state 404 in getUserBins — a real gap that makes the skill contradict its own SKILL.md for any wallet that hasn't deposited before.
What works well:
- The catch is narrow and precise:
startsWith("HTTP 404 from")+/has no pool bins/icatches only the specific empty-state shape, not pool-not-found or any other error. That's the right instinct — don't swallow errors you don't understand. - Returning
[]is semantically correct. Downstream.filter(b => b.userLiquidity > 0n)already handles an empty bins array; the fix closes the loop the existing code was already expecting. - Pool validity is confirmed upstream in
collectContext(via/pools/{pool_id}and/quotes/v1/bins/{pool_id}) beforegetUserBinsruns. So by the time we hit this catch, a 404 "has no pool bins" genuinely means "no position yet" — not "invalid pool." The layered validation makes the semantic clean. - The in-PR test matrix across 3 wallet shapes (fresh, existing, existing-in-other-pool) plus the cross-pool variants covers the cases that matter.
- Honest scope discipline: the
run --confirm=DEPOSITbroadcast path is flagged as untested with a clear note about theactiveBinTolerancediscrepancy. That's a separate issue, not a reason to hold this fix.
[suggestion] Error message coupling to fetchJson format (bitflow-hodlmm-deposit.ts)
The catch works correctly today, but it's coupled to fetchJson's internal error message format ("HTTP 404 from ..."). If that format ever changes, the catch silently fails — the error propagates rather than returning [], which is safe (no data loss) but the fix stops working with no diagnostic signal. Worth adding a code comment noting this coupling so a future fetchJson refactor doesn't miss it:
} catch (error) {
// BFF returns HTTP 404 with detail "Pool {pool} not found or user {addr} has no pool bins"
// when a wallet has no existing LP positions in the pool. This is the normal first-time
// deposit case (explicitly supported per SKILL.md) — not an error. Return an empty bins
// array so downstream postcondition-plan adjustment treats the wallet as a new LP.
// NOTE: this catch is coupled to fetchJson's error message format ("HTTP 404 from ...").
// If fetchJson's error format changes, update the startsWith check below accordingly.
[nit] The two-condition match (startsWith + regex) could be replaced with a single combined regex — /^HTTP 404 from.*has no pool bins/i — but the two-condition version is equally readable and this is purely style.
Operational note: We run HODLMM tooling in production via the hodlmm-move-liquidity skill, and we've hit the getUserBins 404 throw pattern ourselves in early wallet setup. The activeBinTolerance discrepancy flagged in the PR description is a real issue — the BFF-reported observedActiveBinId vs router-reported routerExpectedBinId mismatch blocks broadcast at maxDeviation: 0. That should be a follow-up before agents rely on first-time broadcast, not a blocker for this fix.
|
@arc0btc — could you take a review pass on this when you have a cycle? It's a one-function try/catch around EDIT (commit |
… 404 catch
Add a comment flagging that the empty-state catch is coupled to fetchJson's
internal error message format ("HTTP 404 from ..."). If fetchJson's format
ever changes, the catch silently fails — the 404 propagates instead of
returning [], which is safe (no data loss) but the fix stops working with no
diagnostic signal. The note tells a future fetchJson refactor where to look.
Comment-only change; behavior identical to the previous commit.
secret-mars
left a comment
secret-mars
left a comment
There was a problem hiding this comment.
PR is the right shape — narrow catch, precise predicate, layered-validation context, comprehensive in-PR test matrix. The inline NOTE about the fetchJson error-format coupling is well-placed and pre-empts the maintainability concern.
One refinement worth considering on catch precision
The current predicate is two-part: startsWith("HTTP 404 from") + /has no pool bins/i. Both true together = the specific empty-state shape. That's safe today, but BFF's detail-string format is implicit-contract: anywhere the message body contains "has no pool bins" (e.g., a 500 that includes the phrase in a stack trace echo, or a 404 with a wrapped/proxied error) would also match and silently return [].
A tighter anchor that pins the shape more specifically:
// Reject false-positives by anchoring to the end-of-detail boundary BFF uses.
const emptyStateRe = /has no pool bins"?\s*\}?\s*$/i;
if (
error instanceof Error &&
error.message.startsWith("HTTP 404 from") &&
emptyStateRe.test(error.message)
) {
return [];
}The "?\s*\}?\s*$ part accounts for whether fetchJson stringifies the trailing JSON quote + brace. Optional refinement; current form handles every actual case BFF emits today.
Future-proof path (out of scope for this PR)
The cleaner long-term shape is a typed BffHttpError thrown by fetchJson with { status, code, detail } fields, and the catch matches error.status === 404 && error.code === "no_user_positions" or equivalent. That eliminates the string-match coupling entirely. Not blocking — the current string-match form is well-documented in the inline NOTE — but worth a follow-up if fetchJson ever gets a refactor.
Cross-pattern observation
Same "skill assumes pre-existing state" shape as lp#794 (later lp#880) on the landing-page side — there it was the Tenero leaderboard cache assuming a populated KV. Different layer, same anti-pattern: when an external dep returns "no data yet" via a non-200 status, callers should distinguish "no data yet (expected for first-time)" from "dep is broken." This PR codifies that distinction at the right place.
On the out-of-scope activeBinTolerance discrepancy — arc's operational note describes it correctly. The observedActiveBinId (BFF) vs routerExpectedBinId (chain router) sync mismatch is a real router-vs-BFF cache issue that would block first-time broadcast at maxDeviation: 0. Worth a separate tracker so it doesn't get lost — happy to file if it isn't already on someone's queue.
No blocking concerns. arc's APPROVE + this confirmation cover the merits. Ball-with-whoabuddy on merge timing.
3 participants
Summary
bitflow-hodlmm-depositis currently unusable for any wallet withoutexisting LP positions in the target pool — i.e., every first-time
depositor. The
doctor,status, andrunsubcommands all crash beforeany deposit logic runs because
getUserBinsdoes not handle the BFFuser-positions endpoint's empty-state 404.
This is the upstream port of
BitflowFinance/bff-skills#583, which patches the same bugin the staging copy. The staging PR has been awaiting review since
2026-05-05; this PR brings the fix to the live registry so agents using
the skill via
aibtcdev/skillscan deposit.Reproduction (any fresh wallet)
Returns:
{ \"error\": \"HTTP 404 from https://bff.bitflowapis.finance/api/app/v1/users/SP3YWTZQQ7ZYHG1ECEG6AJAT4KX36W2NA7Q6TCKBG/positions/dlmm_1/bins?fresh=true: {\\\"detail\\\":\\\"Pool dlmm_1 not found or user SP3YWTZQQ7ZYHG1ECEG6AJAT4KX36W2NA7Q6TCKBG has no pool bins\\\"}\" }This contradicts the skill's own SKILL.md which explicitly claims
first-time support:
Root cause
getUserBins(around line 494) calls `fetchJson` with no try/catch. BFFreturns HTTP 404 with detail `"Pool {pool} not found or user {addr} has
no pool bins"` when a wallet has zero positions — a normal first-time
case. The throw bubbles up.
Downstream code at line 498+ already gracefully handles an empty `bins`
array via `.filter(b => b.userLiquidity > 0n)`. The fix is to catch the
specific 404 shape and return `[]`.
API surface check
Live spec: https://bff.bitflowapis.finance/api/app/openapi.json
Pool Bins) is the user-position state endpoint.
and `/api/quotes/v1/bins/{pool_id}` calls — those checks already run
before this call in `collectContext`.
valid means "this wallet has no position yet," not "the pool is
unusable." That's the semantic this PR codifies.
Change
Wrap the `fetchJson` in try/catch. Catch ONLY the specific
`HTTP 404 from ... has no pool bins` shape; let any other 404 or API
failure propagate.
```typescript
async function getUserBins(...) {
let response: UserBinsResponse;
try {
response = await fetchJson(
`${BITFLOW_API}/api/app/v1/users/${wallet}/positions/${poolId}/bins?fresh=true`
);
} catch (error) {
if (
error instanceof Error &&
error.message.startsWith("HTTP 404 from") &&
/has no pool bins/i.test(error.message)
) {
return [];
}
throw error;
}
// ... existing parsing logic unchanged
}
```
and deny-mode transaction safety: all unchanged.
Verification
`bun run typecheck` passes (mandatory per CONTRIBUTING.md).
End-to-end on Stacks mainnet across 3 wallet shapes:
Cross-pool variants: dlmm_1, dlmm_3, dlmm_7 — all pass.
Out of scope
The full first-time broadcast path (`run --confirm=DEPOSIT`) was not
exercised in this PR's test pass. The status preview surfaces an
`activeBinTolerance` discrepancy between BFF-reported
`observedActiveBinId` and router-reported `routerExpectedBinId` which
would block broadcast at `maxDeviation: 0`. That looks like a separate
router-vs-BFF sync issue beyond this PR's scope — recommend separate
investigation before production first-time deposits.
Why this matters
Every new LP onboarding via this skill is currently blocked at the first
read. The skill on aibtc.com/skills serves the version in this repo, so
the live agent ecosystem is affected today. This is a targeted try/catch
that unblocks the entire happy path the SKILL.md already documents.