add ERC721 and ERC1155 receiver support in Governor, Timelock and Min…

…imalForwarder (OpenZeppelin#3174)

contracts/governance/Governor.sol

@@ -438,4 +438,56 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
     function _executor() internal view virtual returns (address) {
         return address(this);
     }
+
+
+    // Hooks
+    // IMPORTANT: When inheriting this contract, you must include a way to use the received tokens, otherwise they will be stuck.
+
+     /**
+     * @dev See {IERC721Receiver-onERC721Received}.
+     *
+     * Always returns `onERC721Received.selector`.
+     * The reason for adding this hook here is to enable Governor contract to accept ERC-721 tokens if required after proposal has been executed.
+     */
+    function onERC721Received(
+        address,
+        address,
+        uint256,
+        bytes memory
+    ) public virtual returns (bytes4) {
+        return this.onERC721Received.selector;
+    }
+
+    /**
+     * @dev See {IERC1155Receiver-onERC1155Received}.
+     *
+     * Always returns `onERC1155Received.selector`.
+     * The reason for adding this hook here is to enable Governor contract to accept ERC-1155 tokens if required after proposal has been executed.
+     */
+    function onERC1155Received(
+        address,
+        address,
+        uint256,
+        uint256,
+        bytes memory
+    ) public virtual returns (bytes4) {
+        return this.onERC1155Received.selector;
+    }
+
+    /**
+     * @dev See {IERC1155Receiver-onERC1155BatchReceived}.
+     *
+     * Always returns `onERC1155BatchReceived.selector`.
+     * The reason for adding this hook here is to enable Governor contract to accept batch ERC-1155 tokens if required after proposal has been executed.
+     */
+    function onERC1155BatchReceived(
+        address,
+        address,
+        uint256[] memory,
+        uint256[] memory,
+        bytes memory
+    ) public virtual returns (bytes4) {
+        return this.onERC1155BatchReceived.selector;
+    }
+
 }

contracts/governance/TimelockController.sol

@@ -353,4 +353,54 @@ contract TimelockController is AccessControl {
         emit MinDelayChange(_minDelay, newDelay);
         _minDelay = newDelay;
     }
+
+    // Hooks
+
+     /**
+     * @dev See {IERC721Receiver-onERC721Received}.
+     *
+     * Always returns `onERC721Received.selector`.
+     * The reason for adding this hook here is to enable Timelock contract to accept ERC-721 tokens if required after proposal has been executed.
+     */
+    function onERC721Received(
+        address,
+        address,
+        uint256,
+        bytes memory
+    ) public virtual returns (bytes4) {
+        return this.onERC721Received.selector;
+    }
+
+    /**
+     * @dev See {IERC1155Receiver-onERC1155Received}.
+     *
+     * Always returns `onERC1155Received.selector`.
+     * The reason for adding this hook here is to enable Timelock contract to accept ERC-1155 tokens if required after proposal has been executed.
+     */
+    function onERC1155Received(
+        address,
+        address,
+        uint256,
+        uint256,
+        bytes memory
+    ) public virtual returns (bytes4) {
+        return this.onERC1155Received.selector;
+    }
+
+    /**
+     * @dev See {IERC1155Receiver-onERC1155BatchReceived}.
+     *
+     * Always returns `onERC1155BatchReceived.selector`.
+     * The reason for adding this hook here is to enable Timelock contract to accept batch ERC-1155 tokens if required after proposal has been executed.
+     */
+    function onERC1155BatchReceived(
+        address,
+        address,
+        uint256[] memory,
+        uint256[] memory,
+        bytes memory
+    ) public virtual returns (bytes4) {
+        return this.onERC1155BatchReceived.selector;
+    }
+
 }

contracts/metatx/MinimalForwarder.sol

@@ -64,4 +64,54 @@ contract MinimalForwarder is EIP712 {
 
         return (success, returndata);
     }
