Skip to content

feat(sbtc-yield-maximizer): complete HODLMM routing leg#340

Open
Ololadestephen wants to merge 2 commits intoaibtcdev:mainfrom
Ololadestephen:codex/sbtc-yield-maximizer-hodlmm-leg
Open

feat(sbtc-yield-maximizer): complete HODLMM routing leg#340
Ololadestephen wants to merge 2 commits intoaibtcdev:mainfrom
Ololadestephen:codex/sbtc-yield-maximizer-hodlmm-leg

Conversation

@Ololadestephen
Copy link
Copy Markdown

Summary

Completes the HODLMM routing leg for sbtc-yield-maximizer.

The live upstream version could compare Zest vs HODLMM, but still stopped at the HODLMM path with executable: false. This update makes that branch real: when HODLMM is the highest safe executable route and the wallet has an out-of-range LP position, the skill now executes the HODLMM rebalance through hodlmm-move-liquidity via CLI.

What Changed

  • added executable HODLMM route orchestration via hodlmm-move-liquidity
  • added matched-freshness / stale-data gating for Zest and HODLMM reads
  • added --min-apy-diff-bps
  • added router-level cooldown
  • added mempool-depth guard
  • added wallet-scoped state
  • added explicit no-broadcast HODLMM preflight via hodlmm-move-liquidity scan
  • removed subprocess password exposure
  • hardened execution lock handling
  • updated HODLMM candidate evaluation so qualifying pools are not prematurely excluded by liquidity-first slicing

Why

This closes the main capability gap in the current upstream skill:

  • HODLMM could win in the decision function
  • but the skill could not actually execute the HODLMM leg

After this change, sbtc-yield-maximizer becomes a true two-route write skill for existing sBTC capital:

  • Zest supply when Zest wins
  • HODLMM rebalance when HODLMM wins and the position is out of range

Composition

This keeps the skill aligned with the composition rules:

  • no bundled skill directories
  • no cross-skill source imports
  • HODLMM execution delegated via CLI to hodlmm-move-liquidity
  • the owning skill remains sbtc-yield-maximizer

Mainnet Proof

HODLMM-leg proof tx from the reviewed implementation:

  • Tx: 0xbed1b7131689f5e45095a53ba8bc47f34f53b67e5b494698fe72c075566634ec
  • Sender: SP3FY9BQ6NZ85XT9PBXQVKP47WJAEMZZ7K6KF7TSS
  • Contract: SM1FKXGNZJWSTWDWXQZJNF7B5TV5ZB235JTCXYXKD.dlmm-liquidity-router-v-1-1
  • Function: move-relative-liquidity-multi

Hiro verification:

curl -s https://api.hiro.so/extended/v1/tx/0xbed1b7131689f5e45095a53ba8bc47f34f53b67e5b494698fe72c075566634ec | jq '{status:.tx_status, sender:.sender_address, contract:.contract_call.contract_id, function:.contract_call.function_name}'
{
  "status": "success",
  "sender": "SP3FY9BQ6NZ85XT9PBXQVKP47WJAEMZZ7K6KF7TSS",
  "contract": "SM1FKXGNZJWSTWDWXQZJNF7B5TV5ZB235JTCXYXKD.dlmm-liquidity-router-v-1-1",
  "function": "move-relative-liquidity-multi"
}

Scope Notes

This PR is specifically the HODLMM-leg completion for sbtc-yield-maximizer.

It is not claiming full DCA + HODLMM + Zest completion. STX -> sBTC normalization / DCA remains out of scope for this version.

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.

Completes the HODLMM routing leg in sbtc-yield-maximizer — turns what was a read-only decision path (executable: false) into a real two-route write skill that delegates to hodlmm-move-liquidity via CLI. The security lift (password passed via env/wrapper rather than CLI args), the wallet-scoped state files, the mempool-depth guard, and the matched-freshness gating are all solid additions. Mainnet proof tx is a welcome inclusion.

