Skip to content

feat: add hodlmm-range-keeper (BFF Skills Comp Day 12 winner by @tearful-saw)#307

Closed
diegomey wants to merge 1 commit into
aibtcdev:mainfrom
diegomey:bff-comp/hodlmm-range-keeper
Closed

feat: add hodlmm-range-keeper (BFF Skills Comp Day 12 winner by @tearful-saw)#307
diegomey wants to merge 1 commit into
aibtcdev:mainfrom
diegomey:bff-comp/hodlmm-range-keeper

Conversation

@diegomey
Copy link
Copy Markdown
Contributor

@diegomey diegomey commented Apr 7, 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.

Frontmatter has been converted to the aibtcdev/skills metadata: convention. Command paths updated to match this repo root-level skill layout.

Files

  • hodlmm-range-keeper/SKILL.md — Skill definition with AIBTC-format frontmatter
  • hodlmm-range-keeper/AGENT.md — Agent behavior rules and guardrails
  • hodlmm-range-keeper/hodlmm-range-keeper.ts — TypeScript implementation

Attribution

Original author: @tearful-saw. The metadata.author field in SKILL.md preserves their attribution permanently.


Automated by BFF Skills Bot on merge of PR #163.

Submitted by @tearful-saw (Elegant Orb) via the AIBTC x Bitflow Skills Pay the Bills competition.

Competition PR: BitflowFinance/bff-skills#163
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.

Adds hodlmm-range-keeper — a full-loop HODLMM position manager that monitors active-bin drift, estimates fee accrual, and re-centers LP positions when needed. The design is solid: --confirm gate on all write ops, 30-minute cooldown, gas cap, dust filter, dry-run mode. Good competition win.

What works well:

  • Safety-first design throughout — recenter requires explicit --confirm, cooldown is enforced at two levels (health analysis + command layer), --force does NOT bypass cooldown (correct per AGENT.md guardrails)
  • Consistent JSON-to-stdout / logs-to-stderr output contract; Commander help/error redirected to stderr so MCP callers get clean JSON
  • BigInt arithmetic for token amounts avoids precision loss on large positions
  • State history capped at 500 events — good housekeeping
  • Clear partial-failure semantics: funds stay in wallet if deposit fails after a successful withdraw; documented in both SKILL.md and AGENT.md

[suggestion] reserve_x = 0 falsy fallback in fetchUserPositions (hodlmm-range-keeper.ts:360)
The current code:

reserve_x: String(b.reserve_x || Math.floor(b.userLiquidity || 0)),

When b.reserve_x === 0 (a token-Y-heavy bin, legitimately zero on the X side), this falls back to userLiquidity — mixing liquidity units with token amounts. Position center and fee estimation downstream would silently use wrong values.

reserve_x: String(b.reserve_x !== undefined && b.reserve_x !== null ? b.reserve_x : Math.floor(b.userLiquidity || 0)),

[suggestion] STX balance not decremented between recenters in run (hodlmm-range-keeper.ts:1065, 1115)
stxBalance is fetched once at the top of run and reused across all pool iterations. If MAX_RECENTER_PER_CYCLE = 3 and each costs ~4 STX, a wallet with 5 STX would pass the gas check on iteration 2 despite having ~1 STX remaining after iteration 1. MAX_RECENTER_PER_CYCLE is the primary safety cap here, but the gas check gives a false sense of security. Consider tracking estimatedGasUsed and subtracting from stxBalance each iteration.

[suggestion] Sequential pool iteration in status and run vs parallel in doctor (hodlmm-range-keeper.ts:769, 1070)
doctor correctly uses Promise.allSettled for parallel position scanning, but status and run use sequential for...of with await. For users with positions across many pools this adds noticeable latency. status could parallelize fetchUserPositions calls the same way doctor does.

[nit] Dead code: fetchPoolAppStats (hodlmm-range-keeper.ts:368)
This function is defined but never called anywhere. If it's not needed for the current implementation, removing it keeps the surface area clean.

[nit] Unused local variables in planRecenter (hodlmm-range-keeper.ts:609-612)
binIdsWithdraw and amountsWithdraw are computed but never used — they don't appear in the return value or MCP command strings. Dead assignments.

Code quality notes:
The run command duplicates ~40 lines of logic from recenter (baseline recording, event creation, pool state update, MCP command output). Extracting an internal executeRecenter(poolId, plan, poolState, state) helper would eliminate the duplication and make both commands easier to audit. Not blocking — the duplication is straightforward to follow — but worth cleaning up if the skill evolves.

Operational context:
We use Bitflow LP positions on Stacks and monitor the HODLMM API. The API response shapes look correct based on what we've seen (bff.bitflowapis.finance endpoints are consistent). The 15-minute cron cadence in AGENT.md is aggressive but appropriate for concentrated liquidity — bin drift can happen quickly in volatile markets. The 30-minute cooldown prevents cascading gas spend if price is choppy.

The reserve_x fallback is the only issue I'd want fixed before this gets widely used — it's subtle and would silently corrupt fee estimates on positions where one token side is zero, which is the normal state for bins above or below the active bin.

@tearful-saw
Copy link
Copy Markdown
Contributor

All 5 review items from @arc0btc have been addressed in our source repo:

Commit: tearful-saw/bff-skills@d326843

Fixes applied:

  1. reserve_x = 0 falsy fallback — switched from || to !== undefined && !== null check so legitimate zero values don't fall through to userLiquidity
  2. STX balance not decremented between recenters — added remainingStxBalance tracking, decremented by DEFAULT_GAS_ESTIMATE_STX after each execution
  3. Sequential pool iteration in status — parallelized fetchUserPositions calls with Promise.allSettled (matching doctor)
  4. Dead code fetchPoolAppStats — removed
  5. Unused binIdsWithdraw / amountsWithdraw — removed

Happy to submit a follow-up PR with the updated file, or @diegomey can cherry-pick the commit above into this branch.

@diegomey
Copy link
Copy Markdown
Contributor Author

diegomey commented Apr 7, 2026

o submit a follow-up PR with the updated file

Feel free to submit final version here. We'll be using AIBTC merged skills as final source of truth.

@whoabuddy
Copy link
Copy Markdown
Contributor

Closing in favor of #308, which includes arc0btc review fixes and is based on main. Thank you @tearful-saw for the submission and the cleanup!

@TheBigMacBTC
Copy link
Copy Markdown

TheBigMacBTC commented Apr 16, 2026

@tearful-saw — This PR was closed in favor of #308, which includes all arc0btc review fixes and was merged on 2026-04-08. Payment confirmation for Day 12 has been posted there.

cc @diegomey @TheBigMacBTC @BFFARMY-Agent @arc0btc

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