Skip to content

Add AccessControlDefaultAdminRulesComponent#1432

Merged
ericnordelo merged 26 commits intoOpenZeppelin:mainfrom
ericnordelo:feat/add-default-admin-rules-ext-#1164
Jul 2, 2025
Merged

Add AccessControlDefaultAdminRulesComponent#1432
ericnordelo merged 26 commits intoOpenZeppelin:mainfrom
ericnordelo:feat/add-default-admin-rules-ext-#1164

Conversation

@ericnordelo
Copy link
Copy Markdown
Member

@ericnordelo ericnordelo commented May 13, 2025

Fixes #1164

PR Checklist

  • Tests
  • Documentation
  • Added entry to CHANGELOG.md
  • Tried the feature on a public network

@ericnordelo ericnordelo requested a review from immrsd May 13, 2025 11:37
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2025

Codecov Report

Attention: Patch coverage is 93.24324% with 10 lines in your changes missing coverage. Please review.

Project coverage is 93.00%. Comparing base (46bf4a1) to head (48b8c21).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...s/src/accesscontrol/extensions/pending_delay.cairo 50.00% 9 Missing ⚠️
...extensions/accesscontrol_default_admin_rules.cairo 99.22% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1432      +/-   ##
==========================================
+ Coverage   92.99%   93.00%   +0.01%     
==========================================
  Files          78       80       +2     
  Lines        2083     2230     +147     
==========================================
+ Hits         1937     2074     +137     
- Misses        146      156      +10     
Files with missing lines Coverage Δ
...kages/access/src/accesscontrol/accesscontrol.cairo 100.00% <100.00%> (ø)
...extensions/accesscontrol_default_admin_rules.cairo 99.22% <99.22%> (ø)
...s/src/accesscontrol/extensions/pending_delay.cairo 50.00% <50.00%> (ø)

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 46bf4a1...48b8c21. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@immrsd immrsd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a look only at the interface so far

Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
Copy link
Copy Markdown
Collaborator

@immrsd immrsd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job, Eric! The implementation looks good, I haven't noticed any inconsistencies with our AccessControl or Solidity AccessControlDefaultAdminRules implementations. I left a couple of comments and suggestions, but overall it's for sure on the right track

Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
@ericnordelo
Copy link
Copy Markdown
Member Author

Perfect, I will address the comments and finish the PR then with this idea.

@ericnordelo ericnordelo marked this pull request as ready for review June 16, 2025 12:40
@ericnordelo ericnordelo requested a review from immrsd June 24, 2025 10:53
Copy link
Copy Markdown
Collaborator

@immrsd immrsd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Left some comments, mostly minor ones

Comment thread packages/access/src/accesscontrol/extensions/interface.cairo Outdated
Comment thread packages/access/src/accesscontrol/extensions/pending_delay.cairo
Comment thread docs/modules/ROOT/pages/api/access.adoc Outdated
Comment thread docs/modules/ROOT/pages/api/access.adoc Outdated
Comment thread docs/modules/ROOT/pages/api/access.adoc Outdated
@ericnordelo ericnordelo requested a review from immrsd July 1, 2025 09:58
Copy link
Copy Markdown
Collaborator

@immrsd immrsd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, LGTM!

@ericnordelo ericnordelo merged commit cbfda3c into OpenZeppelin:main Jul 2, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add AccessControlDefaultAdminRules extension.

2 participants