# stSTX-STX stableswap pool static-analysis report

Contract: `SPQC38PW542EQJ5M11CR25P7BS1CA6QT4TBXGB3M.stableswap-stx-ststx-v-1-2`

Source reviewed: `stableswap.clar`, 1124 lines

Source SHA-256: `1A165FEEA723AEB8EDAF5AD0E1718397A861C3D0E43E9E7F12F1E841D25BCA58`

Scope note: this review covers the supplied stableswap core contract only. It treats STX as token X and a SIP-010 token, expected stSTX, as token Y. LP token behavior is delegated through the supplied `<lp-trait>`.

Responsible disclosure: no high or critical issue was identified. The strongest finding is medium severity fee misapplication / fee bypass, so public submission is appropriate under the bounty rules.

## 1. State model

### Constants

| Constant | Line | Purpose |
|---|---:|---|
| `this-contract` | 20 | Contract principal captured with `as-contract tx-sender`; used conceptually for custody. |
| `deployment-height` | 23 | Burn-block height at deployment; used to compute fee cycles. |
| `cycle-length` | 26 | Cycle duration in burn blocks, initialized to 144. |
| `index-list` | 29 | Fixed iteration list for Newton-Raphson convergence, length 384. |
| `number-of-tokens` | 32 | Stable pool token count, fixed at 2. |
| `contract-deployer` | 35 | Initial deployer principal, cannot be removed from admin list. |

### Data vars

| Store | Line | Type / initial value | Mutators | Authority |
|---|---:|---|---|---|
| `staking-and-rewards-contract` | 47 | `principal`, initially `tx-sender` | `set-staking-contract` | Admin, one-time only |
| `staking-and-rewards-contract-is-set` | 50 | `bool`, initially `false` | `set-staking-contract` | Admin, one-time only |
| `stacking-dao-contract` | 53 | `principal`, default `SP4SZE...` | `set-stacking-dao-contract` | Admin |
| `bitflow-contract` | 56 | `principal`, default `SP1G6...` | `set-bitflow-contract` | Admin |
| `admins` | 59 | `(list 5 principal)`, initially deployer | `add-admin`, `remove-admin` | Any current admin |
| `buy-fees` | 62 | `{lps, stacking-dao, bitflow}`, default `{3,0,2}` bps | `change-buy-fee` | Admin |
| `sell-fees` | 65 | `{lps, stacking-dao, bitflow}`, default `{3,195,2}` bps | `change-sell-fee` | Admin |
| `admin-swap-fees` | 68 | `{lps, stacking-dao, bitflow}`, default `{0,0,0}` bps | `change-admin-swap-fee` | Admin |
| `liquidity-fees` | 71 | `uint`, default `3` bps | `change-liquidity-fee` | Admin |
| `helper-principal` | 74 | `principal`, initially `tx-sender` | `remove-admin` | Any current admin |
| `convergence-threshold` | 77 | `uint`, default `2` | `change-convergence-threshold` | Admin |

### Maps

| Map | Lines | Key | Value fields | Mutators |
|---|---:|---|---|---|
| `PairsDataMap` | 84-93 | `{y-token, lp-token}` principals | `approval`, `total-shares`, `x-decimals`, `y-decimals`, `balance-x`, `balance-y`, `d`, `amplification-coefficient` | `create-pair`, `set-pair-approval`, swaps, `add-liquidity`, `withdraw-liquidity`, `change-amplification-coefficient` |
| `CycleDataMap` | 95-97 | `{y-token, lp-token, cycle-num}` | `cycle-fee-balance-x` | `swap-x-for-y`, `swap-y-for-x` |

## 2. Function inventory

### Read-only functions

| Function | Lines | Authority | Preconditions / asserts | State mutations | External calls / transfers |
|---|---:|---|---|---|---|
| `get-pair-data` | 108-110 | Open | None | None | Reads `PairsDataMap` |
| `get-cycle-data` | 113-115 | Open | None | None | Reads `CycleDataMap` |
| `get-current-cycle` | 118-120 | Open | Assumes current burn height >= deployment height | None | None |
| `get-cycle-from-height` | 123-125 | Open | Underflows if `height < deployment-height` | None | None |
| `get-starting-height-from-cycle` | 128-130 | Open | None | None | None |
| `get-deployment-height` | 133-135 | Open | None | None | None |
| `get-dx` | 141-174 | Open | Pair must exist; stable math must not underflow/divide by zero | None | Reads pair and sell fees |
| `get-x` | 177-189 | Open | Inputs must keep denominators nonzero | None | Calls `get-D`, private loop |
| `get-dy` | 225-260 | Open | Pair must exist; stable math must not underflow/divide by zero | None | Reads pair and buy fees |
| `get-y` | 263-275 | Open | Inputs must keep denominators nonzero | None | Calls `get-D`, private loop |
| `get-D` | 749-751 | Open | `x-bal`, `y-bal`, and `ann` must avoid zero denominators in loop | None | Private loop |

