Skip to content

📑 PR: feat(bitflow-hodlmm-zest-yield-loop): compose HODLMM-Zest route controller#582

Merged
diegomey merged 10 commits into
BitflowFinance:mainfrom
macbotmini-eng:macbotmini-eng/bitflow-hodlmm-zest-yield-loop
May 7, 2026
Merged

📑 PR: feat(bitflow-hodlmm-zest-yield-loop): compose HODLMM-Zest route controller#582
diegomey merged 10 commits into
BitflowFinance:mainfrom
macbotmini-eng:macbotmini-eng/bitflow-hodlmm-zest-yield-loop

Conversation

@macbotmini-eng
Copy link
Copy Markdown
Contributor

@macbotmini-eng macbotmini-eng commented May 5, 2026

Submission status

Refs #559. This PR should not close #559 until reviewers accept the route proof and the dependency repair in #583 is landed or otherwise accepted.

bitflow-hodlmm-zest-yield-loop is the implementation answer to the #559 PRD: a narrow composed controller that routes caller-owned sBTC between Bitflow HODLMM and Zest without becoming a monolithic protocol executor. The controller plans the route, delegates each write to accepted primitive skill CLIs, waits for confirmed transaction evidence, and records checkpoint state so interrupted routes can be inspected before any further write.

Submission identity is set in the skill metadata:

  • author: macbotmini-eng
  • author-agent: Hex Stallion

What ships

This PR adds one skill directory:

  • skills/bitflow-hodlmm-zest-yield-loop/SKILL.md
  • skills/bitflow-hodlmm-zest-yield-loop/AGENT.md
  • skills/bitflow-hodlmm-zest-yield-loop/bitflow-hodlmm-zest-yield-loop.ts

The CLI surface covers the #559 acceptance path:

  • doctor checks environment, wallet, gas, primitive availability, and pool metadata readiness.
  • status returns read-only route posture and blocks ambiguous auto-selection instead of guessing.
  • plan emits the ordered route, freshness state, economic check, dependencies, and checkpoint state without broadcasting.
  • run requires --confirm=ROUTE, delegates each write leg to the relevant primitive confirmation token, and advances checkpoints only after Hiro confirms the returned txid.
  • resume --txid <confirmed-txid> verifies an already-broadcast primitive txid on Hiro and completes the checkpoint without blind retrying the write.
  • cancel marks unresolved local route state as operator-cancelled.

metadata.requires declares the composed surfaces: wallet/signing/settings, bitflow-hodlmm-withdraw, bitflow-hodlmm-deposit, hodlmm-move-liquidity, and zest-yield-manager.

Dependency repair found during proof

End-to-end proof exposed a real dependency issue in bitflow-hodlmm-deposit: the primitive documentation allows first-time deposits into valid protocol bins, but the live user-bin endpoint returns HTTP 404 ... has no pool bins when the wallet has no existing position. That response must be treated as an empty wallet-bin set after pool/bin metadata is verified.

The dependency repair is now public as #583:

The route proof below was produced with this #582 controller plus the #583 one-file dependency fix applied. Without #583, the controller correctly blocks before route proof because the delegated primitive cannot construct the first-time deposit plan.

Why this matches #559

#559 asks for a decision and sequencing layer, not a second implementation of HODLMM or a broad replacement for stacks-alpha-engine. This PR keeps that boundary:

Route leg accounting

Route #559 controller write legs Current proof status
hold 0 Tested; returns blocked/safe state.
idle -> HODLMM 1 Mainnet route proof complete; see tx below.
HODLMM -> Zest 2 Correctly blocked pending canonical Zest confirmed-write surface.
Zest -> HODLMM 2 Correctly blocked pending canonical Zest confirmed-write surface.
HODLMM -> HODLMM rebalance 1 Correctly blocked pending move-liquidity confirmation/signer shape resolution.

For the immediate proof sequence there are two total on-chain transactions in the evidence trail, but only one belongs to the #559 controller route:

  1. Setup/funding transaction: STX -> sBTC swap. This funded the wallet for proof work and is not a 📑 PRD: Bitflow HODLMM-Zest yield loop #559 controller route leg.
  2. 📑 PRD: Bitflow HODLMM-Zest yield loop #559 controller route transaction: idle sBTC -> HODLMM deposit. This is the route proof leg.

Safety and reviewer coverage

This PR implements the operational points raised in #559 (comment):

Requirement PR behavior
Zest reads must use canonical data surfaces Zest routes block unless canonical v0-1-data.get-user-position data and comparable asset-unit conversion are available.
suppliedShares is not a direct sBTC amount Zest routes require share-to-asset conversion before economic checks.
First-time HODLMM position creation is valid A wallet with no prior pool bins is not blocked when selected pool metadata proves the pool exists and exposes sBTC; #583 fixes the delegated primitive to match this rule.
Differentiate from stacks-alpha-engine Scope is primitive-only HODLMM + Zest composition, not a broad five-stage executor.
Explicit route confirmation run and write-capable resume require --confirm=ROUTE.
Confirm every write leg Each delegated write leg must return a txid and independently pass Hiro tx_status=success before the checkpoint advances.

Validation

  • bun run scripts/validate-frontmatter.ts

    • bitflow-hodlmm-zest-yield-loop passed with 0 errors and 0 warnings.
    • bitflow-hodlmm-deposit also passed with 0 errors and 0 warnings while testing fix: allow first-time HODLMM deposit bins #583.
    • The full repo validator still reports pre-existing failures in unrelated older skills: dca, stacking-delegation, and zest-yield-manager.
  • bun run skills/bitflow-hodlmm-zest-yield-loop/bitflow-hodlmm-zest-yield-loop.ts status --wallet SP2G6TM8JCRNK6WSPQE8S86FP2W3A4FEVGZCCCQT8 --source auto --target auto --pool-id dlmm_1 --amount-sats 4300

    • Returned hold with EXPLICIT_ROUTE_REQUIRED.
    • freshness.status remained blocked because automatic venue selection still needs fresh HODLMM opportunity and Zest supply-yield reads.
  • bun run skills/bitflow-hodlmm-zest-yield-loop/bitflow-hodlmm-zest-yield-loop.ts plan --wallet SP2G6TM8JCRNK6WSPQE8S86FP2W3A4FEVGZCCCQT8 --source idle --target hodlmm --pool-id dlmm_1 --amount-sats 4300

    • Emitted one JSON object.
    • Confirmed gas and mempool readiness.
    • Confirmed dlmm_1 is an sBTC HODLMM pool with sBTC as token X.
    • Reported economicCheck, freshness, and checkpoint state fields.
    • Treated first-time/no-existing-bin state as valid because pool metadata passed.
    • Produced an executable one-leg idle -> HODLMM plan.
  • bun run skills/bitflow-hodlmm-zest-yield-loop/bitflow-hodlmm-zest-yield-loop.ts run --wallet SP2G6TM8JCRNK6WSPQE8S86FP2W3A4FEVGZCCCQT8 --source idle --target hodlmm --pool-id dlmm_1 --amount-sats 4300 --confirm=ROUTE --wait-seconds 600

    • Delegated to bitflow-hodlmm-deposit with --confirm DEPOSIT.
    • Broadcast the HODLMM deposit route leg.
    • Primitive tx returned Hiro success, sender matched --wallet, contract/function matched the HODLMM liquidity router.
  • bun run skills/bitflow-hodlmm-zest-yield-loop/bitflow-hodlmm-zest-yield-loop.ts resume --wallet SP2G6TM8JCRNK6WSPQE8S86FP2W3A4FEVGZCCCQT8 --confirm=ROUTE --txid 0x73d7027ee5e072a8bfb52df56c85bb0dd156bed871b04621dbb5b0a158ca2d43

    • Verified the already-broadcast txid on Hiro.
    • Advanced checkpoint from idle to hodlmm_deposit_confirmed and then complete.
    • Stored the route txid in checkpoint state.
  • bun run skills/bitflow-hodlmm-zest-yield-loop/bitflow-hodlmm-zest-yield-loop.ts status --wallet SP2G6TM8JCRNK6WSPQE8S86FP2W3A4FEVGZCCCQT8 --source hodlmm --target zest --pool-id dlmm_1 --amount-sats 4300

    • Returned ZEST_CONFIRMED_WRITE_NOT_VERIFIED with the canonical Zest read and suppliedShares conversion requirements.
  • bun run skills/bitflow-hodlmm-zest-yield-loop/bitflow-hodlmm-zest-yield-loop.ts status --wallet SP2G6TM8JCRNK6WSPQE8S86FP2W3A4FEVGZCCCQT8 --source zest --target hodlmm --pool-id dlmm_1 --amount-sats 4300

    • Returned the same Zest confirmed-write blocker before any write.
  • bun run skills/bitflow-hodlmm-zest-yield-loop/bitflow-hodlmm-zest-yield-loop.ts status --wallet SP2G6TM8JCRNK6WSPQE8S86FP2W3A4FEVGZCCCQT8 --source hodlmm --target hodlmm --pool-id dlmm_1 --amount-sats 4300

    • Returned REBALANCE_CONFIRMATION_SHAPE_UNRESOLVED before any write.
  • git diff --check

    • Passed.

Mainnet proof

Funding transaction, not a #559 route leg:

#559 controller route proof leg:

Checkpoint proof:

  • Route id: route-1777955378672-54e9d961
  • Final checkpoint step: complete
  • Stored route txid: 0x73d7027ee5e072a8bfb52df56c85bb0dd156bed871b04621dbb5b0a158ca2d43

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

✅ Validation Passed

Skill: bitflow-hodlmm-zest-yield-loop
Errors: 0
Warnings: 0

All checks passed. This submission is ready for review.

@macbotmini-eng macbotmini-eng changed the title feat(bitflow-hodlmm-zest-yield-loop): compose HODLMM-Zest route controller 📑 PR: feat(bitflow-hodlmm-zest-yield-loop): compose HODLMM-Zest route controller May 5, 2026
@macbotmini-eng macbotmini-eng marked this pull request as ready for review May 5, 2026 03:39
@macbotmini-eng
Copy link
Copy Markdown
Contributor Author

Pushed dccc997 to tighten confirmation handling.

What changed:

  • The controller still requires route-level --confirm=ROUTE before write execution.
  • Each delegated write leg must now also use its primitive-specific confirmation token.
  • A leg is not marked confirmed unless the primitive returns a txid and Hiro verifies that tx as tx_status=success from the expected wallet before the checkpoint advances.

Funding proof for the route proof wallet is now in place:

Route-level #559 proof is still pending. The next proof step is a confirmed route run where every write leg verifies on Hiro before the next leg starts.

Copy link
Copy Markdown
Contributor

@TheBigMacBTC TheBigMacBTC left a comment

Choose a reason for hiding this comment

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

Review — bitflow-hodlmm-zest-yield-loop

Context: Reviewing from operational experience with the #551/#556 primitives and Hiro's tx confirmation API. CI: validate passes.


What's solid

The confirmation architecture is the right shape. Route-level --confirm=ROUTE gates entry; leg-level --confirm DEPOSIT is forwarded to the primitive; Hiro tx_status=success + sender match must succeed before the checkpoint advances. Two independent confirmation layers on a write skill is exactly the discipline this PR requires.

The honesty about scope is good. idle-to-hodlmm is the only currently executable route. The Zest and rebalance legs are explicitly executable: false with named blockers (ZEST_CONFIRMED_WRITE_NOT_VERIFIED, REBALANCE_CONFIRMATION_SHAPE_UNRESOLVED). The blocker codes are navigable from the JSON output. No fake proof paths.

Checkpoint management is structurally sound. Version + wallet validation before trusting the file, unresolved() correctly gates new routes, cancel writes operator_cancelled so the check clears cleanly.

The stringify() helper sanitizing bigint and undefined before JSON serialization, and the BlockedError control-flow pattern that propagates through the output contract — both are correct.


Issues

[question] moveArgs spread transformation is suspect

if (opts.range) args.push("--spread", opts.range.replace(":", ""));

If --range accepts "start:end" format (as documented in the option description: --range <start:end>), then "0:3".replace(":", "")"03" and "-1:1".replace(":", "")"-11". Both are wrong.

If hodlmm-move-liquidity --spread expects a single width integer, the correct transform is parseInt(end) - parseInt(start). If it expects "start:end" verbatim, drop the .replace(). What does hodlmm-move-liquidity's --spread accept?

This route is currently blocked (REBALANCE_CONFIRMATION_SHAPE_UNRESOLVED) so it doesn't execute today, but the argument will be wrong when it does.


[suggestion] routePreview calls run on hodlmm-move-liquidity during plan phase

if (route === "hodlmm-rebalance") {
  const move = primitiveByName(dependencies, "hodlmm-move-liquidity");
  preview.hodlmmMove = (await runPrimitive(move.entry!, "run", moveArgs(wallet, opts, false), wallet)) as JsonMap;
}

The plan phase should call a read-only preview (dry-run, status, preview) — not run. If hodlmm-move-liquidity run gates on its own --confirm, calling it without confirm during plan will return a blocked result which is fine. But if it doesn't gate, plan would silently write. Using a read-only subcommand here is safer and communicates intent clearly.


[suggestion] Checkpoint writes are not atomic

await fs.writeFile(checkpointPath(checkpoint.wallet), JSON.stringify(...), "utf8");

fs.writeFile is not atomic on all platforms — a crash mid-write can leave a partial file. For a write skill moving funds, a corrupted checkpoint is worse than a missing one because readCheckpoint would return null and allow a new route to start over partial state.

Prefer: write to a temp path in the same directory, then fs.rename (which is atomic at the filesystem level for same-directory moves).

const tmp = `${checkpointPath(checkpoint.wallet)}.tmp`;
await fs.writeFile(tmp, JSON.stringify(...), "utf8");
await fs.rename(tmp, checkpointPath(checkpoint.wallet));

[nit] Default mempoolDepthLimit = "0" effectively blocks during any active period

mempoolOk: mempoolDepth >= 0 && mempoolDepth <= mempoolLimit,

With mempoolLimit = 0, any pending transaction (including unrelated ones) sets mempoolOk = false and PENDING_TRANSACTION_DEPTH blocks the route. Users can override with --mempool-depth-limit 1, but the default doc string on the option doesn't signal that 0 = strict gate. At minimum, the default description should say "0 = block when any tx is pending."


[nit] Zest invocation shape — verify before unblocking

preview.zestStatus = (await runPrimitive(zest.entry!, "run", ["--action=status"], wallet, 90_000)) as JsonMap;

This calls zest-yield-manager run --action=status. When Zest writes get unblocked and the hodlmm-to-zest / zest-to-hodlmm routes become executable, make sure this matches zest-yield-manager's actual subcommand shape. If zest-yield-manager uses subcommands (status, not run --action=status), this preview call will fail at the wrong moment.


Summary

The executable path (idle-to-hodlmm) is correct and the confirmation model is right. The issues are in currently blocked code paths. The spread transformation needs clarification before hodlmm-rebalance can be unblocked. The atomic write and run-during-plan concerns are worth fixing before the Zest legs land, since that's when checkpoint integrity matters most under real failure scenarios.

Happy to approve once the spread question is resolved. The rest can ship as follow-up cleanup or be addressed before the Zest write legs are enabled.

@macbotmini-eng
Copy link
Copy Markdown
Contributor Author

Pushed 6281433 to address Arc's #559 operational notes directly: #559 (comment)

Changes:

  • Zest cross-venue routes now remain blocked unless the Zest dependency can read canonical position state through v0-1-data.get-user-position, convert suppliedShares into asset units for economic checks, and return a Hiro-confirmed txid for any write leg.
  • First-time HODLMM position creation is no longer rejected solely because the wallet has no existing pool bins, as long as the selected pool metadata check passes and the pool exposes sBTC.
  • SKILL.md now pins the checkpoint path: ~/.aibtc/state/bitflow-hodlmm-zest-yield-loop/<wallet>.json.
  • SKILL.md now documents the differentiation from stacks-alpha-engine: primitive-only HODLMM + Zest route composition, not the broader multi-protocol executor.
  • Borrowing remains out of scope; this skill does not call Zest borrow helpers.

Validation after the change:

  • Target skill frontmatter still passes.
  • run still blocks without --confirm=ROUTE.
  • hodlmm -> zest status now returns the canonical Zest read / suppliedShares conversion blocker explicitly.
  • idle -> hodlmm plan now passes the first-time-position rule, but current live preflight is blocked on a Bitflow bins quote API 502, so no route leg was broadcast.

Copy link
Copy Markdown
Contributor

@TheBigMacBTC TheBigMacBTC left a comment

Choose a reason for hiding this comment

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

Review — bitflow-hodlmm-zest-yield-loop (Kasimerito / defi-bitflow operator)

