CRITICAL C-01: No MINIMUM_LIQUIDITY — First Depositor Inflation Attack

Location: calc-mint, mint

Description: UniswapV2 burns the first MINIMUM_LIQUIDITY (1000) LP tokens to a dead address to prevent the first depositor from manipulating the LP token price. This contract's calc-mint returns sqrti(amt0 * amt1) directly with no minimum subtraction.

(define-read-only
  (calc-mint (amt0 uint) (amt1 uint) (reserve0 uint) (reserve1 uint) (total-supply uint))
  (if (is-eq total-supply u0)
      (sqrti (* amt0 amt1))   ;; No MINIMUM_LIQUIDITY burned
      (min (/ (* amt0 total-supply) reserve0)
           (/ (* amt1 total-supply) reserve1))))

Impact: An attacker can: (1) Create a pool and deposit a tiny amount (e.g. 1 wei of each token), receiving 1 LP token. (2) Directly transfer (donate) a large amount of one token to the contract address. (3) The reserves don't update (no sync), but when the next user mints, the actual balance vs. tracked reserve divergence causes their LP tokens to round down to near-zero, effectively stealing their deposit. Alternatively, even without donation, the first depositor with 1 LP token controls 100% of the pool and can set an arbitrary initial price.

Recommendation: Burn a fixed MINIMUM_LIQUIDITY amount on first deposit, or require a minimum initial deposit size. Since Clarity lacks a burn address for LP tokens, mint the minimum to the contract itself or to a known dead principal.

HIGH H-01: Inconsistent Access Control — contract-caller vs tx-sender

Location: check-owner (line ~28), check-protocol-fee-to (line ~37)

Description: check-owner validates against contract-caller, which is correct — it prevents intermediary contracts from spoofing the owner. However, check-protocol-fee-to validates against tx-sender:

;; Owner check — uses contract-caller (correct)
(define-private (check-owner)
  (ok (asserts! (is-eq contract-caller (get-owner)) err-check-owner)))

;; Protocol fee check — uses tx-sender (inconsistent)
(define-private (check-protocol-fee-to)
  (ok (asserts! (is-eq tx-sender (get-protocol-fee-to)) err-auth)))

Impact: If the protocol-fee-to principal interacts through a proxy/router contract, tx-sender still resolves to the original signer, so collect works. But if a malicious contract tricks the protocol-fee-to address into calling it (which then calls collect), the check passes because tx-sender is the original signer. This is the opposite of the threat model check-owner protects against.

Recommendation: Use contract-caller consistently for all privileged operations, or document the design intent if tx-sender is deliberate for collect.

MEDIUM M-01: Mismatched Parentheses in Swap Fee Validation

Location: swap preconditions

Description: The swap precondition block has a subtle parenthesis grouping issue:

(or (is-eq (get num swap-fee) (get den swap-fee))
    (and (> amt-fee-lps u0))
         (or (is-eq (get num protocol-fee) (get den protocol-fee))
             (> amt-fee-protocol u0)))

The (and (> amt-fee-lps u0)) closes immediately — and with a single argument is identity. The protocol-fee or becomes a third argument to the outer or, not nested under and. The effective logic is: (or swap-fee-is-100% amt-fee-lps>0 protocol-fee-check). The intended logic was likely: (or swap-fee-is-100% (and amt-fee-lps>0 protocol-fee-check)).

Impact: The fee validation is weaker than intended. If amt-fee-lps > 0, the protocol fee check is bypassed. In practice this is mitigated because the k-invariant still holds and fees are bounded by anti-rug constraints, but it means the assertion doesn't catch edge cases the developer intended to guard against.

Recommendation: Fix parenthesization to match intended logic:

(or (is-eq (get num swap-fee) (get den swap-fee))
    (and (> amt-fee-lps u0)
         (or (is-eq (get num protocol-fee) (get den protocol-fee))
             (> amt-fee-protocol u0))))

MEDIUM M-02: No Slippage Protection in Mint

Location: mint

Description: The mint function accepts amt0 and amt1 but provides no min-liquidity parameter. The caller has no way to enforce a minimum number of LP tokens received.

Impact: A front-runner can sandwich a mint transaction — swapping to skew the price before the victim's mint, causing them to receive fewer LP tokens than expected, then swapping back afterward. This is a standard MEV/sandwich attack on AMM liquidity provision.

Recommendation: Add a min-liquidity parameter to mint and assert (>= liquidity min-liquidity). Alternatively, implement this in a router contract (common UniV2 pattern).

MEDIUM M-03: Pre-Clarity 4 as-contract — Unrestricted Asset Authority

Location: All as-contract usages (mint, burn, swap, collect)

Description: The contract uses as-contract (pre-Clarity 4) which grants blanket authority over all assets held by the contract principal. Any token trait passed to mint, burn, swap, or collect executes under the contract's full authority.

While the contract validates that trait principals match the stored pool data, the as-contract block itself has no language-level restriction on which assets can be moved. A vulnerability in the trait validation logic or a malicious token implementation could potentially move unrelated assets.

Impact: If a token contract's transfer implementation contains malicious logic, it executes with the AMM contract's full asset authority — potentially draining tokens from other pools.

Recommendation: Migrate to Clarity 4 and use as-contract? with explicit with-ft / with-stx allowances to restrict which assets each operation can move.

LOW L-01: No Timelock on Ownership Transfer

Location: set-owner

Description: Ownership can be transferred instantly in a single transaction. There is no two-step transfer (propose → accept) or timelock delay.

Impact: If the owner's key is compromised, the attacker can immediately transfer ownership and change all fee parameters (within anti-rug bounds). A timelock would give users time to exit.

Recommendation: Implement a two-step ownership transfer pattern: propose-owner + accept-owner, optionally with a timelock.

LOW L-02: Integer Rounding Favors Zero Fees on Small Swaps

Location: calc-swap

Description: Fee calculations use integer division which truncates:

(/ (* amt-in (get num swap-fee)) (get den swap-fee))

For very small amt-in values, amt-fee-total rounds to 0, meaning the swap is effectively fee-free. With default 998/1000, any amt-in < 500 yields zero fee.

Impact: Dust-sized swaps bypass fees entirely. Low practical impact since gas costs exceed any profit from fee-free dust swaps, but it represents a deviation from intended fee collection.

Recommendation: Accept as known behavior or add a minimum swap amount requirement.

INFO I-01: No Sync/Skim — Reserve Desynchronization Risk

Location: Contract-wide

Description: UniswapV2 provides sync() and skim() to handle cases where actual token balances diverge from tracked reserves (e.g., tokens sent directly to the contract). The contract acknowledges this limitation in comments, noting it's not implementable because all pools share a single contract principal.

Impact: Tokens sent directly to the contract address are permanently locked and cannot be recovered. Reserve tracking assumes all balance changes go through the contract's functions.

INFO I-02: Single Principal Holds All Pool Assets

Location: Architecture

Description: Unlike Ethereum's UniswapV2 where each pair is a separate contract, all pool reserves are held by the single univ2-core contract principal. This is an architectural consequence of Clarity's design.

Impact: A critical vulnerability in any pool operation could potentially affect all pools' assets. The blast radius of a single bug is the entire DEX TVL rather than a single pair. Post-conditions at the transaction level can mitigate this, but users must set them correctly.