feat(sbtc-yield-maximizer): complete HODLMM routing leg#340
feat(sbtc-yield-maximizer): complete HODLMM routing leg#340Ololadestephen wants to merge 2 commits intoaibtcdev:mainfrom
Conversation
arc0btc
left a comment
There was a problem hiding this comment.
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
acquireExecutionLockwithopenSync(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>/cmdlineonly showsbun <wrapper>, not the password preflightHodlmmMoveas 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.
arc0btc
left a comment
There was a problem hiding this comment.
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
acquireExecutionLockwithopenSync(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>/cmdlineonly showsbun <wrapper>, not the password preflightHodlmmMoveas 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.
|
@arc0btc Addressed the blocking issue and followed up on the review notes. Updates in the reviewed files:
That should close the misleading |
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 throughhodlmm-move-liquidityvia CLI.What Changed
hodlmm-move-liquidity--min-apy-diff-bpshodlmm-move-liquidity scanWhy
This closes the main capability gap in the current upstream skill:
After this change,
sbtc-yield-maximizerbecomes a true two-route write skill for existing sBTC capital:Composition
This keeps the skill aligned with the composition rules:
hodlmm-move-liquiditysbtc-yield-maximizerMainnet Proof
HODLMM-leg proof tx from the reviewed implementation:
0xbed1b7131689f5e45095a53ba8bc47f34f53b67e5b494698fe72c075566634ecSP3FY9BQ6NZ85XT9PBXQVKP47WJAEMZZ7K6KF7TSSSM1FKXGNZJWSTWDWXQZJNF7B5TV5ZB235JTCXYXKD.dlmm-liquidity-router-v-1-1move-relative-liquidity-multiHiro verification:
{ "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 + Zestcompletion. STX -> sBTC normalization / DCA remains out of scope for this version.