# PrimeOTC.sol — Security Review (Job 1117, seventh review)

**Target:** `PrimeOTC.sol` (Solidity =0.8.26, cancun, single file, no imports, 829 lines, 41,677 bytes)
**Source:** https://gist.githubusercontent.com/ASTRALDEFI/af7cbd2f37820b92cdf10296bba6faf8/raw/d38c8ec57f65315cf471f7835180224ffb4ed617/PrimeOTC.sol
**sha256:** `84f6ccece0cd612ef9b425156bba0abc78577a3eedb221432c15da53cbe6a65f` (re-computed after download; matches the job description)
**Chain:** Base (8453), not deployed. No repo / commit; the gist revision above is the pin.

**Severity counts:** 0 Critical · 0 High · 0 Medium · 0 Low · 6 Informational

## Method and one honest deviation

I read the whole file line by line and traced `createOffer`, `cancel`, `buy` and `sell`, every internal helper, the constructor and the views, against the notes and KNOWN LIMITS.

**I did not run the 12-agent fan-out of the v3 pipeline.** I did the review in a single context, and I did not run the x-ray scripts, the pashov V4 agents or the ethskills sub-agents. Because there was no fan-out, nothing died with an API error. That includes the two lenses job 1093 lost, and I covered both myself, in full, as part of the trace:
- **Execution trace (lens 4):** every external call in each of the four entrypoints, in order, with the gas, lock and revert behaviour at each point. See "Execution trace" below.
- **Invariants (lens 5):** each invariant you asked about is checked in the table below.

Nothing was executed. The policy is no target-code execution, so the fork gas figures (60,110 gas for the WETH fallback, 487,000 for two burning payouts) are **not reproduced** here. I only checked that they are consistent with the constants in the code.

## Findings

No Critical, High, Medium or Low findings.

### I-1 — Hostile-token cancel draw, `balanceOf` lies and other trusted-token assumptions rest on documented trust
**Severity:** Informational
Nothing new. `cancel` calls `_push` with no balance check (`PrimeOTC.sol:500-501`), so a fee-on-send token can draw on other offers in the same token. This is stated in note 7 and in KNOWN LIMITS (lines 196-201). I confirmed the scope is limited to offers in that same token. A hostile token's code cannot reach any other token's escrow, because every `transferFrom` has `from == msg.sender` (`:463,466,471,551,557,590`) and payout amounts come from the offer record.

### I-2 — Price limit is per-token when `tokenAmount` exceeds what is left
**Severity:** Informational
`buy`/`sell` call `_mulGt(quoteAmount, tokenAmount, maxQuote, filled)` (`:535`) and `_mulGt(minQuote, filled, quoteAmount, tokenAmount)` (`:582`), with `tokenAmount` still the caller's original figure. A taker who asks for N tokens when K < N are left and passes the total price for K as `maxQuote` gets a limit stricter than intended (`maxQuote/N` per token), so the call reverts with `PriceLimit`. It never pays more than intended. Note 13 describes this ("held as a price per token"). Integrators should pass `tokenAmount == tokenLeft` and the exact total, or scale `maxQuote` by `asked/left`.

### I-3 — ETH force-sent to the contract, or tokens sent directly, are unrecoverable
**Severity:** Informational
`receive()` (`:399-401`) refuses non-WETH senders, but ETH can still arrive through `selfdestruct` or as a coinbase reward. It sits there forever. This does not affect accounting, because no code path reads `address(this).balance`: ETH payouts use exact amounts, and the WETH fallback deposits exactly `amount`. Tokens transferred straight to the contract are also stuck, since there is no sweep. That is consistent with "no levers" (note 2).

### I-4 — Part fills can leave a 1-unit token remainder at an extreme price
**Severity:** Informational
The dust rule (note 6) is on the quote side only (`_checkPart`, `:716-721`). A maker who sets a high per-token price can have a part fill leave `tokenLeft = 1` with `quoteLeft >= min`. The offer is consistent (`Filled` iff `tokenLeft == 0`, see the invariants) and the maker chose that price. No accounting effect.

