fix(stacks-alpha-engine): post-#339 audit sweep — rebase of #346#367
Conversation
Squashed rebase of #346 (cliqueengagements). Original 18 commits replaced with single squash on current main + manifest regen. All audit-sweep content from the original PR preserved (+496/-105 lines across stacks-alpha-engine.ts/SKILL.md/AGENT.md). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e165e2e9a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return "dlmm_1"; | ||
| } | ||
| return "dlmm_1"; |
There was a problem hiding this comment.
The new borrow/repay commands are still funneled through inferTargetPoolId, which falls back to dlmm_1 for any non-deploy/migrate command, so _runPipeline applies HODLMM slippage/volume gates to Zest debt operations that do not touch DLMM at all. In practice this means borrow/repay can be refused when dlmm_1 volume is low or Bitflow pool data is unavailable, even though the Zest transaction itself is valid.
Useful? React with 👍 / 👎.
arc0btc
left a comment
arc0btc
left a comment
There was a problem hiding this comment.
Post-#339 audit sweep: adds borrow/repay commands to stacks-alpha-engine, fixes DLMM swap min-received calculation, corrects FT assetName bindings across protocols, and flips safety gates to fail-CLOSED. Each change is backed by on-chain proof — clean work.
What works well:
- The DLMM swap min-received fix is the right call. The old
Math.floor(amount * (1 - slip / 100))computed the floor in input-token units, which is meaningless to the router. Computing in expected output-token units is correct, andexpectedSwapOutputis a clean helper for it. - FT assetName corrections (
usdh-token→usdh,bridged-usdc→aeUSDC, etc.) match what the contracts actually emit — these were silent failures waiting to happen. - Granite redeem PC shape is now right. The old bindings (
lp-v1as principal,bridged-usdcas asset name) didn't match the real flow; anchoring tostate-v1+ 3-PC envelope (receive floor, receive cap, burn floor) is the correct pattern. The on-chain proof txids close the loop. - Fail-CLOSED on slippage, volume, and gas exceptions is a meaningful safety improvement. The old
catch { /* allow */ }pattern was a silent footgun. inferTargetPoolIdcorrectly routes guardian checks to the pool the operation will actually touch rather than always measuring dlmm_1. That's an important fix for Hermetica/Granite deploys.max-steps6 → 230 per the #339 audit recommendation.- migrate
--amountnow required. The old implicit sBTC-balance default was a silent foot-gun for stablecoin protocol migrations.
[suggestion] Guardian slippage/volume gates don't apply to borrow/repay (stacks-alpha-engine.ts borrow/repay pipeline, inferTargetPoolId)
borrow and repay don't touch any DLMM pool — they call zest_borrow / zest_repay directly against the Zest market contract. But inferTargetPoolId falls through to "dlmm_1" for these commands, so the Guardian will refuse a valid borrow if dlmm_1 happens to have high slippage or low 24h volume that day. The cooldown and gas gates make sense for borrow/repay; the HODLMM slippage and volume gates don't.
One fix: add borrow and repay to inferTargetPoolId and return a sentinel like "none", then skip the slippage/volume blocks in checkGuardian when targetPoolId === "none". Alternatively, extract a dedicated checkGuardianForDebt() that only runs price, gas, and cooldown gates.
[nit] validTokens_borrowRepay: Record<string, string[]> is a local const initialized inline as { zest: ["usdh"] } and only read once two lines later. A direct const validBorrowTokens = ["usdh"] (dropping the protocol nesting, since we've already asserted protocol === "zest" in the preceding guard) would be simpler and avoids the surprising mixed-case name.
Operational context: We run Zest sBTC supply/withdraw ops nightly. From MEMORY: borrow-helper-v2-1-7 is confirmed required for mainnet borrow operations — the USDh-only gate here matches what we've seen empirically. The on-chain proof txids for the full borrow/repay cycle at the top of SKILL.md are the right way to document this. The DLMM dual-pin post-condition pattern (willSendLte on caller + willSendGte on pool) matches what we validated from bff-skills#494.
The fail-CLOSED refactors alone are worth merging. The borrow/repay guardian issue is an improvement-not-blocker — borrow will still work most of the time; it'll just refuse on days dlmm_1 liquidity dips. Worth a follow-up if you want the gate to be purely debt-condition-aware.
There was a problem hiding this comment.
Pull request overview
Rebases and lands the post-#339 audit sweep for the stacks-alpha-engine skill, focusing on fund-safety hardening (post-conditions), more accurate guardian gating, and adding Zest USDh borrow/repay to support a documented leveraged-yield flow.
Changes:
- Hardened Bitflow DLMM swap safety (dual-pin allow-mode envelope, output-based min-received, higher max-steps) and improved Guardian pool targeting/fail-closed behavior.
- Added Zest
borrow/repaycommand surface (USDh-only) plus updated docs/agent guidance. - Regenerated
skills.jsonmanifest to include new commands.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| stacks-alpha-engine/stacks-alpha-engine.ts | Adds borrow/repay commands, refines guardian gating, updates DLMM swap post-conditions, and parameterizes HODLMM deploy by pool-id. |
| stacks-alpha-engine/SKILL.md | Updates surfaced command list and expands operational docs (proofs + leveraged-yield pattern + post-condition rationale). |
| stacks-alpha-engine/AGENT.md | Updates agent operational rules to match the new Zest borrow/repay scope and restrictions. |
| skills.json | Manifest regeneration to include new commands and updated generated timestamp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **Zest USDh borrow**: [`0x2b465aae…`](https://explorer.hiro.so/txid/0x2b465aae05812d25e4f52799b5f2882b21ca411d892359aba5157dba85d1162a?chain=mainnet) — 50M µUSDh borrowed against sBTC collateral via `v0-4-market.borrow` | ||
| - **Zest USDh repay**: [`0xd3b46ae7…`](https://explorer.hiro.so/txid/0xd3b46ae74b666af2e06a765d29e30bd2b0341507266827a2140cc4d9e6053fba?chain=mainnet) — full 50M µUSDh debt cleared via `v0-4-market.repay` |
There was a problem hiding this comment.
The on-chain proof bullets label USDh amounts as “µUSDh” (micro), but elsewhere (and in code) USDh is treated as 8-decimal. Using “µUSDh” implies 6 decimals and can lead to 100× mis-sized amounts. Suggest rewording these to “atomic USDh (8 decimals)” or “10^-8 units” (and similarly in the leveraged-yield example placeholders).
| - **Zest USDh borrow**: [`0x2b465aae…`](https://explorer.hiro.so/txid/0x2b465aae05812d25e4f52799b5f2882b21ca411d892359aba5157dba85d1162a?chain=mainnet) — 50M µUSDh borrowed against sBTC collateral via `v0-4-market.borrow` | |
| - **Zest USDh repay**: [`0xd3b46ae7…`](https://explorer.hiro.so/txid/0xd3b46ae74b666af2e06a765d29e30bd2b0341507266827a2140cc4d9e6053fba?chain=mainnet) — full 50M µUSDh debt cleared via `v0-4-market.repay` | |
| - **Zest USDh borrow**: [`0x2b465aae…`](https://explorer.hiro.so/txid/0x2b465aae05812d25e4f52799b5f2882b21ca411d892359aba5157dba85d1162a?chain=mainnet) — 50M atomic USDh (8 decimals) borrowed against sBTC collateral via `v0-4-market.borrow` | |
| - **Zest USDh repay**: [`0xd3b46ae7…`](https://explorer.hiro.so/txid/0xd3b46ae74b666af2e06a765d29e30bd2b0341507266827a2140cc4d9e6053fba?chain=mainnet) — full 50M atomic USDh (8 decimals) debt cleared via `v0-4-market.repay` |
| borrow --protocol zest --token usdh --amount <debt_micro_usdh> # take USDh debt (~7% APR) | ||
| deploy --protocol hermetica --token usdh --amount <debt_micro_usdh> # stake for 40% APY | ||
|
|
||
| # ---- earning ~33% positive carry on debt_micro_usdh while sBTC exposure preserved ---- |
There was a problem hiding this comment.
In the leveraged-yield example, the placeholders use <debt_micro_usdh> for USDh amounts, but the repo treats USDh as 8-decimal. Consider renaming the placeholder to reflect 10^-8 base units (or just “atomic units”) to avoid users passing 6-decimal-scaled values.
| borrow --protocol zest --token usdh --amount <debt_micro_usdh> # take USDh debt (~7% APR) | |
| deploy --protocol hermetica --token usdh --amount <debt_micro_usdh> # stake for 40% APY | |
| # ---- earning ~33% positive carry on debt_micro_usdh while sBTC exposure preserved ---- | |
| borrow --protocol zest --token usdh --amount <debt_atomic_usdh> # take USDh debt (~7% APR) | |
| deploy --protocol hermetica --token usdh --amount <debt_atomic_usdh> # stake for 40% APY | |
| # ---- earning ~33% positive carry on debt_atomic_usdh while sBTC exposure preserved ---- |
| // USDCx → aeUSDC (pool: aeUSDC/USDCx, selling Y for X) | ||
| if ((tokenIn === "usdcx" || tokenIn === "stx") && tokenOut === "aeusdc") { | ||
| return { | ||
| pool: "SM1FKXGNZJWSTWDWXQZJNF7B5TV5ZB235JTCXYXKD.dlmm-pool-aeusdc-usdcx-v-1-bps-1", | ||
| xToken: AEUSDC_TOKEN, yToken: USDCX_TOKEN, xForY: false, | ||
| inputSymbol: tokenIn, outputSymbol: tokenOut, | ||
| }; | ||
| } | ||
| // USDCx → USDh (pool: USDh/USDCx, selling Y for X) | ||
| if ((tokenIn === "usdcx" || tokenIn === "stx") && tokenOut === "usdh") { | ||
| return { | ||
| pool: "SM1FKXGNZJWSTWDWXQZJNF7B5TV5ZB235JTCXYXKD.dlmm-pool-usdh-usdcx-v-1-bps-1", | ||
| xToken: USDH_TOKEN, yToken: USDCX_TOKEN, xForY: false, |
There was a problem hiding this comment.
getDlmmSwapRoute currently treats tokenIn === "stx" as routable directly into the USDh/USDCx and aeUSDC/USDCx pools, but those pools don’t support STX as an input asset. As written, a deploy --protocol hermetica --token stx (or granite) path will build a swap that actually sends USDCx, causing an invalid/failed instruction. Either remove stx from these single-hop routes (and from the deploy validTokens list), or implement an explicit STX→USDCx hop (using an STX/USDCx pool) and emit a 2-step swap-simple-multi route.
| const guardianPools = await fetchBitflowPools().catch(() => [] as BitflowPoolData[]); | ||
| try { | ||
| const dlmm1 = guardianPools.find(p => p.poolId === "dlmm_1"); | ||
| if (dlmm1?.tokens) { | ||
| const pool1 = HODLMM_POOLS[0]; | ||
| const abr = await callReadOnly(pool1.contract, "get-active-bin-id", []); | ||
| const targetPool = guardianPools.find(p => p.poolId === targetPoolId); | ||
| if (targetPool?.tokens) { | ||
| const abr = await callReadOnly(targetPoolDef.contract, "get-active-bin-id", []); | ||
| if (abr.okay && abr.result) { | ||
| const binsData = await fetchJson<{ bins?: Array<{ bin_id: number; price?: string }>; active_bin_id?: number }>(`${BITFLOW_API}/api/quotes/v1/bins/dlmm_1`); | ||
| const binsData = await fetchJson<{ bins?: Array<{ bin_id: number; price?: string }>; active_bin_id?: number }>(`${BITFLOW_API}/api/quotes/v1/bins/${targetPoolId}`); | ||
| const activeBinId = binsData.active_bin_id ?? 0; | ||
| const activeBinData = binsData.bins?.find(b => b.bin_id === activeBinId); | ||
| if (activeBinData?.price) { | ||
| const binPrice = parseFloat(activeBinData.price); | ||
| const hodlmmPriceUsd = (binPrice / PRICE_SCALE) * Math.pow(10, dlmm1.tokens.tokenX.decimals - dlmm1.tokens.tokenY.decimals); | ||
| const marketPrice = dlmm1.tokens.tokenX.priceUsd; | ||
| const hodlmmPriceUsd = (binPrice / PRICE_SCALE) * Math.pow(10, targetPool.tokens.tokenX.decimals - targetPool.tokens.tokenY.decimals); | ||
| const marketPrice = targetPool.tokens.tokenX.priceUsd; | ||
| if (marketPrice > 0) { | ||
| slippagePct = round(Math.abs(hodlmmPriceUsd - marketPrice) / marketPrice * 100, 4); | ||
| slippageOk = slippagePct <= MAX_SLIPPAGE_PCT; | ||
| if (!slippageOk) refusals.push(`Slippage ${slippagePct}% > ${MAX_SLIPPAGE_PCT}% cap`); | ||
| if (!slippageOk) refusals.push(`Slippage ${slippagePct}% > ${MAX_SLIPPAGE_PCT}% cap on ${targetPoolId}`); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The slippage gate only fails-closed on thrown errors. If fetchBitflowPools() returns an empty list or the targetPoolId entry is missing/has no tokens, the code silently skips the slippage check (leaving slippageOk=true) and may allow writes without measuring price drift. Consider treating “pool not found / tokens missing” as an unavailable slippage check and refusing (similar to the catch-path), and likewise distinguishing “pool data unavailable” from a real $0 volume reading.
| const from = opts.from as Protocol; | ||
| const to = opts.to as Protocol; | ||
| instructions.push(...buildWithdrawInstructions(from, scout)); | ||
| const token = opts.token ?? inferToken(to); |
There was a problem hiding this comment.
In the migrate executor path, const token = opts.token ?? inferToken(to); is passed through to buildDeployInstructions without normalization. If a user supplies --token with different casing (e.g. USDH), the downstream comparisons (token === "usdh", route lookup keys, etc.) won’t match and the migrate will take the wrong branch. Normalize opts.token to lowercase (and ideally validate it against the target protocol’s allowed tokens) the same way the deploy path does.
| const token = opts.token ?? inferToken(to); | |
| const token = (opts.token?.toLowerCase()) ?? inferToken(to); |
| .requiredOption("--protocol <name>", "Protocol: zest") | ||
| .requiredOption("--token <symbol>", "Borrow asset (Zest: usdh)") | ||
| .requiredOption("--amount <value>", "Amount in smallest unit (µUSDh for usdh; USDh has 8 decimals)") | ||
| .option("--confirm", "Execute the transaction (without this flag, outputs a dry-run preview)") | ||
| .action(async (opts: Record<string, string>) => { | ||
| const result = await runPipeline(opts.wallet, "borrow", opts); | ||
| console.log(JSON.stringify(result, null, 2)); | ||
| if (result.status !== "ok") process.exit(1); | ||
| }); | ||
|
|
||
| program | ||
| .command("repay") | ||
| .description("Repay a borrowed Zest debt asset") | ||
| .requiredOption("--wallet <address>", "Stacks wallet address (SP...)") | ||
| .requiredOption("--protocol <name>", "Protocol: zest") | ||
| .requiredOption("--token <symbol>", "Repay asset (Zest: usdh)") | ||
| .requiredOption("--amount <value>", "Amount in smallest unit (µUSDh for usdh)") | ||
| .option("--confirm", "Execute the transaction (without this flag, outputs a dry-run preview)") |
There was a problem hiding this comment.
The borrow/repay CLI help strings describe the smallest USDh unit as “µUSDh”, but the code/config treats USDh as having 8 decimals, so the smallest unit is 1e-8 (not micro/1e-6). Please adjust the wording to avoid implying 6 decimals (e.g., “atomic units (10^-8)” or similar) so users don’t pass incorrectly-scaled amounts.
Summary
Rebased version of #346 (cliqueengagements) onto current main. Same content, single squashed commit.
Why a fresh PR
#346 was 18 commits behind main with cascading skills.json conflicts on each commit during the rebase chain (each commit touched the manifest, and main moved a lot since Apr 21). Rather than walk through 18 separate
take-theirs + regenresolutions, took the cleaner approach: reset to current main, cherry-pick thestacks-alpha-engine/directory contents from the PR's tip in one shot, regen manifest. Same end-state, single clean commit.Per the established Wave 2 pattern (#427→#490 etc.) for forks where rebase-on-PR is impractical or would obscure history.
Diff
stacks-alpha-engine/stacks-alpha-engine.ts— main audit-sweep changesstacks-alpha-engine/SKILL.md— doc updatesstacks-alpha-engine/AGENT.md— agent guidance updatesskills.json— regen for manifestTotal: +496/-105 lines (matches the original PR's +503/-105 closely; small delta is just from the manifest format).
Closes #346.
Test plan
bun run typecheckcleanbun run manifestclean (95 skills)🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com