+
+    // Hooks
+
+     /**
+     * @dev See {IERC721Receiver-onERC721Received}.
+     *
+     * Always returns `onERC721Received.selector`.
+     * The reason for adding this hook here is to enable Forwarder contract to accept ERC-721 tokens if required after proposal has been executed.
+     */
+    function onERC721Received(
+        address,
+        address,
+        uint256,
+        bytes memory
+    ) public virtual returns (bytes4) {
+        return this.onERC721Received.selector;
+    }
+
+    /**
+     * @dev See {IERC1155Receiver-onERC1155Received}.
+     *
+     * Always returns `onERC1155Received.selector`.
+     * The reason for adding this hook here is to enable Forwarder contract to accept ERC-1155 tokens if required after proposal has been executed.
+     */
+    function onERC1155Received(
+        address,
+        address,
+        uint256,
+        uint256,
+        bytes memory
+    ) public virtual returns (bytes4) {
+        return this.onERC1155Received.selector;
+    }
+
+    /**
+     * @dev See {IERC1155Receiver-onERC1155BatchReceived}.
+     *
+     * Always returns `onERC1155BatchReceived.selector`.
+     * The reason for adding this hook here is to enable Forwarder contract to accept batch ERC-1155 tokens if required after proposal has been executed.
+     */
+    function onERC1155BatchReceived(
+        address,
+        address,
+        uint256[] memory,
+        uint256[] memory,
+        bytes memory
+    ) public virtual returns (bytes4) {
+        return this.onERC1155BatchReceived.selector;
+    }
+
 }

test/governance/Governor.test.js

@@ -16,6 +16,8 @@ const {
 const Token = artifacts.require('ERC20VotesMock');
 const Governor = artifacts.require('GovernorMock');
 const CallReceiver = artifacts.require('CallReceiverMock');
+const ERC721Mock = artifacts.require('ERC721Mock');
+const ERC1155Mock = artifacts.require('ERC1155Mock');
 
 contract('Governor', function (accounts) {
   const [ owner, proposer, voter1, voter2, voter3, voter4 ] = accounts;
@@ -943,4 +945,69 @@ contract('Governor', function (accounts) {
       });
     });
   });
+
+  describe('onERC721Received', function() {
+    const name = 'Non Fungible Token';
+    const symbol = 'NFT';
+
+    it('receives an ERC721 token', async function() {
+      const token = await ERC721Mock.new(name, symbol);
+      const tokenId = new BN(1);
+      await token.mint(owner, tokenId);
+
+      await token.safeTransferFrom(owner, this.mock.address, tokenId, { from: owner });
+
+      expect(await token.ownerOf(tokenId)).to.be.equal(this.mock.address);
+    });
+  });
+
+  describe('onERC1155Received', function () {
+    const uri = 'https://token-cdn-domain/{id}.json';
+    const multiTokenIds = [new BN(1), new BN(2), new BN(3)];
+    const multiTokenAmounts = [new BN(1000), new BN(2000), new BN(3000)];
+    const transferData = '0x12345678';
+
+    beforeEach(async function () {
+      this.multiToken = await ERC1155Mock.new(uri, { from: owner });
+      await this.multiToken.mintBatch(owner, multiTokenIds, multiTokenAmounts, '0x', { from: owner });
+    });
+
+    it('receives ERC1155 tokens from a single ID', async function () {
+      await this.multiToken.safeTransferFrom(
+        owner,
+        this.mock.address,
+        multiTokenIds[0],
+        multiTokenAmounts[0],
+        transferData,
+        { from: owner },
+      );
+
+      expect(await this.multiToken.balanceOf(this.mock.address, multiTokenIds[0]))
+        .to.be.bignumber.equal(multiTokenAmounts[0]);
+
+      for (let i = 1; i < multiTokenIds.length; i++) {
+        expect(await this.multiToken.balanceOf(this.mock.address, multiTokenIds[i])).to.be.bignumber.equal(new BN(0));
+      }
+    });
+
+    it('receives ERC1155 tokens from a multiple IDs', async function () {
+      for (let i = 0; i < multiTokenIds.length; i++) {
+        expect(await this.multiToken.balanceOf(this.mock.address, multiTokenIds[i])).to.be.bignumber.equal(new BN(0));
+      };
+
+      await this.multiToken.safeBatchTransferFrom(
+        owner,
+        this.mock.address,
+        multiTokenIds,
+        multiTokenAmounts,
+        transferData,
+        { from: owner },
+      );
+
+      for (let i = 0; i < multiTokenIds.length; i++) {
+        expect(await this.multiToken.balanceOf(this.mock.address, multiTokenIds[i]))
+          .to.be.bignumber.equal(multiTokenAmounts[i]);
+      }
+    }); 
+  });
 });

