ERC721Upgradable v5.0 breaking changes for _beforeTokenTransfer migrating to _update

Smart Contracts
1

After reading the docs I see that you now need to override the _update function, but I have a question about efficiency and if I'm going about this the wrong way.

When I did the override, vsc (w/ hardhat solidity plugin) is erroring if I don't return (address). I see that super._update returns the from address, but here's the question:

In order to do a complex operation before the transfer, you need to know the current owner -> ownerOf(tokenId), but this is the same exact thing the _update function returns.

So my code ended up looking like this in order to avoid errors in the IDE:

function _update(...) internal virtual override returns (address) {
address prevOwner = ownerOf(tokenId);
ownedByOwner[prevOwner] -= 1;
super._update(...);
return prevOwner;
}

Is there a way to avoid this double declaration while still ensuring it happens before the update, or is this type of action something I should do after the update using the returned address? My reasoning for having it before is to ensure no replay attack nonsense can happen while the transfer tx is executing. Maybe my understanding of the changes is wrong and it's safe to move this operation to after super._update(...)?

Or could this be the VSC hardhat plugin just isn't updated for 5.0? I feel like it is really inefficient to double declare the prevOwner unless I shouldn't be doing storage operations before the update in general.

Thanks!

2

What's wrong with:

address prevOwner = super._update(...);
ownedByOwner[prevOwner] -= 1;
return prevOwner;

?

3

Doing it that way updates the storage after the transfer, essentially making it _afterTokenTransfer.

I'm not sure if anything is wrong with it though, which was part of the question.

I'm just always cautious about storage updates happening after transfers because of possible replays (aka the person transferring and frontrunning their own tx).

Maybe my concern is more applicable to ERC20 (where adjustments to balances happen that need modification before the transfer in the case of taxes etc, and ERC20 doesn't return anything so it wouldn't duplicate) than this case.

4

In the aspect of storage-update during transfers, there is no difference between v4 and v5.

In other words, replacing _beforeTokenTransfer and _afterTokenTransfer with _update does not impact storage-update during transfers.

So your question, at least to me, is not clear.

5

It definitely changes the dynamics. I'm talking about storage updates not related to the _owners or _balances that are handled in _update.

For instance if you have an auto-stake mechanism in place that is triggered on transfer, it needs to be run before the token is actually transferred (_update'ed) to account for positions, as the staking contract call uses the from and to.

I think I'm also grossly overthinking how much a simple address declaration costs gas wise.

But as I said in the last post, I don't think this effects 721 in the way I originally thought. It could effect 1155 if you wanted to tax some NFTs before the transfer, perhaps, but that's a totally different topic.