### Public swaps/liquidity

| Function | Lines | Authority | Preconditions / asserts | State mutations | External calls / transfers |
|---|---:|---|---|---|---|
| `swap-x-for-y` | 321-439 | Open | Pair exists; pair approved; `x-amount < 10 * balance-x`; `dy > min-y-amount` | Updates `PairsDataMap`; updates/creates `CycleDataMap` fee balance | STX from swapper to contract; fee STX from swapper to staking/DAO/Bitflow recipients; Y token from contract to swapper |
| `swap-y-for-x` | 443-557 | Open | Pair exists; pair approved; `y-amount < 10 * balance-y`; `dx > min-x-amount` | Updates `PairsDataMap`; updates/creates `CycleDataMap` fee balance | Y token from swapper to contract; fee STX from contract to staking/DAO/Bitflow recipients; STX from contract to swapper |
| `add-liquidity` | 568-680 | Open | Pair exists; pair approved; nonzero X or Y amount; post-fee invariant `d2 > d0`; minted LP amount `> min-lp-amount` | Updates pair balances, total shares, and `d` | STX/Y from provider to contract; imbalance fees to Bitflow; LP mint to provider |
| `withdraw-liquidity` | 685-738 | Open LP holder path | Pair exists; min X/Y outputs strictly exceeded | Updates pair balances, total shares, and `d` | LP burn from remover; STX/Y from contract to remover |

### Public governance

| Function | Lines | Authority | Preconditions / asserts | State mutations | External calls / transfers |
|---|---:|---|---|---|---|
| `create-pair` | 865-909 | Admin | Pair absent; at least one initial balance positive; scaled initial balances equal; Y decimals readable | Creates `PairsDataMap` entry approved by default | LP mint; initial STX/Y transfers into contract |
| `set-pair-approval` | 915-932 | Admin | Pair exists | Changes pair `approval` | None |
| `add-admin` | 937-953 | Any current admin | New admin not already present; max length <= 5 | Appends to `admins` | None |
| `remove-admin` | 956-981 | Any current admin | Caller is admin; target is admin; target is not deployer | Sets `helper-principal`; filters `admins` | None |
| `change-buy-fee` | 984-994 | Admin | None beyond admin | Replaces `buy-fees` | None |
| `change-sell-fee` | 997-1007 | Admin | None beyond admin | Replaces `sell-fees` | None |
| `change-admin-swap-fee` | 1010-1020 | Admin | None beyond admin | Replaces `admin-swap-fees` | None |
| `change-liquidity-fee` | 1023-1033 | Admin | None beyond admin | Replaces `liquidity-fees` | None |
| `change-amplification-coefficient` | 1037-1055 | Admin | Pair exists; no bound on new coefficient | Replaces pair `amplification-coefficient` | None |
| `change-convergence-threshold` | 1058-1068 | Admin | None beyond admin | Replaces `convergence-threshold` | None |
| `set-staking-contract` | 1072-1092 | Admin | Can only be called once | Sets staking contract and lock bool | None |
| `set-stacking-dao-contract` | 1096-1108 | Admin | None beyond admin | Replaces DAO fee recipient | None |
| `set-bitflow-contract` | 1112-1124 | Admin | None beyond admin | Replaces Bitflow fee recipient | None |

### Private helpers

| Function | Lines | Purpose |
|---|---:|---|
| `x-for-loop` | 192-222 | Newton iteration for X output. |
| `y-for-loop` | 278-308 | Newton iteration for Y output. |
| `D-for-loop` | 754-797 | Stable invariant iteration. |
| `get-scaled-up-token-amounts` | 801-823 | Normalizes X/Y amounts to shared precision. |
| `get-scaled-down-token-amounts` | 827-849 | Converts scaled amounts back to token precision. |
| `is-not-removeable` | 852-854 | `remove-admin` filter helper based on `helper-principal`. |

## 3. Post-condition coverage matrix

| Public function | Token movements | Caller post-conditions |
|---|---|---|
| `swap-x-for-y` | Debits STX input and STX fee components from caller; credits Y token output from contract | Require max STX debit equal to intended `x-amount`; require Y token credit greater than/equal to expected minimum; restrict fee recipient transfers if wallet supports it. |
| `swap-y-for-x` | Debits Y token from caller; credits STX output from contract; fee STX is paid from contract reserves | Require max Y token debit equal to intended `y-amount`; require STX credit greater than/equal to expected minimum. |
| `add-liquidity` | Debits STX and/or Y from provider; debits imbalance fees to Bitflow; mints LP | Require max STX/Y debits; require LP mint at least expected amount; if adding imbalanced liquidity, account for fee debits. |
| `withdraw-liquidity` | Burns LP from caller; credits STX and Y from contract | Require max LP burn equal to intended `lp-amount`; require STX and Y minimum credits. |
| `create-pair` | Admin deposits initial STX/Y and receives initial LP | Require max initial STX/Y debits and LP mint equal to scaled initial balances sum. |
| Governance setters/admin functions | No token movement | No token post-conditions; signers should verify target pair, fee values, recipient principal, and admin list change. |