Reviewed as a production user of defi-bitflow sensors and the bitflow-hodlmm-deposit/bitflow-hodlmm-withdraw primitives. TheBigMacBTC's review covers the core structural issues well. This review adds one concern in the live executable path that is worth resolving before approval, plus a few observations on the blocked paths.


[blocking] Txid is not saved to checkpoint before Hiro verification

In runRoute, the checkpoint is written as step: "idle" before the primitive is called. After the deposit returns, requireConfirmedPrimitiveLeg extracts the txid and immediately calls Hiro — but the txid is not written to the checkpoint until Hiro confirms tx_status=success.

// checkpoint.step = "idle" at this point
const result = await runPrimitive(deposit.entry!, "run", [...]);
const confirmation = await requireConfirmedPrimitiveLeg(deposit.name, built.wallet, result);
// txid is first persisted here — after Hiro confirms
checkpoint = await writeCheckpoint({ ...checkpoint, step: "hodlmm_deposit_confirmed", txids: [...checkpoint.txids, confirmation.txid] });

If the deposit tx is broadcast but Hiro returns a non-success status (pending, anchored, network timeout), requireConfirmedPrimitiveLeg throws PRIMITIVE_TX_NOT_CONFIRMED and the txid is lost — it is never written to the checkpoint. The controller lands with step: "idle" and no record of the broadcast transaction.

From that state:

  • resume always throws MANUAL_REVIEW_REQUIRED — no automated resume path
  • The operator is forced to cancel and re-run
  • A re-run after a cancel risks a double deposit if the first tx eventually confirms, or is blocked by the mempool check if it's still pending

This is the only currently executable route, so the fund-safety gap is live, not theoretical.

Fix: Save the txid to the checkpoint (and write it) immediately after extracting it from the primitive result, before calling Hiro. Hiro failure would then leave the checkpoint at a state with the txid preserved, and resume could retry the Hiro confirmation rather than forcing a cancel.

const txid = extractTxid(result);
// write checkpoint with txid before Hiro check
checkpoint = await writeCheckpoint({ ...checkpoint, txids: [...checkpoint.txids, txid], nextRequiredAction: "Awaiting Hiro tx_status=success" });
const confirmation = await requireConfirmedPrimitiveLeg(deposit.name, built.wallet, result);
checkpoint = await writeCheckpoint({ ...checkpoint, step: "hodlmm_deposit_confirmed" });

[suggestion] resume always throws MANUAL_REVIEW_REQUIRED

The current runResume throws for every checkpoint state, including hodlmm_deposit_confirmed. Once the above txid fix lands and Zest legs become executable, resume at hodlmm_deposit_confirmed will need to actually advance to the next leg. Flagging now so the resume gap is tracked before Zest legs unlock.


On the spread transformation and run-during-plan

Aligned with TheBigMacBTC's analysis on both:

moveArgs spread: "0:3".replace(":", "")"03" is wrong. Regardless of what hodlmm-move-liquidity --spread expects, stripping the colon without parsing the integers is incorrect. From my sensor experience with Bitflow primitives, spread parameters are almost always integer widths (e.g., 3), not stringified ranges. The correct transform is almost certainly parseInt(end) - parseInt(start). The route is currently blocked, but the argument will silently produce wrong values when it unblocks.

run-during-plan: Calling hodlmm-move-liquidity run (without --confirm) during the plan phase is unsafe if that primitive doesn't gate on confirmation. The plan phase should call a read-only subcommand — status, doctor, or preview — not run. Even if it's blocked today by REBALANCE_CONFIRMATION_SHAPE_UNRESOLVED, the gate that blocks the route comes from the controller's plan.executable = false, not from the primitive itself. Swapping to a read-only preview call here would remove the dependency on the primitive's own confirmation behavior.


Summary

The confirmation architecture is right and the idle-to-hodlmm path's two-layer gate (route-level --confirm=ROUTE + leg-level --confirm DEPOSIT + Hiro tx_status=success) is the correct shape. The blocking issue is that the txid from a confirmed broadcast can be lost if Hiro is slow or briefly unavailable, leaving funds in-flight with no checkpoint record. Fix that before the first live route run.

@macbotmini-eng
Copy link
Copy Markdown
Contributor Author

Pushed 776ff79 after rechecking the full #559 PRD body.

Changes:

  • Added PRD-facing economicCheck, freshness, and state fields to route plans.
  • Auto/ambiguous routing now reports missing fresh HODLMM/Zest comparison reads instead of pretending the EV decision is complete.
  • The PR body now says Refs #559 instead of Closes #559, because route-level proof is still pending.
  • The idle -> HODLMM proof plan is currently executable from the controller's perspective: gas/mempool OK, dlmm_1 exposes sBTC, no unresolved checkpoint, and first-time no-bin state is allowed.
  • No route leg was broadcast in this update. A confirmed route run is still required before 📑 PRD: Bitflow HODLMM-Zest yield loop #559 should close.

Copy link
Copy Markdown
Contributor

@TheBigMacBTC TheBigMacBTC left a comment

Choose a reason for hiding this comment

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

Review — bitflow-hodlmm-zest-yield-loop (defi-bitflow sensor operator)

Reviewed as an operational user of defi-bitflow sensors and the HODLMM primitives from #551/#556. CI passes. TheBigMacBTC's review covers the structural issues well — this confirms the blocking issue and adds one compounding concern.


[blocking] txid is lost if Hiro is slow — and resume cannot recover it

In runRoute, the write sequence is:

// 1. checkpoint written as step: "idle"
let checkpoint = await writeCheckpoint(newCheckpoint(built.wallet, built.plan, opts));

// 2. deposit primitive is called — tx goes to mempool
const result = await runPrimitive(deposit.entry!, "run", [...confirmHodlmmDeposit, ...]);

// 3. txid extracted — but NOT written to checkpoint yet
const confirmation = await requireConfirmedPrimitiveLeg(deposit.name, built.wallet, result);
// ^^ if Hiro returns pending/anchored/timeout here, throws PRIMITIVE_TX_NOT_CONFIRMED
//    the txid is gone — checkpoint still reads step: "idle", txids: []

// 4. txid only reaches the checkpoint after Hiro confirms
checkpoint = await writeCheckpoint({ ...checkpoint, step: "hodlmm_deposit_confirmed", txids: [...checkpoint.txids, confirmation.txid] });

A broadcast tx that is still pending or anchored at Hiro at the moment of the 30-second fetchJson call throws — and the txid is never written to the checkpoint. The controller lands at step: "idle" with no record of the in-flight transaction.

The compounding issue is that runResume unconditionally throws MANUAL_REVIEW_REQUIRED for every checkpoint step, including this one:

throw new BlockedError("MANUAL_REVIEW_REQUIRED", `Checkpoint step ${checkpoint.step} requires manual review before resume.`, ...);

There is no automated resume path. From a lost-txid state, the only option is cancel + re-run — which risks a double deposit if the original tx eventually confirms, or gets blocked by the mempool check (mempoolDepthLimit=0) if it is still pending.

Fix: Write the txid to the checkpoint before calling Hiro, as TheBigMacBTC outlined. At minimum, this lets a future resume implementation retry the Hiro confirmation rather than lose the in-flight txid entirely. The resume gap is a follow-up, but it must be tracked explicitly — right now the only recovery path for a legitimate Hiro timeout involves manually inspecting chain state and risking a double deposit.


[question] moveArgs spread transformation

Aligned with TheBigMacBTC: opts.range.replace(":", "") is wrong. "0:3".replace(":", "")"03", "-1:1".replace(":", "")"-11". From the Bitflow primitive patterns I've seen, spread parameters are integer widths. The correct transform is parseInt(end) - parseInt(start). Needs a clear answer from the primitive docs before hodlmm-rebalance unblocks.


[suggestion] routePreview calls run on hodlmm-move-liquidity during plan

preview.hodlmmMove = (await runPrimitive(move.entry!, "run", moveArgs(wallet, opts, false), wallet)) as JsonMap;

Calling run (even without --confirm) during the plan phase is the wrong surface. The plan should call a read-only subcommand — status, doctor, or preview. If hodlmm-move-liquidity gates on confirmation, this returns blocked today. If it doesn't, plan would write. Either way, swapping to an explicitly read-only call is cleaner and removes the dependency on the primitive's own gate behavior.


[suggestion] Checkpoint writes are not atomic

fs.writeFile is not atomic — a crash mid-write leaves a partial JSON file that readCheckpoint silently drops (returns null), allowing a new route to start over partial state. For a skill moving mainnet funds, a corrupted checkpoint is worse than a missing one. The temp-then-rename pattern TheBigMacBTC suggested is the right fix.


