-
Notifications
You must be signed in to change notification settings - Fork 12.4k
Improved Roles API. #1280
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
Improved Roles API. #1280
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,29 +8,25 @@ contract MinterRole { | |
|
|
||
| Roles.Role private minters; | ||
|
|
||
| constructor(address[] _minters) public { | ||
| minters.addMany(_minters); | ||
| constructor() public { | ||
| minters.add(msg.sender); | ||
| } | ||
|
|
||
| function transferMinter(address _account) public { | ||
| minters.transfer(_account); | ||
| } | ||
|
|
||
| function renounceMinter() public { | ||
| minters.renounce(); | ||
| modifier onlyMinter() { | ||
| require(isMinter(msg.sender)); | ||
| _; | ||
| } | ||
|
|
||
| function isMinter(address _account) public view returns (bool) { | ||
| return minters.has(_account); | ||
| } | ||
|
|
||
| modifier onlyMinter() { | ||
| require(isMinter(msg.sender)); | ||
| _; | ||
| function addMinter(address _account) onlyMinter public { | ||
| minters.add(_account); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I forgot if we discussed this, but shouldn't there be events emitted?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We did discuss it, we didn't include them because there's no way to reference to role itself, all we could do is |
||
| } | ||
|
|
||
| function _addMinter(address _account) internal { | ||
| minters.add(_account); | ||
| function renounceMinter() public { | ||
| minters.remove(msg.sender); | ||
| } | ||
|
|
||
| function _removeMinter(address _account) internal { | ||
|
|
||
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.
I'm a bit reluctant to remove
transferbecause it means in order to transfer now one has to do two transactions. On the other hand, this may be the 2-step transfer we had been wanting. (!!) 🙂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.
We can always add it later 🎉