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: Info
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: Info
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: Info
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: Info
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: Info
_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: Info
(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:

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)

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.