Stackswap Swap Router v1b

The router has no deadline parameter. A swap transaction can sit in the mempool indefinitely and execute at a time when market conditions have changed dramatically, resulting in the user receiving far less than expected.

How it works: A user submits a swap with min-amounts based on current prices. The transaction gets delayed (congestion, low fee, or deliberate withholding by a miner). Hours or days later, prices have moved significantly. The transaction executes and still passes the min-amount checks (if the user set them loosely), but the user receives much less value than they would have at submission time.

;; No deadline check — swap can execute at any future block
(define-public (router-swap
  (from <sip-010-token>) 
  (bridge <sip-010-token>)
  (to <sip-010-token>)
  (from-lp <liquidity-token>)
  (to-lp <liquidity-token>)
  (from-type bool)
  (to-type bool)
  (from-amt uint)
  (from-2-bridge-min-amt uint)
  (bridge-2-to-min-amt uint)
  ;; Missing: (deadline uint) with (asserts! (<= block-height deadline) ERR_EXPIRED)
)

Impact: Users may unknowingly execute swaps at stale prices. Combined with sandwich attacks (manipulate price → let delayed tx execute → reverse), this becomes a MEV extraction vector.

Recommendation: Add a deadline parameter (block height) and assert (<= block-height deadline) at the start of the function. This is standard practice in Uniswap-style routers.

Both swap direction checks use (and from-type true) and (and to-type true), which are logically equivalent to just from-type and to-type. The (and x true) pattern is a no-op.

;; Current (redundant):
(if (and from-type true) ...)
(if (and to-type true) ...)

;; Should be:
(if from-type ...)
(if to-type ...)

Impact: No functional impact. This is dead code that suggests the developer may have intended a second condition that was never added, or it's a copy-paste artifact.

Recommendation: Simplify to (if from-type ...) and (if to-type ...).

The function reads the user's initial from token balance into pre_from_amt but never references it. The return value uses the input parameter from-amt directly instead of computing a delta like it does for bridge and to tokens.

(let
    (
        (pre_from_amt (unwrap-panic (contract-call? from get-balance tx-sender)))  ;; never used
        ...
    )
    ...
    (ok (list from-amt delta_bridge_amt delta_to_amt))  ;; uses from-amt, not delta
)

Impact: Wastes gas on an unnecessary external call. More importantly, the return value reports the requested input amount rather than the actual amount deducted, which could differ if the token has transfer fees or rebasing mechanics.

Recommendation: Either remove pre_from_amt or compute delta_from_amt for an accurate return value: (- pre_from_amt (contract-call? from get-balance tx-sender)).

All six get-balance calls use unwrap-panic. If any token's get-balance returns an error (e.g., a non-compliant SIP-010 implementation), the entire transaction aborts with a runtime error rather than a descriptive error code.

Impact: Poor error reporting. Users see a generic abort rather than understanding which token failed. Not exploitable, but degrades UX.

Recommendation: Use unwrap! with descriptive error codes (e.g., ERR_BALANCE_CHECK_FAILED).

Access to the router is gated by stackswap-security-list-v1a.is-secure-router-or-user. This external contract controls who can use the router. If the security list is misconfigured or the StackSwap team removes a previously-approved caller, integrations break without warning.

(asserts! (contract-call? .stackswap-security-list-v1a is-secure-router-or-user contract-caller) ERR_INVALID_ROUTER)

Impact: Standard centralization trade-off. The security list provides protection against malicious contract interactions but creates a single point of control. This is a reasonable design choice for a DEX router.

This contract predates Clarity 4 (epoch 3.3). However, since the router never uses as-contract and holds no assets, the absence of Clarity 4's as-contract? with explicit asset allowances is not a security concern here. The contract operates entirely under the user's authority via tx-sender.

Impact: None for this specific contract. Noted for completeness.