What works well:

  • Composition model is correct: HODLMM execution fully delegated via CLI, no cross-skill source imports
  • acquireExecutionLock with openSync(lockFile, "wx") is atomically correct — the exclusive-create flag is the right tool here
  • The wrapper script approach to avoid password in OS-level cmdline is clever — argv injection happens in-process, so /proc/<pid>/cmdline only shows bun <wrapper>, not the password
  • preflightHodlmmMove as a no-broadcast scan before commit is exactly the right safety model

[blocking] HODLMM execution path skips the safe gate (sbtc-yield-maximizer.ts, decideRoute)

The new HODLMM winning branch checks apyDiffBps and moveHealth.inRange, but does not check topHodlmm.safe. A pool that fails volume, TVL, or price-divergence gates (i.e. reasons.length > 0, safe: false) can still reach executable: true if it leads Zest on yield and its position is out-of-range.

The no-position case is the sharpest edge: when !moveHealth.hasPosition, safe is false (the reason "no active LP position..." is added), but moveHealth.inRange is also false. The if (topHodlmm.moveHealth.inRange) guard does not fire, so the code falls through and returns { route: "deploy-to-hodlmm", executable: true }. Operators checking status would see executable: true and invoke run --confirm=MAXIMIZE — which would then fail at preflightHodlmmMove because the scan finds no position for the pool. The preflight catches it, but the status output is misleading and the error surface is wrong.

Same issue applies to pools that pass volume/TVL filters but fail the divergence gate — they end up in hodlmmCandidates with safe: false, and decideRoute never checks it.

This mirrors how the old code handled the Zest path: log the safety gate failure in rationale, return hold.


[suggestion] Fallback pool logic can surface unsafe pools to the HODLMM executable path

In fetchHodlmmCandidates, when no pool passes the volume/TVL thresholds, the fallback selects the top-3 by combined TVL+volume — explicitly including pools that failed the filter. These fallback candidates will have safe: false (volume/TVL reasons are already populated at that point). Once the blocking issue above is fixed and the safe gate is added to decideRoute, this path becomes harmless — they'll be rejected correctly. But it's worth documenting that the fallback is intentionally a status-only signal (shows what pools exist even when none qualify) rather than an execution candidate pool.


[question] State file migration on upgrade

The state file moved from ~/.sbtc-yield-maximizer-state.json (flat, shared across wallets) to ~/.aibtc/sbtc-yield-maximizer-${walletId}.json (scoped per wallet). Any existing cooldown from a previous run is silently dropped after upgrade — the new readState call won't find the old file and returns {}, making the cooldown check pass immediately. Is this intentional (fresh-start on migration), or should there be a one-time migration note in the docs?


[question] ageSeconds staleness check and FETCH_TIMEOUT_MS overlap

Both Zest and HODLMM set fetchedAt immediately before their awaits, then compute ageSeconds after. For live fetches, ageSeconds measures fetch duration — so the ageSeconds > maxDataAgeSeconds (default 30s) check would only fire if a fetch took longer than 30s, which is exactly when FETCH_TIMEOUT_MS would have already thrown. Is the intent for maxDataAgeSeconds to guard against stale data passed in from a future caching layer, or is there a race this covers that I'm missing?


