Improve AccessControlDefaultAdminRules admin transfer delay change logic

[ericnordelo]

No description provided.

[Copilot]

Pull Request Overview

This PR adds a maximum transfer delay validation to the AccessControlDefaultAdminRules component to prevent accidentally setting an excessively high admin transfer delay that could lock the contract for an extended period.

Key changes:

• Introduces MAXIMUM_TRANSFER_DELAY constant (set to 30 days by default) in the ImmutableConfig trait
• Adds validation in both initializer and change_default_admin_delay functions to enforce the maximum delay
• Renames INVALID_DELAY error constant to INVALID_GRANT_ROLE_DELAY for clarity

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
packages/access/src/accesscontrol/extensions/accesscontrol_default_admin_rules.cairo	Implements the maximum transfer delay validation logic, adds the new MAXIMUM_TRANSFER_DELAY constant, updates documentation, and renames error constant
packages/access/src/tests/test_accesscontrol_default_admin_rules.cairo	Adds test cases to verify that delays exceeding the maximum are properly rejected in both initialization and delay change scenarios

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

🧪 Cairo Contract Size Benchmark Diff

BYTECODE SIZE (felts) (limit: 81,920 felts)

Contract	Old	New	Δ	Note
DualCaseAccessControlDefaultAdminRulesMock	8246	8303	🟢 +57
ERC2981AccessControlDefaultAdminRulesMock	6976	7006	🟢 +30

SIERRA CONTRACT CLASS SIZE (bytes) (limit: 4,089,446 bytes)

Contract	Old	New	Δ	Note
DualCaseAccessControlDefaultAdminRulesMock	169089	170171	🟢 +1082
ERC2981AccessControlDefaultAdminRulesMock	142584	143301	🟢 +717

This comment was generated automatically from benchmark diffs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.37%. Comparing base (ba2228e) to head (d713ceb).
⚠️ Report is 1 commits behind head on v3.0.0-rc.

Additional details and impacted files

@@              Coverage Diff              @@
##           v3.0.0-rc    #1567      +/-   ##
=============================================
- Coverage      92.38%   92.37%   -0.02%     
=============================================
  Files             83       85       +2     
  Lines           2272     2282      +10     
=============================================
+ Hits            2099     2108       +9     
- Misses           173      174       +1     

Files with missing lines	Coverage Δ
...extensions/accesscontrol_default_admin_rules.cairo	99.21% <100.00%> (+0.02%)	⬆️
...ckages/governance/src/governor/proposal_core.cairo	100.00% <ø> (ø)
...ckages/governance/src/multisig/storage_utils.cairo	92.85% <ø> (ø)
packages/token/src/erc20/erc20.cairo	98.03% <ø> (ø)

... and 3 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba2228e...d713ceb. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[immrsd]

Looking good, left a couple of minor comments

[immrsd]

Can we add a test case testing the successful recovery flow after setting a high delay value?

[ericnordelo]

There is no a specific flow for that, you can recover by decreasing the delay as you would do before, the new constraint is that you can't set a value over the configurable maximun, and as long as that value makes sense (as documented), you should be able to recover by decreasing.

I'm adding some tests to ensure the delay can't be set greater than this maximun.

[immrsd]

LGTM