Add ERC721Votes for NFT-based governance

contracts/governance/extensions/GovernorVotesERC721.sol

@@ -4,23 +4,26 @@
 pragma solidity ^0.8.0;
 
 import "../Governor.sol";
-import "../../token/ERC721/extensions/ERC721Votes.sol";
+import "../../token/ERC721/extensions/draft-ERC721Votes.sol";
 import "../../utils/math/Math.sol";
 
 /**
  * @dev Extension of {Governor} for voting weight extraction from an {ERC721Votes} token.
  *
- * _Available since v4.3._
+ * _Available since v4.5._
  */
 abstract contract GovernorVotesERC721 is Governor {
     ERC721Votes public immutable token;
 
+    /**
+     * @dev Need the ERC721Votes address to be initialized
+     */
     constructor(ERC721Votes tokenAddress) {
         token = tokenAddress;
     }
 
     /**
-     * Read the voting weight from the token's built in snapshot mechanism (see {IGovernor-getVotes}).
+     * @dev Read the voting weight from the token's built in snapshot mechanism (see {IGovernor-getVotes}).
      */
     function getVotes(address account, uint256 blockNumber) public view virtual override returns (uint256) {
         return token.getPastVotes(account, blockNumber);

contracts/mocks/ERC721VotesMock.sol

@@ -2,10 +2,10 @@
 
 pragma solidity ^0.8.0;
 
-import "../token/ERC721/extensions/ERC721Votes.sol";
+import "../token/ERC721/extensions/draft-ERC721Votes.sol";
 
 contract ERC721VotesMock is ERC721Votes {
-    constructor(string memory name, string memory symbol) ERC721(name, symbol) EIP712(name, "1") {}
+    constructor(string memory name, string memory symbol) ERC721Votes(name, symbol) {}
 
     function mint(address account, uint256 tokenId) public {
         _mint(account, tokenId);
@@ -19,7 +19,7 @@ contract ERC721VotesMock is ERC721Votes {
         return block.chainid;
     }
 
-    function _maxSupply() internal pure override returns(uint224){
-        return uint224(4);
+    function _maxSupply() internal pure override returns (uint224) {
+        return uint224(5);
     }
 }

contracts/token/ERC721/ERC721.sol

@@ -343,7 +343,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
 
         emit Transfer(from, to, tokenId);
 
-        _afterTokenTransfer(from, to, tokenId);
+        _afterTokenTransfer(from, to);
     }
 
     /**
@@ -423,24 +423,17 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
         address to,
         uint256 tokenId
     ) internal virtual {}
-     
-     /**
+
+    /**
      * @dev Hook that is called after any transfer of tokens. This includes
      * minting and burning.
      *
      * Calling conditions:
      *
-     * - when `from` and `to` are both non-zero, `tokenId` of ``from``'s tokens
-     * has been transferred to `to`.
-     * - when `from` is zero, `tokenId` tokens have been minted for `to`.
-     * - when `to` is zero, `tokenId` of ``from``'s tokens have been burned.
+     * - when `from` and `to` are both non-zero.
      * - `from` and `to` are never both zero.
      *
      * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
      */
-    function _afterTokenTransfer(
-        address from,
-        address to,
-        uint256 tokenId
-    ) internal virtual {}
+    function _afterTokenTransfer(address from, address to) internal virtual {}
 }

contracts/token/ERC721/README.adoc

@@ -15,6 +15,7 @@ Additionally there are multiple custom extensions, including:
 
 * designation of addresses that can pause token transfers for all users ({ERC721Pausable}).
 * destruction of own tokens ({ERC721Burnable}).
+* support for voting and vote delegation ({ERC721Votes})
 
 NOTE: This core set of contracts is designed to be unopinionated, allowing developers to access the internal functions in ERC721 (such as <<ERC721-_mint-address-uint256-,`_mint`>>) and expose them as external functions in the way they prefer. On the other hand, xref:ROOT:erc721.adoc#Presets[ERC721 Presets] (such as {ERC721PresetMinterPauserAutoId}) are designed using opinionated patterns to provide developers with ready to use, deployable contracts.
 

contracts/token/ERC721/extensions/ERC721Votes.sol → contracts/token/ERC721/extensions/draft-ERC721Votes.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-// OpenZeppelin Contracts v4.3.2 (token/ERC721/extensions/ERC721Votes.sol)
+// OpenZeppelin Contracts v4.3.2 (token/ERC721/extensions/draft-ERC721Votes.sol)
 
 pragma solidity ^0.8.0;
 
@@ -9,6 +9,7 @@ import "../../../utils/math/Math.sol";
 import "../../../utils/math/SafeCast.sol";
 import "../../../utils/cryptography/ECDSA.sol";
 import "../../../utils/cryptography/draft-EIP712.sol";
+
 /**
  * @dev Extension of ERC721 to support Compound-like voting and delegation. This version is more generic than Compound's,
  * and supports token supply up to 2^224^ - 1, while COMP is limited to 2^96^ - 1.
@@ -22,7 +23,7 @@ import "../../../utils/cryptography/draft-EIP712.sol";
  * Enabling self-delegation can easily be done by overriding the {delegates} function. Keep in mind however that this
  * will significantly increase the base gas cost of transfers.
  *
- * _Available since v4.2._
+ * _Available since v4.5._
  */
 abstract contract ERC721Votes is ERC721, EIP712 {
     using Counters for Counters.Counter;
@@ -44,7 +45,8 @@ abstract contract ERC721Votes is ERC721, EIP712 {
      * @dev Initializes the {EIP712} domain separator using the `name` parameter, and setting `version` to `"1"`.
      *
      * It's a good idea to use the same `name` that is defined as the ERC721 token name.
-    */
+     */
+    constructor(string memory name, string memory symbol) ERC721(name, symbol) EIP712(name, "1") {}
 
     /**
      * @dev Emitted when an account changes their delegate.
@@ -179,11 +181,11 @@ abstract contract ERC721Votes is ERC721, EIP712 {
      * @dev Snapshots the totalSupply after it has been increased.
      */
     function _mint(address account, uint256 tokenId) internal virtual override {
-        require(_totalSupply+1 <= _maxSupply(), "ERC721Votes: total supply risks overflowing votes");
-        
+        require(_totalSupply + 1 <= _maxSupply(), "ERC721Votes: total supply risks overflowing votes");
+
         super._mint(account, tokenId);
         _totalSupply += 1;
-        
+
         _writeCheckpoint(_totalSupplyCheckpoints, _add, 1);
     }
 
@@ -201,11 +203,9 @@ abstract contract ERC721Votes is ERC721, EIP712 {
      *
      * Emits a {DelegateVotesChanged} event.
      */
-    function _afterTokenTransfer(
-        address from,
-        address to,
-        uint256 tokenId
-    ) internal virtual override{
+    function _afterTokenTransfer(address from, address to) internal virtual override {
+        super._afterTokenTransfer(from, to);
+
         _moveVotingPower(delegates(from), delegates(to), 1);
     }
 
@@ -284,6 +284,11 @@ abstract contract ERC721Votes is ERC721, EIP712 {
         return _domainSeparatorV4();
     }
 
+    /**
+    * @dev Returns token voting power
+    */
+    function _getVotingPower(uint tokenId) internal virtual returns(uint256) {}
+
     function _add(uint256 a, uint256 b) private pure returns (uint256) {
         return a + b;
     }

docs/modules/ROOT/pages/erc721.adoc

@@ -1,6 +1,6 @@
 = ERC721
 
-We've discussed how you can make a _fungible_ token using xref:erc20.adoc[ERC20], but what if not all tokens are alike? This comes up in situations like *real estate*, *voting rights*, or *collectibles*, where some items are valued more than others, due to their usefulness, rarity, etc. ERC721 is a standard for representing ownership of xref:tokens.adoc#different-kinds-of-tokens[_non-fungible_ tokens], that is, where each token is unique.
+We've discussed how you can make a _fungible_ token using xref:erc20.adoc[ERC20], but what if not all tokens are alike? This comes up in situations like *real estate* or *collectibles*, where some items are valued more than others, due to their usefulness, rarity, etc. ERC721 is a standard for representing ownership of xref:tokens.adoc#different-kinds-of-tokens[_non-fungible_ tokens], that is, where each token is unique.
 
 ERC721 is a more complex standard than ERC20, with multiple optional extensions, and is split across a number of contracts. The OpenZeppelin Contracts provide flexibility regarding how these are combined, along with custom useful extensions. Check out the xref:api:token/ERC721.adoc[API Reference] to learn more about these.
 

docs/modules/ROOT/pages/governance.adoc

@@ -130,26 +130,25 @@ If your project requires The voting power of each account in our governance setu
 pragma solidity ^0.8.2;
 
 import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
-import "@openzeppelin/contracts/token/ERC721/extensions/ERC721Votes.sol";
+import "@openzeppelin/contracts/token/ERC721/extensions/draft-ERC721Votes.sol";
 
-contract MyToken is ERC721, EIP712, ERC721Votes {
-    constructor() ERC721("MyToken", "MTK") EIP712("MyToken", "1") {}
+contract MyToken is ERC721Votes {
+    constructor() ERC721Votes("MyToken", "MTK"){}
 
     // The functions below are overrides required by Solidity.
 
     function _afterTokenTransfer(
         address from,
-        address to,
-        uint256 tokenId
-    ) internal override(ERC721, ERC721Votes) {
-        super._afterTokenTransfer(from, to, tokenId);
+        address to
+    ) internal override(ERC721Votes) {
+        super._afterTokenTransfer(from, to);
     }
 
-    function _mint(address to, uint256 tokenId) internal override(ERC721, ERC721Votes) {
+    function _mint(address to, uint256 tokenId) internal override(ERC721Votes) {
         super._mint(to, tokenId);
     }
 
-    function _burn(uint256 tokenId) internal override(ERC721, ERC721Votes) {
+    function _burn(uint256 tokenId) internal override(ERC721Votes) {
         super._burn(tokenId);
     }
 }
@@ -159,15 +158,15 @@ NOTE: Voting power could be determined in different ways: multiple ERC20 tokens,
 
 === Governor
 
-Initially, we will build a Governor without a timelock. The core logic is given by the Governor contract, but we still need to choose: 1) how voting power is determined, 2) how many votes are needed for quorum, 3) what options people have when casting a vote and how those votes are counted, and 4)what type of token should be use to vote. Each of these aspects is customizable by writing your own module, or more easily choosing one from OpenZeppelin Contracts.
+Initially, we will build a Governor without a timelock. The core logic is given by the Governor contract, but we still need to choose: 1) how voting power is determined, 2) how many votes are needed for quorum, 3) what options people have when casting a vote and how those votes are counted, and 4) what type of token should be used to vote. Each of these aspects is customizable by writing your own module, or more easily choosing one from OpenZeppelin Contracts.
 
 For 1) we will use the GovernorVotes module, which hooks to an ERC20Votes instance to determine the voting power of an account based on the token balance they hold when a proposal becomes active. This module requires as a constructor parameter the address of the token.
 
 For 2) we will use GovernorVotesQuorumFraction which works together with ERC20Votes to define quorum as a percentage of the total supply at the block a proposal’s voting power is retrieved. This requires a constructor parameter to set the percentage. Most Governors nowadays use 4%, so we will initialize the module with parameter 4 (this indicates the percentage, resulting in 4%).
 
 For 3) we will use GovernorCountingSimple, a module that offers 3 options to voters: For, Against, and Abstain, and where only For and Abstain votes are counted towards quorum.
 
-For 4) we will use the GovernorVotesERC721 module, which hooks to an ERC721Votes instance to determine the voting power of an account based on the token balance they hold when a proposal becomes active. This module requires as a constructor parameter the address of the token.
+For 4) we will use the GovernorVotesERC721 module, which hooks to an ERC721Votes instance to determine the voting power of an account based on the NFTs they hold when a proposal becomes active. This module requires as a constructor parameter the address of the token.
 
 Besides these modules, Governor itself has some parameters we must set.
 

test/governance/GovernorWorkflow.behavior.js

@@ -31,9 +31,11 @@ function runGovernorWorkflow () {
       for (const voter of this.settings.voters) {
         if (voter.weight) {
           await this.token.transfer(voter.voter, voter.weight, { from: this.settings.tokenHolder });
-        } else if (voter.nftWeight) {
-          await this.token.transferFrom(this.settings.tokenHolder, voter.voter, voter.nftWeight,
-            { from: this.settings.tokenHolder });
+        } else if (voter.nfts) {
+          for (const nft of voter.nfts) {
+            await this.token.transferFrom(this.settings.tokenHolder, voter.voter, nft,
+              { from: this.settings.tokenHolder });
+          }
         }
       }
     }

test/governance/extensions/GovernorERC721.test.js

@@ -1,4 +1,5 @@
 const { expectEvent } = require('@openzeppelin/test-helpers');
