Submitted by hyh
If Alice the NFT owner first delegates her votes to herself, second delegates to anyone else with delegate() or delegateBySig() then all her NFT ids will become stuck: their transfers and burning will be disabled.
The issue is _afterTokenTransfer() callback running the _moveDelegateVotes() with an owner instead of her delegate. As Alices votes in the checkpoint is zero after she delegated them, the subtraction _moveDelegateVotes() tries to perform during the move of the votes will be reverted.
As ERC721Votes is parent to Token and delegate is a kind of common and frequent operation, the impact is governance token moves being frozen in a variety of use cases, which interferes with governance voting process and can be critical for the project.
Suppose Alice delegated all her votes to herself and then decided to delegate them to someone else with either delegate() or delegateBySig() calling _delegate():
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/token/ERC721Votes.sol#L179-L190
_moveDelegateVotes() will set her votes to 0 as _from == Alice and prevTotalVotes = _amount = balanceOf(Alice) (as _afterTokenTransfer() incremented Alices vote balance on each mint to her):
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/token/ERC721Votes.sol#L196-L217
After that her votes in the checkpoint become zero. She will not be able to transfer the NFT as _afterTokenTransfer will revert on _moveDelegateVotess attempt to move 1 vote from Alice to _to, while checkpoints[Alice][nCheckpoints - 1].votes is 0:
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/token/ERC721Votes.sol#L262-L268
The root issue is _afterTokenTransfer() dealing with Alice instead of Alices delegate.
Consider including delegates() call as a fix:
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/token/ERC721Votes.sol#L262-L268
As delegates(address(0)) == address(0) the burning/minting flow will persist:
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/token/ERC721Votes.sol#L124-L129
tbtstl (Nouns Builder) confirmed
Alex the Entreprenerd (judge) commented:
