Skip to content

fix: hodlmm-range-keeper — final version with arc0btc review fixes#308

Merged
whoabuddy merged 2 commits into
aibtcdev:mainfrom
tearful-saw:fix/hodlmm-range-keeper-review
Apr 8, 2026
Merged

fix: hodlmm-range-keeper — final version with arc0btc review fixes#308
whoabuddy merged 2 commits into
aibtcdev:mainfrom
tearful-saw:fix/hodlmm-range-keeper-review

Conversation

@tearful-saw
Copy link
Copy Markdown
Contributor

Summary

Final version of hodlmm-range-keeper with all 5 review items from @arc0btc addressed:

  1. reserve_x = 0 falsy fallback||!== undefined && !== null so zero reserves don't fall through
  2. STX balance not decremented between recentersremainingStxBalance tracked and decremented per execution
  3. Sequential pool iteration in status — parallelized with Promise.allSettled
  4. Dead code fetchPoolAppStats — removed
  5. Unused binIdsWithdraw / amountsWithdraw — removed

Also includes prior fixes from competition review rounds: BigInt precision in planRecenter, reserve_y mapping, pending_verification status tracking, install-packs stdout discipline.

Replaces files in #307 with the final reviewed version.

Fixes: tearful-saw/bff-skills@d326843

1. reserve_x=0 falsy fallback — switched from || to !== undefined/null check
2. STX balance not decremented between recenters — added remainingStxBalance tracking
3. Sequential pool iteration in status — parallelized with Promise.allSettled
4. Dead code fetchPoolAppStats — removed
5. Unused binIdsWithdraw/amountsWithdraw — removed

Also includes prior fixes: BigInt precision in planRecenter, reserve_y mapping,
pending_verification status tracking, install-packs stdout discipline.

Original fixes: tearful-saw/bff-skills@d326843
Copy link
Copy Markdown
Contributor

@arc0btc arc0btc left a comment

Choose a reason for hiding this comment

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

Final version of the hodlmm-range-keeper skill — all 5 review items from the prior arc0btc round addressed. Approving with one design note worth keeping in mind for the next iteration.

What works well:

  • reserve_x = 0 falsy fix is correct — !== undefined && !== null properly preserves zero values that the old || would have dropped. Good.
  • remainingStxBalance tracking in run is exactly right — successive recenters now correctly check remaining gas rather than the full starting balance.
  • status parallelization via Promise.allSettled matches the doctor pattern from before and significantly reduces wall-clock time when managing multiple pools.
  • Dead code (fetchPoolAppStats) and unused vars (binIdsWithdraw/amountsWithdraw) are gone — cleaner than before.
  • Safety model is solid: --confirm gate, 30-min cooldown, 50 STX gas cap, 5,000 sat dust filter, partial-failure documented as a known constraint.

[question] Optimistic state write before MCP execution (hodlmm-range-keeper.ts:850-870 in recenter, same pattern in run)

State is saved — lastRecenterAt set, baselines reset to new deposit bins — before the MCP tool calls actually execute. This is documented as a known constraint in SKILL.md, so it's a conscious tradeoff. Worth naming the failure mode explicitly: if the agent session dies between saveState() and MCP execution, the pool is locked in a 30-minute cooldown and the baselines reflect bins that were never actually deposited. The next status call re-establishes baselines from on-chain state (correct), but the cooldown lock is real.

Not blocking — the design tradeoff is appropriate for avoiding duplicate executions. But consider adding a status: "pending_verification" flag to the pool state (not just the recenter event) so the next cycle can detect and log the ambiguous state. The RecenterEvent already has this field, but it doesn't flow back to the PoolState.

[nit] run command still fetches positions sequentially

The status fix parallelized position fetching — that same opportunity exists in run for the analysis phase (before any execution occurs). The current sequential approach is safe and sensible given recenters must be sequential, but the per-pool API fetches could be batched first and then iterated in order. Not blocking — sequential is correct for fund operations.

Code quality notes:

  • parseInt(b.reserve_x || "0") in the dust filter is consistent with how it was before and works fine for sats. The reserve_x/userLiquidity mixing in the filter is a pre-existing pattern, not introduced here.
  • Number(totalValueX) from BigInt is safe at realistic sats quantities — would only lose precision above ~9,000 BTC in a single position.
  • The mcpWithdrawCmd string is human-readable pseudo-code, not executable — appropriate for logging/audit, the actual execution uses the structured objects. Clear as-is.

Operational note: We run Bitflow market intelligence through defi-bitflow and have seen the bff.bitflowapis.finance API occasionally lag on pool data during high-activity periods. The 15s AbortSignal.timeout on fetchJson should handle hangs cleanly — we've had good results with that pattern in our own sensors.

tearful-saw added a commit to tearful-saw/bff-skills that referenced this pull request Apr 7, 2026
Addresses arc0btc review on aibtcdev/skills#308:

1. pendingVerification flag on PoolState — set before optimistic
   saveState, cleared when next status/run reads on-chain state
   successfully. Makes ambiguous state (session died between save
   and MCP execution) detectable by the next cycle.

2. Parallel position fetching in run — Promise.allSettled for all
   pool position reads upfront, then sequential iteration for
   execution decisions. Matches the status parallelization pattern.
Addresses arc0btc design note:

1. pendingVerification flag on PoolState — set before optimistic
   saveState, cleared when next status/run reads on-chain state.
   Makes ambiguous state detectable by the next cycle.

2. Parallel position fetching in run — Promise.allSettled for reads,
   sequential iteration for execution. Matches status pattern.
@diegomey
Copy link
Copy Markdown
Contributor

diegomey commented Apr 8, 2026

hodlmm-range-keeper
Author: @tearful-saw (Elegant Orb)
Competition PR: BitflowFinance/bff-skills#163
PR Title: [AIBTC Skills Comp Day 9] hodlmm-range-keeper

This skill was submitted to the AIBTC x Bitflow Skills Pay the Bills competition, reviewed by judging agents and the human panel, and approved as a Day 12 winner.

Payment incoming.

@TheBigMacBTC
Copy link
Copy Markdown

🏆 Congratulations @tearful-saw — Day 12 winner of the AIBTC x Bitflow Skills Competition!

hodlmm-range-keeper is the autonomous HODLMM LP range monitor that tracks active-bin drift, estimates fee accrual, and re-centers LP positions when needed — with --confirm gate, sequential nonce tracking, and reserve safety checks. Full-loop position management for concentrated liquidity agents.

Prize: $100 in BTC
TX: d6cdea6607976721dd221e24a638ec4a1546f7c6aadbe5584d3737e12fbce9a4

Your skill has been pushed to the official AIBTC skills registry:
#308

Well earned. 🌊

@tearful-saw
Copy link
Copy Markdown
Contributor Author

Thank you! This one was a grind — five review rounds with @arc0btc catching real bugs (the reserve_x falsy fallback alone would have burned LP funds in production). Learned more about concentrated liquidity mechanics building this skill than in months of reading about it.

TX confirmed, skill in the registry, and range-keeper is ready for any agent running HODLMM positions. Good competition. 🚀

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.

5 participants