test/governance/TimelockController.test.js

@@ -1,11 +1,14 @@
-const { constants, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers');
+const { BN, constants, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers');
 const { ZERO_BYTES32 } = constants;
 
 const { expect } = require('chai');
 
 const TimelockController = artifacts.require('TimelockController');
 const CallReceiverMock = artifacts.require('CallReceiverMock');
 const Implementation2 = artifacts.require('Implementation2');
+const ERC721Mock = artifacts.require('ERC721Mock');
+const ERC1155Mock = artifacts.require('ERC1155Mock');
+
 const MINDELAY = time.duration.days(1);
 
 function genOperation (target, value, data, predecessor, salt) {
@@ -1029,4 +1032,69 @@ contract('TimelockController', function (accounts) {
       expect(await web3.eth.getBalance(this.callreceivermock.address)).to.be.bignumber.equal(web3.utils.toBN(0));
     });
   });
+
+  describe('onERC721Received', function() {
+    const name = 'Non Fungible Token';
+    const symbol = 'NFT';
+
+    it('receives an ERC721 token', async function() {
+      const token = await ERC721Mock.new(name, symbol);
+      const tokenId = new BN(1);
+      await token.mint(other, tokenId);
+
+      await token.safeTransferFrom(other, this.timelock.address, tokenId, { from: other });
+
+      expect(await token.ownerOf(tokenId)).to.be.equal(this.timelock.address);
+    });
+  });
+
+  describe('onERC1155Received', function () {
+    const uri = 'https://token-cdn-domain/{id}.json';
+    const multiTokenIds = [new BN(1), new BN(2), new BN(3)];
+    const multiTokenAmounts = [new BN(1000), new BN(2000), new BN(3000)];
+    const transferData = '0x12345678';
+
+    beforeEach(async function () {
+      this.multiToken = await ERC1155Mock.new(uri, { from: other });
+      await this.multiToken.mintBatch(other, multiTokenIds, multiTokenAmounts, '0x', { from: other });
+    });
+
+    it('receives ERC1155 tokens from a single ID', async function () {
+      await this.multiToken.safeTransferFrom(
+        other,
+        this.timelock.address,
+        multiTokenIds[0],
+        multiTokenAmounts[0],
+        transferData,
+        { from: other },
+      );
+
+      expect(await this.multiToken.balanceOf(this.timelock.address, multiTokenIds[0]))
+        .to.be.bignumber.equal(multiTokenAmounts[0]);
+
+      for (let i = 1; i < multiTokenIds.length; i++) {
+        expect(await this.multiToken.balanceOf(this.timelock.address, multiTokenIds[i])).to.be.bignumber.equal(new BN(0));
+      }
+    });
+
+    it('receives ERC1155 tokens from a multiple IDs', async function () {
+      for (let i = 0; i < multiTokenIds.length; i++) {
+        expect(await this.multiToken.balanceOf(this.timelock.address, multiTokenIds[i])).to.be.bignumber.equal(new BN(0));
+      };
+
+      await this.multiToken.safeBatchTransferFrom(
+        other,
+        this.timelock.address,
+        multiTokenIds,
+        multiTokenAmounts,
+        transferData,
+        { from: other },
+      );
+
+      for (let i = 0; i < multiTokenIds.length; i++) {
+        expect(await this.multiToken.balanceOf(this.timelock.address, multiTokenIds[i]))
+          .to.be.bignumber.equal(multiTokenAmounts[i]);
+      }
+    }); 
+  });
 });