Critical C-01: Inverted Admin Fee Logic — All Regular Users Pay Zero Fees

Location: swap-x-for-y (lines 331–342), swap-y-for-x (lines 467–478)

Description: The comment says “Admins pay no fees on swaps” but the conditional branches are inverted. When tx-sender IS an admin (is-some returns true), the code assigns the full buy-fees/sell-fees. When NOT an admin, it assigns admin-swap-fees which is initialized to all zeros.

;; Admins pay no fees on swaps
(swap-fee-lps (if (is-some (index-of (var-get admins) tx-sender))
    (get lps (var-get buy-fees))        ;; Admin → full fees (WRONG)
    (get lps (var-get admin-swap-fees))  ;; Non-admin → zero fees (WRONG)
))

Impact: Every non-admin swap executes with zero fees. The protocol earns no swap revenue. LPs receive no fee income from swaps. Stacking DAO and Bitflow protocol receive zero fee distributions. This is a direct and total loss of protocol revenue.

;; Exploit test for C-01: Regular users pay zero fees
(define-public (test-exploit-c01-zero-fees)
  (let
    (
      ;; admin-swap-fees defaults to {lps: u0, stacking-dao: u0, bitflow: u0}
      ;; Non-admin users hit the else branch, getting zero fees
      ;; Result: full swap amount passes through with no fee deduction
      (fee-check (var-get admin-swap-fees))
    )
    ;; Verify all fee components are zero for non-admins
    (asserts! (is-eq (get lps fee-check) u0) (err u1))
    (asserts! (is-eq (get stacking-dao fee-check) u0) (err u2))
    (asserts! (is-eq (get bitflow fee-check) u0) (err u3))
    (ok true)
  )
)

Recommendation: Swap the conditional branches:

(swap-fee-lps (if (is-some (index-of (var-get admins) tx-sender))
    (get lps (var-get admin-swap-fees))  ;; Admin → zero/reduced fees
    (get lps (var-get buy-fees))          ;; Non-admin → standard fees
))

High H-01: No Fee Bounds Validation — Admins Can Set Arbitrarily High Fees

Location: change-buy-fee, change-sell-fee, change-admin-swap-fee, change-liquidity-fee

Description: All fee-setting functions accept any uint value with no upper bound validation. An admin can set combined fees above 10,000 bps (100%).

(define-public (change-buy-fee (new-lps-fee uint) (new-protocol-fee uint) (new-bitflow-fee uint))
    (let ((current-admins (var-get admins)))
        (asserts! (is-some (index-of current-admins tx-sender)) (err "err-not-admin"))
        ;; No bound check!
        (ok (var-set buy-fees {lps: new-lps-fee, stacking-dao: new-protocol-fee, bitflow: new-bitflow-fee}))
    )
)

Impact: Fees >100% cause arithmetic underflow in swap functions, reverting all transactions. A compromised or malicious admin could DoS the entire pool. Even without malice, an accidental misconfiguration could freeze swaps.

Recommendation: Add bounds checking:

(asserts! (<= (+ new-lps-fee new-protocol-fee new-bitflow-fee) u10000) (err "err-fee-too-high"))

High H-02: get-dy Uses Unscaled Amount for Total Fee Calculation (Precision Mismatch)

Location: get-dy (line 249)

Description: The individual fee calculations correctly use x-amount-scaled, but x-amount-total-fees-scaled uses the raw unscaled x-amount. This value is then subtracted from the scaled amount to compute updated-x-amount-scaled.

;; Individual fees use scaled amount (correct):
(x-amount-fees-lps-scaled (/ (* x-amount-scaled swap-fee-lps) u10000))
(x-amount-fees-stacking-dao-scaled (/ (* x-amount-scaled swap-fee-stacking-dao) u10000))
(x-amount-fees-bitflow-scaled (/ (* x-amount-scaled swap-fee-bitflow) u10000))

;; Total fee uses UNSCALED amount (BUG):
(x-amount-total-fees-scaled (/ (* x-amount total-swap-fee) u10000))
(updated-x-amount-scaled (- x-amount-scaled x-amount-total-fees-scaled))

Impact: For pairs where x-decimals < y-decimals, the fee subtracted is much smaller than intended, resulting in inflated dy quotes. For STX/stSTX (both 6 decimals), scaling is identity so there is no practical impact on this specific pool. However, the actual swap-x-for-y function computes fees individually (correctly), so quotes diverge from execution.

Recommendation: Use x-amount-scaled consistently:

(x-amount-total-fees-scaled (/ (* x-amount-scaled total-swap-fee) u10000))

