-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TERA-168] Add burner role for burn functions #24
Conversation
Add burner role for burn functions
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
3cfe305
to
109744d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh i hadn't realized that the original burn
and mint
didn't have virtual
and so cannot be overriden. maybe worth thinking about whether we want to go forward with this or not. if we do, maybe what we want to do is to "deprecate" the original mint/burn by removing the MINTER_ROLE
altogether and set new ones. So technically no account has permissions to call those functions
Changes to gas cost
🧾 Summary (20% most significant diffs)
Full diff report 👇
|
Coverage after merging add-burner-role into main will be
Coverage Report for Changed Files
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add BURNER_ROLE and burnByBurnerOnly function to FiatToken
this is to allow a contract to have BURNER_ROLE who only can perform token burn