[nit] The removed comment about Zest BPS math (// Verified against the live v0-vault-sbtc source...) documented why get-interest-rate is treated as basis points — non-obvious from the function name alone. Worth keeping or moving to SKILL.md rather than dropping entirely.

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.

Completes the HODLMM routing leg in sbtc-yield-maximizer — turns what was a read-only decision path (executable: false) into a real two-route write skill that delegates to hodlmm-move-liquidity via CLI. The security lift (password passed via env/wrapper rather than CLI args), the wallet-scoped state files, the mempool-depth guard, and the matched-freshness gating are all solid additions. Mainnet proof tx is a welcome inclusion.

What works well:

  • Composition model is correct: HODLMM execution fully delegated via CLI, no cross-skill source imports
  • acquireExecutionLock with openSync(lockFile, "wx") is atomically correct — the exclusive-create flag is the right tool here
  • The wrapper script approach to avoid password in OS-level cmdline is clever — argv injection happens in-process, so /proc/<pid>/cmdline only shows bun <wrapper>, not the password
  • preflightHodlmmMove as a no-broadcast scan before commit is exactly the right safety model

[blocking] HODLMM execution path skips the safe gate (sbtc-yield-maximizer.ts, decideRoute)

The new HODLMM winning branch checks apyDiffBps and moveHealth.inRange, but does not check topHodlmm.safe. A pool that fails volume, TVL, or price-divergence gates (i.e. reasons.length > 0, safe: false) can still reach executable: true if it leads Zest on yield and its position is out-of-range.

The no-position case is the sharpest edge: when !moveHealth.hasPosition, safe is false (the reason "no active LP position..." is added), but moveHealth.inRange is also false. The if (topHodlmm.moveHealth.inRange) guard does not fire, so the code falls through and returns { route: "deploy-to-hodlmm", executable: true }. Operators checking status would see executable: true and invoke run --confirm=MAXIMIZE — which would then fail at preflightHodlmmMove because the scan finds no position for the pool. The preflight catches it, but the status output is misleading and the error surface is wrong.

Same issue applies to pools that pass volume/TVL filters but fail the divergence gate — they end up in hodlmmCandidates with safe: false, and decideRoute never checks it.

The fix is to add a !topHodlmm.safe guard before the moveHealth.inRange check:

if (!topHodlmm.safe) {
  rationale.push(`HODLMM pool ${topHodlmm.poolId} failed safety gates: ${topHodlmm.reasons.join("; ")}`);
  return { route: "hold", deploySats, rationale, zest, topHodlmm, executable: false, winnerApyBps: hodlmmBps, apyDiffBps };
}

This mirrors how the Zest path handles failed safety gates.


[suggestion] Fallback pool logic can surface unsafe pools to the HODLMM executable path

In fetchHodlmmCandidates, when no pool passes the volume/TVL thresholds, the fallback selects the top-3 by combined TVL+volume — explicitly including pools that failed the filter. These fallback candidates will have safe: false (volume/TVL reasons are already populated). Once the blocking issue above is fixed with the safe gate in decideRoute, this path becomes harmless — they'll be rejected correctly. But it's worth documenting that the fallback is intentionally a status-only signal (surfaces pool data even when none qualify) rather than an execution candidate pool.


[question] State file migration on upgrade

The state file moved from ~/.sbtc-yield-maximizer-state.json (flat, shared) to ~/.aibtc/sbtc-yield-maximizer-${walletId}.json (scoped per wallet). Any existing cooldown from a previous run is silently dropped after upgrade — readState won't find the old file and returns {}, making the cooldown check pass immediately. Is this intentional (fresh-start on migration), or should there be a migration note in the docs?


[question] ageSeconds staleness check and FETCH_TIMEOUT_MS overlap

Both Zest and HODLMM set fetchedAt immediately before their awaits, then compute ageSeconds after. For live fetches, ageSeconds measures fetch duration — so the ageSeconds > maxDataAgeSeconds (default 30s) check would only fire if a fetch took longer than 30s, which is exactly when FETCH_TIMEOUT_MS would have already thrown. Is the intent for maxDataAgeSeconds to guard against stale data passed in from a future caching layer, or is there a race this covers that I'm missing?


[nit] The removed comment about Zest BPS math (// Verified against the live v0-vault-sbtc source...) documented why get-interest-rate is treated as basis points — non-obvious from the function name alone. Worth keeping or moving to SKILL.md rather than dropping entirely.

@Ololadestephen
Copy link
Copy Markdown
Author

@arc0btc Addressed the blocking issue and followed up on the review notes.

Updates in the reviewed files:

  • enforced the HODLMM safe gate before returning deploy-to-hodlmm, so unsafe candidates now correctly fall back to hold with the safety failure reflected in rationale
  • documented the fallback pool path as status-oriented unless the candidate also passes the downstream safety gate
  • added a note that the wallet-scoped state file intentionally starts fresh rather than silently migrating the previous shared cooldown file
  • added a clarifying comment that ageSeconds is mainly future-proofing for cached/reused reads
  • restored the Zest BPS comment

That should close the misleading executable: true case on unsafe HODLMM candidates while also making the surrounding behavior more explicit.

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.

2 participants