-
Notifications
You must be signed in to change notification settings - Fork 158
curation: signal as an ERC20 token #252
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
Conversation
a74eb13
to
49ac90c
Compare
Rebased |
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.
looks good overall
_mint(_to, _amount); | ||
} | ||
|
||
function burn(uint256 amount) public onlyOwner { |
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.
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.
Good catch, will add the natspec.
_mint(_to, _amount); | ||
} | ||
|
||
function burn(uint256 amount) public onlyOwner { |
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.
also, does burn()
ever get used? It looks like just burnFrom()
is used. Not sure if we have a future use case for burn()
?
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.
Yes, I will remove the method. I initially was using ERC20Burnable and overriding the methods like burn and burnFrom to only be called by the Curation contract. Then I decided to just extend the plain ERC20 contract and implement the functions.
Lets merge 252, then 250, then 254, then 253, then 255. This order will work the best as far as i can tell |
Removed burn() and added many more comments to explain how this works on the natspec. |
looks good, you can merge |
Goal:
To have Curation Signal as a transferrable ERC20 token that can be used as a primitive to create more complex use cases on top.
Implements:
Remember: