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

ERC777: _burn should be internal #1979

Closed
MicahZoltu opened this issue Oct 31, 2019 · 2 comments
Closed

ERC777: _burn should be internal #1979

MicahZoltu opened this issue Oct 31, 2019 · 2 comments

Comments

@MicahZoltu
Copy link
Contributor

function _burn(
address operator,
address from,
uint256 amount,
bytes memory data,
bytes memory operatorData
)
private

Similar to _mint, the _burn method may need to be called by a particular token's implementation when various events occur. One could work around this by making the token be its own operator, but that just seems silly and is a waste of gas.

@frangio
Copy link
Contributor

frangio commented Oct 31, 2019

Hi @MicahZoltu! Thanks for reporting this issue.

We have an open PR (#1908) tackling this that has become stuck but I would like to move it forward.

Can you let me know your thoughts on the concern I expressed there before we implement this?

My only concern with this PR is that it allows passing an arbitrary operator. This would be consistent with the current _mint function, but I feel that may have been an oversight. In my mind it would make much more sense to automatically set either this (or msg.sender?) as the operator in the internal functions. Otherwise the events emitted could be "lying". I realize this may be too much of a detail but I want us to think about it a bit before merging.

The concern is that the contract could be observed through the events as having any arbitrary operator that hasn't been registered before as a default or account-specific operator. I'm not sure how this could affect users of the interface.

@frangio
Copy link
Contributor

frangio commented Dec 16, 2019

Was fixed in #1908.

@frangio frangio closed this as completed Dec 16, 2019
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

No branches or pull requests

2 participants