3.7.7 Anti-Patterns Catalog

The prior six subsections of 3.7 presented patterns to use. This section catalogs patterns to avoid — code shapes that look reasonable, often compile cleanly, and have produced real exploits. Each entry is short by design: a one-paragraph identification of the anti-pattern, a minimal vulnerable example, the correct alternative, and a note on where the deeper treatment lives.

The catalog is meant to be scannable. A developer reviewing their own code or a teammate's pull request can run through this list as a quick checklist; the format here mirrors that intended use. Most entries cross-reference into Section 3.8 (Common Vulnerabilities) or Book 4 for full treatment — this section is the index, not the encyclopedia.

The catalog is organized by category: Identity & Authorization, External Calls, Arithmetic & Logic, State & Storage, Time & Randomness, Solidity Language Pitfalls, and Operational. Within each category, entries are ordered roughly from most-common to least-common in production exploits.

Identity & Authorization

tx.origin for Authorization

// ANTI-PATTERN
modifier onlyOwner() {
    require(tx.origin == owner, "not owner");
    _;
}

tx.origin is the EOA that initiated the transaction, regardless of how many contracts intermediate the call chain. If the owner is tricked into calling a malicious contract (a phishing dApp, a poisoned router), the malicious contract can call the vulnerable contract and pass the tx.origin == owner check.

Correct form:

modifier onlyOwner() {
    require(msg.sender == owner, "not owner");
    _;
}

The only legitimate use of tx.origin is to exclude contract callers (require(tx.origin == msg.sender) to enforce EOA-only) — and even this is fragile, since the introduction of EIP-7702 in Pectra makes EOAs capable of executing contract code.

Full treatment: Section 3.7.3 (Access & Authorization Patterns); Section 3.8.4 (Access Control Failures).

Unprotected Initializer

// ANTI-PATTERN
function initialize(address admin) external {
    _grantRole(DEFAULT_ADMIN_ROLE, admin);
}

In upgradeable contracts using the initializer pattern, an unprotected initialize function can be called by anyone after deployment. The first caller becomes the admin. This was the exact root cause of the second Parity multi-sig bug ($280M frozen).

Correct form:

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

contract MyContract is Initializable {
    function initialize(address admin) external initializer {
        _grantRole(DEFAULT_ADMIN_ROLE, admin);
    }
}

The initializer modifier ensures the function can only run once. Equally important: the deployment script must call initialize in the same transaction as the proxy deployment, or atomically via a factory — otherwise an attacker can front-run the call.

Full treatment: Section 3.5 (Smart Contract Upgradeability); Section 3.7.3 (Access & Authorization Patterns).

Modifier-Only Auth Without Reverts

// ANTI-PATTERN
modifier onlyOwner() {
    if (msg.sender == owner) {
        _;
    }
    // No revert — function silently does nothing for non-owners
}

The function appears to succeed (returns true, emits no error event) when called by a non-owner. Off-chain code interpreting "success" as "the operation happened" gets the wrong answer.

Correct form:

modifier onlyOwner() {
    require(msg.sender == owner, "not owner");
    _;
}

Or with a custom error for gas efficiency:

error Unauthorized(address caller);

modifier onlyOwner() {
    if (msg.sender != owner) revert Unauthorized(msg.sender);
    _;
}

Using Public When External Suffices

// ANTI-PATTERN (functional but wasteful and exposes attack surface)
function withdraw(uint256 amount) public { /* ... */ }

public functions can be called both externally and internally; the compiler copies arguments from calldata to memory unconditionally. external functions can only be called externally, allowing the compiler to leave arguments in calldata. The gas difference is small per call but adds up; more importantly, public widens the API surface beyond what's intended.

Correct form: Use external for functions called only from outside the contract; use internal (not private, unless the function should never be overridden) for helpers.

function withdraw(uint256 amount) external { /* ... */ }
function _calculatePenalty(uint256 amount) internal view returns (uint256) { /* ... */ }

External Calls

Unchecked Return Values

// ANTI-PATTERN
function distribute(address recipient, uint256 amount) external {
    recipient.call{value: amount}("");  // result discarded
}

Low-level calls (call, delegatecall, staticcall, send) return a boolean indicating success. Discarding this value means the contract continues as if the call succeeded even when it didn't, potentially in a corrupted state. The historical exploits include Etherpot, King of the Ether Throne, and an early version of BTC Relay.

