Allow any address holding the REVOKER_ROLE to revoke tokens #37#46
Allow any address holding the REVOKER_ROLE to revoke tokens #37#46shrilakshmikakati wants to merge 1 commit intoStabilityNexus:mainfrom
Conversation
WalkthroughModified the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/src/TNT.sol (1)
78-88:⚠️ Potential issue | 🟠 MajorAdd regression tests for the widened revoke authority.
This changes a security-sensitive authorization path, but there are no companion tests here proving that a non-issuer with
REVOKER_ROLEcan now revoke while callers without the role and non-revokable contracts still revert. Please add those cases before merge.As per coding guidelines, "Verify that any modification to contract logic includes corresponding updates to automated tests." and "Ensure security-sensitive logic changes are not introduced without adequate test coverage."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/src/TNT.sol` around lines 78 - 88, Add regression tests covering the widened revoke authority in revokeToken: create scenarios where (1) an account granted REVOKER_ROLE (but not the issuer) successfully calls revokeToken and emits TokenRevoked, (2) an account without REVOKER_ROLE is rejected (expect revert), and (3) when revokable is false revokeToken reverts with NotRevokable; also include a test that when the revoked owner's token list becomes empty the IFactory.unregisterToken(address) path is invoked. Use the revokeToken function name, REVOKER_ROLE, revokable/NotRevokable check, and verify IFactory.unregisterToken is called as part of the flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@contracts/src/TNT.sol`:
- Around line 78-88: Add regression tests covering the widened revoke authority
in revokeToken: create scenarios where (1) an account granted REVOKER_ROLE (but
not the issuer) successfully calls revokeToken and emits TokenRevoked, (2) an
account without REVOKER_ROLE is rejected (expect revert), and (3) when revokable
is false revokeToken reverts with NotRevokable; also include a test that when
the revoked owner's token list becomes empty the
IFactory.unregisterToken(address) path is invoked. Use the revokeToken function
name, REVOKER_ROLE, revokable/NotRevokable check, and verify
IFactory.unregisterToken is called as part of the flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64d1b58f-ae99-4390-8d85-44ae21484fce
📒 Files selected for processing (1)
contracts/src/TNT.sol
1 participant
Addressed Issues:
Fixes #(TODO:issue number)
Screenshots/Recordings:
TODO: If applicable, add screenshots or recordings that demonstrate the interface before and after the changes.
Additional Notes:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
Checklist
Images
Summary by CodeRabbit
Release Notes