Skip to content
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

Create BurnerRole.sol #2049

Closed
wants to merge 1 commit into from
Closed

Create BurnerRole.sol #2049

wants to merge 1 commit into from

Conversation

Ro5s
Copy link
Contributor

@Ro5s Ro5s commented Jan 10, 2020

Creates role for operator burns of tokens. For example, in the case of NFTs, tokens might represent certain certifications or other accreditation. There may be cases where a private key is lost and a central operator, such as a DAO, might then act to burn the balance at the lost address, or otherwise act to enforce rules associated with token registry.

@nventuro
Copy link
Contributor

This is one of the reasons why I want to implement #1772.

We currently need a lot of boilerplate (the 40 lines of this PR) to define a new Role, and doing so is very error-prone. Since it doesn't make sense for us to host a bunch of roles which are essentially the same (except the names), we're left in a weird position where the roles we need are defined, but rolling your own is cumbersome.

Given this, I'd rather work on #1772 than keep adding roles like this one. @Ro5s would it be an issue for you to have your own BurnerRole in your project?

@Ro5s
Copy link
Contributor Author

Ro5s commented Jan 14, 2020

This makes sense to me! I realize after making the PR that I was basically just proposing a copy-paste of other roles with a slight adaption ERC20Burnable down flow to include operatorBurn function...... this can be seen in the following implementation of an NFT meant to certify Accred Investors.... making this all more modular is priority

@nventuro
Copy link
Contributor

Alright then, closing in favor of #1772.

@nventuro nventuro closed this Jan 14, 2020
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.

2 participants