diff --git a/.gitmodules b/.gitmodules index d3ccca37..bc643ec3 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,3 +4,6 @@ [submodule "solidity/lib/openzeppelin-contracts"] path = solidity/lib/openzeppelin-contracts url = https://github.com/openzeppelin/openzeppelin-contracts +[submodule "solidity/lib/openzeppelin-contracts-upgradeable"] + path = solidity/lib/openzeppelin-contracts-upgradeable + url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable.git diff --git a/Cargo.lock b/Cargo.lock index e8df220a..94f7ad2c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2627,6 +2627,7 @@ dependencies = [ "alloy-signer-local", "alloy-sol-types", "color-eyre", + "serde_json", "tokio", ] diff --git a/Makefile b/Makefile index b047383a..1e63c23d 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,7 @@ all: build build: + forge build cargo build cd custom-reth && cargo build diff --git a/contracts/Cargo.toml b/contracts/Cargo.toml index 14e33221..e86252f8 100644 --- a/contracts/Cargo.toml +++ b/contracts/Cargo.toml @@ -12,6 +12,9 @@ alloy-sol-types = { workspace = true } alloy-primitives = { workspace = true } alloy-contract = { workspace = true } +[build-dependencies] +serde_json = "1" + [dev-dependencies] alloy-network = "1.4.3" alloy-node-bindings = "1.4.3" diff --git a/contracts/build.rs b/contracts/build.rs index 371be652..097cb3eb 100644 --- a/contracts/build.rs +++ b/contracts/build.rs @@ -1,37 +1,88 @@ +//! Extract UUPS `__self` immutable reference offsets from the Foundry build artifact. +//! +//! The ValidatorManager contract uses UUPS (ERC1822) which stores an immutable +//! `__self = address(this)` set during construction. When genesis injects deployed +//! bytecode directly (bypassing the constructor), these locations must be patched. +//! This build script reads the offsets from the Foundry JSON artifact so they stay +//! in sync automatically after every `forge build`. + +use std::env; use std::fs; -use std::path::PathBuf; -use std::process::Command; +use std::path::Path; fn main() { - let workspace_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(".."); - - println!("cargo::rerun-if-changed=../solidity/src"); - println!("cargo::rerun-if-changed=../foundry.toml"); - - let status = Command::new("forge") - .args(["build", "--skip", "test", "script"]) - .current_dir(&workspace_root) - .status(); - - match status { - Ok(s) if s.success() => {} - Ok(s) => match s.code() { - Some(code) => panic!("forge build failed with exit code {code}"), - None => panic!("forge build terminated by signal"), - }, - Err(e) => { - panic!("failed to run forge: {e}\ninstall Foundry: https://getfoundry.sh/"); + let artifact_path = Path::new("../solidity/out/ValidatorManager.sol/ValidatorManager.json"); + + // Re-run if the artifact changes (i.e. after `forge build`) + println!( + "cargo::rerun-if-changed={}", + artifact_path.display() + ); + + let json_str = fs::read_to_string(artifact_path).unwrap_or_else(|e| { + panic!( + "Failed to read Foundry artifact at {}: {e}. Run `forge build` in the solidity/ directory first.", + artifact_path.display() + ) + }); + + let artifact: serde_json::Value = + serde_json::from_str(&json_str).expect("failed to parse Foundry artifact JSON"); + + let refs = artifact["deployedBytecode"]["immutableReferences"] + .as_object() + .expect("missing deployedBytecode.immutableReferences in artifact"); + + // Collect all (offset, length) pairs across all immutable IDs. + // For ValidatorManager there is exactly one immutable (__self from UUPSUpgradeable). + let mut offsets: Vec = Vec::new(); + let mut length: Option = None; + + for (_ast_id, locations) in refs { + for loc in locations.as_array().expect("immutable locations should be an array") { + let start = loc["start"] + .as_u64() + .expect("immutable reference missing 'start'") as usize; + let len = loc["length"] + .as_u64() + .expect("immutable reference missing 'length'") as usize; + + // All references to the same immutable have the same length + if let Some(prev) = length { + assert_eq!(prev, len, "inconsistent immutable reference lengths"); + } + length = Some(len); + offsets.push(start); } } - let compiled_contracts = ["ValidatorManager"]; + offsets.sort(); - for name in compiled_contracts { - let artifact = workspace_root.join(format!("solidity/out/{name}.sol/{name}.json")); - assert!( - fs::metadata(&artifact).is_ok(), - "expected artifact not found: {}", - artifact.display() - ); - } + assert!( + !offsets.is_empty(), + "no immutable references found in ValidatorManager artifact" + ); + + let length = length.unwrap(); + + // Generate Rust source + let out_dir = env::var("OUT_DIR").unwrap(); + let dest = Path::new(&out_dir).join("uups_immutable_offsets.rs"); + + let offsets_str = offsets + .iter() + .map(|o| o.to_string()) + .collect::>() + .join(", "); + + let generated = format!( + "/// Byte offsets of the UUPS `__self` immutable in ValidatorManager DEPLOYED_BYTECODE.\n\ + /// Auto-generated by build.rs from the Foundry artifact.\n\ + pub const UUPS_SELF_IMMUTABLE_OFFSETS: &[usize] = &[{offsets_str}];\n\ + \n\ + /// Byte length of each immutable reference (address left-padded to 32 bytes).\n\ + pub const UUPS_SELF_IMMUTABLE_LENGTH: usize = {length};\n" + ); + + fs::write(&dest, generated).expect("failed to write generated offsets file"); } diff --git a/contracts/src/lib.rs b/contracts/src/lib.rs index 888377b8..53e86c4d 100644 --- a/contracts/src/lib.rs +++ b/contracts/src/lib.rs @@ -1,5 +1,8 @@ pub mod validator_manager; pub use validator_manager::{ - ValidatorManager, GENESIS_ACCOUNT as GENESIS_VALIDATOR_MANAGER_ACCOUNT, + ValidatorManager, ValidatorManagerProxy, + GENESIS_ACCOUNT as GENESIS_VALIDATOR_MANAGER_ACCOUNT, + GENESIS_IMPL_ACCOUNT as GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT, + UUPS_SELF_IMMUTABLE_LENGTH, UUPS_SELF_IMMUTABLE_OFFSETS, }; diff --git a/contracts/src/validator_manager.rs b/contracts/src/validator_manager.rs index c44532ba..3e2bc804 100644 --- a/contracts/src/validator_manager.rs +++ b/contracts/src/validator_manager.rs @@ -1,8 +1,14 @@ use alloy_primitives::{address, Address}; -/// Genesis validator manager account address +/// Proxy address: all callers interact with this address (0x2000) pub const GENESIS_ACCOUNT: Address = address!("0x0000000000000000000000000000000000002000"); +/// Implementation address: the ValidatorManager logic contract deployed at genesis (0x2001) +pub const GENESIS_IMPL_ACCOUNT: Address = address!("0x0000000000000000000000000000000000002001"); + +// UUPS __self immutable offsets, auto-generated by build.rs from the Foundry artifact. +include!(concat!(env!("OUT_DIR"), "/uups_immutable_offsets.rs")); + alloy_sol_types::sol!( #[derive(Debug)] #[sol(rpc)] @@ -10,6 +16,13 @@ alloy_sol_types::sol!( "../solidity/out/ValidatorManager.sol/ValidatorManager.json" ); +alloy_sol_types::sol!( + #[derive(Debug)] + #[sol(rpc)] + ValidatorManagerProxy, + "../solidity/out/ValidatorManagerProxy.sol/ValidatorManagerProxy.json" +); + #[cfg(test)] mod tests { use alloy_network::EthereumWallet; @@ -17,9 +30,10 @@ mod tests { use alloy_primitives::{hex, Bytes, U256}; use alloy_provider::ProviderBuilder; use alloy_signer_local::PrivateKeySigner; + use alloy_sol_types::SolCall; use color_eyre::Result; - use super::ValidatorManager; + use super::{ValidatorManager, ValidatorManagerProxy}; const SECP256K1_G_UNCOMPRESSED: [u8; 65] = hex!( "04" @@ -42,7 +56,19 @@ mod tests { .wallet(EthereumWallet::from(signer)) .connect_http(anvil.endpoint_url()); - let contract = ValidatorManager::deploy(provider).await?; + // Deploy implementation then proxy + let impl_contract = ValidatorManager::deploy(provider.clone()).await?; + let init_data = ValidatorManager::initializeCall { + initialOwner: deployer, + } + .abi_encode(); + let proxy = ValidatorManagerProxy::deploy( + provider.clone(), + *impl_contract.address(), + Bytes::from(init_data), + ) + .await?; + let contract = ValidatorManager::new(*proxy.address(), provider); assert_eq!(contract.owner().call().await?, deployer); assert_eq!(contract.getValidatorCount().call().await?, U256::ZERO); diff --git a/docs/operational-docs/src/SUMMARY.md b/docs/operational-docs/src/SUMMARY.md index f33418ad..84dc5cdf 100644 --- a/docs/operational-docs/src/SUMMARY.md +++ b/docs/operational-docs/src/SUMMARY.md @@ -15,6 +15,7 @@ - [Consensus Layer](./architecture/consensus.md) - [Execution Layer](./architecture/execution-client.md) - [Proof-of-Authority Model](./architecture/poa.md) + - [Contract Upgrades](./architecture/contract-upgrades.md) - [Block Syncing](./architecture/syncing.md) - [EVM Compatibility](./evm-compatibility.md) diff --git a/docs/operational-docs/src/architecture/contract-upgrades.md b/docs/operational-docs/src/architecture/contract-upgrades.md new file mode 100644 index 00000000..47bba365 --- /dev/null +++ b/docs/operational-docs/src/architecture/contract-upgrades.md @@ -0,0 +1,129 @@ +# ValidatorManager Contract Upgrades + +The `ValidatorManager` contract is deployed behind a UUPS (Universal Upgradeable Proxy Standard) proxy at the predefined address `0x0000000000000000000000000000000000002000`. This allows the contract logic to be upgraded without changing the address that validators, the consensus engine, and tooling interact with. + +## Architecture + +``` +Callers (Rust app, CLI, etc.) + | + v + ERC1967Proxy at 0x2000 <-- permanent address, holds all storage + | delegatecall + v + ValidatorManager impl at 0x2001 <-- replaceable logic contract +``` + +- The **proxy** at `0x2000` holds all storage (validators, owner, total power) and delegates every call to the current implementation. +- The **implementation** at `0x2001` (at genesis) contains only the contract logic. It can be replaced by deploying a new implementation and calling `upgradeToAndCall` on the proxy. +- Only the contract **owner** can authorize upgrades. + +## How to Upgrade + +### 1. Prepare the New Implementation + +Write the updated `ValidatorManager` contract. The new version **must**: + +- Inherit from the same base contracts (`Initializable`, `OwnableUpgradeable`, `ReentrancyGuardUpgradeable`, `UUPSUpgradeable`) +- Keep all existing state variables in the same order +- Only append new state variables at the end + +### 2. Deploy the New Implementation + +Deploy the new implementation contract to any address using a standard transaction: + +```bash +forge create --rpc-url --private-key src/ValidatorManager.sol:ValidatorManager +``` + +Note the deployed address (e.g. `0xNewImplAddress`). + +### 3. Call `upgradeToAndCall` on the Proxy + +As the contract owner, call the upgrade function on the proxy address (`0x2000`): + +```bash +cast send 0x0000000000000000000000000000000000002000 \ + "upgradeToAndCall(address,bytes)" \ + \ + "0x" \ + --rpc-url \ + --private-key +``` + +If the new version needs migration logic, encode a `reinitializer(n)` call as the second argument instead of `"0x"`. + +### 4. Verify + +```bash +# Check the implementation address changed +cast storage 0x0000000000000000000000000000000000002000 \ + 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc \ + --rpc-url + +# Verify existing state is intact +cast call 0x0000000000000000000000000000000000002000 \ + "getValidatorCount()(uint256)" \ + --rpc-url +``` + +## Storage Layout Rules + +The proxy holds all storage. Upgrades must preserve storage compatibility: + +| Rule | Detail | +|------|--------| +| Never reorder state variables | Changing the order shifts all subsequent slots, corrupting data | +| Never remove state variables | Removing a variable shifts all subsequent slots | +| Never insert state variables in the middle | Same as reorder -- shifts slots | +| Only append new variables at the end | New variables get fresh sequential slots after existing ones | +| Do not change variable types to different sizes | e.g. changing `uint64` to `uint256` changes the slot packing | + +OpenZeppelin base contracts (`OwnableUpgradeable`, etc.) use ERC-7201 namespaced storage, so their internal storage will not collide with the contract's sequential slots. + +## Using `reinitializer` for Migration Logic + +If an upgrade needs to run one-time migration logic (e.g. initializing a new state variable), use the `reinitializer` modifier: + +```solidity +function initializeV2(uint256 newParam) public reinitializer(2) { + _newStateVar = newParam; +} +``` + +Then pass the encoded call as the second argument to `upgradeToAndCall`: + +```bash +cast send 0x0000000000000000000000000000000000002000 \ + "upgradeToAndCall(address,bytes)" \ + \ + $(cast calldata "initializeV2(uint256)" 42) \ + --rpc-url \ + --private-key +``` + +The `reinitializer(n)` modifier ensures the migration can only run once and in sequence (version 2 after version 1, etc.). + +## Limitations + +- **Proxy address is permanent**: `0x2000` can never change. All callers reference this address. +- **Constructor logic does not run on upgrade**: Use `reinitializer(n)` for any migration logic needed by a new version. +- **Owner key compromise is critical**: The owner can upgrade to an arbitrary implementation, effectively taking full control. Protect the owner key accordingly. +- **`renounceOwnership` permanently locks upgrades**: Once ownership is renounced, no further upgrades are possible. This is irreversible. +- **Existing chains migration**: Chains already running with the non-upgradeable contract require a hard fork to migrate to the proxy pattern. This involves replacing the code at `0x2000` with proxy bytecode, deploying the implementation at a new address, and adjusting storage layout (the slot positions change between the non-upgradeable and upgradeable versions). + +## Pre-Upgrade Checklist + +1. Verify storage compatibility: all existing state variables are in the same position +2. Run `forge test` against the new implementation with upgrade tests +3. Test the upgrade on a local devnet first +4. Audit the new implementation for correctness and security +5. Verify the owner key is available and functional +6. Communicate the upgrade plan to all network participants + +## Genesis Deployment Details + +At genesis, the proxy and implementation are deployed via alloc (no constructor execution): + +- **`0x2000`**: `ValidatorManagerProxy` runtime bytecode + pre-computed storage (EIP-1967 implementation slot, ERC-7201 Ownable/ReentrancyGuard/Initializable slots, validator state) +- **`0x2001`**: `ValidatorManager` runtime bytecode + Initializable storage set to `type(uint64).max` (locks the implementation against direct initialization, since the constructor's `_disableInitializers()` doesn't run during genesis alloc) diff --git a/foundry.toml b/foundry.toml index 8e21baf3..35b5808c 100644 --- a/foundry.toml +++ b/foundry.toml @@ -5,6 +5,7 @@ out = "out" libs = [ "lib" ] remappings = [ "@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/", + "@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/", "forge-std/=lib/forge-std/src/", ] diff --git a/solidity/lib/openzeppelin-contracts-upgradeable b/solidity/lib/openzeppelin-contracts-upgradeable new file mode 160000 index 00000000..e725abdd --- /dev/null +++ b/solidity/lib/openzeppelin-contracts-upgradeable @@ -0,0 +1 @@ +Subproject commit e725abddf1e01cf05ace496e950fc8e243cc7cab diff --git a/solidity/src/ValidatorManager.sol b/solidity/src/ValidatorManager.sol index 46d451e9..f9235af8 100644 --- a/solidity/src/ValidatorManager.sol +++ b/solidity/src/ValidatorManager.sol @@ -1,16 +1,19 @@ // SPDX-License-Identifier: Apache 2.0 pragma solidity ^0.8.28; -import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; +import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol"; +import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; -import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; /** * @title ValidatorManager - * @dev Manages a set of validators with associated voting power - * @dev Ownership controls who can register, unregister, and update validator power + * @dev Manages a set of validators with associated voting power. + * Deployed behind an ERC1967 proxy (UUPS pattern). Only the owner can + * authorize upgrades via `_authorizeUpgrade`. */ -contract ValidatorManager is Ownable, ReentrancyGuard { +contract ValidatorManager is Initializable, OwnableUpgradeable, ReentrancyGuardUpgradeable, UUPSUpgradeable { using EnumerableSet for EnumerableSet.AddressSet; uint256 internal constant SECP256K1_P = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F; @@ -39,7 +42,33 @@ contract ValidatorManager is Ownable, ReentrancyGuard { mapping(address => ValidatorInfo) private _validators; uint64 private _totalPower; - constructor() Ownable(_msgSender()) {} + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + + /** + * @dev Initializes the contract behind a proxy. Must be called exactly once. + * @param initialOwner The address that will own the contract and can authorize upgrades. + */ + function initialize(address initialOwner) public initializer { + __Ownable_init(initialOwner); + __ReentrancyGuard_init(); + __UUPSUpgradeable_init(); + } + + /** + * @dev Required by UUPSUpgradeable. Restricts upgrades to the owner. + */ + function _authorizeUpgrade(address) internal override onlyOwner {} + + /** + * @dev Contract version identifier for upgrade/migration tooling. + * @return Current implementation version. + */ + function version() external pure virtual returns (uint256) { + return 1; + } // Events event ValidatorRegistered(address indexed validatorAddress, Secp256k1Key validatorKey, uint64 power); diff --git a/solidity/src/ValidatorManagerProxy.sol b/solidity/src/ValidatorManagerProxy.sol new file mode 100644 index 00000000..0f7ee398 --- /dev/null +++ b/solidity/src/ValidatorManagerProxy.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: Apache 2.0 +pragma solidity ^0.8.28; + +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +/** + * @title ValidatorManagerProxy + * @dev Thin ERC1967 proxy wrapper used for the ValidatorManager UUPS deployment. + * Exists as a concrete contract so that Foundry emits a build artifact that + * Rust can reference via the `sol!` macro. + */ +contract ValidatorManagerProxy is ERC1967Proxy { + constructor(address logic, bytes memory data) ERC1967Proxy(logic, data) {} +} diff --git a/solidity/test/ValidatorManager.t.sol b/solidity/test/ValidatorManager.t.sol index 83c60746..4a12da15 100644 --- a/solidity/test/ValidatorManager.t.sol +++ b/solidity/test/ValidatorManager.t.sol @@ -3,10 +3,12 @@ pragma solidity ^0.8.28; import {Test} from "forge-std/Test.sol"; import {ValidatorManager} from "../src/ValidatorManager.sol"; -import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; +import {ValidatorManagerProxy} from "../src/ValidatorManagerProxy.sol"; +import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; contract ValidatorManagerTest is Test { ValidatorManager internal validatorManager; + ValidatorManager internal implementation; uint64 internal constant INITIAL_POWER = 100; uint64 internal constant UPDATED_POWER = 200; @@ -56,7 +58,12 @@ contract ValidatorManagerTest is Test { ); function setUp() public { - validatorManager = new ValidatorManager(); + // Deploy implementation and proxy + implementation = new ValidatorManager(); + bytes memory initData = abi.encodeCall(ValidatorManager.initialize, (address(this))); + ValidatorManagerProxy proxy = new ValidatorManagerProxy(address(implementation), initData); + validatorManager = ValidatorManager(address(proxy)); + ValidatorManager.Secp256k1Key memory aliceKeyMem = validatorManager._secp256k1KeyFromBytes(ALICE_UNCOMPRESSED); aliceKey = aliceKeyMem; aliceValidatorAddress = validatorManager._validatorAddress(aliceKeyMem); @@ -151,7 +158,7 @@ contract ValidatorManagerTest is Test { function testNonOwnerCannotRegisterValidator() public { bytes memory alicePublicKey = ALICE_UNCOMPRESSED; - vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, NON_OWNER)); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, NON_OWNER)); vm.prank(NON_OWNER); validatorManager.register(alicePublicKey, INITIAL_POWER); } @@ -192,7 +199,7 @@ contract ValidatorManagerTest is Test { bytes memory alicePublicKey = ALICE_UNCOMPRESSED; validatorManager.register(alicePublicKey, INITIAL_POWER); - vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, NON_OWNER)); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, NON_OWNER)); vm.prank(NON_OWNER); validatorManager.updatePower(aliceValidatorAddress, UPDATED_POWER); } @@ -252,7 +259,7 @@ contract ValidatorManagerTest is Test { bytes memory alicePublicKey = ALICE_UNCOMPRESSED; validatorManager.register(alicePublicKey, INITIAL_POWER); - vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, NON_OWNER)); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, NON_OWNER)); vm.prank(NON_OWNER); validatorManager.unregister(aliceValidatorAddress); } @@ -312,7 +319,7 @@ contract ValidatorManagerTest is Test { validatorManager.transferOwnership(NEW_OWNER); assertEq(validatorManager.owner(), NEW_OWNER); - vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, NON_OWNER)); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, NON_OWNER)); vm.prank(NON_OWNER); validatorManager.transferOwnership(address(0xBAD)); @@ -323,7 +330,7 @@ contract ValidatorManagerTest is Test { validatorManager.register(alicePublicKey, INITIAL_POWER); bytes memory bobPublicKey = BOB_COMPRESSED; - vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, address(this))); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, address(this))); validatorManager.register(bobPublicKey, SECOND_POWER); } @@ -332,10 +339,10 @@ contract ValidatorManagerTest is Test { assertEq(validatorManager.owner(), address(0)); bytes memory alicePublicKey = ALICE_UNCOMPRESSED; - vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, address(this))); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, address(this))); validatorManager.register(alicePublicKey, INITIAL_POWER); - vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, NON_OWNER)); + vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, NON_OWNER)); vm.prank(NON_OWNER); validatorManager.updatePower(aliceValidatorAddress, UPDATED_POWER); } diff --git a/solidity/test/ValidatorManagerUpgrade.t.sol b/solidity/test/ValidatorManagerUpgrade.t.sol new file mode 100644 index 00000000..a5b658bd --- /dev/null +++ b/solidity/test/ValidatorManagerUpgrade.t.sol @@ -0,0 +1,141 @@ +// SPDX-License-Identifier: Apache 2.0 +pragma solidity ^0.8.28; + +import {Test} from "forge-std/Test.sol"; +import {ValidatorManager} from "../src/ValidatorManager.sol"; +import {ValidatorManagerProxy} from "../src/ValidatorManagerProxy.sol"; +import {ValidatorManagerV2} from "./ValidatorManagerV2.sol"; +import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; + +contract ValidatorManagerUpgradeTest is Test { + ValidatorManager internal implementation; + ValidatorManager internal validatorManager; // proxy cast + ValidatorManagerProxy internal proxy; + + address internal constant NON_OWNER = address(0xBEEF); + address internal constant NEW_OWNER = address(0xCAFE); + + bytes constant ALICE_UNCOMPRESSED = + hex"048318535b54105d4a7aae60c08fc45f9687181b4fdfc625bd1a753fa7397fed753547f11ca8696646f2f3acb08e31016afac23e630c5d11f59f61fef57b0d2aa5"; + uint64 internal constant ALICE_POWER = 100; + + function setUp() public { + implementation = new ValidatorManager(); + bytes memory initData = abi.encodeCall(ValidatorManager.initialize, (address(this))); + proxy = new ValidatorManagerProxy(address(implementation), initData); + validatorManager = ValidatorManager(address(proxy)); + } + + // -- Upgrade authorization ------------------------------------------------ + + function testUpgradeByOwner() public { + // Register a validator before upgrade + validatorManager.register(ALICE_UNCOMPRESSED, ALICE_POWER); + + ValidatorManagerV2 v2Impl = new ValidatorManagerV2(); + validatorManager.upgradeToAndCall(address(v2Impl), ""); + + // State survives + assertEq(validatorManager.getValidatorCount(), 1); + assertEq(validatorManager.getTotalPower(), ALICE_POWER); + assertEq(validatorManager.owner(), address(this)); + + // V2 function accessible + assertEq(ValidatorManagerV2(address(proxy)).version(), 2); + } + + function testUpgradeRevertsForNonOwner() public { + ValidatorManagerV2 v2Impl = new ValidatorManagerV2(); + + vm.expectRevert( + abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, NON_OWNER) + ); + vm.prank(NON_OWNER); + validatorManager.upgradeToAndCall(address(v2Impl), ""); + } + + // -- State preservation --------------------------------------------------- + + function testStatePreservedAfterUpgrade() public { + // Register two validators + validatorManager.register(ALICE_UNCOMPRESSED, ALICE_POWER); + + ValidatorManager.Secp256k1Key memory aliceKey = + validatorManager._secp256k1KeyFromBytes(ALICE_UNCOMPRESSED); + address aliceAddr = validatorManager._validatorAddress(aliceKey); + + // Upgrade + ValidatorManagerV2 v2Impl = new ValidatorManagerV2(); + validatorManager.upgradeToAndCall(address(v2Impl), ""); + + // Verify all state + assertEq(validatorManager.owner(), address(this)); + assertEq(validatorManager.getValidatorCount(), 1); + assertEq(validatorManager.getTotalPower(), ALICE_POWER); + assertTrue(validatorManager.isValidator(aliceAddr)); + + ValidatorManager.ValidatorInfo memory info = validatorManager.getValidator(aliceAddr); + assertKeyEq(info.validatorKey, aliceKey); + assertEq(info.power, ALICE_POWER); + } + + // -- Double initialization protection ------------------------------------ + + function testCannotInitializeTwice() public { + vm.expectRevert(Initializable.InvalidInitialization.selector); + validatorManager.initialize(NON_OWNER); + } + + function testCannotInitializeImplementation() public { + vm.expectRevert(Initializable.InvalidInitialization.selector); + implementation.initialize(NON_OWNER); + } + + // -- Ownership transfer + upgrade ---------------------------------------- + + function testTransferOwnershipThenUpgrade() public { + validatorManager.transferOwnership(NEW_OWNER); + assertEq(validatorManager.owner(), NEW_OWNER); + + // Old owner can no longer upgrade + ValidatorManagerV2 v2Impl = new ValidatorManagerV2(); + vm.expectRevert( + abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, address(this)) + ); + validatorManager.upgradeToAndCall(address(v2Impl), ""); + + // New owner can upgrade + vm.prank(NEW_OWNER); + validatorManager.upgradeToAndCall(address(v2Impl), ""); + assertEq(ValidatorManagerV2(address(proxy)).version(), 2); + } + + function testRenounceOwnershipBlocksUpgrade() public { + validatorManager.renounceOwnership(); + assertEq(validatorManager.owner(), address(0)); + + ValidatorManagerV2 v2Impl = new ValidatorManagerV2(); + vm.expectRevert( + abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, address(this)) + ); + validatorManager.upgradeToAndCall(address(v2Impl), ""); + } + + // -- Helpers -------------------------------------------------------------- + + function keysEqual(ValidatorManager.Secp256k1Key memory a, ValidatorManager.Secp256k1Key memory b) + internal + pure + returns (bool) + { + return a.x == b.x && a.y == b.y; + } + + function assertKeyEq(ValidatorManager.Secp256k1Key memory actual, ValidatorManager.Secp256k1Key memory expected) + internal + pure + { + require(keysEqual(actual, expected), "validator key mismatch"); + } +} diff --git a/solidity/test/ValidatorManagerV2.sol b/solidity/test/ValidatorManagerV2.sol new file mode 100644 index 00000000..5cb1fe94 --- /dev/null +++ b/solidity/test/ValidatorManagerV2.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: Apache 2.0 +pragma solidity ^0.8.28; + +import {ValidatorManager} from "../src/ValidatorManager.sol"; + +/** + * @dev Minimal V2 mock used only for upgrade testing. + * Adds a `version()` view that returns 2. + */ +contract ValidatorManagerV2 is ValidatorManager { + function version() external pure override returns (uint256) { + return 2; + } +} diff --git a/utils/src/genesis.rs b/utils/src/genesis.rs index c3c5239f..c3d6a769 100644 --- a/utils/src/genesis.rs +++ b/utils/src/genesis.rs @@ -17,7 +17,9 @@ use malachitebft_eth_types::{ use tracing::debug; use crate::validator_manager::{ - generate_storage_data, Validator, ValidatorManager, GENESIS_VALIDATOR_MANAGER_ACCOUNT, + generate_impl_storage, generate_storage_data, patched_impl_bytecode, Validator, + ValidatorManagerProxy, GENESIS_VALIDATOR_MANAGER_ACCOUNT, + GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT, }; /// EIP-4788 Beacon Roots Contract address @@ -162,12 +164,29 @@ pub(crate) fn generate_evm_genesis( unreachable!("unable to determine PoA owner address"); }; - let storage = generate_storage_data(initial_validators, poa_address_owner)?; + // Proxy at 0x2000: ERC1967Proxy runtime code + all contract storage + let proxy_storage = generate_storage_data( + initial_validators, + poa_address_owner, + GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT, + )?; alloc.insert( GENESIS_VALIDATOR_MANAGER_ACCOUNT, GenesisAccount { - code: Some(ValidatorManager::DEPLOYED_BYTECODE.clone()), - storage: Some(storage), + code: Some(ValidatorManagerProxy::DEPLOYED_BYTECODE.clone()), + storage: Some(proxy_storage), + ..Default::default() + }, + ); + + // Implementation at 0x2001: ValidatorManager logic code + disabled initializer. + // The bytecode is patched to set the UUPS `__self` immutable to the impl address, + // since genesis alloc bypasses the constructor which normally sets it. + alloc.insert( + GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT, + GenesisAccount { + code: Some(patched_impl_bytecode(GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT)), + storage: Some(generate_impl_storage()), ..Default::default() }, ); diff --git a/utils/src/validator_manager/mod.rs b/utils/src/validator_manager/mod.rs index e0ddc811..33531961 100644 --- a/utils/src/validator_manager/mod.rs +++ b/utils/src/validator_manager/mod.rs @@ -11,20 +11,47 @@ pub mod types; use std::collections::{BTreeMap, HashSet}; -use alloy_primitives::{Address, B256, U256}; -pub use emerald_contracts::{ValidatorManager, GENESIS_VALIDATOR_MANAGER_ACCOUNT}; +use alloy_primitives::{Address, Bytes, B256, U256}; +pub use emerald_contracts::{ + ValidatorManager, ValidatorManagerProxy, GENESIS_VALIDATOR_MANAGER_ACCOUNT, + GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT, UUPS_SELF_IMMUTABLE_LENGTH, + UUPS_SELF_IMMUTABLE_OFFSETS, +}; pub use error::{Error as ValidatroManagerError, Result}; pub use storage::StorageSlotCalculator; pub use types::{Validator, ValidatorKey, ValidatorSet}; +/// Return ValidatorManager DEPLOYED_BYTECODE with the UUPS `__self` immutable +/// patched to `impl_address`. Required because genesis bypasses the constructor +/// which normally sets the immutable `__self = address(this)`. +pub fn patched_impl_bytecode(impl_address: Address) -> Bytes { + let mut code = ValidatorManager::DEPLOYED_BYTECODE.to_vec(); + let padded = impl_address.into_word(); + for &offset in UUPS_SELF_IMMUTABLE_OFFSETS { + assert!( + code[offset..offset + UUPS_SELF_IMMUTABLE_LENGTH] + .iter() + .all(|&b| b == 0), + "expected zero bytes at offset {offset}; UUPS_SELF_IMMUTABLE_OFFSETS may be stale" + ); + code[offset..offset + UUPS_SELF_IMMUTABLE_LENGTH].copy_from_slice(padded.as_slice()); + } + Bytes::from(code) +} + use crate::validator_manager::storage::{ - set_validator_addresses_set, set_validator_entries_mapping, + set_validator_addresses_set, set_validator_entries_mapping, EIP1967_IMPL_SLOT, + INITIALIZABLE_SLOT, OWNABLE_SLOT, REENTRANCY_GUARD_SLOT, }; -/// Generate storage slots and values for a given validator list +/// Generate proxy storage slots and values for a given validator list. +/// +/// The returned map is intended for the **proxy** account at `0x2000`. +/// It contains EIP-1967 + ERC-7201 slots plus the contract's own state. pub fn generate_storage_data( validators: Vec, owner: Address, + impl_address: Address, ) -> Result> { // Validate validators if validators.is_empty() { @@ -54,38 +81,55 @@ pub fn generate_storage_data( } // Generate storage data - generate_from_validator_set(&validator_set, owner) + generate_from_validator_set(&validator_set, owner, impl_address) } -/// Generate storage data from validator set +/// Generate proxy storage from a validator set. +/// +/// Storage layout (UUPS upgradeable, OZ 5.x with ERC-7201 namespaced storage): +/// +/// EIP-1967 impl slot : implementation address +/// ERC-7201 Ownable : _owner +/// ERC-7201 ReentrancyGuard : _status = 1 (NOT_ENTERED) +/// ERC-7201 Initializable : _initialized = 1, _initializing = false +/// Slot 0 : _validatorAddresses._inner._values (EnumerableSet) +/// Slot 1 : _validatorAddresses._inner._positions +/// Slot 2 : _validators mapping(address => ValidatorInfo) +/// Slot 3 : _totalPower pub fn generate_from_validator_set( validator_set: &ValidatorSet, owner: Address, + impl_address: Address, ) -> Result> { - // Storage layout for ValidatorManager contract: - // Slot 0: Ownable._owner (set separately by deployment or genesis tooling) - // Slot 1: ReentrancyGuard._status (set to 1) - // Slot 2: _validatorAddresses._values (EnumerableSet internal storage) - // Slot 3: _validatorAddresses._positions - // Slot 4: _validators mapping(address => ValidatorInfo) - // Slot 5: _totalPower - let mut storage = BTreeMap::new(); - // Ownable owner - storage.insert(B256::ZERO, owner.into_word()); + // -- EIP-1967: proxy implementation pointer -- + storage.insert(EIP1967_IMPL_SLOT, impl_address.into_word()); + + // -- ERC-7201: OwnableUpgradeable._owner -- + storage.insert(OWNABLE_SLOT, owner.into_word()); - // ReentrancyGuard initial status (_status = 1) at slot 1 - let status_slot = B256::from(U256::from(1u64).to_be_bytes::<32>()); + // -- ERC-7201: ReentrancyGuardUpgradeable._status = 1 (NOT_ENTERED) -- storage.insert( - status_slot, + REENTRANCY_GUARD_SLOT, B256::from(U256::from(1u64).to_be_bytes::<32>()), ); - set_validator_addresses_set(&mut storage, validator_set, U256::from(2))?; // _validatorAddresses base at slot 2 - set_validator_entries_mapping(&mut storage, validator_set, U256::from(4))?; // _validators mapping at slot 4 + // -- ERC-7201: Initializable._initialized = 1, _initializing = false -- + // Both fields are packed into a single slot: uint64 _initialized || bool _initializing + // _initialized = 1 occupies the low 8 bytes; _initializing = false is already zero. + storage.insert( + INITIALIZABLE_SLOT, + B256::from(U256::from(1u64).to_be_bytes::<32>()), + ); - let total_power_slot = B256::from(U256::from(5u64).to_be_bytes::<32>()); // _totalPower at slot 5 + // -- Contract state: sequential slots starting at 0 -- + // Slot 0-1: _validatorAddresses (EnumerableSet, occupies 2 slots) + set_validator_addresses_set(&mut storage, validator_set, U256::from(0))?; + // Slot 2: _validators mapping + set_validator_entries_mapping(&mut storage, validator_set, U256::from(2))?; + // Slot 3: _totalPower + let total_power_slot = B256::from(U256::from(3u64).to_be_bytes::<32>()); let total_power = validator_set.total_power()?; storage.insert( total_power_slot, @@ -94,3 +138,19 @@ pub fn generate_from_validator_set( Ok(storage) } + +/// Generate storage for the **implementation** account at genesis. +/// +/// The only thing needed is to disable initializers so nobody can call +/// `initialize()` on the bare implementation. This mimics what +/// `_disableInitializers()` does in the constructor (which doesn't run +/// during genesis alloc). +pub fn generate_impl_storage() -> BTreeMap { + let mut storage = BTreeMap::new(); + // _initialized = type(uint64).max = 0xffffffffffffffff + storage.insert( + INITIALIZABLE_SLOT, + B256::from(U256::from(u64::MAX).to_be_bytes::<32>()), + ); + storage +} diff --git a/utils/src/validator_manager/storage.rs b/utils/src/validator_manager/storage.rs index 9acfbc45..10fa4f44 100644 --- a/utils/src/validator_manager/storage.rs +++ b/utils/src/validator_manager/storage.rs @@ -1,12 +1,69 @@ -//! Storage layout and encoding for ValidatorManager contract +//! Storage layout and encoding for the upgradeable ValidatorManager contract. +//! +//! The proxy (ERC1967) sits at `0x2000`; its storage contains: +//! - EIP-1967 implementation slot pointing to the logic contract +//! - ERC-7201 namespaced slots for OwnableUpgradeable, ReentrancyGuardUpgradeable, +//! and Initializable +//! - Sequential slots 0..3 for the contract's own state variables use std::collections::BTreeMap; -use alloy_primitives::{keccak256, Address, B256, U256}; +use alloy_primitives::{hex, keccak256, Address, B256, U256}; use crate::validator_manager::error::Result; use crate::validator_manager::types::{ValidatorKey, ValidatorSet}; +// --------------------------------------------------------------------------- +// ERC-7201 namespace slots (computed & pinned against OZ 5.4.0 source) +// Formula: keccak256(abi.encode(uint256(keccak256(id)) - 1)) & ~bytes32(uint256(0xff)) +// --------------------------------------------------------------------------- + +/// Namespace identifier for OwnableUpgradeable storage. +pub const OWNABLE_NAMESPACE: &str = "openzeppelin.storage.Ownable"; + +/// Pre-computed ERC-7201 slot for OwnableUpgradeable (OZ 5.4.0). +pub const OWNABLE_SLOT: B256 = + B256::new(hex!("9016d09d72d40fdae2fd8ceac6b6234c7706214fd39c1cd1e609a0528c199300")); + +/// Namespace identifier for ReentrancyGuardUpgradeable storage. +pub const REENTRANCY_GUARD_NAMESPACE: &str = "openzeppelin.storage.ReentrancyGuard"; + +/// Pre-computed ERC-7201 slot for ReentrancyGuardUpgradeable (OZ 5.4.0). +pub const REENTRANCY_GUARD_SLOT: B256 = + B256::new(hex!("9b779b17422d0df92223018b32b4d1fa46e071723d6817e2486d003becc55f00")); + +/// Namespace identifier for Initializable storage. +pub const INITIALIZABLE_NAMESPACE: &str = "openzeppelin.storage.Initializable"; + +/// Pre-computed ERC-7201 slot for Initializable (OZ 5.4.0). +pub const INITIALIZABLE_SLOT: B256 = + B256::new(hex!("f0c57e16840df040f15088dc2f81fe391c3923bec73e23a9662efc9c229c6a00")); + +// --------------------------------------------------------------------------- +// EIP-1967 proxy slots +// --------------------------------------------------------------------------- + +/// EIP-1967 implementation slot: `bytes32(uint256(keccak256("eip1967.proxy.implementation")) - 1)` +pub const EIP1967_IMPL_SLOT: B256 = + B256::new(hex!("360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc")); + +// --------------------------------------------------------------------------- +// ERC-7201 computation +// --------------------------------------------------------------------------- + +/// Compute the ERC-7201 storage slot for a given namespace id string. +/// +/// `keccak256(abi.encode(uint256(keccak256(id)) - 1)) & ~bytes32(uint256(0xff))` +pub fn erc7201_slot(namespace: &str) -> B256 { + let inner_hash = keccak256(namespace.as_bytes()); + let inner_u256 = U256::from_be_bytes(inner_hash.0) - U256::from(1u64); + let outer_hash = keccak256(inner_u256.to_be_bytes::<32>()); + let outer_u256 = U256::from_be_bytes(outer_hash.0); + // Mask: clear the lowest byte (& ~0xff) + let masked = outer_u256 & !(U256::from(0xffu64)); + B256::from(masked.to_be_bytes::<32>()) +} + /// Storage slot calculator for Solidity mappings and arrays pub struct StorageSlotCalculator; diff --git a/utils/src/validator_manager/tests.rs b/utils/src/validator_manager/tests.rs index 78075575..86d67374 100644 --- a/utils/src/validator_manager/tests.rs +++ b/utils/src/validator_manager/tests.rs @@ -1,23 +1,30 @@ use core::str::FromStr; +use std::{fs, path::Path}; +use alloy_genesis::Genesis; use alloy_network::EthereumWallet; use alloy_node_bindings::anvil::Anvil; -use alloy_primitives::{address, Address, U256}; +use alloy_primitives::{address, Address, Bytes, U256}; use alloy_provider::{Provider, ProviderBuilder}; use alloy_signer_local::coins_bip39::English; use alloy_signer_local::{MnemonicBuilder, PrivateKeySigner}; +use alloy_sol_types::SolCall; use color_eyre::eyre; use reqwest::Url; +use tempfile::tempdir; use tracing::debug; -use super::{generate_storage_data, Validator}; -use crate::validator_manager::ValidatorManager; +use super::{ + generate_impl_storage, generate_storage_data, patched_impl_bytecode, Validator, + ValidatorManager, ValidatorManagerProxy, GENESIS_VALIDATOR_MANAGER_ACCOUNT, + GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT, +}; +use crate::genesis::generate_evm_genesis; +use crate::validator_manager::storage::{ + erc7201_slot, EIP1967_IMPL_SLOT, INITIALIZABLE_NAMESPACE, INITIALIZABLE_SLOT, + OWNABLE_NAMESPACE, OWNABLE_SLOT, REENTRANCY_GUARD_NAMESPACE, REENTRANCY_GUARD_SLOT, +}; -/// Generate validators from "test test ... junk" mnemonic using sequential derivation paths. -/// -/// Each validator is derived from path `m/44'/60'/0'/0/{index}` and includes both -/// the validator metadata and the associated ECDSA signing key required to submit -/// transactions on behalf of that validator. const TEST_OWNER_ADDRESS: Address = address!("0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65"); const TEST_OWNER_PRIVATE_KEY: &str = "0x47e179ec197488593b187f80a00eb0da91f1b9d0b13f8733639f19c30a34926a"; @@ -55,53 +62,307 @@ fn generate_validators_from_mnemonic(count: usize) -> eyre::Result eyre::Result<()> { + let mut content = String::new(); + for validator in validators { + let (x, y) = validator.validator_key; + let mut raw = [0u8; 64]; + raw[..32].copy_from_slice(&x.to_be_bytes::<32>()); + raw[32..].copy_from_slice(&y.to_be_bytes::<32>()); + content.push_str(&hex::encode(raw)); + content.push('\n'); + } + fs::write(path, content)?; + Ok(()) +} + +fn with_genesis_power(validators: &[Validator], power: u64) -> Vec { + validators + .iter() + .map(|v| Validator::from_public_key(v.validator_key, power)) + .collect() +} + +// --------------------------------------------------------------------------- +// Golden tests: verify pinned ERC-7201 slot constants against the formula +// --------------------------------------------------------------------------- + +#[test] +fn test_erc7201_ownable_slot() { + assert_eq!(erc7201_slot(OWNABLE_NAMESPACE), OWNABLE_SLOT); +} + +#[test] +fn test_erc7201_reentrancy_guard_slot() { + assert_eq!( + erc7201_slot(REENTRANCY_GUARD_NAMESPACE), + REENTRANCY_GUARD_SLOT + ); +} + +#[test] +fn test_erc7201_initializable_slot() { + assert_eq!(erc7201_slot(INITIALIZABLE_NAMESPACE), INITIALIZABLE_SLOT); +} + +#[test] +fn test_generate_evm_genesis_alloc_matches_expected_storage() -> eyre::Result<()> { + let tmp = tempdir()?; + let keys_path = tmp.path().join("validator_keys.txt"); + let genesis_path = tmp.path().join("genesis.json"); + + let validators = generate_validators_from_mnemonic(5)?; + write_validator_keys_file(&validators, &keys_path)?; + + let owner = Some(format!("{TEST_OWNER_ADDRESS:#x}")); + let testnet = false; + let testnet_balance = 0u64; + let chain_id = 12345u64; + generate_evm_genesis( + keys_path + .to_str() + .ok_or_else(|| eyre::eyre!("validator keys path is not UTF-8"))?, + &owner, + &testnet, + &testnet_balance, + &chain_id, + genesis_path + .to_str() + .ok_or_else(|| eyre::eyre!("genesis path is not UTF-8"))?, + )?; + + let genesis: Genesis = serde_json::from_slice(&fs::read(&genesis_path)?)?; + + let proxy_account = genesis + .alloc + .get(&GENESIS_VALIDATOR_MANAGER_ACCOUNT) + .ok_or_else(|| eyre::eyre!("missing proxy alloc entry at 0x2000"))?; + let impl_account = genesis + .alloc + .get(&GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT) + .ok_or_else(|| eyre::eyre!("missing implementation alloc entry at 0x2001"))?; + + assert_eq!( + proxy_account.code.as_ref(), + Some(&ValidatorManagerProxy::DEPLOYED_BYTECODE) + ); + let expected_impl_bytecode = patched_impl_bytecode(GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT); + assert_eq!( + impl_account.code.as_ref(), + Some(&expected_impl_bytecode) + ); + + let expected_proxy_storage = generate_storage_data( + with_genesis_power(&validators, 100), + TEST_OWNER_ADDRESS, + GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT, + )?; + let expected_impl_storage = generate_impl_storage(); + + assert_eq!(proxy_account.storage.as_ref(), Some(&expected_proxy_storage)); + assert_eq!(impl_account.storage.as_ref(), Some(&expected_impl_storage)); + + assert_eq!( + proxy_account + .storage + .as_ref() + .and_then(|s| s.get(&EIP1967_IMPL_SLOT)), + Some(&GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT.into_word()) + ); + assert_eq!( + impl_account + .storage + .as_ref() + .and_then(|s| s.get(&INITIALIZABLE_SLOT)), + expected_impl_storage.get(&INITIALIZABLE_SLOT) + ); + + Ok(()) +} + #[tokio::test] #[test_log::test] -async fn test_anvil_storage_comparison() -> eyre::Result<()> { - let anvil = Anvil::new().spawn(); +async fn test_anvil_boot_from_generated_genesis_proxy_and_impl_behavior() -> eyre::Result<()> { + let tmp = tempdir()?; + let keys_path = tmp.path().join("validator_keys.txt"); + let genesis_path = tmp.path().join("genesis.json"); + + let validators = generate_validators_from_mnemonic(5)?; + write_validator_keys_file(&validators, &keys_path)?; + + let owner = Some(format!("{TEST_OWNER_ADDRESS:#x}")); + let testnet = false; + let testnet_balance = 0u64; + let chain_id = 12345u64; + generate_evm_genesis( + keys_path + .to_str() + .ok_or_else(|| eyre::eyre!("validator keys path is not UTF-8"))?, + &owner, + &testnet, + &testnet_balance, + &chain_id, + genesis_path + .to_str() + .ok_or_else(|| eyre::eyre!("genesis path is not UTF-8"))?, + )?; + + let genesis_path_str = genesis_path + .to_str() + .ok_or_else(|| eyre::eyre!("genesis path is not UTF-8"))?; + let anvil = Anvil::new().args(["--init", genesis_path_str]).spawn(); let rpc_url: Url = anvil.endpoint().parse()?; + let provider = ProviderBuilder::new().connect_http(rpc_url); - debug!("🚀 Starting Anvil storage comparison test"); + let vm_proxy = ValidatorManager::new(GENESIS_VALIDATOR_MANAGER_ACCOUNT, &provider); + assert_eq!(vm_proxy.owner().call().await?, TEST_OWNER_ADDRESS); + assert_eq!(vm_proxy.getValidatorCount().call().await?, U256::from(5)); + assert_eq!(vm_proxy.getTotalPower().call().await?, 500u64); + assert_eq!(vm_proxy.getValidators().call().await?.len(), 5); - // Generate validators from mnemonic with sequential derivation paths - let validators = generate_validators_from_mnemonic(5)?; - debug!("✅ Generated {} validators from mnemonic", validators.len()); - for (i, validator) in validators.iter().enumerate() { - debug!( - " Validator {} key: ({:#x}, {:#x})", - i, validator.validator_key.0, validator.validator_key.1 - ); - } + let impl_slot = provider + .get_storage_at(GENESIS_VALIDATOR_MANAGER_ACCOUNT, EIP1967_IMPL_SLOT.into()) + .await?; + assert_eq!( + impl_slot.to_be_bytes::<32>(), + GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT.into_word() + ); - let expected_storage = generate_storage_data(validators.clone(), TEST_OWNER_ADDRESS)?; - debug!( - "✅ Generated {} expected storage slots", - expected_storage.len() + let vm_impl = ValidatorManager::new(GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT, &provider); + + // proxiableUUID has a `notDelegated` guard so it must be called directly on the + // implementation, not through the proxy. It verifies __self == address(this), which + // requires the patched bytecode to have the correct implementation address. + let uuid = vm_impl.proxiableUUID().call().await?; + assert_eq!( + uuid, + EIP1967_IMPL_SLOT, + "proxiableUUID on implementation should return the EIP-1967 implementation slot" + ); + let init_result = vm_impl.initialize(TEST_OWNER_ADDRESS).call().await; + assert!( + init_result.is_err(), + "implementation initialize should revert when initializers are disabled" ); - // Deploy contract and register validators on Anvil - let contract_address = - deploy_and_register_validators(&validators, TEST_OWNER_ADDRESS, &rpc_url).await?; - debug!( - "✅ Contract deployed and validators registered at: {:#x}", - contract_address + Ok(()) +} + +#[tokio::test] +#[test_log::test] +async fn test_anvil_boot_from_generated_genesis_upgrade_succeeds() +-> eyre::Result<()> { + let tmp = tempdir()?; + let keys_path = tmp.path().join("validator_keys.txt"); + let genesis_path = tmp.path().join("genesis.json"); + + let validators = generate_validators_from_mnemonic(5)?; + write_validator_keys_file(&validators, &keys_path)?; + + let owner = Some(format!("{TEST_OWNER_ADDRESS:#x}")); + let testnet = false; + let testnet_balance = 0u64; + let chain_id = 12345u64; + generate_evm_genesis( + keys_path + .to_str() + .ok_or_else(|| eyre::eyre!("validator keys path is not UTF-8"))?, + &owner, + &testnet, + &testnet_balance, + &chain_id, + genesis_path + .to_str() + .ok_or_else(|| eyre::eyre!("genesis path is not UTF-8"))?, + )?; + + let genesis_path_str = genesis_path + .to_str() + .ok_or_else(|| eyre::eyre!("genesis path is not UTF-8"))?; + let anvil = Anvil::new().args(["--init", genesis_path_str]).spawn(); + let rpc_url: Url = anvil.endpoint().parse()?; + + // Owner wallet provider (same key as genesis owner) so the upgrade path + // reaches UUPS call-context checks instead of failing onlyOwner(). + let owner_key = PrivateKeySigner::from_str(TEST_OWNER_PRIVATE_KEY)?; + debug_assert_eq!(owner_key.address(), TEST_OWNER_ADDRESS); + let owner_provider = ProviderBuilder::new() + .wallet(EthereumWallet::from(owner_key)) + .connect_http(rpc_url); + + // Sanity check: proxy is initialized and owned by the expected account. + let vm_proxy = ValidatorManager::new(GENESIS_VALIDATOR_MANAGER_ACCOUNT, &owner_provider); + assert_eq!(vm_proxy.owner().call().await?, TEST_OWNER_ADDRESS); + + // Deploy a new UUPS-compatible implementation. + let new_impl = ValidatorManager::deploy(owner_provider.clone()).await?; + let new_impl_address = *new_impl.address(); + + // Capture implementation slot before attempting the upgrade. + let impl_before = owner_provider + .get_storage_at(GENESIS_VALIDATOR_MANAGER_ACCOUNT, EIP1967_IMPL_SLOT.into()) + .await?; + assert_eq!( + impl_before.to_be_bytes::<32>(), + GENESIS_VALIDATOR_MANAGER_IMPL_ACCOUNT.into_word() ); - let provider = ProviderBuilder::new().connect_http(rpc_url.clone()); + // Expected behavior: upgrade should succeed when called by owner. + let receipt = vm_proxy + .upgradeToAndCall(new_impl_address, Bytes::new()) + .send() + .await? + .get_receipt() + .await?; + assert!(receipt.status(), "upgrade transaction should succeed"); - // Basic storage check - just verify non-empty storage exists - let zero_slot = provider - .get_storage_at(contract_address, U256::ZERO) + // Implementation pointer should be updated to the new implementation. + let impl_after = owner_provider + .get_storage_at(GENESIS_VALIDATOR_MANAGER_ACCOUNT, EIP1967_IMPL_SLOT.into()) .await?; - debug!("✅ Storage at slot 0: {}", zero_slot); + assert_ne!(impl_after, impl_before); + assert_eq!(impl_after.to_be_bytes::<32>(), new_impl_address.into_word()); + + // Proxy state should remain intact after upgrade. + assert_eq!(vm_proxy.owner().call().await?, TEST_OWNER_ADDRESS); + assert_eq!(vm_proxy.getValidatorCount().call().await?, U256::from(5)); + assert_eq!(vm_proxy.getTotalPower().call().await?, 500u64); + + Ok(()) +} + +// --------------------------------------------------------------------------- +// Anvil integration: deploy behind proxy, compare storage +// --------------------------------------------------------------------------- + +/// Deploy ValidatorManager behind an ERC1967 proxy on Anvil and compare +/// proxy storage against `generate_storage_data`. +#[tokio::test] +#[test_log::test] +async fn test_anvil_storage_comparison() -> eyre::Result<()> { + let anvil = Anvil::new().spawn(); + let rpc_url: Url = anvil.endpoint().parse()?; + + debug!("Starting Anvil storage comparison test"); + + let validators = generate_validators_from_mnemonic(5)?; + debug!("Generated {} validators from mnemonic", validators.len()); + + // Deploy implementation + proxy via transactions (normal deploy path) + let (proxy_address, impl_address) = + deploy_proxy_and_register_validators(&validators, TEST_OWNER_ADDRESS, &rpc_url).await?; + debug!("Proxy at {proxy_address:#x}, impl at {impl_address:#x}"); + + // Generate expected storage (same function genesis uses) + let expected_storage = + generate_storage_data(validators.clone(), TEST_OWNER_ADDRESS, impl_address)?; + debug!("Generated {} expected storage slots", expected_storage.len()); + + let provider = ProviderBuilder::new().connect_http(rpc_url.clone()); for (slot, expected_value) in expected_storage.iter() { let actual_value = provider - .get_storage_at(contract_address, (*slot).into()) + .get_storage_at(proxy_address, (*slot).into()) .await?; assert_eq!( actual_value.to_be_bytes::<32>(), @@ -110,20 +371,19 @@ async fn test_anvil_storage_comparison() -> eyre::Result<()> { ); } - debug!("🎉 Anvil integration test completed successfully!"); - debug!(" Contract deployed and all storage slots match expected values."); debug!( - " Expected {} storage slots verified.", + "Anvil storage comparison passed: {} slots verified.", expected_storage.len() ); Ok(()) } -async fn deploy_and_register_validators( +/// Deploy implementation, proxy(impl, initData), then register validators. +async fn deploy_proxy_and_register_validators( validators: &[Validator], owner: Address, rpc_endpoint: &Url, -) -> eyre::Result
{ +) -> eyre::Result<(Address, Address)> { let deployer_key = PrivateKeySigner::from_str(TEST_OWNER_PRIVATE_KEY)?; debug_assert_eq!(deployer_key.address(), owner); let deployer_wallet = EthereumWallet::from(deployer_key); @@ -132,40 +392,40 @@ async fn deploy_and_register_validators( .wallet(deployer_wallet) .connect_http(rpc_endpoint.clone()); - // Deploy the contract using the generated bindings - let deployed_contract = ValidatorManager::deploy(deployer_provider.clone()).await?; - let contract_address = *deployed_contract.address(); - - debug!( - "✅ Deployed ValidatorManager contract at: {:#x}", - contract_address - ); + // 1. Deploy implementation + let impl_contract = ValidatorManager::deploy(deployer_provider.clone()).await?; + let impl_address = *impl_contract.address(); + debug!("Deployed implementation at {impl_address:#x}"); - // check bytecode exists at address - let provider = ProviderBuilder::new().connect_http(rpc_endpoint.clone()); - let code = provider.get_code_at(contract_address).await?; - - // assert bytecode matches - assert_eq!(code, ValidatorManager::DEPLOYED_BYTECODE); - - debug!("✅ Contract verified to have bytecode"); + // 2. Deploy proxy with initialize calldata + let init_data = ValidatorManager::initializeCall { + initialOwner: owner, + } + .abi_encode(); + let proxy_contract = ValidatorManagerProxy::deploy( + deployer_provider.clone(), + impl_address, + Bytes::from(init_data), + ) + .await?; + let proxy_address = *proxy_contract.address(); + debug!("Deployed proxy at {proxy_address:#x}"); - let owner_contract = ValidatorManager::new(contract_address, deployer_provider.clone()); + // 3. Register validators through the proxy + let vm = ValidatorManager::new(proxy_address, deployer_provider.clone()); for (i, validator) in validators.iter().enumerate() { let info: ValidatorManager::ValidatorInfo = validator.clone().into(); - - // Encode the public key as uncompressed format (65 bytes: 0x04 + x + y) let mut pubkey_bytes = Vec::with_capacity(65); pubkey_bytes.push(0x04); pubkey_bytes.extend_from_slice(&info.validatorKey.x.to_be_bytes::<32>()); pubkey_bytes.extend_from_slice(&info.validatorKey.y.to_be_bytes::<32>()); - let pending_tx = owner_contract + let receipt = vm .register(pubkey_bytes.into(), info.power) .send() + .await? + .get_receipt() .await?; - - let receipt = pending_tx.get_receipt().await?; if !receipt.status() { return Err(eyre::anyhow!( "Failed to register validator {}: ({:#x}, {:#x})", @@ -176,8 +436,8 @@ async fn deploy_and_register_validators( } } - let onchain_total_power = owner_contract.getTotalPower().call().await?; - debug!("✅ On-chain total power after registration: {onchain_total_power:?}",); + let total_power = vm.getTotalPower().call().await?; + debug!("On-chain total power: {total_power:?}"); - Ok(contract_address) + Ok((proxy_address, impl_address)) }