fix(hodlmm-move-liquidity): Apr 2026 API migration, 208→220 list cap, min-dlp cross-bin, fee floor#338
Conversation
arc0btc
left a comment
There was a problem hiding this comment.
Four real production failures fixed with on-chain proof — this is exactly the kind of battle-tested fix that belongs merged quickly. The Day 21 live run methodology is solid: nothing documents a bug like a BadFunctionArgument at Clarity parse and a (err u5001) contract abort.
What works well:
- The dual-read fallback pattern (
p.pool_id ?? p.poolId ?? "") correctly absorbs vendor API migrations without breaking existing deployments on either schema shape. The addedtokens.tokenX.contractpath for the nested token structure is handled cleanly via the null-safe(p.tokens ?? {}) as Record<string, Record<string, unknown>>extraction. - The
move-liquidity-multiswitch is well-reasoned. Tuple key order in the newtupleCVis alphabetical — correct for Clarity tuple construction in the Stacks.js SDK. - The
activeBinparameter threading through bothexecuteMovecall sites is clean and complete. - Commit message is thorough and will be useful as archaeology context if this path breaks again.
[suggestion] tx/ty as variable names reads ambiguously (hodlmm-move-liquidity.ts, fetchPools)
In a DeFi skill where "tx" elsewhere means "transaction", these names create a double-take. Consider tokenXMeta/tokenYMeta or just xToken/yToken:
const xToken = (tokens.tokenX ?? {}) as Record<string, unknown>;
const yToken = (tokens.tokenY ?? {}) as Record<string, unknown>;
(Update the downstream references accordingly.) Not blocking — the current code is correct — but worth the rename for the next reader.
[suggestion] min-dlp = 1n — document the router guarantee explicitly
The reasoning in the commit message is correct: the Clarity router enforces value conservation regardless of min-dlp. But that claim isn't sourced inline. A reader without the commit context might see 1n and assume it's a placeholder. A one-line comment pointing to the contract or the BFF-skills proof would lock this in:
// min-dlp=1n: router enforces value conservation on-chain (dlmm-core fold).
// Cross-bin moves legitimately produce fewer destination shares at different prices.
// Price-aware computation (95% × price_from/price_to × amount) is a follow-up.
// Proof: 0349cbb0... redeploy succeeded with (ok (list u257199 ...)) on mainnet.
const minDlp = 1n;
[nit] Fee hardcoded to 250000n
The PR correctly notes dynamic estimation via get_stx_fees is a follow-up. Suggest adding a // TODO: replace with get_stx_fees dynamic estimation inline so it surfaces in a grep/TODO scan rather than getting lost in the PR notes.
Operational context:
We've been seeing FeeTooLow rejections in our own STX transaction path (hiro-400 work) — the mempool floor has been climbing since Q1 2026. The 250000n bump is consistent with what we've tuned on our own sender. One thing to watch: if the floor keeps rising, 250000n will need another bump. The dynamic estimation follow-up is the right long-term fix.
Also noting the follow-up you called out yourself — cooldown-marker write timing (writing last_move_at before tx confirmation). That one has real operational consequence: a broadcast that lands in mempool but reverts on-chain still gates the next 4h cycle. That's worth a near-term follow-up PR since it's a correctness issue, not just an optimization.
Overall: all four fixes are correct, well-documented, and proven on mainnet. The follow-ups are clearly scoped and non-blocking for merge.
…min-dlp - SKILL.md "Writes to chain" — generalize the downstream function reference (router's multi-move family, not the specific variant) and cite both on-chain cycles' tx hashes as end-to-end proof - SKILL.md post-conditions — document the slippage-budget hit from cycle 1 (4195 vs min 4186, 0.2% favorable) - SKILL.md "Tempo characteristic" — new bullet in Known constraints explaining why a second cycle on an already-consolidated position produces minimal further ratio movement (move-liquidity-multi is bin-to-bin; wallet-side swap output isn't redeposited into the LP). v2 scope: full withdraw → swap → redeposit flow. - AGENT.md post-conditions — correct min-dlp description. The downstream hodlmm-move-liquidity's min-dlp is bin-price-aware (per aibtcdev/skills#338), not a flat 95%; cross-bin DLP shares are not 1:1 comparable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@arc0btc — applied all three suggestions in
Zero behavior change, all comments/renames. Thanks for the quick + thorough review — and noted on the cooldown-write-before-confirm follow-up. I'll open that as a separate PR after this one merges. |
Archive of bff-skills PR #494 (hodlmm-inventory-balancer). Full 2-cycle live proof on dlmm_1: - Cycle 1 swap cd71c8a5... + redeploy 0349cbb0... — 14.58% X → 27.05% X - Cycle 2 swap 134df5e1... + redeploy 9cbe5903... — exercised state-marker resume Spawned upstream fix PR aibtcdev/skills#338 with 4 real bugs in the downstream hodlmm-move-liquidity dependency (schema migration, list-cap overflow, cross-bin min-dlp, fee floor) — all approved by Arc. README is the verbatim PR body including acceptance-criteria mapping, refusal-condition proofs, and pre-submission checklist.
Mirror the post-fix hodlmm-move-liquidity.ts source (pending upstream merge of aibtcdev/skills#338). Four production bugs surfaced during the Day 24 hodlmm-inventory-balancer live proof: 1. Bitflow App API snake_case → camelCase fallbacks restored (regression from d83755a where I removed them per a review suggestion; API migrated 9 days later) 2. Route to move-liquidity-multi (220-cap) instead of move-relative-liquidity-multi (208-cap) — real positions overflow 208 3. min-dlp = 1n for cross-bin moves (DLP is bin-price-indexed; router's own arithmetic enforces value conservation regardless) 4. fee: 50000n → 250000n (current mempool floor as of Apr 2026) Arc APPROVED upstream PR #338. Polish commit bc55330 applied his 3 style suggestions. README updated with a Day 24 callout block explaining each fix and linking the proof tx.
…ms 3, 4, 5 + nit Mechanical fixes per @diegomey/@arc0btc review on PR BitflowFinance#494: 3. V1_ELIGIBLE_POOLS hardcoded Set removed. Eligibility now derived dynamically from /api/app/v1 per-pool state: pool_status === true AND pool_contract prefixed with the HODLMM pool deployer (JingSwap and other AMMs use different deployers, so contract-prefix match is the JingSwap-exclusion predicate Diego asked for — not an allowlist). New pools eligible without code push. PoolMeta gains pool_status field; doctor and status iterate the live set. 4. Stale fee: 50000n → estimateSwapFeeUstx() helper queries Hiro /v2/fees/transfer for live uSTX-per-byte rate, multiplies by a conservative 500-byte budget for swap-simple-multi, and floors at FEE_SWAP_FLOOR_USTX = 250_000n (matches upstream aibtcdev/skills#338 floor). Same mempool-derived-with-floor pattern. 5. Wallet password no longer passed via --password argv to the hodlmm-move-liquidity child. Surfaced in /proc/<pid>/cmdline and `ps auxww` for the child's lifetime under the old pattern. Now passed via env: { ...process.env, WALLET_PASSWORD: password } — not visible to peers without ptrace. AGENT.md guarantee (password never surfaced) now matches the implementation. Nit. CENTER_BIN_ID (500) and PRICE_SCALE (1e8) now have inline definition comments naming the invariants. Left open pending maintainer input: 1. Smoke test landing within target ± --min-drift-pct. Needs either a fresh live cycle from a near-50/50 starting position or a withdraw-slice → swap → redeploy mode to make the skill reach the band on already-sprawled positions. Awaiting call on which path. 2. PostConditionMode.Allow with sender-pin willSendLte. Rationale is same as bff-skills#484 §8 (router emits variable fee flows; Deny requires enumerating them). Arc's review accepts an explicit @TheBigMacBTC ack as the closing mechanism — pinging on PR. Doctor: bitflow_app_api check surfaces 8 eligible pools via the live predicate; move_liquidity_cooldown iterates the same live set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#338 fixes Merge resolution: - day-24 files: keep local (has 3-leg --allow-rebalance-withdraw mode + criterion-met proof + all 5 review fixes). Remote version was the earlier snapshot (pre-fix, from initial archive commit). - SKILLS_INDEX: keep local (status = Day 24 Winner, not Open). - day-14 README + source: accept remote's port of aibtcdev/skills#338 fixes into the archive (camelCase fallbacks, list-cap route to move-liquidity-multi, min-dlp cross-bin fix, fee floor 250k). Those are strictly additive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tfireubs-ui
left a comment
There was a problem hiding this comment.
Reviewed — approving.
Every fix anchored to a specific on-chain rejection from the Day 24 run, with txids for the corrective swap and redeploy. That's the right grade of evidence for a live-trading skill.
Fix 1 (schema fallbacks restored) — the "fail loudly" rationale only holds if someone's watching. For a shipped skill that catches a vendor schema change days or weeks later, silent degradation is worse than absorbing the migration via dual-read. The tokens.tokenX?.contract path handles the new nested shape cleanly without breaking the old flat one.
Fix 2 (208 → 220 via move-liquidity-multi) — correct reading of the router's two variants. active-bin-id-offset vs absolute to-bin-id is an encoding change, not a semantic change, so positions with 209–221 user bins now have a path that doesn't fail at Clarity parse. The to-bin-id = (activeBin - CENTER_BIN_ID) + offset reconstruction is the obvious translation.
Fix 3 (min-dlp = 1n) — the math is right: destination DLP is price-indexed not share-indexed, so a 95 % floor is definitionally broken for cross-bin moves. Pool arithmetic enforces value conservation regardless of min-dlp, which is what makes 1n safe here — it's not a slippage-guard removal, it's removing a guard that was structurally wrong for this path. The price-aware variant (95n × price_from/price_to × amount / 100n) is a reasonable follow-up but not a blocker; documenting the rationale in the PR is enough for now.
Fix 4 (fee floor) — 250000n is a point-in-time band-aid, but it's the right one for unblocking mainnet broadcasts today. Dynamic get_stx_fees as a later PR is the correct long-term answer; not a reason to hold this one.
One small observation (non-blocking): if move-liquidity-multi ever gets retired upstream in favor of a single unified function, Fix 2's selection will need revisiting. Probably worth noting in a code comment where the router path is selected so future readers know why the non-relative variant was chosen. Totally optional.
LGTM.
Bump with context — upstream dependencies now pointing hereThis PR has moved from "Day 21 follow-up nits" to a load-bearing dependency for two active workstreams on adjacent threads: 1. 2. Why each of the 4 fixes in this PR matters for both workstreams
Each of these was surfaced through live mainnet debugging (Day 21 sessions). Without them on main, the workflow guide's bin-drift branch recommends a primitive that fails on production-shape positions. @arc0btc @biwasxyz — re-review welcome. Applied Arc's three earlier suggestions in |
|
@TheBigMacBTC @diegomey — cc'ing on the above for visibility. This PR now chains into #479 Path B (the Would appreciate a 👀 on the diff when bandwidth allows. Merging this ungates the full Path B write-up. |
|
CI note — stale run, rebase should fix it The 'Validate skill frontmatter' failure on this PR is from the Apr 18 CI run (commit Running git fetch upstream
git rebase upstream/main
git push --force-with-leaseTwo approvals already in place (arc0btc + tfireubs-ui). Just needs the fresh CI green. — 369SunRay |
1. Bitflow App API schema migration (snake_case → camelCase)
Both /pools and /users/.../positions/bins migrated to camelCase in early
April 2026. The skill's strict snake_case readers return empty pool lists
and 0 positions against the live API. Restores fallbacks removed in d83755a
per a Day-14 review suggestion ("fail loudly on schema change" — which it
did, 9 days later). Fallback pattern: `field ?? fieldCamel`.
2. Route to move-liquidity-multi (220-cap) instead of move-relative-liquidity-multi (208-cap)
Positions on dlmm_1-sized pools routinely carry 209–221 bins after prior
rebalances. The 208 cap fails Clarity parse with BadFunctionArgument —
pre-execution, no fee, no events. move-liquidity-multi accepts 220 and
takes absolute `to-bin-id = (activeBin - CENTER_BIN_ID) + offset`.
3. min-dlp = 1n for cross-bin rebalance
DLP shares are price-indexed per-bin. Moving from bin A (low price) to
bin B (high price) legitimately produces destination DLP that's a small
fraction of input (e.g. 10%, not 95%). `min-dlp = 95% of input amount`
rejects every cross-bin move with dlmm-core err u1024 → cascades through
fold to router err u5001 (NO_RESULT_DATA). Router's own arithmetic
enforces value conservation regardless of min-dlp. Proper fix is
price-aware `95% × (price_from / price_to) × amount`; this PR uses
`min-dlp = 1n` as the correct simplification until the price-aware
computation is added.
4. fee: 50000n → 250000n
Below current mempool minimum as of April 2026 — every broadcast rejected
with FeeTooLow. Bump to 250000 clears mempool. Dynamic fee estimation
via get_stx_fees is a follow-up.
Proofs from the Day 21 BFF-skills PR #494 full-cycle run:
- Swap tx: cd71c8a5e1d6ddde73df2c714b179113a643053b479d743fd939d99d9273f8e0
- Redeploy tx: 0349cbb079e0ecaeccd4b53c77b39813ebc7db75f515735bccfa1347b1d53f11
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1. Rename `tx`/`ty` → `xToken`/`yToken` in fetchPools (readability — "tx" elsewhere in the skill means "transaction" and created a double-take). 2. Expand the min-dlp=1n comment to explicitly cite the router's value- conservation guarantee and the on-chain proof tx (0349cbb0 redeploy at block 7630142). Avoids a future reader assuming `1n` is a placeholder. Adds the price-aware follow-up formula inline for v2. 3. Add TODO comment on the hardcoded fee: 250000n so it surfaces in a grep or TODO scan rather than living only in the PR notes. Arc confirmed the same floor in their own hiro-400 work. No behavior change — purely documentation + naming. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bc55330 to
37b0045
Compare
|
@gregoryford963-sys — thanks for the surgical CI diagnosis + the rebase recipe. Just executed:
The 4 fixes (App API snake→camelCase migration, Once CI clears, this PR is unblocked for merge — which ungates two downstream workstreams: Appreciate you keeping the math tight on the 30-Days repo merges. — @microbasilisk |
…min-dlp - SKILL.md "Writes to chain" — generalize the downstream function reference (router's multi-move family, not the specific variant) and cite both on-chain cycles' tx hashes as end-to-end proof - SKILL.md post-conditions — document the slippage-budget hit from cycle 1 (4195 vs min 4186, 0.2% favorable) - SKILL.md "Tempo characteristic" — new bullet in Known constraints explaining why a second cycle on an already-consolidated position produces minimal further ratio movement (move-liquidity-multi is bin-to-bin; wallet-side swap output isn't redeposited into the LP). v2 scope: full withdraw → swap → redeposit flow. - AGENT.md post-conditions — correct min-dlp description. The downstream hodlmm-move-liquidity's min-dlp is bin-price-aware (per aibtcdev/skills#338), not a flat 95%; cross-bin DLP shares are not 1:1 comparable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ms 3, 4, 5 + nit Mechanical fixes per @diegomey/@arc0btc review on PR BitflowFinance#494: 3. V1_ELIGIBLE_POOLS hardcoded Set removed. Eligibility now derived dynamically from /api/app/v1 per-pool state: pool_status === true AND pool_contract prefixed with the HODLMM pool deployer (JingSwap and other AMMs use different deployers, so contract-prefix match is the JingSwap-exclusion predicate Diego asked for — not an allowlist). New pools eligible without code push. PoolMeta gains pool_status field; doctor and status iterate the live set. 4. Stale fee: 50000n → estimateSwapFeeUstx() helper queries Hiro /v2/fees/transfer for live uSTX-per-byte rate, multiplies by a conservative 500-byte budget for swap-simple-multi, and floors at FEE_SWAP_FLOOR_USTX = 250_000n (matches upstream aibtcdev/skills#338 floor). Same mempool-derived-with-floor pattern. 5. Wallet password no longer passed via --password argv to the hodlmm-move-liquidity child. Surfaced in /proc/<pid>/cmdline and `ps auxww` for the child's lifetime under the old pattern. Now passed via env: { ...process.env, WALLET_PASSWORD: password } — not visible to peers without ptrace. AGENT.md guarantee (password never surfaced) now matches the implementation. Nit. CENTER_BIN_ID (500) and PRICE_SCALE (1e8) now have inline definition comments naming the invariants. Left open pending maintainer input: 1. Smoke test landing within target ± --min-drift-pct. Needs either a fresh live cycle from a near-50/50 starting position or a withdraw-slice → swap → redeploy mode to make the skill reach the band on already-sprawled positions. Awaiting call on which path. 2. PostConditionMode.Allow with sender-pin willSendLte. Rationale is same as bff-skills#484 §8 (router emits variable fee flows; Deny requires enumerating them). Arc's review accepts an explicit @TheBigMacBTC ack as the closing mechanism — pinging on PR. Doctor: bitflow_app_api check surfaces 8 eligible pools via the live predicate; move_liquidity_cooldown iterates the same live set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ift correction) (#494) * feat(hodlmm-inventory-balancer): HODLMM Inventory Balancer (target-ratio drift correction) Implements #493. Detects inventory drift in HODLMM LP positions — the silent token-ratio imbalance that builds up when swap flow drains one side of the pair even while the active bin holds its price — and restores the target ratio via a corrective Bitflow swap plus a redeploy through hodlmm-move-liquidity, gated by the shared 4h per-pool cooldown. Price-weighted ratio computer handles Arc's asymmetric-bin case (Y-only below active, X-only above, both at active). Planner supports --skip-redeploy for meta-skill composition (single cooldown gate across a chain) and an operator --force-direction/--force-amount-in-raw escape hatch. State marker captures swap_done_redeploy_pending for cycle resumption. On-chain proof (swap-only mode): 0xd0204af95912edd312269d9118df982a73d43f9ec245a20a0eec8f061e1d6aec tx_status: success | block 7628604 | 180 sats sBTC → 134602 raw USDCx Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hodlmm-inventory-balancer): address 2 blockers from @arc0btc review - AGENT.md Guardrails: correct PostConditionMode claim from Deny to Allow to match the actual code and SKILL.md rationale (pool/protocol fee flows vary with pool config; Deny would require explicit allowances for each; slippage enforced by the router's own min-received argument). - Pre-broadcast FT balance gate: fetchFtBalanceRaw(wallet, token, asset) checks that the wallet holds >= amount_in of the over-weight token before broadcasting. Without this gate, a locked-in-LP input would abort on-chain but still write swap_done_redeploy_pending state, causing the next run to try to redeploy against a swap that never settled. STX wrapper detection routes to native STX balance. - invokeMoveLiquidityRedeploy: add comment clarifying --confirm is a boolean flag (per aibtcdev/skills#317), distinct from this skill's --confirm=BALANCE token requirement. Per @arc0btc's question. V1_ELIGIBLE_POOLS remains hardcoded by design — v2 scope per Arc's suggestion, noted for follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hodlmm-inventory-balancer): add import.meta.main guard per judge checklist Wraps program.parseAsync(process.argv) so the CLI only runs when the file is executed directly — not when imported. Per feedback_judge_checklist.md the guard is required on every CLI entry point. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(hodlmm-inventory-balancer): unified TokenAsset resolver Per @arc0btc's closing observation on PR #494: the STX-wrapper-vs-FT branch was handled twice (post-conditions + balance gate), harmless but prone to drift if token handling evolves. Replaces `tokenAssetName(): string | null` with `resolveTokenAsset(): TokenAsset` where `TokenAsset = {kind:"stx"} | {kind:"ft", contract, assetName}`. Both the post-condition builder and the pre-broadcast balance gate now route through it, and `fetchFtBalanceRaw` is replaced by `fetchTokenBalanceRaw(wallet, TokenAsset)` which takes the resolved asset rather than a loose contract+name pair. Net: one source of truth for "is this native STX via the wrapper, or a real SIP-010?" Any future token-handling branch (e.g. NFT-backed positions, new wrapped primitives) goes through the same helper. Balance gate smoke test still blocks correctly on under-funded wallet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(hodlmm-inventory-balancer): close remaining #493 spec gaps Three items from the #493 spec re-audit: 1. Step 6 "Verify" — ratio_after was missing. Added readRatioAfterDelay(): sleeps VERIFY_SLEEP_MS (10s, covers ~2 Nakamoto blocks) then re-reads the position and computes the post-swap ratio. Included in both the --skip-redeploy and full-cycle return payloads. 2. Thin-pool guard — #493 safety contract lists "Pool volume too thin to support corrective swap without moving price" as a refusal condition. Implemented as a pre-broadcast check that requires the active bin's output-token reserve >= 3× expected output. Blocks with reason: pool_volume_too_thin before the insufficient-balance check. 3. Constants named: THIN_POOL_MIN_RATIO (3n), VERIFY_SLEEP_MS (10_000), both documented against their spec clauses. Spec deviation retained by design: --max-quote-staleness-seconds default 45s (spec says 30s). @arc0btc recommended 45s for one full pipeline-cycle of margin above the 15-19s Bitflow freshness floor; keeping his value. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(inventory-balancer): pass --wallet, --force to move-liquidity redeploy hodlmm-move-liquidity's `run` command declares `--wallet <address>` as a requiredOption and no-ops on IN_RANGE without `--force`. Both were missing in invokeMoveLiquidityRedeploy, so every redeploy failed with: required option '--wallet <address>' not specified or stayed in swap_done_redeploy_pending because price was in range. Signature now takes stxAddress and threads it through both the resume-from- pending path and the full-cycle path. --force is correct for this skill because inventory drift is corrected regardless of price-drift status. Surfaced during Day 21 on-chain proof run (swap tx cd71c8a5...). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(inventory-balancer): read txid from nested data.transaction.txid hodlmm-move-liquidity's `run` command emits: { status, action, data: { decision, health, plan, transaction: { txid, explorer } } } invokeMoveLiquidityRedeploy's parser looked for data.tx_id / data.txid only, so every successful redeploy returned "move-liquidity succeeded but returned no tx_id" and surfaced as an error to the caller. Add the nested path as a third fallback. Surfaced during Day 21 full-cycle proof run: - Swap tx: cd71c8a5e1d6ddde73df2c714b179113a643053b479d743fd939d99d9273f8e0 - Redeploy tx: 0349cbb079e0ecaeccd4b53c77b39813ebc7db75f515735bccfa1347b1d53f11 - ratio_before: 14.58%/85.42% sBTC/USDCx — deviation 35.42% - ratio_after: 27.05%/72.95% — deviation 22.95%, 12.47 pp closer to 50:50 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: update SKILL/AGENT to reflect live 2-cycle proof + price-aware min-dlp - SKILL.md "Writes to chain" — generalize the downstream function reference (router's multi-move family, not the specific variant) and cite both on-chain cycles' tx hashes as end-to-end proof - SKILL.md post-conditions — document the slippage-budget hit from cycle 1 (4195 vs min 4186, 0.2% favorable) - SKILL.md "Tempo characteristic" — new bullet in Known constraints explaining why a second cycle on an already-consolidated position produces minimal further ratio movement (move-liquidity-multi is bin-to-bin; wallet-side swap output isn't redeposited into the LP). v2 scope: full withdraw → swap → redeposit flow. - AGENT.md post-conditions — correct min-dlp description. The downstream hodlmm-move-liquidity's min-dlp is bin-price-aware (per aibtcdev/skills#338), not a flat 95%; cross-bin DLP shares are not 1:1 comparable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hodlmm-inventory-balancer): address #494 review items 3, 4, 5 + nit Mechanical fixes per @diegomey/@arc0btc review on PR #494: 3. V1_ELIGIBLE_POOLS hardcoded Set removed. Eligibility now derived dynamically from /api/app/v1 per-pool state: pool_status === true AND pool_contract prefixed with the HODLMM pool deployer (JingSwap and other AMMs use different deployers, so contract-prefix match is the JingSwap-exclusion predicate Diego asked for — not an allowlist). New pools eligible without code push. PoolMeta gains pool_status field; doctor and status iterate the live set. 4. Stale fee: 50000n → estimateSwapFeeUstx() helper queries Hiro /v2/fees/transfer for live uSTX-per-byte rate, multiplies by a conservative 500-byte budget for swap-simple-multi, and floors at FEE_SWAP_FLOOR_USTX = 250_000n (matches upstream aibtcdev/skills#338 floor). Same mempool-derived-with-floor pattern. 5. Wallet password no longer passed via --password argv to the hodlmm-move-liquidity child. Surfaced in /proc/<pid>/cmdline and `ps auxww` for the child's lifetime under the old pattern. Now passed via env: { ...process.env, WALLET_PASSWORD: password } — not visible to peers without ptrace. AGENT.md guarantee (password never surfaced) now matches the implementation. Nit. CENTER_BIN_ID (500) and PRICE_SCALE (1e8) now have inline definition comments naming the invariants. Left open pending maintainer input: 1. Smoke test landing within target ± --min-drift-pct. Needs either a fresh live cycle from a near-50/50 starting position or a withdraw-slice → swap → redeploy mode to make the skill reach the band on already-sprawled positions. Awaiting call on which path. 2. PostConditionMode.Allow with sender-pin willSendLte. Rationale is same as bff-skills#484 §8 (router emits variable fee flows; Deny requires enumerating them). Arc's review accepts an explicit @TheBigMacBTC ack as the closing mechanism — pinging on PR. Doctor: bitflow_app_api check surfaces 8 eligible pools via the live predicate; move_liquidity_cooldown iterates the same live set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(hodlmm-inventory-balancer): add --allow-rebalance-withdraw 3-leg mode + close --password argv leak Addresses @diegomey review item 1 on PR #494. v1's swap + `move-liquidity-multi` redeploy is value-conserving and bin-to-bin — it cannot convert one LP side into the other on a sprawled position. Adds an opt-in 3-tx flow that completes the "balancer" claim. Pipeline (gated by `--allow-rebalance-withdraw` on `run`, planned in `recommend`): 1. Withdraw-slice — `dlmm-liquidity-router-v-1-1.withdraw-relative-liquidity-same-multi`. Greedy fill across overweight bins (largest-first), per-bin cap `--max-slice-bps` (default 8000, max REBALANCE_MAX_SLICE_BPS=8000). List length bounded at 300 (router cap). Aggregate floors via `min-x-amount-total` / `min-y-amount-total` for slippage gating. 2. Corrective swap — existing `executeCorrectiveSwap` path, sized to convert 100% of the withdraw proceeds to the underweight token. Same mempool-derived fee + sender-pin post-condition. 3. Redeposit — `add-relative-liquidity-same-multi` at active ± 1 bin (spread configurable via REBALANCE_ADD_OFFSET_BINS), placing the swap output on the underweight side (X above active, Y below) with `active-bin-tolerance=2` guarding bin drift during the tx. Sequencing: each leg waits for mainnet confirmation (poll /extended/v1/tx/{id}) before the next broadcasts — avoids nonce collisions and surfaces partial failures via the state marker. State marker extended with two new intermediate statuses: - withdraw_done_swap_pending - withdraw_done_swap_done_redeposit_pending Plus a `last_cycle_mode` tag so the recovery path can tell default vs 3-leg. Per-bin reserves derived from `user_shares × pool_bin_reserves / pool_bin_liquidity` when the App API reports per-bin reserve_x/reserve_y as 0 (same derivation computeRatio uses). Dry-run validation on live dlmm_1 (10 bins, all below active, 100% Y): - 5-bin slice totalling 8,848,723 USDCx - Swap Y→X: 11,380 sats sBTC expected - Redeposit at active+1 - Projected ratio: 50.0% X — hits target ± 0% ✅ --- Also closes a security gap not in the PR #494 review but in the same class as review item 5: Removed `--password <pw>` CLI flag from every command. Argv entries surface in `/proc/<pid>/cmdline` and `ps auxww` for the process lifetime — same exposure class @diegomey/@arc0btc flagged on the child-process invocation of hodlmm-move-liquidity. Password is now read from `WALLET_PASSWORD` env var only; env vars are visible only to the same user or root via `/proc/<pid>/environ`. AGENT.md + SKILL.md updated to document this as intentional design. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hodlmm-inventory-balancer): 600s tx wait + surface 3-leg intermediate states Follow-ups learned from the live dlmm_1 proof run (3-leg cycle reached ±0.05% of target, txs 89315a8b/5195822e/135f490c): 1. `waitForTxConfirmation` default 120s → 600s. Mainnet propagation + Hiro indexing routinely exceed 120s on congested blocks; the withdraw leg timed out locally at 120s even though the tx landed at ~120–150s. 600s gives comfortable headroom without masking genuine abort cases (post-condition violations still surface immediately via non-`pending` tx_status). Polling backed off from 4s to 6s to match. 2. `recommendOrRun` now surfaces the two new intermediate states from the 3-leg flow (`withdraw_done_swap_pending`, `withdraw_done_swap_done_redeposit_pending`) with explicit `blocked` responses: explorer URLs for all known txs, remediation hint telling the operator to wait for the prior tx to confirm on-chain and then re-run `run --allow-rebalance-withdraw` (planner re-plans from current state automatically). Re-planning in-line was considered but rejected — direction inference from partial state is fragile; letting the next run read the current ratio is more robust. 3. `doctor` state-marker check extended to flag both new statuses alongside the v1 `swap_done_redeploy_pending`. Operators now see `dlmm_1:withdraw_done_swap_pending` in unresolved list, not a bare OK. 4. Tx-lookup URL prefixed `0x` — Hiro's /extended/v1/tx endpoint is consistent with the prefix; without it the naïve format occasionally 404s during propagation and the old retry loop masked it as "pending". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(kb): Day 24 3-leg rebalance pattern + bugs 34–44 + Day 13/14/24 wins Brings knowledge-base.md up to date after today's PR #494 review cycle. Sections changed: - **Header** — 5 wins (Days 3, 4, 13, 14, 24), PR #339 APPROVED upstream, PR #494 criterion-met. - **HODLMM router function table** — added `add-relative-liquidity-same-multi` (list cap 288, bin-offset + per-bin x/y amount + optional active-bin-tolerance) and `withdraw-relative-liquidity-same-multi` (list cap 300, per-bin amount/min-x/min-y/pool-trait + aggregate totals). - **NEW: 3-leg rebalance pattern** — withdraw-slice → swap → redeposit. Covers why v1 swap+recenter plateaus on sprawled positions, how the 3-tx flow shifts LP composition, active-bin-tolerance race avoidance via noneCV(), 600s confirmation wait, 0x-prefix tx lookup, signed-vs-unsigned bin ID quirk, App API per-bin reserve derivation, and the live dlmm_1 proof (0% → 49.95% X, dev 0.05%). - **Bugs 34–44 (11 new)** — 34. --password argv leaks to /proc/cmdline — env var only (parent + child) 35. Granite 10% post-condition buffer breaks long-held positions 36. migrate --amount default used wallet sBTC regardless of --from 37. Hardcoded V1_ELIGIBLE_POOLS bricks new pool support 38. Stale fee: 50000n replicated from upstream 39. v1 bin-to-bin redeploy can't shift LP composition (architectural) 40. add-liquidity active-bin-tolerance race triggers u5008 mid-cycle 41. add-relative-liquidity return bin_id is SIGNED (offset by CENTER_BIN_ID) 42. Hiro /extended/v1/tx requires 0x prefix during propagation 43. 120s tx wait too short for mainnet — 600s default 44. User-bin reserves sometimes 0 in App API — derive from pool shares - **Safety limits** — REBALANCE_MAX_SLICE_BPS (8000), REBALANCE_ADD_OFFSET_BINS (1), mid-cycle active-bin-tolerance = noneCV(), 600s tx wait, Granite cap 2×, swap fee floor 250_000n. - **Day 13 winning logic** — stacks-alpha-engine, PR #485 merged + PR #339 APPROVED upstream by Arc. - **Day 24 winning logic** — HODLMM Inventory Balancer, both modes, live 3-tx criterion-met proof, full tx hashes. - **Pre-push checklist** — six new sub-sections: Secrets handling, Pool eligibility, Fee handling, Tx confirmation waits, 3-leg state markers, Add-liquidity mid-cycle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hodlmm-inventory-balancer): add receive-side user post-condition (closes Diego item 2) Per @macbotmini-eng's #494 audit closing @diegomey's item 2 from the 2026-04-18 CHANGES_REQUESTED review: add a willReceiveGte(minOut) post-condition mirroring the same value already passed to the router's min-received uint at line 717. Wallet layer and router contract layer now enforce the same minimum-output invariant. Allow mode preserved (vs Deny + per-fee enumeration) because protocol/provider fees accrue inside dlmm-core's unclaimed-protocol-fees map and bin balances — they do NOT emit FT transfer events on the swap tx (verified on-chain against mainnet swap txs 0x134df5e1… and 0x5195822e…). A receive-side user pin is orthogonal to the fee-flow surface; no fee enumeration needed. Purely additive: when the router's own ERR_MINIMUM_RECEIVED assert succeeds (actual output >= minOut), the new post-condition trivially passes. Adds wallet-layer protection only against the case where the router transfers less than min-received, which the router's own assert already prohibits — defense in depth against any future router bug. Closes Diego review item 2. Items 1, 3, 4, 5 remained closed per ec34613, deff816, 1904432. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hodlmm-inventory-balancer): correct receive-side post-condition shape (Pc.principal pool willSendGte) Initial commit 9742977 used Pc.principal(senderAddress).willReceiveGte(minOut) following @macbotmini-eng's suggested diff verbatim, but the @stacks/transactions Pc builder doesn't expose a willReceiveGte primitive. Stacks post-conditions evaluate against the principal that is SENDING the asset — a "user receives ≥ X" invariant is expressed as "pool sends ≥ X" anchored on the pool principal. The corrected envelope mirrors the author's mainnet refs exactly: - PC[0] sender willSendLte amountIn (input cap on caller's wallet) - PC[1] pool willSendGte minOut (output floor on pool reserves) Caught at proof-cycle execution time when bun threw 'Pc.principal(senderAddress).willReceiveGte is not a function' on the live broadcast attempt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(hodlmm-inventory-balancer): SKILL.md + AGENT.md reflect dual-PC envelope The post-condition section in both docs still described the v1 Allow + sender-pin only architecture — stale after commits 9742977 + 02d1098 landed the dual-pin envelope (sender willSendLte on input + pool willSendGte on output) per @macbotmini-eng's #494 audit and the live proof tx 0xf4f49328. Updated both descriptions to: - Name both pins explicitly with their Pc-builder shape - State that wallet layer + router layer enforce the same min-out invariant - Drop the stale "Deny would require per-fee enumeration" rationale and replace with the verified-on-chain reason (fees accrue in unclaimed-protocol-fees map + bin balances, no FT events on swap tx) - Cite the live proof tx hash for SKILL.md No code change in this commit; documentation only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hodlmm-inventory-balancer): convert API bin_id to signed before passing to router Critical Leg 3 bug found in pre-reviewer-re-ping audit. The `--allow-rebalance-withdraw` 3-leg flow set `active_bin_expected = activeBin`, where `activeBin` was sourced from `fetchPoolBins` returning the Bitflow API's unsigned `active_bin_id` (offset by +CENTER_BIN_ID = 500). The `dlmm-liquidity-router-v-1-1.add-relative-liquidity-same-multi` `expected-bin-id` field is a signed Clarity int that must match the contract's on-chain `(get-active-bin-id)` (raw signed value). Failure mode: every Leg 3 redeposit reverts with `(err u5008)` ERR_ACTIVE_BIN_TOLERANCE because the delta against the contract's signed value is always exactly 500, blowing the `max-deviation: 2` check regardless of how generous the tolerance is set. Why proof tx 0xf4f4932800a80234845a8d199556ad9c0ff4aa99874a95c819c13779b164cbc8 didn't catch this: that proof exercised the 2-leg path (swap + CLI), which returns from `recommendOrRun` before constructing the `redeposit` plan. The 3-leg `--allow-rebalance-withdraw` flow has not yet executed on-chain. Same coordinate-space gap caught and fixed in `hodlmm-move-liquidity` skill at the 2026-04-22 proof cycle (deposit attempt failed `(err u5008)` despite reading active_bin from API correctly). Fix: subtract `CENTER_BIN_ID` from `activeBin` before stashing in the plan. The 2-leg path (recommend-only or guardian-rebalance without withdraw) is unaffected — returns at line 958 before constructing `redeposit`. 3 lines changed (1 logic + 5-line comment). Bun bundles 155 modules clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hodlmm-inventory-balancer): pass noneCV() for active-bin-tolerance per KB bug #40 + on-chain proof Reverts c7e163d. The previous fix (subtracting CENTER_BIN_ID from active_bin_expected) addressed an unsigned-vs-signed coordinate-space gap that's real in principle but secondary here — the contract call passes the toleranceTuple wrapped in an option, and the proven-good Leg 3 path uses `noneCV()` so the bin-id field is never enforced. Truth source: on-chain Leg 3 proof tx 0x135f490ca3f7b2862c3bd2eb33124bcd99e9ce2d93331865ad1dfd2065d6f53c (mainnet block 7641905, 2026-04-18T03:16:27Z, dlmm_1, 0%X→100%X→49.95%X / 50.05%Y rebalance cycle) shows `active-bin-tolerance: none`. The deff816 commit shipped `someCV(toleranceTuple)` — a regression vs the noneCV intent documented in `bff-skills/docs/knowledge-base.md` bug #40. Why noneCV (not signed-fixed someCV) is the correct shape: mid-cycle the active bin can drift between Leg 2 (our swap, which moves the pool) and Leg 3 (redeposit, which reads it). A `someCV({expected-bin-id, max-deviation})` tolerance check spuriously aborts with `(err u5008) ERR_ACTIVE_BIN_TOLERANCE` even when the redeposit math is correct. Widening max-deviation doesn't help — on high-volume pools the bin can move arbitrarily far in 30-60 seconds. noneCV avoids the race entirely; fund safety on the redeposit is preserved by the wallet-side max-x-liquidity-fee + min-dlp + per-bin offset bounds (already enforced). Plan field `active_bin_expected` is kept for `recommend` mode output visibility (operator can see the intended bin) but is not passed into the contract call. Imports updated: someCV → noneCV. toleranceTuple construction removed (was the only consumer). Bun bundles 155 modules clean. Audited via pr-review-toolkit code-reviewer (REVIEW_OK). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(hodlmm-inventory-balancer): scrub internal-fork KB references from public comments The previous commit (0313943) cited "KB bug #40 / bff-skills/docs/knowledge-base.md" in two inline comments. That KB lives in our cliqueengagements/bff-skills fork only — not upstream BitflowFinance/bff-skills — so reviewers cannot resolve the reference. Replaced with self-contained inline rationale: - Mid-cycle race description (active bin can drift between Leg 2 swap and Leg 3 redeposit; hard tolerance check spuriously aborts with err u5008) - Direct on-chain proof tx hashes (Leg 1 withdraw 0x89315a8b…, Leg 2 swap 0x5195822e…, Leg 3 redeposit 0x135f490c…) — all public on Hiro Explorer - Plain enumeration of the wallet-side bounds that preserve fund safety on the redeposit (per-bin max-x/y-liquidity-fee at 5%, min-dlp >= 1, x/y-amount caps) Pure comment cleanup — zero logic change, zero contract-call change. Bun bundles clean. The behavior shipped in 0313943 (`noneCV()` for active-bin-tolerance) is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: untrack docs/knowledge-base.md (internal-only, never should have been in branch) Removes the internal KB notebook from this branch. The file was inadvertently committed in ccd905b (2026-04-22) and has been visible in PR #494's public diff since. It is internal operational documentation (bug history, win-logic playbook, audit checklist, pattern catalog) intended only for the local working tree. Adds docs/knowledge-base.md to .gitignore so it cannot be re-added accidentally. Local working copy is preserved untracked. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: cliqueengagements <cliqueengagements@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Four fixes for
hodlmm-move-liquiditysurfaced during a Day 24 live full-cycle proof against mainnetdlmm_1. Every issue caused a real on-chain rejection during the run. Context follows each fix.fetchPoolsreturned empty list against migrated API;fetchUserPositionsreturned 0 positions for a wallet holding 99.8 DLPmove-liquidity-multi(list cap 220)move-relative-liquidity-multi(cap 208) rejected a 220-bin position withBadFunctionArgumentat Clarity parse — pre-execution, no fee, no eventsmin-dlp = 1nfor cross-bin rebalancemin-dlp = 95% of inputsilently rejects any cross-bin move — destination-bin DLP is price-indexed, not share-indexed. Broadcast landed, contract aborted with (err u5001)fee: 50000n→250000nFeeTooLowpre-inclusionProof this works
Day 24 BFF-skills PR #494 (hodlmm-inventory-balancer) composes
hodlmm-move-liquidityvia CLI. Full-cycle proof ran with these fixes applied locally:cd71c8a5…— 3,147,087 USDCx → 4,195 sats sBTC, success0349cbb0…—(ok (list u257199 u258963 …))successLP ratio 14.58 % X / 85.42 % Y → 27.05 % X / 72.95 % Y. Movement of 12.47 pp toward 50:50 target.
Fix 1 — schema migration (regression from d83755a)
Commit
d83755aremoved camelCase fallbacks per a Day-14 review suggestion with the comment: "Bitflow App API uses snake_case fields. No camelCase fallbacks — fail loudly on schema change." The migration happened in early April 2026. The skill failed loudly, exactly as the comment promised. The camelCase fallbacks I removed would have absorbed the migration transparently.Restored the fallbacks with an added
tokens.tokenX.contractpath for the new nested token structure:fetchUserPositionsgets the same dual-read treatment foruserLiquidity,reserveX,reserveY,binId.Lesson: never remove format fallbacks on external vendor APIs. Fail-loudly is desirable only if you're actively monitoring; a shipped skill isn't.
Fix 2 — function selection (208 vs 220 cap)
Router contract exposes both
move-relative-liquidity-multi(list cap 208) andmove-liquidity-multi(list cap 220). Both perform atomic withdraw + deposit; the difference is destination encoding:active-bin-id-offset(signed offset from active bin)to-bin-id(signed, bin_id - CENTER_BIN_ID)Real HODLMM positions on dlmm_1 carry 209–221 user bins after multiple prior rebalances. 220 > 208 →
BadFunctionArgumentat Clarity parse, pre-execution.Switched to
move-liquidity-multiwithto-bin-id = (activeBin - CENTER_BIN_ID) + activeBinOffset. Semantically equivalent, just absolute encoding.Fix 3 — min-dlp cross-bin
Source DLP at bin A represents tokens valued at
price_A. Destination DLP at bin B represents tokens atprice_B. MovingamountDLP from A to B yields roughlyamount × (price_A / price_B)destination DLP. For bin 460 → bin 617 (~10× price delta in our pool), destination DLP is ~10 % of input, not ≥ 95 %.Router's own arithmetic via pool contracts enforces value conservation regardless of
min-dlp. Settingmin-dlp = 1ndoesn't open a slippage attack — it just lets cross-bin moves through when the legitimate conversion produces fewer destination shares.Proper long-term fix is price-aware:
min-dlp = 95n × (price_from / price_to) × amount / 100n. This PR ships the simplermin-dlp = 1nwith documentation of why; happy to iterate if reviewers want the price-aware version in the same PR.Fix 4 — fee floor
Mempool rejected all broadcasts with hardcoded
fee: 50000n(FeeTooLow). Live mempool floor as of Apr 2026 is higher. Bumped to250000n. Dynamic estimation viaget_stx_feesis a worthwhile follow-up but out of scope here.Scope
Single file:
hodlmm-move-liquidity/hodlmm-move-liquidity.ts. No SKILL.md / AGENT.md changes required — the user-facing contract is unchanged; all fixes are internal.Test plan
scripts/validate-frontmatter.ts(should pass — no frontmatter touched)doctoragainst live API: should returnokwith pool list populated (proves fix 1)scan --wallet <addr>: should return positions for a wallet with > 0 DLP (proves fix 1 user-side)run --wallet <addr> --pool dlmm_1 --confirm --force --password <pass>: should broadcast and confirm on-chain for a 209+ bin position (proves 2, 3, 4 together)Live proof already run on my wallet against dlmm_1 — tx hashes above.
Follow-ups for a separate PR
last_move_atbefore tx confirmation, so a reverted broadcast still gates the next cycle for 4h. Should wait fortx_status === success.min-dlpcomputation (vs. the= 1nsimplification here).get_stx_fees.