Next v5.0 erc721 update hook

[RenanSouza2]

This is proposal to remove before and after hooks from ERC721 and replace them with an update hook

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: f35a8a01799a1252a6c711161bad0efdde1fd44b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[frangio]

Thank you @RenanSouza2, we will review this shortly.

[Amxx]

FYI, we already have a branch (that has been sitting for the last 3 months) with the _update refactor for ERC721.

here is what I found (https://github.com/Amxx/openzeppelin-contracts/blob/refactor/erc721-mint-burn-transfer/contracts/token/ERC721/ERC721.sol) ... there was something on julissa's fork that I pushed to mine.

AFAIK, we were worried about the duplicate sload when doing a transferFrom:

• in approvedOrOwner we need to get the current owner to check authorisation
• in _update we need to get the the current owner to check that from is correct. I proposed a solution that uses fnPointers to put the "constraint" check inside _update.

It was supposed to get a review ... but I guess we forgot about it ?

[Amxx]

Note: I rebased the above code onto next-v5 to have a clean diff.

There is an issue with this design. the "simple" hooks can be replace by an override to _update ... however the ERC721 hooks currently include a batch size for dealing with Consecutive+Votes contracts. If we remove the hooks, we lose our ability to hook into the batch mints. This breaks Consecutive+Votes. It also breaks the security we have for Consecutive+Enumerable.

So we need to find a new design for Consecutive ...

[RenanSouza2]

I've expanded the code in the PR in this other branch https://github.com/RenanSouza2/openzeppelin-contracts/blob/update-ERC721Votes/contracts/token/ERC721/extensions/ERC721Votes.sol

There is a function _increaseBalance that is also used as a hook. This function is called in the _mintBatch function in Consecutive and is overriden in Enumerable to avoit batch sizes bigger than one.

In this branch specifically it is used to in the Votes to distribute voting power