docs(IAccessManager): document locked roles for label/admin/guardian/grantDelay

[Forostovec]

Align IAccessManager NatSpec with AccessManager implementation and tests by documenting locked-role constraints.
ADMIN_ROLE and PUBLIC_ROLE cannot be targeted by labelRole, setRoleAdmin, or setRoleGuardian, and PUBLIC_ROLE cannot be targeted by setGrantDelay.
Notes clarify that admin/guardian can be set to PUBLIC_ROLE, but the target roleId itself must not be a locked role.
This prevents integrator confusion and ensures interface documentation accurately reflects enforced behavior (see
AccessManager.sol and AccessManager.test.js).

[changeset-bot]

⚠️ No Changeset found

Latest commit: aa4ef7d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

Documentation constraints are added to several IAccessManager functions in contracts/access/manager/IAccessManager.sol. The constraints specify that labelRole, setRoleAdmin, and setRoleGuardian functions must not accept ADMIN_ROLE or PUBLIC_ROLE as arguments, while setGrantDelay must not accept PUBLIC_ROLE. Additional notes clarify that PUBLIC_ROLE can be set as an admin or guardian, but the target role itself must not be locked. No implementation, error handling, or control flow changes are made.

Suggested labels

ignore-changeset

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "docs(IAccessManager): document locked roles for label/admin/guardian/grantDelay" directly corresponds to the main change in the changeset: adding NatSpec documentation constraints to IAccessManager functions regarding locked-role restrictions. The title is specific and clearly identifies which functions are affected and what type of documentation is being added. It follows a clear, concise format without vague language or noise, making it easy for reviewers to understand the primary purpose of the changes at a glance.
Description Check	✅ Passed	The pull request description is directly related to the changeset and provides meaningful context about the documentation updates. It specifically explains which locked roles cannot be targeted by which functions, clarifies the distinction between setting an admin/guardian to PUBLIC_ROLE versus targeting a locked role as the function parameter, and provides rationale for the changes (aligning interface with implementation, preventing integrator confusion). The description is neither vague nor generic, offering concrete details about the nature and purpose of the NatSpec documentation being added.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

contracts/access/manager/IAccessManager.sol (2)

258-261: Consider simplifying the NOTE for better clarity.

The NOTE is somewhat verbose since line 258 already states that roleId must not be ADMIN_ROLE or PUBLIC_ROLE. Consider rewording the NOTE to focus solely on clarifying that the admin parameter can be set to PUBLIC_ROLE:

- * NOTE: Setting `admin` to the `PUBLIC_ROLE` is allowed, but the target `roleId` itself
- * must not be a locked role (`ADMIN_ROLE` or `PUBLIC_ROLE`).
+ * NOTE: The `admin` parameter may be set to `PUBLIC_ROLE`, but the `roleId` parameter
+ * is still subject to the locked-role restriction above.

This reduces redundancy while maintaining clarity about which parameter has which restriction.

---

273-276: Apply the same NOTE simplification as suggested for setRoleAdmin.

For consistency with setRoleAdmin, consider simplifying this NOTE to avoid redundancy:

- * NOTE: Setting `guardian` to the `PUBLIC_ROLE` is allowed, but the target `roleId` itself
- * must not be a locked role (`ADMIN_ROLE` or `PUBLIC_ROLE`).
+ * NOTE: The `guardian` parameter may be set to `PUBLIC_ROLE`, but the `roleId` parameter
+ * is still subject to the locked-role restriction above.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8631702 and aa4ef7d.

📒 Files selected for processing (1)

• contracts/access/manager/IAccessManager.sol (4 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)

• GitHub Check: Redirect rules - solidity-contracts
• GitHub Check: Header rules - solidity-contracts
• GitHub Check: Pages changed - solidity-contracts
• GitHub Check: slither
• GitHub Check: coverage
• GitHub Check: tests-foundry
• GitHub Check: tests-upgradeable
• GitHub Check: tests
• GitHub Check: halmos

🔇 Additional comments (2)

contracts/access/manager/IAccessManager.sol (2)

194-203: LGTM! Clear documentation of locked-role constraint.

The added requirement clearly documents that labelRole cannot target ADMIN_ROLE or PUBLIC_ROLE, aligning with the implementation constraints.

---

282-292: ****

The documentation is correct. The implementation of _setGrantDelay at line 363 deliberately checks only for PUBLIC_ROLE, not ADMIN_ROLE, allowing the grant delay of ADMIN_ROLE to be modified. This is intentional design—unlike setRoleAdmin and setRoleGuardian which exclude both roles, setGrantDelay has different validation logic that only excludes PUBLIC_ROLE. The documentation accurately reflects the implementation behavior.

Likely an incorrect or invalid review comment.

[ernestognw]

Thanks