### I-5 — Hostile or odd token return data makes fills revert
**Severity:** Informational
`_call` requires "no data" or a first word of exactly 1 (`:787`). A token that returns another non-zero word, such as some non-standard tokens, makes every fill and cancel in it revert `TransferFailed`, and the escrow sticks. This stays within "do not list non-ERC-20s" (KNOWN LIMITS) and only affects offers made in that token.

### I-6 — Claims I cannot verify from the source alone
**Severity:** Informational
(a) The DGUY description in KNOWN LIMITS (address `0xe34356DC…`, 2,407 bytes, no proxy, no owner) names a truncated address. I could not check it from this file alone. (b) The fork gas figures are execution results and are not reproduced (see Method). Neither is contradicted by the code. The constants give `ETH_GAS_FLOOR = 100,000·64/63 + 36,600 + 90,000 = 228,187`, which matches the "about 228,000" in the text.

## Change-by-change confirmation (job 1093 → this version)

- **I-1 decimals check (`:385-387`):** complete. It runs after the zero, code, distinct and floor checks, and it only adds a failed-deploy path. A token whose `decimals()` reverts, returns the wrong length or returns dirty high bits makes the deploy revert, and nothing else changes. No runtime effect and no new state.
- **I-2 buy payment check (`:539`), every `msg.value` case:** I traced the case matrix below, and every case is covered:

| quote | `msg.value` | path |
|---|---|---|
| ETH | 0 | `else` branch (`:556-559`): pull WETH, unwrap, pay the maker |
| ETH | `< quoteAmount` | `BadValue` (`:539`) |
| ETH | `== quoteAmount` | direct branch, no excess, the maker paid in ETH |
| ETH | `> quoteAmount` | excess refunded first, then the maker (`:554-555`) |
| USDC / DGUY | 0 | `quote != ETH` branch (`:549-552`) |
| USDC / DGUY | `!= 0` | `BadValue` (`:539`) |

  This holds for whole fills (`quoteAmount = quoteLeft`) and clamped fills (`tokenAmount >= left`). `quoteAmount >= MIN > 0`, so `msg.value == 0` can never be mistaken for "pay nothing". The WETH-pull path with no value is the `else` branch.
- **Check-before-`_record` effects:** the new revert is ordered after `_fillable`, `_size`, `_checkPart` and the price limit. A call with both a wrong value and a bad size or price now reverts with the size or price error first, as before. A wrong value now beats `Shortfall`, `Overdrawn` and `TransferFailed` from token code, because it runs first. Reverts undo `_record`, so accounting is unchanged. The only observable difference is which error name a doubly-wrong call returns. A test expecting `Shortfall`/`TransferFailed` together with a wrong `msg.value` would now see `BadValue`. I found no other dependency.
- **Notes and KNOWN LIMITS against the code (the last two Lows were about comments):** I checked each sentence against the code. In particular:
  - the payout-is-last claim holds in `buy` (both ETH branches, and the ERC-20 branch ends with `_pushChecked(q, maker)`), in `sell` (`_payQuote` to the taker is the last call) and in `cancel`;
  - the "frozen maker" description is accurate: a Buy offer's refund is the quote, a Sell offer's refund is the token, and fills still work for the party who is not frozen;
  - the lock covers all four state-changing functions;
  - the "Sell offer holds only its token" and "two ETH payouts, each with its own floor" statements are true of `:554-555`.

  I found no sentence that contradicts the code.

## Execution trace (lens 4)

- **createOffer:** the lock is taken, then validation, then storage writes (`_offers.push`, `_open.push`, `_openPos`, `_byMaker`), then exactly one funding action. A failed funding call reverts the pushes, so ids are never half-created. Per side: Sell with value reverts; Buy in USDC/DGUY with value reverts; Buy in ETH with value requires exact `quoteAmount` and calls `WETH.deposit`; Buy in ETH with no value pulls WETH. All of them are measured except the trusted-WETH deposit.
- **cancel:** the status flips to `Cancelled` and `_close` runs before any external call. Then a single `_push` (token for Sell) or `_payQuote(..., false)` (quote for Buy); only one is non-zero. The ETH refund goes through `withdraw` → `_sendEth` → the WETH fallback.
- **buy:** `_fillable` → `_size` → price → value → `_record` (state final) → token push to the taker (measured at both ends) → quote leg. Either ERC-20 pull+push, or direct ETH with the excess refund first, or WETH pull+unwrap+pay. The maker's payout is last in every branch. All external calls run under the lock, with the token's reply truncated to one word.
- **sell:** `_record` → pull the token from the taker (measured) → push to the maker (measured at both ends) → pay the taker in the quote, last.
- **Gas:** `ETH_GAS_FLOOR` is 228,187. After the check, the call overhead of at most ~36,600 plus the stack work leaves more than 100,000 × 64/63 for the forward, so the receiver gets the full 100,000. If the receiver burns all of it, at least about 90,000 remains, which covers the ~60,000 fallback. `withdraw` runs before `_sendEth`'s gas check in `_payEthFromWeth`; the check only governs what follows, which is what the floor models.
- **Scratch memory:** `_call` and `_balanceOf` write to memory 0..0x20 inside `assembly ("memory-safe")`, which is permitted scratch space. The word is read only when `size >= 32`, or in `_call` when `size == 0` it is never consumed.