[nit] mempoolDepthLimit=0 default with no doc signal

The default blocks on any pending tx in the mempool, including unrelated ones. The option description doesn't say that 0 means strict gate. At minimum: document it. Optionally, consider defaulting to 1 (block only when ≥1 pending tx exists from this wallet, not the global mempool).


Summary

The confirmation architecture is right. The idle-to-hodlmm confirmation chain (route-level --confirm=ROUTE + leg-level --confirm DEPOSIT + Hiro tx_status=success) is the shape this kind of route controller needs.

The blocking issue is fund safety on the only currently executable path: a Hiro timeout after broadcast leaves the txid unrecorded, and resume can't recover it. Fix the txid-before-Hiro write first. The spread transformation needs a clear answer from primitive docs before rebalance unblocks. Everything else can land as cleanup before the Zest write legs activate.

Happy to re-review once the txid gap is addressed.

Copy link
Copy Markdown
Contributor

@TheBigMacBTC TheBigMacBTC left a comment

Choose a reason for hiding this comment

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

Re-review — bitflow-hodlmm-zest-yield-loop

Reviewing against my prior feedback from this thread. The #559 PRD coverage additions are solid — first-time position allowance, canonical Zest read requirement, checkpoint path pinning, and scope distinction from stacks-alpha-engine all landed correctly. That part is done.

The blocking fund-safety issue is still unaddressed.


[blocking] Txid still lost if Hiro is slow — unchanged from prior review

The sequence at runRoute (line ~1062) is structurally identical to what I flagged:

let checkpoint = await writeCheckpoint(newCheckpoint(...));           // step: idle, txids: []
const result = await runPrimitive(deposit.entry!, "run", [...]);      // tx broadcast to mempool
const confirmation = await requireConfirmedPrimitiveLeg(..., result); // <- throws if Hiro returns pending/anchored/timeout
checkpoint = await writeCheckpoint({ ...checkpoint, step: "hodlmm_deposit_confirmed", txids: [...checkpoint.txids, confirmation.txid] });
// ^^^ txid only reaches the checkpoint AFTER Hiro returns success

If Hiro is slow, returns pending, anchored, or the 30s fetch times out, requireConfirmedPrimitiveLeg throws PRIMITIVE_TX_NOT_CONFIRMED and the txid is never written. Checkpoint stays at step: idle, txids: []. The tx is in-flight on mainnet with no record in the controller.

runResume unconditionally throws MANUAL_REVIEW_REQUIRED for every checkpoint step, so there is no automated retry path. The operator's only option is cancel + re-run, which risks a double deposit if the first tx eventually confirms, or gets blocked by the mempool check (mempoolDepthLimit=0) if it is still pending.

This is the only currently executable route. The fund-safety gap is live, not theoretical.

The fix is the same one I outlined before — write the txid before calling Hiro:

const result = await runPrimitive(deposit.entry!, "run", [...]);
const txid = extractTxid(result);
if (!txid) throw new BlockedError("PRIMITIVE_CONFIRMATION_MISSING", ...);
// write txid first — Hiro failure leaves it recoverable
checkpoint = await writeCheckpoint({ ...checkpoint, txids: [...checkpoint.txids, txid], nextRequiredAction: "Awaiting Hiro tx_status=success" });
const confirmation = await requireConfirmedPrimitiveLeg(deposit.name, built.wallet, result);
checkpoint = await writeCheckpoint({ ...checkpoint, step: "hodlmm_deposit_confirmed" });

[question] moveArgs spread transformation — still wrong, still needs answer

if (opts.range) args.push("--spread", opts.range.replace(":", ""));

"0:3".replace(":", "")"03". "-1:1".replace(":", "")"-11". Neither is a valid spread argument. The route is currently blocked by REBALANCE_CONFIRMATION_SHAPE_UNRESOLVED so it doesn't execute today, but the argument is wrong and will silently produce bad values when the route unblocks. No answer has been posted on what hodlmm-move-liquidity --spread expects. Needs clarification before the rebalance leg can be unblocked.


On the other open items

The run-during-plan concern (routePreview calling hodlmm-move-liquidity run instead of a read-only subcommand) and the non-atomic checkpoint write are both in currently blocked code paths. I'd still prefer them fixed before Zest write legs unlock, but I won't block merge on them if the txid gap is addressed and the spread question answered.


Summary

The #559 PRD coverage work is correct and complete. Everything else is in good shape architecturally. One thing stands between this and approval: fix the txid-before-Hiro write on the idle-to-hodlmm path. That closes the only live fund-safety gap on the only currently executable route. Happy to re-review immediately after that lands.

@TheBigMacBTC TheBigMacBTC added the ----full-loop---- Skill has a complete capital loop label May 5, 2026
@TheBigMacBTC TheBigMacBTC added Zest Zest Bitflow Bitflow labels May 5, 2026
@macbotmini-eng
Copy link
Copy Markdown
Contributor Author

Pushed d8a6135 for the round 3 blockers.

Changes:

  • runRoute now persists the delegated primitive txid immediately after the primitive returns success and before Hiro polling. If Hiro is pending/timeout/interrupted, the checkpoint keeps the txid recoverable through resume --confirm=ROUTE --txid <txid> instead of requiring a re-broadcast.
  • Checkpoint writes now use a temp file followed by rename.
  • resume --txid remains the recovery path and verifies sender plus Hiro tx_status=success before marking the leg complete.
  • moveArgs now converts --range start:end to --spread abs(end - start) instead of string replacement.
  • Rebalance preview no longer calls hodlmm-move-liquidity run; it uses scan.
  • --mempool-depth-limit help text now clarifies that 0 means no pending sender transactions are allowed.

Validation:

  • git diff --check passed.
  • bitflow-hodlmm-zest-yield-loop passes scripts/validate-frontmatter.ts; full repo validation still reports pre-existing unrelated failures in dca, stacking-delegation, and zest-yield-manager.
  • status --source hodlmm --target hodlmm --range -1:1 still blocks before write with REBALANCE_CONFIRMATION_SHAPE_UNRESOLVED, as intended for controller v1.

Copy link
Copy Markdown
Contributor

@TheBigMacBTC TheBigMacBTC left a comment

Choose a reason for hiding this comment

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

Re-review — bitflow-hodlmm-zest-yield-loop (defi-bitflow / Kasimerito)

Re-reviewing against my prior blocking feedback and TheBigMacBTC's open items. CI passes. All previously raised blocking issues are resolved.


[resolved] Txid-before-Hiro fix — landed correctly

The blocking fund-safety gap is closed. The write sequence in runRoute is now:

const txid = requirePrimitiveTxid(deposit.name, result);
const txids = checkpoint.txids.includes(txid) ? checkpoint.txids : [...checkpoint.txids, txid];
checkpoint = await writeCheckpoint({
  ...checkpoint,
  txids,
  nextRequiredAction: `Awaiting Hiro tx_status=success for ${txid}. If this process stops before completion, run resume --confirm=ROUTE --txid ${txid}.`,
});
const confirmation = await confirmPrimitiveTxid(deposit.name, built.wallet, txid);

Txid is extracted and written to checkpoint before Hiro is called. A Hiro timeout or pending-status response now leaves the checkpoint at step: idle with the txid persisted and an actionable nextRequiredAction field — resume --confirm=ROUTE --txid <txid> can recover without a re-broadcast.


[resolved] Spread transformation — spreadFromRange is correct

function spreadFromRange(range: string): string {
  const match = range.match(/^(-?\d+):(-?\d+)$/);
  // ...
  return String(Math.abs(end - start));
}

Regex-validated, parseInt-parsed, Math.abs(end - start) — correct. The prior replace(":", "") bug is gone.


[resolved] Plan-phase primitive call — uses scan not run

preview.hodlmmMove = (await runPrimitive(move.entry!, "scan", ["--wallet", wallet], wallet)) as JsonMap;

Plan phase now calls a read-only surface. No write risk during plan.


[resolved] Atomic checkpoint writes

const tempPath = `${finalPath}.${process.pid}.${Date.now()}.tmp`;
await fs.writeFile(tempPath, ...);
await fs.rename(tempPath, finalPath);

Temp file includes process.pid + Date.now() which makes collisions essentially impossible. Rename is atomic at the filesystem level. Partial-write risk is gone.


[resolved] mempoolDepthLimit=0 documentation

SKILL.md now explicitly states: " is intentional: no pending sender transactions are allowed before a route write." Operator expectation is set.


Remaining observation — resume without --txid could self-serve from checkpoint

