stacking-pool-signer-v1
StackingDAO Pool Signer v1 — Security Audit
Pool signer contract — manages STX stacking delegation to PoX signers for the StackingDAO protocol
Summary
| Severity | Count |
|---|---|
| MEDIUM | 2 |
| LOW | 2 |
| INFO | 3 |
Architecture Overview
This contract is StackingDAO's pool signer — it manages the delegation of STX to PoX-4 signers on behalf of the stacking pool. The contract:
- Delegates STX from pool members to the PoX-4 stacking contract
- Manages signer keys, signatures, and aggregation commits per reward cycle
- Handles extend and increase operations for already-stacking delegates
- Provides admin functions for pool owner and DAO governance to manage signer configuration
Access control model: Three-way OR — pool-owner (mutable), any DAO-active contract via .dao, or the contract itself (via as-contract tx-sender). All admin and PoX wrapper functions use this same guard.
Key flow: prepare-stacking-dao → fetches delegate list from .data-pools-v1 → for each delegate: delegate-stack-stx (new) or extend+increase (existing) → aggregate-commit or aggregate-increase to the signer.
Findings
MEDIUM M-01: No timelock on critical admin state changes
Location: set-pool-owner, set-pox-reward-address
Description: The pool owner can instantly change both the pool owner address and the PoX reward address. A compromised pool-owner key allows an attacker to immediately redirect all future stacking rewards to their own address and lock out the legitimate owner.
(define-public (set-pox-reward-address (new-address { version: (buff 1), hashbytes: (buff 32) }))
(begin
(asserts! (or
(is-eq contract-caller (var-get pool-owner))
(is-eq true (contract-call? .dao get-contract-active contract-caller))
(is-eq contract-caller (as-contract tx-sender)))
(err ERR_UNAUTHORISED))
(var-set pox-reward-address new-address) ;; Immediate effect, no timelock
(ok true)))Impact: If the pool-owner private key is compromised, the attacker can redirect BTC stacking rewards and transfer ownership in a single transaction. Delegators' STX remains locked in stacking but rewards flow to the attacker for the duration of the cycle.
Recommendation: Implement a two-step ownership transfer pattern (propose + accept) and a timelock on reward address changes, giving delegators time to react. Alternatively, route all admin changes through the DAO governance exclusively.
MEDIUM M-02: Blanket asset access via pre-Clarity 4 as-contract
Location: All as-contract calls (lines throughout delegation, aggregation, PoX wrappers)
Description: The contract uses as-contract (pre-Clarity 4) which grants unrestricted asset transfer authority for the enclosed expression. While the contract currently only uses this authority for PoX stacking operations, any future code change or composability path could inadvertently transfer STX, FTs, or NFTs held by the contract.
;; Example: delegation function
(try! (as-contract (delegate-stack-stx delegate delegation-amount
(get-pox-reward-address) burn-block-height u1)))
;; Clarity 4 equivalent (recommended):
(try! (as-contract? (delegate-stack-stx delegate delegation-amount
(get-pox-reward-address) burn-block-height u1)
with-stacking))Impact: Low immediate risk (contract doesn't hold transferable assets), but increases the attack surface if the contract is upgraded or composed with other contracts that deposit assets.
Recommendation: Migrate to Clarity 4 and use as-contract? with explicit with-stacking allowance. This restricts the contract's authority to only stacking operations at the language level.
LOW L-01: Silent error swallowing for stacking threshold
Location: prepare-delegate-many
Description: The aggregation step silently ignores ERR_STACKING_THRESHOLD_NOT_MET (error 11). While this is intentional (small pools may not meet the minimum), it masks any other error with code 11 and provides no observability into how often the threshold is missed.
(match (aggregation)
success true
error (begin
(asserts! (is-eq error u11) (err error)) ;; Only error 11 is swallowed
true))Impact: If the threshold is persistently not met, delegators' STX sits idle without earning rewards, and there's no on-chain signal to alert operators.
Recommendation: Emit a print event when the threshold is not met so off-chain monitoring can detect and alert on the condition: (begin (print { action: "threshold-not-met", cycle: next-cycle }) true)
LOW L-02: No cycle validation in set-cycle-signer-info
Location: set-cycle-signer-info
Description: The admin function accepts any reward cycle value, including past cycles. Setting signer info for an already-completed cycle wastes gas and creates confusing map state.
Impact: Low — only the admin can call this, and past-cycle signer info is never read by the aggregation logic (which always uses next-cycle). However, it could lead to operator confusion.
Recommendation: Add a check: (asserts! (>= reward-cycle (+ (current-pox-reward-cycle) u1)) (err ERR_INVALID_CYCLE))
INFO I-01: Missing print events for admin state changes
Location: set-pool-owner, set-pox-reward-address, set-cycle-to-index
Description: Admin functions that modify critical state (pool-owner, pox-reward-address, cycle-to-index) do not emit print events. This makes off-chain monitoring and audit trails harder to maintain.
Recommendation: Add print statements to all admin functions, e.g.: (print { action: "set-pool-owner", old-owner: (var-get pool-owner), new-owner: owner })
INFO I-02: can-prepare has no upper bound
Location: can-prepare
Description: The timing check (> burn-block-height (- start-block-next-cycle withdraw-offset)) only enforces a lower bound — it doesn't prevent preparation after the next cycle has already started. In practice, PoX-4 will reject stale operations, but an explicit upper bound would fail faster with a clearer error.
(define-read-only (can-prepare)
(let (
(current-cycle (current-pox-reward-cycle))
(start-block-next-cycle (reward-cycle-to-burn-height (+ current-cycle u1)))
(withdraw-offset (contract-call? .data-core-v1 get-cycle-withdraw-offset))
)
(> burn-block-height (- start-block-next-cycle withdraw-offset))))Recommendation: Add: (and (> burn-block-height (- start-block-next-cycle withdraw-offset)) (< burn-block-height start-block-next-cycle))
INFO I-03: Excessive unwrap-panic usage in aggregation
Location: aggregation function
Description: The aggregation function calls (unwrap-panic (get pox-addr signer-info)) and similar patterns on fields of signer-info that is already validated as (is-some). While safe (the unwrap will never panic given the prior assert), using unwrap-panic is a code smell — if the logic is ever refactored and the assert is moved, the contract could abort entire transactions.
Recommendation: Use unwrap! with explicit error codes for defense-in-depth: (unwrap! (get pox-addr signer-info) (err ERR_MISSING_SIGNER_INFO))
Positive Patterns
- Clean PoX wrapper layer: All PoX-4 interactions are wrapped in private functions with consistent error conversion (
to-uint), making the contract easy to audit and upgrade. - Consistent access control: Every admin and public function uses the same three-way OR pattern, reducing the risk of missing an auth check.
- Delegation amount zero-check: The
delegationfunction correctly handles the case where a delegate has no active delegation, avoiding unnecessary PoX calls. - Extend-before-increase ordering: For already-stacking delegates, the contract correctly extends first (to ensure the stacker is active for the next cycle) before increasing the locked amount.
- Aggregation commit vs increase: Correctly uses
stack-aggregation-commit-indexedfor first-time commits andstack-aggregation-increasefor subsequent additions, tracking the reward index per cycle.
Trust Assumptions
.dao— DAO governance contract determines which contracts are "active" and authorized. Compromise of the DAO grants full control over this contract..data-pools-v1— Provides the delegate list. A corrupted delegate list could cause delegation to wrong principals..data-core-v1— Provides the withdraw offset timing parameter. An incorrect offset could prevent or premature-trigger preparation.pool-owner— Single key with full admin access. Key compromise = full control.
Independent audit by cocoa007.btc · Full audit portfolio · Not a professional security audit — see repo for methodology