Correct form:

function distribute(address recipient, uint256 amount) external {
    (bool ok, ) = recipient.call{value: amount}("");
    require(ok, "transfer failed");
}

For ERC-20 token transfers, use SafeERC20.safeTransfer which performs both the return-value check and handles tokens that don't return a boolean.

Full treatment: Section 4.11.11 (Unchecked Return Values).

Caller Account Type Assumptions

// ANTI-PATTERN
modifier eoaOnly() {
    require(msg.sender == tx.origin, "no contracts");
    _;
}

Two problems. First, this excludes legitimate use cases — smart contract wallets (Safe, Argent, Account Abstraction wallets) are contracts but are user-controlled and should be allowed. Second, the check is bypassable: a contract calling during its own constructor has no code yet, so msg.sender == tx.origin may hold inside the constructor of a contract callee.

After EIP-7702 (Pectra), EOAs can also execute contract code temporarily during a transaction. The tx.origin == msg.sender heuristic is becoming meaningless.

Correct form: Don't try to exclude contract callers. If you need to enforce something specific (e.g., "this function must be called atomically with its prerequisites"), use the actual property you want — typically commit-reveal, signatures, or per-block state checks.

Full treatment: Section 3.7.4 (External Interaction Patterns) for commit-reveal; Section 3.10 case studies for historical exploits relying on the EOA assumption.

Hardcoded Gas Limits

// ANTI-PATTERN
recipient.call{value: amount, gas: 2300}("");  // assumes recipient is an EOA

The infamous 2300 gas stipend from the transfer() and send() functions was once the safe-by-default Ether-sending pattern. The 2300 gas was supposed to be enough for an event emission but not enough for re-entrant logic — a kind of accidental reentrancy defense.

Several EVM gas-cost changes (most consequentially EIP-2929 in the Berlin hard fork) increased the gas costs of basic operations, meaning 2300 gas is no longer enough even for legitimate recipients to log a basic event. Contracts that hardcode 2300 gas may now fail to pay any contract recipient. Future gas-cost changes will continue this drift.

Correct form: Use call{value: amount}("") without a gas limit, and add an explicit reentrancy guard for safety. The 2300-gas mechanism is obsolete.

(bool ok, ) = recipient.call{value: amount}("");
require(ok);

Assuming External Calls Don't Reenter

Section 3.7.1 and 3.8.2 cover reentrancy in depth. The anti-pattern here is the belief that certain calls don't reenter:

• "ERC-20 transfer is safe" — true for standard ERC-20s, but ERC-777, ERC-1363, and various "fee-on-transfer" tokens execute code on the recipient.
• "An ETH transfer to an EOA can't trigger anything" — true for plain EOAs, but EIP-7702 changes this in Pectra. Account-abstraction wallets (ERC-4337) execute code on receipt.
• "Internal contract calls can't reenter" — true for internal Solidity functions, but a chain of contract calls through external interfaces is reentrancy-eligible.

Correct form: Treat every external call as potentially reentrant. Apply CEI (Section 3.7.1) and nonReentrant (Section 3.7.1) by default.

Arithmetic & Logic

Division Before Multiplication

// ANTI-PATTERN
function calculateReward(uint256 amount, uint256 rate, uint256 totalRate) external pure returns (uint256) {
    return amount / totalRate * rate;  // precision loss
}

Integer division truncates. Performing division before multiplication loses precision that multiplication-before-division would preserve. For small amounts or high precision requirements, this can result in rewards rounding to zero entirely.

Correct form:

function calculateReward(uint256 amount, uint256 rate, uint256 totalRate) external pure returns (uint256) {
    return amount * rate / totalRate;  // preserves precision
}

For more complex calculations, use OpenZeppelin's Math.mulDiv which handles 256-bit intermediate results and avoids overflow during the multiplication.

import "@openzeppelin/contracts/utils/math/Math.sol";

return Math.mulDiv(amount, rate, totalRate);

Full treatment: Section 3.8.3 (Arithmetic & Precision).

Rounding Direction Ignored

// ANTI-PATTERN: always rounds down, regardless of context
function shareToAsset(uint256 shares, uint256 totalAssets, uint256 totalShares) external pure returns (uint256) {
    return shares * totalAssets / totalShares;
}

