ALEX amm-registry-v2-01

Location: update-pool function

Description: The update-pool function accepts a full pool-data struct (including pool-id) and writes it directly to pools-data-map. However, it does not validate that the pool-id in the supplied struct matches the pool-id originally assigned when the pool was created. This means the DAO could (accidentally or intentionally) set a pool-id in pools-data-map that doesn't match the reverse lookup in pools-id-map.

(define-public (update-pool (token-x principal) (token-y principal) (factor uint)
    (pool-data {
        pool-id: uint,      ;; <-- no validation against pools-id-map
        total-supply: uint,
        balance-x: uint,
        balance-y: uint,
        ...}))
    (begin
        (try! (is-dao-or-extension))
        (ok (map-set pools-data-map { token-x: token-x, token-y: token-y, factor: factor } pool-data))))

Impact: get-pool-details-by-id would return the original key, but reading the pool data via that key would show a different pool-id. Any off-chain or on-chain logic that cross-references pool-id across both maps could break or return incorrect data. Since the pool contract (amm-pool-v2-01) uses this registry for pool state, inconsistent pool-ids could confuse integrators.

Recommendation: Either remove pool-id from the update-pool input struct (derive it from the existing map entry), or add an assertion:

(asserts! (is-eq (get pool-id pool-data) (get pool-id existing-pool)) ERR-INVALID-POOL)

Location: set-fee-rate-x, set-fee-rate-y

Description: Fee rate setters accept any uint value with no maximum bound. Unlike set-oracle-average (which validates < ONE_8) and set-max-in-ratio/set-max-out-ratio (which validate against max-ratio-limit), fee rates can be set to values exceeding 100% (ONE_8 = u100000000).

(define-public (set-fee-rate-x (token-x principal) (token-y principal) (factor uint) (fee-rate-x uint))
    (let (
            (pool (try! (get-pool-details token-x token-y factor))))
        (try! (is-dao-or-extension))
        ;; No bounds check on fee-rate-x
        (ok (map-set pools-data-map ... (merge pool { fee-rate-x: fee-rate-x })))))

Impact: A malicious or buggy DAO proposal could set fee rates above 100%, which — depending on how the pool contract interprets them — could cause swaps to fail, drain more fees than the swap amount, or create arithmetic overflow/underflow conditions.

Recommendation: Add validation: (asserts! (<= fee-rate-x ONE_8) ERR-PERCENT-GREATER-THAN-ONE). The error constant ERR-PERCENT-GREATER-THAN-ONE already exists in the contract but is never used.

Location: set-start-block, set-end-block

Description: The start and end block setters operate independently with no cross-validation. It's possible to set start-block > end-block, creating an invalid time window where the pool can never be active (or is always active, depending on how the pool contract interprets the range).

Impact: Low — requires a DAO proposal to trigger, and the pool contract may have its own validation. But the registry should enforce basic invariants at the source.

Recommendation: Add cross-validation: when setting start-block, assert new-start-block <= end-block; when setting end-block, assert new-end-block >= start-block.

Location: set-fee-rebate

Description: Similar to M-02, fee-rebate has no bounds check. A rebate exceeding 100% could cause the pool contract to pay out more in rebates than collected in fees.

Recommendation: Add (asserts! (<= fee-rebate ONE_8) ERR-PERCENT-GREATER-THAN-ONE).

Description: This contract was deployed before Clarity 4 (epoch 3.3). While it doesn't use as-contract (no token operations), it cannot benefit from Clarity 4 safety features like as-contract? with explicit asset allowances or contract-hash? for on-chain code verification.

Recommendation: If the registry is ever redeployed, use Clarity 4 to benefit from the latest safety builtins.

Description: The error constant ERR-PERCENT-GREATER-THAN-ONE (err u5000) is defined but never referenced in any validation. It appears intended for fee rate bounds checking that was never implemented.

Recommendation: Use this constant in fee rate and rebate validation (see M-02, L-02).

Description: Once created, a pool entry cannot be removed from the registry. The only way to effectively disable a pool is to set start-block and end-block to values that prevent trading. This is a common pattern in Clarity contracts (maps don't support deletion), but worth noting for operational awareness.

Recommendation: Consider adding an explicit active: bool field to pool data for cleaner pool lifecycle management, or document the start/end block approach as the canonical deactivation mechanism.