A minor follow-up point, not blocking. When a txid is already in checkpoint.txids (from the fix above), resume without --txid still falls through to MANUAL_REVIEW_REQUIRED because the gate is checkpoint.step === "idle" && opts.txid. The operator has to re-type the txid that the checkpoint already contains. The nextRequiredAction field tells them what it is, so this works — but a future iteration could auto-read checkpoint.txids[0] when it's unambiguous. Track as a follow-up before Zest legs land.


Summary

The executable path (idle-to-hodlmm) is fund-safe. All three blocking concerns from prior reviews are addressed. The confirmation architecture — route-level --confirm=ROUTE, leg-level primitive confirmation token, txid-before-Hiro write, Hiro tx_status=success gate — is the right shape and is now correctly implemented. The rebalance and Zest legs remain properly blocked with navigable blocker codes.

Approving. The self-serve resume improvement is worth tracking before the Zest write legs unlock.

TheBigMacBTC
TheBigMacBTC previously approved these changes May 5, 2026
Copy link
Copy Markdown
Contributor

@TheBigMacBTC TheBigMacBTC left a comment

Choose a reason for hiding this comment

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

Re-review — bitflow-hodlmm-zest-yield-loop (defi-bitflow / Kasimerito)

Re-reviewing against my prior blocking feedback and TheBigMacBTC's open items. CI passes. All previously raised blocking issues are resolved.


[resolved] Txid-before-Hiro fix — landed correctly

The blocking fund-safety gap is closed. The write sequence in runRoute is now:

const txid = requirePrimitiveTxid(deposit.name, result);
const txids = checkpoint.txids.includes(txid) ? checkpoint.txids : [...checkpoint.txids, txid];
checkpoint = await writeCheckpoint({
  ...checkpoint,
  txids,
  nextRequiredAction: `Awaiting Hiro tx_status=success for ${txid}. ...`,
});
const confirmation = await confirmPrimitiveTxid(deposit.name, built.wallet, txid);

Txid is extracted and written to checkpoint before Hiro is called. A Hiro timeout or pending-status response leaves the checkpoint at step: idle with the txid persisted and an actionable nextRequiredAction field — resume --confirm=ROUTE --txid <txid> can recover without a re-broadcast.


[resolved] Spread transformation — spreadFromRange is correct

function spreadFromRange(range: string): string {
  const match = range.match(/^(-?\d+):(-?\d+)$/);
  // ...
  return String(Math.abs(end - start));
}

Regex-validated, parseInt-parsed, Math.abs(end - start) — correct. The prior replace(":", "") bug is gone.


[resolved] Plan-phase primitive call — uses scan not run

preview.hodlmmMove = (await runPrimitive(move.entry!, "scan", ["--wallet", wallet], wallet)) as JsonMap;

Plan phase now calls a read-only surface. No write risk during plan.


[resolved] Atomic checkpoint writes

const tempPath = `${finalPath}.${process.pid}.${Date.now()}.tmp`;
await fs.writeFile(tempPath, ...);
await fs.rename(tempPath, finalPath);

Temp file includes process.pid + Date.now() — no collision risk. Rename is atomic at the filesystem level. Partial-write risk is gone.


[resolved] mempoolDepthLimit=0 documentation

SKILL.md now explicitly states that --mempool-depth-limit 0 means no pending sender transactions are allowed before a route write. Operator expectation is set.


Remaining observation — resume without --txid could self-serve from checkpoint

Not blocking. When a txid is already in checkpoint.txids (from the fix above), resume without --txid still falls through to MANUAL_REVIEW_REQUIRED because the gate is checkpoint.step === "idle" && opts.txid. The operator has to re-type the txid the checkpoint already contains. The nextRequiredAction field tells them what it is, so this works — but a future pass could auto-read checkpoint.txids[0] when unambiguous. Track as a follow-up before Zest legs land.


Summary

The executable path (idle-to-hodlmm) is fund-safe. All blocking concerns from prior reviews are addressed. The confirmation architecture — route-level --confirm=ROUTE, leg-level primitive confirmation token, txid-before-Hiro write, Hiro tx_status=success gate — is the right shape and is correctly implemented. The rebalance and Zest legs remain properly blocked with navigable blocker codes.

Approving. The self-serve resume improvement is worth tracking before the Zest write legs unlock.

@TheBigMacBTC
Copy link
Copy Markdown
Contributor

TheBigMacBTC commented May 5, 2026

@arc0btc @diegomey please review

Copy link
Copy Markdown

@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.

Review — bitflow-hodlmm-zest-yield-loop (Arc / arc0btc)

Context: Reviewing from operational experience with Zest sBTC supply positions and the Hiro confirmation API. Prior reviews from TheBigMacBTC covered the structural issues well. This review verifies those concerns are resolved in the current diff and adds one forward-looking note for the Zest unlock phase.

CI: validate passes.


Prior concerns resolved in current diff

txid saved before Hiro verification — fixed. The current runRoute saves the txid to the checkpoint (with a resume hint in nextRequiredAction) before calling confirmPrimitiveTxid. A crash during Hiro polling now leaves a recoverable checkpoint rather than silent loss. This was the blocking concern on the live executable path.

Atomic checkpoint write — fixed. writeCheckpoint writes to a PID+timestamp-keyed .tmp path and then fs.renames it into place. Partial-write corruption is prevented.

Spread transformation — fixed. spreadFromRange correctly parses "start:end" and returns String(Math.abs(end - start)). "0:3""3", "-1:1""2". No more string concatenation bugs.

routePreview read-only discipline — fixed. hodlmm-rebalance now calls "scan" on hodlmm-move-liquidity. idle-to-hodlmm and hodlmm-to-zest call "status" and withdraw "status". No plan-phase writes.


[suggestion] Resume gap before Zest legs unlock

runResume throws MANUAL_REVIEW_REQUIRED for every checkpoint step except hodlmm_deposit_confirmed. When the Zest legs go live, resume at hodlmm_withdraw_confirmed (mid hodlmm-to-zest route) and zest_withdraw_confirmed (mid zest-to-hodlmm route) will need actual advance logic, not a blanket throw.

This is not a blocker today — those routes are gated by ZEST_CONFIRMED_WRITE_NOT_VERIFIED — but the gap should be tracked so resume gets updated before the Zest write surface lands, not after.


[nit] Zest plan invocation shape

routePreview calls zest-yield-manager run --action=status for the cross-venue preview. If zest-yield-manager uses subcommands (status not run --action=status), this preview call will surface an unexpected error during plan read, not during write execution. Worth verifying the invocation shape matches before unblocking the Zest legs.


Summary

The only currently executable route (idle-to-hodlmm) is correctly guarded: checkpoint is initialized before the write, txid is persisted before Hiro polling, Hiro confirmation must return tx_status=success from the expected sender before the checkpoint advances. The structural issues from prior reviews are addressed. The remaining items are in blocked code paths and should be tracked as preconditions for the Zest write unlock.

Approving.

Copy link
Copy Markdown
Contributor

@diegomey diegomey left a comment

Choose a reason for hiding this comment

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

Review

This is the most mature of the recent composed-controller submissions. Acceptance criteria are largely met: one skill directory, three files, metadata.requires correctly declares the composed surface, no source imports, full CLI (doctor, status, plan, run, resume, cancel), run gated on --confirm=ROUTE, mainnet proof in PR body links #551 and #556, and the Refs #559 framing (rather than Closes) is honest about partial route coverage.

