# fakfun-wallet-v18 - review of the NEW smart-router surface

Scope: only the v18 diff (the 4 `smart-*` entries + private `authorize-smart`) and
its 4 support contracts, as the bounty asks. Target: the **deployed** bytecode.

Repro (mainnet fork, deployed bytecode, 15/15 steps as labelled):
https://stxer.xyz/simulations/mainnet/0cc2073bb3ecf430eb35d2fd62e29884
Harness: `simul-ky-v18-audit.js` (same `stxer` pattern as the repo harness).

I first diffed the 5 deployed sources out of the node API against the repo:
`faktory-smart-trait-v1`, `smart-execute-auth-helper`, `fakfun-smart-router-registry`,
`usdcx-sbtc-swap`, `fakfun-wallet-v18` are **byte-identical** to the repo, so the repo
is a valid audit target.

---

## F1 (Medium) - the signed `fak-ratio` field is inverted on the two STX-leg entries

`smart-buy-stx` / `smart-sell-stx` name the 3rd parameter `fak-ratio` and
`authorize-smart` binds it under that name:

    (define-private (authorize-smart (op (buff 1)) (smart principal) (amount uint)
      (min-out uint) (fak-ratio uint) (flag bool) ...)
      ... auth-id, op, smart, amount, min-out, fak-ratio, flag

But all 9 deployed routers take an **`alex-ratio`** on the STX legs, with the
opposite meaning:

    (define-public (buy-with-stx (stx-amount uint) (min-token-out uint) (alex-ratio uint) (flag bool))
      (alex-amount (/ (* stx-amount alex-ratio) TOTAL))
      (fak-amount  (- stx-amount alex-amount)))

(`pepe-smart-faktory.clar`; identical shape in b-/mia-/flatearth-/fakfun-/leo-/
lwb-/welsh-/rock-smart-faktory, and again in `sell-for-stx`.)

Concrete sequence, step [8]/[9] of the sim (ADMIN path, so nothing but the field
is in play - the same value is what a passkey would sign):

    smart-buy-stx('...pepe-smart-faktory, u20_000_000, u1, fak-ratio=u100, false, none, none)
      => (ok (tuple (stx-amount u20000000) (token-from-alex u1112914221)
                    (token-from-fak u0) (total-token-out u1112914221)))

    smart-buy-stx(..., fak-ratio=u0, ...)
      => (ok (tuple (stx-amount u20000000) (token-from-alex u0)
                    (token-from-fak u1096508343) (total-token-out u1096508343)))

So the value the wallet's own comment calls "fak-ratio TOTAL: u100 = everything
through the faktory pool" (`simul-v18-smart-swap.js:74`, repeated in
`README-v18-smart-swap-sims.md`: "Trades route fak-ratio = u100 (100% through the
faktory pool)", and that harness passes `FAK` into every STX leg too) routes the
STX legs **100% through ALEX / 0% through Faktory** - and `u0` does the reverse.

Impact: a passkey signature authorizes a venue split that the code inverts. A
signer / SDK / FE that follows the field name (as the wallet itself does) sends
STX-leg trades to the opposite venue; `min-out` still caps price slippage, so the
loss is wrong-venue execution + confusing reverts rather than a direct drain -
but the authorized intent is not the executed intent. `(<= fak-ratio TOTAL)` is
the only validation, so a basis-points-style value (u10000) reverts `u1002`.

Fix options: bind a per-leg field name (or separate topics) so `fak-ratio` always
means the faktory share, or normalize in the wallet for op 0x01/0x03
(`(- TOTAL fak-ratio)`), and correct the sim comment / README.

---

## F2 (Medium) - the 4 new entries ignore `token-lock-enabled`

Every existing passkey-reachable asset-moving entry asserts the lock inside its
`(match sig-auth ...)` Some-arm, e.g. `extension-call`, `sip010-transfer`,
`sbtc-initiate-withdrawal`, the three `-now` fast paths, the three staking paths
(10 sites in v17/v18). `authorize-smart` has no such assert, so all four new
entries are exempt from the passkey freeze.

Steps [10]-[14]:

    1. toggle-token-lock(true)                       ADMIN  => (ok true)
    2. extension-call(usdcx-sbtc-swap, ..., PASSKEY)        => (err u4023)   ; locked, as designed
    3. smart-buy-sbtc(pepe, 50000, u1, u100, false, PASSKEY) => (ok (tuple ... (token-from-fak u7878367167) ...))
    4. smart-sell-sbtc(pepe, tokensoft-token, 500000000, u1, u100, false, PASSKEY)
                                                              => (ok (tuple ... (total-sbtc-out u3175)))
    5. toggle-token-lock(false)                      ADMIN  => (ok true)

Step 3 moved 50,000 sats of sBTC out of the wallet and step 4 moved a sell
through it, while the passkey path was supposed to be frozen.

Impact: the lock is the documented incident-response lever for a phished passkey
("the token lock freezes the PASSKEY", `README-clarinet-rv.md`); the newest,
most complex and allowance-granting entries silently opt out of it, so a
compromised passkey keeps routing assets during a lock.

Fix: one assert in `authorize-smart`'s Some-arm covers all four entries
(`(asserts! (not (var-get token-lock-enabled)) err-token-locked)`); gate the
admin arm too if the intent is a true freeze.

---

## F3 (Low / latent) - the sell challenge does not bind `token` / `token-name`

`build-smart-execute-hash` binds `{auth-id, op, smart, amount, min-out,
fak-ratio, flag}`. The sell entries additionally take
`(token <sip-010-trait>)` and `(token-name (string-ascii 128))` and hand them to
`(with-ft (contract-of token) token-name amount)` - i.e. the passkey authorizes
an **allowance asset it never saw**.

Steps [15]/[16] - one signature (`op=0x02`, pepe router, 500,000,000) submitted
twice:

    token = USDCx, token-name "usdcx"  => (err u128)
    token = PEPE,  token-name "tokensoft-token" => (ok (tuple ... (total-sbtc-out u3171)))

The signature is accepted both times; only the router's hard-coded `TOKEN`
constant makes the USDCx attempt revert. Today that means no loss: every
approved router moves its own fixed token, so substituting the asset just
reverts (which is a good property to have, but it is the routers' property, not
the wallet's).

Latent risk: the allowance asset is caller-chosen and unbound, so from the
signer's point of view one phished signature authorizes "up to `amount` of
whatever asset the caller names". The first approved router that touches more
than one asset turns that into "drain up to `amount` of an asset the user never
approved, at a `min-out` that was quoted for a different asset".

Fix: add `token` (and `token-name`) to the challenge tuple.

---

## F4 (Low) - smart trades leave no wallet-core event

`fakfun-wallet-core-v2` exposes `log-stx-transfer`, `log-sip010-transfer`,
`log-sip009-transfer`, `log-extension-call`, `log-extension-whitelisted`,
`log-token-lock-toggled`, the staking logs, etc. Every existing entry calls one
of them; the four new entries call only the local `update-activity`. Movement
through the new surface therefore never reaches the wallet's on-chain activity
log - the same log a dashboard / watcher / incident responder reads, and the log
that makes F2's lock actionable. A passkey compromise that trades through
`smart-*` leaves no trail.

Fix: add a `log-smart-trade`-style function to wallet-core-v2 and call it from
the four entries (or reuse the extension-call log with an op tag).

---

## What I checked and could NOT break (so you don't re-hunt)

* Unapproved routers: `authorize-smart` asserts `is-approved-router` first on
  both arms; the sim-only `mock-smart-router` is rejected `u4033` from the admin
  arm, and the passkey arm cannot be reached before the gate.
* Op confusion / cross-entry replay: the 1-byte `op` tag is inside the hashed
  tuple, so a buy signature never validates a sell; `used-pubkey-authorizations`
  (message hash) plus `used-assertions` (sha256 of authenticator-data ||
  signature) stop replaying one gesture.
* Allowances: `(with-ft SBTC ... amount)` / `(with-stx amount)` /
  `(with-ft <token> ... amount)` cap what the nested calls can move out per
  call; the gas-station path is bounded by the same mechanism.
* `usdcx-sbtc-swap`: payload decode, `amt > 0`, `min-out > 0`, unknown action
  => u301, and the only assets it can touch are the fixed SBTC/USDCX/pool
  constants; `max-steps` going straight to the DLMM is a gas bound, not a fund
  bound.
* Registry governance: propose -> 144-block cooldown -> confirm requires the
  owner throughout; `revoke-pending` cannot touch a confirmed router;
  `accept-owner` needs the proposed principal and its own cooldown. No
  un-approve exists by design.
* Trait dispatch to the 9 routers works even though they do not declare
  `impl-trait`; structural conformance is proven by the calls returning `ok`.

## Harness note (red herring)

A fresh run of the repo's own `simul-v18-smart-swap.js` reports ~14 red steps,
but those are not v18 bugs: its deploy steps now collide with the live
contracts ("Duplicate contract"), and its `STX_WHALE` funding step fails
`Internal(InsufficientBalance)` at the current tip, so the wallet ends up with
0 STX - which is exactly why every `smart-buy-stx` returns `(err u1)` there
(`stx-transfer?` inside `buy-with-stx` from an unfunded wallet). Fund the wallet
from an address that actually holds STX (I used `SP1Z92MP...`) and the same
calls return `(ok ...)`.