## 4. Authority / access-control matrix

| Role / operation | Mechanism | Notes |
|---|---|---|
| Admins | `admins` list checked with `tx-sender` | Any current admin can add another admin up to 5 total, remove non-deployer admins, create/disable pairs, change fees, amplification, convergence threshold, and fee recipients. |
| Contract deployer | Captured as `contract-deployer` | Cannot be removed from admin list, preserving at least one fixed governance principal. |
| Pair approval / pause equivalent | `approval` field in `PairsDataMap` | Swaps and adds require approval. Withdraw does not require approval, which is favorable for letting LPs exit disabled pools. |
| Fee recipients | Mutable principals | Staking/rewards recipient can be set only once; StackingDAO and Bitflow recipients can be changed repeatedly by admins. |
| Oracle dependencies | None found | Pricing is entirely invariant-based from internal balances; no external oracle calls. |
| Amplification governance | `change-amplification-coefficient` | Immediate update, no ramp, no lower/upper bounds. This is a privileged price-shape control. |

## 5. Clarity best-practice review

| Topic | Result |
|---|---|
| `tx-sender` vs `contract-caller` | Admin authorization uses `tx-sender` throughout. This enables admin authority through a contract call initiated by an admin. That may be intentional, but it is broader than direct-call-only `contract-caller` authorization. |
| `unwrap-panic` / `unwrap-err-panic` | No `unwrap-panic` or `unwrap-err-panic` found. The contract uses `unwrap!` with typed string errors for external calls and map lookups. |
| Arithmetic overflow / underflow | Stable math uses many `*`, `+`, `-`, `/` operations in read-only and public paths. Clarity aborts on arithmetic failure. The most important risks are unbounded admin fee values, unbounded amplification/convergence values, and zero denominators if governance creates bad parameters. |
| `as-contract` usage / principal escalation | Used for contract-owned STX/token transfers and LP minting. This matches custody design, but post-conditions are still required for user calls. |
| Trait conformance gaps | User supplies SIP-010 and LP traits. Pair identity is keyed by `contract-of` principals, which is good. The contract still depends on LP token `mint`/`burn` authorization being restricted to this stableswap contract. |
| get_y / get_dy precision | Scaling helpers are used broadly. One quote path uses unscaled `x-amount` when subtracting scaled fees, creating a potential mismatch for token pairs with unequal decimals. |

## 6. Findings table

| ID | Severity | Function | Line | Finding | Recommended fix |
|---|---|---|---:|---|---|
| STABLE-01 | Medium | `swap-x-for-y`, `swap-y-for-x` | 331-343, 453-465 | Fee branch appears inverted: comments say admins pay no fees, but admins receive normal buy/sell fees while non-admins receive `admin-swap-fees`, initialized to zero. This lets ordinary users avoid swap fees under default config and charges admins instead. | Reverse the branches so admins use `admin-swap-fees` and non-admins use `buy-fees`/`sell-fees`; add tests for admin and non-admin fee paths. |
| STABLE-02 | Medium | `change-buy-fee`, `change-sell-fee`, `change-admin-swap-fee`, `change-liquidity-fee` | 984-1031 | Fee setters have no upper bounds. Admin can set total fee above 10000 bps, which can underflow fee subtraction or make swaps/liquidity unusable. | Assert each component and the total fee are within a documented max, especially `<= u10000`, and preferably a much lower protocol limit. |
| STABLE-03 | Medium | `create-pair`, `change-amplification-coefficient`, `change-convergence-threshold` | 865-906, 1037-1066 | Amplification coefficient and convergence threshold have no bounds or ramp. A zero or extreme amplification value can cause division by zero, broken quotes, or abrupt price-shape changes. | Require `amplification-coefficient > 0`, impose upper bounds, and consider a time-ramped amplification update for live pools. Bound convergence threshold to a safe range. |
| STABLE-04 | Low | `get-dy` | 244-252 | `get-dy` scales `x-amount` to `x-amount-scaled` but computes `x-amount-total-fees-scaled` from unscaled `x-amount`. If X/Y decimals differ, the read-only quote can diverge from swap execution. | Compute total fees from `x-amount-scaled` or reuse the same per-component scaled fee calculation used by `swap-x-for-y`. |
| STABLE-05 | Low | Swap and liquidity min checks | 375-376, 493-494, 635-636, 709-713 | Minimum-output checks use strict `>` rather than `>=`. Exact-min executions revert even when the caller-specified minimum is met exactly. | Use `>=` for standard slippage semantics, or document strict-greater behavior in integrator docs. |
| STABLE-06 | Informational | Governance | 937-1124 | Admin controls are broad and authorized by `tx-sender`; there is no timelock, multisig requirement, or delayed activation in this contract. | Document the external governance/multisig process and consider delayed activation for fee recipient and amplification changes. |

