🔍 Velar DEX: univ2-core

Location: swap function, preconditions block (~line 420)

Description: The swap precondition block contains a mismatched parenthesisation that weakens the fee validation logic. The intended logic appears to be: "if swap-fee is not 100%, then LP fees must be positive AND protocol fee check must pass." However, the actual parsed structure is an or with three independent branches:

;; Actual parse (note where parens close):
(or (is-eq (get num swap-fee) (get den swap-fee))   ;; branch 1: swap-fee = 100%
    (and (> amt-fee-lps u0))                          ;; branch 2: amt-fee-lps > 0 (standalone)
         (or (is-eq (get num protocol-fee) ...        ;; branch 3: protocol-fee check
              (> amt-fee-protocol u0)))

The (and (> amt-fee-lps u0)) wraps only one expression, making it equivalent to (> amt-fee-lps u0). Since this is a standalone branch of the outer or, whenever LP fees are positive (which is the normal case for any swap-fee < 100%), the entire or evaluates to true and the protocol-fee check is skipped entirely.

Impact: Limited in practice because: (1) the constant-product postcondition (>= (* a b) k) still enforces economic soundness, and (2) the fee decomposition check (is-eq amt-in (+ amt-in-adjusted amt-fee-lps amt-fee-share amt-fee-rest)) ensures no tokens vanish. However, the protocol-fee validation being bypassed means the precondition is weaker than intended — a swap with a malformed protocol-fee could pass if LP fees are positive.

Recommendation: Fix the parenthesisation to:

(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))))

Location: calc-mint function, mint function

Description: Unlike Uniswap V2 which permanently burns MINIMUM_LIQUIDITY (1000 wei) of LP tokens on the first deposit, this contract mints the full sqrti(amt0 * amt1) to the first depositor. This is a well-known design choice omission that enables a first-depositor price manipulation attack.

Attack scenario:

1. First depositor mints with amt0=1, amt1=1e12, receiving sqrti(1e12) = 1e6 LP tokens
2. Depositor then directly sends a large amount of token0 to the contract (not through mint)
3. Since reserves don't update from direct transfers (no sync), subsequent depositors who use mint get fewer LP tokens than expected because calc-mint uses the stored (lower) reserves while the actual token balance is higher

Impact: Moderate — the attack is limited because (a) only the owner can create pools, providing a trust layer, and (b) tokens sent directly to the contract without going through mint are unrecoverable (no sync/skim), making the attack costly. The more practical risk is LP token precision loss for small initial deposits.

Recommendation: Burn a minimum amount of LP tokens on first mint (e.g., 1000 units permanently locked) to establish a minimum pool value floor.

Location: mint, burn, swap, collect — all as-contract blocks

Description: The contract uses the pre-Clarity 4 as-contract which grants blanket authority over all assets held by the contract principal. Since all pools share a single contract address, any code executing within as-contract has access to all tokens across all pools.

The trait-based design means the contract calls transfer on user-supplied token contracts. A malicious token contract's transfer implementation could, when called under as-contract, re-enter or execute arbitrary operations with the contract's full authority.

Mitigating factors: (1) Only the owner can create pools, so malicious tokens must be explicitly added. (2) Stacks post-conditions at the transaction level can limit actual token movement. (3) The token trait parameters are validated against the stored pool data before any as-contract call.

Recommendation: Migrate to Clarity 4 and use as-contract? with explicit with-ft/with-nft asset allowances. This eliminates the blanket authority risk at the language level.

Location: check-owner uses contract-caller; check-protocol-fee-to uses tx-sender

Description: The owner authentication checks contract-caller (the immediate caller), while the protocol-fee-to authentication checks tx-sender (the original transaction signer). This inconsistency means:

• set-owner, create, fee updates — can be called through an intermediary contract (checks contract-caller)
• collect — must be called directly by the protocol-fee-to address, NOT through another contract (checks tx-sender)

Impact: Low — the protocol-fee-to principal cannot use a multisig contract or any intermediary to collect revenue. This may be intentional (preventing intermediary contracts from collecting fees) but is undocumented and inconsistent.

Recommendation: Standardize on contract-caller for both checks, or document the design rationale for using tx-sender in check-protocol-fee-to.

Location: Contract-wide (noted in comments as "not implementable")

Description: The contract explicitly notes that sync/skim is not implementable because all pools share a single contract address and there's no way to iterate over pools. Any tokens sent directly to the contract (not through mint or swap) are permanently locked — they don't update reserves and cannot be recovered.

Impact: Low — accidental token sends are a known UX hazard. The on-chain comment acknowledges this limitation. The impact is confined to user error (sending tokens directly to the contract address).

Recommendation: Document this prominently in user-facing materials. Consider adding a pools list with a fold-based sync function in a future version.

Location: calc-swap, calc-burn, calc-mint

Description: All fee and liquidity calculations use Clarity's integer division which rounds down. For small swap amounts, the rounding can result in zero fees or zero liquidity. For example, with swap-fee 998/1000 and amt-in=1, the adjusted amount is (/ (* 1 998) 1000) = 0, and the fee total is 1, but the protocol fee split may also round to zero.

Impact: Informational — this is standard behavior for integer AMMs and is mitigated by the precondition checks that require (> amt-in-adjusted u0) and (> amt-fee-lps u0). Very small swaps will simply fail preconditions rather than execute with zero fees.

Location: set-owner, update-swap-fee, update-protocol-fee, update-share-fee, set-protocol-fee-to, set-share-fee-to

Description: The single owner principal has immediate, unilateral control over:

• Transferring ownership
• Changing swap fees (up to 0.5% cap — anti-rug guard at num≥995 with den=1000)
• Changing protocol fees (up to 50% of swap fee — anti-rug guard at num≤500 with den=1000)
• Changing share fees and the share-fee-to contract
• Changing the protocol-fee-to address

The anti-rug guards provide reasonable caps, but there is no timelock or governance delay — changes take effect immediately.

Impact: Informational — this is a standard trust assumption for protocol-controlled DEXes. The anti-rug guards (MAX-SWAP-FEE at 995/1000 = max 0.5% fee, MAX-PROTOCOL-FEE at 500/1000 = max 50% of fees to protocol) are well-designed and limit the blast radius of owner misbehavior.