Upgrade Safety

// VULNERABLE: deployment and initialization are separate transactions
// Deploy proxy → attacker calls initialize() before admin
proxy = new ERC1967Proxy(implementation, "");
// ...gap where attacker calls initialize()...
VaultV1(proxy).initialize(admin);

// SAFE: initialize in the deployment transaction
bytes memory initData = abi.encodeCall(VaultV1.initialize, (admin));
proxy = new ERC1967Proxy(implementation, initData);

contract VaultV1 is Initializable, OwnableUpgradeable {
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();
    }

    function initialize(address admin) external initializer {
        __Ownable_init(admin);
    }
}

// V1 Layout
contract VaultV1 {
    address public owner;       // slot 0
    uint256 public totalSupply; // slot 1
    uint256[48] private __gap;
}

// V2 BAD: inserted variable
contract VaultV2Bad {
    address public owner;       // slot 0
    address public treasury;    // slot 1 ← WAS totalSupply!
    uint256 public totalSupply; // slot 2 ← shifted, reads __gap[0]
    uint256[47] private __gap;
}

// V2 GOOD: appended variable
contract VaultV2Good {
    address public owner;       // slot 0
    uint256 public totalSupply; // slot 1
    address public treasury;    // slot 2 ← new, was __gap[0]
    uint256[47] private __gap;  // reduced from 48 to 47
}

# Compare storage layouts between versions
forge inspect src/VaultV1.sol:VaultV1 storage-layout --pretty > layout-v1.txt
forge inspect src/VaultV2.sol:VaultV2 storage-layout --pretty > layout-v2.txt
diff layout-v1.txt layout-v2.txt

// DANGEROUS: never include selfdestruct in implementations
contract BadImpl {
    function destroy() external onlyOwner {
        selfdestruct(payable(owner)); // bricks all proxies
    }
}

// Post-Dencun (EIP-6780): selfdestruct only sends ETH unless in the
// same transaction as creation. Still, never use it in implementations.

// VULNERABLE: custom initializer without reentrancy protection
bool private _initialized;

function initialize(address admin) external {
    require(!_initialized);
    // External call before setting _initialized
    ICallback(admin).onInitialize(); // can re-enter initialize()
    _initialized = true;
}

// SAFE: use OpenZeppelin's Initializable
function initialize(address admin) external initializer {
    // modifier prevents reentrancy
}

// BAD: no gap in base contract
abstract contract BaseV1 {
    uint256 public value;
    // no __gap — adding variables here shifts derived contract storage
}

contract DerivedV1 is BaseV1 {
    uint256 public derivedValue; // slot 1
}

// GOOD: gap in base contract
abstract contract BaseV1 {
    uint256 public value;
    uint256[49] private __gap;
}

// Now BaseV2 can safely add variables:
abstract contract BaseV2 {
    uint256 public value;
    uint256 public newValue; // uses first __gap slot
    uint256[48] private __gap;
}

// TransparentUpgradeableProxy solves this by routing:
// - Admin calls → proxy logic (upgrade, admin functions)
// - Non-admin calls → delegated to implementation

// UUPS: implementation must include upgradeTo function
// Risk: implementation function selector collides with ERC-1967 functions

// Check selectors:
// upgradeTo(address) → 0x3659cfe6
// Ensure no implementation function has selector 0x3659cfe6

# Detect selector collisions
forge inspect src/Vault.sol:Vault methodIdentifiers | sort
forge inspect src/Proxy.sol:Proxy methodIdentifiers | sort
# Compare for duplicates

import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract VaultV1 is UUPSUpgradeable, OwnableUpgradeable {
    function _authorizeUpgrade(address newImplementation)
        internal
        override
        onlyOwner
    {
        // Can add additional validation here
        // e.g., check newImplementation supports expected interface
    }
}

// V2 MUST also include _authorizeUpgrade
// If omitted, proxy is permanently locked to V2
contract VaultV2 is UUPSUpgradeable, OwnableUpgradeable {
    function _authorizeUpgrade(address newImplementation)
        internal
        override
        onlyOwner
    {}
}

# Foundry: use forge-upgrades
forge install OpenZeppelin/openzeppelin-foundry-upgrades

# In tests: validate upgrade safety
import {Upgrades} from "openzeppelin-foundry-upgrades/Upgrades.sol";

function testUpgradeSafety() public {
    Upgrades.validateUpgrade("VaultV2.sol:VaultV2", referenceContract);
}