When a calculation determines how much a user gets (withdrawal, redemption), rounding down favors the protocol. When determining how much a user owes (deposit, mint), rounding down favors the user — and creates an exploit where a user can deposit zero shares' worth and still receive accounting credit.

The ERC-4626 "inflation attack" exploits exactly this. An attacker donates assets directly to a vault to inflate the asset-per-share ratio, then a victim's deposit rounds down to zero shares while the assets remain in the vault.

Correct form: Choose rounding direction based on which party benefits. OpenZeppelin's Math library has explicit Math.Rounding modes:

import "@openzeppelin/contracts/utils/math/Math.sol";

// Rounding up when calculating shares for a deposit
function assetsToShares(uint256 assets) public view returns (uint256) {
    return Math.mulDiv(assets, totalShares, totalAssets, Math.Rounding.Floor);  // disadvantages depositor
}

// Rounding down when calculating assets for a redemption
function sharesToAssets(uint256 shares) public view returns (uint256) {
    return Math.mulDiv(shares, totalAssets, totalShares, Math.Rounding.Floor);  // advantages protocol
}

Full treatment: Section 3.8.3; Section 3.11 (Advanced Contract Security) for ERC-4626 inflation attacks specifically.

Off-By-One in Bounds Checks

// ANTI-PATTERN
require(index < array.length, "out of bounds");
return array[index];  // OK

// But:
require(index <= array.length, "out of bounds");  // off-by-one
return array[index];  // reverts when index == array.length

Off-by-one errors in bounds checks usually trigger reverts in Solidity 0.8+ rather than silent corruption, which is the safer failure mode. But the opposite off-by-one — using <= to set bounds and then incrementing — is a frequent source of bugs.

Correct form: Use Foundry's bounds-checking tests:

function test_indexAtLengthReverts() public {
    vm.expectRevert();
    contract.access(array.length);
}

function test_indexAtLengthMinusOneSucceeds() public {
    contract.access(array.length - 1);
}

State & Storage

Storage Layout Drift in Upgrades

// ANTI-PATTERN: V2 of an upgradeable contract
contract VaultV2 {
    address public owner;
    bool public paused;        // NEW: inserted in the middle
    uint256 public totalDeposits;
    mapping(address => uint256) public balances;
}

If V1 had (owner, totalDeposits, balances), every storage variable below paused has shifted to the wrong slot. The contract appears to compile and deploy fine but reads garbage.

Correct form: Append new variables to the end of the inheritance chain, never insert in the middle. Or use explicit storage buckets (ERC-7201) which make the layout deterministic regardless of declaration order.

// CORRECT V2
contract VaultV2 {
    address public owner;
    uint256 public totalDeposits;
    mapping(address => uint256) public balances;
    bool public paused;  // appended at end
}

Full treatment: Section 3.7.2 (State & Storage Patterns); Section 3.5 (Smart Contract Upgradeability).

Public State Variables That Reveal Secrets

// ANTI-PATTERN
contract Auction {
    uint256 public secretBidThreshold;  // visible to anyone reading storage
}

"Private" and "internal" in Solidity only restrict access from other contracts and external calls. The storage itself is on-chain and readable by anyone via eth_getStorageAt. There is no on-chain secrecy.

Correct form: If the value must be secret, it cannot live on-chain in plaintext. Options:

• Commit-Reveal — store only a hash; reveal the value later when secrecy no longer matters (Section 3.7.4)
• Off-chain — keep the secret off-chain, post only signatures or proofs on-chain
• Encryption — store encrypted values; impractical for most cases since the contract cannot decrypt

The anti-pattern is believing a private variable is secret. Mark this assumption explicitly:

// Stored on-chain - not secret. Treat as public.
uint256 internal threshold;

Reading from Storage in Loops

// ANTI-PATTERN
function totalStaked() external view returns (uint256) {
    uint256 total;
    for (uint256 i = 0; i < stakers.length; ++i) {  // reads stakers.length from storage each iteration
        total += balances[stakers[i]];               // reads balances[...] from storage each iteration
    }
    return total;
}

Each storage read costs at minimum 100 gas (warm) or 2,100 gas (cold). In a loop, the cost compounds. More dangerously, the contract may grow to a point where this loop exceeds the block gas limit and becomes uncallable.

Correct form:

function totalStaked() external view returns (uint256) {
    uint256 total;
    address[] memory _stakers = stakers;  // copy once to memory
    uint256 len = _stakers.length;
    for (uint256 i = 0; i < len; ++i) {
        total += balances[_stakers[i]];
    }
    return total;
}

