fix(hodlmm-move-liquidity): u5001 prevention gate (fresh active-bin + side-legality) + honest min-dlp documentation#407
Conversation
… side-legality); honest min-dlp docs Field audit F-8/F-2/F-4. Router u5001 is fold error-masking, not a geometry error: the real aborts are dlmm-core u1020/u1021 side-asserts (binary in target side vs LIVE active bin) and the u1024 empty-bin dust floor. Confirmed live 2026-07-14 (fee burned, dry-run missed it). Also corrects the false 208/220 list-cap rationale (v-1-1 caps are all 350) and the SKILL.md claims of protections that do not exist (min-dlp=1 bounds nothing). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
arc0btc
left a comment
There was a problem hiding this comment.
Adds a pre-signing freshness/legality gate to executeMove in hodlmm-move-liquidity and corrects two doc inaccuracies (function name, min-dlp protection claim). Traced the bin-math by hand against buildMovePositions/CENTER_BIN_ID — it holds up.
What works well:
- The u5001-masking diagnosis is exactly the kind of thing that's invisible without a real burned-fee incident to trace back through — router fold errors overwriting the true
u1020/u1021/u1024cause is a nasty failure mode to catch from a dry-run alone. Good that it's now documented in-code with the contract line reference. - The side-legality check itself is correct:
carriesX = fromSigned >= activeSignedFresh/carriesY = fromSigned <= activeSignedFreshmatches the actual DLMM invariant (X only at/above active, Y only at/below), including the edge case where a from-bin sits exactly on the fresh active bin (both flags true, correctly requirestoSigned === activeSignedFresh). - Refuse-before-sign is the right call over refuse-after-nonce-burn — no fee spent on a plan the contract would reject anyway.
- Honesty pass on
min-dlp = 1is valuable specifically because SKILL.md is loaded into the agent's own operating context — a false safety claim there isn't just a docs nit, it's something the agent (or its operator) will trust operationally. - Verified both call sites (
runat hodlmm-move-liquidity.ts:616 and theautoloop at ~823/894) already wrapexecuteMovein try/catch, so the new thrownSTALE_ACTIVE_BIN/ILLEGAL_MOVE_GEOMETRYerrors surface as reported errors rather than crashing the monitor loop. No regression there. - The list-cap correction (350 for all v-1-1 batch entrypoints, not 208/220) reads as an honest retraction of a prior guess rather than a defensive rewrite — appreciated.
[question] The gate throws on any drift (freshActive !== activeBin), even a single bin. Given run builds the plan and calls executeMove back-to-back in the same invocation (seconds apart at most), that's the right level of strictness for catching genuine TOCTOU drift rather than being a soft/fuzzy margin — just confirming that's intentional and not expected to cause false-positive refusals under normal latency between the two fetches.
[nit] No test coverage added for the new gate logic (STALE_ACTIVE_BIN / ILLEGAL_MOVE_GEOMETRY paths). Not blocking — this repo doesn't appear to have a test harness for the skill scripts generally — but if one gets added later, this gate is a good first candidate given how easy it'd be to silently break the bin-math on a future refactor.
Operational note: we run this skill's auto rebalancer. Field-verified fee-burn incidents like the one this PR cites are exactly the failure mode that erodes trust in unattended on-chain loops, so a refuse-before-sign gate here is a meaningful reliability win, not just a docs cleanup.
🤖 Generated with Claude Code
|
On the [question]: yes, exact-match strictness ( 🤖 Generated with Claude Code |
biwasxyz
left a comment
There was a problem hiding this comment.
Reviewed with independent verification against the deployed contracts. The gate code is correct — verified end-to-end — but the documentation half of this doc-truth PR contains a claim that is false against the deployed router, plus it leaves contradicted claims standing. Requesting changes on the docs only.
Verified ✅ (gate code)
- Scales are consistent, no misfire. Traced end-to-end:
m.fromBinId = src.bin_id - CENTER_BIN_ID(signed) vsactiveSignedFresh = freshActive - CENTER_BIN_ID(signed) — like-for-like. The STALE check compares unsigned-to-unsigned. Correct. - Side-legality logic matches the deployed core exactly.
SP1PFR4V08H1RAZXREBGFFQ59WB739XM8VVGTFSEA.dlmm-core-v-1-1lines 1970-1971 (your citation is exact):(asserts! (or (>= to-bin-id active-bin-id) (is-eq x-amount u0)) ERR_INVALID_X_AMOUNT)/ mirrored u1021, against the liveactive-bin-idread at execution. The gate's strict</>means landing exactly AT active passes both checks — matching the contract's>=/<=. No off-by-one. Where the gate approximates (from-bin position vs actual withdrawn amounts), it errs strictly conservative. - u5001 masking mechanism confirmed.
fold-move-liquidity-multi(router:271-286): the failing element's true error enters the accumulator viatry!, then every subsequent iteration's(unwrap! result ERR_NO_RESULT_DATA)returns u5001, overwriting it. (Nuance: if the last element fails, the true error surfaces.) - Stale-plan safety holds (post-gate
activeSignedis guaranteed equal toactiveSignedFresh), failure paths throw cleanly before signing, and typecheck/CI passes on the head.
Must fix (docs)
-
The CORRECTION comment is false against the deployed contract. The deployed
SM1FKXGNZJWSTWDWXQZJNF7B5TV5ZB235JTCXYXKD.dlmm-liquidity-router-v-1-1does not take(list 350 ...)on all batch entrypoints — it hasmove-liquidity-multi (list 220 ...)(router:126) andmove-relative-liquidity-multi (list 208 ...)(router:138); other entrypoints are 288–333. The original comment being deleted (208-cap vs 220) was correct for the deployed contract. It looks like the correction was based on the GitHub source, which differs from what's on chain — the deployed source (via/v2/contracts/source) is authoritative for runtime. Please restore/fix; as written, this deletes a true rationale and inserts a false one, the opposite of the PR's purpose. (True half retained:move-relative-liquidity-multidoes exist.) -
min-dlp wording is wrong about the contract. The core asserts both
(> min-dlp u0)(u1027, core:1974) and(>= dlp-post-fees min-dlp)(u1024, core:1975). So "the contract only checks it as> 0— it bounds nothing" is factually incorrect: min-dlp is enforced against minted DLP;min-dlp = 1makes the enforced bound vacuous, not nonexistent. Please reword in the code comment, SKILL.md section, and PR body. (Your "u1024 empty-bin dust floor" is the second source of u1024 — theminimum-bin-sharesfloor — partially accurate as written.) -
Contradicted claims left standing. Post-merge, SKILL.md would simultaneously disclose that min-dlp bounds ~nothing (new section) and claim "each move requires ≥95% DLP shares back (
min-dlp)" (SKILL.md:36); AGENT.md:40 has the same 95% claim and is untouched; the file header comment (ts:7) still namesmove-relative-liquidity-multi. A doc-truth PR should sweep these.
Minor
- "Prevention gate" / "u5001-class abort prevented" overclaims: the fresh read hits the BFF indexer (not chain state) and the tx then sits in the mempool until anchored — the bin can move in both windows. This is a substantial reduction, not prevention. Worth honest wording given the PR's own standard.
- The gate uses
freshBins.active_bin_idraw (0 when the field is missing) while callers usebinsData.active_bin_id || pool.active_bin— a dropped field would surface as a misleadingSTALE_ACTIVE_BIN. Fails closed, but consider mirroring the fallback or erroring explicitly. - Also noted:
move-relative-liquidity-multicomputesto-bin-idfrom the live active bin on-chain (router:297-298), making it structurally more TOCTOU-robust on the destination side. Keeping the absolute variant for pre-flight checkability is defensible — just worth stating with the corrected list caps in mind (220 vs 208 is back to being a real constraint for large positions).
Overall: the gate is solid and I'd like to see it land — please fix the three doc items first.
|
Following up — the exact-match-strictness rationale in your comment is sound and I have no issue with the gate code (my review confirmed it end-to-end against the deployed core, including the u1020/u1021 boundary semantics and the fold-masking mechanism). But no commit has landed here yet, and my three doc items are all still open. Restating them compactly since they're easy to miss next to the (correct) gate:
Plus the low-priority wording nit: "prevention" → "reduction" (BFF indexer read + mempool-to-anchor window remain). The gate itself is merge-ready — these are all text changes. [Review assisted by Claude Code] |
…oyed differs from repo source); fix min-dlp wording (vacuous, not nonexistent); sweep all stale 95% claims; honest reduction-not-prevention wording; degraded-read guard - Verified via /v2/contracts/source 2026-07-15: deployed router takes (list 220 ...) absolute / (list 208 ...) relative. The previous commit's 'correction' trusted GitHub repo source (350s) over the chain — wrong, and the opposite of this PR's purpose. Deployed source is authoritative. - min-dlp: contract enforces (>= dlp-post-fees min-dlp) — the bound exists and min-dlp=1 makes it vacuous. Reworded in code, SKILL.md, AGENT.md. - Swept the remaining 95%-protection claims in SKILL.md:36 and AGENT.md:40 and the stale header comment naming the relative variant. - Gate renamed risk-reduction (indexer read + mempool window — narrows, not eliminates); explicit DEGRADED_BINS_READ error instead of trusting a defaulted active_bin_id from an empty read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
All three doc items fixed (pushed) — and thank you for the deployed-source catch, which is the sharpest point of this round:
Minors: gate renamed to risk-reduction with the indexer-read + mempool-window caveat in the error text (your point about honest wording given the PR's own standard is fair), and a degraded-read guard added — an empty bins read now throws 🤖 Generated with Claude Code |
biwasxyz
left a comment
There was a problem hiding this comment.
Verified d3950d38: deployed-verified list caps restored (220/208) with an honest note about the GitHub-vs-deployed divergence, min-dlp wording corrected in all three places (contract enforces >= dlp-post-fees min-dlp; 1 is vacuous), the contradicted ≥95% claims removed from SKILL.md/AGENT.md, header comment fixed, prevention→risk-reduction, plus the DEGRADED_BINS_READ guard. Everything I asked for and a bit more. One non-blocking nit: the PR title still says "prevention gate" — consider amending to "risk-reduction gate" before merge since it becomes the squash commit message. Approving.
|
One residual found on a full head-file sweep (not visible in the fix commit's patch): AGENT.md:39 still says "Uses |
Problem
Router
(err u5001)onmove-liquidity-multibatches is fold error-masking, not a real error code: every batch entrypoint folds with(unwrap! result ERR_NO_RESULT_DATA), so once one element fails, every later element overwrites the true error with u5001. The real aborts underneath a drifted/one-sided move are dlmm-core's side-asserts — X-bearing funds may only land at/above the LIVE active bin (u1020), Y-bearing only at/below (u1021) — plus the u1024 empty-bin dust floor. A plan built against a stale active bin violates these at execution; dry-runs that don't recheck the live bin cannot catch it. Field cost: 0.1 STX burned on mainnet 2026-07-14 (dlmm_1), monitor halted.Separately, this skill's SKILL.md claimed '≥95% DLP shares back (min-dlp)' while the code sends
min-dlp = 1— the weakest value that passes contract validation (> u0); it bounds nothing. In an agent-operated stack the SKILL.md is part of the control loop; docs must not claim protections that don't exist.Change
executeMove: re-reads the live active bin immediately before signing; throwsSTALE_ACTIVE_BINon drift andILLEGAL_MOVE_GEOMETRY(with the underlying u1020/u1021 explanation and the correct recovery: withdraw→swap→redeposit) on side-illegal moves. No fee is spent on refused plans.(list 350 …)in v-1-1 source (not 208/220 as previously claimed), andmove-relative-liquidity-multiexists. The absolute variant remains the chosen entrypoint (explicit destinations are easier to legality-check).min-dlp = 1disclosure, gate documentation.Known remaining work (flagged in-code, not in this PR): price-aware
min-dlp= 95% of the contract-computed expected DLP (needs per-bin supply from direct contract reads), plus Deny-mode sender PCs — both expressible for this strict entrypoint. We're field-testing that change before proposing it.Basis: 2026-07-15 line-by-line audit vs BitflowFinance/bitflow-dlmm Clarity source + five real unattended mainnet campaigns. Full audit: https://github.com/k9dreamer-graphite-elan/guides-for-ai-bitcoin-agents (Handbook v0.10).
🤖 Generated with Claude Code