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:

[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.