Medium M-01: Newton-Raphson Solver Returns Zero on Non-Convergence

Location: get-x, get-y, get-D

Description: The iterative solvers use a converged field initialized to u0. If convergence is not achieved within 384 iterations, u0 is returned as the result.

(get converged (fold x-for-loop index-list
    {x: current-D, c: c2, b: b, D: current-D, converged: u0}))

Impact: Non-convergence returns zero, causing swap output to be zero. The min-amount assertion would catch this and revert. However, if get-D returns zero, the fee and share calculations in add-liquidity would divide by zero (runtime abort).

Recommendation: Return the last iteration value instead of zero on non-convergence, or add explicit convergence checks before using results.

Medium M-02: Staking Contract Permanently Locked After First Set

Location: set-staking-contract

Description: A boolean flag staking-and-rewards-contract-is-set prevents the staking contract from ever being changed after the first call.

(asserts! (not is-set) (err "err-staking-and-rewards-contract-already-assigned"))
(var-set staking-and-rewards-contract staking-contract)
(var-set staking-and-rewards-contract-is-set true)

Impact: If the staking/rewards contract needs migration (bug fix, upgrade), LP fee distributions are permanently locked to the original address. Other fee recipients (stacking-dao, bitflow) can be freely changed by admins.

Recommendation: Either allow admin updates (with timelock) or document this as an intentional immutability guarantee.

Medium M-03: Mutable Convergence Threshold Can Break AMM Math

Location: change-convergence-threshold, used in x-for-loop, y-for-loop, D-for-loop

Description: The convergence threshold is admin-mutable and used by all Newton-Raphson solvers. Setting it too high accepts wildly inaccurate results; setting it to u0 may prevent convergence entirely (returning zero per M-01).

(define-public (change-convergence-threshold (new-convergence-threshold uint))
    ;; No bounds check
    (ok (var-set convergence-threshold new-convergence-threshold))
)

Impact: A threshold of u1000000 could accept first-iteration approximations, leading to incorrect swap amounts and potential arbitrage extraction. A threshold of u0 causes all swaps to fail.

Recommendation: Bound to a safe range, e.g., u1 to u10.

Medium M-04: swap-y-for-x total-swap-fee Omits Bitflow Fee

Location: swap-y-for-x (line 480)

Description: The total-swap-fee variable in swap-y-for-x only sums LP and Stacking DAO fees, omitting Bitflow:

(total-swap-fee (+ swap-fee-lps swap-fee-stacking-dao))  ;; Missing: swap-fee-bitflow

Impact: Currently dead code — total-swap-fee is not used in the actual fee calculations (fees are applied individually). However, this indicates a copy-paste error and creates risk if the variable is used in future refactoring.

Recommendation: Add the missing term: (total-swap-fee (+ swap-fee-lps swap-fee-stacking-dao swap-fee-bitflow))

Low L-01: String Error Codes Instead of Uint Constants

Description: All error returns use string literals (e.g., (err "err-not-admin")) instead of uint error codes. String errors consume more block space and are harder to programmatically handle.

Recommendation: Use (define-constant ERR-NOT-ADMIN (err u1000)) pattern for gas efficiency and easier integration.

Low L-02: Amplification Coefficient Has No Bounds

Location: change-amplification-coefficient

Description: The amplification coefficient (A) can be set to any uint. A=0 degenerates to constant-product. Very high A values could cause overflow in the D calculation.

(define-public (change-amplification-coefficient (y-token ...) (lp-token ...) (amplification-coefficient uint))
    ;; No bounds check on amplification-coefficient
    (ok (map-set PairsDataMap ... {amplification-coefficient: amplification-coefficient}))
)

Recommendation: Enforce reasonable bounds (e.g., u1 to u5000) and consider implementing ramping (gradual changes) as done in Curve.

Info I-01: Clarity v2 — Uses as-contract Without Explicit Asset Allowances

Description: This contract uses Clarity v2 which only has as-contract (blanket authority). Clarity 4 introduced as-contract? with with-stx, with-ft, with-nft allowances that restrict exactly which assets the contract can move. Upgrading would provide defense-in-depth against potential token contract exploits.

Info I-02: Initial Pair Creation Requires Exactly Equal Scaled Balances

Location: create-pair

Description: The create-pair function asserts (is-eq initial-x-bal-scaled initial-y-bal-scaled), requiring exactly 1:1 initial deposits after decimal scaling. While appropriate for a stableswap pool (assets should be near parity), this prevents initial seeding at market-rate ratios if the peg has drifted.