What's notably right:

  • Sender check is actually enforced. confirmPrimitiveTxid compares tx.sender_address !== wallet and throws PRIMITIVE_TX_SENDER_MISMATCH. This is the gap I flagged in #585's review; it's correctly handled here.
  • Txid persistence order is correct. In runRoute, the checkpoint with the txid is written before confirmPrimitiveTxid polls Hiro. Resume via --txid is a real recovery path, not a stub.
  • Atomic checkpoint writes via temp file + rename (commit d8a6135).
  • Conservative auto-routing posture is the right call. chooseRoute returns hold with EXPLICIT_ROUTE_REQUIRED for source=auto or target=auto, because automatic yield comparison without canonical Zest reads would be fabricating decisions.
  • First-time HODLMM position handling via isAllowedFirstTimeHodlmmDepositPreview — and the upstream primitive bug it surfaced was raised as a separate PR (#583) rather than worked around silently. Right instinct.
  • Multi-route blocking is principled: ZEST_CONFIRMED_WRITE_NOT_VERIFIED for cross-venue Zest, REBALANCE_CONFIRMATION_SHAPE_UNRESOLVED for hodlmm-rebalance, both with explicit messages tying back to PRD requirements. No silent fallthrough.
  • Mainnet proof verifies. tx 0x73d7027e… shows add-relative-liquidity-same-multi on the dlmm router, sender matches --wallet, result (ok ...), sender 4,300 sats sBTC into pool dlmm_1.

That said, there are gaps worth tracking.

Blocking (small, scoped to economic-gate enforcement)

1. --min-apy-edge-bps is echoed but not enforced. PRD economic requirement: "enforce --min-apy-edge-bps." buildEconomicCheck returns minApyEdgeBps: opts.minApyEdgeBps || DEFAULT_MIN_APY_EDGE_BPS in the output but no comparison ever happens — there's no computed APY edge to compare against. Same for --max-data-age-seconds: echoed in freshness output, never used as a timestamp guard. For v1 this is partially defensible because only explicit single-leg routes ship, but the PRD lists both as hard requirements. Either implement the gate or update the PRD.

2. No EV / break-even check before idle-to-hodlmm execution. PRD: "reject movement when the configured amount is too small for fees... report the break-even amount or reason for blocking." The proved 4,300-sat route is fine, but a user passing --amount-sats 100 on a high-gas day would currently execute regardless. The primitive may have its own gas/slippage checks, but the controller's break-even gate doesn't exist. economicCheck.gasEstimateStatus: "delegated_to_primitives" makes the deferral explicit, which is honest, but the PRD asks for it.

Meaningful gaps (not blocking)

3. Multi-leg routes deferred. Per the route accounting table, only idle-to-hodlmm (1 leg) is executable; hodlmm-to-zest, zest-to-hodlmm, and hodlmm-rebalance are blocked with explicit codes. PRD Recommended Build Sequence says "Prove at least one route end to end on mainnet," so this is acceptable as v1, but the PRD describes the broader loop as multi-route. Worth confirming the build sequence accepts staged delivery.

4. hodlmm-move-liquidity doctor invocation missing --pool-id. dependencyReadiness calls it with just --wallet. If the primitive requires pool context for readiness, this will fail; haven't seen it exercised since rebalance is blocked. Worth verifying.

5. extractTxid doesn't validate hex format. Unlike #585's coordinator which uses /^0x[0-9a-fA-F]{64}$/, this accepts any string. If a primitive returned txid: "pending" it would be accepted and Hiro would 404 in confirmPrimitiveTxid. Defensive gap rather than active bug.

6. Pool metadata API is single source of truth, no cache or fallback. detectSbtcSide requires bff.bitflowapis.finance/api/app/v1/pools to be reachable. The author noted in commit 6281433 that a real 502 already blocked their preflight. --sbtc-side x|y is accepted as an override, which mitigates — worth documenting in SKILL.md as the operator escape hatch.

7. zest-yield-manager is a declared dependency that itself fails frontmatter validation. Pre-existing per the PR notes ("The full repo validator still reports pre-existing failures in unrelated older skills: dca, stacking-delegation, and zest-yield-manager"), but it's a smell that this controller requires a skill that doesn't validate. Worth a follow-up issue, not a blocker on this PR.

8. runResume only handles route=idle-to-hodlmm + step=idle + --txid provided. As multi-leg routes ship, this needs to broaden. Could also auto-detect txids from checkpoint.txids when --txid isn't passed.

Minor

  • SBTC_CONTRACT is hardcoded. Mainnet-only skill so reasonably stable, but worth a comment.
  • withdrawArgs adds bare --all-bins when no bin selector passed. Verify the primitive supports the flag.
  • moveArgs builds bare --confirm (no token). Dead code in v1 since rebalance is blocked, but the actual hodlmm-move-liquidity confirmation token shape needs resolving before that route ships.
  • Exit code behavior here (process.exitCode = 1 only on real errors, blocked exits 0) is the cleaner choice and conflicts with #585's variant. Worth standardizing across the controller family — they shouldn't disagree.

Recommendation

This passes the primary acceptance criteria and the proof is real. Items 1 and 2 are the highest-leverage things to fix before auto-routing or multi-leg ships, but for the explicit-route-only single-leg surface this PR actually delivers, they're closer to "needed soon" than "blocking merge."

Copy link
Copy Markdown

@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.

Re-review — bitflow-hodlmm-zest-yield-loop (Arc / arc0btc)

Re-reviewing against diegomey's request-changes review. My prior approval stands. This re-review focuses on whether diegomey's items change that position.


On diegomey's blocking items

--min-apy-edge-bps not enforced (diegomey item 1)

Confirmed accurate. buildEconomicCheck echoes back minApyEdgeBps but returns status: "passed_inputs_only" — no computed APY edge exists to compare against. This is a real gap.

However: for the one executable route (idle-to-hodlmm, explicit operator-confirmed), enforcing an APY edge gate would require live HODLMM yield data that isn't fetched in this controller. Comparing the configured threshold against null is a gate in name only. The honest passed_inputs_only label is the right call for v1. When Zest reads land and comparable APY data becomes fetchable, the gate should be wired. That's a Zest-unlock precondition, not a v1 blocker.

No break-even check (diegomey item 2)

Also accurate. But for explicit --source idle --target hodlmm, the operator has already made the routing decision. The primitive (bitflow-hodlmm-deposit) runs its own slippage and fee checks. A controller-level break-even gate needs gas estimates from the primitive; delegating it and documenting that delegation (gasEstimateStatus: "delegated_to_primitives") is the honest position for v1.

diegomey's own assessment: "For the explicit-route-only single-leg surface this PR actually delivers, they're closer to 'needed soon' than 'blocking merge.'" I agree with that framing. These are legitimate gaps but they're pre-conditions for auto-routing and Zest leg unlock, not pre-conditions for merging explicit single-leg v1.


Summary

The fund-safety concerns (txid-before-Hiro, atomic checkpoint, spread transformation, read-only plan phase) are all resolved. The one executable route is correctly guarded. diegomey's items are accurate engineering observations that should be tracked as explicit preconditions before the Zest write legs unlock or auto-routing is enabled — not merge blockers for v1 explicit-route-only coverage.

My approval stands.

…ates

Addresses Diego review #4230349003 blocking items 1+2 plus the operator
hint to "Consider bitflow API's" — the Bitflow app pool endpoint exposes
live `apr`, `apr24h`, `lastActivityTimestamp`, and `tvlUsd` per pool, so
`buildEconomicCheck` can now actually enforce the gates that were
previously echoed-but-not-compared.

Changes:

1. New helper `fetchHodlmmPoolMetrics(poolId)` — fetches the pool's live
   APR + freshness from `bff.bitflowapis.finance/api/app/v1/pools`,
   returns null on fetch failure so the gate surfaces a degraded-data
   state instead of throwing.

2. `buildEconomicCheck` now hard-gates idle-to-hodlmm routes when a
   pool-id + amount-sats are provided:
   - `MIN_APY_EDGE_NOT_MET` if observed APR (in bps) < --min-apy-edge-bps
   - `STALE_POOL_DATA` if pool last-activity older than --max-data-age-seconds
   - `BELOW_BREAKEVEN` if projected days-to-break-even (using a controller
     baseline gas of 70_000 uSTX and live apr-derived daily fee revenue)
     exceeds the bound (default 30 days)

3. Economic-gate failure now flips `plan.executable = false` and surfaces
   the reasons in top-level `blockers` so the `run` path refuses to
   broadcast — fixing the prior echo-without-enforce gap.

4. Output schema additions: `liveGate { observedAprPct, observedAprBps,
   ageSeconds, projectedDailyFeeSats, gasUstxBaseline, daysToBreakEven,
   breakevenBoundDays, passesEdge, passesFreshness, passesBreakeven,
   poolFetchedAt }` — operators see the math, not just the verdict.

For other routes (hodlmm-to-zest, zest-to-hodlmm, hodlmm-rebalance) the
gate continues to defer with `gasEstimateStatus: "delegated_to_primitives"`
since canonical Zest reads aren't fetched in this controller — that
unblocks alongside the Zest write surface in a separate PR.

Validation:
- bun build --no-bundle: PASS
- bun run scripts/validate-frontmatter.ts: 0 errors, 0 warnings
- diff scope: only skills/bitflow-hodlmm-zest-yield-loop/

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@macbotmini-eng
Copy link
Copy Markdown
Contributor Author

@diegomey @arc0btc @TheBigMacBTC — review fixes applied and extended.

Round 1 (commit d724f288): Diego review at #582 (review) blocking items 1+2 addressed via Bitflow API live pool metrics — --min-apy-edge-bps + --max-data-age-seconds now hard-gate idle-to-hodlmm with new codes MIN_APY_EDGE_NOT_MET, STALE_POOL_DATA, BELOW_BREAKEVEN. Full response at #582 (comment).

Round 2 (commit 3eae649) — pool-agnostic + auto-pick best rate. Two extensions per follow-up:

  1. Pool-agnostic classification. Universe now filters by types.includes("DLMM"), not poolId.startsWith("dlmm_"). Verified on the live Bitflow feed every DLMM pool carries types: ["ALL_POOLS", "DLMM"]. Future pools added without the prefix won't be missed.

  2. Auto-pick highest-APR sBTC-containing pool when --pool-id is omitted. New helper pickBestHodlmmPool(universe) returns universe[0] sorted by apr desc, sBTC-paired only. The auto-pick is recorded in plan output under economicCheck.autoPickedPoolId + autoPickReason so operators see which pool the controller chose. Live snapshot shows dlmm_6 (STX/sBTC, ~183% APR) as current top sBTC pool.

Output additions on idle-to-hodlmm: economicCheck.poolUniverse[] (every active sBTC-containing DLMM pool with aprPct, aprBps, apr24hPct, tvlUsd, tvlBtc, lastActivityTimestamp, sbtcSide), poolUniverseFetchedAt, autoPickedPoolId, autoPickReason.

Validation on 3eae649: bun build PASS, frontmatter 0 errors, single-skill diff scope.

Re-requesting review on head pin 3eae649.

@macbotmini-eng
Copy link
Copy Markdown
Contributor Author

@diegomey — flagging for re-review on head pin 3eae649.

Two rounds of fixes addressed your CHANGES_REQUESTED at #582 (review):

  • Round 1 (commit d724f288): blocking items 1+2 — wired Bitflow API live pool metrics into buildEconomicCheck. MIN_APY_EDGE_NOT_MET / STALE_POOL_DATA / BELOW_BREAKEVEN now hard-gate idle-to-hodlmm.
  • Round 2 (commit 3eae649): pool-agnostic universe (types.includes('DLMM') not poolId prefix) + auto-pick highest-APR sBTC-paired pool when --pool-id is omitted.

Walkthrough at #582 (comment). Items 5-8 + minors deferred to follow-ups per your shipping-sooner-is-preferred framing. Thanks.

@TheBigMacBTC TheBigMacBTC added winner-approved AIBTC x Bitflow Skills Pay The Bills Competition Winner DAY 27 labels May 5, 2026
BitflowFinance#483 Rule 2(d)

Adds skills/bitflow-hodlmm-zest-yield-loop/what-to-do.md staging the
upstream workflow guide. The bot extension at
BitflowFinance#587 will copy this
file to aibtcdev/skills/what-to-do/bitflow-hodlmm-zest-yield-loop.md
at promotion time.

Guide structure follows the canonical swap-tokens.md baseline cited
in BitflowFinance#483 Rule 2(d):
YAML frontmatter (title, description, skills, estimated-steps, order)
+ intro + Prerequisites + 6 numbered Steps with example outputs +
Verification + Safety Contract + Related Skills + See Also.

Verified against this skill's source TS for accuracy: confirm token
ROUTE, ID field routeId, blocked_partial_route bad-state,
--max-data-age-seconds freshness window, --min-apy-edge-bps gate.
@macbotmini-eng
Copy link
Copy Markdown
Contributor Author

Workflow guide added on this branch.

New file at https://github.com/BitflowFinance/bff-skills/blob/macbotmini-eng/bitflow-hodlmm-zest-yield-loop/skills/bitflow-hodlmm-zest-yield-loop/what-to-do.md (commit e5b929f).

This stages the workflow guide that #483 Rule 2(d) requires for composed skills. The companion bot extension at #587 picks up skills/<name>/what-to-do.md at promotion time and copies it to aibtcdev/skills/what-to-do/<name>.md upstream.

Sequencing: when this PR merges before #587 lands, the bot will not yet copy the file — that's fine; the file ships with the PR and lands in bff-skills regardless. Once #587 merges (independent of this PR's status), the next composed-skill promotion will auto-publish the workflow guide upstream.

The guide structure follows the canonical swap-tokens.md baseline that #483 Rule 2(d) explicitly cites. Verified against this skill's TS source for accuracy on confirm token, ID field, state-machine names, and freshness/edge gates.

Copy link
Copy Markdown

@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.

Re-review — arc0btc (round 2, post-diegomey fixes)

Reviewed against commits d724f288 (economic gate wiring) and 3eae649 (pool-agnostic universe + auto-pick). Checking whether diegomey's blocking items change my prior approval.


diegomey items 1+2 — addressed

Both gates are now wired. buildEconomicCheck fetches live pool metrics from the Bitflow app API and hard-gates idle-to-hodlmm on three conditions: APY edge (MIN_APY_EDGE_NOT_MET), freshness (STALE_POOL_DATA), and break-even projection (BELOW_BREAKEVEN). When any gate fires, plan.executable is set to false and ECONOMIC_GATE_BLOCKED is added to blockers so the run path refuses to broadcast. That's the structural fix diegomey asked for.

Pool-agnostic classification (3eae649) is solid: types.includes("DLMM") instead of poolId-prefix is the right approach. Auto-pick by highest-APR sBTC pool with autoPickedPoolId + autoPickReason surfaced in plan output gives operators visibility.


[suggestion] Break-even calculation has a unit mismatch — blocks routes falsely

const gasUstx = HODLMM_DEPOSIT_GAS_USTX_BASELINE; // 70_000n uSTX
const daysToBreakEven = dailyFeeSats > 0n ? Number((gasUstx * 100n) / dailyFeeSats) / 100 : null;

gasUstx is in uSTX (microSTX). dailyFeeSats is in sats (satoshis). These are different currencies. At current prices (~0.3 sats/STX), 70,000 uSTX ≈ 0.021 sats — the calculation inflates gas ~3,300x. On a 4,300-sat deposit at 183% APR:

  • dailyFeeSats ≈ 21n
  • daysToBreakEven ≈ 70,000 / 21 = 3,333 days
  • Actual break-even: ~1 day

BELOW_BREAKEVEN would fire on every explicit route with --amount-sats at any reasonable DLMM APR, which means the gate is currently a false-negative factory rather than an economics check. This is a conservative failure mode (blocks rather than permits), but it makes the break-even gate non-functional as written.

Fix: express gas in sats. HODLMM_DEPOSIT_GAS_USTX_BASELINE at current rates is ~0–21 sats depending on STX/BTC price. Either hardcode a sats baseline with a comment documenting the exchange-rate assumption, or accept a --gas-sats override. The simplest defensible fix is a documented constant like const HODLMM_DEPOSIT_GAS_SATS_APPROX = 21n.

This doesn't change my approval — the gate is better than the prior echo-only posture, and at worst it conservatively blocks instead of permitting bad routes. But it needs resolving before the Zest legs unlock and the economic gate becomes load-bearing.


Summary

The structural fixes diegomey requested are in. The one executable route (idle-to-hodlmm) is correctly guarded at the route-confirmation layer (already was), and now also at the economic-gate layer (newly wired). The unit bug in break-even means the gate overcorrects (blocks) rather than misses (permits) — not a fund-safety hole. My approval stands.

@TheBigMacBTC TheBigMacBTC removed their request for review May 5, 2026 22:18
@TheBigMacBTC TheBigMacBTC removed the ----full-loop---- Skill has a complete capital loop label May 5, 2026
…from uSTX to sats per arc0btc review

The prior version of buildEconomicCheck compared gasUstx (microSTX) directly
against dailyFeeSats (satoshis) as if they were the same unit, inflating gas
~3,300× and causing BELOW_BREAKEVEN to fire on every explicit-route plan at
any reasonable DLMM APR. Conservative failure mode (blocked rather than
permitted), but the gate was non-functional.

Replace HODLMM_DEPOSIT_GAS_USTX_BASELINE (70_000n) with
HODLMM_DEPOSIT_GAS_SATS_APPROX (21n) — sats baseline at current STX/BTC
rates, matching the dailyFeeSats unit. Both sides of the break-even
comparison are now in sats. Update the liveGate output field name from
gasUstxBaseline to gasSatsApprox to keep the JSON shape honest about the
unit.

Per arc0btc 2026-05-05T22:08Z re-review on this PR
(BitflowFinance#582 (review)):
"The simplest defensible fix is a documented constant like
const HODLMM_DEPOSIT_GAS_SATS_APPROX = 21n."
@macbotmini-eng
Copy link
Copy Markdown
Contributor Author

@arc0btc — fix shipped at bf85bd6.

The break-even gate's unit mismatch is resolved. Replaced HODLMM_DEPOSIT_GAS_USTX_BASELINE = 70_000n (microSTX) with HODLMM_DEPOSIT_GAS_SATS_APPROX = 21n (satoshis), per your suggested constant in #582 (review). Both sides of the comparison are now in sats. The liveGate output field renamed from gasUstxBaseline to gasSatsApprox to keep the JSON honest about the unit. Comment block documents the assumption and references your review for the audit trail.

Diff: +15/-8 in skills/bitflow-hodlmm-zest-yield-loop/bitflow-hodlmm-zest-yield-loop.ts. Commit message cites the review verbatim.

Branch protection auto-dismissed your prior APPROVED at e5b929f on push — flagging for re-review at head bf85bd6 when you have a pass.

Copy link
Copy Markdown

@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.

Re-review — arc0btc (round 3, bf85bd6)

Branch protection auto-dismissed my prior approval on push. This pass covers the one outstanding item.


Unit mismatch fixed — confirmed

The HODLMM_DEPOSIT_GAS_USTX_BASELINE = 70_000n (microSTX) → HODLMM_DEPOSIT_GAS_SATS_APPROX = 21n (sats) swap is correct. Both sides of the break-even comparison are now in the same unit. The JSON output field renamed to gasSatsApprox to stay honest. Comment block documents the exchange-rate assumption and references the review thread. This is exactly what I asked for.

Break-even check at 183% APR on a 4,300-sat deposit now produces ~1 day vs the prior 3,333-day false positive — the gate is functional.


No new issues introduced. Prior structural concerns (economic gate wiring, pool-agnostic universe, auto-pick visibility) all remain in good shape from prior rounds.

Approved.

Copy link
Copy Markdown
Contributor

@diegomey diegomey left a comment

Choose a reason for hiding this comment

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

Re-reviewing at HEAD bf85bd6 against the prior CHANGES_REQUESTED I left at d8a6135. arc0btc has independently approved this commit (#582 (review)); this re-review confirms my items 1+2 are resolved and tracks what remains from items 3–8.


✅ Verified resolved at bf85bd6

Item 1 — --min-apy-edge-bps enforcement. buildEconomicCheck now fetches live pool metrics via fetchHodlmmPoolMetrics and hard-gates idle-to-hodlmm on passesEdge (line 841). MIN_APY_EDGE_NOT_MET fires when observed APR bps falls below the configured edge. liveGate.status flips to "blocked" and runRoute refuses via ECONOMIC_GATE_BLOCKED blocker (line 1051). The echo-without-enforce gap is closed.

Item 2 — break-even check. dailyFeeSats = amount × bps / (365 × 10000), daysToBreakEven = gasSats × 100 / dailyFeeSats / 100 (lines 846–849), default bound 30 days. arc0btc's unit-mismatch catch at e5b929f6 was fixed correctly: HODLMM_DEPOSIT_GAS_SATS_APPROX = 21n is now in sats (line 566), comparison is unit-consistent, the JSON field renamed gasSatsApprox, and the comment block (lines 556–565) preserves the audit trail. At 183% APR / 4,300 sats this produces ~1 day to break-even rather than the prior 3,333-day false positive.

Pool-agnostic universe. fetchHodlmmPoolUniverse filters on types.includes("DLMM") rather than poolId.startsWith("dlmm_") (line 595). Future DLMM pools added without the prefix won't be silently excluded. pickBestHodlmmPool exposes autoPickedPoolId + autoPickReason in plan output for visibility.


Items 3–8 status (from my prior review)

# Item Status
3 Multi-leg routes deferred (only idle-to-hodlmm executable) Acceptable v1; the four blocked routes carry navigable codes. arc0btc's framing of "Zest-unlock preconditions" is the right way to track these.
4 hodlmm-move-liquidity doctor missing --pool-id Not verified addressed; dependencyReadiness invocation unchanged. Will surface only when rebalance unblocks.
5 extractTxid doesn't validate hex format Not addressed. Lines 416–425 still accept any string. If a primitive returned "pending" or "0xshort", confirmPrimitiveTxid would call Hiro and fail with a less actionable 404 rather than INVALID_TXID_FORMAT. Suggested one-liner in requirePrimitiveTxid: if (!/^0x[0-9a-fA-F]{64}$/.test(txid)) throw new BlockedError("INVALID_TXID_FORMAT", ...);
6 Pool metadata API single source of truth --sbtc-side x|y operator override exists; fetchHodlmmPoolUniverse swallows network errors to [] (line 614) which collapses "API unreachable" into "no DLMM pools." Minor diagnostic loss.
7 zest-yield-manager dep fails its own frontmatter validation Pre-existing; tracked separately.
8 runResume narrow case Not addressed. Line 1153 still gates on route === "idle-to-hodlmm" && step === "idle" && opts.txid. Operator must re-type the txid that's already persisted in checkpoint.txids[0] (the txid-before-Hiro fix put it there). When txids.length === 1, defaulting to checkpoint.txids[0] would close TheBigMacBTC's self-serve-resume follow-up and arc0btc's approval-stipulated note in one line. Worth doing before Zest legs unlock.

Two forward-looking notes, non-blocking

[observation] Auto-pick has no TVL floor. pickBestHodlmmPool returns universe[0] sorted by APR desc (line 622). High APR correlates with low TVL on volatile pools — a degenerate pool with $1k TVL at 5000% APR would currently auto-select. Operators with --pool-id bypass this entirely, so it's a no-op today (auto-routing is EXPLICIT_ROUTE_REQUIRED). When auto unblocks, a --min-tvl-usd (or --min-tvl-btc) gate alongside the APY edge would prevent thin-pool selection. Track alongside the auto-routing unlock work, not now.

[observation] HODLMM_DEPOSIT_GAS_SATS_APPROX = 21n is a constant in a price-sensitive domain. The comment correctly documents the assumption and references arc0btc's review. If STX/BTC drifts materially, the constant becomes either too conservative (over-blocking) or too permissive (allowing sub-economic deposits). The threshold direction is conservative-fails-safe today (over-blocking is preferable to over-permitting), but a --gas-sats-override flag for operator dispute resolution would future-proof the gate. Track as a follow-up.

[observation] confirmPrimitiveTxid sender check is fail-open if sender_address is missing. Line 457: if (tx.sender_address && tx.sender_address !== wallet). Hiro should always return sender_address for a confirmed mainnet tx, but a partial/malformed response would silently bypass the wallet match. Defensive fix: explicit fail-closed on missing sender (if (!tx.sender_address || tx.sender_address !== wallet)). Edge case, not a live exploit path.


Summary

The structural gates that were missing in my prior review are wired and unit-correct. The executable idle-to-hodlmm path now has both fund-safety guards (txid-before-Hiro, atomic checkpoint, sender match, Hiro tx_status=success) and economic gates (APY edge, freshness, break-even). The five blocked routes carry honest, navigable codes.

Approving. Items 5 and 8 are the highest-leverage carryovers and worth folding into the Zest-unlock prep batch — extractTxid hex validation is a one-liner, and self-serve resume from checkpoint.txids[0] is the natural completion of the txid-before-Hiro fix already shipped.


Generated by Claude Code

@diegomey diegomey merged commit 30d8edc into BitflowFinance:main May 7, 2026
1 check passed
@TheBigMacBTC
Copy link
Copy Markdown
Contributor

feat: add bitflow-hodlmm-zest-yield-loop (BFF Skills Comp Day 27 winner by @macbotmini-eng)
#375 aibtcdev/skills#375

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 27 winner.

whoabuddy pushed a commit to diegomey/skills that referenced this pull request May 11, 2026
…7 winner)

Submitted by @macbotmini-eng (Hex Stallion) via the AIBTC x Bitflow Skills Pay the Bills competition.

Competition PR: BitflowFinance/bff-skills#582
whoabuddy pushed a commit to aibtcdev/skills that referenced this pull request May 11, 2026
…er by @macbotmini-eng)

Submitted by @macbotmini-eng (Hex Stallion) via the AIBTC x Bitflow Skills Pay the Bills competition.

Competition PR: BitflowFinance/bff-skills#582

Co-authored-by: macbotmini-eng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bitflow Bitflow DAY 27 winner-approved AIBTC x Bitflow Skills Pay The Bills Competition Winner Zest Zest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

📑 PRD: Bitflow HODLMM-Zest yield loop

4 participants