---
name: contract-review
description: Security review of Solidity smart contracts against a practical checklist — access control, reentrancy, token handling, upgradeability, oracle and economic risks — reporting findings by severity with exact locations and fixes. Use when asked to audit, review or harden a contract before deployment.
---

# Smart contract review

Read every contract in scope fully before reporting. Prefer fewer, well-evidenced findings over a long list of generic warnings.

## Scope first

List the contracts, their roles, external dependencies (OpenZeppelin version, oracles, tokens) and the trust model: who are the privileged roles and what can each do? Write this down in two or three sentences — most real bugs are violations of the intended trust model.

## Checklist

**Access control**
- Every state-changing external function: who can call it? Missing `onlyOwner` / role checks, `tx.origin` auth, unprotected `initialize`.
- Ownership transfer is two-step; renouncing cannot brick required admin functions.

**Reentrancy and external calls**
- Checks-effects-interactions: state updated before any external call (token transfers, `call`, hooks such as ERC-777 / ERC-721 `onReceived`).
- Cross-function and read-only reentrancy through view functions used by other protocols.
- Unchecked low-level `call` return values; calls to addresses with no code succeeding silently.

**Token handling**
- Non-standard ERC-20s: missing return values (USDT), fee-on-transfer, rebasing, tokens with blocklists, 18 vs 6 decimals assumptions.
- Use balance-before/after when the received amount matters.
- `approve` race; infinite approvals to upgradeable contracts.

**Arithmetic and logic**
- Rounding direction in share/price math (should favour the protocol), division before multiplication, precision loss with small amounts, first-depositor inflation attacks on vaults.
- Unbounded loops over user-controlled arrays (DoS / gas griefing).

**Signatures**
- Replay protection: nonce, chainId, contract address (EIP-712 domain), deadline. `ecrecover` returning `address(0)`. Signature malleability.

**Upgradeability**
- Storage layout compatibility, `_disableInitializers()` in implementation constructors, who controls upgrades and with what delay.

**Oracles and economics**
- Spot prices from AMMs are manipulable within one transaction; stale Chainlink answers (`updatedAt`), L2 sequencer uptime.
- Flash-loan paths that change any value read in the same transaction.

## Report format

For each finding:

```text
[SEVERITY] Title
Location: Contract.sol:L120-L134, function withdraw()
Impact: what an attacker gains or what breaks, with a concrete scenario
Fix: minimal code change
```

Severity: **Critical** (direct loss of funds, no preconditions), **High** (loss with plausible preconditions), **Medium** (limited loss, DoS, broken invariants), **Low** (best practice, defence in depth), **Info**. End with a short summary of what was reviewed and what was out of scope. If tests exist, propose a failing test for each Critical/High finding.