## Finding details

### STABLE-01: Swap fee branch appears inverted

Severity: Medium

Lines: 331-343 and 453-465

Both swap functions include the comment `Admins pay no fees on swaps`. The initialized `admin-swap-fees` are zero at line 68, while regular buy and sell fees are nonzero at lines 62 and 65. However, the branch in both swap functions selects normal `buy-fees` or `sell-fees` when `tx-sender` is an admin, and selects `admin-swap-fees` otherwise.

Under the default configuration, ordinary non-admin users therefore pay zero swap fees, while admins pay the normal fee schedule. That contradicts the comment and the variable names. Impact is loss of expected LP/protocol fee accrual and incorrect cycle fee accounting. This is not a direct user-funds loss, so I rate it medium rather than high.

Recommended fix: reverse the `if` branches and add regression tests for admin and non-admin swaps in both directions.

### STABLE-02: Fee setters lack bounds

Severity: Medium

Lines: 984-1031

Admin fee setters directly assign new fee components without checking component or total bounds. If the sum of fees exceeds 10000 bps, arithmetic such as `updated-x-amount-scaled` in `swap-x-for-y` and `dx` in `swap-y-for-x` can underflow, making swaps revert. Liquidity fee values can similarly make imbalanced adds unexpectedly fail or transfer excessive fee amounts.

The affected functions are admin-only, but governance mistakes are a realistic risk for DeFi pools.

Recommended fix: assert safe bounds on each component and on total fees. A hard `<= u10000` total is the minimum; a lower policy cap is better.

### STABLE-03: Amplification and convergence controls are unbounded

Severity: Medium

Lines: 865-906 and 1037-1066

`create-pair` accepts any `amplification-coefficient`, and `change-amplification-coefficient` can later replace it with any uint. Stable invariant functions divide by `ann` or values derived from it. A zero coefficient can cause division by zero in `get-D`, `get-x`, or `get-y`; extreme values can cause arithmetic aborts or abrupt pricing behavior.

`change-convergence-threshold` also accepts any value. A very high threshold can mark imprecise Newton iterations as converged; a poorly chosen low threshold can increase failure/revert risk by never reaching useful convergence.

Recommended fix: require `amplification-coefficient > 0`, set a documented maximum, and consider a ramp mechanism for live changes. Bound convergence threshold to a tested precision range.

### STABLE-04: `get-dy` quote can mismatch execution for unequal decimals

Severity: Low

Lines: 244-252

`get-dy` computes `x-amount-scaled`, then computes total fees with the unscaled `x-amount` at line 249 before subtracting from the scaled amount at line 250. The public `swap-x-for-y` path instead computes each fee component from `x-amount-scaled` at lines 352-355.

For the intended STX/stSTX pool this may not manifest if both assets use the same decimals. As a reusable pair contract accepting arbitrary SIP-010 Y tokens, unequal decimals can make the read-only quote diverge from execution.

Recommended fix: compute quote fees from `x-amount-scaled` or share the same helper logic with `swap-x-for-y`.

### STABLE-05: Exact minimum output reverts

Severity: Low

Lines: 375-376, 493-494, 635-636, 709-713

The public swap, add-liquidity, and withdraw functions all require actual output to be strictly greater than the caller's minimum. Standard slippage semantics usually allow equality. With this contract, a transaction that exactly satisfies the user's minimum still reverts.

Recommended fix: use `>=` in min-output checks or explicitly document that callers must set minimums one unit below the exact acceptable amount.

### STABLE-06: Broad admin powers are external-governance dependent

Severity: Informational

Lines: 937-1124

Admin operations include admin-list management, pair approval, fee changes, fee recipient changes, amplification changes, and convergence-threshold changes. The contract itself does not implement a timelock or multisig threshold. It relies on the principals in `admins` to represent appropriate external governance.

Recommended fix: document the actual governance process. For high-impact parameters, consider delayed activation in contract or route admin actions through an audited governance contract.

## Non-findings and positive observations

- `withdraw-liquidity` does not assert pair approval. This appears beneficial: LPs can exit even if admins disable swaps/adds through `approval`.
- Pair identity is keyed by `contract-of` token principals rather than trusting arbitrary string names.
- External call failures are wrapped with explicit `unwrap!` errors rather than panics.
- The deployer cannot be removed from the admin list, which prevents the admin list from being emptied through `remove-admin`.