Skip to content

Commit

Permalink
Add ERC721 and ERC1155 receiver support in Governor, Timelock (#3230)
Browse files Browse the repository at this point in the history 
* add ERC721 and ERC1155 receiver support in Governor, Timelock and MinimalForwarder (#3174)

* revert the nft receiver hooks from MinimalForwarder and linting updates

* add ERC165 support & simplify test

* add changelog entry

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
  • Loading branch information
@ashwinYardi @Amxx
ashwinYardi and Amxx committed Mar 24, 2022
1 parent 74c9130 commit 76fca3a
Show file tree
Hide file tree
Showing 5 changed files with 295 additions and 87 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -13,6 +13,8 @@
* `TimelockController`: Add a separate canceller role for the ability to cancel. ([#3165](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3165))
* `draft-ERC20Permit`: replace `immutable` with `constant` for `_PERMIT_TYPEHASH` since the `keccak256` of string literals is treated specially and the hash is evaluated at compile time. ([#3196](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3196))
* `ERC20Wrapper`: the `decimals()` function now tries to fetch the value from the underlying token instance. If that calls revert, then the default value is used. ([#3259](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3259))
* `Governor`: Implement `IERC721Receiver` and `IERC1155Receiver` to improve token custody by governors. ([#3230](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3230))
* `TimelockController`: Implement `IERC721Receiver` and `IERC1155Receiver` to improve token custody by timelocks. ([#3230](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3230))

### Breaking changes

Expand Down
43 changes: 42 additions & 1 deletion contracts/governance/Governor.sol
View file
Expand Up @@ -3,6 +3,8 @@

pragma solidity ^0.8.0;

import "../token/ERC721/IERC721Receiver.sol";
import "../token/ERC1155/IERC1155Receiver.sol";
import "../utils/cryptography/ECDSA.sol";
import "../utils/cryptography/draft-EIP712.sol";
import "../utils/introspection/ERC165.sol";
Expand All @@ -24,7 +26,7 @@ import "./IGovernor.sol";
*
* _Available since v4.3._
*/
abstract contract Governor is Context, ERC165, EIP712, IGovernor {
abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receiver, IERC1155Receiver {
using DoubleEndedQueue for DoubleEndedQueue.Bytes32Deque;
using SafeCast for uint256;
using Timers for Timers.BlockNumber;
Expand Down Expand Up @@ -97,6 +99,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
this.castVoteWithReasonAndParamsBySig.selector ^
this.getVotesWithParams.selector) ||
interfaceId == type(IGovernor).interfaceId ||
interfaceId == type(IERC1155Receiver).interfaceId ||
super.supportsInterface(interfaceId);
}

Expand Down Expand Up @@ -552,4 +555,42 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
function _executor() internal view virtual returns (address) {
return address(this);
}

/**
* @dev See {IERC721Receiver-onERC721Received}.
*/
function onERC721Received(
address,
address,
uint256,
bytes memory
) public virtual override returns (bytes4) {
return this.onERC721Received.selector;
}

/**
* @dev See {IERC1155Receiver-onERC1155Received}.
*/
function onERC1155Received(
address,
address,
uint256,
uint256,
bytes memory
) public virtual override returns (bytes4) {
return this.onERC1155Received.selector;
}

/**
* @dev See {IERC1155Receiver-onERC1155BatchReceived}.
*/
function onERC1155BatchReceived(
address,
address,
uint256[] memory,
uint256[] memory,
bytes memory
) public virtual override returns (bytes4) {
return this.onERC1155BatchReceived.selector;
}
}
49 changes: 48 additions & 1 deletion contracts/governance/TimelockController.sol
View file
Expand Up @@ -4,6 +4,8 @@
pragma solidity ^0.8.0;

import "../access/AccessControl.sol";
import "../token/ERC721/IERC721Receiver.sol";
import "../token/ERC1155/IERC1155Receiver.sol";

/**
* @dev Contract module which acts as a timelocked controller. When set as the
Expand All @@ -20,7 +22,7 @@ import "../access/AccessControl.sol";
*
* _Available since v3.3._
*/
contract TimelockController is AccessControl {
contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver {
bytes32 public constant TIMELOCK_ADMIN_ROLE = keccak256("TIMELOCK_ADMIN_ROLE");
bytes32 public constant PROPOSER_ROLE = keccak256("PROPOSER_ROLE");
bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
Expand Down Expand Up @@ -117,6 +119,13 @@ contract TimelockController is AccessControl {
*/
receive() external payable {}

/**
* @dev See {IERC165-supportsInterface}.
*/
function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, AccessControl) returns (bool) {
return interfaceId == type(IERC1155Receiver).interfaceId || super.supportsInterface(interfaceId);
}

/**
* @dev Returns whether an id correspond to a registered operation. This
* includes both Pending, Ready and Done operations.
Expand Down Expand Up @@ -365,4 +374,42 @@ contract TimelockController is AccessControl {
emit MinDelayChange(_minDelay, newDelay);
_minDelay = newDelay;
}

/**
* @dev See {IERC721Receiver-onERC721Received}.
*/
function onERC721Received(
address,
address,
uint256,
bytes memory
) public virtual override returns (bytes4) {
return this.onERC721Received.selector;
}

/**
* @dev See {IERC1155Receiver-onERC1155Received}.
*/
function onERC1155Received(
address,
address,
uint256,
uint256,
bytes memory
) public virtual override returns (bytes4) {
return this.onERC1155Received.selector;
}

/**
* @dev See {IERC1155Receiver-onERC1155BatchReceived}.
*/
function onERC1155BatchReceived(
address,
address,
uint256[] memory,
uint256[] memory,
bytes memory
) public virtual override returns (bytes4) {
return this.onERC1155BatchReceived.selector;
}
}
55 changes: 55 additions & 0 deletions test/governance/Governor.test.js
View file
Expand Up @@ -14,6 +14,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;
Expand Down Expand Up @@ -55,6 +57,7 @@ contract('Governor', function (accounts) {

shouldSupportInterfaces([
'ERC165',
'ERC1155Receiver',
'Governor',
'GovernorWithParams',
]);
Expand Down Expand Up @@ -574,4 +577,56 @@ contract('Governor', function (accounts) {
expect(await this.mock.proposalThreshold()).to.be.bignumber.equal('1000000000000000000');
});
});

describe('safe receive', function () {
describe('ERC721', function () {
const name = 'Non Fungible Token';
const symbol = 'NFT';
const tokenId = new BN(1);

beforeEach(async function () {
this.token = await ERC721Mock.new(name, symbol);
await this.token.mint(owner, tokenId);
});

it('can receive an ERC721 safeTransfer', async function () {
await this.token.safeTransferFrom(owner, this.mock.address, tokenId, { from: owner });
});
});

describe('ERC1155', function () {
const uri = 'https://token-cdn-domain/{id}.json';
const tokenIds = {
1: new BN(1000),
2: new BN(2000),
3: new BN(3000),
};

beforeEach(async function () {
this.token = await ERC1155Mock.new(uri);
await this.token.mintBatch(owner, Object.keys(tokenIds), Object.values(tokenIds), '0x');
});

it('can receive ERC1155 safeTransfer', async function () {
await this.token.safeTransferFrom(
owner,
this.mock.address,
...Object.entries(tokenIds)[0], // id + amount
'0x',
{ from: owner },
);
});

it('can receive ERC1155 safeBatchTransfer', async function () {
await this.token.safeBatchTransferFrom(
owner,
this.mock.address,
Object.keys(tokenIds),
Object.values(tokenIds),
'0x',
{ from: owner },
);
});
});
});
});

0 comments on commit 76fca3a

Please sign in to comment.