# Manual Review Checklist — Solidity / EVM

Walk this for **every** contract that moves value. Each item: what to look for →
how to confirm (Foundry test / Slither detector) → fix. Tools catch the known
patterns; this catches the logic bugs they can't.

## 1. Reentrancy (SWC-107)
- [ ] External call (`call`, `transfer`, ERC-777 hooks, ERC-721 `onERC...Received`, arbitrary token) **before** state updates?
- [ ] Cross-function / read-only reentrancy: a view used by another protocol mid-call?
- **Confirm:** Slither `reentrancy-eth`/`reentrancy-no-eth`; write an attacker contract test that re-enters in its `receive()`.
- **Fix:** Checks-Effects-Interactions order; `nonReentrant` (OpenZeppelin `ReentrancyGuard`); pull-over-push payments. ("OZ" = OpenZeppelin throughout.)

## 2. Access control (SWC-105/106/115)
- [ ] Every state-changing/admin function gated (`onlyOwner`, roles, custom modifier)?
- [ ] `initialize()` on upgradeable contracts protected against re-init and front-running?
- [ ] No `tx.origin` for auth (phishable) — use `msg.sender`.
- [ ] `selfdestruct` / `delegatecall` reachable only by trusted roles?
- **Confirm:** a `test_RevertWhen_*` with `vm.prank(attacker)` + `vm.expectRevert` for each guard.
- **Fix:** OZ `Ownable2Step` / `AccessControl`; `initializer` modifier; remove `tx.origin`.

## 3. Oracle / price manipulation (DeFi #1 exploit class)
- [ ] Price from spot `getReserves()` / a single AMM pool? (flash-loan manipulable)
- [ ] Using Chainlink: checked `updatedAt` staleness, `answeredInRound`, min/max bounds, L2 sequencer uptime?
- **Confirm:** fork test that manipulates the pool within one tx and asserts your protocol stays solvent.
- **Fix:** TWAP / Chainlink with staleness+deviation checks; never trust spot for pricing.

## 4. Arithmetic & rounding (SWC-101)
- [ ] Solidity ^0.8 (built-in checked math) — and any `unchecked{}` block justified?
- [ ] Division before multiplication (precision loss)? Rounding always in protocol's favor?
- [ ] Share-inflation / first-depositor attack on ERC-4626-style vaults?
- **Confirm:** `testFuzz_*` over amounts; invariant `assertGe(assets, shares-implied)`.
- **Fix:** mulDiv (OZ `Math.mulDiv`); virtual shares/offset for vaults; explicit rounding direction.

## 5. Unchecked external calls / return values (SWC-104)
- [ ] Return value of low-level `call`/`send` and ERC-20 `transfer`/`transferFrom` checked?
- [ ] Non-standard ERC-20s (no return, fee-on-transfer, rebasing) handled?
- **Confirm:** Slither `unchecked-transfer`, `unchecked-lowlevel`.
- **Fix:** OZ `SafeERC20`; measure balance delta for fee-on-transfer; require success.

## 6. delegatecall / proxy storage (SWC-112)
- [ ] Storage layout identical across implementation upgrades (no reordered/removed vars)?
- [ ] No `delegatecall` to user-supplied address; implementation can't be re-initialized/self-destructed?
- **Confirm:** Slither `controlled-delegatecall`, `unprotected-upgrade`; storage-layout diff between versions.
- **Fix:** OZ `UUPS`/`Transparent` proxies + storage gaps; `_disableInitializers()` in constructor.

## 7. Front-running / MEV / ordering (SWC-114)
- [ ] Approve race (ERC-20 `approve` from non-zero to non-zero)?
- [ ] Slippage / deadline params on swaps and mints? Commit-reveal where needed?
- **Fix:** `increaseAllowance`/`permit`; enforce `minOut` + `deadline`; commit-reveal for sensitive ordering.

## 8. Randomness (SWC-120)
- [ ] Any `block.timestamp`/`blockhash`/`prevrandao` used as randomness for value?
- **Fix:** Chainlink VRF; never on-chain pseudo-randomness for payouts.

## 9. Denial of service (SWC-113/128)
- [ ] Unbounded loops over user-growable arrays? Push payments that can revert and brick the contract?
- [ ] Single external dependency whose revert blocks all users?
- **Fix:** pull payments; bounded iteration / pagination; isolate failures.

## 10. Token / standard pitfalls
- [ ] ERC-20: decimals assumptions, fee-on-transfer, rebasing, missing return.
- [ ] ERC-721/1155: safe-transfer reentrancy hooks; approval scope.
- [ ] Permit (EIP-2612): replay, deadline, signature malleability.

## 11. General hygiene
- [ ] `pragma` pinned (`pragma solidity 0.8.26;` not `^`)? Latest stable solc?
- [ ] Events emitted for every state-changing action (off-chain monitoring)?
- [ ] No leftover `console.log` / test backdoors / hardcoded addresses?
- [ ] Pausability + emergency withdraw for value-holding contracts?
- [ ] Invariants written for every conservation law (total supply, solvency, accounting)?

## Severity triage (impact × likelihood)
- **Critical/High** → direct loss/lock of funds, unauthorized mint/withdraw, broken access control. **Blocks deploy.**
- **Medium** → loss under specific conditions, griefing, precision drift.
- **Low/Info** → best-practice, gas, style.

Cross-check real-world findings on **Solodit** (https://solodit.xyz/) for the contract type
you're shipping (vault, AMM, staking, bridge, governance).