Repository navigation
Conversation
v0-4-market is dead -- confirmed directly, not assumed: v0-market-vault.get-impl() returns v0-8-market exclusively on all 7 of Zest's per-asset vaults, zero exceptions, and v0-4-market's own most recent transactions all abort for exactly that reason (no longer the authorized impl). Adds the real signed Pyth Lazer price-feed requirement v0-8 has that v0-4 didn't: a new price-feed-buffer pre-flight (set-price-feed, same pattern as the existing set-target/set-slippage), threaded into all three liquidate() calls. Proved the format compatibility end-to-end before wiring it in: fetched a real signed update from Pyth's live stream, verified it against Zest's actual deployed decoder, recovered signer matched Zest's registered trusted pubkey exactly. liquidate()'s own auth doesn't need Zest's cooperation -- confirmed by reading the deployed source directly: contract-caller == tx-sender is an anti-proxy guard (already satisfied via as-contract throughout this contract), not an integrator whitelist, and the borrower is an explicit argument. Two bugs fixed along the way: - A real, pre-existing, already-flagged-in-Clarinet.toml type mismatch: Arkadiko's swap-x-for-y min-dy is a bare uint, not (optional uint). Dead code path (mode 2), never caught because the contract was never registered for clarinet check. - A let-binding evaluation-order issue the new price-feed parameter exposed: all let bindings run before any asserts, so an unauthorized caller with no price feed set got a misleading ERR-NO-PRICE-FEED instead of ERR-NOT-CORE. Restructured so contract-caller is checked before anything else touches state. - Added the core == FLASH-CORE defense-in-depth assert this contract was missing relative to alex-arb-receiver-v5 / bitflow-arb-receiver-v5 (caught because clarinet correctly flagged `core` as unused before the fix). 8 tests, mutation-verified the same way as bitflow-arb-receiver-v5's suite. zest-liquidation-receiver.clar (the OTHER receiver file) is untouched -- it targets a completely different, long-dead contract (SP2VCQJGH...pool-borrow-v2-3, 5 transactions ever, none successful since block 1,931,700), not a stale version of the right one. Flagging that as a separate decision rather than silently patching it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Adds modes 4-5 to zest-v2-liquidation-receiver.clar: same-asset liquidations (wSTX/wSTX via execute-stx-flash, sBTC/sBTC via a new execute-sbtc-flash entry point) need no swap at all, so no slippage and no swap-failure mode -- the simplest, lowest-risk liquidations to capture. Mode 5 needed real new surface (a new trait impl, a new flashstack-sbtc-core dependency), not just another branch, since same-asset-sBTC means borrowing sBTC directly rather than round-tripping through STX. These two modes are what zest-liquidation-receiver.clar (now deleted) was reaching for, properly this time: it targeted a long-dead Zest contract (SP2VCQJGH...pool-borrow-v2-3, 5 transactions ever, none successful since block 1,931,700) and had none of this contract's F-10 hardening -- no contract-caller check anywhere, all four repay paths sent to the caller-supplied `core` directly. Confirmed dormant before deleting (0 balance, is-approved-receiver false on both cores), so nothing needed rescuing, but it should never be re-approved or copied as a reference -- retired instead, along with its two dedicated scripts (deploy-zest-receiver.mjs, monitor-zest-liquidations.mjs), neither of which had anything salvageable: both are built around the dead contract's specific interface and pool-borrow-v2-3's own Aave-fork-style user-enumeration functions, which don't exist in v0-8-market's bitmask/asset-id data model. Also fixed the same anti-pattern in docs/INTEGRATION_GUIDE.md's own flash-liquidation example -- it was teaching exactly the caller-supplied repay-target shape that caused F-10, sourced from the file just deleted. 12 tests (up from 8), mutation-verified the new execute-sbtc-flash caller gate the same way as everything else this session. Full suite (288 tests) still green after the deletions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
All 5 modes called liquidate() with the raw underlying token (sbtc-token / wstx) as collateral-ft. Verified empirically against v0-market-vault's own get-collateral() across real registered accounts: every real sBTC/STX collateral position is held as a vault share (v0-vault-sbtc / v0-vault-stx), never the raw underlying -- the registry's collateral:true/false flags only establish structural possibility, not actual usage. liquidate()'s collateral-remove -> send-tokens call would have moved funds from an empty balance, seizing nothing on every real liquidation. Each mode now liquidates against the correct vault-share contract, captures the exact share amount from liquidate()'s own return value, and calls that vault's redeem() (wrapped in as-contract) to convert the shares into real spendable underlying before swapping or repaying. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
unixwhisperer
left a comment
There was a problem hiding this comment.
Thanks. The v0-8 migration and the vault-share handling look right to me. I checked the hardening against the live contracts. The answer to your email question is below, followed by one issue that needs fixing before this gets approved anywhere.
The price-feed parameter is fine
set-price-feed is owner-only (L455), and Zest's decoder verifies the signature and how old the update is, so a bad buffer just makes the transaction fail. Both callbacks check contract-caller against a hardcoded core (L185 and the sBTC equivalent), assert core, and repay to the hardcoded address. Nobody can drain the receiver by calling it directly.
Blocker: anyone can trigger the loan, and the sBTC→STX minimum output does nothing
I confirmed this by reading the code. I haven't simulated an attack.
- Anyone can start the loan. The live
flashstack-stx-core.flash-loanonly checks that the receiver is approved. Anyone can call it, with anyamount, andcontract-callerwill still be the core. The keeper in #102 sendsset-target,set-price-feedandflash-loanas three separate transactions. Once the first two have landed, a third party can fire the loan with your target and feed, at a moment they choose. - Wrong units on the minimum output (L235, L288, L331):
min-stx = sbtc-bal * (10000 - slip) / 10000. That's a number of sats, but Velar'samt-out-minis in output-token units (µSTX; checked against the liveuniv2-router). One sat is worth roughly 2,000 µSTX, so the minimum is effectively zero. - The only remaining check covers the whole balance (L242/294/337/363):
stx-bal >= owedreads the contract's entire balance. Combined with 1 and 2, someone can move the Velar sBTC/STX price around a loan they triggered and take the liquidation bonus. They can also take any profit sitting in the contract that hasn't been swept yet, because the check only makes sure the core is repaid. Same outcome as F-10 (a third party takes the receiver's funds), different route.
Suggested fix: port the bitflow-arb-receiver-v5 pattern from #96.
- An owner-set minimum output, in µSTX, for the sBTC→STX leg, cleared after use.
- A
min-profitcheck measured against the balance before the loan:stx-bal - owed >= balance-before + min-profit. - Bind the armed settings to one loan: store the expected
amountwith the target,asserts! (is-eq amount armed-amount), and clear the target after it's used, the same way the feed already is.
Modes 4–5 don't swap, so point 2 doesn't apply to them. Points 1 and 3 still do, though there's much less to take there.
Operations (about #102, noting it here because it shapes the deploy)
The keeper signs with DEPLOYER_MNEMONIC, and the receiver's owner is whoever deploys it. If that's SP20XD46…, the always-on bot holds the deployer key, which also controls rescue-stx on older receivers. I'd suggest deploying, then moving ownership to a dedicated keeper wallet with propose-owner / accept-ownership, keeping only gas money in it, and sweeping profits regularly. The Lazer token is low-risk by comparison: a leak means billing or rate-limit abuse, not lost funds.
Happy to re-review once the minimum-output and min-profit changes are in.
External review (unixwhisperer, PR #98) found: flashstack-stx-core.flash-loan has no caller restriction at all -- anyone can trigger it for any approved receiver, with any amount, once set-target/set-price-feed are visible on-chain. Combined with a keeper sending those as separate transactions, a third party could fire the loan themselves. The sBTC->STX swap's old minimum-output floor was computed in sats (the input token's units) instead of microSTX (what Velar's amt-out-min actually checks) -- effectively zero slippage protection. And the final balance check only verified the core got repaid, not that the liquidation was profitable -- letting a manipulated swap or an already-unswept balance mask a loss-making execution. Same outcome as F-10, a different route. Verified each claim independently before fixing, same discipline as the rest of this session: - Read flashstack-stx-core's deployed flash-loan: confirmed zero caller-identity checks. - Read Velar's deployed univ2-router: confirmed amt-out-min is checked against the real output amount in the output token's own units. Fix, porting the bitflow-arb-receiver-v5 pattern: - armed-amount: set alongside the target via set-target's new 4th argument, asserted equal to the real flash-loan amount in both callbacks (ERR-AMOUNT-MISMATCH otherwise), reset after use. - min-stx-out: owner-set in the correct units, required nonzero for the swap modes, reset after use. - min-profit-stx / min-profit-sbtc: the final check now captures this transaction's own starting balance first and requires the ending balance to exceed it by fee + min-profit, independent of any stale leftover balance already sitting in the contract. Second, independent finding while wiring the keeper bot's live Velar quote (verified directly against chain state, not assumed): VELAR-POOL-SBTC-STX (u70) is NOT the sBTC/STX pool -- it's an unrelated wstx/meme-coin pool. Scanned all 87 Velar pools: the only wstx-paired pool with "sbtc" in its name (id 60) trades a different sBTC contract than the one this receiver actually redeems from Zest's vault. There is no known Velar route for the real sBTC this receiver holds. Modes 1-3 (every mode needing that swap) are now blocked at set-target (ERR-NO-SWAP-ROUTE) and defended again at execute-stx-flash, rather than left reachable against a route that can't work. Their swap-logic scaffold (now with the fixes above applied) is left in place for whenever a real route is found. The same wrong u70 assumption is also hardcoded in contracts/velar-sbtc-arb-receiver.clar and contracts/hk-sbtc-real-receiver-v1.clar -- flagged separately, not fixed here. Only modes 4/5 (no swap, no DEX dependency) are live now. Keeper bot and scanner updated to match: MODE_MAP only proposes 4/5, set-target's call includes the new amount argument, sizeLoan() simplified now that it's always exact. clarinet check clean (226 contracts), 15/15 tests passing (one new test for ERR-NO-SWAP-ROUTE, others updated for the new set-target signature and tuple field ordering). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
External review (unixwhisperer) found: flashstack-stx-core.flash-loan has no caller restriction at all -- anyone can trigger it for any approved receiver, with any amount, once set-target/set-price-feed are visible on-chain. The sBTC->STX swap's old minimum-output floor was computed in sats (the input token's units) instead of microSTX (what Velar's amt-out-min actually checks) -- effectively zero slippage protection. And the final balance check only verified the core got repaid, not that the liquidation was profitable. Same outcome as F-10, a different route. Verified each claim independently before fixing: read flashstack-stx-core's deployed flash-loan (confirmed zero caller-identity checks) and Velar's deployed univ2-router (confirmed amt-out-min is checked in the output token's own units). Fix, porting the bitflow-arb-receiver-v5 pattern: - armed-amount: set alongside the target via set-target's new 4th argument, asserted equal to the real flash-loan amount in both callbacks (ERR-AMOUNT-MISMATCH otherwise), reset after use. - min-stx-out: owner-set in the correct units, required nonzero for the swap modes, reset after use. - min-profit-stx / min-profit-sbtc: the final check now captures this transaction's own starting balance first and requires the ending balance to exceed it by fee + min-profit, independent of any stale leftover balance already sitting in the contract. Second, independent finding while wiring up the keeper bot's live Velar quote (PR #102), verified directly against chain state: VELAR-POOL-SBTC-STX (u70) is NOT the sBTC/STX pool -- it's an unrelated wstx/meme-coin pool. Scanned all 87 Velar pools: the only wstx-paired pool with "sbtc" in its name (id 60) trades a different sBTC contract than the one this receiver actually redeems from Zest's vault. There is no known Velar route for the real sBTC this receiver holds. Modes 1-3 are now blocked at set-target (ERR-NO-SWAP-ROUTE) and defended again at execute-stx-flash, rather than left reachable against a route that can't work. Their swap-logic scaffold (now with the fixes above applied) is left in place for whenever a real route is found. The same wrong u70 assumption is also hardcoded in contracts/velar-sbtc-arb-receiver.clar and contracts/hk-sbtc-real-receiver-v1.clar -- flagged separately, not fixed here. Only modes 4/5 (no swap, no DEX dependency) are live now. clarinet check clean (226 contracts), 15/15 tests passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for the review, @unixwhisperer — both findings confirmed independently before fixing, same discipline as the rest of this work:
Fixed, porting the
Also found, independently, while wiring the keeper bot's live Velar quote (not something you flagged, but came up verifying the swap mechanics): Re the ops note on #102 — agreed, and noted there: deploy under the current deployer, then
|
unixwhisperer
left a comment
There was a problem hiding this comment.
Re-reviewed at 272cdb2. All three points are fixed:
armed-amountis bound to the actual loan amount and reset after use (both callbacks), andtarget-modeis cleared, so an armed target can only be consumed once.min-stx-outis owner-set in µSTX, required to be nonzero on the swap paths, and reset after use.- The profit check is correct:
balance-beforeis read inside the callback, after the core's transfer, sostx-bal >= balance-before + fee + min-profitmeans the post-repayment balance is at least the starting balance plusmin-profit. Unswept leftovers no longer count toward it. The same holds for the sBTC path.
Good catch on u70. Disabling modes 1–3 at set-target and again in the callback is the right call until a real route exists. The same pool ID in velar-sbtc-arb-receiver and hk-sbtc-real-receiver-v1 should get its own tracking item.
Ran tests/zest-v2-liquidation-receiver-v0-8.test.ts from a clean worktree at this head: 15/15 pass. Dependency Audit fails here only because of the known #94 issue.
Approving. The ownership handoff to a dedicated keeper wallet at deploy time still applies, as agreed.
Summary
v0-4-marketis dead — confirmed directly:v0-market-vault.get-impl()returnsv0-8-marketexclusively on all 7 of Zest's per-asset vaults, zero exceptions, andv0-4-market's own most recent transactions all abort for exactly that reason (no longer the authorized impl).zest-v2-liquidation-receiver.clarontov0-8-market, and adds the real signed Pyth Lazer price-feed requirement v0-8 has that v0-4 didn't: aset-price-feedpre-flight (same pattern as the existingset-target/set-slippage), threaded into everyliquidate()call.liquidate()'s own auth doesn't need Zest's cooperation — confirmed by reading the deployed source directly:contract-caller == tx-senderis an anti-proxy guard (already satisfied viaas-contractthroughout this contract), not an integrator whitelist, and the borrower is an explicit argument.execute-sbtc-flashentry point (new trait impl, newflashstack-sbtc-coredependency) since same-asset-sBTC means borrowing sBTC directly, not round-tripping through STX.zest-liquidation-receiver.clar(deleted) — it targeted a long-dead Zest contract (SP2VCQJGH...pool-borrow-v2-3, 5 transactions ever, none successful since block 1,931,700) and had none of this contract's F-10 hardening (nocontract-callercheck anywhere, all four repay paths sent to the caller-suppliedcoredirectly). Confirmed dormant before deleting (0 balance,is-approved-receiver = falseon both cores) — nothing to rescue, but it should never be re-approved or copied as a reference. Its two dedicated scripts (deploy-zest-receiver.mjs,monitor-zest-liquidations.mjs) are deleted too — both built around the dead contract's interface andpool-borrow-v2-3's own Aave-fork-style user-enumeration functions, which don't exist in v0-8-market's bitmask/asset-id data model. Modes 4-5 above are what it was reaching for, done properly this time.docs/INTEGRATION_GUIDE.md's own flash-liquidation example — it was teaching exactly the shape that caused F-10, sourced from the file just deleted.Bugs fixed along the way
Clarinet.toml's own comments but never fixed: Arkadiko'sswap-x-for-ymin-dyis a bareuint, not(optional uint). Dead code path (mode 2), never caught because the contract was never registered forclarinet check.let-binding evaluation-order issue the new price-feed parameter exposed: allletbindings run before anyasserts!, so an unauthorized caller with no price feed set got a misleadingERR-NO-PRICE-FEEDinstead ofERR-NOT-CORE. Restructured socontract-calleris checked before anything else touches state.core == FLASH-COREdefense-in-depth assert this contract was missing relative toalex-arb-receiver-v5/bitflow-arb-receiver-v5— caught because Clarinet correctly flaggedcoreas unused before the fix.Critical fix, added after this PR was opened
All 5 modes called
liquidate()with the raw underlying token(
sbtc-token/wstx) ascollateral-ft. Caught before merge bycross-checking the registry's
collateral:true/falseflags against realon-chain state, not trusting the flags alone — querying
v0-market-vault.get-collateral()directly across real registeredaccounts shows 100% of real sBTC/STX collateral is actually held as a
vault share (
v0-vault-sbtc/v0-vault-stx, the ERC4626-styletokenized-vault contracts), never the raw underlying. The registry flag
only establishes that the raw underlying is structurally allowed as
collateral somewhere in the protocol — it says nothing about what real
positions actually use.
Passing the raw underlying as
collateral-ftwould have madeliquidate()'s owncollateral-remove -> send-tokenscall move fundsfrom an empty balance on every real liquidation attempt — this contract,
as originally migrated in this PR, could never have successfully
liquidated a real position.
Fix: every mode now calls
liquidate()against the correct vault-sharecontract, captures the exact share amount from
liquidate()'s own returnvalue (
(get collateral liq-result)), then calls that vault'sredeem()(wrapped in
as-contract, same pattern as every other self-call in thiscontract) to convert the seized shares into real spendable underlying
before swapping (modes 1-3) or repaying directly (modes 4-5). Added
ERR-REDEEM-FAILED(u911) for the new failure mode.clarinet checkre-run clean after the fix — 226 contracts, zeroerrors (the
(get collateral liq-result)field access and theredeem()argument types both type-check, which would have caughta mismatch against
liquidate()'s real return shape orredeem()'sreal signature)
tests/zest-v2-liquidation-receiver-v0-8.test.tsre-run: still12/12. Not expanded for this specific fix — the suite's own header
already documents why:
FLASH-CORE/FLASH-SBTC-COREare hardcodedmainnet addresses, so every test path hits the unforgeable
contract-callergate before ever reaching the collateral-ft/redeemlogic deeper in the function body, same structural limitation that
already applied before this fix.
Critical fix #2: front-running/slippage gap + modes 1-3 disabled (external review)
Found by @unixwhisperer's review of this PR:
flashstack-stx-core.flash-loanhas no caller restriction at all -- confirmed directly against its deployed
source. Anyone can trigger it for any approved receiver, with any amount,
the moment
set-target/set-price-feedare visible on-chain (the keeperin #102 sends them as three separate transactions). The sBTC->STX swap's
minimum-output floor was computed in sats (the input token's units)
instead of microSTX (what Velar's
amt-out-minactually checks, confirmedagainst the live
univ2-router) -- effectively zero slippage protection.The final balance check only verified the flash loan got repaid, not that
the liquidation was profitable. Combined, a third party could trigger the
loan, move the Velar price around their own trigger, and take the
liquidation bonus (and any unswept profit already in the contract) while
the core still got repaid cleanly -- same outcome as F-10, a different
route.
Fixed, porting the
bitflow-arb-receiver-v5pattern:armed-amount--set-target's new 4th argument, asserted equal to thereal flash-loan amount in both callbacks (
ERR-AMOUNT-MISMATCHotherwise), reset after use.
min-stx-out-- owner-set, correct units, required nonzero for the swapmodes, reset after use.
min-profit-stx/min-profit-sbtc-- captures this transaction's ownstarting balance first, requires the ending balance to exceed it by
fee + min-profit, independent of any stale leftover balance.Second, independent finding (not part of the review -- surfaced while
wiring the keeper bot's live Velar quote, verified directly against chain
state):
VELAR-POOL-SBTC-STX(u70) is NOT the sBTC/STX pool. It's anunrelated wstx/meme-coin pool. Scanned all 87 Velar pools directly: the
only wstx-paired pool with "sbtc" in its name (id 60) trades a different
sBTC contract than the one this receiver actually redeems from Zest's
vault. There's no known Velar route for the real sBTC this receiver
holds, so modes 1-3 (the only modes needing that swap) could never have
executed, independent of the slippage fix above. They're now blocked at
set-target(ERR-NO-SWAP-ROUTE) and again atexecute-stx-flashasdefense in depth, rather than left reachable against a route that
doesn't work -- their swap-logic scaffold (now with the fixes above
applied) is left in place for whenever a real route is found. The same
wrong
u70assumption is also hardcoded incontracts/velar-sbtc-arb-receiver.clarandcontracts/hk-sbtc-real-receiver-v1.clar-- flagged to Hillaryseparately, not fixed here.
Only modes 4/5 (no swap, no DEX dependency) are live right now.
clarinet checkre-run clean -- 226 contracts, zero errorstests/zest-v2-liquidation-receiver-v0-8.test.tsre-run: 15/15(one new test for
ERR-NO-SWAP-ROUTE, others updated for the newset-targetsignature)deployed; the Velar pool-id finding and the armed-amount/min-profit
checks are each independently verified against real chain state and
the existing test suite, but not against a real liquidation)
Not included
Test plan
clarinet check— clean, zero errors, 226 contracts (v0-8-market's full dependency chain — the Lazer oracle stack,market-trait-v7, vault contracts — resolved automatically as Clarinet requirements, no manual cascade needed)tests/zest-v2-liquidation-receiver-v0-8.test.ts— 12/12 passing: direct-call rejection on bothexecute-stx-flashand the newexecute-sbtc-flash, rejection even via this project's own local core copies, mode-range validation (1-5),set-target/set-price-feed/set-slippageownership gating, two-step ownership transfer, sweep functions,get-targetstate🤖 Generated with Claude Code