## Invariant Verification (lens 5)

| ID | Property | Verdict |
|---|---|---|
| E-1 | Per-token escrow solvency: balance(T) ≥ Σ open Sell offers' `tokenLeft` in T + Σ open Buy offers' `quoteLeft` in T | **Holds**, given tokens behave as stated. Every increase is a measured pull and every decrease is paired with a payout of the same amount. Fill legs net to zero. The exception is the documented unchecked-cancel hostile-token case. |
| E-2 | `status == Filled` iff `tokenLeft == 0` | **Holds.** `_record` (`:729-733`) is the only place `Filled` is set. A part fill has `filled < tokenLeft`, so it never reaches 0. A whole fill sets `quoteLeft = 0` as well. `_checkPart` keeps `quoteLeft ≥ min > 0` while `tokenLeft > 0`. |
| E-3 | `_open` swap-and-pop is consistent | **Holds.** `_close` (`:737-744`) writes `last` into the gap, fixes `_openPos[last]`, pops, then deletes `_openPos[id]`. When `id == last` the final delete clears the entry. It is only called on Open offers (`pos ≥ 1`), once per offer, because status flips first. |
| E-4 | The payout to the other side is last | **Holds** for `buy`, `sell` and `cancel` (see the trace). |
| E-5 | Totals are exact: a fully filled offer pays exactly `quoteTotal` / `tokenTotal` | **Holds.** The last fill takes the remainders (`quoteAmount = quoteLeft`). Parts only subtract. `uint128` casts are safe because `filled ≤ tokenLeft` and `quoteAmount ≤ quoteLeft`. |
| E-6 | Rounding never favours the taker against the maker within a fill, and never harms the next taker | **Holds.** `buy` rounds up and `sell` rounds down. The remainder's per-token price moves only in the next taker's favour. |
| E-7 | Price-limit comparison is exact | **Holds.** The 512-bit `mulmod` form (`:696-710`) is correct: `hi = mm − lo − (mm < lo)`. The inequality directions in `:535` and `:582` match the stated per-token semantics. |
| E-8 | A maker cannot fill their own offer or name themselves taker | **Holds** (`:437`, `:679`). |
| E-9 | Offer terms are immutable after creation | **Holds.** The only writes after creation are `tokenLeft`, `quoteLeft`, `fills`, `status` and `closedAt`. |
| E-10 | No re-entry into token-moving functions | **Holds.** The transient lock is taken on all four functions and is cleared on every successful exit; a revert rolls it back. The read functions are unlocked (documented). |

**Coverage:** 4 state-changing entrypoints, 4 addressed. The 8 views and `receive()` were read. 10 invariants checked, 10 answered (10 hold, 0 violated, 0 unverified).

## Direct answers to the questions in the brief

1. **Is every `msg.value` case still covered?** Yes, for all three quotes, whole and clamped fills, and the WETH-pull path (see the table).
2. **Does the earlier check change any revert or accounting?** Accounting, no. The only difference is which error name a call with several faults gets (see above).
3. **Do the three `decimals()` calls add anything beyond a failed deploy?** No.
4. **Is every sentence in the notes and KNOWN LIMITS true of the code?** I found none that is false. Two claims are untestable without execution (I-6).
5. **Fresh review:** no Critical, High, Medium or Low findings; the six Informational items above are all either documented or have no effect on funds.