From 24f8f74ce90e1d8bd1e26a3e5b0a7322cfe53476 Mon Sep 17 00:00:00 2001 From: hudsonhrh Date: Wed, 13 May 2026 13:14:48 -0400 Subject: [PATCH] feat(TaskManager): project folders + organizer hat (v4 upgrade) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an IPFS-rooted folder tree for projects, gated by a configurable organizer hat. Folder structure (names, parents, ordering, project assignments) lives off-chain in a JSON document; only the root hash sits on-chain, swapped atomically via `setFolders(expectedRoot, newRoot)` with CAS-guarded concurrency. Strict permissions: executor or organizer hat only — creator hats deliberately do NOT inherit, to avoid silent reparenting of the whole tree by widely-distributed creator roles. Also: full NatSpec pass across TaskManager errors, events, and externals; 13 new folder tests + storage-preservation upgrade test (199 + 25 pass); v4 upgrade script with `DryRun_GnosisUpgrade` exercised against a live Gnosis fork. CLAUDE.md tightened to require Claude to run the sim (not punt to the user) and to pick `VERSION` by querying `getVersionCount` + `cast code` on every target chain instead of guessing. Follow-up issue #158 tracks splitting TaskManager into shared-storage libraries (mirror HybridVoting pattern). Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 44 ++- .../upgrades/UpgradeTaskManagerFolders.s.sol | 316 ++++++++++++++++++ src/TaskManager.sol | 259 +++++++++++++- test/TaskManager.t.sol | 160 +++++++++ test/UpgradeSafety.t.sol | 81 +++++ 5 files changed, 844 insertions(+), 16 deletions(-) create mode 100644 script/upgrades/UpgradeTaskManagerFolders.s.sol diff --git a/CLAUDE.md b/CLAUDE.md index 904e165..37df677 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,6 +83,8 @@ Any new forge script that mutates on-chain state (`Hub.adminCall`, `Hub.adminCallCrossChain`, `Satellite.adminCall`, beacon upgrades, `setRulesBatch`, etc.) must be simulated against a real RPC fork before being called complete. `forge build` + a unit test selector check is **not** enough — transport/permission/struct-decoding bugs only show up against live state. +**Claude runs the sim — do not punt to the user.** The public RPC aliases in `foundry.toml` (`gnosis`, `arbitrum`, etc.) require no env vars, no keys, no auth for fork sims (no `--broadcast`, just `--fork-url `). Writing the `SimX` sibling and listing it as "user follow-up" is **not** a substitute. The task is unfinished until Claude has executed the sim and seen `PASS` output. If the user is the one running it, that's a process failure on Claude's part. + **Required setup for every new mutating script:** 1. Pair every `BroadcastX` contract with a `SimX` sibling that exercises the exact same call path under `vm.prank()`. @@ -93,7 +95,47 @@ Any new forge script that mutates on-chain state (`Hub.adminCall`, `Hub.adminCal ```sh forge script :SimX --fork-url -vvv ``` -6. Production profile (`FOUNDRY_PROFILE=production`) on this branch currently has a pre-existing `Stack too deep` issue independent of any single script — default profile is the one that has to compile cleanly for every change. +6. **Pick `VERSION` by querying the on-chain ImplementationRegistry, then double-checking the CREATE2 slot.** Two independent collision surfaces exist and you must probe both: + - **Registry collision** — `ImplementationRegistry.registerImplementation(...)` reverts `VersionExists` if `(typeName, version)` is already in storage. Query via `getImplementation(typeName, version)` — succeeds = taken, reverts `VersionUnknown` = free. + - **CREATE2 collision** — `DeterministicDeployer.deploy(salt, code)` no-ops if any bytecode exists at the predicted address. This is independent of the registry: it's not uncommon for an old CREATE2 slot to contain code that was later registered under a *different* version string (e.g. on Gnosis today, the `v3` CREATE2 slot holds bytecode that the registry knows as `v1`). Query via `cast code` against `dd.computeAddress(dd.computeSalt(typeName, version))`. + + **Do not use event logs as a source of truth.** `ImplementationRegistered` events accumulate over the proxy's lifetime, including dev-time registrations that were later wiped by re-initialization. Only `getVersionCount` + `getVersionIdAt` reflect current storage. Likewise, do not grep prior `script/upgrades/*.s.sol` for "the last version" — that's brittle and out of date the moment someone broadcasts without committing the script. + + The anchor is `getVersionCount(typeName)`: it gives a lower-bound starting point. Probe from there forward, checking both surfaces. Resolve the registry address per chain once (Gnosis: `0x72c16812aE2a6819F4d0D9E432A3818712fa5c63`; Arbitrum: look up via `PoaManagerHub.registry()` on `0xB72840B343654eAfb2CFf7acC4Fc6b59E6c3CC71`), then run this loop per chain: + + ```sh + DD=0x4aC8B5ebEb9D8C3dE3180ddF381D552d59e8835a + TYPE=TaskManager + chain=gnosis + registry=0x72c16812aE2a6819F4d0D9E432A3818712fa5c63 + + count=$(cast call --rpc-url $chain $registry 'getVersionCount(string)(uint256)' "$TYPE" 2>&1 | grep -oE '^[0-9]+' | head -1) + echo "$TYPE on $chain: registry has $count versions — probing v$((count + 1)) upward" + + for n in $(seq $((count + 1)) $((count + 20))); do + v="v$n" + # Cast exit code is the reliable signal: 0 = call succeeded (version is registered), + # non-zero = revert (VersionUnknown). Do NOT grep the revert message — its text varies. + if cast call --rpc-url $chain $registry 'getImplementation(string,string)(address)' "$TYPE" "$v" >/dev/null 2>&1; then + reg_taken=yes + else + reg_taken=no + fi + salt=$(cast call --rpc-url $chain $DD 'computeSalt(string,string)(bytes32)' "$TYPE" "$v" 2>/dev/null) + addr=$(cast call --rpc-url $chain $DD 'computeAddress(bytes32)(address)' "$salt" 2>/dev/null) + code=$(cast code --rpc-url $chain "$addr" 2>/dev/null) + if [ "$code" = "0x" ]; then create2_taken=no; else create2_taken=yes; fi + if [ "$reg_taken" = no ] && [ "$create2_taken" = no ]; then + echo "$v: FREE ($addr)"; break + else + echo "$v: TAKEN (registry=$reg_taken create2=$create2_taken)" + fi + done + ``` + + Use the lowest `vN` that's FREE on **every** chain you plan to deploy to. Confirmed on this branch (2026-05-13): for `TaskManager` on Gnosis, this picks `v4` in two RPC iterations — `v3` was rejected because its CREATE2 slot is occupied (the bytecode there is registered under "v1" historically), `v4` is clean on both surfaces. Sim failure with a "selector missing from impl bytecode" error is the *late* signal of a CREATE2 collision — proactive probing is cheaper than re-running the sim. + +7. Production profile (`FOUNDRY_PROFILE=production`) on this branch currently has a pre-existing `Stack too deep` issue independent of any single script — default profile is the one that has to compile cleanly for every change. ## Subgraph (live deployment lookups) diff --git a/script/upgrades/UpgradeTaskManagerFolders.s.sol b/script/upgrades/UpgradeTaskManagerFolders.s.sol new file mode 100644 index 0000000..b22ce74 --- /dev/null +++ b/script/upgrades/UpgradeTaskManagerFolders.s.sol @@ -0,0 +1,316 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.20; + +import "forge-std/Script.sol"; +import "forge-std/console.sol"; +import {TaskManager} from "../../src/TaskManager.sol"; +import {PoaManagerHub} from "../../src/crosschain/PoaManagerHub.sol"; +import {PoaManager} from "../../src/PoaManager.sol"; +import {DeterministicDeployer} from "../../src/crosschain/DeterministicDeployer.sol"; + +/* + * ============================================================================ + * TaskManager Upgrade — folders + organizer hat (v4) + * ============================================================================ + * + * Adds project folders, organized off-chain in IPFS as a JSON tree. The contract + * stores only the root hash plus a designated `organizerHatIds` array; any + * wearer of those hats (or the Executor) can publish a new root via + * `setFolders(expectedCurrentRoot, newRoot)`. The CAS guard prevents two + * organizers editing the tree simultaneously from silently clobbering each + * other. + * + * Layout change: two new fields appended at the end of `Layout` — `bytes32 + * foldersRoot` and `uint256[] organizerHatIds`. Append-only; no reordering. + * Storage slot is unchanged. + * + * ABI change: one new external function (`setFolders`), one new ConfigKey + * (`ORGANIZER_HAT_ALLOWED`), two new events (`FoldersUpdated`, + * `OrganizerHatAllowed`), two new errors (`NotOrganizer`, `FoldersRootStale`), + * and two new lens variants (`t == 10` returns `foldersRoot`, `t == 11` returns + * the organizer hat array). The existing `setConfig` switch was also tightened + * to enumerate the project-branch keys explicitly so the new key doesn't fall + * through the open-ended `>= BOUNTY_CAP` check. + * + * Three-step cross-chain upgrade pattern (mirrors v2): + * 1. Deploy impl on Gnosis via DeterministicDeployer + * 2. Deploy on Arbitrum + upgradeBeaconCrossChain + * 3. Verify on Gnosis + * + * Usage: + * source .env && FOUNDRY_PROFILE=production forge script \ + * script/upgrades/UpgradeTaskManagerFolders.s.sol: \ + * --rpc-url --broadcast --slow + * ============================================================================ + */ + +address constant DD = 0x4aC8B5ebEb9D8C3dE3180ddF381D552d59e8835a; +address constant HUB = 0xB72840B343654eAfb2CFf7acC4Fc6b59E6c3CC71; +address constant GNOSIS_POA_MANAGER = 0x794fD39e75140ee1545B1B022E5486B7c863789b; +uint256 constant HYPERLANE_FEE = 0.005 ether; +// Previous TaskManager impl was registered at "v2" (see +// script/upgrades/UpgradeTaskManagerCreateTasksBatch.s.sol). "v3" is already +// occupied on Gnosis with non-folders bytecode (older experimental deploy); +// the DryRun sim caught this. Use "v4" for a fresh deterministic address with +// setFolders + organizer hat support. +string constant VERSION = "v4"; + +/** + * @title Step1_DeployImplOnGnosis + * @notice Deploy TaskManager v4 implementation on Gnosis via DD. + * + * Usage: + * source .env && FOUNDRY_PROFILE=production forge script \ + * script/upgrades/UpgradeTaskManagerFolders.s.sol:Step1_DeployImplOnGnosis \ + * --rpc-url gnosis --broadcast --slow + */ +contract Step1_DeployImplOnGnosis is Script { + function run() public { + uint256 deployerKey = vm.envOr("PRIVATE_KEY", vm.envUint("DEPLOYER_PRIVATE_KEY")); + DeterministicDeployer dd = DeterministicDeployer(DD); + + bytes32 salt = dd.computeSalt("TaskManager", VERSION); + address predicted = dd.computeAddress(salt); + console.log("\n=== Step 1: Deploy TaskManager v4 impl on Gnosis ==="); + console.log("Predicted:", predicted); + + if (predicted.code.length > 0) { + console.log("Already deployed. Skipping."); + return; + } + + vm.startBroadcast(deployerKey); + address deployed = dd.deploy(salt, type(TaskManager).creationCode); + vm.stopBroadcast(); + + require(deployed == predicted, "Address mismatch"); + console.log("Deployed:", deployed); + console.log("\nNext: Run Step2_UpgradeFromArbitrum on Arbitrum"); + } +} + +/** + * @title Step2_UpgradeFromArbitrum + * @notice Deploy impl on Arbitrum via DD, upgrade beacon cross-chain. + * + * Usage: + * source .env && FOUNDRY_PROFILE=production forge script \ + * script/upgrades/UpgradeTaskManagerFolders.s.sol:Step2_UpgradeFromArbitrum \ + * --rpc-url arbitrum --broadcast --slow + */ +contract Step2_UpgradeFromArbitrum is Script { + function run() public { + uint256 deployerKey = vm.envOr("PRIVATE_KEY", vm.envUint("DEPLOYER_PRIVATE_KEY")); + address deployer = vm.addr(deployerKey); + + PoaManagerHub hub = PoaManagerHub(payable(HUB)); + DeterministicDeployer dd = DeterministicDeployer(DD); + + require(hub.owner() == deployer, "Deployer must own Hub"); + require(!hub.paused(), "Hub is paused"); + + bytes32 salt = dd.computeSalt("TaskManager", VERSION); + address predicted = dd.computeAddress(salt); + console.log("\n=== Step 2: Upgrade TaskManager from Arbitrum ==="); + console.log("DD impl address:", predicted); + + vm.startBroadcast(deployerKey); + + if (predicted.code.length == 0) { + dd.deploy(salt, type(TaskManager).creationCode); + console.log("Deployed on Arbitrum"); + } else { + console.log("Already deployed on Arbitrum"); + } + + hub.upgradeBeaconCrossChain{value: HYPERLANE_FEE}("TaskManager", predicted, VERSION); + console.log("Beacon upgraded cross-chain"); + + vm.stopBroadcast(); + console.log("\nWait ~5 min for Hyperlane relay, then run Step3 on Gnosis."); + } +} + +/** + * @title Step3_VerifyGnosis + * @notice Verify the Gnosis beacon upgrade landed. + * + * Usage: + * forge script script/upgrades/UpgradeTaskManagerFolders.s.sol:Step3_VerifyGnosis \ + * --rpc-url gnosis + */ +contract Step3_VerifyGnosis is Script { + function run() public view { + DeterministicDeployer dd = DeterministicDeployer(DD); + bytes32 salt = dd.computeSalt("TaskManager", VERSION); + address expectedImpl = dd.computeAddress(salt); + + address currentImpl = PoaManager(GNOSIS_POA_MANAGER).getCurrentImplementationById(keccak256("TaskManager")); + + console.log("\n=== Step 3: Verify Gnosis TaskManager Upgrade ==="); + console.log("Expected impl:", expectedImpl); + console.log("Current impl: ", currentImpl); + + if (currentImpl == expectedImpl) { + console.log("PASS: TaskManager upgraded to v4 on Gnosis"); + console.log("\nNew capability: setFolders(bytes32 expectedCurrentRoot, bytes32 newRoot)"); + console.log(" - Folder tree (names/parents/order/assignments) lives in IPFS JSON"); + console.log(" - On-chain stores only the root hash + organizer hat array"); + console.log(" - CAS guard: pass current root to avoid silent overwrite"); + console.log(" - Permission: executor OR wearer of any ORGANIZER_HAT_ALLOWED hat"); + } else { + console.log("WAITING: Hyperlane message not yet relayed."); + } + } +} + +interface IOrgRegistry { + function orgIds(uint256 index) external view returns (bytes32); + function proxyOf(bytes32 orgId, bytes32 typeId) external view returns (address); +} + +/** + * @title DryRun_GnosisUpgrade + * @notice Pre-flight test on a Gnosis fork. Deploys impl via DD, upgrades the + * beacon, and exercises setFolders against a live, autoUpgrade-tracking + * TaskManager proxy. Does not broadcast. + * + * Asserts: + * 1. DD-predicted address matches deployed address. + * 2. PoaManager beacon updates to the new impl. + * 3. New `setFolders` selector exists in impl runtime bytecode. + * 4. A live TaskManager proxy on Gnosis (org #0 from OrgRegistry): + * a. Pre-existing storage is preserved (executor address survives + * the impl swap — proves Layout struct is compatible). + * b. The new lens variants `t == 10` (foldersRoot) and `t == 11` + * (organizerHatIds) are reachable through the proxy. + * c. A fresh-deploy proxy starts with foldersRoot == bytes32(0) + * and no organizer hats — confirming append-only field + * initialization matches Solidity default-zero semantics. + * d. Non-organizer caller is rejected with NotOrganizer. + * e. Executor can publish a folders root with CAS-guard zero. + * f. CAS guard catches a stale expectedCurrentRoot. + * + * Usage: + * FOUNDRY_PROFILE=production forge script \ + * script/upgrades/UpgradeTaskManagerFolders.s.sol:DryRun_GnosisUpgrade \ + * --rpc-url gnosis + */ +contract DryRun_GnosisUpgrade is Script { + address constant ORG_REGISTRY = 0x3744b372abc41589226313F2bB1dB3aCAa22A854; + + function run() public { + console.log("\n=== DRY RUN: TaskManager v4 upgrade on Gnosis fork ===\n"); + + DeterministicDeployer dd = DeterministicDeployer(DD); + PoaManager pm = PoaManager(GNOSIS_POA_MANAGER); + + // 1. Pre-state snapshot. + address implBefore = pm.getCurrentImplementationById(keccak256("TaskManager")); + console.log("Impl before:", implBefore); + + // 2. Step1 simulation: deploy v4 impl via DD. + bytes32 salt = dd.computeSalt("TaskManager", VERSION); + address predicted = dd.computeAddress(salt); + console.log("DD predicted impl:", predicted); + + address deployed; + if (predicted.code.length == 0) { + vm.prank(DeterministicDeployer(DD).owner()); + deployed = dd.deploy(salt, type(TaskManager).creationCode); + } else { + console.log("Already deployed at predicted (skipping deploy)"); + deployed = predicted; + } + require(deployed == predicted, "DryRun: DD address mismatch"); + require(deployed.code.length > 0, "DryRun: impl code missing"); + console.log("Deployed impl:", deployed); + + // 3. Step2 simulation: upgrade beacon as PoaManager owner. + address pmOwner = pm.owner(); + vm.prank(pmOwner); + pm.upgradeBeacon("TaskManager", deployed, VERSION); + address implAfter = pm.getCurrentImplementationById(keccak256("TaskManager")); + require(implAfter == deployed, "DryRun: beacon upgrade did not stick"); + console.log("Impl after :", implAfter); + + // 4. Selector presence in impl bytecode. + bytes4 sel = TaskManager.setFolders.selector; + bytes memory code = deployed.code; + bool found = false; + for (uint256 i; i + 4 <= code.length; ++i) { + if (code[i] == sel[0] && code[i + 1] == sel[1] && code[i + 2] == sel[2] && code[i + 3] == sel[3]) { + found = true; + break; + } + } + require(found, "DryRun: setFolders selector missing from impl bytecode"); + console.log("setFolders selector present in impl bytecode"); + + // 5. Live-proxy exercise. + _exerciseLiveProxy(); + + console.log("\n=== ALL DRY-RUN CHECKS PASSED ==="); + console.log("Safe to broadcast Step1/Step2/Step3 against mainnet."); + } + + function _exerciseLiveProxy() internal { + IOrgRegistry reg = IOrgRegistry(ORG_REGISTRY); + bytes32 orgId = reg.orgIds(0); + address proxy = reg.proxyOf(orgId, keccak256("TaskManager")); + require(proxy != address(0), "DryRun: no TaskManager proxy for org 0"); + TaskManager tm = TaskManager(proxy); + + console.log("\n--- Live-proxy exercise ---"); + console.log("orgId:", vm.toString(orgId)); + console.log("TaskManager proxy:", proxy); + + // 5a. Storage preservation: executor address must survive the impl swap. + bytes memory execData = tm.getLensData(4, ""); + address executor = abi.decode(execData, (address)); + require(executor != address(0), "DryRun: executor unset post-upgrade (storage drift?)"); + console.log("Executor (preserved):", executor); + + // 5b. New lens variants are reachable. + bytes32 rootInitial = abi.decode(tm.getLensData(10, ""), (bytes32)); + uint256[] memory organizerHats = abi.decode(tm.getLensData(11, ""), (uint256[])); + console.log("Initial folders root (bytes32):", vm.toString(rootInitial)); + console.log("Initial organizer hats count:", organizerHats.length); + + // 5c. Pre-existing org has never set folders or organizer hats -> defaults. + require(rootInitial == bytes32(0), "DryRun: folders root should default to zero on fresh upgrade"); + require(organizerHats.length == 0, "DryRun: organizer hats should default to empty on fresh upgrade"); + + // 5d. Non-organizer caller is rejected (use the DD owner EOA — definitely + // not the org's executor and not wearing any org hat). + address randomEoa = address(0xBEEF); + vm.prank(randomEoa); + (bool okRandom, bytes memory randomRet) = + proxy.call(abi.encodeCall(TaskManager.setFolders, (bytes32(0), keccak256("anything")))); + require(!okRandom, "DryRun: non-organizer call must revert"); + require(bytes4(randomRet) == TaskManager.NotOrganizer.selector, "DryRun: wrong revert reason for non-organizer"); + console.log("NotOrganizer revert path OK"); + + // 5e. Executor can publish a folders root with CAS-guard zero. + bytes32 root1 = keccak256("dryrun-root-1"); + vm.prank(executor); + tm.setFolders(bytes32(0), root1); + require(abi.decode(tm.getLensData(10, ""), (bytes32)) == root1, "DryRun: root1 did not land"); + console.log("Executor setFolders OK"); + + // 5f. CAS guard catches a stale expectedCurrentRoot. + bytes32 root2 = keccak256("dryrun-root-2"); + vm.prank(executor); + (bool okStale, bytes memory staleRet) = proxy.call(abi.encodeCall(TaskManager.setFolders, (bytes32(0), root2))); + require(!okStale, "DryRun: stale-root call must revert"); + require(bytes4(staleRet) == TaskManager.FoldersRootStale.selector, "DryRun: wrong revert for stale root"); + require(abi.decode(tm.getLensData(10, ""), (bytes32)) == root1, "DryRun: stale revert must not mutate state"); + console.log("FoldersRootStale CAS guard OK"); + + // 5g. Executor can chain a follow-up update with the correct current root. + vm.prank(executor); + tm.setFolders(root1, root2); + require(abi.decode(tm.getLensData(10, ""), (bytes32)) == root2, "DryRun: chained root did not land"); + console.log("Chained setFolders OK"); + } +} diff --git a/src/TaskManager.sol b/src/TaskManager.sol index 5579a77..ad54d57 100644 --- a/src/TaskManager.sol +++ b/src/TaskManager.sol @@ -29,21 +29,42 @@ contract TaskManager is Initializable, ContextUpgradeable { using ValidationLib for bytes; /*──────── Errors ───────*/ + /// @notice Project or task ID does not exist (or task id beyond `nextTaskId`). error NotFound(); + /// @notice Task status forbids this transition (e.g. completing an unclaimed task). error BadStatus(); + /// @notice Caller does not wear any creator hat and is not the executor. error NotCreator(); + /// @notice Caller is not the task's current claimer. error NotClaimer(); + /// @notice Caller is not the configured executor. error NotExecutor(); + /// @notice Caller is not the bootstrap deployer (or bootstrap phase is over). error NotDeployer(); + /// @notice Caller lacks the hat-derived permission and is not a project manager. error Unauthorized(); + /// @notice Address has not applied to this task. error NotApplicant(); + /// @notice Applicant has already submitted an application for this task. error AlreadyApplied(); + /// @notice Task is application-only; caller used the direct-claim path. error RequiresApplication(); + /// @notice Task does not accept applications; caller used the application path. error NoApplicationRequired(); + /// @notice Bootstrap task referenced a project index outside the project array. error InvalidIndex(); + /// @notice Claimer attempted to complete their own task without SELF_REVIEW permission. error SelfReviewNotAllowed(); + /// @notice Parallel calldata arrays have mismatched lengths. error ArrayLengthMismatch(); + /// @notice Batch input array is empty. error EmptyBatch(); + /// @notice Caller is neither the executor nor a wearer of any organizer hat. + error NotOrganizer(); + /// @notice CAS guard: caller-supplied current folders root does not match storage. + /// @param expected Root the caller believed was current. + /// @param actual Root that is actually current on-chain. + error FoldersRootStale(bytes32 expected, bytes32 actual); /*──────── Constants ─────*/ bytes4 public constant MODULE_ID = 0x54534b32; // "TSK2" @@ -60,7 +81,8 @@ contract TaskManager is Initializable, ContextUpgradeable { PROJECT_ROLE_PERM, BOUNTY_CAP, PROJECT_MANAGER, - PROJECT_CAP + PROJECT_CAP, + ORGANIZER_HAT_ALLOWED } /*──────── Data Types ────*/ @@ -141,6 +163,11 @@ contract TaskManager is Initializable, ContextUpgradeable { mapping(uint256 => mapping(address => bytes32)) taskApplications; // task ID => applicant => application hash address deployer; // OrgDeployer address for bootstrap operations mapping(uint256 => uint256) projectPermHatRefCount; // hat ID => number of projects with non-zero project mask + // ─── Folders (v3) ─── + // Folder tree (names, parents, ordering, project assignments) lives off-chain in IPFS. + // Only the root hash is on-chain; reorganization = swap the hash via setFolders. + bytes32 foldersRoot; + uint256[] organizerHatIds; // hats authorized to reorganize the folder tree } bytes32 private constant _STORAGE_SLOT = keccak256("poa.taskmanager.storage"); @@ -153,14 +180,27 @@ contract TaskManager is Initializable, ContextUpgradeable { } /*──────── Events ───────*/ + /// @notice A role hat of `hatType` was added or removed from its enumeration array. event HatSet(HatType hatType, uint256 hat, bool allowed); + /// @notice A project was created. `metadataHash` is an IPFS CID — not stored on-chain. event ProjectCreated(bytes32 indexed id, bytes title, bytes32 metadataHash, uint256 cap); + /// @notice The participation-token cap on a project changed. event ProjectCapUpdated(bytes32 indexed id, uint256 oldCap, uint256 newCap); + /// @notice A project manager was added or removed. event ProjectManagerUpdated(bytes32 indexed id, address indexed manager, bool isManager); + /// @notice A project and all of its hat-permission overrides were deleted. event ProjectDeleted(bytes32 indexed id); + /// @notice A hat's project-specific permission mask was updated. event ProjectRolePermSet(bytes32 indexed id, uint256 indexed hatId, uint8 mask); + /// @notice A per-project bounty-token cap changed. event BountyCapSet(bytes32 indexed projectId, address indexed token, uint256 oldCap, uint256 newCap); + /// @notice The IPFS root for this org's folder tree changed. + /// @dev Subgraph consumers resolve the JSON off-chain at `newRoot`. `oldRoot` lets indexers chain revisions. + event FoldersUpdated(bytes32 indexed newRoot, bytes32 indexed oldRoot, address indexed sender); + /// @notice A hat was added to or removed from the organizer-hat array. + event OrganizerHatAllowed(uint256 indexed hatId, bool allowed); + /// @notice A new task was created under `project`. event TaskCreated( uint256 indexed id, bytes32 indexed project, @@ -171,17 +211,27 @@ contract TaskManager is Initializable, ContextUpgradeable { bytes title, bytes32 metadataHash ); + /// @notice An unclaimed task's mutable fields were updated. event TaskUpdated( uint256 indexed id, uint256 payout, address bountyToken, uint256 bountyPayout, bytes title, bytes32 metadataHash ); + /// @notice A claimer submitted work for review. event TaskSubmitted(uint256 indexed id, bytes32 submissionHash); + /// @notice A task was claimed by `claimer`. event TaskClaimed(uint256 indexed id, address indexed claimer); + /// @notice A task was assigned to `assignee` by `assigner` (bypasses claim flow). event TaskAssigned(uint256 indexed id, address indexed assignee, address indexed assigner); + /// @notice A task was marked completed and payouts/bounties were dispatched. event TaskCompleted(uint256 indexed id, address indexed completer); + /// @notice A task was cancelled and its budget reservations rolled back. event TaskCancelled(uint256 indexed id, address indexed canceller); + /// @notice A submitted task was rejected and reverted to CLAIMED for resubmission. event TaskRejected(uint256 indexed id, address indexed rejector, bytes32 rejectionHash); + /// @notice An applicant submitted an application for a task that requires one. event TaskApplicationSubmitted(uint256 indexed id, address indexed applicant, bytes32 applicationHash); + /// @notice An application was approved and the task moved to CLAIMED for `applicant`. event TaskApplicationApproved(uint256 indexed id, address indexed applicant, address indexed approver); + /// @notice The executor address was set or changed. event ExecutorUpdated(address newExecutor); /// @custom:oz-upgrades-unsafe-allow constructor @@ -190,6 +240,15 @@ contract TaskManager is Initializable, ContextUpgradeable { } /*──────── Initialiser ───────*/ + /** + * @notice One-time proxy initializer. Wires the org's PT, Hats, executor, and + * (optional) bootstrap deployer; seeds the creator-hat array. + * @param tokenAddress Participation token (must implement `mint`). + * @param hatsAddress Hats Protocol contract. + * @param creatorHats Initial hat IDs allowed to create projects. + * @param executorAddress Executor address (DAO execution layer). + * @param deployerAddress OrgDeployer address for `bootstrapProjectsAndTasks`; may be zero. + */ function initialize( address tokenAddress, address hatsAddress, @@ -222,28 +281,42 @@ contract TaskManager is Initializable, ContextUpgradeable { } /*──────── Internal Check Functions ─────*/ + /// @dev Caller must wear a creator hat or be the executor; reverts NotCreator otherwise. function _requireCreator() internal view { Layout storage l = _layout(); address s = _msgSender(); if (!_hasCreatorHat(s) && s != l.executor) revert NotCreator(); } + /// @dev Reverts NotFound if the project does not exist. function _requireProjectExists(bytes32 pid) internal view { if (!_layout()._projects[pid].exists) revert NotFound(); } + /// @dev Reverts NotExecutor if the caller is not the configured executor. function _requireExecutor() internal view { if (_msgSender() != _layout().executor) revert NotExecutor(); } + /// @dev Caller must be the executor or wear any hat in `organizerHatIds`; reverts NotOrganizer otherwise. + function _requireOrganizer() internal view { + Layout storage l = _layout(); + address s = _msgSender(); + if (s == l.executor) return; + if (!HatManager.hasAnyHat(l.hats, l.organizerHatIds, s)) revert NotOrganizer(); + } + + /// @dev Caller must hold CREATE permission on `pid` (or be a project manager / executor). function _requireCanCreate(bytes32 pid) internal view { _checkPerm(pid, TaskPerm.CREATE); } + /// @dev Caller must hold CLAIM permission on the task's project. function _requireCanClaim(uint256 tid) internal view { _checkPerm(_layout()._tasks[tid].projectId, TaskPerm.CLAIM); } + /// @dev Caller must hold ASSIGN permission on `pid`. function _requireCanAssign(bytes32 pid) internal view { _checkPerm(pid, TaskPerm.ASSIGN); } @@ -333,6 +406,12 @@ contract TaskManager is Initializable, ContextUpgradeable { } } + /** + * @notice Delete a project and clear every hat's project-specific permission entries. + * @dev Permission: creator hat or executor. Does not reclaim spent participation tokens + * already minted by completed tasks; only erases project state and per-hat overrides. + * @param pid Project ID to delete. + */ function deleteProject(bytes32 pid) external { _requireCreator(); Layout storage l = _layout(); @@ -431,6 +510,17 @@ contract TaskManager is Initializable, ContextUpgradeable { } /*──────── Task Logic ───────*/ + /** + * @notice Create a task under `pid` with the given payout and optional bounty. + * @dev Permission: CREATE on `pid` (hat-derived) or project manager / executor. + * @param payout Participation-token payout amount. + * @param title Raw UTF-8 title (validated for length). + * @param metadataHash IPFS CID; emitted in `TaskCreated`, not stored. + * @param pid Project ID this task belongs to. + * @param bountyToken ERC-20 bounty token; `address(0)` for no bounty. + * @param bountyPayout Bounty amount in `bountyToken` units. + * @param requiresApplication If true, claimants must submit an application first. + */ function createTask( uint256 payout, bytes calldata title, @@ -510,6 +600,17 @@ contract TaskManager is Initializable, ContextUpgradeable { emit TaskCreated(id, pid, payout, bountyToken, bountyPayout, requiresApplication, title, metadataHash); } + /** + * @notice Update an UNCLAIMED task's payout, title, metadata, and bounty fields. + * @dev Permission: CREATE on the task's project. Reverts BadStatus once the task is claimed. + * Re-runs validation on the new values and adjusts both PT and bounty budgets. + * @param id Task ID. + * @param newPayout New participation-token payout. + * @param newTitle New title. + * @param newMetadataHash New IPFS CID (emitted; not stored). + * @param newBountyToken New bounty token (or `address(0)` to clear). + * @param newBountyPayout New bounty amount. + */ function updateTask( uint256 id, uint256 newPayout, @@ -554,6 +655,12 @@ contract TaskManager is Initializable, ContextUpgradeable { emit TaskUpdated(id, newPayout, newBountyToken, newBountyPayout, newTitle, newMetadataHash); } + /** + * @notice Claim an UNCLAIMED task that does not require an application. + * @dev Permission: CLAIM on the task's project. Reverts RequiresApplication for + * application-only tasks (use `applyForTask`). + * @param id Task ID. + */ function claimTask(uint256 id) external { _requireCanClaim(id); Layout storage l = _layout(); @@ -566,6 +673,12 @@ contract TaskManager is Initializable, ContextUpgradeable { emit TaskClaimed(id, _msgSender()); } + /** + * @notice Force-assign an UNCLAIMED task to `assignee`, bypassing the claim flow. + * @dev Permission: ASSIGN on the task's project. Task must be UNCLAIMED. + * @param id Task ID. + * @param assignee Address to record as the claimer. + */ function assignTask(uint256 id, address assignee) external { _requireCanAssign(_layout()._tasks[id].projectId); assignee.requireNonZeroAddress(); @@ -579,6 +692,13 @@ contract TaskManager is Initializable, ContextUpgradeable { emit TaskAssigned(id, assignee, _msgSender()); } + /** + * @notice Claimer submits their finished work for review. + * @dev Caller must be the task's current claimer; task must be CLAIMED. + * `submissionHash` must be non-zero (typically an IPFS CID). + * @param id Task ID. + * @param submissionHash IPFS CID of the submission payload. + */ function submitTask(uint256 id, bytes32 submissionHash) external { Layout storage l = _layout(); Task storage t = _task(l, id); @@ -590,6 +710,13 @@ contract TaskManager is Initializable, ContextUpgradeable { emit TaskSubmitted(id, submissionHash); } + /** + * @notice Approve a SUBMITTED task: mint participation tokens to the claimer and + * transfer the bounty (if any). + * @dev Permission: REVIEW on the project. If the caller is the claimer themself, + * they additionally need SELF_REVIEW unless they are a project manager / executor. + * @param id Task ID. + */ function completeTask(uint256 id) external { Layout storage l = _layout(); bytes32 pid = l._tasks[id].projectId; @@ -616,6 +743,12 @@ contract TaskManager is Initializable, ContextUpgradeable { emit TaskCompleted(id, _msgSender()); } + /** + * @notice Reject a SUBMITTED task. Task reverts to CLAIMED so the claimer can resubmit. + * @dev Permission: REVIEW on the project. `rejectionHash` must be non-zero (IPFS CID of feedback). + * @param id Task ID. + * @param rejectionHash IPFS CID of the rejection reasoning. + */ function rejectTask(uint256 id, bytes32 rejectionHash) external { Layout storage l = _layout(); _checkPerm(l._tasks[id].projectId, TaskPerm.REVIEW); @@ -627,6 +760,11 @@ contract TaskManager is Initializable, ContextUpgradeable { emit TaskRejected(id, _msgSender(), rejectionHash); } + /** + * @notice Cancel an UNCLAIMED task and roll back its PT/bounty budget reservations. + * @dev Permission: CREATE on the task's project. Pending applications are cleared. + * @param id Task ID. + */ function cancelTask(uint256 id) external { _requireCanCreate(_layout()._tasks[id].projectId); Layout storage l = _layout(); @@ -656,9 +794,11 @@ contract TaskManager is Initializable, ContextUpgradeable { /*──────── Application System ─────*/ /** - * @dev Submit application for a task with IPFS hash containing submission - * @param id Task ID to apply for - * @param applicationHash IPFS hash of the application/submission + * @notice Apply to claim a task that requires applications. + * @dev Permission: CLAIM on the task's project. Reverts AlreadyApplied if the + * caller already submitted an application for this task. + * @param id Task ID to apply for. + * @param applicationHash IPFS CID of the application/submission payload. */ function applyForTask(uint256 id, bytes32 applicationHash) external { _requireCanClaim(id); @@ -681,9 +821,11 @@ contract TaskManager is Initializable, ContextUpgradeable { } /** - * @dev Approve an application, moving task to CLAIMED status - * @param id Task ID - * @param applicant Address of the applicant to approve + * @notice Approve a pending application: the task moves to CLAIMED for `applicant` + * and the remaining applicants are dropped. + * @dev Permission: ASSIGN on the task's project. + * @param id Task ID. + * @param applicant Address of the applicant to approve. */ function approveApplication(uint256 id, address applicant) external { _requireCanAssign(_layout()._tasks[id].projectId); @@ -699,13 +841,18 @@ contract TaskManager is Initializable, ContextUpgradeable { } /** - * @dev Creates a task and immediately assigns it to the specified assignee in a single transaction. - * @param payout The payout amount for the task - * @param title Task title (required, raw UTF-8) - * @param metadataHash IPFS CID sha256 digest (optional, bytes32(0) valid) - * @param pid Project ID - * @param assignee Address to assign the task to - * @return taskId The ID of the created task + * @notice Create a task and assign it to `assignee` in a single transaction. + * @dev Permission: caller must hold both CREATE and ASSIGN on `pid`, or be a project + * manager / executor. The task is created in CLAIMED state with the assignee as claimer. + * @param payout Participation-token payout. + * @param title Raw UTF-8 task title. + * @param metadataHash IPFS CID; emitted, not stored. + * @param pid Project ID. + * @param assignee Address to assign the task to. + * @param bountyToken ERC-20 bounty token (or `address(0)` for none). + * @param bountyPayout Bounty amount in `bountyToken` units. + * @param requiresApplication Recorded on the task even though it's already claimed. + * @return taskId ID of the created task. */ function createAndAssignTask( uint256 payout, @@ -774,6 +921,21 @@ contract TaskManager is Initializable, ContextUpgradeable { } /*──────── Config Setter (Optimized) ─────── */ + /** + * @notice Executor-only setter for every mutable, non-project-creation parameter. + * @dev Single entry point keeps the executor surface area small. The `key` selects + * the operation and `value` is its ABI-encoded payload: + * - `EXECUTOR`: `abi.encode(address newExecutor)` — rotate the executor. + * - `CREATOR_HAT_ALLOWED`: `abi.encode(uint256 hatId, bool allowed)` — add/remove creator hat. + * - `ROLE_PERM`: `abi.encode(uint256 hatId, uint8 mask)` — set global permission mask. + * - `ORGANIZER_HAT_ALLOWED`: `abi.encode(uint256 hatId, bool allowed)` — add/remove folder organizer hat. + * - `BOUNTY_CAP`: `abi.encode(bytes32 pid, address token, uint256 newCap)` — set per-project bounty cap. + * - `PROJECT_MANAGER`: `abi.encode(bytes32 pid, address mgr, bool isManager)` — toggle PM. + * - `PROJECT_CAP`: `abi.encode(bytes32 pid, uint256 newCap)` — change PT cap. + * `PROJECT_ROLE_PERM` is intentionally not handled here; use {setProjectRolePerm}. + * @param key Which configuration field to mutate. + * @param value ABI-encoded payload matching the variant above. + */ function setConfig(ConfigKey key, bytes calldata value) external { _requireExecutor(); Layout storage l = _layout(); @@ -800,9 +962,16 @@ contract TaskManager is Initializable, ContextUpgradeable { return; } + if (key == ConfigKey.ORGANIZER_HAT_ALLOWED) { + (uint256 hat, bool allowed) = abi.decode(value, (uint256, bool)); + HatManager.setHatInArray(l.organizerHatIds, hat, allowed); + emit OrganizerHatAllowed(hat, allowed); + return; + } + // Project-related configs - consolidate common logic bytes32 pid; - if (key >= ConfigKey.BOUNTY_CAP) { + if (key == ConfigKey.BOUNTY_CAP || key == ConfigKey.PROJECT_MANAGER || key == ConfigKey.PROJECT_CAP) { pid = abi.decode(value, (bytes32)); Project storage p = l._projects[pid]; if (!p.exists) revert NotFound(); @@ -832,6 +1001,14 @@ contract TaskManager is Initializable, ContextUpgradeable { } } + /** + * @notice Replace a hat's permission mask on a specific project (overrides global). + * @dev Permission: creator hat or executor. Setting `mask` to zero removes the override + * and falls back to the hat's global mask (if any). + * @param pid Project ID. + * @param hatId Hat whose mask to set. + * @param mask Bitwise OR of `TaskPerm.CREATE|CLAIM|REVIEW|ASSIGN|SELF_REVIEW`. + */ function setProjectRolePerm(bytes32 pid, uint256 hatId, uint8 mask) external { _requireCreator(); _requireProjectExists(pid); @@ -844,6 +1021,33 @@ contract TaskManager is Initializable, ContextUpgradeable { emit ProjectRolePermSet(pid, hatId, mask); } + + /*──────── Folders ─────────*/ + /** + * @notice Update the IPFS root pointing to this org's folder tree. + * @dev Folder structure (names, parents, ordering, project assignments) lives + * off-chain in IPFS as JSON. Only the root hash is on-chain. Callers must + * pass the current root they observed off-chain; if the on-chain root has + * moved on (another organizer published first), the call reverts. The UI + * should then re-pin and retry against the new root. + * + * Permission: executor OR any wearer of an `organizerHatIds` hat. + * Creator hats deliberately do NOT inherit this power — creators are + * widely distributed and silent reparenting of the folder tree is a + * footgun. To grant a creator reorganize rights, add the creator's hat + * to `organizerHatIds` via `setConfig(ORGANIZER_HAT_ALLOWED, ...)`. + * + * @param expectedCurrentRoot The root the caller believes is current (CAS guard). + * @param newRoot The new IPFS root hash to publish (bytes32(0) clears). + */ + function setFolders(bytes32 expectedCurrentRoot, bytes32 newRoot) external { + _requireOrganizer(); + Layout storage l = _layout(); + bytes32 current = l.foldersRoot; + if (current != expectedCurrentRoot) revert FoldersRootStale(expectedCurrentRoot, current); + l.foldersRoot = newRoot; + emit FoldersUpdated(newRoot, current, _msgSender()); + } /*──────── Internal Perm helpers ─────*/ function _permMask(address user, bytes32 pid) internal view returns (uint8 m) { @@ -979,6 +1183,25 @@ contract TaskManager is Initializable, ContextUpgradeable { } /*──────── Minimal External Getters for Lens ─────── */ + /** + * @notice Read-only dispatcher used by `TaskManagerLens` to surface storage fields + * without bloating the proxy ABI. + * @dev Variants: + * - `1` → Task: `d = abi.encode(uint256 id)` → `(bytes32 projectId, uint96 payout, address claimer, uint96 bountyPayout, bool requiresApplication, Status status, address bountyToken)`. + * - `2` → Project: `d = abi.encode(bytes32 pid)` → `(uint128 cap, uint128 spent, bool exists)`. + * - `3` → Hats contract: `d = ""` → `(address hats)`. + * - `4` → Executor: `d = ""` → `(address executor)`. + * - `5` → Creator hats: `d = ""` → `(uint256[] hatIds)`. + * - `6` → Permission hats enumeration: `d = ""` → `(uint256[] hatIds)`. + * - `7` → Task applicants: `d = abi.encode(uint256 id)` → `(address[] applicants)`. + * - `8` → One applicant's hash: `d = abi.encode(uint256 id, address applicant)` → `(bytes32 hash)`. + * - `9` → Bounty budget: `d = abi.encode(bytes32 pid, address token)` → `(uint128 cap, uint128 spent)`. + * - `10` → Folders root: `d = ""` → `(bytes32 foldersRoot)`. + * - `11` → Organizer hats: `d = ""` → `(uint256[] hatIds)`. + * @param t Variant selector. + * @param d ABI-encoded variant payload (see above). + * @return ABI-encoded result whose shape depends on `t`. + */ function getLensData(uint8 t, bytes calldata d) external view returns (bytes memory) { Layout storage l = _layout(); if (t == 1) { @@ -1028,6 +1251,12 @@ contract TaskManager is Initializable, ContextUpgradeable { if (!p.exists) revert NotFound(); BudgetLib.Budget storage b = p.bountyBudgets[token]; return abi.encode(b.cap, b.spent); + } else if (t == 10) { + // FoldersRoot + return abi.encode(l.foldersRoot); + } else if (t == 11) { + // OrganizerHats + return abi.encode(HatManager.getHatArray(l.organizerHatIds)); } revert NotFound(); } diff --git a/test/TaskManager.t.sol b/test/TaskManager.t.sol index 611406e..c16ae76 100644 --- a/test/TaskManager.t.sol +++ b/test/TaskManager.t.sol @@ -7162,3 +7162,163 @@ contract MockToken is Test, IERC20 { assertEq(projectId, PID, "id 0 must be the post-revert task: counter did not advance"); } } + + /*──────────────── Folders + Organizer Hat ────────────────*/ + contract TaskManagerFoldersTest is TaskManagerTestBase { + uint256 constant ORGANIZER_HAT = 99; + address organizer = makeAddr("organizer"); + address randomUser = makeAddr("randomUser"); + bytes32 ROOT_A = keccak256("folders-root-a"); + bytes32 ROOT_B = keccak256("folders-root-b"); + bytes32 PID; + + function setUp() public { + setUpBase(); + setHat(organizer, ORGANIZER_HAT); + // Designate ORGANIZER_HAT as an authorized folders organizer. + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.ORGANIZER_HAT_ALLOWED, abi.encode(ORGANIZER_HAT, true)); + PID = _createDefaultProject("FOLDERS_PROJ", 0); + } + + function _foldersRoot() internal view returns (bytes32 root) { + bytes memory data = tm.getLensData(10, ""); + root = abi.decode(data, (bytes32)); + } + + function _organizerHats() internal view returns (uint256[] memory hats_) { + bytes memory data = tm.getLensData(11, ""); + hats_ = abi.decode(data, (uint256[])); + } + + /*───────── happy paths ─────────*/ + + function test_SetFolders_ByExecutor_Succeeds() public { + vm.prank(executor); + tm.setFolders(bytes32(0), ROOT_A); + assertEq(_foldersRoot(), ROOT_A, "root should be ROOT_A"); + } + + function test_SetFolders_ByOrganizerHat_Succeeds() public { + vm.prank(organizer); + tm.setFolders(bytes32(0), ROOT_A); + assertEq(_foldersRoot(), ROOT_A); + + // organizer can also chain a follow-up update with the current root + vm.prank(organizer); + tm.setFolders(ROOT_A, ROOT_B); + assertEq(_foldersRoot(), ROOT_B); + } + + function test_SetFolders_ZeroRootClear_Allowed() public { + vm.prank(organizer); + tm.setFolders(bytes32(0), ROOT_A); + + vm.prank(organizer); + tm.setFolders(ROOT_A, bytes32(0)); + assertEq(_foldersRoot(), bytes32(0), "root should clear to zero"); + } + + function test_SetFolders_EmitsFoldersUpdated_OldAndNewRoots() public { + vm.expectEmit(true, true, true, false, address(tm)); + emit TaskManager.FoldersUpdated(ROOT_A, bytes32(0), organizer); + vm.prank(organizer); + tm.setFolders(bytes32(0), ROOT_A); + + vm.expectEmit(true, true, true, false, address(tm)); + emit TaskManager.FoldersUpdated(ROOT_B, ROOT_A, executor); + vm.prank(executor); + tm.setFolders(ROOT_A, ROOT_B); + } + + /*───────── permission strictness ─────────*/ + + function test_RevertWhen_SetFolders_ByCreatorHat() public { + // creator1 wears CREATOR_HAT but not ORGANIZER_HAT — must revert. + vm.prank(creator1); + vm.expectRevert(TaskManager.NotOrganizer.selector); + tm.setFolders(bytes32(0), ROOT_A); + } + + function test_RevertWhen_SetFolders_ByProjectManager() public { + // Make pm1 a project manager on PID and prove that still doesn't grant folder rights. + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.PROJECT_MANAGER, abi.encode(PID, pm1, true)); + + vm.prank(pm1); + vm.expectRevert(TaskManager.NotOrganizer.selector); + tm.setFolders(bytes32(0), ROOT_A); + } + + function test_RevertWhen_SetFolders_ByRandom() public { + vm.prank(randomUser); + vm.expectRevert(TaskManager.NotOrganizer.selector); + tm.setFolders(bytes32(0), ROOT_A); + } + + /*───────── CAS guard ─────────*/ + + function test_RevertWhen_SetFolders_StaleExpectedRoot() public { + vm.prank(organizer); + tm.setFolders(bytes32(0), ROOT_A); + + // Caller still thinks current is bytes32(0) — must revert with FoldersRootStale. + vm.prank(organizer); + vm.expectRevert(abi.encodeWithSelector(TaskManager.FoldersRootStale.selector, bytes32(0), ROOT_A)); + tm.setFolders(bytes32(0), ROOT_B); + + // Storage unchanged. + assertEq(_foldersRoot(), ROOT_A, "state must be unchanged after stale revert"); + } + + /*───────── organizer hat config plumbing ─────────*/ + + function test_RevertWhen_SetConfig_OrganizerHatAllowed_NonExecutor() public { + vm.prank(organizer); + vm.expectRevert(TaskManager.NotExecutor.selector); + tm.setConfig(TaskManager.ConfigKey.ORGANIZER_HAT_ALLOWED, abi.encode(uint256(42), true)); + } + + function test_SetConfig_OrganizerHatAllowed_AddRemove_RoundTrip() public { + // Setup added ORGANIZER_HAT already. Add a second hat. + uint256 secondHat = 42; + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.ORGANIZER_HAT_ALLOWED, abi.encode(secondHat, true)); + + uint256[] memory after_ = _organizerHats(); + assertEq(after_.length, 2, "should have two organizer hats"); + + // Remove the original. + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.ORGANIZER_HAT_ALLOWED, abi.encode(ORGANIZER_HAT, false)); + + uint256[] memory after2 = _organizerHats(); + assertEq(after2.length, 1, "should have one organizer hat left"); + assertEq(after2[0], secondHat, "remaining hat should be the second one"); + + // Original wearer can no longer reorganize. + vm.prank(organizer); + vm.expectRevert(TaskManager.NotOrganizer.selector); + tm.setFolders(bytes32(0), ROOT_A); + } + + function test_SetConfig_OrganizerHatAllowed_EmitsEvent() public { + uint256 newHat = 77; + vm.expectEmit(true, false, false, true, address(tm)); + emit TaskManager.OrganizerHatAllowed(newHat, true); + vm.prank(executor); + tm.setConfig(TaskManager.ConfigKey.ORGANIZER_HAT_ALLOWED, abi.encode(newHat, true)); + } + + /*───────── lens ─────────*/ + + function test_GetLensData_FoldersRoot_DefaultsToZero() public view { + assertEq(_foldersRoot(), bytes32(0), "fresh deploy should have zero root"); + } + + function test_GetLensData_OrganizerHats_ReturnsArray() public view { + uint256[] memory hats_ = _organizerHats(); + assertEq(hats_.length, 1, "should have one organizer hat from setUp"); + assertEq(hats_[0], ORGANIZER_HAT, "should be ORGANIZER_HAT"); + } + } diff --git a/test/UpgradeSafety.t.sol b/test/UpgradeSafety.t.sol index 119d254..ef2dac0 100644 --- a/test/UpgradeSafety.t.sol +++ b/test/UpgradeSafety.t.sol @@ -26,6 +26,7 @@ import {PaymasterHub} from "../src/PaymasterHub.sol"; import {PoaManager} from "../src/PoaManager.sol"; import {SwitchableBeacon} from "../src/SwitchableBeacon.sol"; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; +import {MockHats} from "./mocks/MockHats.sol"; /// @title UpgradeSafetyTest /// @notice Comprehensive tests verifying upgrade safety invariants for all upgradeable contracts @@ -229,6 +230,86 @@ contract UpgradeSafetyTest is Test { assertEq(registry.getUsername(address(this)), "alice"); } + /// @notice Proves that appending the folders fields (foldersRoot, organizerHatIds) + /// to TaskManager.Layout does not disturb pre-existing project state, and + /// that the new `setFolders` function is callable through a proxy upgraded + /// from a previous impl. Two impls of the same struct simulate the + /// upgrade — append-only storage means the new impl reads old state + /// identically. + function testTaskManagerStoragePreservedAfterFoldersUpgrade() public { + MockHats hats = new MockHats(); + uint256 creatorHat = 1; + uint256 organizerHat = 99; + address creator = makeAddr("creator"); + address exec = makeAddr("executor"); + address organizer = makeAddr("organizer"); + hats.mintHat(creatorHat, creator); + hats.mintHat(organizerHat, organizer); + + // Token doesn't need to be real for these reads — any non-zero contract works. + address token = address(new DummyImplV1()); + + // Deploy "v1" impl + proxy and write state. + TaskManager implV1 = new TaskManager(); + UpgradeableBeacon beacon = new UpgradeableBeacon(address(implV1), address(this)); + BeaconProxy proxy = new BeaconProxy(address(beacon), ""); + TaskManager tm = TaskManager(address(proxy)); + + uint256[] memory creators = new uint256[](1); + creators[0] = creatorHat; + vm.prank(creator); + tm.initialize(token, address(hats), creators, exec, address(0)); + + // Pre-upgrade state: an organizer hat allowance + a project + a folders root. + vm.prank(exec); + tm.setConfig(TaskManager.ConfigKey.ORGANIZER_HAT_ALLOWED, abi.encode(organizerHat, true)); + + vm.prank(creator); + bytes32 pid = tm.createProject( + TaskManager.BootstrapProjectConfig({ + title: bytes("PRE_UPGRADE"), + metadataHash: bytes32(0), + cap: 5 ether, + managers: new address[](0), + createHats: creators, + claimHats: new uint256[](0), + reviewHats: new uint256[](0), + assignHats: new uint256[](0), + bountyTokens: new address[](0), + bountyCaps: new uint256[](0) + }) + ); + + bytes32 rootBefore = keccak256("root-before-upgrade"); + vm.prank(organizer); + tm.setFolders(bytes32(0), rootBefore); + + // Read state via lens for comparison post-upgrade. + bytes memory execLensBefore = tm.getLensData(4, ""); + bytes memory creatorHatsBefore = tm.getLensData(5, ""); + bytes memory projectLensBefore = tm.getLensData(2, abi.encode(pid)); + bytes memory foldersLensBefore = tm.getLensData(10, ""); + bytes memory organizersLensBefore = tm.getLensData(11, ""); + + // "Upgrade" — same impl class, new instance. + TaskManager implV2 = new TaskManager(); + beacon.upgradeTo(address(implV2)); + + // Storage must be byte-identical for every lens variant. + assertEq(keccak256(tm.getLensData(4, "")), keccak256(execLensBefore), "executor drifted"); + assertEq(keccak256(tm.getLensData(5, "")), keccak256(creatorHatsBefore), "creator hats drifted"); + assertEq(keccak256(tm.getLensData(2, abi.encode(pid))), keccak256(projectLensBefore), "project drifted"); + assertEq(keccak256(tm.getLensData(10, "")), keccak256(foldersLensBefore), "folders root drifted"); + assertEq(keccak256(tm.getLensData(11, "")), keccak256(organizersLensBefore), "organizer hats drifted"); + + // New folder writes must still go through CAS guard, and organizer hat plumbing + // must survive the upgrade so organizer can still reorg. + bytes32 rootAfter = keccak256("root-after-upgrade"); + vm.prank(organizer); + tm.setFolders(rootBefore, rootAfter); + assertEq(abi.decode(tm.getLensData(10, ""), (bytes32)), rootAfter, "new folder write must land"); + } + // ══════════════════════════════════════════════════════════════════════ // SECTION 4: SwitchableBeacon mode-switch safety // ══════════════════════════════════════════════════════════════════════