Better: don't accumulate state in unbounded structures that require iteration. Use running totals updated on each state change.

Full treatment: Section 4.11.6 (Gas Vulnerabilities); Section 4.11.7 (DoS).

Time & Randomness

block.timestamp for Randomness

// ANTI-PATTERN
function pickWinner() external view returns (address) {
    uint256 random = uint256(keccak256(abi.encode(block.timestamp))) % participants.length;
    return participants[random];
}

Block timestamps are set by block proposers, who have several seconds of latitude. A proposer can choose the timestamp that produces a winner they prefer. Combining timestamp with block.difficulty (now block.prevrandao after the Merge), blockhash, or msg.sender doesn't help — all of these are either equally manipulable or known in advance.

Correct form: Use a verifiable randomness source. Chainlink VRF is the standard solution; for less critical applications, commit-reveal between participants can suffice. For prevrandao-only randomness with acceptable risk, wait at least 2 blocks beyond the commit to make manipulation prohibitively expensive.

import "@chainlink/contracts/src/v0.8/vrf/VRFConsumerBaseV2.sol";

// Use VRF for any random outcome with monetary value

Full treatment: Section 4.11.5 (Timestamp Dependence).

Timestamp for Time-Sensitive Logic

// ANTI-PATTERN
require(block.timestamp <= deadline, "expired");

This is not always wrong — for human-scale deadlines (an hour, a day), the block proposer's few-second window doesn't matter. But for short deadlines (within a single block, or comparing timestamps in the same transaction), the timestamp can be manipulated.

Correct form: Use timestamps for human-scale durations only. For block-scale precision, use block.number:

require(block.number <= deadlineBlock, "expired");

For protocol-level time tracking (e.g., interest accrual), use block.timestamp but make sure the math is monotonic and handles small backward jitter (some chains allow timestamps to drift slightly backward).

Solidity Language Pitfalls

Floating Pragma

// ANTI-PATTERN
pragma solidity ^0.8.0;

The caret pragma accepts any 0.8.x version. A library compiled with 0.8.0 and a consumer compiled with 0.8.24 may produce different bytecode for the same source — typically harmless but occasionally meaningful. More problematically, the floating pragma means deployment may compile with a different compiler version than the audit was performed against, defeating the audit.

Correct form: Pin the compiler version exactly.

pragma solidity 0.8.24;

In libraries intended for wide reuse, a tighter range (e.g., >=0.8.20 <0.9.0) is acceptable since the library author may not know the consumer's compiler version. For application contracts, pin exactly.

Variable Shadowing

// ANTI-PATTERN
contract Vault {
    uint256 public balance;

    function deposit(uint256 balance) external payable {  // parameter shadows state var
        // Now `balance` refers to the parameter, not the state variable
        // Author probably intended to update state but is updating the parameter
        balance = msg.value;
    }
}

Solidity allows shadowing without error. The compiler emits a warning but doesn't fail. In code generated through templating or auto-completion, shadows can sneak in.

Correct form: Use distinct names for parameters and state variables. Conventional patterns include underscore prefixes for parameters (_balance) or new prefixes for "the value being set" (newBalance).

function deposit(uint256 newBalance) external payable {
    balance = newBalance;
}

Modern Solidity linters (e.g., Solhint) flag shadowing by default; enable them in CI.

Fallback and Receive Confusion

// ANTI-PATTERN: only fallback, no receive
contract Confused {
    fallback() external payable {
        // Handles BOTH plain ETH transfers AND calls with calldata
        // The contract may not distinguish, leading to bugs
    }
}

receive() handles plain ETH transfers (no calldata). fallback() handles calls with unrecognized function selectors. A contract with only fallback() (and no receive()) routes plain ETH transfers through fallback(), which can confuse the logic.

Correct form: Implement both if both are needed, or neither:

contract Clear {
    receive() external payable {
        // Logic specific to receiving plain ETH
    }

    fallback() external payable {
        // Logic specific to unrecognized function calls
    }
}

If the contract should reject unexpected ETH transfers, omit receive() and have fallback() revert. If both should accept ETH but with the same logic, factor the logic into an internal function called from both.

assert for Input Validation

// ANTI-PATTERN
function withdraw(uint256 amount) external {
    assert(balance[msg.sender] >= amount);  // wrong tool
    balance[msg.sender] -= amount;
}