+const { BN } = require('bn.js');
 const Enums = require('../../helpers/enums');
 
 const {
@@ -15,20 +16,34 @@ contract('GovernorERC721Mock', function (accounts) {
   const name = 'OZ-Governor';
   const tokenName = 'MockNFToken';
   const tokenSymbol = 'MTKN';
-  const initalTokenId = web3.utils.toWei('100');
+  const NFT0 = web3.utils.toWei('100');
   const NFT1 = web3.utils.toWei('10');
   const NFT2 = web3.utils.toWei('20');
   const NFT3 = web3.utils.toWei('30');
+  const NFT4 = web3.utils.toWei('40');
+
+  // Must be the same as in contract
+  const ProposalState = {
+    Pending: new BN('0'),
+    Active: new BN('1'),
+    Canceled: new BN('2'),
+    Defeated: new BN('3'),
+    Succeeded: new BN('4'),
+    Queued: new BN('5'),
+    Expired: new BN('6'),
+    Executed: new BN('7'),
+  };
 
   beforeEach(async function () {
     this.owner = owner;
     this.token = await Token.new(tokenName, tokenSymbol);
     this.mock = await Governor.new(name, this.token.address);
     this.receiver = await CallReceiver.new();
-    await this.token.mint(owner, initalTokenId);
+    await this.token.mint(owner, NFT0);
     await this.token.mint(owner, NFT1);
     await this.token.mint(owner, NFT2);
     await this.token.mint(owner, NFT3);
+    await this.token.mint(owner, NFT4);
 
     await this.token.delegate(voter1, { from: voter1 });
     await this.token.delegate(voter2, { from: voter2 });
@@ -55,10 +70,10 @@ contract('GovernorERC721Mock', function (accounts) {
         ],
         tokenHolder: owner,
         voters: [
-          { voter: voter1, nftWeight: initalTokenId, support: Enums.VoteType.For },
-          { voter: voter2, nftWeight: NFT1, support: Enums.VoteType.For },
-          { voter: voter3, nftWeight: NFT2, support: Enums.VoteType.Against },
-          { voter: voter4, nftWeight: NFT3, support: Enums.VoteType.Abstain },
+          { voter: voter1, nfts: [NFT0], support: Enums.VoteType.For },
+          { voter: voter2, nfts: [NFT1, NFT2], support: Enums.VoteType.For },
+          { voter: voter3, nfts: [NFT3], support: Enums.VoteType.Against },
+          { voter: voter4, nfts: [NFT4], support: Enums.VoteType.Abstain },
         ],
       };
     });
@@ -77,16 +92,25 @@ contract('GovernorERC721Mock', function (accounts) {
           this.settings.voters.find(({ address }) => address === voter),
         );
 
-        expect(await this.token.getVotes(voter, vote.blockNumber)).to.be.bignumber.equal('1');
+        if (voter === voter2) {
+          expect(await this.token.getVotes(voter, vote.blockNumber)).to.be.bignumber.equal('2');
+        } else {
+          expect(await this.token.getVotes(voter, vote.blockNumber)).to.be.bignumber.equal('1');
+        }
       }
 
       await this.mock.proposalVotes(this.id).then(result => {
         for (const [key, value] of Object.entries(Enums.VoteType)) {
           expect(result[`${key.toLowerCase()}Votes`]).to.be.bignumber.equal(
-            Object.values(this.settings.voters).filter(({ support }) => support === value).length.toString(),
+            Object.values(this.settings.voters).filter(({ support }) => support === value).reduce(
+              (acc, { nfts }) => acc.add(new BN(nfts.length)),
+              new BN('0'),
+            ),
           );
         }
       });
+
+      expect(await this.mock.state(this.id)).to.be.bignumber.equal(ProposalState.Executed);
     });
 
     runGovernorWorkflow();