Migrate `MembershipNFT` access control to OpenZeppelin `AccessControl`

[Lakes41]

Difficulty: Expert
Type: Security / Refactor (Solidity)

Background
contracts/SECURITY_REVIEW_MembershipNFT.md (finding #4) explicitly considered and deferred migrating from the hand-rolled owner + admins mapping to OpenZeppelin's AccessControl, citing the cost of adding openzeppelin-contracts as a new stateful dependency as out of scope for that review's "audit and fix" mandate. The review notes that openzeppelin-contracts (pinned v5.6.1) was later added as a lib/ dependency, but only for stateless libraries (MerkleProof.sol, BitMaps.sol) with "no inheritance, no constructors, no effect on storage layout" — explicitly distinguishing that addition from a full, stateful AccessControl migration, which remains unimplemented.

Problem
The current owner/admins model is a minimal, functional, but non-standard access-control implementation. It lacks role granularity (all admins can mint/renew/suspend — there's no way to give one address renew-only rights, for instance), lacks the tooling ecosystem (block explorers, wallets, and audit tools widely recognize AccessControl's standard role events and interfaces), and — as the review notes — any future migration must "keep onlyAdmin's revert string (NOT_ADMIN) stable or coordinate the change with any off-chain code that pattern-matches on it."

Expected outcome
MembershipNFT uses OpenZeppelin AccessControl (or AccessControlDefaultAdminRules for safer default-admin transfer semantics) with at least two distinct roles (e.g. DEFAULT_ADMIN_ROLE for governance-level operations like setMembershipMerkleRoot/role management, and a narrower MINTER_ROLE/OPERATOR_ROLE for day-to-day mint/renew/setSuspended calls), while preserving every existing external function signature, revert-string compatibility (or an explicitly documented, coordinated breaking change), and all invariants already covered by the existing test suite (MembershipNFT.t.sol, MembershipFuzzInvariant.t.sol).

Suggested implementation

1. Add openzeppelin-contracts's AccessControl (already vendored at v5.6.1 per the review) as an actual inherited base contract — this is the "meaningfully larger" addition the prior review explicitly scoped out; treat it as a first-class supply-chain change: pin the exact commit/version already vendored, and audit for any transitive dependency surface introduced beyond what's already present for MerkleProof/BitMaps.
2. Define MINTER_ROLE (mint/renew/setSuspended) and keep DEFAULT_ADMIN_ROLE for setAdmin-equivalent operations (granting/revoking MINTER_ROLE) and the existing two-step ownership-equivalent transfer, using AccessControlDefaultAdminRules to preserve (or improve on) the two-step-transfer safety already added in the prior review.
3. Decide and document explicitly: either preserve the onlyAdmin modifier's exact revert behavior/string by wrapping hasRole checks with a compatibility shim, or make a clean breaking change and update every off-chain caller/test that depends on the old revert string — per the review's own guidance, this must be a deliberate, coordinated decision, not an incidental side effect.
4. Migrate every existing test in MembershipNFT.t.sol and MembershipFuzzInvariant.t.sol to grant roles via AccessControl's API instead of the old setAdmin, updating assertions to check hasRole where they previously checked the admins mapping.
5. Add new tests specifically for the migration: role granularity (a MINTER_ROLE-only address cannot call admin-role-gated functions), role revocation, and the interaction between AccessControlDefaultAdminRules's transfer delay and the previously-added two-step ownership semantics (avoid having two competing "pending owner" concepts).
6. Update contracts/SECURITY_REVIEW_MembershipNFT.md with a new dated entry documenting this migration, explicitly closing out finding Add unit tests for the policy engine #4's deferred recommendation and recording the final decision on revert-string compatibility.
7. Run the existing GasBenchmark.t.sol to quantify and document the gas-cost change from switching to AccessControl, consistent with the review's existing practice of reporting gas impact for storage/interface changes.

Acceptance criteria

• MembershipNFT inherits AccessControl/AccessControlDefaultAdminRules with at least two distinct, correctly-scoped roles.
• All existing tests pass (migrated to the new role-granting API) with no loss of invariant coverage.
• New tests prove role granularity is enforced (least-privilege role cannot perform admin-only actions).
• The onlyAdmin revert-string compatibility decision is explicit, documented, and consistently applied.
• contracts/SECURITY_REVIEW_MembershipNFT.md is updated with a dated entry closing out finding Add unit tests for the policy engine #4.
• Gas-impact of the migration is measured and documented, consistent with existing reporting conventions in that file.

Likely affected files/directories
contracts/src/MembershipNFT.sol, contracts/test/MembershipNFT.t.sol, contracts/test/MembershipFuzzInvariant.t.sol, contracts/test/GasBenchmark.t.sol, contracts/SECURITY_REVIEW_MembershipNFT.md, foundry.lock/foundry.toml (dependency pinning)