Skip to content

refactor: split TaskManager into shared-storage libraries (mirror HybridVoting pattern) #158

@hudsonhrh

Description

@hudsonhrh

Motivation

src/TaskManager.sol is currently ~1100 lines and covers projects, tasks, applications, permissions, budgets, folders, and the lens dispatcher all in one contract. After the folders + NatSpec PR (#TBD), the file is well-documented but feels overloaded. The codebase already has a precedent for splitting a large upgradeable contract into multiple library files that share a single ERC-7201 storage slot — see HybridVoting and its three siblings (HybridVotingCore, HybridVotingConfig, HybridVotingProposals), called out explicitly in CLAUDE.md.

Proposed split

One thin contract + N libraries, all sharing keccak256("poa.taskmanager.storage"):

  • TaskManager.sol (entry point) — initialize, Layout struct, _layout(), errors, events, ERC-7201 slot constant, top-level external dispatchers that call into the libraries.
  • libs/TaskManagerProjects.solcreateProject, _createProjectCore, deleteProject, bootstrapProjectsAndTasks, _initBountyBudgets, setProjectRolePerm, _setBatchHatPerm.
  • libs/TaskManagerTasks.solcreateTask, createTasksBatch, _createTask, updateTask, claimTask, assignTask, submitTask, completeTask, rejectTask, cancelTask, createAndAssignTask.
  • libs/TaskManagerApplications.solapplyForTask, approveApplication.
  • libs/TaskManagerFolders.solsetFolders, organizer-hat plumbing, _requireOrganizer. Clean home for the v3 feature.
  • libs/TaskManagerPerm.sol (or extend existing TaskPerm) — _permMask, _isPM, _checkPerm, _requireCreator, _requireOrganizer, _syncPermissionHat, _hasAnyProjectPermissionLegacy, _rebuildProjectPermRefCount, _updateProjectPermRefCount, _hasCreatorHat.
  • getLensData stays on the entry contract (it's the public read surface) but its body delegates to per-variant view functions on the libraries.

NatSpec moves with the code — each library carries the docs for the functions it owns, so the entry contract reads as a clean table of contents rather than 1100 lines of mixed implementation.

Hard constraints

  • Storage layout must stay byte-identical. The shared slot is keccak256(\"poa.taskmanager.storage\") and the Layout struct's field order/types cannot change. Append-only rule still applies; this refactor must touch zero storage fields.
  • upgrades/baseline/TaskManager.sol must not be edited. CI compares current vs. baseline; the refactored entry contract must compile to a Layout that the baseline checker considers compatible.
  • External ABI must stay byte-identical. All function selectors, parameter orders, return types, event signatures, and error selectors are part of the public surface and cannot drift. Existing callers (subgraph, frontend, lens, factories) keep working without changes.
  • Mechanical, function-by-function commits. Each commit moves one function (or a tightly-related cluster) from the entry contract to its library, runs forge build + forge test --match-path 'test/TaskManager*.sol' + the UpgradeSafety suite, and lands green. Easier to review and to bisect if something breaks.
  • No behavior changes. Any tempting cleanups (TaskPerm.SELF_REVIEW handling, the legacy ref-count fallback, the >= ConfigKey.BOUNTY_CAP switch that PR #TBD already tightened to == … || == …) go in their own follow-up PRs.

Risks

  • Library functions reading shared Layout storage l via _layout() must be internal (so they inline and share calldata) — going external would silently change msg.sender semantics through delegatecall. All current externals stay on the entry contract; libraries hold the internal helpers and the per-variant business logic.
  • The lens variant numbering (1–11 today) is part of the off-chain ABI used by TaskManagerLens; any reshuffling will break the subgraph. Keep variants numbered as-is.
  • Production profile (FOUNDRY_PROFILE=production) has a pre-existing Stack too deep issue — verify each commit compiles under the default profile.
  • Cross-chain: this is a pure code-organization change with no on-chain behavior change, so no Upgrade*.s.sol is needed. Don't bump a VERSION for this refactor (no impl deploy required).

Acceptance criteria

  • src/TaskManager.sol shrinks to ~250 lines (Layout + errors + events + initializer + external dispatchers + getLensData).
  • Each new library file carries the NatSpec for its functions.
  • forge test is green for all TaskManager suites (currently 199 tests) and UpgradeSafetyTest (currently 25 tests, including the folders storage-preservation test).
  • forge fmt --check clean.
  • CI run-upgrade-safety check passes against the unchanged upgrades/baseline/TaskManager.sol.
  • PR description includes a side-by-side selector list (before vs. after) proving ABI is byte-identical.

Out of scope

  • Behavior changes, even "obviously correct" ones.
  • Touching EducationHub, HybridVoting, PaymasterHub, etc. — those are separate refactor candidates if the pattern proves out here.
  • Subgraph changes (none needed since ABI is preserved).

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions