Liquid stacking protocol — deposit STX, receive stSTX, withdraw via NFT receipt (v3 upgrade)
set-stack-fee and set-unstack-fee now assert <= DENOMINATOR_BPS (10000). Fixes v2's H-01 partially — cap is now 100%, not unlimited, but still unreasonably high.cancel-withdraw removed: Users can no longer cancel a pending withdrawal and recover stSTX. This is a significant UX change — once init-withdraw is called, the user is committed.(> burn-block-height unlock-burn-height) to (>= burn-block-height unlock-burn-height). Users can now withdraw exactly at the unlock block, not one block after..ststx-withdraw-nft → .ststx-withdraw-nft-v2.DENOMINATOR_6 and DENOMINATOR_BPS.withdraw now returns { stx-user-amount, stx-fee-amount } instead of just stx-amount.| Severity | Count |
|---|---|
| HIGH | 1 |
| MEDIUM | 2 |
| LOW | 2 |
| INFO | 2 |
StackingDAO v3 is a liquid stacking protocol on Stacks. Users deposit STX and receive stSTX (a fungible token). Withdrawals are two-phase: init-withdraw mints an NFT receipt and locks stSTX, then withdraw burns both and returns STX after a PoX cycle boundary. Unlike v2, there is no cancel-withdraw — withdrawals are irrevocable once initiated.
External contracts (reserve, commission, staking, direct-helpers) are passed as trait parameters and validated against a DAO registry via check-is-protocol.
migrate-ststx) is a one-time admin operationLocation: set-stack-fee, set-unstack-fee
Description: V3 added a fee cap that was missing in v2, but the cap is DENOMINATOR_BPS (10000 = 100%). A compromised DAO governance contract could set fees to 100%, causing users to lose their entire deposit or withdrawal amount.
(define-public (set-stack-fee (fee uint))
(begin
(try! (contract-call? .dao check-is-protocol contract-caller))
(asserts! (<= fee DENOMINATOR_BPS) (err ERR_WRONG_BPS))
;; fee can be u10000 = 100%
(var-set stack-fee fee)
(ok true)
)
)Impact: If governance is compromised, all new deposits can be fully drained to fee recipients. All pending withdrawals would lose 100% to fees.
Recommendation: Set a reasonable maximum fee constant, e.g. (define-constant MAX_FEE u500) (5%) and use that instead of DENOMINATOR_BPS in the assertion. The v1 contract had MAX_COMMISSION u2000 (20%) which was already generous — even that was removed in v2. V3 re-added a cap but at the useless 100% level.
as-contract grants blanket asset authorityLocation: Multiple: deposit, init-withdraw, withdraw, migrate-ststx
Description: The contract uses as-contract extensively to perform operations on behalf of the contract principal. In pre-Clarity 4, as-contract grants unrestricted access to all assets held by the contract. Any contract-called code executing within an as-contract block can move any asset the contract holds.
Impact: If a DAO-approved protocol contract (reserve, commission, staking, direct-helpers) is malicious or compromised, it could drain all contract-held assets during any as-contract call. The trait parameters are validated against the DAO registry, but governance compromise enables this attack.
Recommendation: Migrate to Clarity 4's as-contract? with explicit asset allowances (with-stx, with-ft, with-nft) to limit the blast radius of each privileged call.
Location: deposit, init-withdraw
Description: There is no minimum amount check on deposits or withdrawals. A user can deposit 1 micro-STX, which due to integer division would mint 0 stSTX (if stx-ststx ratio is high enough), effectively donating STX to the reserve. Conversely, tiny init-withdraw calls would mint NFTs with near-zero value, bloating state.
;; In deposit: if stx-user-amount * DENOMINATOR_6 < stx-ststx, ststx-amount = 0
(ststx-amount (/ (* stx-user-amount DENOMINATOR_6) stx-ststx))Impact: State bloat from dust withdrawal NFTs. Rounding-to-zero deposits donate STX without receiving stSTX. Low severity in practice since gas costs make this uneconomical on Stacks.
Recommendation: Add minimum amount assertions, e.g. (asserts! (>= stx-amount u1000000) (err ERR_MIN_AMOUNT)) for deposits and (asserts! (>= ststx-amount u1000000) (err ERR_MIN_AMOUNT)) for withdrawals.
cancel-withdraw removal is irreversible UX regressionLocation: Absent from v3 (was in v2)
Description: V2 allowed users to cancel a pending withdrawal before the unlock height, recovering their stSTX. V3 removes this entirely. Once a user calls init-withdraw, they are locked into waiting for the PoX cycle boundary — potentially weeks — with no way to change their mind.
Impact: Users who accidentally withdraw or who need liquidity before the unlock height have no recourse. The NFT is non-transferable via this contract (though the NFT contract itself may allow transfers).
Recommendation: Consider re-adding cancel-withdraw as a governance-controlled feature, or document prominently that withdrawals are irrevocable.
unwrap-panic on get-last-token-id in init-withdrawLocation: init-withdraw
Description: The NFT ID is retrieved using unwrap-panic. If the underlying NFT contract's get-last-token-id ever returns an error, the entire transaction will abort with no informative error code.
(nft-id (unwrap-panic (contract-call? .ststx-withdraw-nft-v2 get-last-token-id)))Impact: Poor debuggability if the NFT contract is in an unexpected state. No fund risk.
Recommendation: Use (unwrap! ... (err ERR_NFT_ID)) for better error reporting.
Location: deposit, init-withdraw, withdraw
Description: Integer division throughout the contract consistently rounds down:
stx-fee-amount rounds down (user pays less fee), ststx-amount rounds down (user gets fewer stSTX). Net effect: protocol gains from rounding.stx-amount rounds down from stSTX → STX conversion. User receives less.stx-fee-amount rounds down (less fee extracted). User gets slightly more.Impact: Negligible in practice — sub-micro-STX amounts. Standard behavior for integer-arithmetic DeFi protocols.
migrate-ststx burns from v1 and mints to v3 contractLocation: migrate-ststx
Description: The migration function burns stSTX held by .stacking-dao-core-v1 and mints the same amount to this contract (v3). This is a one-time admin operation gated by check-is-protocol. Note it references v1, not v2 — suggesting v2 migration may have already occurred or is handled separately.
Impact: Informational. Admin-only function, no user risk.
> to >= change in withdraw fixes a genuine off-by-one that forced users to wait an extra block.DENOMINATOR_6 and DENOMINATOR_BPS improves readability.| V2 Finding | Status in V3 |
|---|---|
| H-01: No fee cap | ⚠️ Partially fixed — cap added but at 100% |
| H-02: as-contract blanket authority | ❌ Not fixed — still pre-Clarity 4 |
| M-01: Rounding | ℹ️ Same behavior, now with named constants |
| M-02: cancel-withdraw race condition | ✅ Removed — function no longer exists |
| L-01: unwrap-panic | ❌ Still present in init-withdraw |
Audit by cocoa007.btc · Full audit portfolio