diff --git a/src/DirectDemocracyVoting.sol b/src/DirectDemocracyVoting.sol index a009ca7..38d6cb6 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 ─────────── */ @@ -125,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_, @@ -408,9 +412,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..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, @@ -198,7 +203,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 +234,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 b554d05..4b055e5 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 @@ -105,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; @@ -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 ═════════════════════════════════════════*/ @@ -189,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); @@ -760,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); @@ -787,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 @@ -797,6 +806,9 @@ 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"); @@ -804,6 +816,38 @@ contract EligibilityModule is Initializable, IHatsEligibility { 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 +962,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..0d1d8c7 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 { @@ -61,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(); @@ -77,10 +83,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); } @@ -129,6 +132,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/HybridVoting.sol b/src/HybridVoting.sol index 9cc0ac5..97429ee 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 ─────── */ @@ -116,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 ca6225d..d5da0c3 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, @@ -139,7 +140,10 @@ contract OrgDeployer is Initializable { /*════════════════ INITIALIZATION ════════════════*/ - constructor() initializer {} + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } function initialize( address _governanceFactory, @@ -173,14 +177,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; } @@ -325,6 +327,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 58390b6..937b57c 100644 --- a/src/OrgRegistry.sol +++ b/src/OrgRegistry.sol @@ -1,9 +1,10 @@ -// SPDX‑License‑Identifier: MIT +// SPDX-License-Identifier: MIT 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"; +import {IHats} from "@hats-protocol/src/Interfaces/IHats.sol"; /* ─────────── Custom errors ─────────── */ error InvalidParam(); @@ -12,6 +13,7 @@ error OrgUnknown(); error TypeTaken(); error ContractUnknown(); error NotOrgExecutor(); +error NotOrgMetadataAdmin(); error OwnerOnlyDuringBootstrap(); // deployer tried after bootstrap error AutoUpgradeRequired(); // deployer must set autoUpgrade=true @@ -59,6 +61,10 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { mapping(bytes32 => mapping(uint256 => uint256)) roleHatOf; // orgId => roleIndex => hatId bytes32[] orgIds; 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 @@ -82,19 +88,30 @@ 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); /// @custom:oz-upgrades-unsafe-allow constructor - constructor() initializer {} + constructor() { + _disableInitializers(); + } /** * @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 ═════════════════ */ @@ -157,6 +174,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 +190,57 @@ contract OrgRegistry is Initializable, OwnableUpgradeable { emit MetaUpdated(orgId, newName, newMetadataHash); } + /** + * @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) + */ + 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(); + + // 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) { + metadataAdminHat = l.topHatOf[orgId]; + } + if (metadataAdminHat == 0) revert NotOrgMetadataAdmin(); + + if (!hats.isWearerOfHat(msg.sender, metadataAdminHat)) revert NotOrgMetadataAdmin(); + + emit MetaUpdated(orgId, newName, newMetadataHash); + } + + /** + * @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 as fallback) + */ + 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.metadataAdminHatOf[orgId] = hatId; + emit OrgMetadataAdminHatSet(orgId, hatId); + } + + /** + * @dev Get the metadata admin hat for an org (returns 0 if not set, meaning topHat is used) + */ + function getOrgMetadataAdminHat(bytes32 orgId) external view returns (uint256) { + return _layout().metadataAdminHatOf[orgId]; + } + /* ══════════ CONTRACT REGISTRATION ══════════ */ /** * ‑ During **bootstrap** (`o.bootstrap == true`) the registry owner _may_ @@ -296,21 +370,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/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/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..75dc2bb 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 @@ -126,6 +129,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)) @@ -166,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 } /** @@ -292,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(); @@ -335,6 +341,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(); @@ -410,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(); @@ -611,18 +625,27 @@ 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; + + SolidarityFund storage solidarity = _getSolidarityStorage(); - // 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) { + 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; + + // Deposits are fully exhausted — revert early before solidarity check + if (depositAvailable == 0) { + revert InsufficientOrgBalance(); + } + // Partial coverage: solidarity will cover the rest (validated by _checkSolidarityAccess) } /** @@ -723,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; @@ -1033,6 +1064,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 @@ -1063,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 @@ -1201,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]; @@ -1208,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; @@ -1330,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 ============ @@ -1403,6 +1483,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(); @@ -1413,7 +1497,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) @@ -1665,13 +1748,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); @@ -1683,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/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 f79fd86..41a5cec 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" @@ -162,10 +163,16 @@ 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); + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + /*──────── Initialiser ───────*/ function initialize( address tokenAddress, @@ -271,6 +278,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 +299,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 { @@ -498,10 +505,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)); @@ -513,6 +529,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/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/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/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 d7aefae..ebe87b3 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 ───────────*/ @@ -680,8 +685,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 @@ -721,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)); @@ -3674,4 +3685,336 @@ 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"); + } + + 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/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 18c19e7..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); @@ -70,4 +74,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/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/OrgRegistry.t.sol b/test/OrgRegistry.t.sol index 957b60d..20c32d8 100644 --- a/test/OrgRegistry.t.sol +++ b/test/OrgRegistry.t.sol @@ -5,13 +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"); + 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))); + bytes memory data = abi.encodeCall(OrgRegistry.initialize, (address(this), address(mockHats))); ERC1967Proxy proxy = new ERC1967Proxy(address(impl), data); reg = OrgRegistry(address(proxy)); } @@ -25,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))); + } } diff --git a/test/Passkey.t.sol b/test/Passkey.t.sol index 222b833..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(); @@ -1061,4 +1063,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..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"; @@ -287,10 +287,17 @@ 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(); + + // Unpause distribution so existing tests work as before + // Pause-specific tests re-pause explicitly + vm.prank(poaManager); + hub.unpauseSolidarityDistribution(); } // ============ Initialization Tests ============ @@ -316,9 +323,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 +940,276 @@ contract PaymasterHubSolidarityTest is Test { assertFalse(requiresDeposit); 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 { + 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/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/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..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); @@ -3214,6 +3218,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 +5042,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 */ @@ -4872,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; @@ -5330,3 +5904,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); + } + } 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; + } +}