0

In https://github.com/OpenZeppelin/openzeppelin-contracts/blob/783ac759a902a7b4a218c2d026a77e6a26b6c42d/contracts/token/ERC721/extensions/ERC721Royalty.sol#L34 we have

function _burn(uint256 tokenId) internal virtual override {
    super._burn(tokenId);
    _resetTokenRoyalty(tokenId);
}

Why resetting royalty info before burning as once burnt there's no way to use or transfer this token?

I can't find a good reason and it doesn't seem to be required in the EIP https://eips.ethereum.org/EIPS/eip-2981

2 Answers 2

2
+50

The term reset can be misleading, as you can see in ERC2981.sol that is a sclear operation (or delete in Solidity):

/**
 * @dev Resets royalty information for the token id back to the global default.
 */
function _resetTokenRoyalty(uint256 tokenId) internal virtual {
    delete _tokenRoyaltyInfo[tokenId];
}

That action has two main effects: freeing data from the chain and avoiding future inconsistencies.

Freeing data

Deleting data from the global storage is a best practice to keep the global state of the chain as clean as possible. To incentivize developers to apply this practice, the sender is refunded 15.000 gas from the overall transaction's cost.

selfdestruct and sclear are currently the only two EVM operations that refund gas instead of consuming it.

From the yellow paper, page 27:

sclear: 15000 Refund given (added into refund counter) when the storage value is set to zero from non-zero.

selfdestruct: 24000 Refund given (added into refund counter) for self-destructing an account.

Avoiding inconsistencies

As Julissa DC already noted in another answer, the current OpenZeppelin's implementation of EIP721 allows the minting of burned tokens. Minting and burning are not part of the standard, so this can be done without breaking shared rules. If they want otherwise, it's up to each developer to override that behavior in their smart contract.

About ERC721Royalty.sol, it is an optional extension to ERC721.sol, so deleting royalties information regarding a token also ensures that, if a burned token is minted again, it will not have royalty information already in place, causing potential leaks of funds to previous receiver account.

2
  • I read on other answer that it prevents issues in case of reminting. Could you tell me more about this part? Jan 27, 2022 at 13:02
  • 1
    I added more info about the reminting behavior (it's intended, I can't agree with that choice, but it's not a bug). Jan 27, 2022 at 18:51
1

As a security measure and to free space, more on the later here. Now back to the security reasoning, as you can see nothing prevents you from re-minting a tokenId after it was burned, and someone could mint a burned tokenId that had royalty information in place, causing some fund leaks to the previous receiver account.

Disclaimer: I'm the one who made that commit.

7
  • 1
    Interesting topic, and thanks for your work :) A question: minting and burning are not part of the EIP721, shouldn't the most intuitive and safest implementation avoid allowing minting again a burned token? There are specific reasons to permit that instead? Jan 27, 2022 at 0:26
  • 1
    Thanks for the clarifications. My worries arise from the expectations that a burn method sets in the user's mind more than from code. Up now, talking about burning, I always meant that the burned could not resurrect later in any case. I used burn operations to generate proofs-of-burn, obtaining something in return for that sacrifice, off-chain or on-chain. The current implementation seems a bit counterintuitive: I can really burn a token sending a simple transfer to address 0x0, but if I use the dedicated burn I didn't really finish the job and I need to implement it by myself. Jan 27, 2022 at 1:16
  • 1
    Plus this bounty is looking for reputable sources. I know you wrote the code, but that's not what we call a reputable source. It requires an article, reference to a paper (whatever the colour) or a security audit, something not only based on one's opinion. Thanks for your help! Jan 27, 2022 at 8:18
  • 1
    Dont worry @NicolasMasart I just answer to let you know, not to win the bounty, I even upvoted his answer because it was more comprehensive to read.
    – Julissa DC
    Jan 27, 2022 at 11:13
  • 1
    I added information provided by Julissa to the response. About sources, I cannot think of a more reputable source than the person that wrote the code ;) About the bounty, it's not a problem for me either, the real goal is to provide a satisfying answer to the question. Jan 27, 2022 at 18:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.