assert is designed for invariant checks that should never be false in correct code. Pre-0.8 it consumed all remaining gas; even in 0.8+, it indicates a Panic error rather than a graceful revert, which downstream tooling treats as a bug in the contract rather than a user error.

Correct form: Use require for input validation, revert with a custom error for the same with a cheaper error encoding, and reserve assert for genuine invariants:

function withdraw(uint256 amount) external {
    require(balance[msg.sender] >= amount, "insufficient");
    balance[msg.sender] -= amount;
    assert(balance[msg.sender] <= type(uint256).max);  // genuine invariant
}

Operational

Magic Numbers in Code

// ANTI-PATTERN
require(elapsedTime > 604800, "too soon");  // what is 604800?

Magic numbers are gas-equivalent to named constants but vastly worse for review and maintenance. The example above is one week in seconds — the next maintainer of this code has to derive that fact from context.

Correct form:

uint256 public constant ONE_WEEK = 7 days;

require(elapsedTime > ONE_WEEK, "too soon");

Use Solidity's built-in time and ether unit suffixes (1 days, 1 weeks, 1 ether, 1 gwei) where applicable; they compile to the same constants and read better.

Missing Events for State Changes

// ANTI-PATTERN
function setFeeRate(uint256 newRate) external onlyOwner {
    feeRate = newRate;
    // No event emitted
}

Off-chain monitoring tools, indexers, and the protocol's own dashboards depend on events to track state changes. A change without an event is invisible to anything not watching storage directly.

Correct form:

event FeeRateUpdated(uint256 oldRate, uint256 newRate);

function setFeeRate(uint256 newRate) external onlyOwner {
    emit FeeRateUpdated(feeRate, newRate);
    feeRate = newRate;
}

Indexed parameters in events make filtering efficient — index the parameter the indexer will most often filter on (often the affected user's address, or an entity ID).

Hard-coded Addresses

// ANTI-PATTERN
IERC20 public constant USDC = IERC20(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48);

Hard-coding an address makes the contract non-portable: it cannot be tested against a mock, cannot be deployed on a testnet (where the address differs), and cannot be migrated if the dependency moves to a new address.

Correct form: Pass dependencies through the constructor or an initializer:

IERC20 public immutable usdc;

constructor(address _usdc) {
    usdc = IERC20(_usdc);
}

Mainnet deployment scripts hardcode the address at deployment time. Test deployments inject mocks. The contract source is reusable.

Deploying Without Verifying Source

Etherscan verification publishes the source code alongside the deployed bytecode, allowing anyone to audit what's actually running. An unverified contract is a black box; users, integrators, and security researchers cannot independently confirm what it does.

Deployment workflows should include verification as a required step. Foundry's forge verify-contract automates this; deployment scripts can be configured to verify automatically on successful deployment.

This is not a code-level anti-pattern but appears in this catalog because the absence of verification is itself a recurring failure that masks other issues.

Quick Reference

Categorized for fast scanning during code review:

Identity & Auth: tx.origin auth, unprotected initializers, silent modifiers, public-instead-of-external External Calls: unchecked return values, EOA-only checks, hardcoded gas limits, "doesn't reenter" assumptions Arithmetic: division before multiplication, rounding direction ignored, off-by-one bounds State: layout drift in upgrades, "private" implying secret, storage reads in loops Time & Randomness: block.timestamp as randomness, block.timestamp for short deadlines Solidity: floating pragma, variable shadowing, fallback/receive confusion, assert for input validation Operational: magic numbers, missing events, hardcoded addresses, unverified deployment

Cross-References

The deeper "why" and "how" for each anti-pattern lives elsewhere:

• Section 3.7.1 — Reentrancy patterns and CEI
• Section 3.7.2 — Storage layout patterns
• Section 3.7.3 — Access control patterns
• Section 3.7.4 — Commit-reveal, signatures, multicall
• Section 3.7.5 — Defensive patterns (pause, rate limit)
• Section 3.7.6 — Optimization trade-offs
• Section 3.8 — Common Vulnerabilities (full vulnerability treatment)
• Section 3.10 — Past Exploits (historical context)
• Section 4.11 — Auditor's detection heuristics for these patterns

The catalog above is the developer's pre-review checklist. The auditor's framing of the same material lives in Book 4.