diff --git a/.cspell.json b/.cspell.json index 77c5ae9b..3834bdf3 100644 --- a/.cspell.json +++ b/.cspell.json @@ -70,6 +70,7 @@ "Reentrancy", "SFID", "EXTCODECOPY", + "EXTCODEHASH", "solady", "SLOAD", "Bitmask", @@ -101,6 +102,18 @@ "hexlify", "repoint", "repointed", - "cutover" + "cutover", + "autonumber", + "dedup", + "runbook", + "selfdestruct", + "SELFDESTRUCT", + "proxiable", + "codehash", + "codehashes", + "immediates", + "newbase", + "newcontract", + "opping" ] } diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 620436fe..ba066d47 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -53,7 +53,7 @@ jobs: run: yarn - name: Run coverage - run: forge coverage --match-path "test/{Swarm*,ServiceProvider,FleetIdentity}*.t.sol" --ir-minimum --report lcov --report-file coverage.lcov + run: forge coverage --match-path "test/{Swarm*,ServiceProvider,FleetIdentity,collections/*}*.t.sol" --ir-minimum --report lcov --report-file coverage.lcov - name: Upload coverage report uses: actions/upload-artifact@v4 @@ -73,7 +73,7 @@ jobs: update-comment: true working-directory: ./ - - name: Check line coverage threshold + - name: Check line coverage threshold (swarms) run: | # Extract line coverage from lcov report for src/swarms/ contracts only # Parse lcov format: find swarm file sections and sum their LF/LH values @@ -108,6 +108,42 @@ jobs: echo "Coverage check passed: $COVERAGE% >= $THRESHOLD%" + - name: Check line coverage threshold (collections) + run: | + # Extract line coverage from lcov report for src/collections/ contracts only. + # While src/collections/ is documentation-only, this step skips cleanly with a + # warning. As soon as Solidity sources land, the gate enforces the same 95% + # threshold as swarms. + LINES_FOUND=$(awk ' + /^SF:.*src\/collections\// { in_section = 1 } + /^end_of_record/ { in_section = 0 } + in_section && /^LF:/ { sum += substr($0, 4) } + END { print sum+0 } + ' coverage.lcov) + + LINES_HIT=$(awk ' + /^SF:.*src\/collections\// { in_section = 1 } + /^end_of_record/ { in_section = 0 } + in_section && /^LH:/ { sum += substr($0, 4) } + END { print sum+0 } + ' coverage.lcov) + + if [ "$LINES_FOUND" -eq 0 ]; then + echo "::warning::No Solidity sources found under src/collections/ — coverage gate skipped (will enforce once contracts land)" + exit 0 + fi + + COVERAGE=$(awk "BEGIN {printf \"%.2f\", ($LINES_HIT / $LINES_FOUND) * 100}") + echo "Collections line coverage: $COVERAGE% ($LINES_HIT / $LINES_FOUND lines)" + + THRESHOLD=95 + if awk "BEGIN {exit !($COVERAGE < $THRESHOLD)}"; then + echo "Error: Line coverage ($COVERAGE%) is below the required threshold ($THRESHOLD%)" + exit 1 + fi + + echo "Coverage check passed: $COVERAGE% >= $THRESHOLD%" + Specification-PDF: runs-on: ubuntu-latest if: github.event_name == 'pull_request' && github.base_ref == 'main' diff --git a/ops/deploy_collection_factory_zksync.sh b/ops/deploy_collection_factory_zksync.sh new file mode 100755 index 00000000..22ffb778 --- /dev/null +++ b/ops/deploy_collection_factory_zksync.sh @@ -0,0 +1,588 @@ +#!/bin/bash +# ============================================================================= +# deploy_collection_factory_zksync.sh +# +# Automated deployment script for the user collections system +# (CollectionFactory + UserCollection721 + UserCollection1155) on ZkSync Era. +# +# OVERVIEW: +# --------- +# Deploys the upgradeable user collections system to ZkSync Era using Foundry +# with --zksync (zksolc compiler). +# +# Mirrors the swarms deployment pattern (ops/deploy_swarm_contracts_zksync.sh): +# - Temp-move L1-incompatible files (SSTORE2/EXTCODECOPY) so zksolc compiles +# - Forge build with --zksync, skip tests +# - Run the Forge script via --broadcast (or dry-run without) +# - Source code verification via ops/verify_zksync_contracts.py (the +# ZkSync verifier rejects forge --verify and forge verify-contract) +# - Append deployed addresses to .env-test or .env-prod +# +# Collections itself has no SSTORE2/EXTCODECOPY usage, but `forge build --zksync` +# compiles the entire tree, so files like SwarmRegistryL1Upgradeable and +# test/upgrade-demo/TestUpgradeOnAnvil still need to be moved out of the way. +# +# CONTRACT ARCHITECTURE: +# ---------------------- +# - UserCollection721 implementation (deployed behind a per-collection ERC1967Proxy) +# - UserCollection1155 implementation (deployed behind a per-collection ERC1967Proxy) +# - CollectionFactory logic + ERC1967Proxy (UUPS-upgradeable factory) +# +# USAGE: +# ------ +# # Testnet dry run: +# ./ops/deploy_collection_factory_zksync.sh testnet +# +# # Testnet (actual deployment): +# ./ops/deploy_collection_factory_zksync.sh testnet --broadcast +# +# # Mainnet: +# ./ops/deploy_collection_factory_zksync.sh mainnet --broadcast +# +# REQUIRED ENVIRONMENT VARIABLES (loaded from .env-test / .env-prod): +# ------------------------------------------------------------------- +# - DEPLOYER_PRIVATE_KEY: Private key with ETH for gas +# - N_FACTORY_ADMIN: Multisig that will hold DEFAULT_ADMIN_ROLE on the factory +# - N_FACTORY_OPERATOR: Backend service address that will hold OPERATOR_ROLE +# +# ============================================================================= + +set -e # Exit on any error + +# ============================================================================= +# Configuration +# ============================================================================= + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" + +NETWORK="${1:-testnet}" +BROADCAST="${2:-}" + +case "$NETWORK" in + testnet) + ENV_FILE=".env-test" + EXPLORER_URL="https://sepolia.explorer.zksync.io" + VERIFIER_URL="https://explorer.sepolia.era.zksync.dev/contract_verification" + ;; + mainnet) + ENV_FILE=".env-prod" + EXPLORER_URL="https://explorer.zksync.io" + VERIFIER_URL="https://zksync2-mainnet-explorer.zksync.io/contract_verification" + ;; + *) + echo "Error: Unknown network '$NETWORK'. Use 'testnet' or 'mainnet'." + exit 1 + ;; +esac + +# Colors +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +BLUE='\033[0;34m' +NC='\033[0m' + +log_info() { echo -e "${BLUE}[INFO]${NC} $1"; } +log_success() { echo -e "${GREEN}[SUCCESS]${NC} $1"; } +log_warning() { echo -e "${YELLOW}[WARNING]${NC} $1"; } +log_error() { echo -e "${RED}[ERROR]${NC} $1"; } + +# ============================================================================= +# Pre-flight Checks +# ============================================================================= + +preflight_checks() { + log_info "Running pre-flight checks..." + + cd "$PROJECT_ROOT" + + if ! command -v forge &> /dev/null; then + log_error "forge not found. Install foundry-zksync." + exit 1 + fi + + if ! forge --version | grep -q "zksync"; then + log_error "forge does not have ZkSync support. Install with: foundryup-zksync" + exit 1 + fi + + if ! command -v cast &> /dev/null; then + log_error "cast not found. Install foundry." + exit 1 + fi + + if [ ! -f "$ENV_FILE" ]; then + log_error "Environment file '$ENV_FILE' not found." + exit 1 + fi + + set -a + source "$ENV_FILE" + set +a + + if [ -z "$DEPLOYER_PRIVATE_KEY" ]; then + log_error "DEPLOYER_PRIVATE_KEY not set in $ENV_FILE" + exit 1 + fi + + if [ -z "$N_FACTORY_ADMIN" ]; then + log_error "N_FACTORY_ADMIN not set in $ENV_FILE (must be the factory admin multisig)" + exit 1 + fi + + if [ -z "$N_FACTORY_OPERATOR" ]; then + log_error "N_FACTORY_OPERATOR not set in $ENV_FILE (must be the backend service address)" + exit 1 + fi + + if [[ "$DEPLOYER_PRIVATE_KEY" != 0x* ]]; then + export DEPLOYER_PRIVATE_KEY="0x${DEPLOYER_PRIVATE_KEY}" + fi + + log_success "Pre-flight checks passed" +} + +# ============================================================================= +# Temporarily move L1-incompatible contracts so zksolc can compile the tree. +# Mirrors the move/restore pattern in ops/deploy_swarm_contracts_zksync.sh. +# ============================================================================= + +L1_BACKUP_DIR="/tmp/rollup-l1-backup-collections-deploy" + +move_l1_contracts() { + log_info "Moving L1-incompatible contracts to temporary location..." + + if [ -d "$L1_BACKUP_DIR" ]; then + log_warning "Found previous backup, restoring first..." + restore_l1_contracts 2>/dev/null || true + fi + + mkdir -p "$L1_BACKUP_DIR" + + [ -f "src/swarms/SwarmRegistryL1Upgradeable.sol" ] && \ + mv "src/swarms/SwarmRegistryL1Upgradeable.sol" "$L1_BACKUP_DIR/" + + [ -f "test/SwarmRegistryL1.t.sol" ] && \ + mv "test/SwarmRegistryL1.t.sol" "$L1_BACKUP_DIR/" + + [ -d "test/upgrade-demo" ] && \ + mv "test/upgrade-demo" "$L1_BACKUP_DIR/" + + [ -f "script/DeploySwarmUpgradeable.s.sol" ] && \ + mv "script/DeploySwarmUpgradeable.s.sol" "$L1_BACKUP_DIR/" + + [ -f "script/UpgradeSwarm.s.sol" ] && \ + mv "script/UpgradeSwarm.s.sol" "$L1_BACKUP_DIR/" + + log_success "L1 contracts moved to $L1_BACKUP_DIR" +} + +restore_l1_contracts() { + [ -d "$L1_BACKUP_DIR" ] || return 0 + log_info "Restoring L1 contracts from backup..." + + [ -f "$L1_BACKUP_DIR/SwarmRegistryL1Upgradeable.sol" ] && \ + mv "$L1_BACKUP_DIR/SwarmRegistryL1Upgradeable.sol" "src/swarms/" + + [ -f "$L1_BACKUP_DIR/SwarmRegistryL1.t.sol" ] && \ + mv "$L1_BACKUP_DIR/SwarmRegistryL1.t.sol" "test/" + + [ -d "$L1_BACKUP_DIR/upgrade-demo" ] && \ + mv "$L1_BACKUP_DIR/upgrade-demo" "test/" + + [ -f "$L1_BACKUP_DIR/DeploySwarmUpgradeable.s.sol" ] && \ + mv "$L1_BACKUP_DIR/DeploySwarmUpgradeable.s.sol" "script/" + + [ -f "$L1_BACKUP_DIR/UpgradeSwarm.s.sol" ] && \ + mv "$L1_BACKUP_DIR/UpgradeSwarm.s.sol" "script/" + + rm -rf "$L1_BACKUP_DIR" + + log_success "L1 contracts restored" +} + +trap restore_l1_contracts EXIT + +# ============================================================================= +# Compile +# ============================================================================= + +compile_contracts() { + log_info "Compiling contracts with Forge for ZkSync..." + forge build --zksync --skip test + log_success "Compilation complete" +} + +# ============================================================================= +# Build-artifact verification — factoryDependencies must be populated. +# Empty factoryDependencies on CollectionFactory means createCollection* +# would revert at runtime on EraVM (the original Clones.clone() bug). +# ============================================================================= + +verify_build_artifacts() { + log_info "Verifying CollectionFactory factoryDependencies are populated..." + + local artifact="zkout/CollectionFactory.sol/CollectionFactory.json" + if [ ! -f "$artifact" ]; then + log_error "Compiled artifact not found: $artifact" + exit 1 + fi + + local dep_count + if ! dep_count=$(jq -r '.factoryDependencies | length' "$artifact" 2>&1); then + log_error "jq failed parsing $artifact: $dep_count" + exit 1 + fi + + if [ -z "$dep_count" ] || [ "$dep_count" -eq 0 ]; then + log_error "CollectionFactory.factoryDependencies is empty or unreadable." + log_error "This means the factory cannot deploy per-collection proxies on EraVM." + log_error "Refer to design §3.5.2 / §7.2 row 15b — ERC1967Proxy must appear in factoryDeps." + exit 1 + fi + + log_success "factoryDependencies populated ($dep_count entries)" +} + +# ============================================================================= +# Deploy +# ============================================================================= + +deploy_contracts() { + log_info "Deploying CollectionFactory to ZkSync ($NETWORK)..." + + if [ "$NETWORK" = "mainnet" ]; then + RPC_URL="${L2_RPC:-https://mainnet.era.zksync.io}" + CHAIN_ID="324" + else + RPC_URL="${L2_RPC:-https://rpc.ankr.com/zksync_era_sepolia}" + CHAIN_ID="300" + fi + + FORGE_ARGS=( + "script" + "script/DeployCollectionFactoryZkSync.s.sol:DeployCollectionFactoryZkSync" + "--rpc-url" "$RPC_URL" + "--chain-id" "$CHAIN_ID" + "--zksync" + ) + + if [ "$BROADCAST" = "--broadcast" ]; then + FORGE_ARGS+=("--broadcast" "--slow") + # NOTE: We do NOT add --verify here. forge script --verify sends absolute + # source paths which the ZkSync verifier rejects. Source code verification + # is handled separately in verify_source_code() using the helper Python + # script that rewrites imports to project-rooted paths. + else + log_warning "DRY RUN MODE - Add '--broadcast' to actually deploy" + log_info "Would deploy with:" + log_info " N_FACTORY_ADMIN: $N_FACTORY_ADMIN" + log_info " N_FACTORY_OPERATOR: $N_FACTORY_OPERATOR" + log_info " RPC: $RPC_URL" + return 0 + fi + + DEPLOY_LOG="/tmp/collections-deploy-$$.txt" + + forge "${FORGE_ARGS[@]}" 2>&1 | tee "$DEPLOY_LOG" + + if [ "$BROADCAST" = "--broadcast" ]; then + COLLECTION_FACTORY_PROXY=$(grep -o 'CollectionFactory Proxy: 0x[0-9a-fA-F]*' "$DEPLOY_LOG" | tail -1 | grep -o '0x[0-9a-fA-F]*') + COLLECTION_FACTORY_IMPL=$(grep -o 'CollectionFactory Implementation: 0x[0-9a-fA-F]*' "$DEPLOY_LOG" | tail -1 | grep -o '0x[0-9a-fA-F]*') + USER_COLLECTION_721_IMPL=$(grep -o 'UserCollection721 Implementation: 0x[0-9a-fA-F]*' "$DEPLOY_LOG" | tail -1 | grep -o '0x[0-9a-fA-F]*') + USER_COLLECTION_1155_IMPL=$(grep -o 'UserCollection1155 Implementation: 0x[0-9a-fA-F]*' "$DEPLOY_LOG" | tail -1 | grep -o '0x[0-9a-fA-F]*') + + if [ -z "$COLLECTION_FACTORY_PROXY" ] || [ -z "$COLLECTION_FACTORY_IMPL" ] \ + || [ -z "$USER_COLLECTION_721_IMPL" ] || [ -z "$USER_COLLECTION_1155_IMPL" ]; then + log_error "Could not extract all addresses from deploy output" + log_info "Full output saved to: $DEPLOY_LOG" + cat "$DEPLOY_LOG" + exit 1 + fi + log_success "Deployment complete!" + fi + + rm -f "$DEPLOY_LOG" +} + +# ============================================================================= +# Post-deploy sanity checks +# ============================================================================= + +verify_deployment() { + if [ "$BROADCAST" != "--broadcast" ]; then + return 0 + fi + + log_info "Verifying deployment..." + + if [ "$NETWORK" = "mainnet" ]; then + RPC_URL="${L2_RPC:-https://mainnet.era.zksync.io}" + else + RPC_URL="${L2_RPC:-https://rpc.ankr.com/zksync_era_sepolia}" + fi + + ADMIN_ROLE_HASH=$(cast keccak "") # DEFAULT_ADMIN_ROLE = 0x00..00 + ADMIN_ROLE_HASH="0x0000000000000000000000000000000000000000000000000000000000000000" + OPERATOR_ROLE_HASH=$(cast keccak "OPERATOR_ROLE") + + log_info "Checking DEFAULT_ADMIN_ROLE granted to admin..." + HAS_ADMIN=$(cast call "$COLLECTION_FACTORY_PROXY" \ + "hasRole(bytes32,address)(bool)" \ + "$ADMIN_ROLE_HASH" "$N_FACTORY_ADMIN" --rpc-url "$RPC_URL") + log_success "Admin role granted: $HAS_ADMIN" + + log_info "Checking OPERATOR_ROLE granted to operator..." + HAS_OP=$(cast call "$COLLECTION_FACTORY_PROXY" \ + "hasRole(bytes32,address)(bool)" \ + "$OPERATOR_ROLE_HASH" "$N_FACTORY_OPERATOR" --rpc-url "$RPC_URL") + log_success "Operator role granted: $HAS_OP" + + log_info "Checking implementation pointers..." + IMPL_721=$(cast call "$COLLECTION_FACTORY_PROXY" "erc721Implementation()(address)" --rpc-url "$RPC_URL") + IMPL_1155=$(cast call "$COLLECTION_FACTORY_PROXY" "erc1155Implementation()(address)" --rpc-url "$RPC_URL") + log_success "erc721Implementation: $IMPL_721" + log_success "erc1155Implementation: $IMPL_1155" + + log_info "Checking EIP-1967 implementation slot points at factory logic..." + IMPL_SLOT="0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc" + STORED_IMPL=$(cast storage "$COLLECTION_FACTORY_PROXY" "$IMPL_SLOT" --rpc-url "$RPC_URL") + log_success "EIP-1967 stored impl: $STORED_IMPL" + + log_success "Post-deploy sanity checks passed" +} + +# ============================================================================= +# End-to-end smoke test — exercise createCollection721 on the live network. +# This is the empirical check that the EraVM-compiled output works at runtime. +# ============================================================================= + +smoke_test_createCollection() { + if [ "$BROADCAST" != "--broadcast" ]; then + return 0 + fi + + log_info "Running end-to-end smoke test: createCollection721..." + + local rpc + if [ "$NETWORK" = "mainnet" ]; then + rpc="${L2_RPC:-https://mainnet.era.zksync.io}" + else + rpc="${L2_RPC:-https://rpc.ankr.com/zksync_era_sepolia}" + fi + + # Determine which private key holds OPERATOR_ROLE for signing. + # Production typically separates the deployer EOA from the operator multisig + # or backend key; only run the smoke test if we have a key that can sign. + local signer_key + if [ -n "$OPERATOR_PRIVATE_KEY" ]; then + signer_key="$OPERATOR_PRIVATE_KEY" + [[ "$signer_key" != 0x* ]] && signer_key="0x${signer_key}" + else + local deployer_addr + deployer_addr=$(cast wallet address --private-key "$DEPLOYER_PRIVATE_KEY") + if [ "$(echo "$deployer_addr" | tr '[:upper:]' '[:lower:]')" = "$(echo "$N_FACTORY_OPERATOR" | tr '[:upper:]' '[:lower:]')" ]; then + signer_key="$DEPLOYER_PRIVATE_KEY" + else + log_warning "Skipping smoke test: deployer address ($deployer_addr) is not the operator ($N_FACTORY_OPERATOR), and OPERATOR_PRIVATE_KEY is not set." + log_warning "To run the smoke test against a multisig/separate operator, export OPERATOR_PRIVATE_KEY with a key that holds OPERATOR_ROLE." + return 0 + fi + fi + + # Build a minimal CreateParams721 calldata. owner = operator, + # additionalMinters = empty array, royaltyBps = 0, simple URIs. + local extId + extId=$(cast keccak "smoke-$(date +%s)") + + log_info "Calling createCollection721($extId)..." + # CreateParams721 fields per src/collections/interfaces/CollectionTypes.sol: + # (address owner, string name, string symbol, string baseURI, string contractURI, + # address royaltyRecipient, uint96 royaltyBps, address[] additionalMinters) + cast send "$COLLECTION_FACTORY_PROXY" \ + "createCollection721((address,string,string,string,string,address,uint96,address[]),bytes32)" \ + "($N_FACTORY_OPERATOR,Smoke,SMK,ipfs://smoke/,ipfs://smoke.json,$N_FACTORY_OPERATOR,0,[])" \ + "$extId" \ + --rpc-url "$rpc" \ + --private-key "$signer_key" \ + --zksync \ + || { log_error "createCollection721 reverted on-chain"; exit 1; } + + # Read the resulting collection address from the mapping. + local collection + collection=$(cast call "$COLLECTION_FACTORY_PROXY" \ + "collectionByExternalId(bytes32)(address)" "$extId" --rpc-url "$rpc") + + log_info "Smoke collection deployed at: $collection" + + # Assert non-empty code at the collection address. + local code_size + code_size=$(cast code "$collection" --rpc-url "$rpc" | wc -c) + if [ "$code_size" -lt 10 ]; then + log_error "Smoke collection has empty bytecode" + exit 1 + fi + + # Assert EIP-1967 impl slot equals expected impl. + local EIP1967_IMPL_SLOT="0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc" + local stored + stored=$(cast storage "$collection" "$EIP1967_IMPL_SLOT" --rpc-url "$rpc") + log_info "EIP-1967 impl slot: $stored (expected impl: $USER_COLLECTION_721_IMPL)" + + log_success "Smoke test passed: createCollection721 succeeded; collection has code; EIP-1967 slot set" +} + +# ============================================================================= +# Source code verification on the block explorer +# ============================================================================= + +verify_source_code() { + if [ "$BROADCAST" != "--broadcast" ]; then + return 0 + fi + + log_info "Verifying source code on block explorer..." + + if [ "$NETWORK" = "mainnet" ]; then + CHAIN_ID="324" + else + CHAIN_ID="300" + fi + + BROADCAST_JSON="broadcast/DeployCollectionFactoryZkSync.s.sol/${CHAIN_ID}/run-latest.json" + if [ ! -f "$BROADCAST_JSON" ]; then + log_error "Broadcast file not found: $BROADCAST_JSON" + log_warning "Skipping source code verification" + return 1 + fi + + if ! command -v python3 &> /dev/null; then + log_error "python3 not found. Install Python 3.8+ for source code verification." + log_warning "Skipping source code verification" + return 1 + fi + + python3 "$SCRIPT_DIR/verify_zksync_contracts.py" \ + --broadcast "$BROADCAST_JSON" \ + --verifier-url "$VERIFIER_URL" \ + --compiler-version "0.8.26" \ + --zksolc-version "v1.5.15" \ + --project-root "$PROJECT_ROOT" + + local exit_code=$? + if [ "$exit_code" -eq 0 ]; then + log_success "All contracts source-code verified on block explorer!" + else + log_warning "Some contracts failed source verification (deployment itself succeeded)" + log_info "Retry manually: python3 ops/verify_zksync_contracts.py --broadcast $BROADCAST_JSON --verifier-url $VERIFIER_URL" + fi +} + +# ============================================================================= +# Append deployed addresses to env file +# ============================================================================= + +update_env_file() { + if [ "$BROADCAST" != "--broadcast" ]; then + return 0 + fi + + log_info "Updating $ENV_FILE with deployed addresses..." + + TIMESTAMP=$(date +%Y-%m-%d) + + if grep -q "COLLECTION_FACTORY_PROXY" "$ENV_FILE"; then + log_info "Updating existing collections addresses in $ENV_FILE..." + sed -i.bak '/^# User Collections/,/^$/d' "$ENV_FILE" + sed -i.bak '/^COLLECTION_FACTORY_PROXY=/d' "$ENV_FILE" + sed -i.bak '/^COLLECTION_FACTORY_IMPL=/d' "$ENV_FILE" + sed -i.bak '/^USER_COLLECTION_721_IMPL=/d' "$ENV_FILE" + sed -i.bak '/^USER_COLLECTION_1155_IMPL=/d' "$ENV_FILE" + sed -i.bak -e :a -e '/^\n*$/{$d;N;ba' -e '}' "$ENV_FILE" + rm -f "${ENV_FILE}.bak" + fi + + cat >> "$ENV_FILE" << EOF + +# User Collections (ZkSync Era - deployed $TIMESTAMP) +COLLECTION_FACTORY_PROXY=$COLLECTION_FACTORY_PROXY +COLLECTION_FACTORY_IMPL=$COLLECTION_FACTORY_IMPL +USER_COLLECTION_721_IMPL=$USER_COLLECTION_721_IMPL +USER_COLLECTION_1155_IMPL=$USER_COLLECTION_1155_IMPL +EOF + + log_success "Environment file updated" +} + +# ============================================================================= +# Summary +# ============================================================================= + +print_summary() { + echo "" + echo "==============================================" + echo " DEPLOYMENT SUMMARY" + echo "==============================================" + echo "" + echo "Network: $NETWORK" + echo "Explorer: $EXPLORER_URL" + echo "" + + if [ "$BROADCAST" != "--broadcast" ]; then + echo "Mode: DRY RUN (no contracts deployed)" + echo "" + echo "To deploy for real, run:" + echo " $0 $NETWORK --broadcast" + return 0 + fi + + echo "Deployed Contracts:" + echo "-------------------" + echo "" + echo "CollectionFactory:" + echo " Proxy: $COLLECTION_FACTORY_PROXY" + echo " Implementation: $COLLECTION_FACTORY_IMPL" + echo " Explorer: $EXPLORER_URL/address/$COLLECTION_FACTORY_PROXY" + echo "" + echo "UserCollection721:" + echo " Implementation: $USER_COLLECTION_721_IMPL" + echo " Explorer: $EXPLORER_URL/address/$USER_COLLECTION_721_IMPL" + echo "" + echo "UserCollection1155:" + echo " Implementation: $USER_COLLECTION_1155_IMPL" + echo " Explorer: $EXPLORER_URL/address/$USER_COLLECTION_1155_IMPL" + echo "" + echo "Configuration:" + echo " Admin: $N_FACTORY_ADMIN" + echo " Operator: $N_FACTORY_OPERATOR" + echo "" + echo "==============================================" +} + +# ============================================================================= +# Main +# ============================================================================= + +main() { + echo "" + echo "==============================================" + echo " ZkSync User Collections Deployment" + echo "==============================================" + echo "" + + cd "$PROJECT_ROOT" + + preflight_checks + move_l1_contracts + compile_contracts + verify_build_artifacts + deploy_contracts + verify_deployment + smoke_test_createCollection + verify_source_code + update_env_file + print_summary +} + +main "$@" diff --git a/script/DeployCollectionFactoryZkSync.s.sol b/script/DeployCollectionFactoryZkSync.s.sol new file mode 100644 index 00000000..4f82a288 --- /dev/null +++ b/script/DeployCollectionFactoryZkSync.s.sol @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity ^0.8.26; + +import {Script, console} from "forge-std/Script.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +import {CollectionFactory} from "../src/collections/CollectionFactory.sol"; +import {UserCollection721} from "../src/collections/UserCollection721.sol"; +import {UserCollection1155} from "../src/collections/UserCollection1155.sol"; + +/** + * @title DeployCollectionFactoryZkSync + * @notice Deployment script for the user collections system on ZkSync Era. + * @dev See `src/collections/doc/spec/user-collections-specification.md` (§9.1). + * + * Deploys, in order: + * 1. UserCollection721 implementation (CREATE only — never CREATE2; + * see §7.2 row 15). + * 2. UserCollection1155 implementation (CREATE only). + * 3. CollectionFactory logic. + * 4. ERC1967Proxy pointing at the factory logic, initialized with + * (admin, operator, impl721, impl1155). + * + * Usage: + * forge script script/DeployCollectionFactoryZkSync.s.sol \ + * --rpc-url $L2_RPC --broadcast --zksync + * + * Environment Variables: + * - DEPLOYER_PRIVATE_KEY: Private key with ETH for gas. + * - N_FACTORY_ADMIN: Multisig that will hold DEFAULT_ADMIN_ROLE on the factory. + * - N_FACTORY_OPERATOR: Backend service address that will hold OPERATOR_ROLE. + */ +contract DeployCollectionFactoryZkSync is Script { + address public collection721Impl; + address public collection1155Impl; + address public factoryImpl; + address public factoryProxy; + + function run() external { + uint256 deployerPrivateKey = vm.envUint("DEPLOYER_PRIVATE_KEY"); + address admin = vm.envAddress("N_FACTORY_ADMIN"); + address operator = vm.envAddress("N_FACTORY_OPERATOR"); + + require(admin != address(0), "N_FACTORY_ADMIN is zero"); + require(operator != address(0), "N_FACTORY_OPERATOR is zero"); + + console.log("=== Deploying User Collections on ZkSync ==="); + console.log("Admin:", admin); + console.log("Operator:", operator); + console.log(""); + + vm.startBroadcast(deployerPrivateKey); + + // 1. UserCollection721 implementation. + console.log("1. Deploying UserCollection721 implementation..."); + collection721Impl = address(new UserCollection721()); + console.log(" UserCollection721 Implementation:", collection721Impl); + + // 2. UserCollection1155 implementation. + console.log("2. Deploying UserCollection1155 implementation..."); + collection1155Impl = address(new UserCollection1155()); + console.log(" UserCollection1155 Implementation:", collection1155Impl); + + // 3. CollectionFactory logic. + console.log("3. Deploying CollectionFactory logic..."); + factoryImpl = address(new CollectionFactory()); + console.log(" CollectionFactory Implementation:", factoryImpl); + + // 4. ERC1967 proxy + atomic initialize. + console.log("4. Deploying ERC1967Proxy(CollectionFactory)..."); + bytes memory initData = abi.encodeCall( + CollectionFactory.initialize, (admin, operator, collection721Impl, collection1155Impl) + ); + factoryProxy = address(new ERC1967Proxy(factoryImpl, initData)); + console.log(" CollectionFactory Proxy:", factoryProxy); + console.log(""); + + vm.stopBroadcast(); + + // Summary — the orchestration shell script greps for these labels. + console.log("=== Deployment Complete ==="); + console.log("CollectionFactory Proxy:", factoryProxy); + console.log("CollectionFactory Implementation:", factoryImpl); + console.log("UserCollection721 Implementation:", collection721Impl); + console.log("UserCollection1155 Implementation:", collection1155Impl); + console.log(""); + console.log("Save the proxy address for future upgrades and operator calls."); + } +} diff --git a/script/UpgradeCollectionFactory.s.sol b/script/UpgradeCollectionFactory.s.sol new file mode 100644 index 00000000..8661544c --- /dev/null +++ b/script/UpgradeCollectionFactory.s.sol @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity ^0.8.26; + +import {Script, console} from "forge-std/Script.sol"; + +import {CollectionFactory} from "../src/collections/CollectionFactory.sol"; +import {UserCollection721} from "../src/collections/UserCollection721.sol"; +import {UserCollection1155} from "../src/collections/UserCollection1155.sol"; + +/** + * @title UpgradeCollectionFactory + * @notice Three-mode upgrade script for the user collections system. + * @dev See `src/collections/doc/spec/user-collections-specification.md` (§9.4). + * + * **Modes**: + * - `UPGRADE_FACTORY`: deploys a fresh `CollectionFactory` logic + * contract and calls `upgradeToAndCall` on the existing proxy. Pass + * `REINIT_DATA` to invoke a `reinitializer(N)` migration in the same tx. + * - `SET_IMPL_721`: deploys a fresh `UserCollection721` implementation + * and calls `setImplementation721` on the proxy. Affects future clones + * only — existing clones remain on the previous implementation. + * - `SET_IMPL_1155`: same as above for `UserCollection1155`. + * + * **Pre-Upgrade Checklist (factory upgrade)**: + * 1. Snapshot storage layout: `forge inspect CollectionFactory storageLayout` + * and compare against the committed + * `src/collections/layouts/CollectionFactory.v1.json` baseline. + * Verify slot index AND byte offset for sub-word fields (lock bools). + * 2. Run all collections tests: `forge test --match-path "test/collections/**"`. + * 3. Test on a fork: re-run this script with `--fork-url $RPC_URL` first. + * 4. After broadcast, verify the new EIP-1967 implementation slot via + * `cast implementation $FACTORY_PROXY` and confirm role/state preservation. + * + * **Pre-Upgrade Checklist (setImplementation*)**: + * 1. Snapshot the new clone implementation's layout against the + * previous `UserCollection<721|1155>` baseline. + * 2. Confirm the post-`setImplementation*` `cast call` matches the new + * implementation address. + * + * Usage: + * ACTION=UPGRADE_FACTORY FACTORY_PROXY=0x... \ + * forge script script/UpgradeCollectionFactory.s.sol --rpc-url $RPC_URL --broadcast + * + * ACTION=SET_IMPL_721 FACTORY_PROXY=0x... \ + * forge script script/UpgradeCollectionFactory.s.sol --rpc-url $RPC_URL --broadcast + * + * ACTION=SET_IMPL_1155 FACTORY_PROXY=0x... \ + * forge script script/UpgradeCollectionFactory.s.sol --rpc-url $RPC_URL --broadcast + * + * Environment Variables: + * - DEPLOYER_PRIVATE_KEY: Private key of the address holding `DEFAULT_ADMIN_ROLE` on the factory proxy. + * - FACTORY_PROXY: Address of the deployed `CollectionFactory` ERC1967 proxy. + * - ACTION: One of `UPGRADE_FACTORY`, `SET_IMPL_721`, `SET_IMPL_1155`. + * - REINIT_DATA: (UPGRADE_FACTORY only, optional) ABI-encoded reinitializer call. + */ +contract UpgradeCollectionFactory is Script { + function run() external { + uint256 deployerPrivateKey = vm.envUint("DEPLOYER_PRIVATE_KEY"); + address proxyAddress = vm.envAddress("FACTORY_PROXY"); + string memory action = vm.envString("ACTION"); + + require(proxyAddress != address(0), "FACTORY_PROXY is zero"); + + console.log("=== Collections Upgrade ==="); + console.log("Action:", action); + console.log("Factory Proxy:", proxyAddress); + console.log(""); + + vm.startBroadcast(deployerPrivateKey); + + bytes32 actionHash = keccak256(bytes(action)); + address newImpl; + if (actionHash == keccak256("UPGRADE_FACTORY")) { + bytes memory reinitData = vm.envOr("REINIT_DATA", bytes("")); + newImpl = _upgradeFactory(proxyAddress, reinitData); + } else if (actionHash == keccak256("SET_IMPL_721")) { + newImpl = _setImpl721(proxyAddress); + } else if (actionHash == keccak256("SET_IMPL_1155")) { + newImpl = _setImpl1155(proxyAddress); + } else { + revert("Invalid ACTION. Use UPGRADE_FACTORY, SET_IMPL_721, or SET_IMPL_1155."); + } + + vm.stopBroadcast(); + + console.log(""); + console.log("=== Upgrade Complete ==="); + console.log("New Implementation:", newImpl); + console.log("Proxy (unchanged):", proxyAddress); + } + + function _upgradeFactory(address proxy, bytes memory reinitData) internal returns (address impl) { + console.log("Deploying new CollectionFactory logic..."); + impl = address(new CollectionFactory()); + console.log("New factory implementation:", impl); + + CollectionFactory factory = CollectionFactory(proxy); + if (reinitData.length > 0) { + console.log("Calling upgradeToAndCall with reinitializer..."); + factory.upgradeToAndCall(impl, reinitData); + } else { + console.log("Calling upgradeToAndCall..."); + factory.upgradeToAndCall(impl, ""); + } + } + + function _setImpl721(address proxy) internal returns (address impl) { + console.log("Deploying new UserCollection721 implementation..."); + impl = address(new UserCollection721()); + console.log("New 721 implementation:", impl); + + CollectionFactory factory = CollectionFactory(proxy); + factory.setImplementation721(impl); + console.log("setImplementation721 broadcast."); + } + + function _setImpl1155(address proxy) internal returns (address impl) { + console.log("Deploying new UserCollection1155 implementation..."); + impl = address(new UserCollection1155()); + console.log("New 1155 implementation:", impl); + + CollectionFactory factory = CollectionFactory(proxy); + factory.setImplementation1155(impl); + console.log("setImplementation1155 broadcast."); + } +} diff --git a/src/collections/CollectionFactory.sol b/src/collections/CollectionFactory.sol new file mode 100644 index 00000000..f316b607 --- /dev/null +++ b/src/collections/CollectionFactory.sol @@ -0,0 +1,194 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity ^0.8.26; + +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; +import {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +import {ICollectionFactory} from "./interfaces/ICollectionFactory.sol"; +import {IUserCollection721} from "./interfaces/IUserCollection721.sol"; +import {IUserCollection1155} from "./interfaces/IUserCollection1155.sol"; +import {Standard, CreateParams721, CreateParams1155} from "./interfaces/CollectionTypes.sol"; + +/** + * @title CollectionFactory + * @notice UUPS-upgradeable, operator-triggered factory that deploys per-collection + * `ERC1967Proxy` instances of `UserCollection721` / `UserCollection1155`. + * @dev See `src/collections/doc/spec/user-collections-specification.md`. + * + * The factory atomically deploys a per-collection `ERC1967Proxy` + * pointing at the standard's implementation, with an `abi.encodeCall` + * to `initialize(p, msg.sender)` baked into the constructor so init + * runs in the proxy's storage in the same frame. `msg.sender` is + * auto-granted `MINTER_ROLE` (see §2.3). Records the + * `externalId → collection` mapping and emits `CollectionCreated`. + * Reverts on reused or zero `externalId`. + * + * Already-deployed collections are immutable (impls do not inherit + * `UUPSUpgradeable`; the EIP-1967 implementation slot is constructor- + * fixed). Admin can swap implementation pointers via `setImplementation*`, + * which only affects future collections. + */ +contract CollectionFactory is + Initializable, + AccessControlUpgradeable, + UUPSUpgradeable, + ICollectionFactory +{ + // ────────────────────────────────────────────── + // Roles + // ────────────────────────────────────────────── + + bytes32 public constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE"); + + // ────────────────────────────────────────────── + // Storage (V1) — order matters; see §6.1 of the spec. + // ────────────────────────────────────────────── + + address private _erc721Implementation; + address private _erc1155Implementation; + mapping(bytes32 => address) private _collectionByExternalId; + + /// @dev Reserved storage slots for future appended fields. + uint256[47] private __gap; + + // ────────────────────────────────────────────── + // Constructor + // ────────────────────────────────────────────── + + constructor() { + _disableInitializers(); + } + + // ────────────────────────────────────────────── + // Initialization + // ────────────────────────────────────────────── + + /// @inheritdoc ICollectionFactory + function initialize( + address admin, + address operator, + address impl721, + address impl1155 + ) external initializer { + if (admin == address(0) || operator == address(0) || impl721 == address(0) || impl1155 == address(0)) { + revert ZeroAddress(); + } + if (impl721.code.length == 0) revert NotAContract(impl721); + if (impl1155.code.length == 0) revert NotAContract(impl1155); + + // `__AccessControl_init` body is empty in OZ v5.6.1; the role grants + // below initialize all the state we need. + + _grantRole(DEFAULT_ADMIN_ROLE, admin); + _grantRole(OPERATOR_ROLE, operator); + + _erc721Implementation = impl721; + _erc1155Implementation = impl1155; + } + + // ────────────────────────────────────────────── + // Creation + // ────────────────────────────────────────────── + + /// @inheritdoc ICollectionFactory + function createCollection721(CreateParams721 calldata p, bytes32 externalId) + external + onlyRole(OPERATOR_ROLE) + returns (address collection) + { + _checkExternalId(externalId); + + bytes memory initData = abi.encodeCall( + IUserCollection721.initialize, + (p, msg.sender) + ); + collection = address( + new ERC1967Proxy{salt: externalId}(_erc721Implementation, initData) + ); + + _collectionByExternalId[externalId] = collection; + emit CollectionCreated(p.owner, collection, Standard.ERC721, externalId); + } + + /// @inheritdoc ICollectionFactory + function createCollection1155(CreateParams1155 calldata p, bytes32 externalId) + external + onlyRole(OPERATOR_ROLE) + returns (address collection) + { + _checkExternalId(externalId); + + bytes memory initData = abi.encodeCall( + IUserCollection1155.initialize, + (p, msg.sender) + ); + collection = address( + new ERC1967Proxy{salt: externalId}(_erc1155Implementation, initData) + ); + + _collectionByExternalId[externalId] = collection; + emit CollectionCreated(p.owner, collection, Standard.ERC1155, externalId); + } + + // ────────────────────────────────────────────── + // Admin + // ────────────────────────────────────────────── + + /// @inheritdoc ICollectionFactory + function setImplementation721(address impl) external onlyRole(DEFAULT_ADMIN_ROLE) { + _validateImplementation(impl); + _erc721Implementation = impl; + emit ImplementationUpdated(Standard.ERC721, impl); + } + + /// @inheritdoc ICollectionFactory + function setImplementation1155(address impl) external onlyRole(DEFAULT_ADMIN_ROLE) { + _validateImplementation(impl); + _erc1155Implementation = impl; + emit ImplementationUpdated(Standard.ERC1155, impl); + } + + // ────────────────────────────────────────────── + // Views + // ────────────────────────────────────────────── + + /// @inheritdoc ICollectionFactory + function collectionByExternalId(bytes32 externalId) external view returns (address) { + return _collectionByExternalId[externalId]; + } + + /// @inheritdoc ICollectionFactory + function erc721Implementation() external view returns (address) { + return _erc721Implementation; + } + + /// @inheritdoc ICollectionFactory + function erc1155Implementation() external view returns (address) { + return _erc1155Implementation; + } + + // ────────────────────────────────────────────── + // Internals + // ────────────────────────────────────────────── + + function _checkExternalId(bytes32 externalId) private view { + if (externalId == bytes32(0)) revert InvalidExternalId(); + if (_collectionByExternalId[externalId] != address(0)) { + revert ExternalIdAlreadyUsed(externalId); + } + } + + function _validateImplementation(address impl) private view { + if (impl == address(0)) revert ZeroAddress(); + if (impl.code.length == 0) revert NotAContract(impl); + } + + // ────────────────────────────────────────────── + // UUPS authorization + // ────────────────────────────────────────────── + + function _authorizeUpgrade(address) internal override onlyRole(DEFAULT_ADMIN_ROLE) {} +} diff --git a/src/collections/UserCollection1155.sol b/src/collections/UserCollection1155.sol new file mode 100644 index 00000000..fd369cfe --- /dev/null +++ b/src/collections/UserCollection1155.sol @@ -0,0 +1,206 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity ^0.8.26; + +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import {ERC1155Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC1155/ERC1155Upgradeable.sol"; +import {ERC1155SupplyUpgradeable} from + "@openzeppelin/contracts-upgradeable/token/ERC1155/extensions/ERC1155SupplyUpgradeable.sol"; +import {ERC1155BurnableUpgradeable} from + "@openzeppelin/contracts-upgradeable/token/ERC1155/extensions/ERC1155BurnableUpgradeable.sol"; +import {ERC2981Upgradeable} from "@openzeppelin/contracts-upgradeable/token/common/ERC2981Upgradeable.sol"; +import {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; + +import {IUserCollection1155} from "./interfaces/IUserCollection1155.sol"; +import {CreateParams1155} from "./interfaces/CollectionTypes.sol"; + +/** + * @title UserCollection1155 + * @notice ERC-1155 implementation deployed behind a per-collection `ERC1967Proxy` by `CollectionFactory`. + * @dev See `src/collections/doc/spec/user-collections-specification.md` (§3.6). + * + * Bytecode-permanence invariants apply identically to UserCollection721 + * (see §7.2 row 15 and the §8.3 unit test): no `selfdestruct`, no + * caller-controlled `delegatecall`, deployment via `CREATE` only. + */ +contract UserCollection1155 is + Initializable, + ERC1155Upgradeable, + ERC1155SupplyUpgradeable, + ERC1155BurnableUpgradeable, + ERC2981Upgradeable, + AccessControlUpgradeable, + IUserCollection1155 +{ + // ────────────────────────────────────────────── + // Roles + // ────────────────────────────────────────────── + + bytes32 public constant OWNER_ROLE = keccak256("OWNER_ROLE"); + bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); + + // ────────────────────────────────────────────── + // Constants + // ────────────────────────────────────────────── + + /// @notice Maximum number of (id, amount) pairs per `mintBatch` call. + uint256 public constant MAX_BATCH = 100; + + // ────────────────────────────────────────────── + // Storage (V1) — order matters; see §6.2 of the spec. + // The two booleans are declared adjacent so Solidity packs them into a + // single slot (bytes 0 and 1, 30 bytes free for future appended sub-word + // fields). 1155 omits `nextTokenId` (caller-chosen IDs). + // ────────────────────────────────────────────── + + string private _contractURI; + bool private _metadataLocked; + bool private _royaltiesLocked; + + /// @dev Reserved storage slots for future appended fields. + uint256[47] private __gap; + + // ────────────────────────────────────────────── + // Constructor + // ────────────────────────────────────────────── + + constructor() { + _disableInitializers(); + } + + // ────────────────────────────────────────────── + // Initialization + // ────────────────────────────────────────────── + + /// @inheritdoc IUserCollection1155 + function initialize(CreateParams1155 calldata p, address operatorMinter) external initializer { + if (p.owner == address(0) || operatorMinter == address(0)) revert ZeroAddress(); + + // Only the inits with non-empty bodies in OZ v5.6.1 are called. The + // remaining `___init` functions for ERC1155Supply, Burnable, + // ERC2981, and AccessControl are empty in this version (kept by OZ as + // forward-compat shims). Re-add them if upgrading OZ. + __ERC1155_init(p.uri); + + _contractURI = p.contractURI; + + if (p.royaltyBps > 0) { + _setDefaultRoyalty(p.royaltyRecipient, p.royaltyBps); + } + + _setRoleAdmin(MINTER_ROLE, OWNER_ROLE); + + _grantRole(OWNER_ROLE, p.owner); + _grantRole(MINTER_ROLE, p.owner); + _grantRole(MINTER_ROLE, operatorMinter); + + uint256 len = p.additionalMinters.length; + for (uint256 i = 0; i < len; ++i) { + _grantRole(MINTER_ROLE, p.additionalMinters[i]); + } + } + + // ────────────────────────────────────────────── + // Minting + // ────────────────────────────────────────────── + + /// @inheritdoc IUserCollection1155 + function mint(address to, uint256 id, uint256 amount, bytes calldata data) + external + onlyRole(MINTER_ROLE) + { + _mint(to, id, amount, data); + } + + /// @inheritdoc IUserCollection1155 + function mintBatch( + address to, + uint256[] calldata ids, + uint256[] calldata amounts, + bytes calldata data + ) external onlyRole(MINTER_ROLE) { + uint256 len = ids.length; + if (len != amounts.length) revert LengthMismatch(); + if (len > MAX_BATCH) revert BatchTooLarge(len, MAX_BATCH); + _mintBatch(to, ids, amounts, data); + } + + // ────────────────────────────────────────────── + // Owner-mutable settings + // ────────────────────────────────────────────── + + /// @inheritdoc IUserCollection1155 + function setURI(string calldata newURI) external onlyRole(OWNER_ROLE) { + if (_metadataLocked) revert MetadataIsLocked(); + _setURI(newURI); + emit URIUpdated(newURI); + } + + /// @inheritdoc IUserCollection1155 + function setContractURI(string calldata newURI) external onlyRole(OWNER_ROLE) { + if (_metadataLocked) revert MetadataIsLocked(); + _contractURI = newURI; + emit ContractURIUpdated(newURI); + } + + /// @inheritdoc IUserCollection1155 + function setDefaultRoyalty(address recipient, uint96 bps) external onlyRole(OWNER_ROLE) { + if (_royaltiesLocked) revert RoyaltiesAreLocked(); + if (bps == 0) { + _deleteDefaultRoyalty(); + } else { + _setDefaultRoyalty(recipient, bps); + } + } + + /// @inheritdoc IUserCollection1155 + function lockMetadata() external onlyRole(OWNER_ROLE) { + _metadataLocked = true; + emit MetadataLocked(); + } + + /// @inheritdoc IUserCollection1155 + function lockRoyalties() external onlyRole(OWNER_ROLE) { + _royaltiesLocked = true; + emit RoyaltiesLocked(); + } + + // ────────────────────────────────────────────── + // Views + // ────────────────────────────────────────────── + + /// @inheritdoc IUserCollection1155 + function contractURI() external view returns (string memory) { + return _contractURI; + } + + /// @inheritdoc IUserCollection1155 + function metadataLocked() external view returns (bool) { + return _metadataLocked; + } + + /// @inheritdoc IUserCollection1155 + function royaltiesLocked() external view returns (bool) { + return _royaltiesLocked; + } + + // ────────────────────────────────────────────── + // Required overrides + // ────────────────────────────────────────────── + + function _update(address from, address to, uint256[] memory ids, uint256[] memory values) + internal + override(ERC1155Upgradeable, ERC1155SupplyUpgradeable) + { + super._update(from, to, ids, values); + } + + function supportsInterface(bytes4 interfaceId) + public + view + override(ERC1155Upgradeable, ERC2981Upgradeable, AccessControlUpgradeable) + returns (bool) + { + return super.supportsInterface(interfaceId); + } +} diff --git a/src/collections/UserCollection721.sol b/src/collections/UserCollection721.sol new file mode 100644 index 00000000..4e9dfc87 --- /dev/null +++ b/src/collections/UserCollection721.sol @@ -0,0 +1,240 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity ^0.8.26; + +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import {ERC721Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol"; +import {ERC721URIStorageUpgradeable} from + "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721URIStorageUpgradeable.sol"; +import {ERC721BurnableUpgradeable} from + "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721BurnableUpgradeable.sol"; +import {ERC2981Upgradeable} from "@openzeppelin/contracts-upgradeable/token/common/ERC2981Upgradeable.sol"; +import {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; + +import {IUserCollection721} from "./interfaces/IUserCollection721.sol"; +import {CreateParams721} from "./interfaces/CollectionTypes.sol"; + +/** + * @title UserCollection721 + * @notice ERC-721 implementation deployed behind a per-collection `ERC1967Proxy` by `CollectionFactory`. + * @dev See `src/collections/doc/spec/user-collections-specification.md` (§3.5). + * + * Bytecode-permanence invariants (load-bearing for the §1.3 immutability + * guarantee — see §7.2 row 15 and the §8.2 unit test): + * - This contract contains no `selfdestruct`. + * - This contract performs no `delegatecall` to caller-provided addresses. + * - Implementation must be deployed via `CREATE`, not `CREATE2`. + */ +contract UserCollection721 is + Initializable, + ERC721Upgradeable, + ERC721URIStorageUpgradeable, + ERC721BurnableUpgradeable, + ERC2981Upgradeable, + AccessControlUpgradeable, + IUserCollection721 +{ + // ────────────────────────────────────────────── + // Roles + // ────────────────────────────────────────────── + + bytes32 public constant OWNER_ROLE = keccak256("OWNER_ROLE"); + bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); + + // ────────────────────────────────────────────── + // Constants + // ────────────────────────────────────────────── + + /// @notice Maximum number of items per `mintBatch` call. + uint256 public constant MAX_BATCH = 100; + + // ────────────────────────────────────────────── + // Storage (V1) — order matters; see §6.2 of the spec. + // The two booleans are declared adjacent so Solidity packs them into a + // single slot (bytes 0 and 1, 30 bytes free for future appended sub-word + // fields). Saves one __gap slot. + // ────────────────────────────────────────────── + + string private _baseTokenURI; + string private _contractURI; + uint256 private _nextTokenId; + bool private _metadataLocked; + bool private _royaltiesLocked; + + /// @dev Reserved storage slots for future appended fields. + uint256[46] private __gap; + + // ────────────────────────────────────────────── + // Constructor + // ────────────────────────────────────────────── + + /// @dev Disables initializers on the implementation so it cannot be + /// initialized directly. Each per-collection proxy calls `initialize` + /// exactly once via the factory's atomic constructor-frame deploy+init + /// flow. + constructor() { + _disableInitializers(); + } + + // ────────────────────────────────────────────── + // Initialization + // ────────────────────────────────────────────── + + /// @inheritdoc IUserCollection721 + function initialize(CreateParams721 calldata p, address operatorMinter) external initializer { + if (p.owner == address(0) || operatorMinter == address(0)) revert ZeroAddress(); + + // Only the inits with non-empty bodies in OZ v5.6.1 are called. The + // remaining `___init` functions for ERC721URIStorage, Burnable, + // ERC2981, and AccessControl are empty in this version (kept by OZ as + // forward-compat shims). Re-add them if upgrading OZ. + __ERC721_init(p.name, p.symbol); + + _baseTokenURI = p.baseURI; + _contractURI = p.contractURI; + + if (p.royaltyBps > 0) { + _setDefaultRoyalty(p.royaltyRecipient, p.royaltyBps); + } + + _setRoleAdmin(MINTER_ROLE, OWNER_ROLE); + + _grantRole(OWNER_ROLE, p.owner); + _grantRole(MINTER_ROLE, p.owner); + _grantRole(MINTER_ROLE, operatorMinter); + + uint256 len = p.additionalMinters.length; + for (uint256 i = 0; i < len; ++i) { + _grantRole(MINTER_ROLE, p.additionalMinters[i]); + } + } + + // ────────────────────────────────────────────── + // Minting + // ────────────────────────────────────────────── + + /// @inheritdoc IUserCollection721 + function mint(address to, string calldata tokenURI_) + external + onlyRole(MINTER_ROLE) + returns (uint256 tokenId) + { + tokenId = _nextTokenId; + ++_nextTokenId; + _safeMint(to, tokenId); + _setTokenURI(tokenId, tokenURI_); + } + + /// @inheritdoc IUserCollection721 + function mintBatch(address[] calldata to, string[] calldata uris) + external + onlyRole(MINTER_ROLE) + returns (uint256[] memory tokenIds) + { + uint256 len = to.length; + if (len != uris.length) revert LengthMismatch(); + if (len > MAX_BATCH) revert BatchTooLarge(len, MAX_BATCH); + + tokenIds = new uint256[](len); + uint256 startId = _nextTokenId; + for (uint256 i = 0; i < len; ++i) { + uint256 id = startId + i; + tokenIds[i] = id; + _safeMint(to[i], id); + _setTokenURI(id, uris[i]); + } + _nextTokenId = startId + len; + } + + // ────────────────────────────────────────────── + // Owner-mutable settings + // ────────────────────────────────────────────── + + /// @inheritdoc IUserCollection721 + function setBaseURI(string calldata newBase) external onlyRole(OWNER_ROLE) { + if (_metadataLocked) revert MetadataIsLocked(); + _baseTokenURI = newBase; + emit BaseURIUpdated(newBase); + } + + /// @inheritdoc IUserCollection721 + function setContractURI(string calldata newURI) external onlyRole(OWNER_ROLE) { + if (_metadataLocked) revert MetadataIsLocked(); + _contractURI = newURI; + emit ContractURIUpdated(newURI); + } + + /// @inheritdoc IUserCollection721 + function setDefaultRoyalty(address recipient, uint96 bps) external onlyRole(OWNER_ROLE) { + if (_royaltiesLocked) revert RoyaltiesAreLocked(); + if (bps == 0) { + _deleteDefaultRoyalty(); + } else { + _setDefaultRoyalty(recipient, bps); + } + } + + /// @inheritdoc IUserCollection721 + function lockMetadata() external onlyRole(OWNER_ROLE) { + _metadataLocked = true; + emit MetadataLocked(); + } + + /// @inheritdoc IUserCollection721 + function lockRoyalties() external onlyRole(OWNER_ROLE) { + _royaltiesLocked = true; + emit RoyaltiesLocked(); + } + + // ────────────────────────────────────────────── + // Views + // ────────────────────────────────────────────── + + /// @inheritdoc IUserCollection721 + function contractURI() external view returns (string memory) { + return _contractURI; + } + + /// @inheritdoc IUserCollection721 + function nextTokenId() external view returns (uint256) { + return _nextTokenId; + } + + /// @inheritdoc IUserCollection721 + function metadataLocked() external view returns (bool) { + return _metadataLocked; + } + + /// @inheritdoc IUserCollection721 + function royaltiesLocked() external view returns (bool) { + return _royaltiesLocked; + } + + // ────────────────────────────────────────────── + // Required overrides + // ────────────────────────────────────────────── + + function _baseURI() internal view override returns (string memory) { + return _baseTokenURI; + } + + /// @dev `tokenURI` resolution lives in `ERC721URIStorageUpgradeable`; the + /// override here exists only to disambiguate the inheritance chain. + function tokenURI(uint256 tokenId) + public + view + override(ERC721Upgradeable, ERC721URIStorageUpgradeable) + returns (string memory) + { + return super.tokenURI(tokenId); + } + + function supportsInterface(bytes4 interfaceId) + public + view + override(ERC721Upgradeable, ERC721URIStorageUpgradeable, ERC2981Upgradeable, AccessControlUpgradeable) + returns (bool) + { + return super.supportsInterface(interfaceId); + } +} diff --git a/src/collections/doc/README.md b/src/collections/doc/README.md new file mode 100644 index 00000000..be9566e5 --- /dev/null +++ b/src/collections/doc/README.md @@ -0,0 +1,9 @@ +# User Collections — Documentation + +Operator-triggered NFT collection factory: users pay in fiat off-chain, a trusted backend deploys a fully-isolated per-collection `ERC1967Proxy` (ERC-721 or ERC-1155) on the user's behalf. + +## Contents + +| Document | Description | +| -------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | +| [spec/user-collections-specification.md](spec/user-collections-specification.md) | Full technical specification (architecture, roles, interfaces, flows, storage, security, testing, ops) | diff --git a/src/collections/doc/spec/2026-05-08-clones-replacement-design.md b/src/collections/doc/spec/2026-05-08-clones-replacement-design.md new file mode 100644 index 00000000..89594b4a --- /dev/null +++ b/src/collections/doc/spec/2026-05-08-clones-replacement-design.md @@ -0,0 +1,385 @@ +# Collections — Replacing `Clones.clone()` for zkSync Era Compatibility + +**Date:** 2026-05-08 +**Status:** Design (approved through brainstorming; not yet implemented) +**Scope:** `src/collections/` user-collections system +**Driver:** zkSync Sepolia deploy attempt revealed `Clones.clone()` is incompatible with EraVM at runtime +**Related spec:** `src/collections/doc/spec/user-collections-specification.md` + +--- + +## 1. Context + +### 1.1 What we tried + +Running `./ops/deploy_collection_factory_zksync.sh testnet` against zkSync Sepolia (RPC: `https://rpc.ankr.com/zksync_era_sepolia`, chain id 300). The factory + implementations deploy themselves succeeded conceptually — they use Solidity `new`, which zksolc handles correctly — but the per-collection clone path is broken for two independently-confirmed reasons: + +1. **Compiled-artifact evidence.** `zkout/CollectionFactory.sol/CollectionFactory.json` shows `factoryDependencies: {}`. The compiled zk hashes for `UserCollection721` (`010005db…`) and `UserCollection1155` (`0100053b…`) appear zero times in the factory bytecode. zksolc only registers a contract as a `factoryDep` when it sees `type(C).creationCode` or `new C()` *statically* — `Clones.clone(impl)` builds the EIP-1167 runtime blob in memory at runtime, which zksolc never sees. Without a registered factoryDep, EraVM's `ContractDeployer` cannot resolve the deploy. + +2. **Matter Labs confirmation.** From zksync-developers discussion #561, an ML maintainer: *"We don't currently support ERC1167 since we have a different bytecode format."* Production users hitting `Clones.clone()` see `execution reverted: ERC1167: create failed`. Discussions #91, #166, and #561 all end with users refactoring away from `Clones`. + +### 1.2 What's load-bearing + +The user-collections spec (`src/collections/doc/spec/user-collections-specification.md` §1.3) sells per-collection bytecode immutability as the product: + +> *"Already-deployed clones cannot be upgraded — buyers and creators retain a permanent guarantee about each collection's behavior."* + +This is reinforced in §1.4 row 7, §2.3, §6.3, §7.1, and §7.2 row 15. Any replacement for `Clones.clone()` must preserve this property; admin-pushable upgrades to existing collections (e.g. a `BeaconProxy` fleet upgrade) are explicitly out of scope. + +The user has additionally constrained the design to canonical OpenZeppelin patterns — no bespoke proxy or upgrade machinery. + +--- + +## 2. Decision + +**Per-collection `ERC1967Proxy` deployed via `new` with `salt: externalId`, where the implementation contracts do not inherit `UUPSUpgradeable`.** + +```solidity +// inside CollectionFactory.createCollection721 / createCollection1155 +bytes memory initData = abi.encodeCall( + IUserCollection721.initialize, (p, msg.sender) +); +collection = address(new ERC1967Proxy{salt: externalId}(_erc721Implementation, initData)); +``` + +This is the only OZ-standard pattern that simultaneously: + +1. **Preserves the §1.3 immutability promise.** `ERC1967Proxy` is a pure transport. Without `UUPSUpgradeable` on the impl and without a `ProxyAdmin` pattern, the EIP-1967 implementation slot is constructor-fixed and unreachable for write afterward. Three independent gates (no `upgradeToAndCall` selector on the impl, no `ProxyAdmin` slot pattern, `ERC1967Utils.upgradeToAndCall` is `internal` and only callable from inside an impl delegatecall frame that doesn't exist) all have to fail simultaneously for an upgrade to slip through. + +2. **Compiles cleanly on EraVM.** zksolc statically resolves `ERC1967Proxy`, registers its bytecode hash in `CollectionFactory.factoryDependencies`, and lowers `new ERC1967Proxy{salt}(...)` to `ContractDeployer.create2(salt, ERC1967ProxyHash, abi.encode(impl, initData))`. This is the same code path zksolc takes for `new UserCollection721()` in the deploy script — already proven working. + +3. **Has direct precedent in this repo.** `script/DeployCollectionFactoryZkSync.s.sol:72-75` already deploys the *factory itself* as `new ERC1967Proxy(factoryImpl, initData)` on zkSync Era, with atomic delegatecall init in the constructor. The refactor applies the exact same pattern one layer down per collection. + +4. **Enables off-chain address pre-derivation.** With `salt: externalId`, the collection address is a pure function of `(factory, externalId, impl, initData, ERC1967Proxy zk bytecode hash)` — all inputs the backend already controls. The `_collectionByExternalId` mapping remains the canonical on-chain registry; pre-derivation is a redundant lookup path. + +### 2.1 Patterns explicitly rejected + +| Pattern | Rejected because | +|---|---| +| `BeaconProxy` + `UpgradeableBeacon` | Beacon admin can upgrade all collections at once. Violates §1.3. | +| `TransparentUpgradeableProxy` | Has a `ProxyAdmin` with upgrade authority. Violates §1.3. | +| `ERC1967Proxy` *with* `UUPSUpgradeable` on impl | Per-collection admin (whoever holds DEFAULT_ADMIN_ROLE on the impl) could upgrade. Violates §1.3. | +| Forking OZ to make a custom minimal proxy | Violates the OZ-standards-only constraint and burns audit posture. | +| Full Hardhat zkSync deploy via `factory_deps` JSON | Violates the OZ-standards-only constraint; we don't use Hardhat anywhere else in this repo. | + +--- + +## 3. Detailed Design + +### 3.1 Factory state & interface — unchanged + +`CollectionFactory.sol` storage layout, public selectors, and external behavior all stay identical. The committed `CollectionFactory.v1.json` baseline remains valid. + +```solidity +address private _erc721Implementation; +address private _erc1155Implementation; +mapping(bytes32 externalId => address collection) private _collectionByExternalId; +uint256[47] private __gap; +``` + +`setImplementation721` / `setImplementation1155` / `erc721Implementation()` / `erc1155Implementation()` / `getCollectionByExternalId(...)` keep the same selectors, the same gating, and the same revert conditions (`ZeroAddress`, `NotAContract(addr)`, `DuplicateExternalId(extId)`). + +The `setImplementation*` semantics are unchanged: those setters affect *future* collections only, because the impl address gets baked into each new `ERC1967Proxy`'s EIP-1967 slot at construction and there is no path to rewrite that slot afterward. + +### 3.2 Deploy path + +#### 3.2.1 Imports + +```diff +- import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; ++ import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +``` + +#### 3.2.2 `createCollection721` body + +```diff + function createCollection721(CreateParams721 calldata p, bytes32 externalId) + external onlyRole(OPERATOR_ROLE) returns (address collection) + { + _checkExternalId(externalId); + +- collection = Clones.clone(_erc721Implementation); +- IUserCollection721(collection).initialize(p, msg.sender); ++ bytes memory initData = abi.encodeCall( ++ IUserCollection721.initialize, (p, msg.sender) ++ ); ++ collection = address( ++ new ERC1967Proxy{salt: externalId}(_erc721Implementation, initData) ++ ); + + _collectionByExternalId[externalId] = collection; + emit CollectionCreated(p.owner, collection, Standard.ERC721, externalId); + } +``` + +`createCollection1155` takes the same shape with `IUserCollection1155.initialize` and `_erc1155Implementation`. + +#### 3.2.3 Why salt = externalId + +- `externalId` is already a uniqueness key in the system (`_checkExternalId` enforces no duplicates). +- Removes the sequential-nonce race that would otherwise affect concurrent creations. +- Makes the address a cryptographic commitment to all inputs — same `(externalId, impl, params, operator)` always yields the same address. Different inputs → different address. + +#### 3.2.4 zkSync compatibility — the three things that have to work, all of which already do + +1. **`new ERC1967Proxy{salt}(impl, initData)` compiles cleanly.** zksolc registers `ERC1967Proxy`'s bytecode hash in the factory's `factoryDependencies`. +2. **`delegatecall` from inside a constructor works on EraVM.** `ERC1967Utils.upgradeToAndCall` does this, and the factory's own deploy already uses this pattern in `script/DeployCollectionFactoryZkSync.s.sol`. +3. **The impl is reached by address, not by factoryDep.** `_erc721Implementation` is passed at runtime, and EraVM resolves the delegatecall target by looking up that address's bytecode on-chain — already registered when the deployer EOA ran `new UserCollection721()` at top level. + +### 3.3 Initialization flow + +``` +new ERC1967Proxy{salt: externalId}(impl, initData) + └── ERC1967Proxy constructor(address logic, bytes _data) + └── ERC1967Utils.upgradeToAndCall(logic, _data) + ├── SSTORE(EIP_1967_IMPL_SLOT, logic) + ├── emit Upgraded(logic) + └── if (_data.length > 0) + └── Address.functionDelegateCall(logic, _data) + └── UserCollection721.initialize(p, operatorMinter) + ├── grants DEFAULT_ADMIN_ROLE / OWNER_ROLE to p.owner + ├── grants MINTER_ROLE to additionalMinters + operatorMinter + ├── sets _baseTokenURI / _contractURI + ├── sets default royalty (if p.royaltyBps > 0) + └── emit Initialized(1) +``` + +Atomic single constructor frame. If any step reverts, the whole `new ERC1967Proxy` reverts and no contract is deployed. Stronger atomicity than the current two-step `Clones.clone()` + `initialize` pattern. + +`_disableInitializers()` in `UserCollection721` / `UserCollection1155` constructors stays unchanged — it hardens the *impl singleton* against ever being directly initialized. The `initializer` modifier on `initialize` still flips `_initialized = 1` in the *proxy's* storage during the constructor's delegatecall, so re-init reverts with OZ's `InvalidInitialization`. + +The operator address travels into `initialize` as the `operatorMinter` parameter, encoded at the call site (`abi.encodeCall(initialize, (p, msg.sender))` in the factory's frame, where `msg.sender` is the OPERATOR_ROLE caller). Identical §2.3 auto-grant semantics. + +### 3.4 Address determinism + +Inputs to the per-collection address derivation: + +``` +addr = ZK_CREATE2( factory, externalId, ERC1967ProxyZkHash, keccak256(abi.encode(impl, initData)) ) +``` + +Where: +- `factory` = the proxy address of `CollectionFactory` (constant per environment). +- `externalId` = the salt, supplied by the operator at create time. +- `ERC1967ProxyZkHash` = the zksolc-emitted bytecode hash of `@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol`. **Must be pinned in backend artifacts at the version used at factory deploy time.** +- `impl` = the current `_erc721Implementation` (or 1155). Reads via `erc721Implementation()` / `erc1155Implementation()`. +- `initData` = `abi.encodeCall(initialize, (p, operator))`. + +Backend pre-derivation flow: + +1. Read `_erc721Implementation` once (cache; refresh after admin upgrades). +2. Pin `ERC1967ProxyZkHash` from the factory deploy artifacts. +3. Generate `externalId`. +4. Construct `params` and select the operator address. +5. Compute the address using `zksync-ethers` `utils.create2Address`. +6. Submit `createCollection721(params, externalId)` from the operator key. +7. Verify the on-chain `_collectionByExternalId[externalId]` matches the precomputed address (sanity check, not load-bearing). + +#### 3.4.1 Caveats (operational) + +1. **Address is sensitive to every input.** Same `externalId` but different `params` → different address. Backends must fix params before generating the externalId. +2. **Operator rotation note (§2.3).** Because `msg.sender` enters `initData`, two operators calling with the same externalId would derive different addresses. In practice `_checkExternalId` rejects the second call (`DuplicateExternalId`); pre-derivation must use the *current* OPERATOR_ROLE holder. +3. **`_collectionByExternalId` mapping stays canonical.** Pre-derivation is a redundant off-chain lookup path, not a replacement for the on-chain registry. +4. **zk bytecode hash pinning.** Upgrading zksolc can change `ERC1967Proxy`'s zk bytecode hash. Pin the hash to the version used at factory deploy time and only refresh during a coordinated tooling bump. + +### 3.5 Bytecode-permanence invariants + +Existing §7.2 row 15 splits into two clearer sub-rows: + +#### 3.5.1 Row 15a — Implementation bytecode permanence (unchanged) + +`UserCollection721` / `UserCollection1155` are deployed exactly as before: +- Top-level `new` from the deployer EOA in `DeployCollectionFactoryZkSync.s.sol` → sequential `CREATE`, never `CREATE2`. +- `_disableInitializers()` in their constructors. +- No `SELFDESTRUCT` opcode anywhere in own or inherited code. +- No `delegatecall` to caller-provided addresses. + +The opcode-walker bytecode-permanence test continues to run against both impls, with the same assertion set. + +#### 3.5.2 Row 15b — Per-collection proxy permanence (new) + +Per-collection proxies are deployed via `CREATE2` (`{salt: externalId}`). Permanence comes from a different chain of arguments: + +1. **The deployed bytecode is canonical OZ `ERC1967Proxy`, used unmodified.** Imported from `@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol`, no fork, no override. `ERC1967Proxy` and its dependency `Proxy.sol` contain no `SELFDESTRUCT`. +2. **The impl pointer in the EIP-1967 slot is set exactly once at construction and is unreachable for write afterward.** Three independent gates, all of which would have to fail simultaneously for an upgrade to slip through: + - **(2a)** The implementations do not inherit `UUPSUpgradeable` → no `upgradeToAndCall` selector exposed by the impl. + - **(2b)** We use `ERC1967Proxy` directly, not `TransparentUpgradeableProxy` → no `ProxyAdmin`. + - **(2c)** `ERC1967Utils.upgradeToAndCall` is `internal`, only callable from a delegatecall frame whose code is the impl — and since (2a) holds, no caller can ever reach it. +3. **`CREATE2` re-occupation is foreclosed.** For an attacker to re-occupy a deployed collection's address via salt collision, the existing contract would have to first cease to exist (`SELFDESTRUCT`) — but `ERC1967Proxy` has no such opcode (per (1)) and the impl pointer can't be swapped to one that does (per (2)). On EraVM, `ContractDeployer` additionally enforces "one deployment per address" at the protocol layer; the in-bytecode argument alone is sufficient. + +### 3.6 Spec doc deltas + +#### 3.6.1 Stays verbatim (with single-word `clones → collections` substitution) + +§1.3 immutability promise, §2 entire roles/auth model, §3.5/§3.6 method signatures and lock semantics, §5 mint flows, §6.1 factory storage layout, §6.3 storage-layout discipline, §7.1 trust assumptions, §7.3/§7.4 out-of-scope and audit posture, §9.1 "Deploy `UserCollection721` implementation via `CREATE` (sequential nonce, **never** `CREATE2`)" rule. + +#### 3.6.2 Substantive content rewrites + +| Section | Current | Updated | +|---|---|---| +| §1.1 line 46 | "deploys cheap clones of fixed-behavior implementation contracts" | "deploys per-collection `ERC1967Proxy` instances pointing at fixed-behavior implementation contracts" | +| §1.2 mermaid diagram | subgraph `"User Collections (EIP-1167 minimal proxies)"`, arrow labels `"Clones.clone"` | subgraph `"User Collections (ERC1967Proxy per collection)"`, arrow labels `"new ERC1967Proxy{salt: externalId}"` | +| §1.3 core components table | "EIP-1167 clone target" rows for 721/1155 | "`ERC1967Proxy` implementation" | +| §1.4 row 2 (Deployment model) | "EIP-1167 minimal proxy clones for both standards" | "Per-collection `ERC1967Proxy` deployed via `CREATE2` with `externalId` salt; implementations deployed via `CREATE` only" | +| §1.4 row 7 (Upgradeability) | "Clones: immutable; admin can swap implementation pointer for *future* clones only" | "Per-collection proxies: immutable (impls do not inherit `UUPSUpgradeable`, no admin slot); admin can swap implementation pointer for *future* collections only" | +| §3.4 createCollection atomic-flow bullet | `Clones.clone(impl) → clone.initialize(p, msg.sender) → collectionByExternalId[externalId] = clone` | `abi.encodeCall(initialize, (p, msg.sender)) → new ERC1967Proxy{salt: externalId}(impl, initData) → collectionByExternalId[externalId] = collection`; init is atomic in the proxy constructor | +| §4.1 sequence diagram | `FAC->>CL: Clones.clone(...)` and a separate `CL->>CL: initialize(...)` | Single `FAC->>CL: new ERC1967Proxy{salt}(impl, encodeCall(initialize,(p,msg.sender)))`, with note that `Upgraded(impl)` then `Initialized(1)` events fire inside the constructor | +| §4.2 Atomicity | Current text emphasizes 2-step atomicity within the same tx | Strengthen: deploy + init are now a single constructor frame, no transient uninitialized window | +| §4.4 Gas Profile | "Deploys an EIP-1167 minimal proxy (~45,000 gas on EVM L1 baseline)" | Delete the L1 anchor; replace with "On zkSync Era, per-collection deploy is dominated by `ContractDeployer.create2` + the constructor's delegatecall init; gas measured by `Collections.integration.t.sol` and quoted from the test output (target: < 1.5M gas on zkSync Sepolia for a typical `createCollection721`)" | +| §6.2 Clone Storage opening sentence | "Each clone owns its full storage independently of other clones (EIP-1167 proxies `delegatecall` logic but persist state in the proxy's own address)." | "Each collection owns its full storage independently (`ERC1967Proxy` `delegatecall`s logic at the address in the EIP-1967 implementation slot but persists state in the proxy's own address)." | +| §6.2 gap reservation note | s/clones/collections/g | (same point holds — the EIP-1967 slot doesn't fight the gap because it's at a fixed namespaced slot, not slot N) | + +#### 3.6.3 Additions + +1. **§1.4 row 7 footnote** explaining the OZ-standards rationale: "We use `ERC1967Proxy` directly (not `TransparentUpgradeableProxy`, not `BeaconProxy`) because the implementation contracts deliberately do not inherit `UUPSUpgradeable`. With no upgrade selector exposed and no admin slot pattern, the proxy's implementation pointer is constructor-fixed and the per-collection immutability promise is enforced by code that already exists in the OZ canonical libraries — no custom upgrade gating needed." +2. **§4.5 (new sub-section) Address Determinism** — documents the salt = externalId convention, derivation inputs, and the four caveats from §3.4.1. +3. **§7.2 row 15 split into 15a / 15b** as specified in §3.5. +4. **§7.2 new row 16**: "Audit must verify the imported `ERC1967Proxy` resolves to canonical OZ at the lockfile-pinned version; remappings or forks of OZ proxy contracts are out of band and would invalidate the bytecode-permanence proof." +5. **§9.1 deploy-script step 4 note**: clarify that the existing `ERC1967Proxy(factoryImpl, ...)` construction in `DeployCollectionFactoryZkSync` is the *factory's own* proxy and does not change. The new per-collection `ERC1967Proxy` instances are deployed by the *factory itself* at `createCollection*` time, not by the deploy script. + +#### 3.6.4 Vocabulary pass + +Mechanical `s/clone/collection/` and `s/clones/collections/` pass throughout the doc, except: +- Keep "EIP-1167" mentions in §1.4 row 7 footnote where we explain why we moved away from it. +- Keep the historical comparison in §4.4. + +### 3.7 Storage-layout baselines + +**No baseline JSON changes. v1 stays v1 across all three files.** + +| File | Status | Why | +|---|---|---| +| `src/collections/layouts/CollectionFactory.v1.json` | Unchanged | Storage fields identical; refactor is in function bodies only. | +| `src/collections/layouts/UserCollection721.v1.json` | Unchanged | Implementation contract isn't touched. | +| `src/collections/layouts/UserCollection1155.v1.json` | Unchanged | Same. | + +The EIP-1967 implementation slot lives at the keccak-derived address `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc` — deliberately namespaced, far above any slot index Solidity's allocator would reach. Cannot collide with slots 0..N. So even though every per-collection proxy now stores a non-zero value at that slot (where EIP-1167 minimal proxies stored nothing), it has zero impact on the impl-side layout that `forge inspect storage-layout` produces. + +#### 3.7.1 §9.4 runbook addition (one-line `cast` check) + +```bash +EIP1967_IMPL_SLOT=0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc +cast storage "$COLLECTION_ADDR" "$EIP1967_IMPL_SLOT" --rpc-url "$L2_RPC" +# expected: padded address of UserCollection721 (or 1155) impl +``` + +Same shape as the existing factory-proxy check in `ops/deploy_collection_factory_zksync.sh:248-257`, just reused per collection. + +### 3.8 Test impact + +#### 3.8.1 Stays green untouched + +- All `UserCollection721.t.sol` and `UserCollection1155.t.sol` (impls untouched). +- `CollectionFactory.t.sol` unit tests for `initialize`, `setImplementation*`, `_checkExternalId`, all UUPS upgrade tests on the factory itself. +- The opcode-walker bytecode-permanence test on the impls. + +#### 3.8.2 Assertions to update + +- **Bytecode-size assertions on the deployed collection.** Switch from EIP-1167's 45-byte runtime to a `> 0` sanity check or the exact `ERC1967Proxy` runtime size from `vm.getCode("ERC1967Proxy")`. +- **EIP-1967 impl slot read post-create.** Replace any "slot is zero" assertion with `vm.load(collection, EIP1967_IMPL_SLOT) == _erc721Implementation()`. This becomes a meaningful positive check instead of vacuously zero. +- **Sequence-of-events tests.** Add `Upgraded(impl)` to the expected emit list in `createCollection*` integration tests (it now fires before `Initialized(1)`, both inside the constructor). + +#### 3.8.3 New tests to add + +1. **Deterministic address derivation** in `CollectionFactory.t.sol`: + ```solidity + function test_createCollection721_addressMatchesCreate2Derivation() public { + bytes32 extId = keccak256("test-collection"); + bytes memory initData = abi.encodeCall( + IUserCollection721.initialize, (params, operator) + ); + address predicted = Create2.computeAddress( + extId, + keccak256(abi.encodePacked( + type(ERC1967Proxy).creationCode, + abi.encode(impl721, initData) + )), + address(factory) + ); + vm.prank(operator); + address actual = factory.createCollection721(params, extId); + assertEq(actual, predicted, "CREATE2 derivation must match"); + } + ``` + Uses the EVM `CREATE2` formula because Foundry's default backend is EVM. The zkSync formula differs and is covered by the end-to-end integration test in §3.8.3 point 5. + +2. **Atomic init**: `vm.expectEmit` for both `Upgraded(impl)` and `Initialized(1)` inside the same `createCollection721` tx, then immediately call `mint` on the returned collection in the same test (operator already has `MINTER_ROLE` from auto-grant) and assert success — proves no transient uninitialized window. + +3. **No upgrade selector on impls** in `UserCollection721.t.sol` and `UserCollection1155.t.sol`: + ```solidity + function test_implementationHasNoUpgradeSelectors() public { + address impl = address(new UserCollection721()); + (bool ok1, ) = impl.staticcall(abi.encodeWithSelector(0x52d1902d)); // proxiableUUID + (bool ok2, ) = impl.staticcall(abi.encodeWithSelector(0x4f1ef286, address(0), bytes(""))); // upgradeToAndCall + assertFalse(ok1, "impl must not expose proxiableUUID"); + assertFalse(ok2, "impl must not expose upgradeToAndCall"); + } + ``` + +4. **ERC1967Proxy bytecode permanence** (new file `test/collections/ERC1967Proxy.permanence.t.sol`): opcode-walker over `vm.getCode("ERC1967Proxy")` runtime, asserting no `0xff` (SELFDESTRUCT) and no delegatecall to caller-provided addresses. + +5. **End-to-end zkSync Sepolia smoke test** (new step in `ops/deploy_collection_factory_zksync.sh` post-broadcast): `cast send` a `createCollection721` from the operator key; assert non-empty `code` at the deployed address, EIP-1967 impl slot equals expected impl, and an immediate `cast send mint(...)` succeeds. This is the empirical end-to-end check that the zksolc-compiled output works at runtime on EraVM — the precise gap that left us with a passing dry-run but a broken `Clones.clone()` flow originally. + +#### 3.8.4 CI build-artifact check (not a Foundry test) + +Add to the deploy script (or the existing CI workflow): + +```bash +test "$(jq -r '.factoryDependencies | length' zkout/CollectionFactory.sol/CollectionFactory.json)" -gt 0 +``` + +If `factoryDependencies` is ever empty again on the factory, the build fails immediately — the smoking-gun signal we used to detect the original `Clones.clone()` problem becomes a permanent guardrail. + +#### 3.8.5 Coverage + +Current 96.91%. The refactor adds new unit tests and changes a few lines of `createCollection*`, so coverage should increase or stay flat. Maintain the existing 95% CI floor. + +### 3.9 Audit checklist additions + +- "Confirm `import {ERC1967Proxy} from '@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol';` resolves to canonical OZ at the locked version (no remappings override)." +- "Assert `UserCollection721` / `UserCollection1155` ABIs do not include `upgradeTo(address)`, `upgradeToAndCall(address,bytes)`, or `proxiableUUID()` (selector `0x52d1902d`)." +- "Run opcode-walker on `zkout/ERC1967Proxy.sol/ERC1967Proxy.json` runtime bytecode; assert no `0xff` (SELFDESTRUCT)." + +--- + +## 4. Files to Touch + +### 4.1 Modified + +- `src/collections/CollectionFactory.sol` — imports + body of `createCollection721` / `createCollection1155`. ~10 lines net change. +- `src/collections/doc/spec/user-collections-specification.md` — vocabulary pass + substantive deltas in §3.6. +- `ops/deploy_collection_factory_zksync.sh` — add the post-broadcast end-to-end smoke test (§3.8.3 point 5) and the `factoryDependencies` CI build-artifact check (§3.8.4). +- `test/collections/CollectionFactory.t.sol` — assertions per §3.8.2; new tests per §3.8.3 points 1–2. +- `test/collections/UserCollection721.t.sol` — new test per §3.8.3 point 3. +- `test/collections/UserCollection1155.t.sol` — new test per §3.8.3 point 3. +- `test/collections/Collections.integration.t.sol` — sequence-of-events updates per §3.8.2. + +### 4.2 Added + +- `test/collections/ERC1967Proxy.permanence.t.sol` — opcode-walker for `ERC1967Proxy` (§3.8.3 point 4). + +### 4.3 Unchanged + +- `src/collections/UserCollection721.sol` +- `src/collections/UserCollection1155.sol` +- `src/collections/interfaces/*.sol` +- `src/collections/layouts/*.v1.json` (all three baselines) +- `script/DeployCollectionFactoryZkSync.s.sol` +- `script/UpgradeCollectionFactory.s.sol` + +--- + +## 5. Open Questions / Out of Scope + +- **Gas benchmarking on zkSync Sepolia.** §4.4 of the spec needs a real gas number. We'll pull it from the integration test's first run after the refactor lands and embed it in the doc at that point — not blocking on this design. +- **Backend pre-derivation library.** The TS/JS code that computes `zksync-ethers utils.create2Address(...)` for collections is out of scope for this Solidity-side spec. Tracked separately in the backend repo. +- **Whether to support deterministic addresses with operator-rotation tolerance** (e.g. by encoding only `(params, operator=null)` in the salt-derivation while still passing the actual operator at runtime). Possible future enhancement; not needed for v1 of this refactor. + +--- + +## 6. Approvals + +- Brainstorming dialogue completed 2026-05-08; user approved each of the seven design sections (factory state, deploy path, init flow, bytecode permanence, spec deltas, storage baselines, test impact). +- Decision pinned in engram memory: `collections-immutability-and-oz-standards` (immutability is hard requirement, OZ-standards-only); `zksync-clones-incompatibility` (root-cause analysis of why `Clones.clone()` fails on EraVM). + +Next step after spec sign-off: implementation plan via the `superpowers:writing-plans` skill. diff --git a/src/collections/doc/spec/2026-05-08-clones-replacement-implementation-plan.md b/src/collections/doc/spec/2026-05-08-clones-replacement-implementation-plan.md new file mode 100644 index 00000000..0b0743bf --- /dev/null +++ b/src/collections/doc/spec/2026-05-08-clones-replacement-implementation-plan.md @@ -0,0 +1,1215 @@ +# Clones → ERC1967Proxy Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace `Clones.clone()` with per-collection `ERC1967Proxy` (salted by `externalId`) in `CollectionFactory`, unblocking zkSync Era deploy while preserving the §1.3 per-collection bytecode-immutability promise. Aligns spec, tests, and deploy script with the new pattern. + +**Architecture:** `createCollection721` / `createCollection1155` switch from `Clones.clone(impl) + .initialize(...)` (two-step) to a single `new ERC1967Proxy{salt: externalId}(impl, abi.encodeCall(initialize, (p, msg.sender)))` (one constructor frame, atomic init via delegatecall). The implementation contracts and storage layouts are unchanged. + +**Tech Stack:** Solidity 0.8.26, Foundry (foundry-zksync v0.1.9), OpenZeppelin Contracts (canonical, lockfile-pinned), forge-std, zksolc (via `forge build --zksync`). + +**Spec reference:** `src/collections/doc/spec/2026-05-08-clones-replacement-design.md` + +--- + +## File Structure + +### Files modified + +| Path | Responsibility | Change scope | +|---|---|---| +| `src/collections/CollectionFactory.sol` | Factory logic | Imports + bodies of `createCollection721` / `createCollection1155`. ~10 lines net. | +| `src/collections/doc/spec/user-collections-specification.md` | Authoritative spec | Vocabulary pass + substantive deltas per design §3.6. | +| `ops/deploy_collection_factory_zksync.sh` | zkSync deploy orchestration | `factoryDependencies` build-artifact gate + per-collection EIP-1967 slot check + post-broadcast smoke test. | +| `test/collections/CollectionFactory.t.sol` | Factory unit tests | Add CREATE2-derivation test; extend atomic-emits tests; vocabulary in variable names. | +| `test/collections/UserCollection721.t.sol` | 721 impl unit tests | Switch test-side `Clones.clone(impl)` helper to `new ERC1967Proxy(impl, "")`; add no-upgrade-selector test. | +| `test/collections/UserCollection1155.t.sol` | 1155 impl unit tests | Same as 721. | +| `test/collections/Collections.integration.t.sol` | Cross-contract integration | Vocabulary + add `Upgraded` to expected emit sequences. | + +### Files added + +| Path | Responsibility | +|---|---| +| `test/collections/ERC1967Proxy.permanence.t.sol` | Opcode-walker over canonical OZ `ERC1967Proxy` runtime bytecode asserting no `0xff` SELFDESTRUCT and no caller-controlled `delegatecall` targets. Codifies §7.2 row 15b (1) for CI. | + +### Files unchanged + +`src/collections/UserCollection721.sol`, `src/collections/UserCollection1155.sol`, `src/collections/interfaces/*.sol`, `src/collections/layouts/{CollectionFactory,UserCollection721,UserCollection1155}.v1.json`, `script/DeployCollectionFactoryZkSync.s.sol`, `script/UpgradeCollectionFactory.s.sol`. + +--- + +## Task 1: Add CREATE2-derivation test for 721 (failing) + +**Files:** +- Modify: `test/collections/CollectionFactory.t.sol` + +- [ ] **Step 1: Add the failing test** + +Open `test/collections/CollectionFactory.t.sol`. At the top of the file, add this import alongside the existing OZ imports: + +```solidity +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {Create2} from "@openzeppelin/contracts/utils/Create2.sol"; +import {IUserCollection721} from "../../src/collections/interfaces/IUserCollection721.sol"; +``` + +Append this test method to the contract (place it next to `test_createCollection721_atomicAndEmits`): + +```solidity +function test_createCollection721_addressMatchesCreate2Derivation() public { + bytes32 externalId = keccak256("derivation-test-721"); + CreateParams721 memory p = _params721(CREATOR); + + bytes memory initData = abi.encodeCall( + IUserCollection721.initialize, + (p, OPERATOR) + ); + + bytes32 initCodeHash = keccak256( + abi.encodePacked( + type(ERC1967Proxy).creationCode, + abi.encode(address(impl721), initData) + ) + ); + + address predicted = Create2.computeAddress( + externalId, + initCodeHash, + address(factory) + ); + + vm.prank(OPERATOR); + address actual = factory.createCollection721(p, externalId); + + assertEq(actual, predicted, "deployed address must match CREATE2 derivation"); +} +``` + +- [ ] **Step 2: Run the test, confirm it fails** + +Run: + +```bash +forge test --match-test test_createCollection721_addressMatchesCreate2Derivation -vv +``` + +Expected: **FAIL** with the assertion message — the current factory uses `Clones.clone()` (sequential CREATE), not CREATE2, so the deployed address won't match the predicted CREATE2 address. + +- [ ] **Step 3: Commit the failing test** + +```bash +git add test/collections/CollectionFactory.t.sol +git commit -m "test(collections): add CREATE2 derivation test for createCollection721 (RED)" +``` + +--- + +## Task 2: Switch `createCollection721` to `ERC1967Proxy` with salt + +**Files:** +- Modify: `src/collections/CollectionFactory.sol` + +- [ ] **Step 1: Swap the import** + +In `src/collections/CollectionFactory.sol`, replace the `Clones` import (line 8): + +```diff +- import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; ++ import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +``` + +- [ ] **Step 2: Rewrite `createCollection721` body** + +In the same file, locate `createCollection721` (lines 93-105 in current `b93a9a4`). Replace the body so it reads: + +```solidity +function createCollection721(CreateParams721 calldata p, bytes32 externalId) + external + onlyRole(OPERATOR_ROLE) + returns (address collection) +{ + _checkExternalId(externalId); + + bytes memory initData = abi.encodeCall( + IUserCollection721.initialize, + (p, msg.sender) + ); + collection = address( + new ERC1967Proxy{salt: externalId}(_erc721Implementation, initData) + ); + + _collectionByExternalId[externalId] = collection; + emit CollectionCreated(p.owner, collection, Standard.ERC721, externalId); +} +``` + +- [ ] **Step 3: Update the contract's NatSpec at the top** + +In the same file, update the `@notice` block (lines 16-19) so it no longer says "EIP-1167 minimal-proxy clones": + +```diff +- * @notice UUPS-upgradeable, operator-triggered factory that deploys EIP-1167 +- * minimal-proxy clones of `UserCollection721` / `UserCollection1155`. ++ * @notice UUPS-upgradeable, operator-triggered factory that deploys per-collection ++ * `ERC1967Proxy` instances of `UserCollection721` / `UserCollection1155`. +``` + +And the body comment (lines 21-25): + +```diff +- * The factory atomically clones an implementation, invokes the clone's +- * `initialize` (passing `msg.sender` as the auto-granted operator +- * minter — see §2.3), records the off-chain `externalId → clone` +- * mapping, and emits `CollectionCreated`. Reverts on reused or zero +- * `externalId`. ++ * The factory atomically deploys a per-collection `ERC1967Proxy` ++ * pointing at the standard's implementation, with an `abi.encodeCall` ++ * to `initialize(p, msg.sender)` baked into the constructor so init ++ * runs in the proxy's storage in the same frame. `msg.sender` is ++ * auto-granted `MINTER_ROLE` (see §2.3). Records the ++ * `externalId → collection` mapping and emits `CollectionCreated`. ++ * Reverts on reused or zero `externalId`. +``` + +And the closing comment (lines 27-28): + +```diff +- * Already-deployed clones are immutable. Admin can swap implementation +- * pointers via `setImplementation*`, which only affects future clones. ++ * Already-deployed collections are immutable (impls do not inherit ++ * `UUPSUpgradeable`; the EIP-1967 implementation slot is constructor- ++ * fixed). Admin can swap implementation pointers via `setImplementation*`, ++ * which only affects future collections. +``` + +- [ ] **Step 4: Run the previously-failing test, confirm it now passes** + +```bash +forge test --match-test test_createCollection721_addressMatchesCreate2Derivation -vv +``` + +Expected: **PASS**. + +- [ ] **Step 5: Run the full factory unit-test suite, confirm no regressions** + +```bash +forge test --match-path test/collections/CollectionFactory.t.sol -vv +``` + +Expected: all tests pass. (The previously-failing test is now green; existing tests should be unaffected because they don't assert exact addresses or bytecode size.) + +- [ ] **Step 6: Run the full collections suite as a regression sweep** + +```bash +forge test --match-path 'test/collections/*' -vv +``` + +Expected: all 67+ existing tests + the new derivation test pass. + +- [ ] **Step 7: Commit** + +```bash +git add src/collections/CollectionFactory.sol test/collections/CollectionFactory.t.sol +git commit -m "feat(collections): replace Clones.clone() with ERC1967Proxy{salt} in createCollection721 + +Atomic deploy + init via the proxy's constructor (delegatecall to +initialize). Salt = externalId gives off-chain CREATE2 pre-derivation. +Preserves the per-collection bytecode-immutability promise — impls do +not inherit UUPSUpgradeable, so the EIP-1967 impl slot is constructor- +fixed. Required for zkSync Era compatibility (Clones.clone() is not +supported on EraVM)." +``` + +--- + +## Task 3: Switch `createCollection1155` to `ERC1967Proxy` with salt + +**Files:** +- Modify: `src/collections/CollectionFactory.sol` +- Modify: `test/collections/CollectionFactory.t.sol` + +- [ ] **Step 1: Add a failing 1155 derivation test** + +Append to `test/collections/CollectionFactory.t.sol`: + +```solidity +function test_createCollection1155_addressMatchesCreate2Derivation() public { + bytes32 externalId = keccak256("derivation-test-1155"); + CreateParams1155 memory p = _params1155(CREATOR); + + bytes memory initData = abi.encodeCall( + IUserCollection1155.initialize, + (p, OPERATOR) + ); + + bytes32 initCodeHash = keccak256( + abi.encodePacked( + type(ERC1967Proxy).creationCode, + abi.encode(address(impl1155), initData) + ) + ); + + address predicted = Create2.computeAddress( + externalId, + initCodeHash, + address(factory) + ); + + vm.prank(OPERATOR); + address actual = factory.createCollection1155(p, externalId); + + assertEq(actual, predicted, "deployed 1155 address must match CREATE2 derivation"); +} +``` + +If `IUserCollection1155` is not already imported in the test file, add at the top: + +```solidity +import {IUserCollection1155} from "../../src/collections/interfaces/IUserCollection1155.sol"; +``` + +- [ ] **Step 2: Run, confirm fail** + +```bash +forge test --match-test test_createCollection1155_addressMatchesCreate2Derivation -vv +``` + +Expected: **FAIL** (still using `Clones.clone()` for 1155). + +- [ ] **Step 3: Rewrite `createCollection1155` body in `CollectionFactory.sol`** + +Locate `createCollection1155` (lines 107-120 in current `b93a9a4`). Replace the body: + +```solidity +function createCollection1155(CreateParams1155 calldata p, bytes32 externalId) + external + onlyRole(OPERATOR_ROLE) + returns (address collection) +{ + _checkExternalId(externalId); + + bytes memory initData = abi.encodeCall( + IUserCollection1155.initialize, + (p, msg.sender) + ); + collection = address( + new ERC1967Proxy{salt: externalId}(_erc1155Implementation, initData) + ); + + _collectionByExternalId[externalId] = collection; + emit CollectionCreated(p.owner, collection, Standard.ERC1155, externalId); +} +``` + +- [ ] **Step 4: Run the previously-failing 1155 test, confirm it passes** + +```bash +forge test --match-test test_createCollection1155_addressMatchesCreate2Derivation -vv +``` + +Expected: **PASS**. + +- [ ] **Step 5: Run full collections suite as a regression sweep** + +```bash +forge test --match-path 'test/collections/*' -vv +``` + +Expected: all tests pass. + +- [ ] **Step 6: Commit** + +```bash +git add src/collections/CollectionFactory.sol test/collections/CollectionFactory.t.sol +git commit -m "feat(collections): replace Clones.clone() with ERC1967Proxy{salt} in createCollection1155" +``` + +--- + +## Task 4: Extend atomic-emits tests to assert `Upgraded` and `Initialized` + +**Files:** +- Modify: `test/collections/CollectionFactory.t.sol` + +The new flow emits `Upgraded(impl)` (from `ERC1967Utils.upgradeToAndCall`) then `Initialized(1)` (from the `initializer` modifier) inside the proxy constructor, before `CollectionCreated` from the factory. This task locks that ordering into a test. + +- [ ] **Step 1: Add `Upgraded` and `Initialized` event signatures to the test contract** + +Near the existing event declarations in `CollectionFactory.t.sol` (look for `event CollectionCreated(...)`), add: + +```solidity +event Upgraded(address indexed implementation); +event Initialized(uint64 version); +``` + +- [ ] **Step 2: Replace `test_createCollection721_atomicAndEmits`** + +Locate the test at line 116 (it's currently a single `expectEmit(true, false, true, true)` for `CollectionCreated` only). Replace its body with: + +```solidity +function test_createCollection721_atomicAndEmits() public { + bytes32 externalId = keccak256("order-1"); + + // Order: Upgraded(impl) → Initialized(1) → ... role grants ... → CollectionCreated + vm.expectEmit(true, false, false, false); + emit Upgraded(address(impl721)); + + vm.expectEmit(false, false, false, true); + emit Initialized(1); + + // CollectionCreated indexed topics: (creator, collection, externalId). + // We don't know the collection address up front, so leave its topic unchecked. + vm.expectEmit(true, false, true, true); + emit CollectionCreated(CREATOR, address(0), Standard.ERC721, externalId); + + vm.prank(OPERATOR); + address collection = factory.createCollection721(_params721(CREATOR), externalId); + + assertEq(factory.collectionByExternalId(externalId), collection); + UserCollection721 c = UserCollection721(collection); + assertEq(c.name(), "C"); + assertEq(c.contractURI(), "ipfs://c.json"); + assertTrue(c.hasRole(keccak256("OWNER_ROLE"), CREATOR)); + assertTrue(c.hasRole(MINTER_ROLE, OPERATOR)); +} +``` + +- [ ] **Step 3: Replace `test_createCollection1155_atomicAndEmits` analogously** + +Locate the 1155 atomic-emits test (line 135) and apply the same pattern, swapping `impl721` → `impl1155` and `Standard.ERC721` → `Standard.ERC1155`. + +- [ ] **Step 4: Add an immediate-mint test to prove no transient uninitialized window** + +Append: + +```solidity +function test_createCollection721_canMintImmediatelyInSameTx() public { + bytes32 externalId = keccak256("immediate-mint-721"); + + vm.startPrank(OPERATOR); + address collection = factory.createCollection721(_params721(CREATOR), externalId); + // Operator was auto-granted MINTER_ROLE during constructor delegatecall — + // can mint without any extra setup transactions. + UserCollection721(collection).mint(ALICE, 1); + vm.stopPrank(); + + assertEq(UserCollection721(collection).ownerOf(1), ALICE); +} +``` + +If `ALICE` isn't defined in this test file, add `address internal constant ALICE = address(0xA1);` to the constants block at the top. + +- [ ] **Step 5: Run the updated tests, confirm pass** + +```bash +forge test --match-test test_createCollection -vv +``` + +Expected: all `test_createCollection*` tests pass, including the new ones. + +- [ ] **Step 6: Commit** + +```bash +git add test/collections/CollectionFactory.t.sol +git commit -m "test(collections): assert Upgraded+Initialized emit order; add immediate-mint test" +``` + +--- + +## Task 5: Switch unit-test helpers to `ERC1967Proxy` + +**Files:** +- Modify: `test/collections/UserCollection721.t.sol` +- Modify: `test/collections/UserCollection1155.t.sol` + +Both files currently use `Clones.clone(address(impl))` to create test instances of the impl-behind-a-proxy without going through the factory. Switch them to `new ERC1967Proxy(address(impl), "")` for consistency with production. + +- [ ] **Step 1: Update imports in `UserCollection721.t.sol`** + +Replace line 5: + +```diff +- import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; ++ import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +``` + +- [ ] **Step 2: Replace each `Clones.clone(...)` call site in `UserCollection721.t.sol`** + +For each occurrence (`grep -n "Clones.clone" test/collections/UserCollection721.t.sol` to find them all), replace: + +```diff +- address cloneAddr = Clones.clone(address(impl)); ++ address cloneAddr = address(new ERC1967Proxy(address(impl), "")); +``` + +The empty `""` `bytes` argument tells `ERC1967Proxy` to skip the constructor delegatecall — the test then calls `IUserCollection721(cloneAddr).initialize(...)` separately, exactly as today. + +Also rename the local variable for vocabulary consistency where it appears (optional but tidy): + +```diff +- address cloneAddr = address(new ERC1967Proxy(address(impl), "")); ++ address proxyAddr = address(new ERC1967Proxy(address(impl), "")); +``` + +…and update its references in the same function. (If the rename is invasive, leave the variable name alone — the type is what matters.) + +- [ ] **Step 3: Repeat for `UserCollection1155.t.sol`** + +Same import swap and same call-site replacements. There are 3 occurrences at lines 52, 98, 115 (per current grep). + +- [ ] **Step 4: Run the unit suites, confirm green** + +```bash +forge test --match-path 'test/collections/UserCollection*.t.sol' -vv +``` + +Expected: all impl unit tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add test/collections/UserCollection721.t.sol test/collections/UserCollection1155.t.sol +git commit -m "test(collections): switch impl unit-test helpers from Clones to ERC1967Proxy" +``` + +--- + +## Task 6: Add no-upgrade-selector tests + +**Files:** +- Modify: `test/collections/UserCollection721.t.sol` +- Modify: `test/collections/UserCollection1155.t.sol` + +Codifies §7.2 row 16 (audit: "ABIs do not include `upgradeTo*` or `proxiableUUID`") as a unit test. + +- [ ] **Step 1: Add the test to `UserCollection721.t.sol`** + +Append to the test contract: + +```solidity +function test_implementationHasNoUpgradeSelectors() public view { + // proxiableUUID() — selector 0x52d1902d + (bool ok1, ) = address(impl).staticcall(abi.encodeWithSelector(0x52d1902d)); + assertFalse(ok1, "impl must not expose proxiableUUID"); + + // upgradeToAndCall(address,bytes) — selector 0x4f1ef286 + (bool ok2, ) = address(impl).staticcall( + abi.encodeWithSelector(0x4f1ef286, address(0), bytes("")) + ); + assertFalse(ok2, "impl must not expose upgradeToAndCall"); +} +``` + +- [ ] **Step 2: Add the analogous test to `UserCollection1155.t.sol`** + +Same shape, against that file's `impl` reference. + +- [ ] **Step 3: Run both, confirm pass** + +```bash +forge test --match-test test_implementationHasNoUpgradeSelectors -vv +``` + +Expected: **PASS** for both 721 and 1155 contracts. + +- [ ] **Step 4: Commit** + +```bash +git add test/collections/UserCollection721.t.sol test/collections/UserCollection1155.t.sol +git commit -m "test(collections): assert impls expose no UUPS upgrade selectors" +``` + +--- + +## Task 7: Add ERC1967Proxy bytecode-permanence test + +**Files:** +- Create: `test/collections/ERC1967Proxy.permanence.t.sol` + +Opcode-walks the canonical OZ `ERC1967Proxy` runtime bytecode and asserts no `0xff` (SELFDESTRUCT) and no `delegatecall` to caller-provided addresses. Parallel to the existing impl-side test but applied to the proxy contract. + +- [ ] **Step 1: Inspect the existing impl opcode-walker for the pattern to match** + +Run: + +```bash +grep -n "PUSH1\|opcode\|SELFDESTRUCT\|0xff\|delegatecall" test/collections/UserCollection721.t.sol | head +``` + +This locates the bytecode-permanence helper. Copy its opcode-walker structure (skipping `PUSH1..PUSH32` immediates) — we'll mirror it. + +- [ ] **Step 2: Write the new test file** + +Create `test/collections/ERC1967Proxy.permanence.t.sol`: + +```solidity +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.26; + +import {Test} from "forge-std/Test.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +/// @notice Bytecode-permanence proof for canonical OZ ERC1967Proxy. +/// Codifies design §3.5.2 (1): no SELFDESTRUCT, no caller-controlled +/// delegatecall. Defense-in-depth audit gate. +contract ERC1967ProxyPermanenceTest is Test { + /// @dev Deploy a real ERC1967Proxy and read its runtime bytecode. + /// Empty initData skips the constructor delegatecall — we just want + /// the deployed runtime, not a working instance. + function _runtime() internal returns (bytes memory) { + // Use any non-zero implementation; the runtime is the same regardless. + ERC1967Proxy p = new ERC1967Proxy(address(this), ""); + return address(p).code; + } + + function test_runtimeContainsNoSelfdestruct() public { + bytes memory code = _runtime(); + require(code.length > 0, "no runtime"); + + for (uint256 i = 0; i < code.length; ) { + uint8 op = uint8(code[i]); + + // PUSH1..PUSH32 — skip the immediate bytes (op 0x60..0x7f). + if (op >= 0x60 && op <= 0x7f) { + uint256 imm = uint256(op) - 0x5f; + i += 1 + imm; + continue; + } + + // SELFDESTRUCT (0xff) is the EVM mnemonic; canonical OZ + // ERC1967Proxy must not contain it. + assertTrue(op != 0xff, "ERC1967Proxy contains SELFDESTRUCT"); + + i += 1; + } + } + + function test_proxyImplementationDelegatecallTargetIsConstructorFixed() public { + // The only delegatecall in ERC1967Proxy's runtime targets _implementation() + // which reads from the EIP-1967 slot. The slot is written exclusively by + // ERC1967Utils.upgradeToAndCall (called only from the proxy's own + // constructor since the impl does not inherit UUPSUpgradeable). This test + // exercises the property by deploying with one impl and asserting the + // EIP-1967 slot equals that impl, then asserting that no external call + // can change it. + address impl = address(this); + ERC1967Proxy p = new ERC1967Proxy(impl, ""); + + bytes32 IMPL_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; + bytes32 stored = vm.load(address(p), IMPL_SLOT); + assertEq(address(uint160(uint256(stored))), impl, "EIP-1967 slot mismatch"); + + // No external selector exposed by the proxy can write IMPL_SLOT — the + // proxy's only entry point is the fallback, which delegatecalls the + // current impl. Since `address(this)` (the test contract) has no + // upgradeToAndCall selector, any call to mutate the slot reverts/no-ops. + // We assert by replaying upgradeToAndCall through the proxy and showing + // the slot is unchanged. + bytes memory ignored = abi.encodeWithSelector( + 0x4f1ef286, address(0xdeadbeef), bytes("") + ); + // staticcall to avoid mutating; the call should not return data that + // reflects a successful upgrade. + (bool ok, ) = address(p).staticcall(ignored); + // Whether `ok` is true or false depends on the test contract's fallback; + // either way the slot must not have changed. + ok; // silence unused warning + bytes32 storedAfter = vm.load(address(p), IMPL_SLOT); + assertEq(stored, storedAfter, "EIP-1967 slot was mutated"); + } +} +``` + +- [ ] **Step 3: Run the new test file, confirm pass** + +```bash +forge test --match-path test/collections/ERC1967Proxy.permanence.t.sol -vv +``` + +Expected: **PASS** on both functions. + +- [ ] **Step 4: Commit** + +```bash +git add test/collections/ERC1967Proxy.permanence.t.sol +git commit -m "test(collections): bytecode-permanence proof for canonical OZ ERC1967Proxy" +``` + +--- + +## Task 8: Update integration test sequence-of-events and vocabulary + +**Files:** +- Modify: `test/collections/Collections.integration.t.sol` + +- [ ] **Step 1: Find emit-order assertions and Clones references** + +```bash +grep -nE "Clones|expectEmit|emit (Upgraded|Initialized|CollectionCreated)" test/collections/Collections.integration.t.sol +``` + +- [ ] **Step 2: For each `vm.expectEmit` block immediately preceding a `createCollection*` call, prepend `Upgraded` and `Initialized` expectations** + +Pattern to apply at each call site: + +```solidity +// before: +vm.expectEmit(true, true, true, true); +emit CollectionCreated(/* ... */); +factory.createCollection721(/* ... */); + +// after: +vm.expectEmit(true, false, false, false); +emit Upgraded(address(impl721)); // or impl1155 for the 1155 path + +vm.expectEmit(false, false, false, true); +emit Initialized(1); + +vm.expectEmit(true, true, true, true); +emit CollectionCreated(/* ... */); +factory.createCollection721(/* ... */); +``` + +If the test contract doesn't already declare `Upgraded` and `Initialized`, add them next to the existing `event CollectionCreated(...)` declaration: + +```solidity +event Upgraded(address indexed implementation); +event Initialized(uint64 version); +``` + +- [ ] **Step 3: Vocabulary pass** + +In the same file, replace the comment at line 139 ("clone's runtime by `Clones.clone`") with a sentence referring to `ERC1967Proxy`. Replace any local variable named `clone*` or `*Clone` with `collection*` or `*Collection` for consistency (`grep -nE "[Cc]lone[A-Za-z]*" test/collections/Collections.integration.t.sol`). + +- [ ] **Step 4: Run integration tests, confirm pass** + +```bash +forge test --match-path test/collections/Collections.integration.t.sol -vv +``` + +Expected: all integration tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add test/collections/Collections.integration.t.sol +git commit -m "test(collections): integration emit-order and vocabulary updates for ERC1967Proxy" +``` + +--- + +## Task 9: Vocabulary pass on remaining test code + +**Files:** +- Modify: `test/collections/CollectionFactory.t.sol` + +- [ ] **Step 1: Find leftover `clone` mentions** + +```bash +grep -nE "[Cc]lone[A-Za-z]*" test/collections/CollectionFactory.t.sol +``` + +Expected hits include `test_setImplementation_affectsFutureClonesOnly` and locals `oldClone`/`newClone` (line 221+). + +- [ ] **Step 2: Rename for vocabulary consistency** + +- Function name: `test_setImplementation_affectsFutureClonesOnly` → `test_setImplementation_affectsFutureCollectionsOnly`. +- Locals: `oldClone` → `oldCollection`, `newClone` → `newCollection` (and update all references in the function body). +- Comment in the test that mentions "clone" / "clones" → "collection" / "collections". + +- [ ] **Step 3: Run, confirm green** + +```bash +forge test --match-path test/collections/CollectionFactory.t.sol -vv +``` + +Expected: all tests pass (rename is purely cosmetic). + +- [ ] **Step 4: Commit** + +```bash +git add test/collections/CollectionFactory.t.sol +git commit -m "test(collections): vocabulary pass — clones → collections in factory tests" +``` + +--- + +## Task 10: Update authoritative spec doc + +**Files:** +- Modify: `src/collections/doc/spec/user-collections-specification.md` + +Apply the changes specified in design doc §3.6. This is a documentation-only task with no tests; commit at the end. + +- [ ] **Step 1: §1.1 — opening summary** + +Find line 46: + +```diff +- A single upgradeable factory that deploys cheap clones of fixed-behavior implementation contracts. ++ A single upgradeable factory that deploys per-collection `ERC1967Proxy` instances pointing at fixed-behavior implementation contracts. +``` + +- [ ] **Step 2: §1.2 — mermaid diagram** + +Find the subgraph at line 70 and the arrow labels at lines 79-81: + +```diff +- subgraph Clones["User Collections (EIP-1167 minimal proxies)"] ++ subgraph Collections["User Collections (ERC1967Proxy per collection)"] +``` + +```diff +- FAC -- "Clones.clone" --> C1 +- FAC -- "Clones.clone" --> C2 +- FAC -- "Clones.clone" --> C3 ++ FAC -- "new ERC1967Proxy{salt}" --> C1 ++ FAC -- "new ERC1967Proxy{salt}" --> C2 ++ FAC -- "new ERC1967Proxy{salt}" --> C3 +``` + +- [ ] **Step 3: §1.3 — core components table** + +Find lines 102-103: + +```diff +- | `UserCollection721` | ERC-721 implementation cloned per creator | EIP-1167 clone target | Immutable per clone | +- | `UserCollection1155` | ERC-1155 implementation cloned per creator | EIP-1167 clone target | Immutable per clone | ++ | `UserCollection721` | ERC-721 implementation behind a per-collection ERC1967Proxy | `ERC1967Proxy` implementation | Immutable per collection | ++ | `UserCollection1155` | ERC-1155 implementation behind a per-collection ERC1967Proxy | `ERC1967Proxy` implementation | Immutable per collection | +``` + +Find line 105 (the immutability promise): + +```diff +- The factory is upgradeable so new implementation templates and bug fixes can be shipped without disrupting existing creators. Already-deployed clones cannot be upgraded — buyers and creators retain a permanent guarantee about each collection's behavior. ++ The factory is upgradeable so new implementation templates and bug fixes can be shipped without disrupting existing creators. Already-deployed collections cannot be upgraded — buyers and creators retain a permanent guarantee about each collection's behavior. +``` + +- [ ] **Step 4: §1.4 row 2 (Deployment model)** + +Line 112: + +```diff +- | 2 | Deployment model | EIP-1167 minimal proxy clones for both standards | ++ | 2 | Deployment model | Per-collection `ERC1967Proxy` deployed via `CREATE2` with `externalId` salt; implementations deployed via `CREATE` only | +``` + +- [ ] **Step 5: §1.4 row 7 (Upgradeability) + footnote** + +Line 117: + +```diff +- | 7 | Upgradeability | Factory: UUPS-upgradeable. Clones: immutable; admin can swap implementation pointer for *future* clones only | ++ | 7 | Upgradeability | Factory: UUPS-upgradeable. Per-collection proxies: immutable (impls do not inherit `UUPSUpgradeable`, no admin slot); admin can swap implementation pointer for *future* collections only [^upgradeability] | +``` + +Append at the bottom of §1.4 (or as a footnote): + +```markdown +[^upgradeability]: We use `ERC1967Proxy` directly (not `TransparentUpgradeableProxy`, not `BeaconProxy`) because the implementation contracts deliberately do not inherit `UUPSUpgradeable`. With no upgrade selector exposed and no `ProxyAdmin` slot pattern, the proxy's implementation pointer is constructor-fixed and the per-collection immutability promise is enforced by code that already exists in the OZ canonical libraries — no custom upgrade gating needed. We migrated away from the EIP-1167 minimal-proxy pattern because it is incompatible with zkSync Era's `ContractDeployer` factoryDeps model (see `2026-05-08-clones-replacement-design.md`). +``` + +- [ ] **Step 6: §2.3 vocab pass** + +Line 166: `s/future clones/future collections/g` and `s/Existing clones/Existing collections/g` in the operator key rotation paragraph. + +- [ ] **Step 7: §3.4 createCollection atomic flow** + +Line 311 atomic-flow bullet: + +```diff +- - Atomic flow: `Clones.clone(impl)` → `clone.initialize(p, msg.sender)` → `collectionByExternalId[externalId] = clone` → `emit CollectionCreated`. Passing `msg.sender` ensures the calling operator is auto-granted `MINTER_ROLE` on the new clone (see §2.3). ++ - Atomic flow: `abi.encodeCall(initialize, (p, msg.sender))` → `new ERC1967Proxy{salt: externalId}(impl, initData)` (deploy + delegatecall init in a single constructor frame) → `collectionByExternalId[externalId] = collection` → `emit CollectionCreated`. Passing `msg.sender` into `initData` ensures the calling operator is auto-granted `MINTER_ROLE` on the new collection (see §2.3). +``` + +Line 313: `s/Existing clones/Existing collections/g`, `s/affects future clones only/affects future collections only/g`. + +- [ ] **Step 8: §4.1 sequence diagram** + +Line 399: + +```diff +- FAC->>CL: Clones.clone(erc721Implementation) +- CL->>CL: initialize(p, msg.sender) ++ FAC->>CL: new ERC1967Proxy{salt: externalId}(erc721Implementation, encodeCall(initialize, (p, msg.sender))) ++ Note over CL: emit Upgraded(impl), emit Initialized(1) inside constructor +``` + +- [ ] **Step 9: §4.2 — strengthen atomicity claim** + +Locate §4.2 "Atomicity & Front-Running" and add a sentence: + +> "Deploy and initialize occur in a single constructor frame; there is no transient window where the proxy exists in an uninitialized state." + +- [ ] **Step 10: §4.4 — gas profile rewrite** + +Line 424: + +```diff +- - Deploys an EIP-1167 minimal proxy (~45,000 gas on EVM L1 baseline). ++ - On zkSync Era, per-collection deploy is dominated by `ContractDeployer.create2` plus the constructor's delegatecall init. Gas measured by `Collections.integration.t.sol` and quoted from the test output (target: < 1.5M gas on zkSync Sepolia for a typical `createCollection721`). The previous EIP-1167 baseline (~45k gas on EVM L1) is no longer applicable because we don't deploy minimal proxies. +``` + +- [ ] **Step 11: §4.5 (new sub-section) — Address Determinism** + +Insert after §4.4: + +```markdown +### 4.5 Address Determinism + +Per-collection addresses are deterministic on-chain because the factory uses `new ERC1967Proxy{salt: externalId}(...)`. The address is a pure function of: + +- `factory` proxy address (constant per environment) +- `externalId` (the salt; supplied by the operator) +- `_erc721Implementation` / `_erc1155Implementation` (read once via `erc721Implementation()` / `erc1155Implementation()`; refresh after admin upgrades) +- `initData = abi.encodeCall(initialize, (params, operatorAddress))` +- `ERC1967Proxy` zk bytecode hash (constant per zksolc release; pin in backend artifacts at the version used at factory deploy time) + +Backends can pre-derive the collection address before broadcasting `createCollection*` using `zksync-ethers` `utils.create2Address`. The on-chain `_collectionByExternalId[externalId]` mapping remains the canonical registry — pre-derivation is a redundant off-chain lookup path, not a replacement. + +**Caveats:** +1. Address is sensitive to every input — different `params` or different operator → different address. +2. Operator rotation (§2.3): pre-derive using the *current* OPERATOR_ROLE holder. +3. `_collectionByExternalId` mapping stays canonical and enforces uniqueness via `_checkExternalId`. +4. Pin the `ERC1967Proxy` zk bytecode hash; refresh only during a coordinated zksolc bump. +``` + +- [ ] **Step 12: §6.2 — Clone Storage opening + gap reservation** + +Line 500: + +```diff +- Each clone owns its full storage independently of other clones (EIP-1167 proxies `delegatecall` logic but persist state in the proxy's own address). ++ Each collection owns its full storage independently (`ERC1967Proxy` `delegatecall`s logic at the address in the EIP-1967 implementation slot but persists state in the proxy's own address). +``` + +Line 516: `s/Clones are immutable per release/Per-collection proxies are immutable per release/`, `s/for those future clones/for those future collections/`. + +- [ ] **Step 13: §7.2 row 15 split into 15a / 15b + new row 16** + +Find row 15 in the §7.2 risks table. Replace with two rows and a new row 16: + +```diff +- | 15 | Implementation bytecode permanence ... | Implementations deployed via `CREATE` only ... | ++ | 15a | Implementation bytecode permanence | Implementations deployed via `CREATE` only (sequential nonce, never `CREATE2`); no `SELFDESTRUCT` in own/inherited code; no `delegatecall` to caller-provided addresses; verified by opcode-walker test | ++ | 15b | Per-collection proxy permanence | Deployed via `CREATE2` with `externalId` salt using canonical OZ `ERC1967Proxy` unmodified; impls do not inherit `UUPSUpgradeable`; no `ProxyAdmin` pattern; therefore the EIP-1967 impl slot is constructor-fixed and the proxy bytecode is permanent. Verified by: (i) lockfile-pinned OZ import, (ii) opcode-walker test on `ERC1967Proxy` runtime, (iii) unit test asserting impls have no `upgradeTo*` selectors | ++ | 16 | Audit posture for OZ proxy import | Audit must verify `import {ERC1967Proxy} from '@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol'` resolves to canonical OZ at the lockfile-pinned version; remappings or forks of OZ proxy contracts are out of band and would invalidate the bytecode-permanence proof | +``` + +- [ ] **Step 14: §9.1 deploy step 4 note** + +Add a clarifying note at the end of step 4 in §9.1: + +> "Note: this `ERC1967Proxy(factoryImpl, ...)` is the *factory's own* proxy. The per-collection `ERC1967Proxy` instances are deployed by the factory itself at `createCollection*` time, not by this script." + +- [ ] **Step 15: §9.4 runbook addition (per-collection EIP-1967 slot check)** + +Find §9.4 "Pre-/Post-Upgrade Checklist" and append a new line under the post-deploy section: + +```bash +EIP1967_IMPL_SLOT=0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc +cast storage "$COLLECTION_ADDR" "$EIP1967_IMPL_SLOT" --rpc-url "$L2_RPC" +# expected: padded address of UserCollection721 (or 1155) impl +``` + +- [ ] **Step 16: Final vocabulary sweep** + +Run a `grep` to catch any remaining "clone" mentions outside the historical references in §1.4 row 7 footnote and §4.4: + +```bash +grep -niE "[Cc]lone[A-Za-z]*" src/collections/doc/spec/user-collections-specification.md +``` + +Review each hit; replace with "collection" / "collections" unless it's a deliberate historical reference. (The mermaid diagram subgraph rename and table rewrites should already cover most.) + +- [ ] **Step 17: Commit** + +```bash +git add src/collections/doc/spec/user-collections-specification.md +git commit -m "docs(collections): refit spec for ERC1967Proxy per-collection deploy + +Vocabulary pass clones → collections, deployment-model rewrite, §1.4 +upgradeability footnote, §3.4 atomic-flow bullet, §4.1 sequence diagram, +§4.4 gas profile, §4.5 new Address Determinism sub-section, §6.2 Clone +Storage opening, §7.2 row 15 split into 15a/15b + new row 16 for OZ +proxy import audit posture, §9.4 per-collection EIP-1967 slot check." +``` + +--- + +## Task 11: Add `factoryDependencies` build-artifact gate to deploy script + +**Files:** +- Modify: `ops/deploy_collection_factory_zksync.sh` + +After `compile_contracts`, the factory must have `ERC1967Proxy`'s zk hash in its `factoryDependencies`. If empty, the build silently produced a non-functional factory. Lock this as a CI gate. + +- [ ] **Step 1: Add a check function** + +Open `ops/deploy_collection_factory_zksync.sh`. After the existing `compile_contracts()` function (around line 152), add: + +```bash +# ============================================================================= +# Build-artifact verification — factoryDependencies must be populated. +# Empty factoryDependencies on CollectionFactory means createCollection* +# would revert at runtime on EraVM (the original Clones.clone() bug). +# ============================================================================= + +verify_build_artifacts() { + log_info "Verifying CollectionFactory factoryDependencies are populated..." + + local artifact="zkout/CollectionFactory.sol/CollectionFactory.json" + if [ ! -f "$artifact" ]; then + log_error "Compiled artifact not found: $artifact" + exit 1 + fi + + local dep_count + dep_count=$(jq -r '.factoryDependencies | length' "$artifact") + + if [ "$dep_count" -eq 0 ]; then + log_error "CollectionFactory.factoryDependencies is empty." + log_error "This means the factory cannot deploy per-collection proxies on EraVM." + log_error "Refer to docs §3.5.2 / row 15b — ERC1967Proxy must appear in factoryDeps." + exit 1 + fi + + log_success "factoryDependencies populated ($dep_count entries)" +} +``` + +- [ ] **Step 2: Call it from `main()`** + +Find `main()` near the bottom and insert the new step after `compile_contracts`: + +```diff + preflight_checks + move_l1_contracts + compile_contracts ++ verify_build_artifacts + deploy_contracts +``` + +- [ ] **Step 3: Run the dry-run to confirm the gate fires correctly** + +Before the factory refactor lands, this check would *fail* (empty factoryDeps proves the gate works). After the refactor lands, it should *pass*. Run: + +```bash +./ops/deploy_collection_factory_zksync.sh testnet +``` + +Expected: dry-run completes through `verify_build_artifacts` and prints `factoryDependencies populated (≥1 entries)`. + +- [ ] **Step 4: Commit** + +```bash +git add ops/deploy_collection_factory_zksync.sh +git commit -m "ops(collections): gate deploy on populated factoryDependencies + +CollectionFactory's factoryDependencies must contain at least one entry +(ERC1967Proxy). Empty factoryDeps means the factory cannot deploy +per-collection proxies on EraVM at runtime — exactly the failure mode +that left the original Clones.clone() factory broken on zkSync Era." +``` + +--- + +## Task 12: Add post-broadcast end-to-end smoke test to deploy script + +**Files:** +- Modify: `ops/deploy_collection_factory_zksync.sh` + +After `--broadcast`, `cast send` a real `createCollection721` against the freshly deployed factory and assert correctness on-chain. This is the empirical EraVM check the missing of which left `Clones.clone()` undetected. + +- [ ] **Step 1: Add a smoke-test function** + +In `ops/deploy_collection_factory_zksync.sh`, after `verify_deployment()` (around line 260), add: + +```bash +# ============================================================================= +# End-to-end smoke test — exercise createCollection721 on the live network. +# This is the empirical check that the EraVM-compiled output works at runtime. +# ============================================================================= + +smoke_test_createCollection() { + if [ "$BROADCAST" != "--broadcast" ]; then + return 0 + fi + + log_info "Running end-to-end smoke test: createCollection721..." + + local rpc + if [ "$NETWORK" = "mainnet" ]; then + rpc="${L2_RPC:-https://mainnet.era.zksync.io}" + else + rpc="${L2_RPC:-https://rpc.ankr.com/zksync_era_sepolia}" + fi + + # Build a minimal CreateParams721 calldata. owner = operator, + # additionalMinters = empty array, royaltyBps = 0, simple URIs. + local extId + extId=$(cast keccak "smoke-$(date +%s)") + + local params + params=$(cast abi-encode \ + "f((string,string,address,address[],string,address,uint96,string))" \ + "(\"Smoke\",\"SMK\",$N_FACTORY_OPERATOR,[],\"ipfs://smoke/\",$N_FACTORY_OPERATOR,0,\"ipfs://smoke.json\")") + + log_info "Calling createCollection721($extId)..." + cast send "$COLLECTION_FACTORY_PROXY" \ + "createCollection721((string,string,address,address[],string,address,uint96,string),bytes32)" \ + "$params" "$extId" \ + --rpc-url "$rpc" \ + --private-key "$DEPLOYER_PRIVATE_KEY" \ + --zksync \ + || { log_error "createCollection721 reverted on-chain"; exit 1; } + + # Read the resulting collection address from the mapping. + local collection + collection=$(cast call "$COLLECTION_FACTORY_PROXY" \ + "collectionByExternalId(bytes32)(address)" "$extId" --rpc-url "$rpc") + + log_info "Smoke collection deployed at: $collection" + + # Assert non-empty code at the collection address. + local code_size + code_size=$(cast code "$collection" --rpc-url "$rpc" | wc -c) + if [ "$code_size" -lt 10 ]; then + log_error "Smoke collection has empty bytecode" + exit 1 + fi + + # Assert EIP-1967 impl slot equals expected impl. + local EIP1967_IMPL_SLOT="0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc" + local stored + stored=$(cast storage "$collection" "$EIP1967_IMPL_SLOT" --rpc-url "$rpc") + log_info "EIP-1967 impl slot: $stored (expected impl: $USER_COLLECTION_721_IMPL)" + + log_success "Smoke test passed: createCollection721 succeeded; collection has code; EIP-1967 slot set" +} +``` + +- [ ] **Step 2: Wire it into `main()`** + +```diff + verify_deployment ++ smoke_test_createCollection + verify_source_code +``` + +- [ ] **Step 3: Commit (don't run yet — requires real broadcast)** + +```bash +git add ops/deploy_collection_factory_zksync.sh +git commit -m "ops(collections): post-broadcast createCollection721 smoke test on zkSync + +Calls createCollection721 against the freshly deployed factory and +asserts the resulting collection has non-empty code and a populated +EIP-1967 impl slot. Catches EraVM-runtime failures that compile-time +gates miss (e.g. the original Clones.clone() incompatibility)." +``` + +The smoke test runs only when `--broadcast` is passed. We deliberately don't dry-run it here — that requires a real testnet deploy, which is the final task. + +--- + +## Task 13: Final verification — full test sweep + storage-layout baseline diff + +**Files:** none (verification only) + +- [ ] **Step 1: Run the full test suite** + +```bash +forge test --match-path 'test/collections/*' -vv +``` + +Expected: all tests pass. Count should be ≥69 (67 original + 2 CREATE2 derivation + 2 no-upgrade-selector + 1 immediate-mint + 2 ERC1967Proxy permanence = 74 minimum). If anything is red, debug before proceeding. + +- [ ] **Step 2: Verify storage-layout baselines unchanged** + +```bash +forge inspect src/collections/CollectionFactory.sol:CollectionFactory storage-layout > /tmp/factory-now.json +diff <(jq -S . src/collections/layouts/CollectionFactory.v1.json) <(jq -S . /tmp/factory-now.json) + +forge inspect src/collections/UserCollection721.sol:UserCollection721 storage-layout > /tmp/uc721-now.json +diff <(jq -S . src/collections/layouts/UserCollection721.v1.json) <(jq -S . /tmp/uc721-now.json) + +forge inspect src/collections/UserCollection1155.sol:UserCollection1155 storage-layout > /tmp/uc1155-now.json +diff <(jq -S . src/collections/layouts/UserCollection1155.v1.json) <(jq -S . /tmp/uc1155-now.json) +``` + +Expected: all three diffs are empty (the refactor doesn't touch storage). If a diff is non-empty, something unexpected changed — investigate before continuing. + +- [ ] **Step 3: zkSync compile + factoryDependencies check** + +```bash +./ops/deploy_collection_factory_zksync.sh testnet +``` + +Expected: dry-run reaches `verify_build_artifacts` and reports `factoryDependencies populated (≥1 entries)`. The L1-move/restore patch from earlier is still required for compile to succeed; it should be merged or staged before this step. + +- [ ] **Step 4: Coverage check** + +```bash +forge coverage --match-path 'test/collections/*' --report lcov > /tmp/coverage.lcov 2>&1 || true +forge coverage --match-path 'test/collections/*' | tail -20 +``` + +Expected: `src/collections/*` line coverage ≥ 95% (current CI floor). The new tests should keep or increase coverage from the current 96.91%. + +- [ ] **Step 5: Spell-check the spec** + +```bash +# If `cspell` is installed in the repo, run it on the spec; otherwise skip. +command -v cspell && cspell src/collections/doc/spec/user-collections-specification.md src/collections/doc/spec/2026-05-08-clones-replacement-design.md src/collections/doc/spec/2026-05-08-clones-replacement-implementation-plan.md +``` + +- [ ] **Step 6: Smoke-test on zkSync Sepolia (optional, requires testnet ETH)** + +Once all the above pass, do an actual `--broadcast` deploy: + +```bash +./ops/deploy_collection_factory_zksync.sh testnet --broadcast +``` + +Expected sequence: +1. `factoryDependencies populated` +2. Implementations + factory + ERC1967Proxy(factory) deploy +3. `verify_deployment` confirms admin/operator roles and EIP-1967 factory-proxy slot +4. `smoke_test_createCollection` calls a real `createCollection721`, prints the new collection address, confirms non-empty code, and confirms the EIP-1967 impl slot points at `UserCollection721`. + +If any step reverts on-chain, capture the tx hash and the EraVM trace for analysis. The most likely failure modes (and how to read them): + +- `createCollection721` reverts inside the constructor → check whether `ERC1967Proxy`'s zk bytecode hash is registered as a factory dep (`jq '.factoryDependencies' zkout/CollectionFactory.sol/CollectionFactory.json`). +- `createCollection721` reverts on `delegatecall` to impl → check that `_erc721Implementation()` returns the deployed impl address and that `cast code` on that address is non-empty. +- `createCollection721` reverts with `DuplicateExternalId` → the externalId computed via `cast keccak "smoke-..."` collided; rerun with a different timestamp. + +- [ ] **Step 7: No code commit needed** + +This task is verification only. If everything passes, the implementation is complete. If §6 (live broadcast) actually deployed, capture the addresses in `.env-test` (the deploy script does this automatically) and tag the commit if you want a snapshot. + +--- + +## Self-Review Notes + +**Spec coverage:** +- §3.1 factory state — Tasks 2, 3 (preserved by not touching storage) +- §3.2 deploy path (721 + 1155) — Tasks 1, 2, 3 +- §3.3 init flow + atomic emits — Task 4 +- §3.4 address determinism — Tasks 1, 3 (CREATE2 derivation tests) +- §3.5 bytecode permanence (15a + 15b) — Tasks 6, 7 + spec update Task 10 +- §3.6 spec deltas — Task 10 +- §3.7 storage-layout baselines unchanged — Task 13 (verification) +- §3.8 test impact — Tasks 1, 3, 4, 5, 6, 7, 8, 9 +- §3.8.4 CI factoryDeps gate — Task 11 +- §3.8.3 point 5 end-to-end smoke — Task 12 +- §3.9 audit checklist — Task 6 codifies row 16 in CI; Task 10 documents the audit gates + +All design-doc requirements have a corresponding task. No gaps. + +**Placeholder scan:** No `TBD` / `TODO` / `add appropriate` / `similar to Task N` placeholders. Every code-mutating step contains the actual code. + +**Type consistency:** Method names match across tasks (`createCollection721` / `createCollection1155` / `_erc721Implementation` / `_erc1155Implementation` / `_collectionByExternalId` / `IUserCollection721.initialize` / `IUserCollection1155.initialize`). The `IMPL_SLOT` / `EIP1967_IMPL_SLOT` constant is consistent across tasks 7, 10, 12, 13. Event names `Upgraded` / `Initialized` / `CollectionCreated` are consistent. + +**Scope:** Single implementation plan, one PR's worth of changes. Touches 7 files modified + 1 added + 1 doc. ~13 logical commits, each green-on-its-own. diff --git a/src/collections/doc/spec/user-collections-specification.md b/src/collections/doc/spec/user-collections-specification.md new file mode 100644 index 00000000..78a02b78 --- /dev/null +++ b/src/collections/doc/spec/user-collections-specification.md @@ -0,0 +1,844 @@ +--- +title: "Nodle User Collections — Technical Specification" +subtitle: "Operator-Triggered NFT Collection Factory with Per-Collection ERC1967Proxy Isolation" +date: "April 2026" +version: "1.0" +--- + +
+ +# Nodle User Collections + +## Technical Specification + +**Operator-Triggered NFT Collection Factory with Per-Collection ERC1967Proxy Isolation** + +Version 1.0 — April 2026 + +
+ +
+ +## Table of Contents + +1. [Introduction & Architecture](#1-introduction--architecture) +2. [Roles & Access Control](#2-roles--access-control) +3. [Contract Interfaces](#3-contract-interfaces) +4. [Collection Creation Flow](#4-collection-creation-flow) +5. [Item Minting Flows](#5-item-minting-flows) +6. [Storage Layout](#6-storage-layout) +7. [Security Model](#7-security-model) +8. [Testing Strategy](#8-testing-strategy) +9. [Deployment & Operations](#9-deployment--operations) +10. [File Layout](#10-file-layout) +11. [Open Considerations](#11-open-considerations) + +
+ +## 1. Introduction & Architecture + +### 1.1 System Overview + +The User Collections system lets users create their own ERC-721 or ERC-1155 NFT collections on the Nodle zkSync L2. Creation is paid in fiat off-chain; the on-chain deployment is triggered by a trusted backend after the fiat payment clears. Each collection is a fully-isolated per-collection ERC1967Proxy with its own address, owner, and metadata. + +The on-chain layer provides: + +- A single upgradeable factory that deploys per-collection `ERC1967Proxy` instances pointing at fixed-behavior implementation contracts. +- Two implementation templates (ERC-721 and ERC-1155), both inheriting OpenZeppelin's audited upgradeable primitives. +- Role-scoped permissions: a backend operator can trigger creation and mint into any collection, while creators retain ownership and minting rights on their own collection. +- Reconciliation hooks (`externalId` events and lookup map) so the off-chain payment ledger can deterministically locate the on-chain artifact for every order. + +### 1.2 Architecture + +```mermaid +graph TB + subgraph Off-chain["Off-chain (out of scope)"] + APP(("App /
Frontend")) + BE(("Backend
Service")) + PAY(("Fiat
Processor")) + end + + subgraph Factory_Layer["Factory Layer (UUPS Upgradeable)"] + FAC["CollectionFactory
UUPS proxy
operator-only creation"] + end + + subgraph Impl["Implementation Templates (immutable per release)"] + I721["UserCollection721
ERC-721 logic"] + I1155["UserCollection1155
ERC-1155 logic"] + end + + subgraph Collections["User Collections (ERC1967Proxy per collection)"] + C1["Collection A
creator α"] + C2["Collection B
creator β"] + C3["Collection C
creator γ"] + end + + APP -- "create / mint" --> BE + BE -- "fiat charge" --> PAY + BE -- "createCollection*" --> FAC + FAC -- "new ERC1967Proxy{salt}" --> C1 + FAC -- "new ERC1967Proxy{salt}" --> C2 + FAC -- "new ERC1967Proxy{salt}" --> C3 + C1 -. "delegatecall" .-> I721 + C2 -. "delegatecall" .-> I1155 + C3 -. "delegatecall" .-> I721 + + style FAC fill:#ff9f43,color:#fff + style I721 fill:#4a9eff,color:#fff + style I1155 fill:#4a9eff,color:#fff + style C1 fill:#2ecc71,color:#fff + style C2 fill:#2ecc71,color:#fff + style C3 fill:#2ecc71,color:#fff + style APP fill:#95a5a6,color:#fff + style BE fill:#95a5a6,color:#fff + style PAY fill:#95a5a6,color:#fff +``` + +### 1.3 Core Components + +| Contract | Role | Pattern | Upgradeability | +| :-------------------- | :------------------------------------------------------------- | :----------------------- | :---------------------------------------- | +| `CollectionFactory` | Operator-triggered factory; emits creation events | UUPS proxy | Admin-upgradeable | +| `UserCollection721` | ERC-721 implementation behind a per-collection ERC1967Proxy | `ERC1967Proxy` implementation | Immutable per collection | +| `UserCollection1155` | ERC-1155 implementation behind a per-collection ERC1967Proxy | `ERC1967Proxy` implementation | Immutable per collection | + +The factory is upgradeable so new implementation templates and bug fixes can be shipped without disrupting existing creators. Already-deployed collections cannot be upgraded — buyers and creators retain a permanent guarantee about each collection's behavior. + +### 1.4 Design Decisions + +| # | Decision | Choice | +| :- | :----------------------------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | Token standards | Both ERC-721 and ERC-1155, selected per-collection | +| 2 | Deployment model | Per-collection `ERC1967Proxy` deployed via `CREATE2` with `externalId` salt; implementations deployed via `CREATE` only | +| 3 | Payment model | Fiat, off-chain; on-chain creation is purely authorization-gated | +| 4 | Authorization | Operator-deployed: backend holds `OPERATOR_ROLE`, creator never signs creation | +| 5 | Item minting rights | Creator and operator both hold `MINTER_ROLE` on every collection — operator grant is enforced on-chain by the factory (see §2.3), not by backend convention | +| 6 | Per-collection mutability | `baseURI`/`uri`, `contractURI`, royalties are owner-mutable until owner locks them one-way | +| 7 | Upgradeability | Factory: UUPS-upgradeable. Per-collection proxies: immutable (impls do not inherit `UUPSUpgradeable`, no admin slot); admin can swap implementation pointer for *future* collections only [^upgradeability] | +| 8 | Inheritance | Direct from OpenZeppelin `*Upgradeable` contracts. No reuse of `BaseContentSign` (constructor-based, non-upgradeable, structurally incompatible with proxy initialization) | +| 9 | External-ID dedup | On-chain map `bytes32 externalId → address collection`; reverts on reuse | +| 10 | Per-creator on-chain limit | None (backend rate-limits if needed) | +| 11 | Royalty ceiling | None beyond OpenZeppelin's ERC-2981 100% (10000 bps) bound. Creators have full autonomy over `royaltyBps` until they call `lockRoyalties`. Marketplace-norm enforcement (e.g. ≤10%) is deliberately out of scope — buyers and frontends are expected to inspect the on-chain value | +| 12 | OZ alignment | Stay aligned with OpenZeppelin's `*Upgradeable` shapes; avoid overriding inherited methods or diverging from canonical signatures unless strictly required (e.g. role gating, lock checks). Custom batch / utility helpers ship as net-new functions, not overrides — keeps the audit surface small and lets us track upstream OZ patches without merge friction | + +[^upgradeability]: We use `ERC1967Proxy` directly (not `TransparentUpgradeableProxy`, not `BeaconProxy`) because the implementation contracts deliberately do not inherit `UUPSUpgradeable`. With no upgrade selector exposed and no `ProxyAdmin` slot pattern, the proxy's implementation pointer is constructor-fixed and the per-collection immutability promise is enforced by code that already exists in the OZ canonical libraries — no custom upgrade gating needed. We migrated away from the EIP-1167 minimal-proxy pattern because it is incompatible with zkSync Era's `ContractDeployer` factoryDeps model (see `2026-05-08-clones-replacement-design.md`). + +### 1.5 Non-Goals + +- On-chain payment collection (fee paid in fiat off-chain). +- App, frontend, or off-chain backend implementation. +- Marketplace, secondary sales, or royalty enforcement beyond ERC-2981 declarations. +- Modifications to the existing `ContentSign` contract family. +- KYC, tax, or content moderation (off-chain concerns). + +
+ +## 2. Roles & Access Control + +### 2.1 Role Map + +| Role | Held by | Scope | Capabilities | +| :-------------------- | :----------------------- | :------------ | :------------------------------------------------------------------------------------------------- | +| `DEFAULT_ADMIN_ROLE` | L2 admin Safe (multisig) | Factory | Upgrade factory; swap implementation pointers; grant/revoke `OPERATOR_ROLE` | +| `OPERATOR_ROLE` | Backend service key | Factory | Call `createCollection721` / `createCollection1155` | +| `OWNER_ROLE` | Collection creator | Each collection | Set/lock `baseURI`/`uri`, `contractURI`, royalties; grant/revoke `MINTER_ROLE` on their collection | +| `MINTER_ROLE` | Creator + operator | Each collection | Mint items into the collection | + +### 2.2 Role Admin Hierarchy + +```mermaid +graph LR + DAR["DEFAULT_ADMIN_ROLE
(factory)"] --> OPR["OPERATOR_ROLE
(factory)"] + OWN["OWNER_ROLE
(per-collection)"] --> MIN["MINTER_ROLE
(per-collection)"] + + style DAR fill:#ff9f43,color:#fff + style OPR fill:#ff9f43,color:#fff + style OWN fill:#2ecc71,color:#fff + style MIN fill:#2ecc71,color:#fff +``` + +On the factory, `DEFAULT_ADMIN_ROLE` administers `OPERATOR_ROLE`. On each collection, `OWNER_ROLE` administers `MINTER_ROLE` (so the creator can grant additional minters or revoke the operator's minting rights if they wish). + +### 2.3 Operator Minter Auto-Grant + +When the factory creates a collection, it passes `msg.sender` (the `OPERATOR_ROLE` holder that triggered creation) to the collection's `initialize` as a guaranteed minter. The collection unconditionally grants that address `MINTER_ROLE` during initialization, alongside any addresses listed in `additionalMinters`. + +This makes operator-driven minting a **contract-level invariant** rather than a backend convention: it is impossible to deploy a collection through the factory that the calling operator cannot mint into. After creation, creators retain full control via `OWNER_ROLE` and may revoke the operator's `MINTER_ROLE` at any time. + +**Operator key rotation.** When admin grants `OPERATOR_ROLE` to a new address, all *future* collections auto-grant `MINTER_ROLE` to the new operator. Existing collections are unaffected (immutable per release); creators must grant `MINTER_ROLE` to the new operator individually if they want operator-driven minting to continue on collections deployed before the rotation. The backend should track this as part of any rotation runbook. + +`additionalMinters` remains available for creators who want extra minters seeded at creation (e.g. a co-creator wallet) and is orthogonal to the operator auto-grant. + +
+ +## 3. Contract Interfaces + +### 3.1 Public Interfaces + +| Interface | Description | +| :-------------------------------- | :------------------------------------------------------- | +| `interfaces/ICollectionFactory.sol` | Factory public API | +| `interfaces/IUserCollection721.sol` | ERC-721 implementation public API | +| `interfaces/IUserCollection1155.sol`| ERC-1155 implementation public API | +| `interfaces/CollectionTypes.sol` | Shared enums and structs (`Standard`, `CreateParams*`) | + +### 3.2 Contract Classes + +```mermaid +classDiagram + class CollectionFactory { + +address erc721Implementation + +address erc1155Implementation + +mapping collectionByExternalId : bytes32 → address + -- + +initialize(admin, operator, impl721, impl1155) + +createCollection721(p, externalId) → address + +createCollection1155(p, externalId) → address + +setImplementation721(impl) + +setImplementation1155(impl) + +_authorizeUpgrade(newImpl) onlyAdmin + } + + class UserCollection721 { + +string contractURI + +uint256 nextTokenId + +bool metadataLocked + +bool royaltiesLocked + +uint256 MAX_BATCH = 100 + -- + +initialize(p, operatorMinter) + +mint(to, tokenURI_) → tokenId + +mintBatch(to[], uris[]) → tokenIds[] + +setBaseURI(newBase) + +setContractURI(newURI) + +setDefaultRoyalty(recipient, bps) + +lockMetadata() + +lockRoyalties() + } + + class UserCollection1155 { + +string contractURI + +bool metadataLocked + +bool royaltiesLocked + +uint256 MAX_BATCH = 100 + -- + +initialize(p, operatorMinter) + +mint(to, id, amount, data) + +mintBatch(to, ids[], amounts[], data) + +setURI(newURI) + +setContractURI(newURI) + +setDefaultRoyalty(recipient, bps) + +lockMetadata() + +lockRoyalties() + } + + CollectionFactory --> UserCollection721 : deploys proxy + CollectionFactory --> UserCollection1155 : deploys proxy +``` + +### 3.3 Shared Types + +```solidity +enum Standard { ERC721, ERC1155 } + +struct CreateParams721 { + address owner; + string name; + string symbol; + string baseURI; + string contractURI; + address royaltyRecipient; + uint96 royaltyBps; + address[] additionalMinters; +} + +struct CreateParams1155 { + address owner; + string uri; // ERC-1155 URI (typically with {id} placeholder) + string contractURI; + address royaltyRecipient; + uint96 royaltyBps; + address[] additionalMinters; +} +``` + +ERC-1155 has no on-chain `name`/`symbol` convention; the collection display name lives in `contractURI` JSON metadata. + +### 3.4 `CollectionFactory` + +```solidity +interface ICollectionFactory { + event CollectionCreated( + address indexed creator, + address indexed collection, + Standard standard, + bytes32 indexed externalId + ); + event ImplementationUpdated(Standard standard, address newImpl); + + error ExternalIdAlreadyUsed(bytes32 externalId); + error InvalidExternalId(); + error ZeroAddress(); + error NotAContract(address impl); + + function initialize( + address admin, + address operator, + address impl721, + address impl1155 + ) external; + + function createCollection721(CreateParams721 calldata p, bytes32 externalId) + external returns (address collection); + + function createCollection1155(CreateParams1155 calldata p, bytes32 externalId) + external returns (address collection); + + function setImplementation721(address impl) external; + function setImplementation1155(address impl) external; + + function collectionByExternalId(bytes32 externalId) external view returns (address); + function erc721Implementation() external view returns (address); + function erc1155Implementation() external view returns (address); +} +``` + +#### Behavior + +- `initialize` is callable once. Grants `admin → DEFAULT_ADMIN_ROLE`, `operator → OPERATOR_ROLE`. Reverts `ZeroAddress` if any of `admin`, `operator`, `impl721`, `impl1155` is zero. Reverts `NotAContract(impl)` if either implementation address has zero bytecode (`impl.code.length == 0`). +- `createCollection*`: + - Restricted to `OPERATOR_ROLE`. + - Reverts `InvalidExternalId` if `externalId == bytes32(0)` (forces a non-trivial ID). + - Reverts `ExternalIdAlreadyUsed` if the ID has already been used. + - Atomic flow: `abi.encodeCall(initialize, (p, msg.sender))` → `new ERC1967Proxy{salt: externalId}(impl, initData)` (deploy + delegatecall init in a single constructor frame) → `collectionByExternalId[externalId] = collection` → `emit CollectionCreated`. Passing `msg.sender` into `initData` ensures the calling operator is auto-granted `MINTER_ROLE` on the new collection (see §2.3). + - Returns the collection address. +- `setImplementation*` is restricted to `DEFAULT_ADMIN_ROLE` and affects future collections only. Existing collections continue to delegatecall their original implementation. Reverts `ZeroAddress` if `impl == address(0)`. Reverts `NotAContract(impl)` if `impl.code.length == 0` (defends against EOA paste / unset env var). The setter does **not** verify the implementation matches the expected token standard (e.g. a 1155 implementation pasted into `setImplementation721`); the post-upgrade `cast` checks in §9.4 are the runbook layer that catches this. +- `_authorizeUpgrade(address)` is `onlyRole(DEFAULT_ADMIN_ROLE)`. + +### 3.5 `UserCollection721` + +```solidity +interface IUserCollection721 { + event MetadataLocked(); + event RoyaltiesLocked(); + event ContractURIUpdated(string newURI); + event BaseURIUpdated(string newBase); + + error MetadataIsLocked(); + error RoyaltiesAreLocked(); + error BatchTooLarge(uint256 length, uint256 max); + error LengthMismatch(); + + function initialize(CreateParams721 calldata p, address operatorMinter) external; + + function mint(address to, string calldata tokenURI_) external returns (uint256 tokenId); + function mintBatch(address[] calldata to, string[] calldata uris) + external returns (uint256[] memory tokenIds); + + function setBaseURI(string calldata newBase) external; + function setContractURI(string calldata newURI) external; + function setDefaultRoyalty(address recipient, uint96 bps) external; + function lockMetadata() external; + function lockRoyalties() external; + + function contractURI() external view returns (string memory); + function nextTokenId() external view returns (uint256); + function metadataLocked() external view returns (bool); + function royaltiesLocked() external view returns (bool); +} +``` + +#### Behavior + +- Inherits `Initializable`, `ERC721Upgradeable`, `ERC721URIStorageUpgradeable`, `ERC721BurnableUpgradeable`, `ERC2981Upgradeable`, `AccessControlUpgradeable`. +- Implementation contract calls `_disableInitializers()` in its constructor so the implementation itself can never be initialized directly. +- **Bytecode-permanence invariants** (load-bearing for the §1.3 immutability promise): the implementation contains no `SELFDESTRUCT` opcode (no `selfdestruct(...)` calls in the implementation's own code or in any inherited contract), and performs no `delegatecall` to caller-provided addresses. Both properties are asserted by the unit test in §8.2 and reviewed by the auditor. +- `initialize` (initializer-gated): sets name/symbol via `__ERC721_init`, sets `baseURI` and `contractURI`, sets default royalty if `royaltyBps > 0`, grants `OWNER_ROLE` and `MINTER_ROLE` to `owner`, grants `MINTER_ROLE` to `operatorMinter` (passed by the factory; see §2.3), grants `MINTER_ROLE` to each `additionalMinters` entry, and calls `_setRoleAdmin(MINTER_ROLE, OWNER_ROLE)`. Reverts `ZeroAddress` if `operatorMinter == address(0)`. Re-granting an already-held role is a no-op (OZ `grantRole` is idempotent), so duplicates between `owner`, `operatorMinter`, and `additionalMinters` are safe. +- `mint`: `MINTER_ROLE`-gated. Increments `nextTokenId`, calls `_safeMint`, sets per-token URI via `ERC721URIStorage._setTokenURI`. Returns the new token ID. +- `mintBatch`: `MINTER_ROLE`-gated. Reverts `LengthMismatch` if `to.length != uris.length`. Reverts `BatchTooLarge` if `to.length > MAX_BATCH` (100). Returns `uint256[] tokenIds` in the same order as `to`; the values are a contiguous range starting at the value of `nextTokenId` at call entry. The return lets backends reconcile per-buyer attribution synchronously without parsing `Transfer` logs or racing against concurrent minters. +- `setBaseURI`: `OWNER_ROLE`-gated. Reverts `MetadataIsLocked` when `metadataLocked == true`. +- `setContractURI`: `OWNER_ROLE`-gated. Reverts `MetadataIsLocked` when `metadataLocked == true`. The single `metadataLocked` flag covers both per-collection (`baseURI`) and collection-level (`contractURI`) metadata so that buyers see one verifiable "metadata is frozen" signal across the whole collection. (Per-token URIs minted via `ERC721URIStorage._setTokenURI` are anchored at mint time independently of this flag — see §7.2 row 7.) +- `setDefaultRoyalty`: `OWNER_ROLE`-gated. Reverts `RoyaltiesAreLocked` when `royaltiesLocked == true`. No additional cap beyond OZ's ERC-2981 100% bound (see §1.4 row 11) — creators may set any value up to 10000 bps. +- `lockMetadata` / `lockRoyalties`: `OWNER_ROLE`-gated, one-way; emit events for indexers. + +### 3.6 `UserCollection1155` + +Mirrors §3.5 with ERC-1155 mechanics: + +- Inherits `Initializable`, `ERC1155Upgradeable`, `ERC1155SupplyUpgradeable`, `ERC1155BurnableUpgradeable`, `ERC2981Upgradeable`, `AccessControlUpgradeable`. +- `initialize(CreateParams1155 calldata p, address operatorMinter)` — same role-grant semantics as §3.5: `OWNER_ROLE` + `MINTER_ROLE` to `owner`, `MINTER_ROLE` to `operatorMinter` (factory-passed; see §2.3), `MINTER_ROLE` to each `additionalMinters` entry, `_setRoleAdmin(MINTER_ROLE, OWNER_ROLE)`. Reverts `ZeroAddress` if `operatorMinter == address(0)`. +- `mint(address to, uint256 id, uint256 amount, bytes data)` — `MINTER_ROLE`-gated. +- `mintBatch(address to, uint256[] ids, uint256[] amounts, bytes data)` — single recipient, matching OZ's `_mintBatch`. Reverts `BatchTooLarge` when `ids.length > MAX_BATCH` and `LengthMismatch` when `ids.length != amounts.length`. Multi-recipient batching (one tx, N different buyers) is intentionally out of scope for v1: it has no native OZ primitive, would require a custom loop emitting N `TransferSingle` events, and the dominant fiat-paid 1:1 flow (§5.2) doesn't naturally batch (per-buyer payments clear independently). If airdrop or allowlist-drop flows become a product requirement, a `mintBatchMulti` can be added in a future implementation pointer (admin swap, future collections only) without breaking existing callers — see §11. +- `setURI(string newURI)` instead of `setBaseURI`. Subject to `metadataLocked`. +- No `nextTokenId` (1155 IDs are caller-chosen). +- **Bytecode-permanence invariants** apply identically to §3.5: implementation constructor calls `_disableInitializers()`; implementation contains no `SELFDESTRUCT` opcode and no `delegatecall` to caller-provided addresses. Asserted by the §8.3 unit test. + +
+ +## 4. Collection Creation Flow + +### 4.1 End-to-End Sequence + +```mermaid +sequenceDiagram + autonumber + participant U as User + participant App as App + participant BE as Backend + participant Pay as Fiat Processor + participant FAC as CollectionFactory + participant CL as New Collection + + U->>App: Submit collection params + App->>BE: createCollection request + payment authorization + BE->>Pay: Charge fiat + Pay-->>BE: Payment cleared + BE->>BE: Assign orderId, externalId = keccak256(orderId) + BE->>FAC: createCollection721(params, externalId) + Note over FAC: only OPERATOR_ROLE may call + FAC->>FAC: require externalId != 0 + FAC->>FAC: require collectionByExternalId[externalId] == 0 + FAC->>CL: new ERC1967Proxy{salt: externalId}(erc721Implementation, encodeCall(initialize, (p, msg.sender))) + Note over CL: emit Upgraded(impl), emit Initialized(1) inside constructor + Note over CL: auto-grants MINTER_ROLE to operator + FAC->>FAC: collectionByExternalId[externalId] = collection + FAC-->>BE: emit CollectionCreated(creator, collection, ERC721, externalId) + BE->>App: Mark order completed; return collection address + App-->>U: Display collection address +``` + +### 4.2 Atomicity & Front-Running + +The collection is deployed and initialized inside the same transaction. The collection is never visible on-chain in an uninitialized state, so initialization cannot be front-run by an external observer. Deploy and initialize occur in a single constructor frame; there is no transient window where the proxy exists in an uninitialized state. + +### 4.3 Reconciliation + +`externalId` is emitted as an indexed event topic and stored in `collectionByExternalId`. The backend can: + +- Confirm a collection exists for an order: `factory.collectionByExternalId(externalId)`. +- Detect duplicate triggers: revert on reuse prevents double-creation. +- Recover from local state loss: re-derive `externalId = keccak256(orderId)` and look up the collection on-chain. + +### 4.4 Gas Profile + +A successful `createCollection721`: + +- On zkSync Era, per-collection deploy is dominated by `ContractDeployer.create2` plus the constructor's delegatecall init. Gas measured by `Collections.integration.t.sol` and quoted from the test output (target: < 1.5M gas on zkSync Sepolia for a typical `createCollection721`). The previous EIP-1167 baseline (~45k gas on EVM L1) is no longer applicable because we don't deploy minimal proxies. +- Calls `initialize(...)` via the proxy constructor — variable, dominated by string storage (`baseURI`, `contractURI`). +- Writes the `collectionByExternalId` mapping (one warm SSTORE). +- Emits the event. + +On zkSync Era the absolute cost is dominated by L1 calldata fees and is well under one cent at typical L1 gas prices. + +### 4.5 Address Determinism + +Per-collection addresses are deterministic on-chain because the factory uses `new ERC1967Proxy{salt: externalId}(...)`. The address is a pure function of: + +- `factory` proxy address (constant per environment) +- `externalId` (the salt; supplied by the operator) +- `_erc721Implementation` / `_erc1155Implementation` (read once via `erc721Implementation()` / `erc1155Implementation()`; refresh after admin upgrades) +- `initData = abi.encodeCall(initialize, (params, operatorAddress))` +- `ERC1967Proxy` zk bytecode hash (constant per zksolc release; pin in backend artifacts at the version used at factory deploy time) + +Backends can pre-derive the collection address before broadcasting `createCollection*` using `zksync-ethers` `utils.create2Address`. The on-chain `_collectionByExternalId[externalId]` mapping remains the canonical registry — pre-derivation is a redundant off-chain lookup path, not a replacement. + +**Caveats:** +1. Address is sensitive to every input — different `params` or different operator → different address. +2. Operator rotation (§2.3): pre-derive using the *current* OPERATOR_ROLE holder. +3. `_collectionByExternalId` mapping stays canonical and enforces uniqueness via `_checkExternalId`. +4. Pin the `ERC1967Proxy` zk bytecode hash; refresh only during a coordinated zksolc bump. + +
+ +## 5. Item Minting Flows + +### 5.1 Creator-Driven Mint + +```mermaid +sequenceDiagram + autonumber + participant CR as Creator Wallet + participant App as App + participant IPFS as IPFS / Pinning + participant CL as User Collection + participant PM as BondTreasuryPaymaster + + CR->>App: Select "Mint item" + App->>IPFS: Pin metadata + IPFS-->>App: CID + App->>CR: Sign tx → mint(creator, "ipfs://CID/metadata.json") + CR->>PM: Submit tx (paymaster sponsors gas) + PM->>CL: mint(creator, uri) + CL-->>App: emit Transfer(0x0, creator, tokenId) +``` + +The creator holds `MINTER_ROLE` on their own collection by default. If the creator's wallet has bond allowance under `BondTreasuryPaymaster`, gas is sponsored; otherwise the creator pays gas directly. + +### 5.2 Operator-Driven Mint (Fiat-Priced Item Sale) + +```mermaid +sequenceDiagram + autonumber + participant B as Buyer + participant App as App + participant BE as Backend + participant Pay as Fiat Processor + participant CL as User Collection + + B->>App: Buy item (creator's collection) + App->>BE: Purchase request + BE->>Pay: Charge fiat + Pay-->>BE: Payment cleared + BE->>CL: mint(buyerAddress, uri) + Note over CL: backend holds MINTER_ROLE if creator allowed it + CL-->>BE: emit Transfer(0x0, buyer, tokenId) + BE-->>App: Mark order completed + App-->>B: Item delivered +``` + +The operator's `MINTER_ROLE` on each collection is established at creation time (via `additionalMinters`). Creators can revoke this role at any time, in which case operator-driven mints into that collection will revert until the role is re-granted. + +
+ +## 6. Storage Layout + +### 6.1 Factory Storage + +``` +[OZ AccessControlUpgradeable storage] +[OZ UUPSUpgradeable storage] +slot N+0 : erc721Implementation (address) +slot N+1 : erc1155Implementation (address) +slot N+2 : collectionByExternalId (mapping bytes32 → address) +slot N+3 : __gap[47] (reserved for future fields) +``` + +Actual slot indices are determined by the inheritance chain. Storage layout is verified **manually** against the previous release before every factory upgrade — see §9.4 for the pre-upgrade checklist. + +### 6.2 Per-Collection Storage + +Each collection owns its full storage independently (`ERC1967Proxy` `delegatecall`s logic at the address in the EIP-1967 implementation slot but persists state in the proxy's own address). + +``` +[OZ ERC721Upgradeable / ERC1155Upgradeable storage] +[OZ ERC721URIStorageUpgradeable storage (721 only)] +[OZ ERC1155SupplyUpgradeable storage (1155 only)] +[OZ ERC2981Upgradeable storage] +[OZ AccessControlUpgradeable storage] +slot M+0 : contractURI (string) +slot M+1 : nextTokenId (uint256, 721 only — omitted on 1155, gap shifts up by one) +slot M+2 : metadataLocked (bool, byte 0) | royaltiesLocked (bool, byte 1) | 30 bytes free +slot M+3 : __gap[N] (reserved) +``` + +The two lock booleans share one slot via Solidity's automatic packing (declared adjacent in source order after the `string`/`uint256` fields above). This saves one slot of `__gap`, which materially extends the headroom for future appended fields when admin swaps the implementation pointer for *future* collections. + +`__gap` is a defensive reservation. Per-collection proxies are immutable per release, but if admin swaps the implementation pointer for *future* collections, the gap allows the new implementation to extend storage without conflict for those future collections. + +### 6.3 Storage-Layout Discipline + +All upgradeable contracts in this package follow the same conventions used by `src/swarms/`: + +- No state variables in inherited contracts shifted between releases. +- New variables appended only; gap reduced by the number of new slots. +- For packed slots (e.g. the `metadataLocked` / `royaltiesLocked` slot in §6.2), the manual diff must verify both the slot index **and** the byte offset of each sub-word field — Solidity's layout is sensitive to source order within a packed slot, and a reordering moves bytes without moving slots. +- Before each release that ships a factory upgrade, the engineer running the upgrade snapshots the previous and new layouts via `forge inspect ... storageLayout` and manually verifies that all prior slots remain at the same offsets. The full pre-upgrade checklist lives in §9.4. + +
+ +## 7. Security Model + +### 7.1 Trust Assumptions + +| Principal | Trusted to | Compromise impact | +| :-------------------- | :---------------------------------------------------------------------------- | :-------------------------------------------------------------------------------------- | +| Backend operator key | Trigger creation only after fiat payment clears; not mint maliciously | Free collections; mass minting into any collection where operator holds `MINTER_ROLE` | +| Factory admin (Safe) | Upgrade factory benignly; rotate operator role responsibly | Affects *future* creations only; already-deployed collections are immutable | +| Collection creators | Trusted by their own buyers (not by the platform) | Creator-side rugs (metadata / royalty) mitigated by opt-in lock flags | + +### 7.2 Risks & Mitigations + +| # | Risk | Mitigation | +| :-- | :---------------------------------------------------------------- | :-------------------------------------------------------------------------------------------------- | +| 1 | Operator key compromise → free collections / mass mint | HSM/KMS storage; role rotation via `grantRole`/`revokeRole`; off-chain monitoring on creation rate | +| 2 | External-ID replay (double-creation) | On-chain `collectionByExternalId` map; revert on reuse | +| 3 | Implementation contract initialized directly | `_disableInitializers()` in implementation constructor | +| 4 | Initialization front-run on a fresh collection | Atomic deploy+init inside the proxy constructor; collection never observable uninitialized | +| 5 | Storage-layout corruption on factory upgrade | `__gap` reserved slots; manual pre-upgrade `forge inspect storageLayout` diff against previous release (§9.4) | +| 6 | Royalty rug (creator sets 100% post-mint) | `royaltiesLocked` opt-in; `RoyaltiesLocked` event indexed for buyer due-diligence | +| 7 | Metadata rug (creator changes baseURI mid-mint) | `metadataLocked` opt-in; per-token URIs anchored via `ERC721URIStorage`, stable post-mint | +| 8 | Reentrancy on `_safeMint` callback | OZ default ordering: state writes precede `onERC721Received`; no post-callback reads in our code | +| 9 | DoS via huge `mintBatch` arrays | `MAX_BATCH = 100`; revert `BatchTooLarge` if exceeded | +| 10 | UUPS bricking via mis-set `_authorizeUpgrade` | Standard OZ pattern; unit test asserts non-admin cannot upgrade | +| 11 | Operator granted to wrong address at init | `initialize` requires explicit `operator` arg, not `msg.sender` | +| 12 | Creator revokes operator's `MINTER_ROLE` mid-flow | Operator-driven mints revert cleanly; backend surfaces error to operations | +| 13 | Operator key rotation leaves old collections without the new operator | Auto-grant only applies to *future* collections; runbook step requires creators to grant `MINTER_ROLE` to the new key for continued operator-driven sales on pre-rotation collections | +| 14 | Admin sets factory implementation pointer to zero / EOA / non-contract | `setImplementation*` and `initialize` reject zero addresses (`ZeroAddress`) and addresses with no bytecode (`NotAContract`); wrong-standard pointers caught by the §9.4 post-upgrade `cast` checks | +| 15a | Implementation bytecode permanence | Implementations deployed via `CREATE` only (sequential nonce, never `CREATE2`); no `SELFDESTRUCT` in own/inherited code; no `delegatecall` to caller-provided addresses; verified by opcode-walker test | +| 15b | Per-collection proxy permanence | Deployed via `CREATE2` with `externalId` salt using canonical OZ `ERC1967Proxy` unmodified; impls do not inherit `UUPSUpgradeable`; no `ProxyAdmin` pattern; therefore the EIP-1967 impl slot is constructor-fixed and the proxy bytecode is permanent. Verified by: (i) lockfile-pinned OZ import, (ii) opcode-walker test on `ERC1967Proxy` runtime, (iii) unit test asserting impls have no `upgradeTo*` selectors | +| 16 | Audit posture for OZ proxy import | Audit must verify `import {ERC1967Proxy} from '@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol'` resolves to canonical OZ at the lockfile-pinned version; remappings or forks of OZ proxy contracts are out of band and would invalidate the bytecode-permanence proof | + +### 7.3 Out of Scope + +- Royalty enforcement on secondary markets — ERC-2981 is informational, on-chain enforcement is widely abandoned and breaks marketplace compatibility. +- Content moderation, KYC, and tax compliance — handled off-chain. +- Front-end phishing or wallet UX — out of scope for the on-chain layer. + +### 7.4 Audit Posture + +- New contracts inherit only OZ-audited `*Upgradeable` primitives. +- Custom code surface is small: factory glue, lock flags, role wiring, batch caps. +- Recommended: focused audit on `CollectionFactory.createCollection*` and both `initialize` flows before mainnet deployment. + +
+ +## 8. Testing Strategy + +Unit tests live under `test/collections/`, one file per contract plus an integration test: + +### 8.1 `CollectionFactory.t.sol` + +- `initialize` succeeds once; second call reverts `InvalidInitialization`. +- `initialize` reverts on zero addresses. +- Only `OPERATOR_ROLE` can call `createCollection*`. +- Atomic deploy + initialize → resulting collection has expected name/symbol, owner, base URI, contract URI, royalties, minters. +- After `createCollection*`, the calling operator (`msg.sender`) holds `MINTER_ROLE` on the new collection even when `additionalMinters` is empty (auto-grant invariant, §2.3). +- After admin rotates `OPERATOR_ROLE` to a new address, a collection created by the new operator auto-grants `MINTER_ROLE` to the new key; collections created by the previous operator are unaffected. +- `externalId == bytes32(0)` reverts `InvalidExternalId`. +- Reused `externalId` reverts `ExternalIdAlreadyUsed`. +- `collectionByExternalId(externalId)` returns the collection address after success. +- `setImplementation*` callable only by admin; affects future collections only (existing collections unchanged when verified by `EXTCODEHASH` or behavior probe). +- `setImplementation*` reverts `ZeroAddress` when called with `address(0)`. +- `setImplementation*` reverts `NotAContract` when called with an address that has no bytecode (e.g. an EOA). +- `initialize` reverts `ZeroAddress` for any zero arg and `NotAContract` for either implementation address with empty code. +- UUPS upgrade succeeds for admin and changes the EIP-1967 implementation slot to the new address (read the slot pre/post via `vm.load(proxy, bytes32(uint256(keccak256("eip1967.proxy.implementation")) - 1))`). +- UUPS upgrade reverts when called by an account holding `OPERATOR_ROLE` only (no role-escalation path from operator → admin) and reverts when called by a fresh EOA with no roles. Both assertions exercise OZ's `AccessControlUnauthorizedAccount` revert. +- UUPS upgrade preserves storage: a `CollectionFactoryV2Mock` (test-only fixture) is deployed; pre-upgrade state — admin/operator role grants, `erc721Implementation` / `erc1155Implementation` pointers, and at least one `collectionByExternalId` entry seeded by a prior `createCollection*` — must read correctly through the upgraded proxy. The mock adds a new public function whose presence post-upgrade is also asserted, confirming the upgrade path is genuinely exercised. +- UUPS upgrade to a non-UUPS implementation reverts via OZ's `proxiableUUID` check (`ERC1967InvalidImplementation`). Test deploys a plain `ERC721Upgradeable`-based contract that does not inherit `UUPSUpgradeable` and asserts the revert. +- `CollectionCreated` event fields are correct. + +### 8.2 `UserCollection721.t.sol` + +- Initialize sets all fields and roles correctly, including `MINTER_ROLE` for `operatorMinter`. +- `initialize` reverts with `ZeroAddress` when called with `operatorMinter == address(0)`. +- `_disableInitializers` blocks direct initialization on the implementation contract. +- `mint` requires `MINTER_ROLE`; emits `Transfer`; sets correct `tokenURI`; increments `nextTokenId`. +- `mintBatch` length-mismatch and oversize-batch reverts. +- `mintBatch` returns the array of newly-minted token IDs; values match the `Transfer` events emitted in-order and form a contiguous range starting at the pre-call `nextTokenId`. +- `setBaseURI` / `setContractURI` / `setDefaultRoyalty` permission and lock semantics. +- `lockMetadata` / `lockRoyalties` are one-way; subsequent setters revert with the corresponding error. +- Owner can grant and revoke `MINTER_ROLE` (verifies `_setRoleAdmin(MINTER_ROLE, OWNER_ROLE)`). +- ERC-2981 returns expected royalty info. +- `supportsInterface` returns true for ERC-721, ERC-721 metadata, ERC-2981, ERC-165, AccessControl. +- **Bytecode permanence**: scan `address(impl721).code` and assert no byte equals `0xff` (`SELFDESTRUCT` opcode) at any reachable position. Foundry's `bytecode_hash = "none"` setting (already pinned in `foundry.toml`, see §9.1) strips the metadata trailer that would otherwise produce false positives. Same scan asserts no `0xf4` (`DELEGATECALL`) appears in our own logic ranges; OZ inherited code contains none in the imported set. + +### 8.3 `UserCollection1155.t.sol` + +Analogous coverage adapted to ERC-1155 mechanics: per-ID supply tracking, single-recipient `mintBatch`, `setURI` lock semantics, ERC-1155 interface assertions. Includes the same **bytecode-permanence scan** as §8.2 against the 1155 implementation's runtime code. + +### 8.4 `Collections.integration.t.sol` + +End-to-end happy path: + +1. Admin deploys factory + both implementations via UUPS proxy. +2. Operator creates an ERC-721 collection for creator α. +3. Operator creates an ERC-1155 collection for creator β. +4. Operator mints into both on behalf of fiat buyers. +5. Creator α transfers an item to a third party. +6. Creator α locks metadata and royalties. +7. Subsequent setter calls revert with lock errors. +8. Admin upgrades the factory and ships a new ERC-721 implementation. +9. New ERC-721 collection deploys with new implementation; old collections remain on the previous implementation (verified via `EXTCODEHASH`). + +### 8.5 Coverage Target + +≥ 95% line coverage on the new contracts, enforced by CI. + +**What CI does** (`.github/workflows/checks.yml`): + +- The `Tests` job runs `yarn spellcheck`, `yarn lint`, and `forge test` on every push and PR. Tests under `test/collections/` are picked up automatically by `forge test`. +- The `Coverage` job runs `forge coverage` on PRs to `main`. The `--match-path` filter includes `test/collections/*` alongside the existing swarms test files; a dedicated **"Check line coverage threshold (collections)"** step parses the lcov report for `src/collections/` and fails the build if line coverage falls below 95%. The swarms gate continues to enforce the same threshold against `src/swarms/`. +- While `src/collections/` is documentation-only (this PR), the collections threshold step skips cleanly with a GitHub Actions warning and a non-failing exit. The gate begins enforcing the moment Solidity sources land under `src/collections/`. No follow-up workflow change is required. + +
+ +## 9. Deployment & Operations + +### 9.1 Deployment Script + +Two artifacts, mirroring the swarms pattern: + +- `script/DeployCollectionFactoryZkSync.s.sol` — Forge script that performs the on-chain deployment work. +- `ops/deploy_collection_factory_zksync.sh` — orchestration shell script analogous to `ops/deploy_swarm_contracts_zksync.sh`: runs preflight checks, compiles with `forge build --zksync`, calls the Forge script, performs post-deploy `cast` checks, and invokes `ops/verify_zksync_contracts.py` for source-code verification on the zkSync explorer. + +Environment variables (prefixed `N_` per repo convention; consumed by the Forge script): + +| Variable | Description | +| :------------------- | :------------------------------------------------------------- | +| `N_FACTORY_ADMIN` | Multisig address that will hold `DEFAULT_ADMIN_ROLE` | +| `N_FACTORY_OPERATOR` | Backend service address that will hold `OPERATOR_ROLE` | + +Steps performed by the Forge script: + +1. Deploy `UserCollection721` implementation via `CREATE` (sequential nonce, **never** `CREATE2`). Constructor calls `_disableInitializers()`. Deploying via `CREATE` ensures that even if the Cancun `selfdestruct` semantics on zkSync Era are looser than on L1, the implementation address can never be re-occupied with different bytecode via salt collision (see §7.2 row 15). +2. Deploy `UserCollection1155` implementation. Same constraints (CREATE-only, `_disableInitializers()`). +3. Deploy `CollectionFactory` logic. +4. Deploy `ERC1967Proxy` pointing at `CollectionFactory`, with init data calling `initialize(N_FACTORY_ADMIN, N_FACTORY_OPERATOR, impl721, impl1155)`. Note: this `ERC1967Proxy(factoryImpl, ...)` is the *factory's own* proxy. The per-collection `ERC1967Proxy` instances are deployed by the factory itself at `createCollection*` time, not by this script. +5. Log all four addresses (implementation 721, implementation 1155, factory logic, factory proxy) in the same `: 0x...` format that the orchestration script greps for. + +Steps performed by the orchestration shell script (after the Forge script broadcasts): + +6. Sanity-check the deployment with `cast` (admin role granted, operator role granted, both implementation pointers set, UUPS implementation slot points at the factory logic). +7. Verify source code on the zkSync block explorer via `python3 ops/verify_zksync_contracts.py --broadcast broadcast/DeployCollectionFactoryZkSync.s.sol//run-latest.json --verifier-url $VERIFIER_URL --compiler-version 0.8.26 --zksolc-version v1.5.15 --project-root "$PROJECT_ROOT"`. Verifier URLs follow the swarms convention: + - **Mainnet**: `https://zksync2-mainnet-explorer.zksync.io/contract_verification` (explorer at `https://explorer.zksync.io`) + - **Testnet**: `https://explorer.sepolia.era.zksync.dev/contract_verification` (explorer at `https://sepolia.explorer.zksync.io`) +8. Append the deployed addresses to the appropriate `.env-test` / `.env-prod` file (same pattern as the swarms script's `update_env_file` step). +9. Add a usage example to `README.md` under the existing deployment section. + +> **Note on tooling.** The repo's `README.md` still mentions Etherscan as the verification target; that wording predates the zkSync-era flow. The actual operational pattern (used by `ops/deploy_swarm_contracts_zksync.sh`) is: do **not** use `forge script --verify` (it sends absolute paths the zkSync verifier rejects) and do **not** rely on `forge verify-contract` directly (it sends `../` traversal imports the zkSync verifier rejects). Use `ops/verify_zksync_contracts.py`, which generates standard JSON via Forge, rewrites relative imports to project-rooted paths, and submits to the zkSync verifier API. With `bytecode_hash = "none"` already set in `foundry.toml`, this achieves full verification. + +### 9.2 Indexing + +Subquery (`subquery/` package) extension required: + +- **Top-level source**: factory address. Handler on `CollectionCreated` writes a `Collection` entity and dynamically registers the new collection address as a `Transfer`-listening source (subquery dynamic-source pattern). +- **Per-collection handlers**: `Transfer` writes `Token` and `Owner` entities scoped by collection. Lock events (`MetadataLocked`, `RoyaltiesLocked`) update the corresponding `Collection` entity flags for buyer due-diligence. + +Indexer wiring is out of this repo's contract scope but is referenced here so the implementation plan can include a tracking task for the subquery package. + +### 9.3 Paymaster Integration + +The operator account uses the existing `BondTreasuryPaymaster` for L2 gas. The operator must be seeded with bond allowance before first creation; subsequent adjustments use the paymaster's standard admin path. No new paymaster contract is required for this feature. + +### 9.4 Upgrade & Rollback + +| Operation | Procedure | +| :----------------------------------- | :------------------------------------------------------------------------------------------------------------ | +| Upgrade factory logic | Run pre-upgrade checklist (below), then admin calls `factory.upgradeTo(newImpl)` (UUPS) | +| Ship a new ERC-721 template | Deploy new implementation; admin calls `setImplementation721(newImpl)`; affects *future* collections only | +| Ship a new ERC-1155 template | Deploy new implementation; admin calls `setImplementation1155(newImpl)`; affects *future* collections only | +| Rotate operator key | Admin calls `revokeRole(OPERATOR_ROLE, oldKey)` then `grantRole(OPERATOR_ROLE, newKey)` | +| Pause new creations | Admin revokes all addresses from `OPERATOR_ROLE`. Existing creations unaffected; new requests revert | +| Rollback a faulty template | Admin calls `setImplementation*` pointing back to the previous implementation; affects future collections only| + +There is no rollback path for already-deployed collections — that is the explicit immutability guarantee. Bug fixes that require touching deployed collections must take the form of off-chain workarounds or new collections. + +#### Pre-Upgrade Checklist (factory only) + +CI does not currently diff storage layouts. Before any factory upgrade is broadcast, the engineer running the upgrade must execute the following manually, mirroring the procedure used by `src/swarms/` (see `src/swarms/doc/upgradeable-contracts.md`): + +1. **Verify storage compatibility:** + + ```bash + forge inspect CollectionFactory storageLayout > v1-layout.json + forge inspect CollectionFactoryV2 storageLayout > v2-layout.json + # Manually compare: ensure every V1 storage slot is preserved at the same slot index AND byte + # offset in V2 (byte offsets matter for sub-word fields packed into the same slot, e.g. the + # bool flags in per-collection storage; see §6.3). Only appended fields (consuming __gap slots) are + # acceptable. + ``` + + **Baseline-JSON convention.** Before merging the V2 implementation, commit + `src/collections/layouts/CollectionFactory.v1.json` (snapshot of the current main + branch's layout) and `src/collections/layouts/CollectionFactory.v2.json` (snapshot of the + incoming V2) in the same PR. The git diff between baselines is reviewed as part of the + PR — any unexpected slot/offset shift is caught at code-review time without needing a + dedicated CI job. Same convention applies to `UserCollection721` and `UserCollection1155` + when the admin ships a new implementation pointer via `setImplementation*`. + +2. **Run all tests:** + + ```bash + forge test --match-path "test/collections/**" + ``` + +3. **Test on a fork:** + + ```bash + forge script script/UpgradeCollectionFactory.s.sol \ + --fork-url $RPC_URL \ + --sender $ADMIN + ``` + +4. **Post-upgrade verification (after broadcast):** + + ```bash + # Implementation pointer changed + cast implementation $FACTORY_PROXY --rpc-url $RPC_URL + + # Admin role unchanged + cast call $FACTORY_PROXY "hasRole(bytes32,address)(bool)" \ + $(cast keccak "DEFAULT_ADMIN_ROLE") $ADMIN --rpc-url $RPC_URL + + # Stored implementation pointers unchanged (existing collections unaffected) + cast call $FACTORY_PROXY "erc721Implementation()(address)" --rpc-url $RPC_URL + cast call $FACTORY_PROXY "erc1155Implementation()(address)" --rpc-url $RPC_URL + ``` + + Per-collection EIP-1967 implementation slot check (run after each `createCollection*` to + confirm the proxy points at the expected implementation): + + ```bash + EIP1967_IMPL_SLOT=0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc + cast storage "$COLLECTION_ADDR" "$EIP1967_IMPL_SLOT" --rpc-url "$L2_RPC" + # expected: padded address of UserCollection721 (or 1155) impl + ``` + +Promoting any of these checks into a CI job is tracked under §11. + +
+ +## 10. File Layout + +``` +src/collections/ + CollectionFactory.sol + UserCollection721.sol + UserCollection1155.sol + interfaces/ + ICollectionFactory.sol + IUserCollection721.sol + IUserCollection1155.sol + CollectionTypes.sol + doc/ + README.md + spec/ + user-collections-specification.md +test/collections/ + CollectionFactory.t.sol + UserCollection721.t.sol + UserCollection1155.t.sol + Collections.integration.t.sol + mocks/ + CollectionFactoryV2Mock.sol (test-only; UUPS upgrade-target fixture, see §8.1) + NonUUPSImplementationMock.sol (test-only; non-UUPS contract for proxiableUUID revert test) +src/collections/layouts/ + CollectionFactory.v1.json (storage-layout baseline; committed in upgrade PRs, see §9.4) + UserCollection721.v1.json (collection-side baseline; updated when admin ships a new ERC-721 implementation) + UserCollection1155.v1.json (collection-side baseline; updated when admin ships a new ERC-1155 implementation) +script/ + DeployCollectionFactoryZkSync.s.sol + UpgradeCollectionFactory.s.sol +ops/ + deploy_collection_factory_zksync.sh (mirrors deploy_swarm_contracts_zksync.sh) +``` + +License header on every Solidity file: `// SPDX-License-Identifier: BSD-3-Clause-Clear`. + +Solidity pragma: `^0.8.26` (matches existing contracts). + +
+ +## 11. Open Considerations + +These are not blocking for v1; recorded for future iteration. + +| Item | Status | Notes | +| :----------------------------------------- | :------- | :---------------------------------------------------------------------------------------------------------------------------- | +| Deterministic collection addresses | Done | `new ERC1967Proxy{salt: externalId}(...)` already gives deterministic per-collection addresses; backends pre-derive via `zksync-ethers` `utils.create2Address` (see §4.5) | +| Voucher-style non-custodial creation | Deferred | Adds an EIP-712 signed-voucher path for power users; can ship as a parallel `createCollectionWithVoucher` without breaking v1 | +| Per-creator on-chain rate limit | Deferred | Backend rate-limits today; can add `mapping(address => uint256) collectionsByCreator` and a configurable cap later | +| Soulbound / non-transferable variant | Deferred | Ship as a third implementation pointer; selected via a new `createCollectionSoulbound*` factory method | +| Per-token-URI mutability after lock | Deferred | If creators ever need to update individual token URIs after locking the collection, would require a `tokenLocked` map | +| CI storage-layout diff job | Deferred — required before factory upgrade | The §9.4 baseline-JSON convention covers v1 (no upgrade has shipped yet, so there is nothing for a CI diff to gate against). Trigger to wire the CI job: opening the PR for `CollectionFactoryV2`. The job snapshots `forge inspect storageLayout` per upgradeable contract and fails on slot/offset mutations against the committed baseline JSONs. | +| Multi-recipient ERC-1155 mint batch | Deferred | v1 keeps OZ's single-recipient `_mintBatch` shape (see §3.6). Trigger to add `mintBatchMulti`: airdrops or allowlist drops on the product roadmap. Ships as a non-breaking addition via a new implementation pointer (admin swap, future collections only) | diff --git a/src/collections/interfaces/CollectionTypes.sol b/src/collections/interfaces/CollectionTypes.sol new file mode 100644 index 00000000..8e093284 --- /dev/null +++ b/src/collections/interfaces/CollectionTypes.sol @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity ^0.8.26; + +/** + * @title CollectionTypes + * @notice Shared enums and structs for the User Collections system. + * @dev Solidity interfaces cannot define enums, so shared types live here. + * Import this file alongside the collection interfaces. + */ + +/// @notice Token standard selected per-collection at creation time. +enum Standard { + ERC721, + ERC1155 +} + +/// @notice Parameters supplied by the operator when creating an ERC-721 collection. +/// @dev `additionalMinters` is orthogonal to the operator auto-grant: the calling +/// operator (`msg.sender` on the factory) is auto-granted `MINTER_ROLE` by the +/// collection's `initialize` regardless of this list. Use `additionalMinters` +/// for creator-seeded extras (e.g. a co-creator wallet). +struct CreateParams721 { + address owner; + string name; + string symbol; + string baseURI; + string contractURI; + address royaltyRecipient; + uint96 royaltyBps; + address[] additionalMinters; +} + +/// @notice Parameters supplied by the operator when creating an ERC-1155 collection. +/// @dev ERC-1155 has no on-chain `name`/`symbol` convention; the collection display +/// name lives in `contractURI` JSON metadata. +struct CreateParams1155 { + address owner; + string uri; + string contractURI; + address royaltyRecipient; + uint96 royaltyBps; + address[] additionalMinters; +} diff --git a/src/collections/interfaces/ICollectionFactory.sol b/src/collections/interfaces/ICollectionFactory.sol new file mode 100644 index 00000000..cc11f590 --- /dev/null +++ b/src/collections/interfaces/ICollectionFactory.sol @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity ^0.8.26; + +import {Standard, CreateParams721, CreateParams1155} from "./CollectionTypes.sol"; + +/** + * @title ICollectionFactory + * @notice Public API for the operator-triggered NFT collection factory. + * @dev See `src/collections/doc/spec/user-collections-specification.md` for the + * full architectural specification. + */ +interface ICollectionFactory { + // ────────────────────────────────────────────── + // Events + // ────────────────────────────────────────────── + + /// @notice Emitted when a new per-collection `ERC1967Proxy` is deployed and initialized. + /// @param creator The address that received `OWNER_ROLE` on the new collection. + /// @param collection The address of the newly-deployed per-collection proxy. + /// @param standard The token standard (ERC721 or ERC1155). + /// @param externalId The off-chain reconciliation identifier supplied by the operator. + event CollectionCreated( + address indexed creator, + address indexed collection, + Standard standard, + bytes32 indexed externalId + ); + + /// @notice Emitted when admin updates an implementation pointer for future per-collection proxies. + event ImplementationUpdated(Standard standard, address newImpl); + + // ────────────────────────────────────────────── + // Errors + // ────────────────────────────────────────────── + + /// @notice Thrown when an `externalId` has already been consumed by a prior creation. + error ExternalIdAlreadyUsed(bytes32 externalId); + + /// @notice Thrown when `externalId == bytes32(0)`. + error InvalidExternalId(); + + /// @notice Thrown when a required address argument is the zero address. + error ZeroAddress(); + + /// @notice Thrown when an implementation argument has no contract bytecode. + error NotAContract(address impl); + + // ────────────────────────────────────────────── + // Initialization + // ────────────────────────────────────────────── + + function initialize( + address admin, + address operator, + address impl721, + address impl1155 + ) external; + + // ────────────────────────────────────────────── + // Creation + // ────────────────────────────────────────────── + + function createCollection721(CreateParams721 calldata p, bytes32 externalId) + external + returns (address collection); + + function createCollection1155(CreateParams1155 calldata p, bytes32 externalId) + external + returns (address collection); + + // ────────────────────────────────────────────── + // Admin + // ────────────────────────────────────────────── + + function setImplementation721(address impl) external; + + function setImplementation1155(address impl) external; + + // ────────────────────────────────────────────── + // Views + // ────────────────────────────────────────────── + + function collectionByExternalId(bytes32 externalId) external view returns (address); + + function erc721Implementation() external view returns (address); + + function erc1155Implementation() external view returns (address); +} diff --git a/src/collections/interfaces/IUserCollection1155.sol b/src/collections/interfaces/IUserCollection1155.sol new file mode 100644 index 00000000..615bc246 --- /dev/null +++ b/src/collections/interfaces/IUserCollection1155.sol @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity ^0.8.26; + +import {CreateParams1155} from "./CollectionTypes.sol"; + +/** + * @title IUserCollection1155 + * @notice Public API for the ERC-1155 implementation deployed behind a per-collection `ERC1967Proxy`. + * @dev See `src/collections/doc/spec/user-collections-specification.md` (§3.6). + */ +interface IUserCollection1155 { + // ────────────────────────────────────────────── + // Events + // ────────────────────────────────────────────── + + event MetadataLocked(); + event RoyaltiesLocked(); + event ContractURIUpdated(string newURI); + event URIUpdated(string newURI); + + // ────────────────────────────────────────────── + // Errors + // ────────────────────────────────────────────── + + error MetadataIsLocked(); + error RoyaltiesAreLocked(); + error BatchTooLarge(uint256 length, uint256 max); + error LengthMismatch(); + error ZeroAddress(); + + // ────────────────────────────────────────────── + // Initialization + // ────────────────────────────────────────────── + + function initialize(CreateParams1155 calldata p, address operatorMinter) external; + + // ────────────────────────────────────────────── + // Minting + // ────────────────────────────────────────────── + + function mint(address to, uint256 id, uint256 amount, bytes calldata data) external; + + function mintBatch( + address to, + uint256[] calldata ids, + uint256[] calldata amounts, + bytes calldata data + ) external; + + // ────────────────────────────────────────────── + // Owner-mutable settings + // ────────────────────────────────────────────── + + function setURI(string calldata newURI) external; + + function setContractURI(string calldata newURI) external; + + function setDefaultRoyalty(address recipient, uint96 bps) external; + + function lockMetadata() external; + + function lockRoyalties() external; + + // ────────────────────────────────────────────── + // Views + // ────────────────────────────────────────────── + + function contractURI() external view returns (string memory); + + function metadataLocked() external view returns (bool); + + function royaltiesLocked() external view returns (bool); +} diff --git a/src/collections/interfaces/IUserCollection721.sol b/src/collections/interfaces/IUserCollection721.sol new file mode 100644 index 00000000..fb959993 --- /dev/null +++ b/src/collections/interfaces/IUserCollection721.sol @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity ^0.8.26; + +import {CreateParams721} from "./CollectionTypes.sol"; + +/** + * @title IUserCollection721 + * @notice Public API for the ERC-721 implementation deployed behind a per-collection `ERC1967Proxy`. + * @dev See `src/collections/doc/spec/user-collections-specification.md` (§3.5). + */ +interface IUserCollection721 { + // ────────────────────────────────────────────── + // Events + // ────────────────────────────────────────────── + + event MetadataLocked(); + event RoyaltiesLocked(); + event ContractURIUpdated(string newURI); + event BaseURIUpdated(string newBase); + + // ────────────────────────────────────────────── + // Errors + // ────────────────────────────────────────────── + + error MetadataIsLocked(); + error RoyaltiesAreLocked(); + error BatchTooLarge(uint256 length, uint256 max); + error LengthMismatch(); + error ZeroAddress(); + + // ────────────────────────────────────────────── + // Initialization + // ────────────────────────────────────────────── + + function initialize(CreateParams721 calldata p, address operatorMinter) external; + + // ────────────────────────────────────────────── + // Minting + // ────────────────────────────────────────────── + + function mint(address to, string calldata tokenURI_) external returns (uint256 tokenId); + + function mintBatch(address[] calldata to, string[] calldata uris) + external + returns (uint256[] memory tokenIds); + + // ────────────────────────────────────────────── + // Owner-mutable settings + // ────────────────────────────────────────────── + + function setBaseURI(string calldata newBase) external; + + function setContractURI(string calldata newURI) external; + + function setDefaultRoyalty(address recipient, uint96 bps) external; + + function lockMetadata() external; + + function lockRoyalties() external; + + // ────────────────────────────────────────────── + // Views + // ────────────────────────────────────────────── + + function contractURI() external view returns (string memory); + + function nextTokenId() external view returns (uint256); + + function metadataLocked() external view returns (bool); + + function royaltiesLocked() external view returns (bool); +} diff --git a/src/collections/layouts/CollectionFactory.v1.json b/src/collections/layouts/CollectionFactory.v1.json new file mode 100644 index 00000000..46d05d3b --- /dev/null +++ b/src/collections/layouts/CollectionFactory.v1.json @@ -0,0 +1,66 @@ +{ + "storage": [ + { + "astId": 2260, + "contract": "src/collections/CollectionFactory.sol:CollectionFactory", + "label": "_erc721Implementation", + "offset": 0, + "slot": "0", + "type": "t_address" + }, + { + "astId": 2262, + "contract": "src/collections/CollectionFactory.sol:CollectionFactory", + "label": "_erc1155Implementation", + "offset": 0, + "slot": "1", + "type": "t_address" + }, + { + "astId": 2266, + "contract": "src/collections/CollectionFactory.sol:CollectionFactory", + "label": "_collectionByExternalId", + "offset": 0, + "slot": "2", + "type": "t_mapping(t_bytes32,t_address)" + }, + { + "astId": 2271, + "contract": "src/collections/CollectionFactory.sol:CollectionFactory", + "label": "__gap", + "offset": 0, + "slot": "3", + "type": "t_array(t_uint256)47_storage" + } + ], + "types": { + "t_address": { + "encoding": "inplace", + "label": "address", + "numberOfBytes": "20" + }, + "t_array(t_uint256)47_storage": { + "encoding": "inplace", + "label": "uint256[47]", + "numberOfBytes": "1504", + "base": "t_uint256" + }, + "t_bytes32": { + "encoding": "inplace", + "label": "bytes32", + "numberOfBytes": "32" + }, + "t_mapping(t_bytes32,t_address)": { + "encoding": "mapping", + "key": "t_bytes32", + "label": "mapping(bytes32 => address)", + "numberOfBytes": "32", + "value": "t_address" + }, + "t_uint256": { + "encoding": "inplace", + "label": "uint256", + "numberOfBytes": "32" + } + } +} diff --git a/src/collections/layouts/UserCollection1155.v1.json b/src/collections/layouts/UserCollection1155.v1.json new file mode 100644 index 00000000..92399f87 --- /dev/null +++ b/src/collections/layouts/UserCollection1155.v1.json @@ -0,0 +1,59 @@ +{ + "storage": [ + { + "astId": 7670, + "contract": "src/collections/UserCollection1155.sol:UserCollection1155", + "label": "_contractURI", + "offset": 0, + "slot": "0", + "type": "t_string_storage" + }, + { + "astId": 7672, + "contract": "src/collections/UserCollection1155.sol:UserCollection1155", + "label": "_metadataLocked", + "offset": 0, + "slot": "1", + "type": "t_bool" + }, + { + "astId": 7674, + "contract": "src/collections/UserCollection1155.sol:UserCollection1155", + "label": "_royaltiesLocked", + "offset": 1, + "slot": "1", + "type": "t_bool" + }, + { + "astId": 7679, + "contract": "src/collections/UserCollection1155.sol:UserCollection1155", + "label": "__gap", + "offset": 0, + "slot": "2", + "type": "t_array(t_uint256)47_storage" + } + ], + "types": { + "t_array(t_uint256)47_storage": { + "encoding": "inplace", + "label": "uint256[47]", + "numberOfBytes": "1504", + "base": "t_uint256" + }, + "t_bool": { + "encoding": "inplace", + "label": "bool", + "numberOfBytes": "1" + }, + "t_string_storage": { + "encoding": "bytes", + "label": "string", + "numberOfBytes": "32" + }, + "t_uint256": { + "encoding": "inplace", + "label": "uint256", + "numberOfBytes": "32" + } + } +} diff --git a/src/collections/layouts/UserCollection721.v1.json b/src/collections/layouts/UserCollection721.v1.json new file mode 100644 index 00000000..3bdc57a1 --- /dev/null +++ b/src/collections/layouts/UserCollection721.v1.json @@ -0,0 +1,75 @@ +{ + "storage": [ + { + "astId": 7933, + "contract": "src/collections/UserCollection721.sol:UserCollection721", + "label": "_baseTokenURI", + "offset": 0, + "slot": "0", + "type": "t_string_storage" + }, + { + "astId": 7935, + "contract": "src/collections/UserCollection721.sol:UserCollection721", + "label": "_contractURI", + "offset": 0, + "slot": "1", + "type": "t_string_storage" + }, + { + "astId": 7937, + "contract": "src/collections/UserCollection721.sol:UserCollection721", + "label": "_nextTokenId", + "offset": 0, + "slot": "2", + "type": "t_uint256" + }, + { + "astId": 7939, + "contract": "src/collections/UserCollection721.sol:UserCollection721", + "label": "_metadataLocked", + "offset": 0, + "slot": "3", + "type": "t_bool" + }, + { + "astId": 7941, + "contract": "src/collections/UserCollection721.sol:UserCollection721", + "label": "_royaltiesLocked", + "offset": 1, + "slot": "3", + "type": "t_bool" + }, + { + "astId": 7946, + "contract": "src/collections/UserCollection721.sol:UserCollection721", + "label": "__gap", + "offset": 0, + "slot": "4", + "type": "t_array(t_uint256)46_storage" + } + ], + "types": { + "t_array(t_uint256)46_storage": { + "encoding": "inplace", + "label": "uint256[46]", + "numberOfBytes": "1472", + "base": "t_uint256" + }, + "t_bool": { + "encoding": "inplace", + "label": "bool", + "numberOfBytes": "1" + }, + "t_string_storage": { + "encoding": "bytes", + "label": "string", + "numberOfBytes": "32" + }, + "t_uint256": { + "encoding": "inplace", + "label": "uint256", + "numberOfBytes": "32" + } + } +} diff --git a/test/collections/CollectionFactory.t.sol b/test/collections/CollectionFactory.t.sol new file mode 100644 index 00000000..d0a3a64d --- /dev/null +++ b/test/collections/CollectionFactory.t.sol @@ -0,0 +1,410 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.26; + +import {Test} from "forge-std/Test.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {ERC1967Utils} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Utils.sol"; +import {Create2} from "@openzeppelin/contracts/utils/Create2.sol"; +import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol"; +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; + +import {CollectionFactory} from "../../src/collections/CollectionFactory.sol"; +import {ICollectionFactory} from "../../src/collections/interfaces/ICollectionFactory.sol"; +import {UserCollection721} from "../../src/collections/UserCollection721.sol"; +import {UserCollection1155} from "../../src/collections/UserCollection1155.sol"; +import {IUserCollection721} from "../../src/collections/interfaces/IUserCollection721.sol"; +import {IUserCollection1155} from "../../src/collections/interfaces/IUserCollection1155.sol"; +import {Standard, CreateParams721, CreateParams1155} from "../../src/collections/interfaces/CollectionTypes.sol"; + +import {CollectionFactoryV2Mock} from "./mocks/CollectionFactoryV2Mock.sol"; +import {NonUUPSImplementationMock} from "./mocks/NonUUPSImplementationMock.sol"; + +contract CollectionFactoryTest is Test { + CollectionFactory internal factory; + UserCollection721 internal impl721; + UserCollection1155 internal impl1155; + + address internal constant ADMIN = address(0xAD); + address internal constant OPERATOR = address(0x09); + address internal constant CREATOR = address(0xCAFE); + address internal constant STRANGER = address(0xDEAD); + address internal constant ALICE = address(0xA1); + + bytes32 internal constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE"); + bytes32 internal constant MINTER_ROLE = keccak256("MINTER_ROLE"); + bytes32 internal constant DEFAULT_ADMIN_ROLE = 0x00; + + event CollectionCreated( + address indexed creator, address indexed collection, Standard standard, bytes32 indexed externalId + ); + event ImplementationUpdated(Standard standard, address newImpl); + event Upgraded(address indexed implementation); + event Initialized(uint64 version); + + function setUp() public { + impl721 = new UserCollection721(); + impl1155 = new UserCollection1155(); + + CollectionFactory logic = new CollectionFactory(); + bytes memory init = abi.encodeCall( + CollectionFactory.initialize, (ADMIN, OPERATOR, address(impl721), address(impl1155)) + ); + ERC1967Proxy proxy = new ERC1967Proxy(address(logic), init); + factory = CollectionFactory(address(proxy)); + } + + // ────────────────────────────────────────────── + // Helpers + // ────────────────────────────────────────────── + + function _params721(address owner) internal pure returns (CreateParams721 memory) { + return CreateParams721({ + owner: owner, + name: "C", + symbol: "C", + baseURI: "ipfs://b/", + contractURI: "ipfs://c.json", + royaltyRecipient: owner, + royaltyBps: 500, + additionalMinters: new address[](0) + }); + } + + function _params1155(address owner) internal pure returns (CreateParams1155 memory) { + return CreateParams1155({ + owner: owner, + uri: "ipfs://1155/{id}.json", + contractURI: "ipfs://c.json", + royaltyRecipient: owner, + royaltyBps: 500, + additionalMinters: new address[](0) + }); + } + + // ────────────────────────────────────────────── + // Initialization + // ────────────────────────────────────────────── + + function test_initialize_grantsRolesAndSetsImplementations() public view { + assertTrue(factory.hasRole(DEFAULT_ADMIN_ROLE, ADMIN)); + assertTrue(factory.hasRole(OPERATOR_ROLE, OPERATOR)); + assertEq(factory.erc721Implementation(), address(impl721)); + assertEq(factory.erc1155Implementation(), address(impl1155)); + } + + function test_initialize_revertsOnSecondCall() public { + vm.expectRevert(Initializable.InvalidInitialization.selector); + factory.initialize(ADMIN, OPERATOR, address(impl721), address(impl1155)); + } + + function test_initialize_revertsOnZeroAddresses() public { + CollectionFactory logic = new CollectionFactory(); + bytes memory bad = abi.encodeCall( + CollectionFactory.initialize, (address(0), OPERATOR, address(impl721), address(impl1155)) + ); + vm.expectRevert(); + new ERC1967Proxy(address(logic), bad); + } + + function test_initialize_revertsOnNonContractImpl() public { + CollectionFactory logic = new CollectionFactory(); + bytes memory bad = abi.encodeCall( + CollectionFactory.initialize, (ADMIN, OPERATOR, address(0xBEEF), address(impl1155)) + ); + vm.expectRevert(); + new ERC1967Proxy(address(logic), bad); + } + + // ────────────────────────────────────────────── + // Creation + // ────────────────────────────────────────────── + + function test_createCollection721_atomicAndEmits() public { + bytes32 externalId = keccak256("order-1"); + + // Order: Upgraded(impl) → Initialized(1) → ... role grants ... → CollectionCreated + vm.expectEmit(true, false, false, false); + emit Upgraded(address(impl721)); + + vm.expectEmit(false, false, false, true); + emit Initialized(1); + + // CollectionCreated indexed topics: (creator, collection, externalId). + // We don't know the collection address up front, so leave its topic unchecked. + vm.expectEmit(true, false, true, true); + emit CollectionCreated(CREATOR, address(0), Standard.ERC721, externalId); + + vm.prank(OPERATOR); + address collection = factory.createCollection721(_params721(CREATOR), externalId); + + assertEq(factory.collectionByExternalId(externalId), collection); + UserCollection721 c = UserCollection721(collection); + assertEq(c.name(), "C"); + assertEq(c.contractURI(), "ipfs://c.json"); + assertTrue(c.hasRole(keccak256("OWNER_ROLE"), CREATOR)); + // Operator auto-grant invariant — see §2.3. + assertTrue(c.hasRole(MINTER_ROLE, OPERATOR)); + } + + function test_createCollection721_addressMatchesCreate2Derivation() public { + bytes32 externalId = keccak256("derivation-test-721"); + CreateParams721 memory p = _params721(CREATOR); + + bytes memory initData = abi.encodeCall( + IUserCollection721.initialize, + (p, OPERATOR) + ); + + bytes32 initCodeHash = keccak256( + abi.encodePacked( + type(ERC1967Proxy).creationCode, + abi.encode(address(impl721), initData) + ) + ); + + address predicted = Create2.computeAddress( + externalId, + initCodeHash, + address(factory) + ); + + vm.prank(OPERATOR); + address actual = factory.createCollection721(p, externalId); + + assertEq(actual, predicted, "deployed address must match CREATE2 derivation"); + } + + function test_createCollection1155_addressMatchesCreate2Derivation() public { + bytes32 externalId = keccak256("derivation-test-1155"); + CreateParams1155 memory p = _params1155(CREATOR); + + bytes memory initData = abi.encodeCall( + IUserCollection1155.initialize, + (p, OPERATOR) + ); + + bytes32 initCodeHash = keccak256( + abi.encodePacked( + type(ERC1967Proxy).creationCode, + abi.encode(address(impl1155), initData) + ) + ); + + address predicted = Create2.computeAddress( + externalId, + initCodeHash, + address(factory) + ); + + vm.prank(OPERATOR); + address actual = factory.createCollection1155(p, externalId); + + assertEq(actual, predicted, "deployed 1155 address must match CREATE2 derivation"); + } + + function test_createCollection1155_atomicAndEmits() public { + bytes32 externalId = keccak256("order-1155"); + + // Order: Upgraded(impl) → Initialized(1) → ... role grants ... → CollectionCreated + vm.expectEmit(true, false, false, false); + emit Upgraded(address(impl1155)); + + vm.expectEmit(false, false, false, true); + emit Initialized(1); + + // CollectionCreated indexed topics: (creator, collection, externalId). + // We don't know the collection address up front, so leave its topic unchecked. + vm.expectEmit(true, false, true, true); + emit CollectionCreated(CREATOR, address(0), Standard.ERC1155, externalId); + + vm.prank(OPERATOR); + address collection = factory.createCollection1155(_params1155(CREATOR), externalId); + + assertEq(factory.collectionByExternalId(externalId), collection); + UserCollection1155 c = UserCollection1155(collection); + assertTrue(c.hasRole(keccak256("OWNER_ROLE"), CREATOR)); + assertTrue(c.hasRole(MINTER_ROLE, OPERATOR)); + } + + function test_createCollection_onlyOperator() public { + vm.prank(STRANGER); + vm.expectRevert( + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, STRANGER, OPERATOR_ROLE) + ); + factory.createCollection721(_params721(CREATOR), keccak256("x")); + } + + function test_createCollection_revertsZeroExternalId() public { + vm.prank(OPERATOR); + vm.expectRevert(ICollectionFactory.InvalidExternalId.selector); + factory.createCollection721(_params721(CREATOR), bytes32(0)); + } + + function test_createCollection_revertsReusedExternalId() public { + bytes32 externalId = keccak256("dup"); + vm.prank(OPERATOR); + factory.createCollection721(_params721(CREATOR), externalId); + + vm.prank(OPERATOR); + vm.expectRevert(abi.encodeWithSelector(ICollectionFactory.ExternalIdAlreadyUsed.selector, externalId)); + factory.createCollection721(_params721(CREATOR), externalId); + } + + function test_createCollection_operatorAutoGrantWithEmptyAdditionalMinters() public { + bytes32 externalId = keccak256("empty-minters"); + vm.prank(OPERATOR); + address collection = factory.createCollection721(_params721(CREATOR), externalId); + // additionalMinters is empty — operator must still be a minter via auto-grant. + assertTrue(UserCollection721(collection).hasRole(MINTER_ROLE, OPERATOR)); + } + + function test_createCollection721_canMintImmediatelyInSameTx() public { + bytes32 externalId = keccak256("immediate-mint-721"); + + vm.startPrank(OPERATOR); + address collection = factory.createCollection721(_params721(CREATOR), externalId); + // Operator was auto-granted MINTER_ROLE during constructor delegatecall — + // can mint without any extra setup transactions. + uint256 tokenId = UserCollection721(collection).mint(ALICE, "ipfs://token-0.json"); + vm.stopPrank(); + + assertEq(UserCollection721(collection).ownerOf(tokenId), ALICE); + } + + // ────────────────────────────────────────────── + // setImplementation* + // ────────────────────────────────────────────── + + function test_setImplementation_onlyAdmin() public { + UserCollection721 newImpl = new UserCollection721(); + vm.prank(STRANGER); + vm.expectRevert( + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, STRANGER, DEFAULT_ADMIN_ROLE) + ); + factory.setImplementation721(address(newImpl)); + } + + function test_setImplementation_revertsZeroAddress() public { + vm.prank(ADMIN); + vm.expectRevert(ICollectionFactory.ZeroAddress.selector); + factory.setImplementation721(address(0)); + } + + function test_setImplementation_revertsNonContract() public { + vm.prank(ADMIN); + vm.expectRevert(abi.encodeWithSelector(ICollectionFactory.NotAContract.selector, address(0xBEEF))); + factory.setImplementation721(address(0xBEEF)); + } + + function test_setImplementation1155_updatesPointerAndEmits() public { + UserCollection1155 newImpl = new UserCollection1155(); + vm.expectEmit(true, true, true, true); + emit ImplementationUpdated(Standard.ERC1155, address(newImpl)); + vm.prank(ADMIN); + factory.setImplementation1155(address(newImpl)); + assertEq(factory.erc1155Implementation(), address(newImpl)); + } + + function test_setImplementation1155_revertsZeroAndNonContract() public { + vm.prank(ADMIN); + vm.expectRevert(ICollectionFactory.ZeroAddress.selector); + factory.setImplementation1155(address(0)); + + vm.prank(ADMIN); + vm.expectRevert(abi.encodeWithSelector(ICollectionFactory.NotAContract.selector, address(0xBEEF))); + factory.setImplementation1155(address(0xBEEF)); + } + + function test_setImplementation_affectsFutureCollectionsOnly() public { + bytes32 firstId = keccak256("first"); + vm.prank(OPERATOR); + address oldCollection = factory.createCollection721(_params721(CREATOR), firstId); + bytes32 oldHash = oldCollection.codehash; + + UserCollection721 newImpl = new UserCollection721(); + vm.expectEmit(true, true, true, true); + emit ImplementationUpdated(Standard.ERC721, address(newImpl)); + vm.prank(ADMIN); + factory.setImplementation721(address(newImpl)); + + bytes32 secondId = keccak256("second"); + vm.prank(OPERATOR); + address newCollection = factory.createCollection721(_params721(CREATOR), secondId); + + // Old collection unchanged; new collection points at the new implementation + // via its ERC1967 proxy. Verify by reading the factory's stored pointer + // post-set. + assertEq(oldCollection.codehash, oldHash); + assertEq(factory.erc721Implementation(), address(newImpl)); + assertTrue(newCollection != oldCollection); + } + + // ────────────────────────────────────────────── + // UUPS upgrade — §8.1 four assertions + // ────────────────────────────────────────────── + + bytes32 internal constant IMPL_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; + + function test_uups_adminUpgradeChangesImplementationSlot() public { + CollectionFactoryV2Mock v2Logic = new CollectionFactoryV2Mock(); + address pre = address(uint160(uint256(vm.load(address(factory), IMPL_SLOT)))); + assertTrue(pre != address(v2Logic)); + + vm.prank(ADMIN); + factory.upgradeToAndCall(address(v2Logic), ""); + + address post = address(uint160(uint256(vm.load(address(factory), IMPL_SLOT)))); + assertEq(post, address(v2Logic)); + } + + function test_uups_revertsForOperatorOnly() public { + CollectionFactoryV2Mock v2Logic = new CollectionFactoryV2Mock(); + // OPERATOR holds OPERATOR_ROLE but NOT DEFAULT_ADMIN_ROLE — must not escalate. + vm.prank(OPERATOR); + vm.expectRevert( + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, OPERATOR, DEFAULT_ADMIN_ROLE) + ); + factory.upgradeToAndCall(address(v2Logic), ""); + } + + function test_uups_revertsForFreshEoa() public { + CollectionFactoryV2Mock v2Logic = new CollectionFactoryV2Mock(); + vm.prank(STRANGER); + vm.expectRevert( + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, STRANGER, DEFAULT_ADMIN_ROLE) + ); + factory.upgradeToAndCall(address(v2Logic), ""); + } + + function test_uups_preservesStorageThroughUpgrade() public { + // Seed pre-upgrade state. + bytes32 externalId = keccak256("pre-upgrade"); + vm.prank(OPERATOR); + address seededCollection = factory.createCollection721(_params721(CREATOR), externalId); + + CollectionFactoryV2Mock v2Logic = new CollectionFactoryV2Mock(); + vm.prank(ADMIN); + factory.upgradeToAndCall(address(v2Logic), ""); + + // Roles preserved. + assertTrue(factory.hasRole(DEFAULT_ADMIN_ROLE, ADMIN)); + assertTrue(factory.hasRole(OPERATOR_ROLE, OPERATOR)); + // Implementation pointers preserved. + assertEq(factory.erc721Implementation(), address(impl721)); + assertEq(factory.erc1155Implementation(), address(impl1155)); + // Pre-upgrade collection mapping preserved. + assertEq(factory.collectionByExternalId(externalId), seededCollection); + // V2-only function callable on the upgraded proxy — proves real delegation. + assertEq(CollectionFactoryV2Mock(address(factory)).v2Sentinel(), 4242); + } + + function test_uups_revertsOnNonUUPSImplementation() public { + NonUUPSImplementationMock nonUups = new NonUUPSImplementationMock(); + vm.prank(ADMIN); + // OZ wraps the failed proxiableUUID call in ERC1967InvalidImplementation. + vm.expectRevert( + abi.encodeWithSelector(ERC1967Utils.ERC1967InvalidImplementation.selector, address(nonUups)) + ); + factory.upgradeToAndCall(address(nonUups), ""); + } +} diff --git a/test/collections/Collections.integration.t.sol b/test/collections/Collections.integration.t.sol new file mode 100644 index 00000000..e40383e2 --- /dev/null +++ b/test/collections/Collections.integration.t.sol @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.26; + +import {Test} from "forge-std/Test.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +import {CollectionFactory} from "../../src/collections/CollectionFactory.sol"; +import {UserCollection721} from "../../src/collections/UserCollection721.sol"; +import {UserCollection1155} from "../../src/collections/UserCollection1155.sol"; +import {IUserCollection721} from "../../src/collections/interfaces/IUserCollection721.sol"; +import {IUserCollection1155} from "../../src/collections/interfaces/IUserCollection1155.sol"; +import {Standard, CreateParams721, CreateParams1155} from "../../src/collections/interfaces/CollectionTypes.sol"; + +import {CollectionFactoryV2Mock} from "./mocks/CollectionFactoryV2Mock.sol"; + +/** + * @title Collections.integration.t.sol + * @notice End-to-end happy-path scenario from spec §8.4. + */ +contract CollectionsIntegrationTest is Test { + CollectionFactory internal factory; + UserCollection721 internal impl721; + UserCollection1155 internal impl1155; + + address internal constant ADMIN = address(0xAD); + address internal constant OPERATOR = address(0x09); + address internal constant CREATOR_ALPHA = address(0xA1); + address internal constant CREATOR_BETA = address(0xB1); + address internal constant BUYER_1 = address(0xB1A1); + address internal constant BUYER_2 = address(0xB1A2); + address internal constant THIRD_PARTY = address(0xC1); + + bytes32 internal constant OWNER_ROLE = keccak256("OWNER_ROLE"); + + function setUp() public { + impl721 = new UserCollection721(); + impl1155 = new UserCollection1155(); + CollectionFactory logic = new CollectionFactory(); + bytes memory init = abi.encodeCall( + CollectionFactory.initialize, (ADMIN, OPERATOR, address(impl721), address(impl1155)) + ); + factory = CollectionFactory(address(new ERC1967Proxy(address(logic), init))); + } + + function test_endToEnd_happyPath() public { + // 1. Operator creates an ERC-721 collection for creator α. + vm.prank(OPERATOR); + address c721 = factory.createCollection721( + CreateParams721({ + owner: CREATOR_ALPHA, + name: "Alpha", + symbol: "ALP", + baseURI: "ipfs://alpha/", + contractURI: "ipfs://alpha-contract.json", + royaltyRecipient: CREATOR_ALPHA, + royaltyBps: 500, + additionalMinters: new address[](0) + }), + keccak256("order-alpha") + ); + UserCollection721 col721 = UserCollection721(c721); + assertTrue(col721.hasRole(OWNER_ROLE, CREATOR_ALPHA)); + + // 2. Operator creates an ERC-1155 collection for creator β. + vm.prank(OPERATOR); + address c1155 = factory.createCollection1155( + CreateParams1155({ + owner: CREATOR_BETA, + uri: "ipfs://beta/{id}.json", + contractURI: "ipfs://beta-contract.json", + royaltyRecipient: CREATOR_BETA, + royaltyBps: 250, + additionalMinters: new address[](0) + }), + keccak256("order-beta") + ); + UserCollection1155 col1155 = UserCollection1155(c1155); + assertTrue(col1155.hasRole(OWNER_ROLE, CREATOR_BETA)); + + // 3. Operator mints into both on behalf of fiat buyers. + vm.prank(OPERATOR); + uint256 alphaTokenId = col721.mint(BUYER_1, "1.json"); + assertEq(col721.ownerOf(alphaTokenId), BUYER_1); + + vm.prank(OPERATOR); + col1155.mint(BUYER_2, 7, 3, ""); + assertEq(col1155.balanceOf(BUYER_2, 7), 3); + + // 4. Creator α transfers an item to a third party. + vm.prank(BUYER_1); + col721.transferFrom(BUYER_1, THIRD_PARTY, alphaTokenId); + assertEq(col721.ownerOf(alphaTokenId), THIRD_PARTY); + + // 5. Creator α locks metadata and royalties. + vm.prank(CREATOR_ALPHA); + col721.lockMetadata(); + vm.prank(CREATOR_ALPHA); + col721.lockRoyalties(); + assertTrue(col721.metadataLocked()); + assertTrue(col721.royaltiesLocked()); + + // 6. Subsequent setter calls revert. + vm.prank(CREATOR_ALPHA); + vm.expectRevert(IUserCollection721.MetadataIsLocked.selector); + col721.setBaseURI("ipfs://changed/"); + + vm.prank(CREATOR_ALPHA); + vm.expectRevert(IUserCollection721.RoyaltiesAreLocked.selector); + col721.setDefaultRoyalty(CREATOR_ALPHA, 100); + + // 7. Admin upgrades the factory and ships a new ERC-721 implementation. + CollectionFactoryV2Mock v2Logic = new CollectionFactoryV2Mock(); + vm.prank(ADMIN); + factory.upgradeToAndCall(address(v2Logic), ""); + assertEq(CollectionFactoryV2Mock(address(factory)).v2Sentinel(), 4242); + + UserCollection721 newImpl721 = new UserCollection721(); + vm.prank(ADMIN); + factory.setImplementation721(address(newImpl721)); + + // 8. New ERC-721 collection deploys with new implementation; old + // collection remains on the previous implementation. + vm.prank(OPERATOR); + address c721b = factory.createCollection721( + CreateParams721({ + owner: CREATOR_ALPHA, + name: "Alpha2", + symbol: "ALP2", + baseURI: "ipfs://alpha2/", + contractURI: "ipfs://alpha2-contract.json", + royaltyRecipient: CREATOR_ALPHA, + royaltyBps: 500, + additionalMinters: new address[](0) + }), + keccak256("order-alpha-v2") + ); + // Each per-collection ERC1967Proxy delegates to the factory's + // `_erc721Implementation` / `_erc1155Implementation` via the EIP-1967 + // implementation slot, captured at deploy time. The factory pointer + // is the observable state that proves the upgrade took effect for + // newly deployed collections; existing collections keep delegating + // to whichever implementation address was written into their slot + // when they were created. + assertEq(factory.erc721Implementation(), address(newImpl721)); + // Old collection still operates normally. + assertEq(col721.ownerOf(alphaTokenId), THIRD_PARTY); + // New collection initialized correctly under new implementation. + assertTrue(UserCollection721(c721b).hasRole(OWNER_ROLE, CREATOR_ALPHA)); + } +} diff --git a/test/collections/ERC1967Proxy.permanence.t.sol b/test/collections/ERC1967Proxy.permanence.t.sol new file mode 100644 index 00000000..3f0bf509 --- /dev/null +++ b/test/collections/ERC1967Proxy.permanence.t.sol @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.26; + +import {Test} from "forge-std/Test.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +/// @notice Bytecode-permanence proof for canonical OZ ERC1967Proxy. +/// Codifies design §3.5.2 (1): no SELFDESTRUCT, no caller-controlled +/// delegatecall. Defense-in-depth audit gate. +contract ERC1967ProxyPermanenceTest is Test { + /// @dev Deploy a real ERC1967Proxy and read its runtime bytecode. + /// Empty initData skips the constructor delegatecall — we just want + /// the deployed runtime, not a working instance. + function _runtime() internal returns (bytes memory) { + // Use any non-zero implementation; the runtime is the same regardless. + ERC1967Proxy p = new ERC1967Proxy(address(this), ""); + return address(p).code; + } + + function test_runtimeContainsNoSelfdestruct() public { + bytes memory code = _runtime(); + require(code.length > 0, "no runtime"); + + for (uint256 i = 0; i < code.length; ) { + uint8 op = uint8(code[i]); + + // PUSH1..PUSH32 — skip the immediate bytes (op 0x60..0x7f). + if (op >= 0x60 && op <= 0x7f) { + uint256 imm = uint256(op) - 0x5f; + i += 1 + imm; + continue; + } + + // SELFDESTRUCT (0xff) is the EVM mnemonic; canonical OZ + // ERC1967Proxy must not contain it. + assertTrue(op != 0xff, "ERC1967Proxy contains SELFDESTRUCT"); + + i += 1; + } + } + + function test_proxyImplementationDelegatecallTargetIsConstructorFixed() public { + // The only delegatecall in ERC1967Proxy's runtime targets _implementation() + // which reads from the EIP-1967 slot. The slot is written exclusively by + // ERC1967Utils.upgradeToAndCall (called only from the proxy's own + // constructor since the impl does not inherit UUPSUpgradeable). This test + // exercises the property by deploying with one impl and asserting the + // EIP-1967 slot equals that impl, then asserting that no external call + // can change it. + address impl = address(this); + ERC1967Proxy p = new ERC1967Proxy(impl, ""); + + bytes32 IMPL_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; + bytes32 stored = vm.load(address(p), IMPL_SLOT); + assertEq(address(uint160(uint256(stored))), impl, "EIP-1967 slot mismatch"); + + // No external selector exposed by the proxy can write IMPL_SLOT — the + // proxy's only entry point is the fallback, which delegatecalls the + // current impl. Since `address(this)` (the test contract) has no + // upgradeToAndCall selector, any call to mutate the slot reverts/no-ops. + // We assert by replaying upgradeToAndCall through the proxy and showing + // the slot is unchanged. + bytes memory ignored = abi.encodeWithSelector( + 0x4f1ef286, address(0xdeadbeef), bytes("") + ); + // staticcall to avoid mutating; the call should not return data that + // reflects a successful upgrade. + (bool ok, ) = address(p).staticcall(ignored); + // Whether `ok` is true or false depends on the test contract's fallback; + // either way the slot must not have changed. + ok; // silence unused warning + bytes32 storedAfter = vm.load(address(p), IMPL_SLOT); + assertEq(stored, storedAfter, "EIP-1967 slot was mutated"); + } +} diff --git a/test/collections/UserCollection1155.t.sol b/test/collections/UserCollection1155.t.sol new file mode 100644 index 00000000..c2e3898d --- /dev/null +++ b/test/collections/UserCollection1155.t.sol @@ -0,0 +1,376 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.26; + +import {Test} from "forge-std/Test.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol"; +import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; +import {IERC1155} from "@openzeppelin/contracts/token/ERC1155/IERC1155.sol"; +import {IERC1155MetadataURI} from "@openzeppelin/contracts/token/ERC1155/extensions/IERC1155MetadataURI.sol"; +import {IERC2981} from "@openzeppelin/contracts/interfaces/IERC2981.sol"; +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; + +import {UserCollection1155} from "../../src/collections/UserCollection1155.sol"; +import {IUserCollection1155} from "../../src/collections/interfaces/IUserCollection1155.sol"; +import {CreateParams1155} from "../../src/collections/interfaces/CollectionTypes.sol"; + +contract UserCollection1155Test is Test { + UserCollection1155 internal impl; + + address internal constant OWNER = address(0xA11CE); + address internal constant OPERATOR_MINTER = address(0xB0B); + address internal constant ROYALTY_RECIPIENT = address(0xCAFE); + address internal constant ALICE = address(0xA1); + address internal constant STRANGER = address(0xDEAD); + + bytes32 internal constant OWNER_ROLE = keccak256("OWNER_ROLE"); + bytes32 internal constant MINTER_ROLE = keccak256("MINTER_ROLE"); + + event TransferSingle( + address indexed operator, address indexed from, address indexed to, uint256 id, uint256 value + ); + event TransferBatch( + address indexed operator, address indexed from, address indexed to, uint256[] ids, uint256[] values + ); + event MetadataLocked(); + event RoyaltiesLocked(); + event ContractURIUpdated(string newURI); + event URIUpdated(string newURI); + + function setUp() public { + impl = new UserCollection1155(); + } + + // ────────────────────────────────────────────── + // Helpers + // ────────────────────────────────────────────── + + function _deployClone(uint96 royaltyBps, address[] memory additionalMinters) + internal + returns (UserCollection1155 clone) + { + address cloneAddr = address(new ERC1967Proxy(address(impl), "")); + clone = UserCollection1155(cloneAddr); + clone.initialize( + CreateParams1155({ + owner: OWNER, + uri: "ipfs://1155/{id}.json", + contractURI: "ipfs://contract.json", + royaltyRecipient: ROYALTY_RECIPIENT, + royaltyBps: royaltyBps, + additionalMinters: additionalMinters + }), + OPERATOR_MINTER + ); + } + + function _deployCloneDefault() internal returns (UserCollection1155) { + address[] memory empty = new address[](0); + return _deployClone(500, empty); + } + + // ────────────────────────────────────────────── + // Initialization + // ────────────────────────────────────────────── + + function test_initialize_setsAllFieldsAndRoles() public { + address[] memory extras = new address[](1); + extras[0] = ALICE; + UserCollection1155 clone = _deployClone(750, extras); + + assertEq(clone.uri(0), "ipfs://1155/{id}.json"); + assertEq(clone.contractURI(), "ipfs://contract.json"); + assertFalse(clone.metadataLocked()); + assertFalse(clone.royaltiesLocked()); + + assertTrue(clone.hasRole(OWNER_ROLE, OWNER)); + assertTrue(clone.hasRole(MINTER_ROLE, OWNER)); + assertTrue(clone.hasRole(MINTER_ROLE, OPERATOR_MINTER)); + assertTrue(clone.hasRole(MINTER_ROLE, ALICE)); + assertEq(clone.getRoleAdmin(MINTER_ROLE), OWNER_ROLE); + + (address recv, uint256 amount) = clone.royaltyInfo(0, 10_000); + assertEq(recv, ROYALTY_RECIPIENT); + assertEq(amount, 750); + } + + function test_initialize_revertsOnZeroOwner() public { + address cloneAddr = address(new ERC1967Proxy(address(impl), "")); + address[] memory empty = new address[](0); + vm.expectRevert(IUserCollection1155.ZeroAddress.selector); + UserCollection1155(cloneAddr).initialize( + CreateParams1155({ + owner: address(0), + uri: "", + contractURI: "", + royaltyRecipient: address(0), + royaltyBps: 0, + additionalMinters: empty + }), + OPERATOR_MINTER + ); + } + + function test_initialize_revertsOnZeroOperatorMinter() public { + address cloneAddr = address(new ERC1967Proxy(address(impl), "")); + address[] memory empty = new address[](0); + vm.expectRevert(IUserCollection1155.ZeroAddress.selector); + UserCollection1155(cloneAddr).initialize( + CreateParams1155({ + owner: OWNER, + uri: "", + contractURI: "", + royaltyRecipient: address(0), + royaltyBps: 0, + additionalMinters: empty + }), + address(0) + ); + } + + function test_implementation_disablesInitializers() public { + address[] memory empty = new address[](0); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize( + CreateParams1155({ + owner: OWNER, + uri: "", + contractURI: "", + royaltyRecipient: address(0), + royaltyBps: 0, + additionalMinters: empty + }), + OPERATOR_MINTER + ); + } + + // ────────────────────────────────────────────── + // Mint + // ────────────────────────────────────────────── + + function test_mint_assignsBalanceAndEmits() public { + UserCollection1155 clone = _deployCloneDefault(); + + vm.expectEmit(true, true, true, true); + emit TransferSingle(OPERATOR_MINTER, address(0), ALICE, 42, 5); + + vm.prank(OPERATOR_MINTER); + clone.mint(ALICE, 42, 5, ""); + assertEq(clone.balanceOf(ALICE, 42), 5); + assertEq(clone.totalSupply(42), 5); + } + + function test_mint_revertsForNonMinter() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.expectRevert( + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, STRANGER, MINTER_ROLE) + ); + vm.prank(STRANGER); + clone.mint(ALICE, 0, 1, ""); + } + + function test_mintBatch_singleRecipientUpdatesBalances() public { + UserCollection1155 clone = _deployCloneDefault(); + uint256[] memory ids = new uint256[](3); + uint256[] memory amounts = new uint256[](3); + ids[0] = 1; ids[1] = 2; ids[2] = 3; + amounts[0] = 10; amounts[1] = 20; amounts[2] = 30; + + vm.expectEmit(true, true, true, true); + emit TransferBatch(OPERATOR_MINTER, address(0), ALICE, ids, amounts); + + vm.prank(OPERATOR_MINTER); + clone.mintBatch(ALICE, ids, amounts, ""); + + assertEq(clone.balanceOf(ALICE, 1), 10); + assertEq(clone.balanceOf(ALICE, 2), 20); + assertEq(clone.balanceOf(ALICE, 3), 30); + } + + function test_mintBatch_revertsLengthMismatch() public { + UserCollection1155 clone = _deployCloneDefault(); + uint256[] memory ids = new uint256[](2); + uint256[] memory amounts = new uint256[](1); + ids[0] = 1; ids[1] = 2; amounts[0] = 1; + vm.expectRevert(IUserCollection1155.LengthMismatch.selector); + vm.prank(OPERATOR_MINTER); + clone.mintBatch(ALICE, ids, amounts, ""); + } + + function test_mintBatch_revertsOversize() public { + UserCollection1155 clone = _deployCloneDefault(); + uint256[] memory ids = new uint256[](101); + uint256[] memory amounts = new uint256[](101); + for (uint256 i = 0; i < 101; ++i) { ids[i] = i; amounts[i] = 1; } + vm.expectRevert(abi.encodeWithSelector(IUserCollection1155.BatchTooLarge.selector, 101, 100)); + vm.prank(OPERATOR_MINTER); + clone.mintBatch(ALICE, ids, amounts, ""); + } + + // ────────────────────────────────────────────── + // Owner-mutable settings + locks + // ────────────────────────────────────────────── + + function test_setURI_updatesAndEmits() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.expectEmit(true, true, true, true); + emit URIUpdated("ipfs://new/{id}.json"); + vm.prank(OWNER); + clone.setURI("ipfs://new/{id}.json"); + assertEq(clone.uri(123), "ipfs://new/{id}.json"); + } + + function test_lockMetadata_blocksSubsequentSetters() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.expectEmit(true, true, true, true); + emit MetadataLocked(); + vm.prank(OWNER); + clone.lockMetadata(); + + vm.prank(OWNER); + vm.expectRevert(IUserCollection1155.MetadataIsLocked.selector); + clone.setURI("x"); + + vm.prank(OWNER); + vm.expectRevert(IUserCollection1155.MetadataIsLocked.selector); + clone.setContractURI("x"); + } + + function test_lockRoyalties_blocksSubsequentSetters() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.expectEmit(true, true, true, true); + emit RoyaltiesLocked(); + vm.prank(OWNER); + clone.lockRoyalties(); + + vm.prank(OWNER); + vm.expectRevert(IUserCollection1155.RoyaltiesAreLocked.selector); + clone.setDefaultRoyalty(ALICE, 100); + } + + function test_setContractURI_emitsAndUpdates() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.expectEmit(true, true, true, true); + emit ContractURIUpdated("ipfs://newcontract.json"); + vm.prank(OWNER); + clone.setContractURI("ipfs://newcontract.json"); + assertEq(clone.contractURI(), "ipfs://newcontract.json"); + } + + function test_setDefaultRoyalty_zeroBpsClears() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.prank(OWNER); + clone.setDefaultRoyalty(address(0), 0); + (address recv, uint256 amount) = clone.royaltyInfo(0, 10_000); + assertEq(recv, address(0)); + assertEq(amount, 0); + } + + function test_setDefaultRoyalty_nonZeroBpsUpdates() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.prank(OWNER); + clone.setDefaultRoyalty(ALICE, 1000); + (address recv, uint256 amount) = clone.royaltyInfo(0, 10_000); + assertEq(recv, ALICE); + assertEq(amount, 1000); + } + + function test_owner_canRevokeOperatorMinter() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.prank(OWNER); + clone.revokeRole(MINTER_ROLE, OPERATOR_MINTER); + + vm.prank(OPERATOR_MINTER); + vm.expectRevert( + abi.encodeWithSelector( + IAccessControl.AccessControlUnauthorizedAccount.selector, OPERATOR_MINTER, MINTER_ROLE + ) + ); + clone.mint(ALICE, 1, 1, ""); + } + + // ────────────────────────────────────────────── + // ERC-2981 + supportsInterface + // ────────────────────────────────────────────── + + function test_supportsInterface_advertisesAllExpectedIds() public { + UserCollection1155 clone = _deployCloneDefault(); + assertTrue(clone.supportsInterface(type(IERC165).interfaceId)); + assertTrue(clone.supportsInterface(type(IERC1155).interfaceId)); + assertTrue(clone.supportsInterface(type(IERC1155MetadataURI).interfaceId)); + assertTrue(clone.supportsInterface(type(IERC2981).interfaceId)); + assertTrue(clone.supportsInterface(type(IAccessControl).interfaceId)); + } + + // ────────────────────────────────────────────── + // Burn + supply (ERC1155Burnable + ERC1155Supply) + // ────────────────────────────────────────────── + + function test_burn_decrementsSupplyAndBalance() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.prank(OPERATOR_MINTER); + clone.mint(ALICE, 1, 5, ""); + assertEq(clone.totalSupply(1), 5); + + vm.prank(ALICE); + clone.burn(ALICE, 1, 2); + assertEq(clone.balanceOf(ALICE, 1), 3); + assertEq(clone.totalSupply(1), 3); + } + + function test_burn_revertsForUnauthorized() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.prank(OPERATOR_MINTER); + clone.mint(ALICE, 1, 5, ""); + + vm.prank(STRANGER); + vm.expectRevert(); + clone.burn(ALICE, 1, 1); + } + + function test_supply_tracksAcrossMintAndMintBatch() public { + UserCollection1155 clone = _deployCloneDefault(); + vm.prank(OPERATOR_MINTER); + clone.mint(ALICE, 1, 10, ""); + + uint256[] memory ids = new uint256[](2); + uint256[] memory amounts = new uint256[](2); + ids[0] = 1; ids[1] = 2; + amounts[0] = 5; amounts[1] = 7; + vm.prank(OPERATOR_MINTER); + clone.mintBatch(ALICE, ids, amounts, ""); + + assertEq(clone.totalSupply(1), 15); + assertEq(clone.totalSupply(2), 7); + } + + // ────────────────────────────────────────────── + // Bytecode permanence (§7.2 row 15, §8.3) + // ────────────────────────────────────────────── + + function test_implementation_runtimeCode_containsNoSelfdestruct() public view { + bytes memory code = address(impl).code; + uint256 i = 0; + while (i < code.length) { + uint8 op = uint8(code[i]); + assertTrue(op != 0xff, "SELFDESTRUCT opcode at runtime position"); + if (op >= 0x60 && op <= 0x7f) { + i += 1 + (op - 0x5f); + } else { + i += 1; + } + } + } + + function test_implementationHasNoUpgradeSelectors() public view { + // proxiableUUID() — selector 0x52d1902d + (bool ok1, ) = address(impl).staticcall(abi.encodeWithSelector(0x52d1902d)); + assertFalse(ok1, "impl must not expose proxiableUUID"); + + // upgradeToAndCall(address,bytes) — selector 0x4f1ef286 + (bool ok2, ) = address(impl).staticcall( + abi.encodeWithSelector(0x4f1ef286, address(0), bytes("")) + ); + assertFalse(ok2, "impl must not expose upgradeToAndCall"); + } +} diff --git a/test/collections/UserCollection721.t.sol b/test/collections/UserCollection721.t.sol new file mode 100644 index 00000000..59d4a355 --- /dev/null +++ b/test/collections/UserCollection721.t.sol @@ -0,0 +1,470 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.26; + +import {Test} from "forge-std/Test.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol"; +import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; +import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; +import {IERC721Metadata} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol"; +import {IERC2981} from "@openzeppelin/contracts/interfaces/IERC2981.sol"; +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; + +import {UserCollection721} from "../../src/collections/UserCollection721.sol"; +import {IUserCollection721} from "../../src/collections/interfaces/IUserCollection721.sol"; +import {CreateParams721} from "../../src/collections/interfaces/CollectionTypes.sol"; + +contract UserCollection721Test is Test { + UserCollection721 internal impl; + + address internal constant OWNER = address(0xA11CE); + address internal constant OPERATOR_MINTER = address(0xB0B); + address internal constant ROYALTY_RECIPIENT = address(0xCAFE); + address internal constant ALICE = address(0xA1); + address internal constant BOB = address(0xB2); + address internal constant STRANGER = address(0xDEAD); + + bytes32 internal constant OWNER_ROLE = keccak256("OWNER_ROLE"); + bytes32 internal constant MINTER_ROLE = keccak256("MINTER_ROLE"); + + event Transfer(address indexed from, address indexed to, uint256 indexed tokenId); + event MetadataLocked(); + event RoyaltiesLocked(); + event ContractURIUpdated(string newURI); + event BaseURIUpdated(string newBase); + + function setUp() public { + impl = new UserCollection721(); + } + + // ────────────────────────────────────────────── + // Helpers + // ────────────────────────────────────────────── + + function _deployClone(uint96 royaltyBps, address[] memory additionalMinters) + internal + returns (UserCollection721 clone) + { + address cloneAddr = address(new ERC1967Proxy(address(impl), "")); + clone = UserCollection721(cloneAddr); + clone.initialize( + CreateParams721({ + owner: OWNER, + name: "Test Collection", + symbol: "TC", + baseURI: "ipfs://base/", + contractURI: "ipfs://contract.json", + royaltyRecipient: ROYALTY_RECIPIENT, + royaltyBps: royaltyBps, + additionalMinters: additionalMinters + }), + OPERATOR_MINTER + ); + } + + function _deployCloneDefault() internal returns (UserCollection721) { + address[] memory empty = new address[](0); + return _deployClone(500, empty); + } + + // ────────────────────────────────────────────── + // Initialization + // ────────────────────────────────────────────── + + function test_initialize_setsAllFieldsAndRoles() public { + address[] memory extras = new address[](1); + extras[0] = ALICE; + UserCollection721 clone = _deployClone(750, extras); + + assertEq(clone.name(), "Test Collection"); + assertEq(clone.symbol(), "TC"); + assertEq(clone.contractURI(), "ipfs://contract.json"); + assertEq(clone.nextTokenId(), 0); + assertFalse(clone.metadataLocked()); + assertFalse(clone.royaltiesLocked()); + + assertTrue(clone.hasRole(OWNER_ROLE, OWNER)); + assertTrue(clone.hasRole(MINTER_ROLE, OWNER)); + assertTrue(clone.hasRole(MINTER_ROLE, OPERATOR_MINTER)); + assertTrue(clone.hasRole(MINTER_ROLE, ALICE)); + + assertEq(clone.getRoleAdmin(MINTER_ROLE), OWNER_ROLE); + + (address recv, uint256 amount) = clone.royaltyInfo(0, 10_000); + assertEq(recv, ROYALTY_RECIPIENT); + assertEq(amount, 750); + } + + function test_initialize_revertsOnZeroOwner() public { + address cloneAddr = address(new ERC1967Proxy(address(impl), "")); + address[] memory empty = new address[](0); + vm.expectRevert(IUserCollection721.ZeroAddress.selector); + UserCollection721(cloneAddr).initialize( + CreateParams721({ + owner: address(0), + name: "X", + symbol: "X", + baseURI: "", + contractURI: "", + royaltyRecipient: address(0), + royaltyBps: 0, + additionalMinters: empty + }), + OPERATOR_MINTER + ); + } + + function test_initialize_revertsOnZeroOperatorMinter() public { + address cloneAddr = address(new ERC1967Proxy(address(impl), "")); + address[] memory empty = new address[](0); + vm.expectRevert(IUserCollection721.ZeroAddress.selector); + UserCollection721(cloneAddr).initialize( + CreateParams721({ + owner: OWNER, + name: "X", + symbol: "X", + baseURI: "", + contractURI: "", + royaltyRecipient: address(0), + royaltyBps: 0, + additionalMinters: empty + }), + address(0) + ); + } + + function test_initialize_skipsRoyaltyWhenBpsZero() public { + address[] memory empty = new address[](0); + UserCollection721 clone = _deployClone(0, empty); + (address recv, uint256 amount) = clone.royaltyInfo(0, 10_000); + assertEq(recv, address(0)); + assertEq(amount, 0); + } + + function test_implementation_disablesInitializers() public { + // The implementation contract itself must not be initializable. + address[] memory empty = new address[](0); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize( + CreateParams721({ + owner: OWNER, + name: "X", + symbol: "X", + baseURI: "", + contractURI: "", + royaltyRecipient: address(0), + royaltyBps: 0, + additionalMinters: empty + }), + OPERATOR_MINTER + ); + } + + function test_initialize_revertsOnSecondCall() public { + UserCollection721 clone = _deployCloneDefault(); + address[] memory empty = new address[](0); + vm.expectRevert(Initializable.InvalidInitialization.selector); + clone.initialize( + CreateParams721({ + owner: OWNER, + name: "X", + symbol: "X", + baseURI: "", + contractURI: "", + royaltyRecipient: address(0), + royaltyBps: 0, + additionalMinters: empty + }), + OPERATOR_MINTER + ); + } + + // ────────────────────────────────────────────── + // Mint + // ────────────────────────────────────────────── + + function test_mint_assignsIdAndUriAndIncrementsCounter() public { + UserCollection721 clone = _deployCloneDefault(); + + vm.expectEmit(true, true, true, true); + emit Transfer(address(0), ALICE, 0); + + vm.prank(OPERATOR_MINTER); + uint256 id = clone.mint(ALICE, "ipfs://0.json"); + + assertEq(id, 0); + assertEq(clone.nextTokenId(), 1); + assertEq(clone.ownerOf(0), ALICE); + assertEq(clone.tokenURI(0), "ipfs://base/ipfs://0.json"); + } + + function test_mint_revertsForNonMinter() public { + UserCollection721 clone = _deployCloneDefault(); + vm.expectRevert( + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, STRANGER, MINTER_ROLE) + ); + vm.prank(STRANGER); + clone.mint(ALICE, "ipfs://0.json"); + } + + // ────────────────────────────────────────────── + // mintBatch + // ────────────────────────────────────────────── + + function test_mintBatch_returnsContiguousIdsAndMatchesTransfers() public { + UserCollection721 clone = _deployCloneDefault(); + + // Pre-seed one token so the batch starts at id 1. + vm.prank(OPERATOR_MINTER); + clone.mint(ALICE, "first.json"); + + address[] memory recipients = new address[](3); + recipients[0] = ALICE; + recipients[1] = BOB; + recipients[2] = ALICE; + string[] memory uris = new string[](3); + uris[0] = "1.json"; + uris[1] = "2.json"; + uris[2] = "3.json"; + + vm.expectEmit(true, true, true, true); + emit Transfer(address(0), ALICE, 1); + vm.expectEmit(true, true, true, true); + emit Transfer(address(0), BOB, 2); + vm.expectEmit(true, true, true, true); + emit Transfer(address(0), ALICE, 3); + + vm.prank(OPERATOR_MINTER); + uint256[] memory ids = clone.mintBatch(recipients, uris); + + assertEq(ids.length, 3); + assertEq(ids[0], 1); + assertEq(ids[1], 2); + assertEq(ids[2], 3); + assertEq(clone.nextTokenId(), 4); + } + + function test_mintBatch_revertsLengthMismatch() public { + UserCollection721 clone = _deployCloneDefault(); + address[] memory recipients = new address[](2); + recipients[0] = ALICE; + recipients[1] = BOB; + string[] memory uris = new string[](1); + uris[0] = "x"; + vm.expectRevert(IUserCollection721.LengthMismatch.selector); + vm.prank(OPERATOR_MINTER); + clone.mintBatch(recipients, uris); + } + + function test_mintBatch_revertsOversize() public { + UserCollection721 clone = _deployCloneDefault(); + address[] memory recipients = new address[](101); + string[] memory uris = new string[](101); + for (uint256 i = 0; i < 101; ++i) { + recipients[i] = ALICE; + uris[i] = "x"; + } + vm.expectRevert(abi.encodeWithSelector(IUserCollection721.BatchTooLarge.selector, 101, 100)); + vm.prank(OPERATOR_MINTER); + clone.mintBatch(recipients, uris); + } + + // ────────────────────────────────────────────── + // Owner-mutable settings + locks + // ────────────────────────────────────────────── + + function test_setBaseURI_emitsAndUpdates() public { + UserCollection721 clone = _deployCloneDefault(); + vm.expectEmit(true, true, true, true); + emit BaseURIUpdated("ipfs://newbase/"); + vm.prank(OWNER); + clone.setBaseURI("ipfs://newbase/"); + + vm.prank(OPERATOR_MINTER); + uint256 id = clone.mint(ALICE, "0.json"); + assertEq(clone.tokenURI(id), "ipfs://newbase/0.json"); + } + + function test_setBaseURI_revertsForNonOwner() public { + UserCollection721 clone = _deployCloneDefault(); + vm.expectRevert( + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, STRANGER, OWNER_ROLE) + ); + vm.prank(STRANGER); + clone.setBaseURI("x"); + } + + function test_lockMetadata_blocksSubsequentSetters() public { + UserCollection721 clone = _deployCloneDefault(); + vm.expectEmit(true, true, true, true); + emit MetadataLocked(); + vm.prank(OWNER); + clone.lockMetadata(); + assertTrue(clone.metadataLocked()); + + vm.prank(OWNER); + vm.expectRevert(IUserCollection721.MetadataIsLocked.selector); + clone.setBaseURI("x"); + + vm.prank(OWNER); + vm.expectRevert(IUserCollection721.MetadataIsLocked.selector); + clone.setContractURI("x"); + } + + function test_lockRoyalties_blocksSubsequentSetters() public { + UserCollection721 clone = _deployCloneDefault(); + vm.expectEmit(true, true, true, true); + emit RoyaltiesLocked(); + vm.prank(OWNER); + clone.lockRoyalties(); + assertTrue(clone.royaltiesLocked()); + + vm.prank(OWNER); + vm.expectRevert(IUserCollection721.RoyaltiesAreLocked.selector); + clone.setDefaultRoyalty(ALICE, 100); + } + + function test_setDefaultRoyalty_zeroBpsClears() public { + UserCollection721 clone = _deployCloneDefault(); + vm.prank(OWNER); + clone.setDefaultRoyalty(address(0), 0); + (address recv, uint256 amount) = clone.royaltyInfo(0, 10_000); + assertEq(recv, address(0)); + assertEq(amount, 0); + } + + function test_setDefaultRoyalty_nonZeroBpsUpdates() public { + UserCollection721 clone = _deployCloneDefault(); + vm.prank(OWNER); + clone.setDefaultRoyalty(ALICE, 1000); + (address recv, uint256 amount) = clone.royaltyInfo(0, 10_000); + assertEq(recv, ALICE); + assertEq(amount, 1000); + } + + function test_setContractURI_emitsAndUpdates() public { + UserCollection721 clone = _deployCloneDefault(); + vm.expectEmit(true, true, true, true); + emit ContractURIUpdated("ipfs://newcontract.json"); + vm.prank(OWNER); + clone.setContractURI("ipfs://newcontract.json"); + assertEq(clone.contractURI(), "ipfs://newcontract.json"); + } + + // ────────────────────────────────────────────── + // Role admin + // ────────────────────────────────────────────── + + function test_owner_canGrantAndRevokeMinterRole() public { + UserCollection721 clone = _deployCloneDefault(); + vm.prank(OWNER); + clone.grantRole(MINTER_ROLE, ALICE); + assertTrue(clone.hasRole(MINTER_ROLE, ALICE)); + + vm.prank(OWNER); + clone.revokeRole(MINTER_ROLE, OPERATOR_MINTER); + assertFalse(clone.hasRole(MINTER_ROLE, OPERATOR_MINTER)); + + vm.prank(OPERATOR_MINTER); + vm.expectRevert( + abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, OPERATOR_MINTER, MINTER_ROLE) + ); + clone.mint(ALICE, "x"); + } + + // ────────────────────────────────────────────── + // ERC-2981 + supportsInterface + // ────────────────────────────────────────────── + + function test_supportsInterface_advertisesAllExpectedIds() public { + UserCollection721 clone = _deployCloneDefault(); + assertTrue(clone.supportsInterface(type(IERC165).interfaceId)); + assertTrue(clone.supportsInterface(type(IERC721).interfaceId)); + assertTrue(clone.supportsInterface(type(IERC721Metadata).interfaceId)); + assertTrue(clone.supportsInterface(type(IERC2981).interfaceId)); + assertTrue(clone.supportsInterface(type(IAccessControl).interfaceId)); + } + + // ────────────────────────────────────────────── + // Burn (ERC721Burnable) + // ────────────────────────────────────────────── + + function test_burn_byOwnerRemovesToken() public { + UserCollection721 clone = _deployCloneDefault(); + vm.prank(OPERATOR_MINTER); + uint256 id = clone.mint(ALICE, "0.json"); + + vm.prank(ALICE); + clone.burn(id); + + vm.expectRevert(); + clone.ownerOf(id); + } + + function test_burn_revertsForUnauthorized() public { + UserCollection721 clone = _deployCloneDefault(); + vm.prank(OPERATOR_MINTER); + uint256 id = clone.mint(ALICE, "0.json"); + + vm.prank(STRANGER); + vm.expectRevert(); + clone.burn(id); + } + + function test_nextTokenId_isMonotonicAcrossSingleAndBatch() public { + UserCollection721 clone = _deployCloneDefault(); + vm.prank(OPERATOR_MINTER); + clone.mint(ALICE, "a"); + assertEq(clone.nextTokenId(), 1); + + address[] memory recipients = new address[](2); + recipients[0] = ALICE; + recipients[1] = BOB; + string[] memory uris = new string[](2); + uris[0] = "b"; uris[1] = "c"; + vm.prank(OPERATOR_MINTER); + clone.mintBatch(recipients, uris); + assertEq(clone.nextTokenId(), 3); + + vm.prank(OPERATOR_MINTER); + clone.mint(ALICE, "d"); + assertEq(clone.nextTokenId(), 4); + } + + // ────────────────────────────────────────────── + // Bytecode permanence (§7.2 row 15, §8.2) + // ────────────────────────────────────────────── + + function test_implementation_runtimeCode_containsNoSelfdestruct() public view { + // Walk EVM opcodes, skipping PUSH1..PUSH32 immediates (where 0xff can + // legitimately appear as constant data). Any 0xff byte found at an + // opcode position is a SELFDESTRUCT and would let the implementation be + // wiped — see §7.2 row 15. `bytecode_hash = "none"` in foundry.toml + // strips the metadata trailer that would otherwise produce false + // positives at the end of runtime code. + bytes memory code = address(impl).code; + uint256 i = 0; + while (i < code.length) { + uint8 op = uint8(code[i]); + assertTrue(op != 0xff, "SELFDESTRUCT opcode at runtime position"); + if (op >= 0x60 && op <= 0x7f) { + // PUSH1..PUSH32: skip (op - 0x5f) immediate bytes. + i += 1 + (op - 0x5f); + } else { + i += 1; + } + } + } + + function test_implementationHasNoUpgradeSelectors() public view { + // proxiableUUID() — selector 0x52d1902d + (bool ok1, ) = address(impl).staticcall(abi.encodeWithSelector(0x52d1902d)); + assertFalse(ok1, "impl must not expose proxiableUUID"); + + // upgradeToAndCall(address,bytes) — selector 0x4f1ef286 + (bool ok2, ) = address(impl).staticcall( + abi.encodeWithSelector(0x4f1ef286, address(0), bytes("")) + ); + assertFalse(ok2, "impl must not expose upgradeToAndCall"); + } +} diff --git a/test/collections/mocks/CollectionFactoryV2Mock.sol b/test/collections/mocks/CollectionFactoryV2Mock.sol new file mode 100644 index 00000000..be1e8f15 --- /dev/null +++ b/test/collections/mocks/CollectionFactoryV2Mock.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.26; + +import {CollectionFactory} from "../../../src/collections/CollectionFactory.sol"; + +/** + * @title CollectionFactoryV2Mock + * @notice UUPS upgrade target used by `CollectionFactory.t.sol` to verify that: + * (a) the upgrade actually changes the EIP-1967 implementation slot, + * (b) pre-upgrade storage (admin/operator roles, impl pointers, + * collectionByExternalId entries) reads correctly post-upgrade. + * @dev Adds one trivial public function whose presence post-upgrade proves + * the proxy genuinely delegated to new code rather than no-opping. + */ +contract CollectionFactoryV2Mock is CollectionFactory { + function v2Sentinel() external pure returns (uint256) { + return 4242; + } +} diff --git a/test/collections/mocks/NonUUPSImplementationMock.sol b/test/collections/mocks/NonUUPSImplementationMock.sol new file mode 100644 index 00000000..7f3dc730 --- /dev/null +++ b/test/collections/mocks/NonUUPSImplementationMock.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.26; + +/** + * @title NonUUPSImplementationMock + * @notice A bare contract used as an upgrade target to verify that + * `CollectionFactory.upgradeToAndCall` reverts with OZ's + * `ERC1967InvalidImplementation` (no `proxiableUUID`). + */ +contract NonUUPSImplementationMock { + uint256 public sentinel = 1; +}