From 25f43ea892c2a21c918f9c743dc716d80f7ae9a6 Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Sun, 1 Feb 2026 15:02:31 -0500 Subject: [PATCH 01/13] feat: Add direct metadata editing for org admins - Add updateOrgMetaAsAdmin function for admin hat wearers to edit metadata directly - Add setOrgAdminHat/getOrgAdminHat to configure per-org admin hats - Add setHatsProtocol/getHatsProtocol for Hats Protocol integration - Falls back to topHat if no admin hat is configured - Add IHats interface for checking hat wearers - Add NotOrgAdmin custom error - Add OrgAdminHatSet and HatsProtocolSet events This enables org admins (topHat or configured admin hat wearers) to update organization metadata (name, description, logo, links) without going through the governance proposal flow. Co-Authored-By: Claude Opus 4.5 --- src/OrgRegistry.sol | 83 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/OrgRegistry.sol b/src/OrgRegistry.sol index 58390b6..f787c2c 100644 --- a/src/OrgRegistry.sol +++ b/src/OrgRegistry.sol @@ -5,6 +5,11 @@ import "@openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable. import "@openzeppelin-contracts-upgradeable/contracts/proxy/utils/Initializable.sol"; import {ValidationLib} from "./libs/ValidationLib.sol"; +/* ─────────── Hats Protocol Interface ─────────── */ +interface IHats { + function isWearerOfHat(address account, uint256 hatId) external view returns (bool); +} + /* ─────────── Custom errors ─────────── */ error InvalidParam(); error OrgExists(); @@ -12,6 +17,7 @@ error OrgUnknown(); error TypeTaken(); error ContractUnknown(); error NotOrgExecutor(); +error NotOrgAdmin(); error OwnerOnlyDuringBootstrap(); // deployer tried after bootstrap error AutoUpgradeRequired(); // deployer must set autoUpgrade=true @@ -59,6 +65,9 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { mapping(bytes32 => mapping(uint256 => uint256)) roleHatOf; // orgId => roleIndex => hatId bytes32[] orgIds; uint256 totalContracts; + // New storage for admin hat feature + mapping(bytes32 => uint256) adminHatOf; // orgId => admin hatId for direct metadata editing + address hatsProtocol; // Hats Protocol contract address } // keccak256("poa.orgregistry.storage") to get a unique, collision-free slot @@ -84,6 +93,8 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { ); event AutoUpgradeSet(bytes32 indexed contractId, bool enabled); event HatsTreeRegistered(bytes32 indexed orgId, uint256 topHatId, uint256[] roleHatIds); + event OrgAdminHatSet(bytes32 indexed orgId, uint256 hatId); + event HatsProtocolSet(address hatsProtocol); /// @custom:oz-upgrades-unsafe-allow constructor constructor() initializer {} @@ -97,6 +108,23 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { __Ownable_init(initialOwner); } + /** + * @dev Sets the Hats Protocol contract address + * @param _hats The Hats Protocol contract address + */ + function setHatsProtocol(address _hats) external onlyOwner { + if (_hats == address(0)) revert InvalidParam(); + _layout().hatsProtocol = _hats; + emit HatsProtocolSet(_hats); + } + + /** + * @dev Gets the Hats Protocol contract address + */ + function getHatsProtocol() external view returns (address) { + return _layout().hatsProtocol; + } + /* ═════════════════ ORG LOGIC ═════════════════ */ function registerOrg(bytes32 orgId, address executorAddr, bytes calldata name, bytes32 metadataHash) external @@ -157,6 +185,12 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { o.executor = executorAddr; } + /** + * @dev Updates org metadata (governance path - only executor) + * @param orgId The organization ID + * @param newName New organization name (bytes, validated by ValidationLib) + * @param newMetadataHash New IPFS metadata hash (bytes32) + */ function updateOrgMeta(bytes32 orgId, bytes calldata newName, bytes32 newMetadataHash) external { ValidationLib.requireValidTitle(newName); Layout storage l = _layout(); @@ -167,6 +201,55 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { emit MetaUpdated(orgId, newName, newMetadataHash); } + /** + * @dev Allows an admin hat wearer to update org metadata directly (no governance) + * @param orgId The organization ID + * @param newName New organization name (bytes, validated by ValidationLib) + * @param newMetadataHash New IPFS metadata hash (bytes32) + */ + function updateOrgMetaAsAdmin(bytes32 orgId, bytes calldata newName, bytes32 newMetadataHash) external { + ValidationLib.requireValidTitle(newName); + Layout storage l = _layout(); + OrgInfo storage o = l.orgOf[orgId]; + if (!o.exists) revert OrgUnknown(); + + // Check if caller wears the org's admin hat + uint256 adminHat = l.adminHatOf[orgId]; + if (adminHat == 0) { + // No admin hat configured, fall back to topHat + adminHat = l.topHatOf[orgId]; + } + if (adminHat == 0) revert NotOrgAdmin(); + + address hats = l.hatsProtocol; + if (hats == address(0)) revert InvalidParam(); + if (!IHats(hats).isWearerOfHat(msg.sender, adminHat)) revert NotOrgAdmin(); + + emit MetaUpdated(orgId, newName, newMetadataHash); + } + + /** + * @dev Set the admin hat for an org (only executor can do this) + * @param orgId The organization ID + * @param hatId The hat ID that can edit metadata directly (0 to use topHat) + */ + function setOrgAdminHat(bytes32 orgId, uint256 hatId) external { + Layout storage l = _layout(); + OrgInfo storage o = l.orgOf[orgId]; + if (!o.exists) revert OrgUnknown(); + if (msg.sender != o.executor) revert NotOrgExecutor(); + + l.adminHatOf[orgId] = hatId; + emit OrgAdminHatSet(orgId, hatId); + } + + /** + * @dev Get the admin hat for an org + */ + function getOrgAdminHat(bytes32 orgId) external view returns (uint256) { + return _layout().adminHatOf[orgId]; + } + /* ══════════ CONTRACT REGISTRATION ══════════ */ /** * ‑ During **bootstrap** (`o.bootstrap == true`) the registry owner _may_ From a963de4380dba8d51e0c418e0825ba772e4c14ad Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Sun, 1 Feb 2026 21:09:15 -0500 Subject: [PATCH 02/13] fix: Rename IHats to IHatsMinimal to avoid collision The hats-protocol library already defines an IHats interface in lib/hats-protocol/src/Interfaces/IHats.sol. When OrgRegistry.sol is imported alongside files that import the full IHats, the compiler throws an "Identifier already declared" error. Renamed our minimal interface to IHatsMinimal to avoid the collision. Co-Authored-By: Claude Opus 4.5 --- src/OrgRegistry.sol | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/OrgRegistry.sol b/src/OrgRegistry.sol index f787c2c..a4cda8a 100644 --- a/src/OrgRegistry.sol +++ b/src/OrgRegistry.sol @@ -5,8 +5,10 @@ import "@openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable. import "@openzeppelin-contracts-upgradeable/contracts/proxy/utils/Initializable.sol"; import {ValidationLib} from "./libs/ValidationLib.sol"; -/* ─────────── Hats Protocol Interface ─────────── */ -interface IHats { +/* ─────────── Minimal Hats Protocol Interface ─────────── */ +/// @dev Minimal interface for Hats Protocol - only what we need for admin checks +/// Using a distinct name to avoid collision with the full IHats in lib/hats-protocol +interface IHatsMinimal { function isWearerOfHat(address account, uint256 hatId) external view returns (bool); } @@ -223,7 +225,7 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { address hats = l.hatsProtocol; if (hats == address(0)) revert InvalidParam(); - if (!IHats(hats).isWearerOfHat(msg.sender, adminHat)) revert NotOrgAdmin(); + if (!IHatsMinimal(hats).isWearerOfHat(msg.sender, adminHat)) revert NotOrgAdmin(); emit MetaUpdated(orgId, newName, newMetadataHash); } From 810d57c62b9f1eb459e0ce2724885963ee491d0e Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Sun, 1 Feb 2026 21:20:09 -0500 Subject: [PATCH 03/13] refactor: Import IHats from hats-protocol instead of defining minimal interface Removes the custom IHatsMinimal interface and imports the actual IHats interface from the hats-protocol library. This is the proper approach: - Reuses the existing, well-tested interface - Follows the pattern used elsewhere in the codebase - Avoids interface duplication and potential inconsistencies Co-Authored-By: Claude Opus 4.5 --- src/OrgRegistry.sol | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/OrgRegistry.sol b/src/OrgRegistry.sol index a4cda8a..c1afba4 100644 --- a/src/OrgRegistry.sol +++ b/src/OrgRegistry.sol @@ -4,13 +4,7 @@ pragma solidity ^0.8.20; import "@openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; import "@openzeppelin-contracts-upgradeable/contracts/proxy/utils/Initializable.sol"; import {ValidationLib} from "./libs/ValidationLib.sol"; - -/* ─────────── Minimal Hats Protocol Interface ─────────── */ -/// @dev Minimal interface for Hats Protocol - only what we need for admin checks -/// Using a distinct name to avoid collision with the full IHats in lib/hats-protocol -interface IHatsMinimal { - function isWearerOfHat(address account, uint256 hatId) external view returns (bool); -} +import {IHats} from "@hats-protocol/src/Interfaces/IHats.sol"; /* ─────────── Custom errors ─────────── */ error InvalidParam(); @@ -225,7 +219,7 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { address hats = l.hatsProtocol; if (hats == address(0)) revert InvalidParam(); - if (!IHatsMinimal(hats).isWearerOfHat(msg.sender, adminHat)) revert NotOrgAdmin(); + if (!IHats(hats).isWearerOfHat(msg.sender, adminHat)) revert NotOrgAdmin(); emit MetaUpdated(orgId, newName, newMetadataHash); } From a3990cfcca7a67acc8d60e18e5a689fedf01cae0 Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Sun, 1 Feb 2026 21:34:32 -0500 Subject: [PATCH 04/13] refactor: Simplify metadata admin feature - Remove setHatsProtocol/getHatsProtocol - hats address passed as param instead - Rename to OrgMetadataAdmin to clarify scope (metadata editing only, not full admin) - metadataAdminHatOf is optional - if 0, falls back to topHat - Cleaner interface: no global state, explicit hats address per call Co-Authored-By: Claude Opus 4.5 --- src/OrgRegistry.sol | 67 +++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 42 deletions(-) diff --git a/src/OrgRegistry.sol b/src/OrgRegistry.sol index c1afba4..1565919 100644 --- a/src/OrgRegistry.sol +++ b/src/OrgRegistry.sol @@ -13,7 +13,7 @@ error OrgUnknown(); error TypeTaken(); error ContractUnknown(); error NotOrgExecutor(); -error NotOrgAdmin(); +error NotOrgMetadataAdmin(); error OwnerOnlyDuringBootstrap(); // deployer tried after bootstrap error AutoUpgradeRequired(); // deployer must set autoUpgrade=true @@ -61,9 +61,8 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { mapping(bytes32 => mapping(uint256 => uint256)) roleHatOf; // orgId => roleIndex => hatId bytes32[] orgIds; uint256 totalContracts; - // New storage for admin hat feature - mapping(bytes32 => uint256) adminHatOf; // orgId => admin hatId for direct metadata editing - address hatsProtocol; // Hats Protocol contract address + // Optional per-org metadata admin hat (if 0, falls back to topHat) + mapping(bytes32 => uint256) metadataAdminHatOf; } // keccak256("poa.orgregistry.storage") to get a unique, collision-free slot @@ -89,8 +88,7 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { ); event AutoUpgradeSet(bytes32 indexed contractId, bool enabled); event HatsTreeRegistered(bytes32 indexed orgId, uint256 topHatId, uint256[] roleHatIds); - event OrgAdminHatSet(bytes32 indexed orgId, uint256 hatId); - event HatsProtocolSet(address hatsProtocol); + event OrgMetadataAdminHatSet(bytes32 indexed orgId, uint256 hatId); /// @custom:oz-upgrades-unsafe-allow constructor constructor() initializer {} @@ -104,23 +102,6 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { __Ownable_init(initialOwner); } - /** - * @dev Sets the Hats Protocol contract address - * @param _hats The Hats Protocol contract address - */ - function setHatsProtocol(address _hats) external onlyOwner { - if (_hats == address(0)) revert InvalidParam(); - _layout().hatsProtocol = _hats; - emit HatsProtocolSet(_hats); - } - - /** - * @dev Gets the Hats Protocol contract address - */ - function getHatsProtocol() external view returns (address) { - return _layout().hatsProtocol; - } - /* ═════════════════ ORG LOGIC ═════════════════ */ function registerOrg(bytes32 orgId, address executorAddr, bytes calldata name, bytes32 metadataHash) external @@ -198,52 +179,54 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { } /** - * @dev Allows an admin hat wearer to update org metadata directly (no governance) + * @dev Allows a metadata admin hat wearer to update org metadata directly (no governance) * @param orgId The organization ID * @param newName New organization name (bytes, validated by ValidationLib) * @param newMetadataHash New IPFS metadata hash (bytes32) + * @param hats The Hats Protocol contract address */ - function updateOrgMetaAsAdmin(bytes32 orgId, bytes calldata newName, bytes32 newMetadataHash) external { + function updateOrgMetaAsAdmin(bytes32 orgId, bytes calldata newName, bytes32 newMetadataHash, address hats) + external + { ValidationLib.requireValidTitle(newName); + if (hats == address(0)) revert InvalidParam(); + Layout storage l = _layout(); OrgInfo storage o = l.orgOf[orgId]; if (!o.exists) revert OrgUnknown(); - // Check if caller wears the org's admin hat - uint256 adminHat = l.adminHatOf[orgId]; - if (adminHat == 0) { - // No admin hat configured, fall back to topHat - adminHat = l.topHatOf[orgId]; + // Check if caller wears the org's metadata admin hat (optional, falls back to topHat) + uint256 metadataAdminHat = l.metadataAdminHatOf[orgId]; + if (metadataAdminHat == 0) { + metadataAdminHat = l.topHatOf[orgId]; } - if (adminHat == 0) revert NotOrgAdmin(); + if (metadataAdminHat == 0) revert NotOrgMetadataAdmin(); - address hats = l.hatsProtocol; - if (hats == address(0)) revert InvalidParam(); - if (!IHats(hats).isWearerOfHat(msg.sender, adminHat)) revert NotOrgAdmin(); + if (!IHats(hats).isWearerOfHat(msg.sender, metadataAdminHat)) revert NotOrgMetadataAdmin(); emit MetaUpdated(orgId, newName, newMetadataHash); } /** - * @dev Set the admin hat for an org (only executor can do this) + * @dev Set the metadata admin hat for an org (only executor can do this) * @param orgId The organization ID - * @param hatId The hat ID that can edit metadata directly (0 to use topHat) + * @param hatId The hat ID that can edit metadata directly (0 to use topHat as fallback) */ - function setOrgAdminHat(bytes32 orgId, uint256 hatId) external { + function setOrgMetadataAdminHat(bytes32 orgId, uint256 hatId) external { Layout storage l = _layout(); OrgInfo storage o = l.orgOf[orgId]; if (!o.exists) revert OrgUnknown(); if (msg.sender != o.executor) revert NotOrgExecutor(); - l.adminHatOf[orgId] = hatId; - emit OrgAdminHatSet(orgId, hatId); + l.metadataAdminHatOf[orgId] = hatId; + emit OrgMetadataAdminHatSet(orgId, hatId); } /** - * @dev Get the admin hat for an org + * @dev Get the metadata admin hat for an org (returns 0 if not set, meaning topHat is used) */ - function getOrgAdminHat(bytes32 orgId) external view returns (uint256) { - return _layout().adminHatOf[orgId]; + function getOrgMetadataAdminHat(bytes32 orgId) external view returns (uint256) { + return _layout().metadataAdminHatOf[orgId]; } /* ══════════ CONTRACT REGISTRATION ══════════ */ From 48e54ffeab791145427237334a3bfb26924fe86f Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Sun, 1 Feb 2026 21:56:12 -0500 Subject: [PATCH 05/13] refactor: Store hats address in OrgRegistry instead of passing as parameter Security fix: Previously updateOrgMetaAsAdmin took hats address as a parameter which could allow malicious callers to pass a fake contract. Now the hats address is stored in contract storage and set via initialize. Changes: - Add IHats hats to Layout storage struct - Add hats parameter to initialize() function - Add getHats() view function - Remove hats parameter from updateOrgMetaAsAdmin (now uses stored address) Co-Authored-By: Claude Opus 4.5 --- src/OrgRegistry.sol | 27 +++++++++++++++++++-------- test/OrgRegistry.t.sol | 3 ++- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/OrgRegistry.sol b/src/OrgRegistry.sol index 1565919..3c899fc 100644 --- a/src/OrgRegistry.sol +++ b/src/OrgRegistry.sol @@ -63,6 +63,8 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { uint256 totalContracts; // Optional per-org metadata admin hat (if 0, falls back to topHat) mapping(bytes32 => uint256) metadataAdminHatOf; + // Hats Protocol address for permission checks + IHats hats; } // keccak256("poa.orgregistry.storage") to get a unique, collision-free slot @@ -96,10 +98,19 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { /** * @dev Initializes the contract, replacing the constructor for upgradeable pattern * @param initialOwner The address that will own this registry + * @param _hats The Hats Protocol contract address */ - function initialize(address initialOwner) external initializer { - if (initialOwner == address(0)) revert InvalidParam(); + function initialize(address initialOwner, address _hats) external initializer { + if (initialOwner == address(0) || _hats == address(0)) revert InvalidParam(); __Ownable_init(initialOwner); + _layout().hats = IHats(_hats); + } + + /** + * @dev Returns the Hats Protocol contract address + */ + function getHats() external view returns (address) { + return address(_layout().hats); } /* ═════════════════ ORG LOGIC ═════════════════ */ @@ -183,18 +194,18 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { * @param orgId The organization ID * @param newName New organization name (bytes, validated by ValidationLib) * @param newMetadataHash New IPFS metadata hash (bytes32) - * @param hats The Hats Protocol contract address */ - function updateOrgMetaAsAdmin(bytes32 orgId, bytes calldata newName, bytes32 newMetadataHash, address hats) - external - { + function updateOrgMetaAsAdmin(bytes32 orgId, bytes calldata newName, bytes32 newMetadataHash) external { ValidationLib.requireValidTitle(newName); - if (hats == address(0)) revert InvalidParam(); Layout storage l = _layout(); OrgInfo storage o = l.orgOf[orgId]; if (!o.exists) revert OrgUnknown(); + // Ensure hats protocol is configured + IHats hats = l.hats; + if (address(hats) == address(0)) revert InvalidParam(); + // Check if caller wears the org's metadata admin hat (optional, falls back to topHat) uint256 metadataAdminHat = l.metadataAdminHatOf[orgId]; if (metadataAdminHat == 0) { @@ -202,7 +213,7 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { } if (metadataAdminHat == 0) revert NotOrgMetadataAdmin(); - if (!IHats(hats).isWearerOfHat(msg.sender, metadataAdminHat)) revert NotOrgMetadataAdmin(); + if (!hats.isWearerOfHat(msg.sender, metadataAdminHat)) revert NotOrgMetadataAdmin(); emit MetaUpdated(orgId, newName, newMetadataHash); } diff --git a/test/OrgRegistry.t.sol b/test/OrgRegistry.t.sol index 957b60d..8e643b9 100644 --- a/test/OrgRegistry.t.sol +++ b/test/OrgRegistry.t.sol @@ -8,10 +8,11 @@ import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; contract OrgRegistryTest is Test { OrgRegistry reg; bytes32 ORG_ID = keccak256("ORG"); + address constant MOCK_HATS = address(0x1234); // Mock hats address function setUp() public { OrgRegistry impl = new OrgRegistry(); - bytes memory data = abi.encodeCall(OrgRegistry.initialize, (address(this))); + bytes memory data = abi.encodeCall(OrgRegistry.initialize, (address(this), MOCK_HATS)); ERC1967Proxy proxy = new ERC1967Proxy(address(impl), data); reg = OrgRegistry(address(proxy)); } From 310f4d700263335d5b8798e3821a03d596cd0b1b Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Sun, 1 Feb 2026 22:02:45 -0500 Subject: [PATCH 06/13] test: Add comprehensive tests for metadata admin functionality Tests cover: - updateOrgMetaAsAdmin with topHat (fallback) - updateOrgMetaAsAdmin with custom admin hat - updateOrgMetaAsAdmin reverts for non-hat wearers - updateOrgMetaAsAdmin reverts when no hats configured - updateOrgMetaAsAdmin reverts for unknown org - setOrgMetadataAdminHat by executor - setOrgMetadataAdminHat reverts for non-executor - setOrgMetadataAdminHat can reset to zero - getOrgMetadataAdminHat returns zero by default - Custom admin hat takes precedence over topHat - getHats returns correct address Co-Authored-By: Claude Opus 4.5 --- test/OrgRegistry.t.sol | 153 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 151 insertions(+), 2 deletions(-) diff --git a/test/OrgRegistry.t.sol b/test/OrgRegistry.t.sol index 8e643b9..20c32d8 100644 --- a/test/OrgRegistry.t.sol +++ b/test/OrgRegistry.t.sol @@ -5,14 +5,35 @@ import "forge-std/Test.sol"; import "../src/OrgRegistry.sol"; import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +/// @dev Mock Hats contract for testing +contract MockHats { + mapping(address => mapping(uint256 => bool)) public wearers; + + function setWearer(address account, uint256 hatId, bool isWearer) external { + wearers[account][hatId] = isWearer; + } + + function isWearerOfHat(address account, uint256 hatId) external view returns (bool) { + return wearers[account][hatId]; + } +} + contract OrgRegistryTest is Test { OrgRegistry reg; + MockHats mockHats; bytes32 ORG_ID = keccak256("ORG"); - address constant MOCK_HATS = address(0x1234); // Mock hats address + uint256 constant TOP_HAT_ID = 1; + uint256 constant ADMIN_HAT_ID = 2; + address constant ADMIN_USER = address(0xAD); + address constant NON_ADMIN_USER = address(0xBA); function setUp() public { + // Deploy mock hats + mockHats = new MockHats(); + + // Deploy OrgRegistry with mock hats OrgRegistry impl = new OrgRegistry(); - bytes memory data = abi.encodeCall(OrgRegistry.initialize, (address(this), MOCK_HATS)); + bytes memory data = abi.encodeCall(OrgRegistry.initialize, (address(this), address(mockHats))); ERC1967Proxy proxy = new ERC1967Proxy(address(impl), data); reg = OrgRegistry(address(proxy)); } @@ -26,4 +47,132 @@ contract OrgRegistryTest is Test { address proxy = reg.proxyOf(ORG_ID, typeId); assertEq(proxy, address(0x1)); } + + function testGetHats() public view { + assertEq(reg.getHats(), address(mockHats)); + } + + /* ══════════ Metadata Admin Tests ══════════ */ + + function testUpdateOrgMetaAsAdmin_WithTopHat() public { + // Setup: register org and hats tree + reg.registerOrg(ORG_ID, address(this), bytes("Test Org"), bytes32(0)); + uint256[] memory roleHats = new uint256[](0); + reg.registerHatsTree(ORG_ID, TOP_HAT_ID, roleHats); + + // Make ADMIN_USER wear the top hat + mockHats.setWearer(ADMIN_USER, TOP_HAT_ID, true); + + // Should succeed when caller wears top hat + vm.prank(ADMIN_USER); + reg.updateOrgMetaAsAdmin(ORG_ID, bytes("New Name"), bytes32(uint256(1))); + + // Verify event was emitted (implicitly tested by no revert) + } + + function testUpdateOrgMetaAsAdmin_WithCustomAdminHat() public { + // Setup: register org and hats tree + reg.registerOrg(ORG_ID, address(this), bytes("Test Org"), bytes32(0)); + uint256[] memory roleHats = new uint256[](0); + reg.registerHatsTree(ORG_ID, TOP_HAT_ID, roleHats); + + // Set custom metadata admin hat (as executor) + reg.setOrgMetadataAdminHat(ORG_ID, ADMIN_HAT_ID); + + // Make ADMIN_USER wear the custom admin hat (not the top hat) + mockHats.setWearer(ADMIN_USER, ADMIN_HAT_ID, true); + + // Should succeed when caller wears custom admin hat + vm.prank(ADMIN_USER); + reg.updateOrgMetaAsAdmin(ORG_ID, bytes("New Name"), bytes32(uint256(1))); + } + + function testUpdateOrgMetaAsAdmin_RevertWhenNotWearingHat() public { + // Setup: register org and hats tree + reg.registerOrg(ORG_ID, address(this), bytes("Test Org"), bytes32(0)); + uint256[] memory roleHats = new uint256[](0); + reg.registerHatsTree(ORG_ID, TOP_HAT_ID, roleHats); + + // NON_ADMIN_USER doesn't wear any hat + vm.prank(NON_ADMIN_USER); + vm.expectRevert(NotOrgMetadataAdmin.selector); + reg.updateOrgMetaAsAdmin(ORG_ID, bytes("New Name"), bytes32(uint256(1))); + } + + function testUpdateOrgMetaAsAdmin_RevertWhenNoHatsConfigured() public { + // Setup: register org but NO hats tree + reg.registerOrg(ORG_ID, address(this), bytes("Test Org"), bytes32(0)); + + // Should revert because no top hat or admin hat is set + vm.prank(ADMIN_USER); + vm.expectRevert(NotOrgMetadataAdmin.selector); + reg.updateOrgMetaAsAdmin(ORG_ID, bytes("New Name"), bytes32(uint256(1))); + } + + function testUpdateOrgMetaAsAdmin_RevertWhenOrgUnknown() public { + bytes32 unknownOrgId = keccak256("UNKNOWN"); + + vm.prank(ADMIN_USER); + vm.expectRevert(OrgUnknown.selector); + reg.updateOrgMetaAsAdmin(unknownOrgId, bytes("New Name"), bytes32(uint256(1))); + } + + function testSetOrgMetadataAdminHat_Success() public { + // Setup: register org + reg.registerOrg(ORG_ID, address(this), bytes("Test Org"), bytes32(0)); + + // Set metadata admin hat (as executor - which is address(this)) + reg.setOrgMetadataAdminHat(ORG_ID, ADMIN_HAT_ID); + + // Verify it was set + assertEq(reg.getOrgMetadataAdminHat(ORG_ID), ADMIN_HAT_ID); + } + + function testSetOrgMetadataAdminHat_RevertWhenNotExecutor() public { + // Setup: register org with address(this) as executor + reg.registerOrg(ORG_ID, address(this), bytes("Test Org"), bytes32(0)); + + // Try to set from non-executor + vm.prank(NON_ADMIN_USER); + vm.expectRevert(NotOrgExecutor.selector); + reg.setOrgMetadataAdminHat(ORG_ID, ADMIN_HAT_ID); + } + + function testSetOrgMetadataAdminHat_CanResetToZero() public { + // Setup: register org and set admin hat + reg.registerOrg(ORG_ID, address(this), bytes("Test Org"), bytes32(0)); + reg.setOrgMetadataAdminHat(ORG_ID, ADMIN_HAT_ID); + + // Reset to zero (falls back to topHat) + reg.setOrgMetadataAdminHat(ORG_ID, 0); + + assertEq(reg.getOrgMetadataAdminHat(ORG_ID), 0); + } + + function testGetOrgMetadataAdminHat_ReturnsZeroByDefault() public { + // Setup: register org + reg.registerOrg(ORG_ID, address(this), bytes("Test Org"), bytes32(0)); + + // Should return 0 if not set + assertEq(reg.getOrgMetadataAdminHat(ORG_ID), 0); + } + + function testUpdateOrgMetaAsAdmin_CustomHatTakesPrecedenceOverTopHat() public { + // Setup: register org and hats tree + reg.registerOrg(ORG_ID, address(this), bytes("Test Org"), bytes32(0)); + uint256[] memory roleHats = new uint256[](0); + reg.registerHatsTree(ORG_ID, TOP_HAT_ID, roleHats); + + // Set custom metadata admin hat + reg.setOrgMetadataAdminHat(ORG_ID, ADMIN_HAT_ID); + + // ADMIN_USER wears top hat but NOT custom admin hat + mockHats.setWearer(ADMIN_USER, TOP_HAT_ID, true); + mockHats.setWearer(ADMIN_USER, ADMIN_HAT_ID, false); + + // Should FAIL because custom admin hat takes precedence + vm.prank(ADMIN_USER); + vm.expectRevert(NotOrgMetadataAdmin.selector); + reg.updateOrgMetaAsAdmin(ORG_ID, bytes("New Name"), bytes32(uint256(1))); + } } From e23d0d89b12f909524bb6d692e88be5372e9901c Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Sun, 1 Feb 2026 22:05:06 -0500 Subject: [PATCH 07/13] fix: Use standard hyphen in SPDX license identifier Co-Authored-By: Claude Opus 4.5 --- src/OrgRegistry.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OrgRegistry.sol b/src/OrgRegistry.sol index 3c899fc..96c9c9f 100644 --- a/src/OrgRegistry.sol +++ b/src/OrgRegistry.sol @@ -1,4 +1,4 @@ -// SPDX‑License‑Identifier: MIT +// SPDX-License-Identifier: MIT pragma solidity ^0.8.20; import "@openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; From 25d5130dea287594c0bfdd4fe52680731da6bce3 Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Sun, 1 Feb 2026 22:22:35 -0500 Subject: [PATCH 08/13] fix: Update DeployerTest to use new initialize signature with hats address Co-Authored-By: Claude Opus 4.5 --- test/DeployerTest.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/DeployerTest.t.sol b/test/DeployerTest.t.sol index d7aefae..7d110b6 100644 --- a/test/DeployerTest.t.sol +++ b/test/DeployerTest.t.sol @@ -680,8 +680,8 @@ contract DeployerTest is Test, IEligibilityModuleEvents { address orgRegBeacon = poaManager.getBeaconById(keccak256("OrgRegistry")); address deployerBeacon = poaManager.getBeaconById(keccak256("OrgDeployer")); - // Create OrgRegistry proxy - initialize with poaAdmin as owner - bytes memory orgRegistryInit = abi.encodeWithSignature("initialize(address)", poaAdmin); + // Create OrgRegistry proxy - initialize with poaAdmin as owner and hats address + bytes memory orgRegistryInit = abi.encodeWithSignature("initialize(address,address)", poaAdmin, SEPOLIA_HATS); orgRegistry = OrgRegistry(address(new BeaconProxy(orgRegBeacon, orgRegistryInit))); // Debug to verify OrgRegistry owner From fa8a9434991bea8cb978668a483500fc5d2675aa Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Tue, 3 Feb 2026 18:49:04 -0500 Subject: [PATCH 09/13] fix: emit ProjectCreated event before configuration events Fix event ordering in TaskManager._createProjectInternal to emit ProjectCreated before ProjectManagerUpdated and ProjectRolePermSet events. This ensures subgraph indexing can properly create the Project entity before receiving configuration events. Fixes #84 Co-Authored-By: Claude Haiku 4.5 --- src/TaskManager.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/TaskManager.sol b/src/TaskManager.sol index f79fd86..fafff48 100644 --- a/src/TaskManager.sol +++ b/src/TaskManager.sol @@ -271,6 +271,8 @@ contract TaskManager is Initializable, ContextUpgradeable { p.cap = uint128(cap); p.exists = true; + emit ProjectCreated(projectId, title, metadataHash, cap); + /* managers */ if (defaultManager != address(0)) { p.managers[defaultManager] = true; @@ -290,8 +292,6 @@ contract TaskManager is Initializable, ContextUpgradeable { _setBatchHatPerm(projectId, claimHats, TaskPerm.CLAIM); _setBatchHatPerm(projectId, reviewHats, TaskPerm.REVIEW); _setBatchHatPerm(projectId, assignHats, TaskPerm.ASSIGN); - - emit ProjectCreated(projectId, title, metadataHash, cap); } function deleteProject(bytes32 pid) external { From 737ea01e63f95d7e8f51a468de4d03b480d97fbc Mon Sep 17 00:00:00 2001 From: Hudson Headley <76409831+hudsonhrh@users.noreply.github.com> Date: Fri, 6 Feb 2026 15:38:19 -0500 Subject: [PATCH 10/13] feat: Implement four SwitchableBeacon upgrades and add task/role application systems (#86) ## Changes ### Fix 5: Emit MirrorSet event in constructor - Added MirrorSet event emission when SwitchableBeacon initializes in Mirror mode - Tests verify constructor emits event correctly for both modes ### Fix 2: Implement two-step ownership (Ownable2Step) - SwitchableBeacon: Added pendingOwner state, OwnershipTransferStarted event, acceptOwnership() - Executor: Added acceptBeaconOwnership() helper for deployment flow - GovernanceFactory: Added execBeacon to GovernanceResult, initiates transfer - OrgDeployer: Accepts executor beacon ownership after deployment - Comprehensive test coverage for all two-step scenarios ### Fix 1: Transfer eligibility/toggle beacon ownership to Executor - GovernanceFactory: Pass result.executor as beaconOwner to _deployEligibilityModule and _deployToggleModule - Eligibility/toggle beacons now owned by executor from creation (no transfer needed) - Added 3 tests verifying all module beacons owned by executor after deployment ### Fix 3: Remove setAutoUpgrade from OrgRegistry - Removed setAutoUpgrade() function and AutoUpgradeSet event - autoUpgrade now immutable (set at deployment, read-only thereafter) ### Earlier improvements (from prior context) - TaskManager: Added rejectTask(), TaskApplicationSubmitted, TaskApplicationApproved events - EligibilityModule: Added applyForRole(), withdrawApplication() with application system - Comprehensive tests for task rejection and role application flows All 627 tests pass. Code formatted with forge fmt. Co-authored-by: Claude Haiku 4.5 --- src/EligibilityModule.sol | 55 +++ src/Executor.sol | 6 + src/OrgDeployer.sol | 4 + src/OrgRegistry.sol | 16 - src/SwitchableBeacon.sol | 34 +- src/TaskManager.sol | 12 + src/factories/GovernanceFactory.sol | 16 +- test/DeployerTest.t.sol | 306 +++++++++++++++ test/SwitchableBeacon.t.sol | 116 +++++- test/TaskManager.t.sol | 568 ++++++++++++++++++++++++++++ 10 files changed, 1103 insertions(+), 30 deletions(-) diff --git a/src/EligibilityModule.sol b/src/EligibilityModule.sol index b554d05..04409f1 100644 --- a/src/EligibilityModule.sol +++ b/src/EligibilityModule.sol @@ -37,6 +37,9 @@ contract EligibilityModule is Initializable, IHatsEligibility { error HasNotVouched(); error VouchingRateLimitExceeded(); error NewUserVouchingRestricted(); + error ApplicationAlreadyExists(); + error NoActiveApplication(); + error InvalidApplicationHash(); /*═════════════════════════════════════════ STRUCTS ═════════════════════════════════════════*/ @@ -88,6 +91,9 @@ contract EligibilityModule is Initializable, IHatsEligibility { // Rate limiting for vouching mapping(address => uint256) userJoinTime; mapping(address => mapping(uint256 => uint32)) dailyVouchCount; // user => day => count + // Role application system + mapping(uint256 => mapping(address => bytes32)) roleApplications; // hatId => applicant => applicationHash + mapping(uint256 => address[]) roleApplicants; // hatId => array of applicant addresses } // keccak256("poa.eligibilitymodule.storage") → unique, collision-free slot @@ -166,6 +172,8 @@ contract EligibilityModule is Initializable, IHatsEligibility { event Paused(address indexed account); event Unpaused(address indexed account); event HatMetadataUpdated(uint256 indexed hatId, string name, bytes32 metadataCID); + event RoleApplicationSubmitted(uint256 indexed hatId, address indexed applicant, bytes32 applicationHash); + event RoleApplicationWithdrawn(uint256 indexed hatId, address indexed applicant); /*═════════════════════════════════════════ MODIFIERS ═════════════════════════════════════════*/ @@ -801,9 +809,44 @@ contract EligibilityModule is Initializable, IHatsEligibility { bool success = l.hats.mintHat(hatId, msg.sender); require(success, "Hat minting failed"); + // Clean up any pending role application + delete l.roleApplications[hatId][msg.sender]; + emit HatClaimed(msg.sender, hatId); } + /*═══════════════════════════════════ ROLE APPLICATION SYSTEM ═══════════════════════════════════════*/ + + /// @notice Submit an application for a role (hat) that has vouching enabled. + /// This is a signaling mechanism — it does not grant eligibility. + /// @param hatId The hat ID to apply for + /// @param applicationHash IPFS CID sha256 digest of the application details + function applyForRole(uint256 hatId, bytes32 applicationHash) external whenNotPaused { + if (applicationHash == bytes32(0)) revert InvalidApplicationHash(); + + Layout storage l = _layout(); + VouchConfig memory config = l.vouchConfigs[hatId]; + if (!_isVouchingEnabled(config.flags)) revert VouchingNotEnabled(); + if (l.roleApplications[hatId][msg.sender] != bytes32(0)) revert ApplicationAlreadyExists(); + require(!l.hats.isWearerOfHat(msg.sender, hatId), "Already wearing hat"); + + l.roleApplicants[hatId].push(msg.sender); + l.roleApplications[hatId][msg.sender] = applicationHash; + + emit RoleApplicationSubmitted(hatId, msg.sender, applicationHash); + } + + /// @notice Withdraw a previously submitted role application. + /// @param hatId The hat ID to withdraw the application from + function withdrawApplication(uint256 hatId) external whenNotPaused { + Layout storage l = _layout(); + if (l.roleApplications[hatId][msg.sender] == bytes32(0)) revert NoActiveApplication(); + + delete l.roleApplications[hatId][msg.sender]; + + emit RoleApplicationWithdrawn(hatId, msg.sender); + } + /*═══════════════════════════════════ ELIGIBILITY INTERFACE ═══════════════════════════════════════*/ function getWearerStatus(address wearer, uint256 hatId) external view returns (bool eligible, bool standing) { @@ -918,6 +961,18 @@ contract EligibilityModule is Initializable, IHatsEligibility { return _layout().hasSpecificWearerRules[wearer][hatId]; } + function getRoleApplication(uint256 hatId, address applicant) external view returns (bytes32) { + return _layout().roleApplications[hatId][applicant]; + } + + function getRoleApplicants(uint256 hatId) external view returns (address[] memory) { + return _layout().roleApplicants[hatId]; + } + + function hasActiveApplication(uint256 hatId, address applicant) external view returns (bool) { + return _layout().roleApplications[hatId][applicant] != bytes32(0); + } + /*═════════════════════════════════════ PUBLIC GETTERS ═════════════════════════════════════════*/ function hats() external view returns (IHats) { diff --git a/src/Executor.sol b/src/Executor.sol index 4472b2b..c7da034 100644 --- a/src/Executor.sol +++ b/src/Executor.sol @@ -7,6 +7,7 @@ import "@openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable. import "@openzeppelin-contracts-upgradeable/contracts/utils/PausableUpgradeable.sol"; import "@openzeppelin-contracts-upgradeable/contracts/utils/ReentrancyGuardUpgradeable.sol"; import {IHats} from "@hats-protocol/src/Interfaces/IHats.sol"; +import {SwitchableBeacon} from "./SwitchableBeacon.sol"; interface IExecutor { struct Call { @@ -129,6 +130,11 @@ contract Executor is Initializable, OwnableUpgradeable, PausableUpgradeable, Ree emit BatchExecuted(proposalId, len); } + /* ─────────── Beacon ownership ─────────── */ + function acceptBeaconOwnership(address beacon) external onlyOwner { + SwitchableBeacon(beacon).acceptOwnership(); + } + /* ─────────── Guardian helpers ─────────── */ function pause() external onlyOwner { _pause(); diff --git a/src/OrgDeployer.sol b/src/OrgDeployer.sol index ca6225d..2c11cf2 100644 --- a/src/OrgDeployer.sol +++ b/src/OrgDeployer.sol @@ -22,6 +22,7 @@ interface IParticipationToken { interface IExecutorAdmin { function setCaller(address) external; function setHatMinterAuthorization(address minter, bool authorized) external; + function acceptBeaconOwnership(address beacon) external; function configureVouching( address eligibilityModule, uint256 hatId, @@ -325,6 +326,9 @@ contract OrgDeployer is Initializable { GovernanceFactory.GovernanceResult memory gov = _deployGovernanceInfrastructure(params); result.executor = gov.executor; + /* 2b. Accept executor beacon ownership (two-step transfer initiated by GovernanceFactory) */ + IExecutorAdmin(result.executor).acceptBeaconOwnership(gov.execBeacon); + /* 3. Set the executor for the org */ l.orgRegistry.setOrgExecutor(params.orgId, result.executor); diff --git a/src/OrgRegistry.sol b/src/OrgRegistry.sol index 96c9c9f..2033de0 100644 --- a/src/OrgRegistry.sol +++ b/src/OrgRegistry.sol @@ -88,7 +88,6 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { bool autoUpgrade, address owner ); - event AutoUpgradeSet(bytes32 indexed contractId, bool enabled); event HatsTreeRegistered(bytes32 indexed orgId, uint256 topHatId, uint256[] roleHatIds); event OrgMetadataAdminHatSet(bytes32 indexed orgId, uint256 hatId); @@ -369,21 +368,6 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { } } - function setAutoUpgrade(bytes32 orgId, bytes32 typeId, bool enabled) external { - Layout storage l = _layout(); - OrgInfo storage o = l.orgOf[orgId]; - if (!o.exists) revert OrgUnknown(); - if (msg.sender != o.executor) revert NotOrgExecutor(); - - address proxy = l.proxyOf[orgId][typeId]; - if (proxy == address(0)) revert ContractUnknown(); - - bytes32 contractId = keccak256(abi.encodePacked(orgId, typeId)); - l.contractOf[contractId].autoUpgrade = enabled; - - emit AutoUpgradeSet(contractId, enabled); - } - /* ═════════════════ VIEW HELPERS ═════════════════ */ function getOrgContract(bytes32 orgId, bytes32 typeId) external view returns (address proxy) { Layout storage l = _layout(); diff --git a/src/SwitchableBeacon.sol b/src/SwitchableBeacon.sol index f4785e7..da7975a 100644 --- a/src/SwitchableBeacon.sol +++ b/src/SwitchableBeacon.sol @@ -21,6 +21,9 @@ contract SwitchableBeacon is IBeacon { /// @notice Current owner of this beacon (typically the Executor or UpgradeAdmin) address public owner; + /// @notice Address that has been proposed as the new owner (two-step transfer) + address public pendingOwner; + /// @notice The global POA beacon to mirror when in Mirror mode address public mirrorBeacon; @@ -47,9 +50,15 @@ contract SwitchableBeacon is IBeacon { /// @param implementation The address of the pinned implementation event Pinned(address indexed implementation); + /// @notice Emitted when a new ownership transfer is initiated + event OwnershipTransferStarted(address indexed previousOwner, address indexed newOwner); + /// @notice Thrown when a non-owner attempts a restricted operation error NotOwner(); + /// @notice Thrown when caller is not the pending owner + error NotPendingOwner(); + /// @notice Thrown when a zero address is provided where not allowed error ZeroAddress(); @@ -95,21 +104,36 @@ contract SwitchableBeacon is IBeacon { mode = _mode; emit OwnerTransferred(address(0), _owner); + if (_mode == Mode.Mirror) { + emit MirrorSet(_mirrorBeacon); + } emit ModeChanged(_mode); } /** - * @notice Transfers ownership of the beacon to a new address - * @param newOwner The address of the new owner - * @dev Only callable by the current owner + * @notice Initiates a two-step ownership transfer + * @param newOwner The address of the proposed new owner + * @dev Only callable by the current owner. The new owner must call acceptOwnership() to complete. */ function transferOwnership(address newOwner) external onlyOwner { if (newOwner == address(0)) revert ZeroAddress(); + pendingOwner = newOwner; + emit OwnershipTransferStarted(owner, newOwner); + } + + /** + * @notice Completes the two-step ownership transfer + * @dev Only callable by the pending owner set via transferOwnership() + */ + function acceptOwnership() external { + if (msg.sender != pendingOwner) revert NotPendingOwner(); + address previousOwner = owner; - owner = newOwner; + owner = msg.sender; + pendingOwner = address(0); - emit OwnerTransferred(previousOwner, newOwner); + emit OwnerTransferred(previousOwner, msg.sender); } /** diff --git a/src/TaskManager.sol b/src/TaskManager.sol index fafff48..f3e5a33 100644 --- a/src/TaskManager.sol +++ b/src/TaskManager.sol @@ -162,6 +162,7 @@ contract TaskManager is Initializable, ContextUpgradeable { event TaskAssigned(uint256 indexed id, address indexed assignee, address indexed assigner); event TaskCompleted(uint256 indexed id, address indexed completer); event TaskCancelled(uint256 indexed id, address indexed canceller); + event TaskRejected(uint256 indexed id, address indexed rejector, bytes32 rejectionHash); event TaskApplicationSubmitted(uint256 indexed id, address indexed applicant, bytes32 applicationHash); event TaskApplicationApproved(uint256 indexed id, address indexed applicant, address indexed approver); event ExecutorUpdated(address newExecutor); @@ -513,6 +514,17 @@ contract TaskManager is Initializable, ContextUpgradeable { emit TaskCompleted(id, _msgSender()); } + function rejectTask(uint256 id, bytes32 rejectionHash) external { + Layout storage l = _layout(); + _checkPerm(l._tasks[id].projectId, TaskPerm.REVIEW); + Task storage t = _task(l, id); + if (t.status != Status.SUBMITTED) revert BadStatus(); + if (rejectionHash == bytes32(0)) revert ValidationLib.InvalidString(); + + t.status = Status.CLAIMED; + emit TaskRejected(id, _msgSender(), rejectionHash); + } + function cancelTask(uint256 id) external { _requireCanCreate(_layout()._tasks[id].projectId); Layout storage l = _layout(); diff --git a/src/factories/GovernanceFactory.sol b/src/factories/GovernanceFactory.sol index 3bf2823..6fe089f 100644 --- a/src/factories/GovernanceFactory.sol +++ b/src/factories/GovernanceFactory.sol @@ -90,6 +90,7 @@ contract GovernanceFactory { address toggleModule; address hybridVoting; // Governance mechanism address directDemocracyVoting; // Polling mechanism + address execBeacon; // Executor's SwitchableBeacon (for two-step ownership acceptance) uint256 topHatId; uint256[] roleHatIds; } @@ -138,11 +139,11 @@ contract GovernanceFactory { /* 2. Deploy and configure modules for Hats tree (without registration) */ (result.eligibilityModule, eligibilityBeacon) = _deployEligibilityModule( - params.orgId, params.poaManager, params.orgRegistry, params.hats, params.autoUpgrade, params.deployer + params.orgId, params.poaManager, params.orgRegistry, params.hats, params.autoUpgrade, result.executor ); (result.toggleModule, toggleBeacon) = _deployToggleModule( - params.orgId, params.poaManager, params.orgRegistry, params.hats, params.autoUpgrade, params.deployer + params.orgId, params.poaManager, params.orgRegistry, params.hats, params.autoUpgrade, result.executor ); /* 3. Setup Hats Tree */ @@ -200,8 +201,9 @@ contract GovernanceFactory { IOrgDeployer(params.deployer).batchRegisterContracts(params.orgId, registrations, params.autoUpgrade, false); } - /* 5. Transfer executor beacon ownership back to executor itself */ + /* 5. Initiate two-step ownership transfer for executor beacon */ SwitchableBeacon(execBeacon).transferOwnership(result.executor); + result.execBeacon = execBeacon; return result; } @@ -350,10 +352,10 @@ contract GovernanceFactory { address orgRegistry, address hats, bool autoUpgrade, - address deployer + address beaconOwner ) internal returns (address emProxy, address beacon) { beacon = BeaconDeploymentLib.createBeacon( - ModuleTypes.ELIGIBILITY_MODULE_ID, poaManager, address(this), autoUpgrade, address(0) + ModuleTypes.ELIGIBILITY_MODULE_ID, poaManager, beaconOwner, autoUpgrade, address(0) ); ModuleDeploymentLib.DeployConfig memory config = ModuleDeploymentLib.DeployConfig({ @@ -381,10 +383,10 @@ contract GovernanceFactory { address orgRegistry, address hats, bool autoUpgrade, - address deployer + address beaconOwner ) internal returns (address tmProxy, address beacon) { beacon = BeaconDeploymentLib.createBeacon( - ModuleTypes.TOGGLE_MODULE_ID, poaManager, address(this), autoUpgrade, address(0) + ModuleTypes.TOGGLE_MODULE_ID, poaManager, beaconOwner, autoUpgrade, address(0) ); ModuleDeploymentLib.DeployConfig memory config = ModuleDeploymentLib.DeployConfig({ diff --git a/test/DeployerTest.t.sol b/test/DeployerTest.t.sol index 7d110b6..7e1ed7c 100644 --- a/test/DeployerTest.t.sol +++ b/test/DeployerTest.t.sol @@ -36,6 +36,7 @@ import {ModuleTypes} from "../src/libs/ModuleTypes.sol"; import {EligibilityModule} from "../src/EligibilityModule.sol"; import {ToggleModule} from "../src/ToggleModule.sol"; import {IExecutor} from "../src/Executor.sol"; +import {SwitchableBeacon} from "../src/SwitchableBeacon.sol"; import {IHats} from "@hats-protocol/src/Interfaces/IHats.sol"; import {MockERC20} from "./mocks/MockERC20.sol"; import {PaymasterHub} from "../src/PaymasterHub.sol"; @@ -68,6 +69,10 @@ interface IEligibilityModuleEvents { bool defaultStanding, uint256 mintedCount ); + + event RoleApplicationSubmitted(uint256 indexed hatId, address indexed applicant, bytes32 applicationHash); + + event RoleApplicationWithdrawn(uint256 indexed hatId, address indexed applicant); } /*────────────── Test contract ───────────*/ @@ -3674,4 +3679,305 @@ contract DeployerTest is Test, IEligibilityModuleEvents { assertEq(token.educationHub(), address(0), "EducationHub should be cleared to address(0)"); } + + /*───────────────── ROLE APPLICATION TESTS ───────────────────*/ + + function testRoleApplicationBasic() public { + TestOrgSetup memory setup = _createTestOrg("App Basic DAO"); + address applicant = address(0x400); + + _setupUserForVouching(setup.eligibilityModule, setup.exec, applicant); + + // Configure vouching on DEFAULT hat + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + bytes32 appHash = keccak256("my-application-ipfs-hash"); + + // Apply + vm.prank(applicant); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, appHash); + + // Verify storage + assertEq( + EligibilityModule(setup.eligibilityModule).getRoleApplication(setup.defaultRoleHat, applicant), + appHash, + "Application hash should be stored" + ); + assertTrue( + EligibilityModule(setup.eligibilityModule).hasActiveApplication(setup.defaultRoleHat, applicant), + "Should have active application" + ); + address[] memory applicants = EligibilityModule(setup.eligibilityModule).getRoleApplicants(setup.defaultRoleHat); + assertEq(applicants.length, 1, "Should have 1 applicant"); + assertEq(applicants[0], applicant, "Applicant address should match"); + } + + function testRoleApplicationEmitsEvent() public { + TestOrgSetup memory setup = _createTestOrg("App Event DAO"); + address applicant = address(0x401); + + _setupUserForVouching(setup.eligibilityModule, setup.exec, applicant); + + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + bytes32 appHash = keccak256("app-hash"); + + vm.prank(applicant); + vm.expectEmit(true, true, false, true); + emit RoleApplicationSubmitted(setup.defaultRoleHat, applicant, appHash); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, appHash); + } + + function testRoleApplicationWithdraw() public { + TestOrgSetup memory setup = _createTestOrg("App Withdraw DAO"); + address applicant = address(0x402); + + _setupUserForVouching(setup.eligibilityModule, setup.exec, applicant); + + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + bytes32 appHash = keccak256("app-hash"); + + // Apply + vm.prank(applicant); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, appHash); + + // Withdraw + vm.prank(applicant); + vm.expectEmit(true, true, false, false); + emit RoleApplicationWithdrawn(setup.defaultRoleHat, applicant); + EligibilityModule(setup.eligibilityModule).withdrawApplication(setup.defaultRoleHat); + + assertFalse( + EligibilityModule(setup.eligibilityModule).hasActiveApplication(setup.defaultRoleHat, applicant), + "Application should be cleared" + ); + + // Can reapply after withdrawal + bytes32 newHash = keccak256("updated-app"); + vm.prank(applicant); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, newHash); + + assertEq( + EligibilityModule(setup.eligibilityModule).getRoleApplication(setup.defaultRoleHat, applicant), + newHash, + "New application should be stored" + ); + } + + function testRoleApplicationInvalidHash() public { + TestOrgSetup memory setup = _createTestOrg("App Invalid DAO"); + address applicant = address(0x403); + + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + vm.prank(applicant); + vm.expectRevert(EligibilityModule.InvalidApplicationHash.selector); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, bytes32(0)); + } + + function testRoleApplicationVouchingNotEnabled() public { + TestOrgSetup memory setup = _createTestOrg("App NoVouch DAO"); + address applicant = address(0x404); + + // Don't configure vouching — default hat has no vouching + vm.prank(applicant); + vm.expectRevert(EligibilityModule.VouchingNotEnabled.selector); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, keccak256("app")); + } + + function testRoleApplicationDuplicate() public { + TestOrgSetup memory setup = _createTestOrg("App Dup DAO"); + address applicant = address(0x405); + + _setupUserForVouching(setup.eligibilityModule, setup.exec, applicant); + + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + vm.prank(applicant); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, keccak256("first")); + + vm.prank(applicant); + vm.expectRevert(EligibilityModule.ApplicationAlreadyExists.selector); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, keccak256("second")); + } + + function testRoleApplicationAlreadyWearing() public { + TestOrgSetup memory setup = _createTestOrg("App Wearing DAO"); + + // Mint MEMBER hat to voter1 (default eligibility is true) + _mintHat(setup.exec, setup.memberRoleHat, voter1); + + // Configure vouching with combineWithHierarchy=true so hierarchy eligibility preserves wearer status + _configureVouching( + setup.eligibilityModule, setup.exec, setup.memberRoleHat, 2, setup.defaultRoleHat, true, false + ); + + // voter1 already wears memberRoleHat, so applying should revert + vm.prank(voter1); + vm.expectRevert("Already wearing hat"); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.memberRoleHat, keccak256("app")); + } + + function testRoleApplicationFullFlow() public { + TestOrgSetup memory setup = _createTestOrg("App Full DAO"); + address applicant = address(0x406); + + _setupUserForVouching(setup.eligibilityModule, setup.exec, voter1); + _setupUserForVouching(setup.eligibilityModule, setup.exec, voter2); + _setupUserForVouching(setup.eligibilityModule, setup.exec, applicant); + + // Configure vouching: 2 vouches from MEMBER hat + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + // Mint MEMBER hats to vouchers + _mintHat(setup.exec, setup.memberRoleHat, voter1); + _mintHat(setup.exec, setup.memberRoleHat, voter2); + + // 1. Applicant applies + bytes32 appHash = keccak256("my-application"); + vm.prank(applicant); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, appHash); + + assertTrue( + EligibilityModule(setup.eligibilityModule).hasActiveApplication(setup.defaultRoleHat, applicant), + "Should have active application" + ); + + // 2. Vouchers vouch + _vouchFor(voter1, setup.eligibilityModule, applicant, setup.defaultRoleHat); + _vouchFor(voter2, setup.eligibilityModule, applicant, setup.defaultRoleHat); + + // 3. Applicant claims hat + vm.prank(applicant); + EligibilityModule(setup.eligibilityModule).claimVouchedHat(setup.defaultRoleHat); + + // Verify: wearing hat + _assertWearingHat(applicant, setup.defaultRoleHat, true, "After claim"); + + // Verify: application auto-cleaned + assertFalse( + EligibilityModule(setup.eligibilityModule).hasActiveApplication(setup.defaultRoleHat, applicant), + "Application should be cleared after claim" + ); + } + + function testRoleApplicationMultipleApplicants() public { + TestOrgSetup memory setup = _createTestOrg("App Multi DAO"); + address applicant1 = address(0x407); + address applicant2 = address(0x408); + + _setupUserForVouching(setup.eligibilityModule, setup.exec, applicant1); + _setupUserForVouching(setup.eligibilityModule, setup.exec, applicant2); + + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + vm.prank(applicant1); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, keccak256("app1")); + + vm.prank(applicant2); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, keccak256("app2")); + + // Both should have active applications + assertTrue( + EligibilityModule(setup.eligibilityModule).hasActiveApplication(setup.defaultRoleHat, applicant1), + "Applicant1 should have application" + ); + assertTrue( + EligibilityModule(setup.eligibilityModule).hasActiveApplication(setup.defaultRoleHat, applicant2), + "Applicant2 should have application" + ); + + // Applications should be independent + assertEq( + EligibilityModule(setup.eligibilityModule).getRoleApplication(setup.defaultRoleHat, applicant1), + keccak256("app1"), + "Applicant1 hash should match" + ); + assertEq( + EligibilityModule(setup.eligibilityModule).getRoleApplication(setup.defaultRoleHat, applicant2), + keccak256("app2"), + "Applicant2 hash should match" + ); + + address[] memory applicants = EligibilityModule(setup.eligibilityModule).getRoleApplicants(setup.defaultRoleHat); + assertEq(applicants.length, 2, "Should have 2 applicants"); + } + + function testRoleApplicationWhilePaused() public { + TestOrgSetup memory setup = _createTestOrg("App Paused DAO"); + address applicant = address(0x409); + + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + // Pause the module + vm.prank(setup.exec); + EligibilityModule(setup.eligibilityModule).pause(); + + // Apply should revert + vm.prank(applicant); + vm.expectRevert("Contract is paused"); + EligibilityModule(setup.eligibilityModule).applyForRole(setup.defaultRoleHat, keccak256("app")); + + // Withdraw should also revert (even though there's nothing to withdraw, pause check comes first) + vm.prank(applicant); + vm.expectRevert("Contract is paused"); + EligibilityModule(setup.eligibilityModule).withdrawApplication(setup.defaultRoleHat); + } + + function testWithdrawApplicationNoActiveReverts() public { + TestOrgSetup memory setup = _createTestOrg("App NoActive DAO"); + address applicant = address(0x410); + + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + vm.prank(applicant); + vm.expectRevert(EligibilityModule.NoActiveApplication.selector); + EligibilityModule(setup.eligibilityModule).withdrawApplication(setup.defaultRoleHat); + } + + /* ═══════════════════════════════════════════════════════════════════ + Beacon Ownership Tests (Fix 1 — all module beacons owned by executor) + ═══════════════════════════════════════════════════════════════════ */ + + function _getBeaconForType(bytes32 typeId) internal view returns (address) { + bytes32 contractId = keccak256(abi.encodePacked(ORG_ID, typeId)); + return orgRegistry.getContractBeacon(contractId); + } + + function testExecutorBeaconOwnedByExecutor() public { + TestOrgSetup memory setup = _createTestOrg("Beacon Owner DAO"); + address beacon = _getBeaconForType(ModuleTypes.EXECUTOR_ID); + assertEq(SwitchableBeacon(beacon).owner(), setup.exec, "Executor beacon should be owned by executor"); + } + + function testEligibilityBeaconOwnedByExecutor() public { + TestOrgSetup memory setup = _createTestOrg("Beacon Owner DAO"); + address beacon = _getBeaconForType(ModuleTypes.ELIGIBILITY_MODULE_ID); + assertEq(SwitchableBeacon(beacon).owner(), setup.exec, "Eligibility beacon should be owned by executor"); + } + + function testToggleBeaconOwnedByExecutor() public { + TestOrgSetup memory setup = _createTestOrg("Beacon Owner DAO"); + address beacon = _getBeaconForType(ModuleTypes.TOGGLE_MODULE_ID); + assertEq(SwitchableBeacon(beacon).owner(), setup.exec, "Toggle beacon should be owned by executor"); + } } diff --git a/test/SwitchableBeacon.t.sol b/test/SwitchableBeacon.t.sol index fa2921c..c4e43f3 100644 --- a/test/SwitchableBeacon.t.sol +++ b/test/SwitchableBeacon.t.sol @@ -27,6 +27,7 @@ contract SwitchableBeaconTest is Test { // Events event OwnerTransferred(address indexed previousOwner, address indexed newOwner); + event OwnershipTransferStarted(address indexed previousOwner, address indexed newOwner); event ModeChanged(SwitchableBeacon.Mode mode); event MirrorSet(address indexed mirrorBeacon); event Pinned(address indexed implementation); @@ -48,6 +49,32 @@ contract SwitchableBeaconTest is Test { ); } + // ============ Constructor Event Tests ============ + + function testConstructorEmitsMirrorSetInMirrorMode() public { + vm.expectEmit(true, false, false, true); + emit MirrorSet(address(poaBeacon)); + vm.expectEmit(false, false, false, true); + emit ModeChanged(SwitchableBeacon.Mode.Mirror); + + new SwitchableBeacon(owner, address(poaBeacon), address(0), SwitchableBeacon.Mode.Mirror); + } + + function testConstructorDoesNotEmitMirrorSetInStaticMode() public { + // Should emit Pinned is NOT expected, MirrorSet is NOT expected + // Only OwnerTransferred and ModeChanged + vm.recordLogs(); + new SwitchableBeacon(owner, address(poaBeacon), address(implV1), SwitchableBeacon.Mode.Static); + + Vm.Log[] memory logs = vm.getRecordedLogs(); + for (uint256 i = 0; i < logs.length; i++) { + // MirrorSet event topic + assertTrue( + logs[i].topics[0] != keccak256("MirrorSet(address)"), "MirrorSet should not be emitted in Static mode" + ); + } + } + // ============ Mirror Mode Tests ============ function testMirrorModeTracksPoaBeacon() public { @@ -204,14 +231,29 @@ contract SwitchableBeaconTest is Test { } function testOwnershipTransfer() public { - // Transfer ownership + // Initiate transfer — emits OwnershipTransferStarted, NOT OwnerTransferred vm.expectEmit(true, true, false, false); - emit OwnerTransferred(owner, newOwner); + emit OwnershipTransferStarted(owner, newOwner); switchableBeacon.transferOwnership(newOwner); + // Owner is still the original owner until accepted + assertEq(switchableBeacon.owner(), owner); + assertEq(switchableBeacon.pendingOwner(), newOwner); + + // Old owner can still perform restricted operations + switchableBeacon.pin(address(implV1)); + + // New owner accepts + vm.expectEmit(true, true, false, false); + emit OwnerTransferred(owner, newOwner); + + vm.prank(newOwner); + switchableBeacon.acceptOwnership(); + // Verify new owner assertEq(switchableBeacon.owner(), newOwner); + assertEq(switchableBeacon.pendingOwner(), address(0)); // Old owner can't perform restricted operations vm.expectRevert(SwitchableBeacon.NotOwner.selector); @@ -223,6 +265,70 @@ contract SwitchableBeaconTest is Test { assertEq(switchableBeacon.implementation(), address(implV2)); } + // ============ Two-Step Ownership Tests ============ + + function testPendingOwnerState() public { + assertEq(switchableBeacon.pendingOwner(), address(0)); + + switchableBeacon.transferOwnership(newOwner); + assertEq(switchableBeacon.pendingOwner(), newOwner); + assertEq(switchableBeacon.owner(), owner); + } + + function testNonPendingCannotAccept() public { + switchableBeacon.transferOwnership(newOwner); + + vm.prank(unauthorized); + vm.expectRevert(SwitchableBeacon.NotPendingOwner.selector); + switchableBeacon.acceptOwnership(); + } + + function testAcceptWithoutTransferReverts() public { + vm.prank(newOwner); + vm.expectRevert(SwitchableBeacon.NotPendingOwner.selector); + switchableBeacon.acceptOwnership(); + } + + function testOldOwnerRetainsControlUntilAccept() public { + switchableBeacon.transferOwnership(newOwner); + + // Old owner can still pin, setMirror, etc. + switchableBeacon.pin(address(implV1)); + assertEq(switchableBeacon.implementation(), address(implV1)); + + switchableBeacon.setMirror(address(poaBeacon)); + assertTrue(switchableBeacon.isMirrorMode()); + } + + function testTransferOverwritesPending() public { + address secondCandidate = address(0xABCD); + + switchableBeacon.transferOwnership(newOwner); + assertEq(switchableBeacon.pendingOwner(), newOwner); + + switchableBeacon.transferOwnership(secondCandidate); + assertEq(switchableBeacon.pendingOwner(), secondCandidate); + + // First candidate can no longer accept + vm.prank(newOwner); + vm.expectRevert(SwitchableBeacon.NotPendingOwner.selector); + switchableBeacon.acceptOwnership(); + + // Second candidate can accept + vm.prank(secondCandidate); + switchableBeacon.acceptOwnership(); + assertEq(switchableBeacon.owner(), secondCandidate); + } + + function testPendingClearedAfterAccept() public { + switchableBeacon.transferOwnership(newOwner); + + vm.prank(newOwner); + switchableBeacon.acceptOwnership(); + + assertEq(switchableBeacon.pendingOwner(), address(0)); + } + // ============ Zero Address Guards ============ function testConstructorRevertsOnZeroOwner() public { @@ -398,7 +504,13 @@ contract SwitchableBeaconTest is Test { vm.assume(newAddr != address(0)); switchableBeacon.transferOwnership(newAddr); + assertEq(switchableBeacon.pendingOwner(), newAddr); + assertEq(switchableBeacon.owner(), address(this)); // still original owner + + vm.prank(newAddr); + switchableBeacon.acceptOwnership(); assertEq(switchableBeacon.owner(), newAddr); + assertEq(switchableBeacon.pendingOwner(), address(0)); } } diff --git a/test/TaskManager.t.sol b/test/TaskManager.t.sol index a7c903a..6b75fff 100644 --- a/test/TaskManager.t.sol +++ b/test/TaskManager.t.sol @@ -3214,6 +3214,201 @@ contract MockToken is Test, IERC20 { emit TaskManager.TaskApplicationApproved(0, member1, pm1); tm.approveApplication(0, member1); } + + /*───────────────── TASK REJECTION ───────────────────*/ + + function _prepareSubmittedTask() internal returns (uint256 id, bytes32 pid) { + pid = _createDefaultProject("REJECT", 5 ether); + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.PROJECT_MANAGER, abi.encode(pid, pm1, true)); + + vm.prank(pm1); + tm.createTask(1 ether, bytes("reject_task"), bytes32(0), pid, address(0), 0, false); + id = 0; + + vm.prank(member1); + tm.claimTask(id); + + vm.prank(member1); + tm.submitTask(id, keccak256("submission")); + } + + function test_RejectTaskSendsBackToClaimed() public { + (uint256 id, bytes32 pid) = _prepareSubmittedTask(); + + vm.prank(pm1); + tm.rejectTask(id, keccak256("needs_fixes")); + + bytes memory result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (uint256 payout, TaskManager.Status st, address claimer,,) = + abi.decode(result, (uint256, TaskManager.Status, address, bytes32, bool)); + assertEq(uint8(st), uint8(TaskManager.Status.CLAIMED), "status should be CLAIMED"); + assertEq(claimer, member1, "claimer should be unchanged"); + } + + function test_RejectTaskEmitsEvent() public { + (uint256 id,) = _prepareSubmittedTask(); + bytes32 rejHash = keccak256("needs_fixes"); + + vm.prank(pm1); + vm.expectEmit(true, true, true, true); + emit TaskManager.TaskRejected(id, pm1, rejHash); + tm.rejectTask(id, rejHash); + } + + function test_RejectThenResubmitThenComplete() public { + (uint256 id,) = _prepareSubmittedTask(); + + // reject + vm.prank(pm1); + tm.rejectTask(id, keccak256("try_again")); + + // resubmit + vm.prank(member1); + tm.submitTask(id, keccak256("submission_v2")); + + // complete + uint256 balBefore = token.balanceOf(member1); + vm.prank(pm1); + tm.completeTask(id); + + assertEq(token.balanceOf(member1), balBefore + 1 ether, "minted payout after rejection cycle"); + bytes memory result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (, TaskManager.Status st,,,) = abi.decode(result, (uint256, TaskManager.Status, address, bytes32, bool)); + assertEq(uint8(st), uint8(TaskManager.Status.COMPLETED)); + } + + function test_MultipleRejectionsBeforeComplete() public { + (uint256 id,) = _prepareSubmittedTask(); + + // first rejection + vm.prank(pm1); + tm.rejectTask(id, keccak256("round_1")); + + vm.prank(member1); + tm.submitTask(id, keccak256("v2")); + + // second rejection + vm.prank(pm1); + tm.rejectTask(id, keccak256("round_2")); + + vm.prank(member1); + tm.submitTask(id, keccak256("v3")); + + // complete + vm.prank(pm1); + tm.completeTask(id); + + bytes memory result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (, TaskManager.Status st,,,) = abi.decode(result, (uint256, TaskManager.Status, address, bytes32, bool)); + assertEq(uint8(st), uint8(TaskManager.Status.COMPLETED)); + } + + function test_RejectTaskRequiresReviewPermission() public { + (uint256 id,) = _prepareSubmittedTask(); + + vm.prank(outsider); + vm.expectRevert(TaskManager.Unauthorized.selector); + tm.rejectTask(id, keccak256("nope")); + + vm.prank(member1); + vm.expectRevert(TaskManager.Unauthorized.selector); + tm.rejectTask(id, keccak256("nope")); + } + + function test_RejectTaskByProjectManager() public { + (uint256 id,) = _prepareSubmittedTask(); + + vm.prank(pm1); + tm.rejectTask(id, keccak256("pm_reject")); + + bytes memory result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (, TaskManager.Status st,,,) = abi.decode(result, (uint256, TaskManager.Status, address, bytes32, bool)); + assertEq(uint8(st), uint8(TaskManager.Status.CLAIMED)); + } + + function test_RejectUnclaimedTaskReverts() public { + bytes32 pid = _createDefaultProject("REJ_UNCL", 5 ether); + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.PROJECT_MANAGER, abi.encode(pid, pm1, true)); + + vm.prank(pm1); + tm.createTask(1 ether, bytes("unclaimed"), bytes32(0), pid, address(0), 0, false); + + vm.prank(pm1); + vm.expectRevert(TaskManager.BadStatus.selector); + tm.rejectTask(0, keccak256("nope")); + } + + function test_RejectClaimedTaskReverts() public { + bytes32 pid = _createDefaultProject("REJ_CL", 5 ether); + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.PROJECT_MANAGER, abi.encode(pid, pm1, true)); + + vm.prank(pm1); + tm.createTask(1 ether, bytes("claimed_only"), bytes32(0), pid, address(0), 0, false); + + vm.prank(member1); + tm.claimTask(0); + + vm.prank(pm1); + vm.expectRevert(TaskManager.BadStatus.selector); + tm.rejectTask(0, keccak256("nope")); + } + + function test_RejectCompletedTaskReverts() public { + (uint256 id,) = _prepareSubmittedTask(); + + vm.prank(pm1); + tm.completeTask(id); + + vm.prank(pm1); + vm.expectRevert(TaskManager.BadStatus.selector); + tm.rejectTask(id, keccak256("nope")); + } + + function test_RejectCancelledTaskReverts() public { + bytes32 pid = _createDefaultProject("REJ_CAN", 5 ether); + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.PROJECT_MANAGER, abi.encode(pid, pm1, true)); + + vm.prank(pm1); + tm.createTask(1 ether, bytes("to_cancel"), bytes32(0), pid, address(0), 0, false); + + vm.prank(pm1); + tm.cancelTask(0); + + vm.prank(pm1); + vm.expectRevert(TaskManager.BadStatus.selector); + tm.rejectTask(0, keccak256("nope")); + } + + function test_RejectTaskWithEmptyHashReverts() public { + (uint256 id,) = _prepareSubmittedTask(); + + vm.prank(pm1); + vm.expectRevert(ValidationLib.InvalidString.selector); + tm.rejectTask(id, bytes32(0)); + } + + function test_RejectTaskDoesNotChangeBudget() public { + (uint256 id, bytes32 pid) = _prepareSubmittedTask(); + + bytes memory before_ = lens.getStorage( + address(tm), TaskManagerLens.StorageKey.PROJECT_INFO, abi.encode(pid) + ); + (, uint256 spentBefore,) = abi.decode(before_, (uint256, uint256, bool)); + + vm.prank(pm1); + tm.rejectTask(id, keccak256("reject")); + + bytes memory after_ = lens.getStorage( + address(tm), TaskManagerLens.StorageKey.PROJECT_INFO, abi.encode(pid) + ); + (, uint256 spentAfter,) = abi.decode(after_, (uint256, uint256, bool)); + + assertEq(spentBefore, spentAfter, "budget spent should not change on rejection"); + } } /*───────────────── BOUNTY FUNCTIONALITY TESTS ────────────────────*/ @@ -4843,6 +5038,379 @@ contract MockToken is Test, IERC20 { } } + /*────────────────── Task Application Test Suite ──────────────────*/ + + contract TaskManagerApplicationTest is TaskManagerTestBase { + bytes32 APP_PROJECT_ID; + + function setUp() public { + setUpBase(); + APP_PROJECT_ID = _createDefaultProject("APP_PROJECT", 10 ether); + } + + /// @dev Helper: creates an application-required task and returns its ID + function _createAppTask(uint256 payout) internal returns (uint256 id) { + vm.prank(creator1); + tm.createTask(payout, bytes("app_task"), bytes32(0), APP_PROJECT_ID, address(0), 0, true); + id = 0; // first task + } + + function _createAppTaskN(uint256 payout, uint256 expectedId) internal returns (uint256 id) { + vm.prank(creator1); + tm.createTask(payout, bytes("app_task"), bytes32(0), APP_PROJECT_ID, address(0), 0, true); + id = expectedId; + } + + /*──────── Basic Apply ────────*/ + + function test_ApplyForTask() public { + uint256 id = _createAppTask(1 ether); + bytes32 appHash = keccak256("my application"); + + vm.prank(member1); + tm.applyForTask(id, appHash); + + // Verify application stored via lens + bytes memory result = + lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_APPLICATION, abi.encode(id, member1)); + bytes32 stored = abi.decode(result, (bytes32)); + assertEq(stored, appHash, "application hash should be stored"); + + // Verify applicant in list + bytes memory listResult = + lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_APPLICANTS, abi.encode(id)); + address[] memory applicants = abi.decode(listResult, (address[])); + assertEq(applicants.length, 1, "should have 1 applicant"); + assertEq(applicants[0], member1, "applicant should be member1"); + } + + function test_ApplyForTaskEmitsEvent() public { + uint256 id = _createAppTask(1 ether); + bytes32 appHash = keccak256("my application"); + + vm.expectEmit(true, true, false, true); + emit TaskManager.TaskApplicationSubmitted(id, member1, appHash); + + vm.prank(member1); + tm.applyForTask(id, appHash); + } + + /*──────── Approve Application ────────*/ + + function test_ApproveApplicationClaimsTask() public { + uint256 id = _createAppTask(1 ether); + bytes32 appHash = keccak256("my application"); + + vm.prank(member1); + tm.applyForTask(id, appHash); + + vm.prank(pm1); + tm.approveApplication(id, member1); + + // Verify task is now CLAIMED with member1 as claimer + bytes memory result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (uint256 payout, TaskManagerLens.Status status, address claimer,,) = + abi.decode(result, (uint256, TaskManagerLens.Status, address, bytes32, bool)); + assertEq(uint8(status), uint8(TaskManagerLens.Status.CLAIMED), "status should be CLAIMED"); + assertEq(claimer, member1, "claimer should be member1"); + } + + function test_ApproveApplicationEmitsEvent() public { + uint256 id = _createAppTask(1 ether); + bytes32 appHash = keccak256("my application"); + + vm.prank(member1); + tm.applyForTask(id, appHash); + + vm.expectEmit(true, true, true, true); + emit TaskManager.TaskApplicationApproved(id, member1, pm1); + + vm.prank(pm1); + tm.approveApplication(id, member1); + } + + function test_ApproveApplicationClearsApplicantsList() public { + uint256 id = _createAppTask(1 ether); + + // Two applicants apply + address member2 = makeAddr("member2"); + setHat(member2, MEMBER_HAT); + + vm.prank(member1); + tm.applyForTask(id, keccak256("app1")); + vm.prank(member2); + tm.applyForTask(id, keccak256("app2")); + + // Approve member1 + vm.prank(pm1); + tm.approveApplication(id, member1); + + // Applicants list should be cleared + bytes memory result = lens.getStorage( + address(tm), TaskManagerLens.StorageKey.TASK_APPLICANTS, abi.encode(id) + ); + address[] memory applicants = abi.decode(result, (address[])); + assertEq(applicants.length, 0, "applicants list should be cleared after approval"); + } + + /*──────── Full Lifecycle ────────*/ + + function test_FullApplicationFlow() public { + uint256 id = _createAppTask(1 ether); + bytes32 appHash = keccak256("my application"); + + // Apply + vm.prank(member1); + tm.applyForTask(id, appHash); + + // Approve + vm.prank(pm1); + tm.approveApplication(id, member1); + + // Submit + vm.prank(member1); + tm.submitTask(id, keccak256("submission")); + + // Complete + vm.prank(pm1); + tm.completeTask(id); + + // Verify completed + bytes memory result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (, TaskManagerLens.Status status,,,) = + abi.decode(result, (uint256, TaskManagerLens.Status, address, bytes32, bool)); + assertEq(uint8(status), uint8(TaskManagerLens.Status.COMPLETED), "status should be COMPLETED"); + + // Verify tokens minted + assertEq(token.balanceOf(member1), 1 ether, "member1 should receive payout"); + } + + function test_ApplicationRejectReapplyFlow() public { + uint256 id = _createAppTask(1 ether); + + // Apply -> Approve -> Submit -> Reject -> Resubmit -> Complete + vm.prank(member1); + tm.applyForTask(id, keccak256("app")); + + vm.prank(pm1); + tm.approveApplication(id, member1); + + vm.prank(member1); + tm.submitTask(id, keccak256("bad submission")); + + vm.prank(pm1); + tm.rejectTask(id, keccak256("needs work")); + + vm.prank(member1); + tm.submitTask(id, keccak256("good submission")); + + vm.prank(pm1); + tm.completeTask(id); + + bytes memory result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (, TaskManagerLens.Status status,,,) = + abi.decode(result, (uint256, TaskManagerLens.Status, address, bytes32, bool)); + assertEq(uint8(status), uint8(TaskManagerLens.Status.COMPLETED)); + } + + /*──────── Multiple Applicants ────────*/ + + function test_MultipleApplicants() public { + uint256 id = _createAppTask(1 ether); + + address member2 = makeAddr("member2"); + address member3 = makeAddr("member3"); + setHat(member2, MEMBER_HAT); + setHat(member3, MEMBER_HAT); + + vm.prank(member1); + tm.applyForTask(id, keccak256("app1")); + vm.prank(member2); + tm.applyForTask(id, keccak256("app2")); + vm.prank(member3); + tm.applyForTask(id, keccak256("app3")); + + // Verify 3 applicants + bytes memory result = + lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_APPLICANT_COUNT, abi.encode(id)); + uint256 count = abi.decode(result, (uint256)); + assertEq(count, 3, "should have 3 applicants"); + + // Approve member2 + vm.prank(pm1); + tm.approveApplication(id, member2); + + // Verify claimer is member2 + result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (,, address claimer,,) = abi.decode(result, (uint256, TaskManagerLens.Status, address, bytes32, bool)); + assertEq(claimer, member2, "claimer should be member2"); + } + + /*──────── Permission Checks ────────*/ + + function test_ApplyRequiresClaimPermission() public { + uint256 id = _createAppTask(1 ether); + + vm.prank(outsider); + vm.expectRevert(TaskManager.Unauthorized.selector); + tm.applyForTask(id, keccak256("app")); + } + + function test_ApproveRequiresAssignPermission() public { + uint256 id = _createAppTask(1 ether); + + vm.prank(member1); + tm.applyForTask(id, keccak256("app")); + + vm.prank(outsider); + vm.expectRevert(TaskManager.Unauthorized.selector); + tm.approveApplication(id, member1); + } + + function test_ApproveByProjectManager() public { + uint256 id = _createAppTask(1 ether); + + // Add pm1 as project manager + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.PROJECT_MANAGER, abi.encode(APP_PROJECT_ID, pm1, true)); + + vm.prank(member1); + tm.applyForTask(id, keccak256("app")); + + // PM can approve (bypasses hat-based assign permission check) + vm.prank(pm1); + tm.approveApplication(id, member1); + + bytes memory result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (, TaskManagerLens.Status status,,,) = + abi.decode(result, (uint256, TaskManagerLens.Status, address, bytes32, bool)); + assertEq(uint8(status), uint8(TaskManagerLens.Status.CLAIMED)); + } + + /*──────── Error Cases ────────*/ + + function test_ApplyDuplicateReverts() public { + uint256 id = _createAppTask(1 ether); + + vm.prank(member1); + tm.applyForTask(id, keccak256("app")); + + vm.prank(member1); + vm.expectRevert(TaskManager.AlreadyApplied.selector); + tm.applyForTask(id, keccak256("app2")); + } + + function test_ApplyEmptyHashReverts() public { + uint256 id = _createAppTask(1 ether); + + vm.prank(member1); + vm.expectRevert(ValidationLib.InvalidString.selector); + tm.applyForTask(id, bytes32(0)); + } + + function test_ApplyForNonApplicationTaskReverts() public { + // Create a regular (non-application) task + vm.prank(creator1); + tm.createTask(1 ether, bytes("regular_task"), bytes32(0), APP_PROJECT_ID, address(0), 0, false); + uint256 id = 0; + + vm.prank(member1); + vm.expectRevert(TaskManager.NoApplicationRequired.selector); + tm.applyForTask(id, keccak256("app")); + } + + function test_ApplyForClaimedTaskReverts() public { + uint256 id = _createAppTask(1 ether); + + // Apply and approve to move to CLAIMED + vm.prank(member1); + tm.applyForTask(id, keccak256("app")); + vm.prank(pm1); + tm.approveApplication(id, member1); + + // New applicant tries to apply for already-claimed task + address member2 = makeAddr("member2"); + setHat(member2, MEMBER_HAT); + + vm.prank(member2); + vm.expectRevert(TaskManager.BadStatus.selector); + tm.applyForTask(id, keccak256("app2")); + } + + function test_ApproveNonApplicantReverts() public { + uint256 id = _createAppTask(1 ether); + + vm.prank(member1); + tm.applyForTask(id, keccak256("app")); + + // Try to approve someone who didn't apply + address member2 = makeAddr("member2"); + vm.prank(pm1); + vm.expectRevert(TaskManager.NotApplicant.selector); + tm.approveApplication(id, member2); + } + + function test_ApproveAlreadyClaimedReverts() public { + uint256 id = _createAppTask(1 ether); + + vm.prank(member1); + tm.applyForTask(id, keccak256("app")); + + vm.prank(pm1); + tm.approveApplication(id, member1); + + // Try to approve again (task is now CLAIMED) + vm.prank(pm1); + vm.expectRevert(TaskManager.BadStatus.selector); + tm.approveApplication(id, member1); + } + + function test_ClaimTaskWithApplicationRequiredReverts() public { + uint256 id = _createAppTask(1 ether); + + // Try to claim directly (bypass application) + vm.prank(member1); + vm.expectRevert(TaskManager.RequiresApplication.selector); + tm.claimTask(id); + } + + /*──────── Cancel Clears Applications ────────*/ + + function test_CancelTaskClearsApplications() public { + uint256 id = _createAppTask(1 ether); + + vm.prank(member1); + tm.applyForTask(id, keccak256("app")); + + // Cancel the task + vm.prank(creator1); + tm.cancelTask(id); + + // Verify applicants list is cleared + bytes memory result = lens.getStorage( + address(tm), TaskManagerLens.StorageKey.TASK_APPLICANTS, abi.encode(id) + ); + address[] memory applicants = abi.decode(result, (address[])); + assertEq(applicants.length, 0, "applicants should be cleared after cancel"); + } + + /*──────── Assign bypasses application requirement ────────*/ + + function test_AssignTaskBypassesApplicationRequirement() public { + uint256 id = _createAppTask(1 ether); + + // PM can directly assign even if requiresApplication is true + vm.prank(pm1); + tm.assignTask(id, member1); + + bytes memory result = lens.getStorage(address(tm), TaskManagerLens.StorageKey.TASK_INFO, abi.encode(id)); + (, TaskManagerLens.Status status, address claimer,,) = + abi.decode(result, (uint256, TaskManagerLens.Status, address, bytes32, bool)); + assertEq(uint8(status), uint8(TaskManagerLens.Status.CLAIMED)); + assertEq(claimer, member1); + } + } + /*────────────────── Bootstrap Test Suite ──────────────────*/ contract TaskManagerBootstrapTest is Test { /* test actors */ From bc026dd528aaf1475a90eb5d65b95d002ab62d5e Mon Sep 17 00:00:00 2001 From: Hudson Headley <76409831+hudsonhrh@users.noreply.github.com> Date: Fri, 6 Feb 2026 18:43:16 -0500 Subject: [PATCH 11/13] feat: Implement 11 security fixes and add comprehensive test coverage (#87) - Fix 1: Add replay protection to announceWinner in voting contracts - Fix 2: Enforce strict access control for Executor.setCaller - Fix 3: Restrict OrgDeployer.setUniversalPasskeyFactory to poaManager - Fix 4: Add access control to PaymasterHub.registerOrgWithVoucher - Fix 5: Fix PaymasterHub._checkOrgBalance underflow logic - Fix 6: Move bounty totalPaid increment to post-transfer in PaymasterHub - Fix 7: Add nonReentrant guard and fix CEI violation in claimVouchedHat - Fix 8: Remove unsafe dailyVouchCount decrement in vouch revocation - Fix 9: Add configurable SELF_REVIEW permission for task self-approval - Fix 10: Salt answer hashes with module ID to prevent brute-force - Fix 11: Validate zero public keys and max credentials in PasskeyAccount Added 19 comprehensive tests across 7 test files covering all security fixes. All 646 tests pass with zero failures. Co-authored-by: Claude Haiku 4.5 --- src/DirectDemocracyVoting.sol | 9 ++- src/EducationHub.sol | 4 +- src/EligibilityModule.sol | 17 ++--- src/Executor.sol | 5 +- src/HybridVoting.sol | 1 + src/OrgDeployer.sol | 8 +-- src/PasskeyAccount.sol | 8 +++ src/PaymasterHub.sol | 37 ++++++---- src/TaskManager.sol | 12 +++- src/libs/HybridVotingCore.sol | 8 ++- src/libs/TaskPerm.sol | 1 + src/libs/VotingErrors.sol | 1 + test/DeployerTest.t.sol | 37 ++++++++++ test/DirectDemocracyVoting.t.sol | 28 ++++++++ test/Executor.t.sol | 19 +++++ test/HybridVoting.t.sol | 20 ++++++ test/Passkey.t.sol | 62 +++++++++++++++++ test/PasskeyPaymasterIntegration.t.sol | 9 +-- test/PaymasterHubSolidarity.t.sol | 45 +++++++++++- test/TaskManager.t.sol | 96 ++++++++++++++++++++++++++ 20 files changed, 383 insertions(+), 44 deletions(-) diff --git a/src/DirectDemocracyVoting.sol b/src/DirectDemocracyVoting.sol index a009ca7..f4bbe9b 100644 --- a/src/DirectDemocracyVoting.sol +++ b/src/DirectDemocracyVoting.sol @@ -46,6 +46,7 @@ contract DirectDemocracyVoting is Initializable { uint256[] pollHatIds; // array of specific hat IDs for this poll bool restricted; // if true only allowedHats can vote mapping(uint256 => bool) pollHatAllowed; // O(1) lookup for poll hat permission + bool executed; // finalization guard } /* ─────────── ERC-7201 Storage ─────────── */ @@ -408,9 +409,13 @@ contract DirectDemocracyVoting is Initializable { whenNotPaused returns (uint256 winner, bool valid) { - (winner, valid) = _calcWinner(id); Layout storage l = _layout(); - IExecutor.Call[] storage batch = l._proposals[id].batches[winner]; + Proposal storage prop = l._proposals[id]; + if (prop.executed) revert VotingErrors.AlreadyExecuted(); + prop.executed = true; + + (winner, valid) = _calcWinner(id); + IExecutor.Call[] storage batch = prop.batches[winner]; if (valid && batch.length > 0) { uint256 len = batch.length; diff --git a/src/EducationHub.sol b/src/EducationHub.sol index 2cc2ff6..50d4edb 100644 --- a/src/EducationHub.sol +++ b/src/EducationHub.sol @@ -198,7 +198,7 @@ contract EducationHub is Initializable, ContextUpgradeable, ReentrancyGuardUpgra } l._modules[id] = - Module({answerHash: keccak256(abi.encodePacked(correctAnswer)), payout: uint128(payout), exists: true}); + Module({answerHash: keccak256(abi.encodePacked(id, correctAnswer)), payout: uint128(payout), exists: true}); emit ModuleCreated(id, title, contentHash, payout); } @@ -229,7 +229,7 @@ contract EducationHub is Initializable, ContextUpgradeable, ReentrancyGuardUpgra Layout storage l = _layout(); Module storage m = _module(l, id); if (_isCompleted(l, _msgSender(), id)) revert AlreadyCompleted(); - if (keccak256(abi.encodePacked(answer)) != m.answerHash) revert InvalidAnswer(); + if (keccak256(abi.encodePacked(uint48(id), answer)) != m.answerHash) revert InvalidAnswer(); l.token.mint(_msgSender(), m.payout); _setCompleted(l, _msgSender(), id); diff --git a/src/EligibilityModule.sol b/src/EligibilityModule.sol index 04409f1..4b055e5 100644 --- a/src/EligibilityModule.sol +++ b/src/EligibilityModule.sol @@ -111,7 +111,7 @@ contract EligibilityModule is Initializable, IHatsEligibility { uint256 private _notEntered = 1; modifier nonReentrant() { - require(_notEntered == 1, "ReentrancyGuard: reentrant call"); + require(_notEntered != 2, "ReentrancyGuard: reentrant call"); _notEntered = 2; _; _notEntered = 1; @@ -197,6 +197,8 @@ contract EligibilityModule is Initializable, IHatsEligibility { function initialize(address _superAdmin, address _hats, address _toggleModule) external initializer { if (_superAdmin == address(0) || _hats == address(0)) revert ZeroAddress(); + _notEntered = 1; + Layout storage l = _layout(); l.superAdmin = _superAdmin; l.hats = IHats(_hats); @@ -768,9 +770,8 @@ contract EligibilityModule is Initializable, IHatsEligibility { uint32 newCount = l.currentVouchCount[hatId][wearer] - 1; l.currentVouchCount[hatId][wearer] = newCount; - uint256 currentDay = block.timestamp / SECONDS_PER_DAY; - uint32 dailyCount = l.dailyVouchCount[msg.sender][currentDay] - 1; - l.dailyVouchCount[msg.sender][currentDay] = dailyCount; + // Note: dailyVouchCount is NOT decremented on revocation. + // It's a rate limiter only — revoking doesn't give back vouch slots. emit VouchRevoked(msg.sender, wearer, hatId, newCount); @@ -795,7 +796,7 @@ contract EligibilityModule is Initializable, IHatsEligibility { * The EligibilityModule contract mints the hat using its ELIGIBILITY_ADMIN permissions. * @param hatId The ID of the hat to claim */ - function claimVouchedHat(uint256 hatId) external whenNotPaused { + function claimVouchedHat(uint256 hatId) external whenNotPaused nonReentrant { Layout storage l = _layout(); // Check if caller is eligible to claim this hat @@ -805,13 +806,13 @@ contract EligibilityModule is Initializable, IHatsEligibility { // Check if already wearing the hat require(!l.hats.isWearerOfHat(msg.sender, hatId), "Already wearing hat"); + // State change BEFORE external call (CEI pattern) + delete l.roleApplications[hatId][msg.sender]; + // Mint the hat to the caller using EligibilityModule's admin powers bool success = l.hats.mintHat(hatId, msg.sender); require(success, "Hat minting failed"); - // Clean up any pending role application - delete l.roleApplications[hatId][msg.sender]; - emit HatClaimed(msg.sender, hatId); } diff --git a/src/Executor.sol b/src/Executor.sol index c7da034..b1b6b99 100644 --- a/src/Executor.sol +++ b/src/Executor.sol @@ -78,10 +78,7 @@ contract Executor is Initializable, OwnableUpgradeable, PausableUpgradeable, Ree function setCaller(address newCaller) external { if (newCaller == address(0)) revert ZeroAddress(); Layout storage l = _layout(); - if (l.allowedCaller != address(0)) { - // After first set, only current caller or owner can change - if (msg.sender != l.allowedCaller && msg.sender != owner()) revert UnauthorizedCaller(); - } + if (msg.sender != l.allowedCaller && msg.sender != owner()) revert UnauthorizedCaller(); l.allowedCaller = newCaller; emit CallerSet(newCaller); } diff --git a/src/HybridVoting.sol b/src/HybridVoting.sol index 9cc0ac5..15aa819 100644 --- a/src/HybridVoting.sol +++ b/src/HybridVoting.sol @@ -52,6 +52,7 @@ contract HybridVoting is Initializable { bool restricted; // if true only pollHatIds can vote mapping(uint256 => bool) pollHatAllowed; // O(1) lookup for poll hat permission ClassConfig[] classesSnapshot; // Snapshot the class config to freeze semantics for this proposal + bool executed; // finalization guard } /* ─────── ERC-7201 Storage ─────── */ diff --git a/src/OrgDeployer.sol b/src/OrgDeployer.sol index 2c11cf2..2d522da 100644 --- a/src/OrgDeployer.sol +++ b/src/OrgDeployer.sol @@ -174,14 +174,12 @@ contract OrgDeployer is Initializable { /** * @notice Set the universal passkey factory address - * @dev Callable by PoaManager, or anyone for one-time initial setup (when factory is not yet set) + * @dev Only callable by PoaManager */ function setUniversalPasskeyFactory(address _universalFactory) external { Layout storage l = _layout(); - // Allow one-time setup by anyone (when factory is not yet set), or require PoaManager for updates - if (l.universalPasskeyFactory != address(0) && msg.sender != l.poaManager) { - revert InvalidAddress(); - } + if (msg.sender != l.poaManager) revert InvalidAddress(); + if (_universalFactory == address(0)) revert InvalidAddress(); l.universalPasskeyFactory = _universalFactory; } diff --git a/src/PasskeyAccount.sol b/src/PasskeyAccount.sol index 09e2f69..4745950 100644 --- a/src/PasskeyAccount.sol +++ b/src/PasskeyAccount.sol @@ -236,6 +236,7 @@ contract PasskeyAccount is Initializable, IAccount, IPasskeyAccount { /// @inheritdoc IPasskeyAccount function addCredential(bytes32 credentialId, bytes32 pubKeyX, bytes32 pubKeyY) external override onlySelf { + if (pubKeyX == bytes32(0) || pubKeyY == bytes32(0)) revert InvalidSignature(); Layout storage l = _layout(); // Check if credential already exists @@ -319,6 +320,7 @@ contract PasskeyAccount is Initializable, IAccount, IPasskeyAccount { /// @inheritdoc IPasskeyAccount function initiateRecovery(bytes32 credentialId, bytes32 pubKeyX, bytes32 pubKeyY) external override onlyGuardian { + if (pubKeyX == bytes32(0) || pubKeyY == bytes32(0)) revert InvalidSignature(); Layout storage l = _layout(); // Generate recovery ID @@ -361,6 +363,12 @@ contract PasskeyAccount is Initializable, IAccount, IPasskeyAccount { revert RecoveryDelayNotPassed(); } + // Check max credentials limit before adding + uint8 maxCreds = _getMaxCredentials(); + if (l.credentialIds.length >= maxCreds) { + revert MaxCredentialsReached(); + } + // Add the new credential bytes32 credentialId = request.credentialId; diff --git a/src/PaymasterHub.sol b/src/PaymasterHub.sol index bbb4648..6e4e7a1 100644 --- a/src/PaymasterHub.sol +++ b/src/PaymasterHub.sol @@ -126,6 +126,7 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG address entryPoint; address hats; address poaManager; + address orgRegistrar; // authorized contract that can register orgs (e.g. OrgDeployer) } // keccak256(abi.encode(uint256(keccak256("poa.paymasterhub.main")) - 1)) @@ -335,6 +336,8 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG function registerOrgWithVoucher(bytes32 orgId, uint256 adminHatId, uint256 operatorHatId, uint256 voucherHatId) public { + MainStorage storage main = _getMainStorage(); + if (msg.sender != main.poaManager && msg.sender != main.orgRegistrar) revert NotPoaManager(); if (adminHatId == 0) revert ZeroAddress(); mapping(bytes32 => OrgConfig) storage orgs = _getOrgsStorage(); @@ -611,18 +614,17 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG mapping(bytes32 => OrgFinancials) storage financials = _getFinancialsStorage(); OrgFinancials storage org = financials[orgId]; - // Calculate total available funds - uint256 totalAvailable = uint256(org.deposited) - uint256(org.spent); + uint256 depositAvailable = + uint256(org.deposited) > uint256(org.spent) ? uint256(org.deposited) - uint256(org.spent) : 0; - // Check if org has enough in deposits to cover this - // Note: solidarity is checked separately in _checkSolidarityAccess - if (org.spent + maxCost > org.deposited) { - // Will need to use solidarity - that's checked elsewhere - // Here we just make sure they haven't overdrawn - if (totalAvailable == 0) { - revert InsufficientOrgBalance(); - } + // If deposits alone cover the cost, no solidarity needed + if (depositAvailable >= maxCost) return; + + // Deposits are fully exhausted — revert early before solidarity check + if (depositAvailable == 0) { + revert InsufficientOrgBalance(); } + // Partial coverage: solidarity will cover the rest (validated by _checkSolidarityAccess) } /** @@ -1033,6 +1035,16 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG emit GracePeriodConfigUpdated(_initialGraceDays, _maxSpendDuringGrace, _minDepositRequired); } + /** + * @notice Set the authorized org registrar (e.g. OrgDeployer) + * @dev Only PoaManager can set the registrar + * @param registrar Address authorized to call registerOrg + */ + function setOrgRegistrar(address registrar) external { + if (msg.sender != _getMainStorage().poaManager) revert NotPoaManager(); + _getMainStorage().orgRegistrar = registrar; + } + /** * @notice Ban or unban an org from accessing solidarity fund * @dev Only PoaManager can ban orgs for malicious behavior @@ -1665,13 +1677,10 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG } if (tip > 0) { - // Update total paid - bounty.totalPaid += uint144(tip); - - // Attempt payment with gas limit (bool success,) = bundlerOrigin.call{value: tip, gas: 30000}(""); if (success) { + bounty.totalPaid += uint144(tip); emit BountyPaid(userOpHash, bundlerOrigin, tip); } else { emit BountyPayFailed(userOpHash, bundlerOrigin, tip); diff --git a/src/TaskManager.sol b/src/TaskManager.sol index f3e5a33..9d316ec 100644 --- a/src/TaskManager.sol +++ b/src/TaskManager.sol @@ -41,6 +41,7 @@ contract TaskManager is Initializable, ContextUpgradeable { error RequiresApplication(); error NoApplicationRequired(); error InvalidIndex(); + error SelfReviewNotAllowed(); /*──────── Constants ─────*/ bytes4 public constant MODULE_ID = 0x54534b32; // "TSK2" @@ -499,10 +500,19 @@ contract TaskManager is Initializable, ContextUpgradeable { function completeTask(uint256 id) external { Layout storage l = _layout(); - _checkPerm(l._tasks[id].projectId, TaskPerm.REVIEW); + bytes32 pid = l._tasks[id].projectId; + _checkPerm(pid, TaskPerm.REVIEW); Task storage t = _task(l, id); if (t.status != Status.SUBMITTED) revert BadStatus(); + // Self-review: if caller is the claimer, require SELF_REVIEW permission or PM/executor + address sender = _msgSender(); + if (t.claimer == sender && !_isPM(pid, sender)) { + if (!TaskPerm.has(_permMask(sender, pid), TaskPerm.SELF_REVIEW)) { + revert SelfReviewNotAllowed(); + } + } + t.status = Status.COMPLETED; l.token.mint(t.claimer, uint256(t.payout)); diff --git a/src/libs/HybridVotingCore.sol b/src/libs/HybridVotingCore.sol index bd40ec8..783aa61 100644 --- a/src/libs/HybridVotingCore.sol +++ b/src/libs/HybridVotingCore.sol @@ -137,6 +137,8 @@ library HybridVotingCore { function announceWinner(uint256 id) external returns (uint256 winner, bool valid) { HybridVoting.Layout storage l = _layout(); HybridVoting.Proposal storage p = l._proposals[id]; + if (p.executed) revert VotingErrors.AlreadyExecuted(); + p.executed = true; // Check if any votes were cast bool hasVotes = false; @@ -191,14 +193,14 @@ library HybridVotingCore { ); IExecutor.Call[] storage batch = p.batches[winner]; - bool executed = false; + bool didExecute = false; if (valid && batch.length > 0) { // No target validation needed - Executor has onlyExecutor permission on all org contracts // and handles the actual calls. HybridVoting just passes the batch through. l.executor.execute(id, batch); - executed = true; + didExecute = true; emit ProposalExecuted(id, winner, batch.length); } - emit Winner(id, winner, valid, executed, uint64(block.timestamp)); + emit Winner(id, winner, valid, didExecute, uint64(block.timestamp)); } } diff --git a/src/libs/TaskPerm.sol b/src/libs/TaskPerm.sol index cf46583..c692802 100644 --- a/src/libs/TaskPerm.sol +++ b/src/libs/TaskPerm.sol @@ -10,6 +10,7 @@ library TaskPerm { uint8 internal constant CLAIM = 1 << 1; uint8 internal constant REVIEW = 1 << 2; uint8 internal constant ASSIGN = 1 << 3; + uint8 internal constant SELF_REVIEW = 1 << 4; function has(uint8 mask, uint8 flag) internal pure returns (bool) { return mask & flag != 0; diff --git a/src/libs/VotingErrors.sol b/src/libs/VotingErrors.sol index 988c7b6..935b07a 100644 --- a/src/libs/VotingErrors.sol +++ b/src/libs/VotingErrors.sol @@ -29,4 +29,5 @@ library VotingErrors { error InvalidSliceSum(); error TooManyClasses(); error InvalidStrategy(); + error AlreadyExecuted(); } diff --git a/test/DeployerTest.t.sol b/test/DeployerTest.t.sol index 7e1ed7c..ebe87b3 100644 --- a/test/DeployerTest.t.sol +++ b/test/DeployerTest.t.sol @@ -726,6 +726,12 @@ contract DeployerTest is Test, IEligibilityModuleEvents { ); deployer = OrgDeployer(address(new BeaconProxy(deployerBeacon, deployerInit))); + // Authorize OrgDeployer to register orgs on PaymasterHub + vm.stopPrank(); + vm.prank(address(poaManager)); + paymasterHub.setOrgRegistrar(address(deployer)); + vm.startPrank(poaAdmin); + // Debug to verify Deployer initialization console.log("deployer address:", address(deployer)); @@ -3980,4 +3986,35 @@ contract DeployerTest is Test, IEligibilityModuleEvents { address beacon = _getBeaconForType(ModuleTypes.TOGGLE_MODULE_ID); assertEq(SwitchableBeacon(beacon).owner(), setup.exec, "Toggle beacon should be owned by executor"); } + + function testVouchRevocationCrossDay() public { + TestOrgSetup memory setup = _createTestOrg("Cross-Day Revoke DAO"); + address candidate = address(0x500); + + _setupUserForVouching(setup.eligibilityModule, setup.exec, voter1); + _setupUserForVouching(setup.eligibilityModule, setup.exec, voter2); + _setupUserForVouching(setup.eligibilityModule, setup.exec, candidate); + + _configureVouching( + setup.eligibilityModule, setup.exec, setup.defaultRoleHat, 2, setup.memberRoleHat, false, true + ); + + _mintHat(setup.exec, setup.memberRoleHat, voter1); + _mintHat(setup.exec, setup.memberRoleHat, voter2); + + // Vouch on day 1 + _vouchFor(voter1, setup.eligibilityModule, candidate, setup.defaultRoleHat); + _vouchFor(voter2, setup.eligibilityModule, candidate, setup.defaultRoleHat); + + // Warp to a different day (previously caused underflow in dailyVouchCount decrement) + vm.warp(block.timestamp + 2 days); + + // Revoke on day 3 — should succeed without underflow + _revokeVouch(voter1, setup.eligibilityModule, candidate, setup.defaultRoleHat); + + // Verify the revocation worked correctly + _assertVouchStatus( + setup.eligibilityModule, candidate, setup.defaultRoleHat, 1, false, "After cross-day revocation" + ); + } } diff --git a/test/DirectDemocracyVoting.t.sol b/test/DirectDemocracyVoting.t.sol index 7c2d00e..1ef2528 100644 --- a/test/DirectDemocracyVoting.t.sol +++ b/test/DirectDemocracyVoting.t.sol @@ -612,4 +612,32 @@ contract DDVotingTest is Test { assertEq(hats.balanceOf(bob, executiveHatId), 0, "Bob should not have executive hat"); assertEq(hats.balanceOf(bob, managerHatId), 0, "Bob should not have manager hat"); } + + /*//////////////////////////////////////////////////////////// + ANNOUNCE WINNER REPLAY PROTECTION + ////////////////////////////////////////////////////////////*/ + + function testAnnounceWinnerDoubleCallReverts() public { + IExecutor.Call[][] memory b = new IExecutor.Call[][](2); + b[0] = new IExecutor.Call[](0); + b[1] = new IExecutor.Call[](0); + vm.prank(creator); + dd.createProposal(bytes("Replay Test"), bytes32(0), 10, 2, b, new uint256[](0)); + + uint8[] memory idx = new uint8[](1); + idx[0] = 0; + uint8[] memory w = new uint8[](1); + w[0] = 100; + vm.prank(voter); + dd.vote(0, idx, w); + + vm.warp(block.timestamp + 11 minutes); + + // First call succeeds + dd.announceWinner(0); + + // Second call reverts + vm.expectRevert(VotingErrors.AlreadyExecuted.selector); + dd.announceWinner(0); + } } diff --git a/test/Executor.t.sol b/test/Executor.t.sol index 18c19e7..c90a6e3 100644 --- a/test/Executor.t.sol +++ b/test/Executor.t.sol @@ -70,4 +70,23 @@ contract ExecutorTest is Test { vm.expectRevert(Executor.UnauthorizedCaller.selector); exec.mintHatsForUser(user, hatIds); } + + function testSetCallerUnauthorizedReverts() public { + address random = address(0x99); + vm.prank(random); + vm.expectRevert(Executor.UnauthorizedCaller.selector); + exec.setCaller(address(0x5)); + } + + function testSetCallerZeroAddressReverts() public { + vm.expectRevert(Executor.ZeroAddress.selector); + exec.setCaller(address(0)); + } + + function testAllowedCallerCanSetNewCaller() public { + address newCaller = address(0x5); + vm.prank(caller); + exec.setCaller(newCaller); + assertEq(exec.allowedCaller(), newCaller); + } } diff --git a/test/HybridVoting.t.sol b/test/HybridVoting.t.sol index c3ea412..dc128d5 100644 --- a/test/HybridVoting.t.sol +++ b/test/HybridVoting.t.sol @@ -1483,4 +1483,24 @@ contract MockERC20 is IERC20 { assertTrue(ok, "Should have quorum with multiple participants"); // The result depends on the complex interaction of all classes } + + /* ───────────── ANNOUNCE WINNER REPLAY PROTECTION ───────────── */ + + function testAnnounceWinnerDoubleCallReverts() public { + uint256 id = _create(); + + _voteYES(alice); + _voteYES(carol); + + vm.warp(block.timestamp + 16 minutes); + + // First call succeeds + vm.prank(alice); + hv.announceWinner(id); + + // Second call reverts + vm.expectRevert(VotingErrors.AlreadyExecuted.selector); + vm.prank(alice); + hv.announceWinner(id); + } } diff --git a/test/Passkey.t.sol b/test/Passkey.t.sol index 222b833..10a7f86 100644 --- a/test/Passkey.t.sol +++ b/test/Passkey.t.sol @@ -1061,4 +1061,66 @@ contract PasskeyTest is Test { assertFalse(P256Verifier.isValidSignature(bytes32(0), s)); assertFalse(P256Verifier.isValidSignature(r, bytes32(0))); } + + /*════════════════════════════════════════════════════════════════════ + ZERO PUBKEY VALIDATION TESTS + ════════════════════════════════════════════════════════════════════*/ + + function testAddCredentialZeroPubKeyXReverts() public { + PasskeyAccount account = _createAccount(); + + vm.prank(address(account)); + vm.expectRevert(IPasskeyAccount.InvalidSignature.selector); + account.addCredential(CREDENTIAL_ID_2, bytes32(0), PUB_KEY_Y); + } + + function testAddCredentialZeroPubKeyYReverts() public { + PasskeyAccount account = _createAccount(); + + vm.prank(address(account)); + vm.expectRevert(IPasskeyAccount.InvalidSignature.selector); + account.addCredential(CREDENTIAL_ID_2, PUB_KEY_X, bytes32(0)); + } + + function testInitiateRecoveryZeroPubKeyXReverts() public { + PasskeyAccount account = _createAccount(); + + vm.prank(guardian); + vm.expectRevert(IPasskeyAccount.InvalidSignature.selector); + account.initiateRecovery(keccak256("new_cred"), bytes32(0), PUB_KEY_Y); + } + + function testInitiateRecoveryZeroPubKeyYReverts() public { + PasskeyAccount account = _createAccount(); + + vm.prank(guardian); + vm.expectRevert(IPasskeyAccount.InvalidSignature.selector); + account.initiateRecovery(keccak256("new_cred"), PUB_KEY_X, bytes32(0)); + } + + function testCompleteRecoveryMaxCredentialsReverts() public { + // Set max credentials to 2 + vm.prank(owner); + factory.setMaxCredentials(2); + + PasskeyAccount account = _createAccount(); + + // Add second credential to reach the limit + vm.prank(address(account)); + account.addCredential(CREDENTIAL_ID_2, PUB_KEY_X, PUB_KEY_Y); + + // Initiate recovery (would add a third credential) + bytes32 recoveryCredId = keccak256("recovery_cred"); + vm.prank(guardian); + account.initiateRecovery(recoveryCredId, keccak256("rx"), keccak256("ry")); + + bytes32 recoveryId = keccak256(abi.encodePacked(recoveryCredId, block.timestamp, guardian)); + + // Warp past recovery delay + vm.warp(block.timestamp + 7 days + 1); + + // Complete recovery should revert — already at max credentials + vm.expectRevert(IPasskeyAccount.MaxCredentialsReached.selector); + account.completeRecovery(recoveryId); + } } diff --git a/test/PasskeyPaymasterIntegration.t.sol b/test/PasskeyPaymasterIntegration.t.sol index 73c6932..6a4ac84 100644 --- a/test/PasskeyPaymasterIntegration.t.sol +++ b/test/PasskeyPaymasterIntegration.t.sol @@ -153,11 +153,12 @@ contract PasskeyPaymasterIntegrationTest is Test { ERC1967Proxy hubProxy = new ERC1967Proxy(address(hubImpl), hubInitData); hub = PaymasterHub(payable(address(hubProxy))); - // Register org in PaymasterHub with voucher hat - hub.registerOrgWithVoucher(ORG_ID, ADMIN_HAT, OPERATOR_HAT, VOUCHER_HAT); - vm.stopPrank(); + // Register org in PaymasterHub with voucher hat (requires poaManager) + vm.prank(poaManager); + hub.registerOrgWithVoucher(ORG_ID, ADMIN_HAT, OPERATOR_HAT, VOUCHER_HAT); + // Fund test accounts BEFORE using them for deposits vm.deal(owner, 100 ether); vm.deal(orgAdmin, 100 ether); @@ -929,7 +930,7 @@ contract PasskeyPaymasterIntegrationTest is Test { // Create a new org WITHOUT voucher hat bytes32 noVoucherOrgId = keccak256("NO_VOUCHER_ORG"); - vm.startPrank(owner); + vm.startPrank(poaManager); hub.registerOrg(noVoucherOrgId, ADMIN_HAT, OPERATOR_HAT); // No voucher hat! vm.stopPrank(); diff --git a/test/PaymasterHubSolidarity.t.sol b/test/PaymasterHubSolidarity.t.sol index 7506e92..6342ba5 100644 --- a/test/PaymasterHubSolidarity.t.sol +++ b/test/PaymasterHubSolidarity.t.sol @@ -287,10 +287,12 @@ contract PaymasterHubSolidarityTest is Test { vm.deal(user1, 100 ether); vm.deal(user2, 100 ether); - // Register orgs + // Register orgs (requires poaManager) + vm.startPrank(poaManager); hub.registerOrg(ORG_ALPHA, ADMIN_HAT, OPERATOR_HAT); hub.registerOrg(ORG_BETA, ADMIN_HAT, OPERATOR_HAT); hub.registerOrg(ORG_GAMMA, ADMIN_HAT, OPERATOR_HAT); + vm.stopPrank(); } // ============ Initialization Tests ============ @@ -316,9 +318,11 @@ contract PaymasterHubSolidarityTest is Test { function testOrgRegistration() public { bytes32 newOrgId = keccak256("NEW_ORG"); + vm.startPrank(poaManager); vm.expectEmit(true, false, false, true); emit OrgRegistered(newOrgId, ADMIN_HAT, OPERATOR_HAT); hub.registerOrg(newOrgId, ADMIN_HAT, OPERATOR_HAT); + vm.stopPrank(); PaymasterHub.OrgConfig memory config = hub.getOrgConfig(newOrgId); assertEq(config.adminHatId, ADMIN_HAT); @@ -931,4 +935,43 @@ contract PaymasterHubSolidarityTest is Test { assertFalse(requiresDeposit); assertEq(solidarityLimit, 0.006 ether); // 2x match } + + // ============ registerOrg Access Control Tests ============ + + function testRegisterOrgUnauthorizedReverts() public { + bytes32 newOrgId = keccak256("UNAUTHORIZED_ORG"); + + vm.prank(orgAdmin); + vm.expectRevert(PaymasterHub.NotPoaManager.selector); + hub.registerOrg(newOrgId, ADMIN_HAT, OPERATOR_HAT); + } + + function testRegisterOrgRandomUserReverts() public { + bytes32 newOrgId = keccak256("RANDOM_ORG"); + + vm.prank(user1); + vm.expectRevert(PaymasterHub.NotPoaManager.selector); + hub.registerOrg(newOrgId, ADMIN_HAT, OPERATOR_HAT); + } + + function testSetOrgRegistrar() public { + address registrar = address(0x99); + + vm.prank(poaManager); + hub.setOrgRegistrar(registrar); + + // Registrar can now register orgs + bytes32 newOrgId = keccak256("REGISTRAR_ORG"); + vm.prank(registrar); + hub.registerOrg(newOrgId, ADMIN_HAT, OPERATOR_HAT); + + PaymasterHub.OrgConfig memory config = hub.getOrgConfig(newOrgId); + assertEq(config.adminHatId, ADMIN_HAT); + } + + function testSetOrgRegistrarUnauthorizedReverts() public { + vm.prank(orgAdmin); + vm.expectRevert(PaymasterHub.NotPoaManager.selector); + hub.setOrgRegistrar(address(0x99)); + } } diff --git a/test/TaskManager.t.sol b/test/TaskManager.t.sol index 6b75fff..44c34de 100644 --- a/test/TaskManager.t.sol +++ b/test/TaskManager.t.sol @@ -5898,3 +5898,99 @@ contract MockToken is Test, IERC20 { tm.bootstrapProjectsAndTasks(moreProjects, moreTasks); } } + + /*──────────────────── Self-Review Tests ────────────────────*/ + contract TaskManagerSelfReviewTest is TaskManagerTestBase { + uint256 constant REVIEWER_HAT = 4; + address reviewer = makeAddr("reviewer"); + bytes32 projectId; + + function setUp() public { + setUpBase(); + setHat(reviewer, REVIEWER_HAT); + // CLAIM | REVIEW but NOT SELF_REVIEW + vm.prank(executor); + tm.setConfig( + TaskManager.ConfigKey.ROLE_PERM, abi.encode(REVIEWER_HAT, TaskPerm.CLAIM | TaskPerm.REVIEW) + ); + + projectId = _createDefaultProject("SELF_REVIEW", 10 ether); + } + + function test_SelfReviewBlockedWithoutPermission() public { + vm.prank(creator1); + tm.createTask(1 ether, bytes("task"), bytes32(0), projectId, address(0), 0, false); + + vm.prank(reviewer); + tm.claimTask(0); + + vm.prank(reviewer); + tm.submitTask(0, keccak256("work")); + + vm.prank(reviewer); + vm.expectRevert(TaskManager.SelfReviewNotAllowed.selector); + tm.completeTask(0); + } + + function test_SelfReviewAllowedWithPermission() public { + // Grant SELF_REVIEW in addition to CLAIM | REVIEW + vm.prank(executor); + tm.setConfig( + TaskManager.ConfigKey.ROLE_PERM, + abi.encode(REVIEWER_HAT, TaskPerm.CLAIM | TaskPerm.REVIEW | TaskPerm.SELF_REVIEW) + ); + + vm.prank(creator1); + tm.createTask(1 ether, bytes("task"), bytes32(0), projectId, address(0), 0, false); + + vm.prank(reviewer); + tm.claimTask(0); + + vm.prank(reviewer); + tm.submitTask(0, keccak256("work")); + + vm.prank(reviewer); + tm.completeTask(0); + + assertEq(token.balanceOf(reviewer), 1 ether); + } + + function test_PMCanAlwaysReviewOwnTask() public { + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.PROJECT_MANAGER, abi.encode(projectId, pm1, true)); + + vm.prank(pm1); + tm.createTask(1 ether, bytes("pm_task"), bytes32(0), projectId, address(0), 0, false); + + // PM bypasses _checkPerm, so can claim even without CLAIM flag + vm.prank(pm1); + tm.claimTask(0); + + vm.prank(pm1); + tm.submitTask(0, keccak256("pm_work")); + + // PM bypasses self-review check + vm.prank(pm1); + tm.completeTask(0); + + assertEq(token.balanceOf(pm1), 1 ether); + } + + function test_DifferentReviewerCanAlwaysComplete() public { + vm.prank(creator1); + tm.createTask(1 ether, bytes("task"), bytes32(0), projectId, address(0), 0, false); + + // reviewer claims and submits + vm.prank(reviewer); + tm.claimTask(0); + + vm.prank(reviewer); + tm.submitTask(0, keccak256("work")); + + // pm1 (different person with REVIEW permission) completes — always allowed + vm.prank(pm1); + tm.completeTask(0); + + assertEq(token.balanceOf(reviewer), 1 ether); + } + } From f86fe27e240f41cc49fc654f52c77e77ec0cf53d Mon Sep 17 00:00:00 2001 From: Hudson Headley <76409831+hudsonhrh@users.noreply.github.com> Date: Mon, 9 Feb 2026 13:11:19 -0500 Subject: [PATCH 12/13] feat: Add solidarity fund distribution pause/unpause mechanism (#88) Enable fee collection without distribution: PaymasterHub now supports pausing solidarity fund distribution to allow fee accumulation before enabling the tier matching and grace period systems. When paused, organizations must fund 100% of transactions from their own deposits while 1% fees still accumulate in the solidarity fund. Key changes: - Add distributionPaused boolean flag to SolidarityFund struct - Implement pauseSolidarityDistribution() and unpauseSolidarityDistribution() (PoaManager only) - Update _checkSolidarityAccess, _updateOrgFinancials, and _checkOrgBalance to respect paused state - Block POA onboarding when distribution is paused - Fix getOrgGraceStatus to return accurate values (solidarity=0) when paused - Add idempotency guards to pause/unpause functions (no-op when already in that state) Tests: - 16 new tests covering initialization, access control, state transitions, idempotency, and view function behavior when paused - All 643 existing tests passing Co-authored-by: Claude Haiku 4.5 --- src/PaymasterHub.sol | 78 +++++++++- test/PaymasterHubSolidarity.t.sol | 240 +++++++++++++++++++++++++++++- 2 files changed, 313 insertions(+), 5 deletions(-) diff --git a/src/PaymasterHub.sol b/src/PaymasterHub.sol index 6e4e7a1..db41f4a 100644 --- a/src/PaymasterHub.sol +++ b/src/PaymasterHub.sol @@ -54,6 +54,7 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG error InsufficientOrgBalance(); error OrgIsBanned(); error InsufficientFunds(); + error SolidarityDistributionIsPaused(); error VouchExpired(); error VouchAlreadyUsed(); error InvalidVouchSignature(); @@ -119,6 +120,8 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG event VouchUsed(bytes32 indexed orgId, address indexed account, address indexed voucher); event OnboardingConfigUpdated(uint128 maxGasPerCreation, uint128 dailyCreationLimit, bool enabled); event OnboardingAccountCreated(address indexed account, uint256 gasCost); + event SolidarityDistributionPaused(); + event SolidarityDistributionUnpaused(); // ============ Storage Variables ============ /// @custom:storage-location erc7201:poa.paymasterhub.main @@ -167,7 +170,8 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG uint128 balance; // Current solidarity fund balance uint32 numActiveOrgs; // Number of orgs with deposits > 0 uint16 feePercentageBps; // Fee as basis points (100 = 1%) - uint208 reserved; // Padding + bool distributionPaused; // When true, only collect fees, no payouts + uint200 reserved; // Padding } /** @@ -293,9 +297,10 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG main.hats = _hats; main.poaManager = _poaManager; - // Initialize solidarity fund with 1% fee + // Initialize solidarity fund with 1% fee, distribution paused (collection-only mode) SolidarityFund storage solidarity = _getSolidarityStorage(); solidarity.feePercentageBps = 100; // 1% + solidarity.distributionPaused = true; // Initialize grace period with defaults (90 days, 0.01 ETH ~$30 spend, 0.003 ETH ~$10 deposit) GracePeriodConfig storage grace = _getGracePeriodStorage(); @@ -413,6 +418,12 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG * @param maxCost Maximum cost of the operation (for solidarity limit check) */ function _checkSolidarityAccess(bytes32 orgId, uint256 maxCost) internal view { + SolidarityFund storage solidarity = _getSolidarityStorage(); + + // If distribution is paused, skip solidarity checks entirely + // Orgs pay 100% from deposits when distribution is paused + if (solidarity.distributionPaused) return; + mapping(bytes32 => OrgConfig) storage orgs = _getOrgsStorage(); mapping(bytes32 => OrgFinancials) storage financials = _getFinancialsStorage(); GracePeriodConfig storage grace = _getGracePeriodStorage(); @@ -617,6 +628,16 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG uint256 depositAvailable = uint256(org.deposited) > uint256(org.spent) ? uint256(org.deposited) - uint256(org.spent) : 0; + SolidarityFund storage solidarity = _getSolidarityStorage(); + + if (solidarity.distributionPaused) { + // When distribution is paused, org must cover 100% from deposits + if (depositAvailable < maxCost) { + revert InsufficientOrgBalance(); + } + return; + } + // If deposits alone cover the cost, no solidarity needed if (depositAvailable >= maxCost) return; @@ -725,9 +746,17 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG GracePeriodConfig storage grace = _getGracePeriodStorage(); SolidarityFund storage solidarity = _getSolidarityStorage(); - // Calculate 1% solidarity fee + // Calculate 1% solidarity fee (always collected, even when distribution is paused) uint256 solidarityFee = (actualGasCost * uint256(solidarity.feePercentageBps)) / 10000; + // If distribution is paused, pay 100% from org deposits, still collect fee + if (solidarity.distributionPaused) { + org.spent += uint128(actualGasCost); + solidarity.balance += uint128(solidarityFee); + emit SolidarityFeeCollected(orgId, solidarityFee); + return; + } + // Check if in initial grace period uint256 graceEndTime = config.registeredAt + (uint256(grace.initialGraceDays) * 1 days); bool inInitialGrace = block.timestamp < graceEndTime; @@ -1075,6 +1104,35 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG solidarity.feePercentageBps = feePercentageBps; } + /** + * @notice Pause solidarity fund distribution (collection-only mode) + * @dev When paused: 1% fees still collected, but no distribution to orgs. + * Orgs must fund 100% of gas costs from their own deposits. + * Only PoaManager can pause/unpause. + */ + function pauseSolidarityDistribution() external { + if (msg.sender != _getMainStorage().poaManager) revert NotPoaManager(); + SolidarityFund storage solidarity = _getSolidarityStorage(); + if (!solidarity.distributionPaused) { + solidarity.distributionPaused = true; + emit SolidarityDistributionPaused(); + } + } + + /** + * @notice Unpause solidarity fund distribution + * @dev When unpaused: normal grace period + tier matching resumes. + * Only PoaManager can pause/unpause. + */ + function unpauseSolidarityDistribution() external { + if (msg.sender != _getMainStorage().poaManager) revert NotPoaManager(); + SolidarityFund storage solidarity = _getSolidarityStorage(); + if (solidarity.distributionPaused) { + solidarity.distributionPaused = false; + emit SolidarityDistributionUnpaused(); + } + } + /** * @notice Configure POA onboarding for account creation from solidarity fund * @dev Only PoaManager can modify onboarding parameters @@ -1213,6 +1271,7 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG mapping(bytes32 => OrgConfig) storage orgs = _getOrgsStorage(); mapping(bytes32 => OrgFinancials) storage financials = _getFinancialsStorage(); GracePeriodConfig storage grace = _getGracePeriodStorage(); + SolidarityFund storage solidarity = _getSolidarityStorage(); OrgConfig storage config = orgs[orgId]; OrgFinancials storage org = financials[orgId]; @@ -1220,6 +1279,14 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG uint256 graceEndTime = config.registeredAt + (uint256(grace.initialGraceDays) * 1 days); inGrace = block.timestamp < graceEndTime; + // When distribution is paused, no solidarity is available regardless of grace/tier + if (solidarity.distributionPaused) { + spendRemaining = 0; + requiresDeposit = true; + solidarityLimit = 0; + return (inGrace, spendRemaining, requiresDeposit, solidarityLimit); + } + if (inGrace) { // During grace: track spending limit uint128 spendUsed = org.solidarityUsedThisPeriod; @@ -1415,6 +1482,10 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG // Check onboarding is enabled if (!onboarding.enabled) revert OnboardingDisabled(); + // Onboarding is paid from solidarity fund, so block when distribution is paused + SolidarityFund storage solidarity = _getSolidarityStorage(); + if (solidarity.distributionPaused) revert SolidarityDistributionIsPaused(); + // Check gas cost limit if (maxCost > onboarding.maxGasPerCreation) revert GasTooHigh(); @@ -1425,7 +1496,6 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG } // Check solidarity fund has sufficient balance - SolidarityFund storage solidarity = _getSolidarityStorage(); if (solidarity.balance < maxCost) revert InsufficientFunds(); // Subject key for onboarding is based on the account address (natural nonce) diff --git a/test/PaymasterHubSolidarity.t.sol b/test/PaymasterHubSolidarity.t.sol index 6342ba5..0d1eb24 100644 --- a/test/PaymasterHubSolidarity.t.sol +++ b/test/PaymasterHubSolidarity.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0-only pragma solidity ^0.8.24; -import {Test, console2} from "forge-std/Test.sol"; +import {Test, Vm, console2} from "forge-std/Test.sol"; import {PaymasterHub} from "../src/PaymasterHub.sol"; import {IPaymaster} from "../src/interfaces/IPaymaster.sol"; import {IEntryPoint} from "../src/interfaces/IEntryPoint.sol"; @@ -293,6 +293,11 @@ contract PaymasterHubSolidarityTest is Test { hub.registerOrg(ORG_BETA, ADMIN_HAT, OPERATOR_HAT); hub.registerOrg(ORG_GAMMA, ADMIN_HAT, OPERATOR_HAT); vm.stopPrank(); + + // Unpause distribution so existing tests work as before + // Pause-specific tests re-pause explicitly + vm.prank(poaManager); + hub.unpauseSolidarityDistribution(); } // ============ Initialization Tests ============ @@ -936,6 +941,239 @@ contract PaymasterHubSolidarityTest is Test { assertEq(solidarityLimit, 0.006 ether); // 2x match } + // ============ Distribution Pause Tests ============ + + event SolidarityDistributionPaused(); + event SolidarityDistributionUnpaused(); + + function testInitializedWithDistributionPaused() public { + // Deploy a fresh hub to test initialization state (setUp unpauses) + PaymasterHub freshImpl = new PaymasterHub(); + bytes memory initData = + abi.encodeWithSelector(PaymasterHub.initialize.selector, address(entryPoint), address(hats), poaManager); + ERC1967Proxy freshProxy = new ERC1967Proxy(address(freshImpl), initData); + PaymasterHub freshHub = PaymasterHub(payable(address(freshProxy))); + + PaymasterHub.SolidarityFund memory solidarity = freshHub.getSolidarityFund(); + assertTrue(solidarity.distributionPaused); + } + + function testPauseDistributionOnlyPoaManager() public { + vm.prank(orgAdmin); + vm.expectRevert(PaymasterHub.NotPoaManager.selector); + hub.pauseSolidarityDistribution(); + } + + function testUnpauseDistributionOnlyPoaManager() public { + vm.prank(orgAdmin); + vm.expectRevert(PaymasterHub.NotPoaManager.selector); + hub.unpauseSolidarityDistribution(); + } + + function testPauseEmitsEvent() public { + // setUp already unpaused, so we can test pausing + vm.prank(poaManager); + vm.expectEmit(false, false, false, true); + emit SolidarityDistributionPaused(); + hub.pauseSolidarityDistribution(); + + PaymasterHub.SolidarityFund memory solidarity = hub.getSolidarityFund(); + assertTrue(solidarity.distributionPaused); + } + + function testUnpauseEmitsEvent() public { + // Pause first + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + vm.prank(poaManager); + vm.expectEmit(false, false, false, true); + emit SolidarityDistributionUnpaused(); + hub.unpauseSolidarityDistribution(); + + PaymasterHub.SolidarityFund memory solidarity = hub.getSolidarityFund(); + assertFalse(solidarity.distributionPaused); + } + + function testPausedSolidarityFundStillAcceptsDeposits() public { + // Pause distribution + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + // Org deposits should still work + vm.prank(user1); + hub.depositForOrg{value: 0.01 ether}(ORG_ALPHA); + + PaymasterHub.OrgFinancials memory fin = hub.getOrgFinancials(ORG_ALPHA); + assertEq(fin.deposited, 0.01 ether); + } + + function testPausedSolidarityFundStillAcceptsDonations() public { + // Pause distribution + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + // Direct donations should still work + vm.prank(user1); + hub.donateToSolidarity{value: 1 ether}(); + + PaymasterHub.SolidarityFund memory solidarity = hub.getSolidarityFund(); + assertEq(solidarity.balance, 1 ether); + } + + function testPauseUnpauseRoundtrip() public { + // Starts unpaused (setUp unpaused it) + PaymasterHub.SolidarityFund memory s1 = hub.getSolidarityFund(); + assertFalse(s1.distributionPaused); + + // Pause + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + PaymasterHub.SolidarityFund memory s2 = hub.getSolidarityFund(); + assertTrue(s2.distributionPaused); + + // Unpause + vm.prank(poaManager); + hub.unpauseSolidarityDistribution(); + PaymasterHub.SolidarityFund memory s3 = hub.getSolidarityFund(); + assertFalse(s3.distributionPaused); + } + + function testPausedFeeConfigStillAdjustable() public { + // Pause distribution + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + // Can adjust fee percentage even when distribution is paused + vm.prank(poaManager); + hub.setSolidarityFee(200); // 2% + + PaymasterHub.SolidarityFund memory solidarity = hub.getSolidarityFund(); + assertEq(solidarity.feePercentageBps, 200); + assertTrue(solidarity.distributionPaused); // Still paused + } + + function testPausedGraceConfigStillAdjustable() public { + // Pause distribution + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + // Can adjust grace period config even when distribution is paused + vm.prank(poaManager); + hub.setGracePeriodConfig(120, 0.02 ether, 0.005 ether); + + PaymasterHub.GracePeriodConfig memory grace = hub.getGracePeriodConfig(); + assertEq(grace.initialGraceDays, 120); + assertEq(grace.maxSpendDuringGrace, 0.02 ether); + assertEq(grace.minDepositRequired, 0.005 ether); + } + + // ============ Pause Idempotency Tests ============ + + function testDoublePauseIsNoOp() public { + // Pause first + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + // Calling pause again should succeed but not emit event + vm.recordLogs(); + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + // Should have emitted zero events (no state change) + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(logs.length, 0); + + // State unchanged + PaymasterHub.SolidarityFund memory s = hub.getSolidarityFund(); + assertTrue(s.distributionPaused); + } + + function testDoubleUnpauseIsNoOp() public { + // Already unpaused from setUp + + // Calling unpause again should succeed but not emit event + vm.recordLogs(); + vm.prank(poaManager); + hub.unpauseSolidarityDistribution(); + + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(logs.length, 0); + + PaymasterHub.SolidarityFund memory s = hub.getSolidarityFund(); + assertFalse(s.distributionPaused); + } + + // ============ getOrgGraceStatus When Paused Tests ============ + + function testGetOrgGraceStatus_PausedDuringGrace() public { + // Pause distribution + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + // ORG_ALPHA was just registered, so it's in grace period + (bool inGrace, uint128 spendRemaining, bool requiresDeposit, uint256 solidarityLimit) = + hub.getOrgGraceStatus(ORG_ALPHA); + + // inGrace still reflects the time-based status (useful info) + assertTrue(inGrace); + // But solidarity is unavailable + assertEq(spendRemaining, 0); + assertTrue(requiresDeposit); + assertEq(solidarityLimit, 0); + } + + function testGetOrgGraceStatus_PausedPostGrace() public { + // Pause distribution + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + vm.warp(block.timestamp + 91 days); + + vm.prank(user1); + hub.depositForOrg{value: 0.003 ether}(ORG_ALPHA); + + (bool inGrace, uint128 spendRemaining, bool requiresDeposit, uint256 solidarityLimit) = + hub.getOrgGraceStatus(ORG_ALPHA); + + assertFalse(inGrace); + assertEq(spendRemaining, 0); + assertTrue(requiresDeposit); + assertEq(solidarityLimit, 0); // No match when paused + } + + function testGetOrgGraceStatus_UnpausedShowsNormalValues() public view { + // setUp already unpaused, so this should show normal grace values + (bool inGrace, uint128 spendRemaining, bool requiresDeposit, uint256 solidarityLimit) = + hub.getOrgGraceStatus(ORG_ALPHA); + + assertTrue(inGrace); + assertEq(spendRemaining, 0.01 ether); // Full grace spending available + assertFalse(requiresDeposit); // No deposit needed during grace + assertEq(solidarityLimit, 0.01 ether); // Grace limit + } + + function testGetOrgGraceStatus_PauseThenUnpauseRestoresMatch() public { + vm.warp(block.timestamp + 91 days); + + vm.prank(user1); + hub.depositForOrg{value: 0.003 ether}(ORG_ALPHA); + + // Pause — should show no match + vm.prank(poaManager); + hub.pauseSolidarityDistribution(); + + (,,, uint256 limit1) = hub.getOrgGraceStatus(ORG_ALPHA); + assertEq(limit1, 0); + + // Unpause — match restored + vm.prank(poaManager); + hub.unpauseSolidarityDistribution(); + + (,,, uint256 limit2) = hub.getOrgGraceStatus(ORG_ALPHA); + assertEq(limit2, 0.006 ether); // 2x match restored + } + // ============ registerOrg Access Control Tests ============ function testRegisterOrgUnauthorizedReverts() public { From dbf5fde7491025371caeb7bbb3dffea4422c04da Mon Sep 17 00:00:00 2001 From: Hudson Headley <76409831+hudsonhrh@users.noreply.github.com> Date: Mon, 9 Feb 2026 16:19:43 -0500 Subject: [PATCH 13/13] fix: Add upgrade safety hardening and comprehensive tests (#89) * fix: Add _disableInitializers() to 12 contracts and comprehensive upgrade safety tests - Added _disableInitializers() constructor to 8 contracts with no constructor (Executor, ParticipationToken, TaskManager, QuickJoin, EducationHub, PaymentManager, UniversalAccountRegistry, ImplementationRegistry) preventing re-initialization attacks on implementation contracts - Replaced weak constructor() initializer {} pattern in 4 contracts (HybridVoting, DirectDemocracyVoting, OrgRegistry, OrgDeployer) with _disableInitializers() for stronger initialization guards - Added implementation validation to PoaManager.upgradeBeacon() and addContractType() to reject EOA addresses - Added implementation validation to PaymasterHub._authorizeUpgrade() to prevent UUPS upgrades to invalid addresses - Removed misleading __gap[50] from PaymasterHub (unnecessary with ERC-7201 namespaced storage) - Fixed 8 test files to use BeaconProxy pattern instead of direct initialization of implementation contracts - Created UpgradeSafety.t.sol with 24 comprehensive tests covering re-initialization prevention, upgrade authorization, storage preservation, and SwitchableBeacon mode-switching safety - Created UpgradeEdgeCases.t.sol with 10 production-scenario tests covering full upgrade chains, multi-tenant isolation, mode cycling, sequential upgrades, and ownership transfer flows - All 696 tests pass with zero failures Co-Authored-By: Claude Opus 4.6 * fix: Update 9 test files to use proxy pattern for initialization Tests were directly calling initialize() on implementation contracts, which now revert due to _disableInitializers(). Wrapped all test setups in UpgradeableBeacon + BeaconProxy before calling initialize(). Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- src/DirectDemocracyVoting.sol | 5 +- src/EducationHub.sol | 5 + src/Executor.sol | 5 + src/HybridVoting.sol | 5 +- src/ImplementationRegistry.sol | 5 + src/OrgDeployer.sol | 5 +- src/OrgRegistry.sol | 4 +- src/ParticipationToken.sol | 5 + src/PaymasterHub.sol | 9 +- src/PaymentManager.sol | 5 + src/PoaManager.sol | 2 + src/QuickJoin.sol | 5 + src/TaskManager.sol | 5 + src/UniversalAccountRegistry.sol | 5 + test/EducationHub.t.sol | 10 +- test/Executor.t.sol | 6 +- test/ImplementationRegistry.t.sol | 6 +- test/Passkey.t.sol | 4 +- test/PaymentManagerMerkle.t.sol | 10 +- test/PoaManager.t.sol | 6 +- test/QuickJoin.t.sol | 10 +- test/TaskManager.t.sol | 10 +- test/UniversalAccountRegistry.t.sol | 6 +- test/UpgradeEdgeCases.t.sol | 383 ++++++++++++++++++++++++++++ test/UpgradeSafety.t.sol | 341 +++++++++++++++++++++++++ 25 files changed, 838 insertions(+), 24 deletions(-) create mode 100644 test/UpgradeEdgeCases.t.sol create mode 100644 test/UpgradeSafety.t.sol diff --git a/src/DirectDemocracyVoting.sol b/src/DirectDemocracyVoting.sol index f4bbe9b..38d6cb6 100644 --- a/src/DirectDemocracyVoting.sol +++ b/src/DirectDemocracyVoting.sol @@ -126,7 +126,10 @@ contract DirectDemocracyVoting is Initializable { event QuorumPercentageSet(uint8 pct); /* ─────────── Initialiser ─────────── */ - constructor() initializer {} + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } function initialize( address hats_, diff --git a/src/EducationHub.sol b/src/EducationHub.sol index 50d4edb..c2ecf80 100644 --- a/src/EducationHub.sol +++ b/src/EducationHub.sol @@ -77,6 +77,11 @@ contract EducationHub is Initializable, ContextUpgradeable, ReentrancyGuardUpgra event TokenSet(address indexed newToken); event HatsSet(address indexed newHats); + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /*────────── Initialiser ────────*/ function initialize( address tokenAddr, diff --git a/src/Executor.sol b/src/Executor.sol index b1b6b99..0d1d8c7 100644 --- a/src/Executor.sol +++ b/src/Executor.sol @@ -62,6 +62,11 @@ contract Executor is Initializable, OwnableUpgradeable, PausableUpgradeable, Ree event HatMinterAuthorized(address indexed minter, bool authorized); event HatsMinted(address indexed user, uint256[] hatIds); + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /* ─────────── Initialiser ─────────── */ function initialize(address owner_, address hats_) external initializer { if (owner_ == address(0) || hats_ == address(0)) revert ZeroAddress(); diff --git a/src/HybridVoting.sol b/src/HybridVoting.sol index 15aa819..97429ee 100644 --- a/src/HybridVoting.sol +++ b/src/HybridVoting.sol @@ -117,7 +117,10 @@ contract HybridVoting is Initializable { event QuorumSet(uint8 pct); /* ─────── Initialiser ─────── */ - constructor() initializer {} + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } function initialize( address hats_, diff --git a/src/ImplementationRegistry.sol b/src/ImplementationRegistry.sol index 869415e..ba8ea4c 100644 --- a/src/ImplementationRegistry.sol +++ b/src/ImplementationRegistry.sol @@ -50,6 +50,11 @@ contract ImplementationRegistry is Initializable, OwnableUpgradeable { bool latest ); + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /*───────── Initializer ─────────────*/ function initialize(address owner) external initializer { __Ownable_init(owner); diff --git a/src/OrgDeployer.sol b/src/OrgDeployer.sol index 2d522da..d5da0c3 100644 --- a/src/OrgDeployer.sol +++ b/src/OrgDeployer.sol @@ -140,7 +140,10 @@ contract OrgDeployer is Initializable { /*════════════════ INITIALIZATION ════════════════*/ - constructor() initializer {} + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } function initialize( address _governanceFactory, diff --git a/src/OrgRegistry.sol b/src/OrgRegistry.sol index 2033de0..937b57c 100644 --- a/src/OrgRegistry.sol +++ b/src/OrgRegistry.sol @@ -92,7 +92,9 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { event OrgMetadataAdminHatSet(bytes32 indexed orgId, uint256 hatId); /// @custom:oz-upgrades-unsafe-allow constructor - constructor() initializer {} + constructor() { + _disableInitializers(); + } /** * @dev Initializes the contract, replacing the constructor for upgradeable pattern diff --git a/src/ParticipationToken.sol b/src/ParticipationToken.sol index 04fdce7..fce7eca 100644 --- a/src/ParticipationToken.sol +++ b/src/ParticipationToken.sol @@ -70,6 +70,11 @@ contract ParticipationToken is Initializable, ERC20VotesUpgradeable, ReentrancyG event MemberHatSet(uint256 hat, bool allowed); event ApproverHatSet(uint256 hat, bool allowed); + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /*─────────── Initialiser ──────*/ function initialize( address executor_, diff --git a/src/PaymasterHub.sol b/src/PaymasterHub.sol index db41f4a..75dc2bb 100644 --- a/src/PaymasterHub.sol +++ b/src/PaymasterHub.sol @@ -1409,7 +1409,8 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG function _authorizeUpgrade(address newImplementation) internal override { MainStorage storage main = _getMainStorage(); if (msg.sender != main.poaManager) revert NotPoaManager(); - // newImplementation is intentionally not validated to allow flexibility + if (newImplementation == address(0)) revert ZeroAddress(); + if (newImplementation.code.length == 0) revert ContractNotDeployed(); } // ============ Internal Functions ============ @@ -1762,10 +1763,4 @@ contract PaymasterHub is IPaymaster, Initializable, UUPSUpgradeable, ReentrancyG receive() external payable { emit BountyFunded(msg.value, address(this).balance); } - - /** - * @dev Storage gap for future upgrades - * Reserves 50 storage slots for new variables in future versions - */ - uint256[50] private __gap; } diff --git a/src/PaymentManager.sol b/src/PaymentManager.sol index 5fdf431..fd72a30 100644 --- a/src/PaymentManager.sol +++ b/src/PaymentManager.sol @@ -63,6 +63,11 @@ contract PaymentManager is IPaymentManager, Initializable, OwnableUpgradeable, R INITIALIZER ──────────────────────────────────────────────────────────────────────────*/ + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /** * @notice Initializes the PaymentManager * @param _owner The address that will own the contract (typically the Executor) diff --git a/src/PoaManager.sol b/src/PoaManager.sol index 2c5a2ff..bdb16fb 100644 --- a/src/PoaManager.sol +++ b/src/PoaManager.sol @@ -72,6 +72,7 @@ contract PoaManager is Ownable(msg.sender) { /*──────────── Admin: add & bootstrap ───────────*/ function addContractType(string calldata typeName, address impl) external onlyOwner { if (impl == address(0)) revert ImplZero(); + if (impl.code.length == 0) revert ImplZero(); bytes32 tId = _id(typeName); if (address(beacons[tId]) != address(0)) revert TypeExists(); @@ -91,6 +92,7 @@ contract PoaManager is Ownable(msg.sender) { /*──────────── Admin: upgrade ───────────*/ function upgradeBeacon(string calldata typeName, address newImpl, string calldata version) external onlyOwner { if (newImpl == address(0)) revert ImplZero(); + if (newImpl.code.length == 0) revert ImplZero(); bytes32 tId = _id(typeName); UpgradeableBeacon beacon = beacons[tId]; if (address(beacon) == address(0)) revert TypeUnknown(); diff --git a/src/QuickJoin.sol b/src/QuickJoin.sol index 5b62163..1cd57b4 100644 --- a/src/QuickJoin.sol +++ b/src/QuickJoin.sol @@ -85,6 +85,11 @@ contract QuickJoin is Initializable, ContextUpgradeable, ReentrancyGuardUpgradea address indexed master, address indexed account, string username, bytes32 indexed credentialId, uint256[] hatIds ); + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /* ───────── Initialiser ───── */ function initialize( address executor_, diff --git a/src/TaskManager.sol b/src/TaskManager.sol index 9d316ec..41a5cec 100644 --- a/src/TaskManager.sol +++ b/src/TaskManager.sol @@ -168,6 +168,11 @@ contract TaskManager is Initializable, ContextUpgradeable { event TaskApplicationApproved(uint256 indexed id, address indexed applicant, address indexed approver); event ExecutorUpdated(address newExecutor); + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /*──────── Initialiser ───────*/ function initialize( address tokenAddress, diff --git a/src/UniversalAccountRegistry.sol b/src/UniversalAccountRegistry.sol index 7e77a22..13ba7c6 100644 --- a/src/UniversalAccountRegistry.sol +++ b/src/UniversalAccountRegistry.sol @@ -41,6 +41,11 @@ contract UniversalAccountRegistry is Initializable, OwnableUpgradeable { event UserDeleted(address indexed user, string oldUsername); event BatchRegistered(uint256 count); + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /*────────────────────────── Initializer ────────────────────────────*/ function initialize(address initialOwner) external initializer { if (initialOwner == address(0)) revert InvalidChars(); diff --git a/test/EducationHub.t.sol b/test/EducationHub.t.sol index 67b9034..40cf439 100644 --- a/test/EducationHub.t.sol +++ b/test/EducationHub.t.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.20; import "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; import {EducationHub, IParticipationToken} from "../src/EducationHub.sol"; import {ValidationLib} from "../src/libs/ValidationLib.sol"; @@ -63,7 +65,9 @@ contract MockPT is Test, IParticipationToken { hats.mintHat(MEMBER_HAT, creator); // creator is also a member hats.mintHat(MEMBER_HAT, learner); - hub = new EducationHub(); + EducationHub _hubImpl = new EducationHub(); + UpgradeableBeacon _hubBeacon = new UpgradeableBeacon(address(_hubImpl), address(this)); + hub = EducationHub(address(new BeaconProxy(address(_hubBeacon), ""))); uint256[] memory creatorHats = new uint256[](1); creatorHats[0] = CREATOR_HAT; uint256[] memory memberHats = new uint256[](1); @@ -87,7 +91,9 @@ contract MockPT is Test, IParticipationToken { } function testInitializeZeroAddressReverts() public { - EducationHub tmp = new EducationHub(); + EducationHub _tmpImpl = new EducationHub(); + UpgradeableBeacon _tmpBeacon = new UpgradeableBeacon(address(_tmpImpl), address(this)); + EducationHub tmp = EducationHub(address(new BeaconProxy(address(_tmpBeacon), ""))); uint256[] memory creatorHats = new uint256[](0); uint256[] memory memberHats = new uint256[](0); vm.expectRevert(EducationHub.ZeroAddress.selector); diff --git a/test/Executor.t.sol b/test/Executor.t.sol index c90a6e3..0b6819b 100644 --- a/test/Executor.t.sol +++ b/test/Executor.t.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.20; import "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; import "../src/Executor.sol"; import "./mocks/MockHats.sol"; @@ -22,7 +24,9 @@ contract ExecutorTest is Test { function setUp() public { hats = new MockHats(); - exec = new Executor(); + Executor impl = new Executor(); + UpgradeableBeacon beacon = new UpgradeableBeacon(address(impl), address(this)); + exec = Executor(payable(address(new BeaconProxy(address(beacon), "")))); exec.initialize(owner, address(hats)); target = new Target(); exec.setCaller(caller); diff --git a/test/ImplementationRegistry.t.sol b/test/ImplementationRegistry.t.sol index 1cb5b2a..d2cc2cc 100644 --- a/test/ImplementationRegistry.t.sol +++ b/test/ImplementationRegistry.t.sol @@ -2,13 +2,17 @@ pragma solidity ^0.8.20; import "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; import "../src/ImplementationRegistry.sol"; contract ImplementationRegistryTest is Test { ImplementationRegistry reg; function setUp() public { - reg = new ImplementationRegistry(); + ImplementationRegistry _regImpl = new ImplementationRegistry(); + UpgradeableBeacon _regBeacon = new UpgradeableBeacon(address(_regImpl), address(this)); + reg = ImplementationRegistry(address(new BeaconProxy(address(_regBeacon), ""))); reg.initialize(address(this)); } diff --git a/test/Passkey.t.sol b/test/Passkey.t.sol index 10a7f86..b76ac0c 100644 --- a/test/Passkey.t.sol +++ b/test/Passkey.t.sol @@ -157,7 +157,9 @@ contract PasskeyTest is Test { mockExecutor = new MockExecutor(); // Deploy account registry - accountRegistry = new UniversalAccountRegistry(); + UniversalAccountRegistry _uarImpl = new UniversalAccountRegistry(); + UpgradeableBeacon _uarBeacon = new UpgradeableBeacon(address(_uarImpl), owner); + accountRegistry = UniversalAccountRegistry(address(new BeaconProxy(address(_uarBeacon), ""))); accountRegistry.initialize(owner); vm.stopPrank(); diff --git a/test/PaymentManagerMerkle.t.sol b/test/PaymentManagerMerkle.t.sol index 4e82642..7e04b12 100644 --- a/test/PaymentManagerMerkle.t.sol +++ b/test/PaymentManagerMerkle.t.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; import {console2} from "forge-std/console2.sol"; import {PaymentManager} from "../src/PaymentManager.sol"; import {ParticipationToken} from "../src/ParticipationToken.sol"; @@ -50,7 +52,9 @@ contract PaymentManagerMerkleTest is Test { hats = new MockHats(); // Deploy participation token - participationToken = new ParticipationToken(); + ParticipationToken _ptImpl = new ParticipationToken(); + UpgradeableBeacon _ptBeacon = new UpgradeableBeacon(address(_ptImpl), address(this)); + participationToken = ParticipationToken(address(new BeaconProxy(address(_ptBeacon), ""))); uint256[] memory memberHats = new uint256[](1); memberHats[0] = memberHatId; @@ -60,7 +64,9 @@ contract PaymentManagerMerkleTest is Test { participationToken.initialize(executor, "Participation Token", "PART", address(hats), memberHats, approverHats); // Deploy payment manager - paymentManager = new PaymentManager(); + PaymentManager _pmImpl = new PaymentManager(); + UpgradeableBeacon _pmBeacon = new UpgradeableBeacon(address(_pmImpl), address(this)); + paymentManager = PaymentManager(payable(address(new BeaconProxy(address(_pmBeacon), "")))); paymentManager.initialize(executor, address(participationToken)); // Deploy payment token diff --git a/test/PoaManager.t.sol b/test/PoaManager.t.sol index 0db4be4..601f4ba 100644 --- a/test/PoaManager.t.sol +++ b/test/PoaManager.t.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.20; import "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; import "../src/PoaManager.sol"; import "../src/ImplementationRegistry.sol"; @@ -16,7 +18,9 @@ contract PoaManagerTest is Test { address owner = address(this); function setUp() public { - reg = new ImplementationRegistry(); + ImplementationRegistry _regImpl = new ImplementationRegistry(); + UpgradeableBeacon _regBeacon = new UpgradeableBeacon(address(_regImpl), address(this)); + reg = ImplementationRegistry(address(new BeaconProxy(address(_regBeacon), ""))); reg.initialize(owner); pm = new PoaManager(address(reg)); reg.transferOwnership(address(pm)); diff --git a/test/QuickJoin.t.sol b/test/QuickJoin.t.sol index 01a9055..18d3e3f 100644 --- a/test/QuickJoin.t.sol +++ b/test/QuickJoin.t.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.21; import "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; import "../src/QuickJoin.sol"; import "./mocks/MockHats.sol"; @@ -56,7 +58,9 @@ contract QuickJoinTest is Test { hats = new MockHats(); registry = new MockRegistry(); mockExecutor = new MockExecutorHatMinter(); - qj = new QuickJoin(); + QuickJoin _qjImpl = new QuickJoin(); + UpgradeableBeacon _qjBeacon = new UpgradeableBeacon(address(_qjImpl), address(this)); + qj = QuickJoin(address(new BeaconProxy(address(_qjBeacon), ""))); uint256[] memory memberHats = new uint256[](1); memberHats[0] = DEFAULT_HAT_ID; @@ -76,7 +80,9 @@ contract QuickJoinTest is Test { } function testInitializeZeroAddressReverts() public { - QuickJoin tmp = new QuickJoin(); + QuickJoin _tmpImpl = new QuickJoin(); + UpgradeableBeacon _tmpBeacon = new UpgradeableBeacon(address(_tmpImpl), address(this)); + QuickJoin tmp = QuickJoin(address(new BeaconProxy(address(_tmpBeacon), ""))); uint256[] memory memberHats = new uint256[](1); memberHats[0] = DEFAULT_HAT_ID; vm.expectRevert(QuickJoin.InvalidAddress.selector); diff --git a/test/TaskManager.t.sol b/test/TaskManager.t.sol index 44c34de..3108b66 100644 --- a/test/TaskManager.t.sol +++ b/test/TaskManager.t.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.20; import "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; import "forge-std/console.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; @@ -154,7 +156,9 @@ contract MockToken is Test, IERC20 { setHat(pm1, PM_HAT); setHat(member1, MEMBER_HAT); - tm = new TaskManager(); + TaskManager _tmImpl = new TaskManager(); + UpgradeableBeacon _tmBeacon = new UpgradeableBeacon(address(_tmImpl), address(this)); + tm = TaskManager(address(new BeaconProxy(address(_tmBeacon), ""))); lens = new TaskManagerLens(); uint256[] memory creatorHats = _hatArr(CREATOR_HAT); @@ -5440,7 +5444,9 @@ contract MockToken is Test, IERC20 { hats.mintHat(PM_HAT, pm1); hats.mintHat(MEMBER_HAT, member1); - tm = new TaskManager(); + TaskManager _tmImpl = new TaskManager(); + UpgradeableBeacon _tmBeacon = new UpgradeableBeacon(address(_tmImpl), address(this)); + tm = TaskManager(address(new BeaconProxy(address(_tmBeacon), ""))); lens = new TaskManagerLens(); uint256[] memory creatorHats = new uint256[](1); creatorHats[0] = CREATOR_HAT; diff --git a/test/UniversalAccountRegistry.t.sol b/test/UniversalAccountRegistry.t.sol index 10217a2..f5d75cc 100644 --- a/test/UniversalAccountRegistry.t.sol +++ b/test/UniversalAccountRegistry.t.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.20; import "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; import "../src/UniversalAccountRegistry.sol"; contract UARTest is Test { @@ -9,7 +11,9 @@ contract UARTest is Test { address user = address(1); function setUp() public { - reg = new UniversalAccountRegistry(); + UniversalAccountRegistry _regImpl = new UniversalAccountRegistry(); + UpgradeableBeacon _regBeacon = new UpgradeableBeacon(address(_regImpl), address(this)); + reg = UniversalAccountRegistry(address(new BeaconProxy(address(_regBeacon), ""))); reg.initialize(address(this)); } diff --git a/test/UpgradeEdgeCases.t.sol b/test/UpgradeEdgeCases.t.sol new file mode 100644 index 0000000..0f4de06 --- /dev/null +++ b/test/UpgradeEdgeCases.t.sol @@ -0,0 +1,383 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.20; + +import "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; +import {SwitchableBeacon} from "../src/SwitchableBeacon.sol"; + +/// @title UpgradeEdgeCasesTest +/// @notice Tests the full upgrade chain: PoaBeacon → SwitchableBeacon → BeaconProxy → delegatecall +/// with real proxy state, mode switches, multi-tenancy, and recovery scenarios. +contract UpgradeEdgeCasesTest is Test { + UpgradeableBeacon poaBeacon; + MockUpgradeableV1 implV1; + MockUpgradeableV2 implV2; + MockUpgradeableV3 implV3; + + function setUp() public { + implV1 = new MockUpgradeableV1(); + implV2 = new MockUpgradeableV2(); + implV3 = new MockUpgradeableV3(); + poaBeacon = new UpgradeableBeacon(address(implV1), address(this)); + } + + /// @dev Helper: create a SwitchableBeacon in Mirror mode + a BeaconProxy using it + function _deployMirrorProxy() internal returns (SwitchableBeacon switchable, MockUpgradeableV1 proxy) { + switchable = new SwitchableBeacon(address(this), address(poaBeacon), address(0), SwitchableBeacon.Mode.Mirror); + BeaconProxy bp = new BeaconProxy(address(switchable), ""); + proxy = MockUpgradeableV1(address(bp)); + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 1: Full upgrade chain flows through Mirror to proxy + // ══════════════════════════════════════════════════════════════════════ + + function testPoaBeaconUpgradeFlowsThroughMirrorToProxy() public { + (SwitchableBeacon switchable, MockUpgradeableV1 proxy) = _deployMirrorProxy(); + + // Write state through V1 + proxy.setValue(42); + assertEq(proxy.value(), 42); + assertEq(proxy.version(), 1); + + // Upgrade POA beacon to V2 + poaBeacon.upgradeTo(address(implV2)); + + // Verify chain: proxy → switchable → poaBeacon → V2 + assertEq(switchable.implementation(), address(implV2)); + + // State preserved, V2 features available + MockUpgradeableV2 proxyV2 = MockUpgradeableV2(address(proxy)); + assertEq(proxyV2.value(), 42); + assertEq(proxyV2.version(), 2); + proxyV2.setNewField(99); + assertEq(proxyV2.newField(), 99); + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 2: Pinned proxy ignores POA upgrade + // ══════════════════════════════════════════════════════════════════════ + + function testPinnedProxyIgnoresPoaUpgrade() public { + (SwitchableBeacon switchable, MockUpgradeableV1 proxy) = _deployMirrorProxy(); + + proxy.setValue(77); + assertEq(proxy.version(), 1); + + // Pin to current V1 + switchable.pinToCurrent(); + assertFalse(switchable.isMirrorMode()); + + // Upgrade POA beacon to V2 + poaBeacon.upgradeTo(address(implV2)); + + // Proxy still on V1 + assertEq(proxy.version(), 1); + assertEq(proxy.value(), 77); + assertEq(switchable.implementation(), address(implV1)); + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 3: Two proxies with divergent modes (multi-tenancy) + // ══════════════════════════════════════════════════════════════════════ + + function testTwoProxiesDivergentModes() public { + // Org1: Mirror mode + (SwitchableBeacon switchable1, MockUpgradeableV1 proxy1) = _deployMirrorProxy(); + // Org2: starts Mirror, will pin + (SwitchableBeacon switchable2, MockUpgradeableV1 proxy2) = _deployMirrorProxy(); + + // Write distinct state + proxy1.setValue(10); + proxy2.setValue(20); + + // Org2 pins to current V1 + switchable2.pinToCurrent(); + + // Upgrade POA beacon to V2 + poaBeacon.upgradeTo(address(implV2)); + + // Org1 (mirror) → V2 + MockUpgradeableV2 proxy1V2 = MockUpgradeableV2(address(proxy1)); + assertEq(proxy1V2.version(), 2); + assertEq(proxy1V2.value(), 10); + + // Org2 (pinned) → still V1 + assertEq(proxy2.version(), 1); + assertEq(proxy2.value(), 20); + + // No cross-contamination + assertEq(switchable1.implementation(), address(implV2)); + assertEq(switchable2.implementation(), address(implV1)); + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 4: State preserved through Mirror → Static → Mirror cycle + // ══════════════════════════════════════════════════════════════════════ + + function testProxyStatePreservedThroughMirrorStaticMirrorCycle() public { + (SwitchableBeacon switchable, MockUpgradeableV1 proxy) = _deployMirrorProxy(); + + // 1. Mirror mode, V1 + proxy.setValue(42); + assertEq(proxy.value(), 42); + assertEq(proxy.version(), 1); + + // 2. Pin → Static + switchable.pinToCurrent(); + assertFalse(switchable.isMirrorMode()); + + // 3. Upgrade POA to V2 (proxy should NOT see it) + poaBeacon.upgradeTo(address(implV2)); + assertEq(proxy.version(), 1); + assertEq(proxy.value(), 42); + + // 4. Back to Mirror + switchable.setMirror(address(poaBeacon)); + assertTrue(switchable.isMirrorMode()); + + // 5. Now on V2, state preserved + MockUpgradeableV2 proxyV2 = MockUpgradeableV2(address(proxy)); + assertEq(proxyV2.version(), 2); + assertEq(proxyV2.value(), 42); + + // V2 features work + proxyV2.setNewField(100); + assertEq(proxyV2.newField(), 100); + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 5: pinToCurrent reverts when mirror returns zero + // ══════════════════════════════════════════════════════════════════════ + + function testPinToCurrentWhenMirrorReturnsZero() public { + MockBrokenBeacon brokenBeacon = new MockBrokenBeacon(); + + SwitchableBeacon switchable = + new SwitchableBeacon(address(this), address(brokenBeacon), address(0), SwitchableBeacon.Mode.Mirror); + + // pinToCurrent should revert - mirror returns address(0) + vm.expectRevert(SwitchableBeacon.ImplNotSet.selector); + switchable.pinToCurrent(); + + // State unchanged - still in Mirror mode + assertTrue(switchable.isMirrorMode()); + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 6: Recovery from broken mirror via pin to known good + // ══════════════════════════════════════════════════════════════════════ + + function testRecoveryFromBrokenMirrorViaPinToKnownGood() public { + MockBrokenBeacon brokenBeacon = new MockBrokenBeacon(); + + SwitchableBeacon switchable = + new SwitchableBeacon(address(this), address(brokenBeacon), address(0), SwitchableBeacon.Mode.Mirror); + + // implementation() reverts + vm.expectRevert(SwitchableBeacon.ImplNotSet.selector); + switchable.implementation(); + + // tryGetImplementation returns failure + (bool success,) = switchable.tryGetImplementation(); + assertFalse(success); + + // Recovery: pin to known-good implementation + switchable.pin(address(implV1)); + assertEq(switchable.implementation(), address(implV1)); + assertFalse(switchable.isMirrorMode()); + + // Proxy using this beacon now works + BeaconProxy bp = new BeaconProxy(address(switchable), ""); + MockUpgradeableV1 proxy = MockUpgradeableV1(address(bp)); + proxy.setValue(55); + assertEq(proxy.value(), 55); + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 7: Multiple sequential upgrades preserve state + // ══════════════════════════════════════════════════════════════════════ + + function testMultipleSequentialUpgradesPreserveState() public { + (, MockUpgradeableV1 proxy) = _deployMirrorProxy(); + + // V1: set value + proxy.setValue(10); + assertEq(proxy.version(), 1); + + // Upgrade to V2, set V2 field + poaBeacon.upgradeTo(address(implV2)); + MockUpgradeableV2 proxyV2 = MockUpgradeableV2(address(proxy)); + assertEq(proxyV2.version(), 2); + assertEq(proxyV2.value(), 10); // V1 state preserved + proxyV2.setNewField(20); + + // Upgrade to V3, set V3 field + poaBeacon.upgradeTo(address(implV3)); + MockUpgradeableV3 proxyV3 = MockUpgradeableV3(address(proxy)); + assertEq(proxyV3.version(), 3); + assertEq(proxyV3.value(), 10); // V1 state preserved + assertEq(proxyV3.newField(), 20); // V2 state preserved + proxyV3.setThirdField(30); + assertEq(proxyV3.thirdField(), 30); + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 8: Pin to arbitrary impl not from mirror + // ══════════════════════════════════════════════════════════════════════ + + function testPinToArbitraryImplNotFromMirror() public { + (SwitchableBeacon switchable, MockUpgradeableV1 proxy) = _deployMirrorProxy(); + + proxy.setValue(33); + assertEq(proxy.version(), 1); + + // Pin directly to V3 (never served by mirror, which has V1) + switchable.pin(address(implV3)); + assertEq(switchable.implementation(), address(implV3)); + assertFalse(switchable.isMirrorMode()); + + // Proxy now uses V3 + MockUpgradeableV3 proxyV3 = MockUpgradeableV3(address(proxy)); + assertEq(proxyV3.version(), 3); + assertEq(proxyV3.value(), 33); // State preserved + + // POA beacon upgrade to V2 has no effect + poaBeacon.upgradeTo(address(implV2)); + assertEq(proxyV3.version(), 3); // Still pinned to V3 + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 9: setMirror to a different POA beacon + // ══════════════════════════════════════════════════════════════════════ + + function testSetMirrorToDifferentPoaBeacon() public { + // BeaconA has V1, BeaconB has V2 + UpgradeableBeacon beaconB = new UpgradeableBeacon(address(implV2), address(this)); + + (SwitchableBeacon switchable, MockUpgradeableV1 proxy) = _deployMirrorProxy(); + proxy.setValue(50); + assertEq(proxy.version(), 1); // tracking beaconA (V1) + + // Switch to tracking beaconB + switchable.setMirror(address(beaconB)); + MockUpgradeableV2 proxyV2 = MockUpgradeableV2(address(proxy)); + assertEq(proxyV2.version(), 2); // now on V2 + assertEq(proxyV2.value(), 50); // state preserved + + // Upgrade beaconA to V3 - no effect (tracking beaconB now) + poaBeacon.upgradeTo(address(implV3)); + assertEq(proxyV2.version(), 2); // still V2 + + // Upgrade beaconB to V3 - this one matters + beaconB.upgradeTo(address(implV3)); + MockUpgradeableV3 proxyV3 = MockUpgradeableV3(address(proxy)); + assertEq(proxyV3.version(), 3); + assertEq(proxyV3.value(), 50); // state preserved through all transitions + } + + // ══════════════════════════════════════════════════════════════════════ + // Test 10: Ownership transfer and subsequent pin + // ══════════════════════════════════════════════════════════════════════ + + function testBeaconOwnershipTransferAndSubsequentPin() public { + address factory = address(this); + address executor = address(0xE1E2); + + SwitchableBeacon switchable = + new SwitchableBeacon(factory, address(poaBeacon), address(0), SwitchableBeacon.Mode.Mirror); + + // Factory initiates transfer to executor + switchable.transferOwnership(executor); + assertEq(switchable.owner(), factory); + assertEq(switchable.pendingOwner(), executor); + + // Factory can still manage beacon (owner until accepted) + switchable.pin(address(implV1)); + assertEq(switchable.implementation(), address(implV1)); + switchable.setMirror(address(poaBeacon)); // back to mirror + + // Executor accepts ownership + vm.prank(executor); + switchable.acceptOwnership(); + assertEq(switchable.owner(), executor); + assertEq(switchable.pendingOwner(), address(0)); + + // Factory can no longer manage + vm.expectRevert(SwitchableBeacon.NotOwner.selector); + switchable.pin(address(implV2)); + + vm.expectRevert(SwitchableBeacon.NotOwner.selector); + switchable.setMirror(address(poaBeacon)); + + vm.expectRevert(SwitchableBeacon.NotOwner.selector); + switchable.pinToCurrent(); + + // Executor can manage + vm.prank(executor); + switchable.pin(address(implV2)); + assertEq(switchable.implementation(), address(implV2)); + } +} + +// ══════════════════════════════════════════════════════════════════════ +// Mock implementations with storage-compatible layout progression +// ══════════════════════════════════════════════════════════════════════ + +contract MockUpgradeableV1 { + uint256 public value; // slot 0 + + function setValue(uint256 v) external { + value = v; + } + + function version() external pure returns (uint256) { + return 1; + } +} + +contract MockUpgradeableV2 { + uint256 public value; // slot 0 (same as V1) + uint256 public newField; // slot 1 (new, doesn't overwrite) + + function setValue(uint256 v) external { + value = v; + } + + function setNewField(uint256 v) external { + newField = v; + } + + function version() external pure returns (uint256) { + return 2; + } +} + +contract MockUpgradeableV3 { + uint256 public value; // slot 0 + uint256 public newField; // slot 1 + uint256 public thirdField; // slot 2 + + function setValue(uint256 v) external { + value = v; + } + + function setNewField(uint256 v) external { + newField = v; + } + + function setThirdField(uint256 v) external { + thirdField = v; + } + + function version() external pure returns (uint256) { + return 3; + } +} + +contract MockBrokenBeacon { + function implementation() external pure returns (address) { + return address(0); + } +} diff --git a/test/UpgradeSafety.t.sol b/test/UpgradeSafety.t.sol new file mode 100644 index 0000000..39601fb --- /dev/null +++ b/test/UpgradeSafety.t.sol @@ -0,0 +1,341 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.20; + +import "forge-std/Test.sol"; +import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol"; +import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; +import "@openzeppelin-contracts-upgradeable/contracts/proxy/utils/Initializable.sol"; + +import {Executor} from "../src/Executor.sol"; +import {HybridVoting} from "../src/HybridVoting.sol"; +import {DirectDemocracyVoting} from "../src/DirectDemocracyVoting.sol"; +import {ParticipationToken} from "../src/ParticipationToken.sol"; +import {QuickJoin} from "../src/QuickJoin.sol"; +import {TaskManager} from "../src/TaskManager.sol"; +import {EducationHub} from "../src/EducationHub.sol"; +import {PaymentManager} from "../src/PaymentManager.sol"; +import {UniversalAccountRegistry} from "../src/UniversalAccountRegistry.sol"; +import {ImplementationRegistry} from "../src/ImplementationRegistry.sol"; +import {OrgRegistry} from "../src/OrgRegistry.sol"; +import {OrgDeployer} from "../src/OrgDeployer.sol"; +import {EligibilityModule} from "../src/EligibilityModule.sol"; +import {ToggleModule} from "../src/ToggleModule.sol"; +import {PasskeyAccount} from "../src/PasskeyAccount.sol"; +import {PasskeyAccountFactory} from "../src/PasskeyAccountFactory.sol"; +import {PaymasterHub} from "../src/PaymasterHub.sol"; +import {PoaManager} from "../src/PoaManager.sol"; +import {SwitchableBeacon} from "../src/SwitchableBeacon.sol"; + +/// @title UpgradeSafetyTest +/// @notice Comprehensive tests verifying upgrade safety invariants for all upgradeable contracts +contract UpgradeSafetyTest is Test { + address constant OWNER = address(0xA); + address constant HATS = address(0xB); + address constant UNAUTHORIZED = address(0xDEAD); + + // ══════════════════════════════════════════════════════════════════════ + // SECTION 1: Re-initialization prevention + // Every implementation contract must revert when initialize() is called + // ══════════════════════════════════════════════════════════════════════ + + function testExecutorImplCannotBeInitialized() public { + Executor impl = new Executor(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, HATS); + } + + function testParticipationTokenImplCannotBeInitialized() public { + ParticipationToken impl = new ParticipationToken(); + uint256[] memory hats = new uint256[](0); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, "Token", "TKN", HATS, hats, hats); + } + + function testTaskManagerImplCannotBeInitialized() public { + TaskManager impl = new TaskManager(); + uint256[] memory hats = new uint256[](0); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, HATS, hats, OWNER, OWNER); + } + + function testQuickJoinImplCannotBeInitialized() public { + QuickJoin impl = new QuickJoin(); + uint256[] memory hats = new uint256[](0); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, HATS, OWNER, OWNER, hats); + } + + function testEducationHubImplCannotBeInitialized() public { + EducationHub impl = new EducationHub(); + uint256[] memory hats = new uint256[](0); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, HATS, OWNER, hats, hats); + } + + function testPaymentManagerImplCannotBeInitialized() public { + PaymentManager impl = new PaymentManager(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, OWNER); + } + + function testUniversalAccountRegistryImplCannotBeInitialized() public { + UniversalAccountRegistry impl = new UniversalAccountRegistry(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER); + } + + function testImplementationRegistryImplCannotBeInitialized() public { + ImplementationRegistry impl = new ImplementationRegistry(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER); + } + + function testHybridVotingImplCannotBeInitialized() public { + HybridVoting impl = new HybridVoting(); + uint256[] memory hats = new uint256[](0); + address[] memory targets = new address[](0); + HybridVoting.ClassConfig[] memory classes = new HybridVoting.ClassConfig[](0); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(HATS, OWNER, hats, targets, 51, classes); + } + + function testDirectDemocracyVotingImplCannotBeInitialized() public { + DirectDemocracyVoting impl = new DirectDemocracyVoting(); + uint256[] memory hats = new uint256[](0); + address[] memory targets = new address[](0); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(HATS, OWNER, hats, hats, targets, 51); + } + + function testOrgRegistryImplCannotBeInitialized() public { + OrgRegistry impl = new OrgRegistry(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, HATS); + } + + function testEligibilityModuleImplCannotBeInitialized() public { + EligibilityModule impl = new EligibilityModule(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, HATS, OWNER); + } + + function testToggleModuleImplCannotBeInitialized() public { + ToggleModule impl = new ToggleModule(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER); + } + + function testPaymasterHubImplCannotBeInitialized() public { + PaymasterHub impl = new PaymasterHub(); + // PaymasterHub requires entryPoint to be a contract + address mockEntryPoint = address(new MockEntryPoint()); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(mockEntryPoint, HATS, OWNER); + } + + function testPasskeyAccountImplCannotBeInitialized() public { + PasskeyAccount impl = new PasskeyAccount(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, bytes32(uint256(1)), bytes32(uint256(2)), bytes32(uint256(3)), OWNER, 1 days); + } + + function testPasskeyAccountFactoryImplCannotBeInitialized() public { + PasskeyAccountFactory impl = new PasskeyAccountFactory(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, OWNER, OWNER, 1 days); + } + + function testOrgDeployerImplCannotBeInitialized() public { + OrgDeployer impl = new OrgDeployer(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER, OWNER, OWNER, OWNER, OWNER, HATS, OWNER, OWNER); + } + + // ══════════════════════════════════════════════════════════════════════ + // SECTION 2: PoaManager upgrade authorization + // ══════════════════════════════════════════════════════════════════════ + + function testPoaManagerUpgradeOnlyOwner() public { + ImplementationRegistry reg = new ImplementationRegistry(); + // Initialize via proxy to bypass _disableInitializers on impl + UpgradeableBeacon regBeacon = new UpgradeableBeacon(address(reg), address(this)); + BeaconProxy regProxy = new BeaconProxy(address(regBeacon), ""); + ImplementationRegistry(address(regProxy)).initialize(address(this)); + + PoaManager pm = new PoaManager(address(regProxy)); + ImplementationRegistry(address(regProxy)).transferOwnership(address(pm)); + + // Deploy a real implementation to upgrade to + DummyImplV1 implV1 = new DummyImplV1(); + DummyImplV2 implV2 = new DummyImplV2(); + + pm.addContractType("TestType", address(implV1)); + + // Non-owner cannot upgrade + vm.prank(UNAUTHORIZED); + vm.expectRevert(); + pm.upgradeBeacon("TestType", address(implV2), "v2"); + + // Owner can upgrade + pm.upgradeBeacon("TestType", address(implV2), "v2"); + assertEq(pm.getCurrentImplementationById(keccak256("TestType")), address(implV2)); + } + + function testPoaManagerRejectsEOAImplementation() public { + ImplementationRegistry reg = new ImplementationRegistry(); + UpgradeableBeacon regBeacon = new UpgradeableBeacon(address(reg), address(this)); + BeaconProxy regProxy = new BeaconProxy(address(regBeacon), ""); + ImplementationRegistry(address(regProxy)).initialize(address(this)); + + PoaManager pm = new PoaManager(address(regProxy)); + ImplementationRegistry(address(regProxy)).transferOwnership(address(pm)); + + // addContractType rejects EOA + vm.expectRevert(PoaManager.ImplZero.selector); + pm.addContractType("TestType", address(0x1234)); + + // Set up a valid type first, then try upgrading to EOA + DummyImplV1 implV1 = new DummyImplV1(); + pm.addContractType("TestType", address(implV1)); + + vm.expectRevert(PoaManager.ImplZero.selector); + pm.upgradeBeacon("TestType", address(0x5678), "v2"); + } + + // ══════════════════════════════════════════════════════════════════════ + // SECTION 3: Storage preservation after beacon upgrade + // ══════════════════════════════════════════════════════════════════════ + + function testStoragePreservedAfterBeaconUpgrade() public { + // Deploy V1 implementation behind a beacon + UniversalAccountRegistry implV1 = new UniversalAccountRegistry(); + UpgradeableBeacon beacon = new UpgradeableBeacon(address(implV1), address(this)); + BeaconProxy proxy = new BeaconProxy(address(beacon), ""); + + // Initialize and set state via proxy + UniversalAccountRegistry registry = UniversalAccountRegistry(address(proxy)); + registry.initialize(address(this)); + + // Register a user + registry.registerAccount("alice"); + assertEq(registry.getUsername(address(this)), "alice"); + + // Deploy V2 and upgrade beacon + UniversalAccountRegistry implV2 = new UniversalAccountRegistry(); + beacon.upgradeTo(address(implV2)); + + // Verify state is preserved after upgrade + assertEq(registry.getUsername(address(this)), "alice"); + } + + // ══════════════════════════════════════════════════════════════════════ + // SECTION 4: SwitchableBeacon mode-switch safety + // ══════════════════════════════════════════════════════════════════════ + + function testSwitchableBeaconMirrorToStaticPreservesProxy() public { + // Set up POA global beacon with V1 + DummyImplV1 implV1 = new DummyImplV1(); + UpgradeableBeacon poaBeacon = new UpgradeableBeacon(address(implV1), address(this)); + + // Create SwitchableBeacon in Mirror mode + SwitchableBeacon switchable = + new SwitchableBeacon(address(this), address(poaBeacon), address(0), SwitchableBeacon.Mode.Mirror); + + // Verify mirror mode returns POA impl + assertEq(switchable.implementation(), address(implV1)); + + // Switch to static (pin to current) + switchable.pinToCurrent(); + assertEq(switchable.implementation(), address(implV1)); + assertTrue(!switchable.isMirrorMode()); + + // Upgrade POA beacon to V2 - static beacon should NOT follow + DummyImplV2 implV2 = new DummyImplV2(); + poaBeacon.upgradeTo(address(implV2)); + + // SwitchableBeacon still returns V1 (pinned) + assertEq(switchable.implementation(), address(implV1)); + + // Switch back to mirror - should now follow V2 + switchable.setMirror(address(poaBeacon)); + assertEq(switchable.implementation(), address(implV2)); + assertTrue(switchable.isMirrorMode()); + } + + function testSwitchableBeaconOnlyOwnerCanSwitchModes() public { + DummyImplV1 implV1 = new DummyImplV1(); + UpgradeableBeacon poaBeacon = new UpgradeableBeacon(address(implV1), address(this)); + + SwitchableBeacon switchable = + new SwitchableBeacon(address(this), address(poaBeacon), address(0), SwitchableBeacon.Mode.Mirror); + + // Non-owner cannot pin + vm.prank(UNAUTHORIZED); + vm.expectRevert(SwitchableBeacon.NotOwner.selector); + switchable.pin(address(implV1)); + + // Non-owner cannot set mirror + vm.prank(UNAUTHORIZED); + vm.expectRevert(SwitchableBeacon.NotOwner.selector); + switchable.setMirror(address(poaBeacon)); + + // Non-owner cannot pin to current + vm.prank(UNAUTHORIZED); + vm.expectRevert(SwitchableBeacon.NotOwner.selector); + switchable.pinToCurrent(); + } + + // ══════════════════════════════════════════════════════════════════════ + // SECTION 5: Proxy initialization works correctly + // ══════════════════════════════════════════════════════════════════════ + + function testProxyCanBeInitializedWhileImplBlocked() public { + // Implementation cannot be initialized (blocked by constructor) + ImplementationRegistry impl = new ImplementationRegistry(); + vm.expectRevert(Initializable.InvalidInitialization.selector); + impl.initialize(OWNER); + + // But proxy CAN be initialized through beacon + UpgradeableBeacon beacon = new UpgradeableBeacon(address(impl), address(this)); + BeaconProxy proxy = new BeaconProxy(address(beacon), ""); + ImplementationRegistry(address(proxy)).initialize(OWNER); + assertEq(ImplementationRegistry(address(proxy)).owner(), OWNER); + } + + function testProxyCannotBeInitializedTwice() public { + UniversalAccountRegistry impl = new UniversalAccountRegistry(); + UpgradeableBeacon beacon = new UpgradeableBeacon(address(impl), address(this)); + BeaconProxy proxy = new BeaconProxy(address(beacon), ""); + + // First initialization succeeds + UniversalAccountRegistry(address(proxy)).initialize(OWNER); + + // Second initialization reverts + vm.expectRevert(Initializable.InvalidInitialization.selector); + UniversalAccountRegistry(address(proxy)).initialize(address(0xBEEF)); + } +} + +// ══════════════════════════════════════════════════════════════════════ +// Mock contracts for upgrade testing +// ══════════════════════════════════════════════════════════════════════ + +contract DummyImplV1 { + uint256 public version = 1; +} + +contract DummyImplV2 { + uint256 public version = 2; +} + +contract MockEntryPoint { + mapping(address => uint256) public balances; + + function balanceOf(address account) external view returns (uint256) { + return balances[account]; + } + + function depositTo(address account) external payable { + balances[account] += msg.value; + } +}