diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 05d8b2adedd..264dc71e5d5 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -12,7 +12,7 @@ jobs: - uses: actions/setup-node@v3 with: node-version: 12.x - - uses: actions/cache@v2 + - uses: actions/cache@v3 id: cache with: path: '**/node_modules' diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d119119c0c7..0877b5fef5b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,7 +16,7 @@ jobs: - uses: actions/setup-node@v3 with: node-version: 12.x - - uses: actions/cache@v2 + - uses: actions/cache@v3 id: cache with: path: '**/node_modules' @@ -42,7 +42,7 @@ jobs: - uses: actions/setup-node@v3 with: node-version: 12.x - - uses: actions/cache@v2 + - uses: actions/cache@v3 id: cache with: path: '**/node_modules' @@ -62,7 +62,7 @@ jobs: - uses: actions/setup-node@v3 with: node-version: 12.x - - uses: actions/cache@v2 + - uses: actions/cache@v3 id: cache with: path: '**/node_modules' diff --git a/CHANGELOG.md b/CHANGELOG.md index 68466f3a523..2eef2cb91c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,14 +4,18 @@ * `AccessControl`: add a virtual `_checkRole(bytes32)` function that can be overridden to alter the `onlyRole` modifier behavior. ([#3137](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3137)) * `EnumerableMap`: add new `AddressToUintMap` map type. ([#3150](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3150)) + * `EnumerableMap`: add new `Bytes32ToBytes32Map` map type. ([#3192](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3192)) * `ERC1155`: Add a `_afterTokenTransfer` hook for improved extensibility. ([#3166](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3166)) * `DoubleEndedQueue`: a new data structure that supports efficient push and pop to both front and back, useful for FIFO and LIFO queues. ([#3153](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3153)) * `Governor`: improved security of `onlyGovernance` modifier when using an external executor contract (e.g. a timelock) that can operate without necessarily going through the governance protocol. ([#3147](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3147)) - * `Governor`: Add a way to parameterize votes. This can be used to implement voting systems such as fractionalized voting, ERC721 based voting, or any number of other systems. The `params` argument added to `_countVote` method, and included in the newly added `_getVotes` method, can be used by counting and voting modules respectively for such purposes. + * `Governor`: Add a way to parameterize votes. This can be used to implement voting systems such as fractionalized voting, ERC721 based voting, or any number of other systems. The `params` argument added to `_countVote` method, and included in the newly added `_getVotes` method, can be used by counting and voting modules respectively for such purposes. ([#3043](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3043)) * `Governor`: rewording of revert reason for consistency. ([#3275](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3275)) * `ERC20FlashMint`: support infinite allowance when paying back a flash loan. ([#3226](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3226)) * `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 @@ -457,7 +461,7 @@ Refer to the table below to adjust your inheritance list. * `SignedSafeMath`: added overflow-safe operations for signed integers (`int256`). ([#1559](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1559), [#1588](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1588)) ### Improvements - * The compiler version required by `Array` was behind the rest of the libray so it was updated to `v0.4.24`. ([#1553](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1553)) + * The compiler version required by `Array` was behind the rest of the library so it was updated to `v0.4.24`. ([#1553](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1553)) * Now conforming to a 4-space indentation code style. ([1508](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1508)) * `ERC20`: more gas efficient due to removed redundant `require`s. ([#1409](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1409)) * `ERC721`: fixed a bug that prevented internal data structures from being properly cleaned, missing potential gas refunds. ([#1539](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1539) and [#1549](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1549)) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1c55795d150..501284773a8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,7 +51,7 @@ refer to some very important conditions that your PR must meet in order to be ac 6) Maintainers will review your code and possibly ask for changes before your code is pulled in to the main repository. We'll check that all tests pass, review the coding style, and check for general code correctness. If everything is OK, we'll merge your pull request and your code will be part of OpenZeppelin Contracts. -*IMPORTANT* Please pay attention to the maintainer's feedback, since its a necessary step to keep up with the standards OpenZeppelin Contracts attains to. +*IMPORTANT* Please pay attention to the maintainer's feedback, since it's a necessary step to keep up with the standards OpenZeppelin Contracts attains to. ## All set! diff --git a/audit/2017-03.md b/audit/2017-03.md index 53eb702aba1..5ca874bebaa 100644 --- a/audit/2017-03.md +++ b/audit/2017-03.md @@ -133,7 +133,7 @@ I presume that the goal of this contract is to allow and annotate a migration to We like these pauses! Note that these allow significant griefing potential by owners, and that this might not be obvious to participants in smart contracts using the OpenZeppelin framework. We would recommend that additional sample logic be added to for instance the TokenContract showing safer use of the pause and resume functions. In particular, we would recommend a timelock after which anyone could unpause the contract. -The modifers use the pattern `if(bool){_;}`. This is fine for functions that return false upon failure, but could be problematic for functions expected to throw upon failure. See our comments above on standardizing on `throw` or `return(false)`. +The modifiers use the pattern `if(bool){_;}`. This is fine for functions that return false upon failure, but could be problematic for functions expected to throw upon failure. See our comments above on standardizing on `throw` or `return(false)`. ## Ownership diff --git a/certora/applyHarness.patch b/certora/applyHarness.patch index 42b10fab558..0fbe9acad2b 100644 --- a/certora/applyHarness.patch +++ b/certora/applyHarness.patch @@ -71,7 +71,7 @@ diff -ruN governance/Governor.sol governance/Governor.sol + /** * @dev Restrict access to governor executing address. Some module might override the _executor function to make - * sure this modifier is consistant with the execution model. + * sure this modifier is consistent with the execution model. @@ -167,12 +167,12 @@ /** * @dev Amount of votes already cast passes the threshold limit. diff --git a/certora/specs/GovernorBase.spec b/certora/specs/GovernorBase.spec index 031b2680e9d..3dfc180377d 100644 --- a/certora/specs/GovernorBase.spec +++ b/certora/specs/GovernorBase.spec @@ -173,11 +173,11 @@ rule executionOnlyIfQuoromReachedAndVoteSucceeded(uint256 pId, env e, method f){ /* * A user cannot vote twice */ - // Checked for castVote only. all 3 castVote functions call _castVote, so the completness of the verification is counted on - // the fact that the 3 functions themselves makes no chages, but rather call an internal function to execute. + // Checked for castVote only. all 3 castVote functions call _castVote, so the completeness of the verification is counted on + // the fact that the 3 functions themselves makes no changes, but rather call an internal function to execute. // That means that we do not check those 3 functions directly, however for castVote & castVoteWithReason it is quite trivial // to understand why this is ok. For castVoteBySig we basically assume that the signature referendum is correct without checking it. - // We could check each function seperately and pass the rule, but that would have uglyfied the code with no concrete + // We could check each function separately and pass the rule, but that would have uglyfied the code with no concrete // benefit, as it is evident that nothing is happening in the first 2 functions (calling a view function), and we do not desire to check the signature verification. rule doubleVoting(uint256 pId, uint8 sup, method f) { env e; diff --git a/certora/specs/GovernorCountingSimple.spec b/certora/specs/GovernorCountingSimple.spec index 6d2d5fbb72c..7af73beb6f8 100644 --- a/certora/specs/GovernorCountingSimple.spec +++ b/certora/specs/GovernorCountingSimple.spec @@ -128,11 +128,11 @@ invariant OneIsNotMoreThanAll(uint256 pId) /* * Only sender's voting status can be changed by execution of any cast vote function */ -// Checked for castVote only. all 3 castVote functions call _castVote, so the completness of the verification is counted on - // the fact that the 3 functions themselves makes no chages, but rather call an internal function to execute. +// Checked for castVote only. all 3 castVote functions call _castVote, so the completeness of the verification is counted on + // the fact that the 3 functions themselves makes no changes, but rather call an internal function to execute. // That means that we do not check those 3 functions directly, however for castVote & castVoteWithReason it is quite trivial // to understand why this is ok. For castVoteBySig we basically assume that the signature referendum is correct without checking it. - // We could check each function seperately and pass the rule, but that would have uglyfied the code with no concrete + // We could check each function separately and pass the rule, but that would have uglyfied the code with no concrete // benefit, as it is evident that nothing is happening in the first 2 functions (calling a view function), and we do not desire to check the signature verification. rule noVoteForSomeoneElse(uint256 pId, uint8 sup, method f) { env e; calldataarg args; @@ -205,7 +205,7 @@ rule privilegedOnlyNumerator(method f, uint256 newQuorumNumerator){ uint256 quorumNumAfter = quorumNumerator(e); address executorCheck = getExecutor(e); - assert quorumNumBefore != quorumNumAfter => e.msg.sender == executorCheck, "non priveleged user changed quorum numerator"; + assert quorumNumBefore != quorumNumAfter => e.msg.sender == executorCheck, "non privileged user changed quorum numerator"; } rule privilegedOnlyTimelock(method f, uint256 newQuorumNumerator){ @@ -217,5 +217,5 @@ rule privilegedOnlyTimelock(method f, uint256 newQuorumNumerator){ uint256 timelockAfter = timelock(e); - assert timelockBefore != timelockAfter => e.msg.sender == timelockBefore, "non priveleged user changed timelock"; + assert timelockBefore != timelockAfter => e.msg.sender == timelockBefore, "non privileged user changed timelock"; } diff --git a/contracts/finance/VestingWallet.sol b/contracts/finance/VestingWallet.sol index 5ffbfcb6537..486ad3789b0 100644 --- a/contracts/finance/VestingWallet.sol +++ b/contracts/finance/VestingWallet.sol @@ -120,7 +120,7 @@ contract VestingWallet is Context { } /** - * @dev Virtual implementation of the vesting formula. This returns the amout vested, as a function of time, for + * @dev Virtual implementation of the vesting formula. This returns the amount vested, as a function of time, for * an asset given its total historical allocation. */ function _vestingSchedule(uint256 totalAllocation, uint64 timestamp) internal view virtual returns (uint256) { diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 6abe715797d..a18b985776c 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -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"; @@ -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; @@ -46,7 +48,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { // This queue keeps track of the governor operating on itself. Calls to functions protected by the // {onlyGovernance} modifier needs to be whitelisted in this queue. Whitelisting is set in {_beforeExecute}, - // consummed by the {onlyGovernance} modifier and eventually reset in {_afterExecute}. This ensures that the + // consumed by the {onlyGovernance} modifier and eventually reset in {_afterExecute}. This ensures that the // execution of {onlyGovernance} protected calls can only be achieved through successful proposals. DoubleEndedQueue.Bytes32Deque private _governanceCall; @@ -64,7 +66,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { require(_msgSender() == _executor(), "Governor: onlyGovernance"); if (_executor() != address(this)) { bytes32 msgDataHash = keccak256(_msgData()); - // loop until poping the expected operation - throw if deque is empty (operation not authorized) + // loop until popping the expected operation - throw if deque is empty (operation not authorized) while (_governanceCall.popFront() != msgDataHash) {} } _; @@ -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); } @@ -124,7 +127,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { * * Note that the chainId and the governor address are not part of the proposal id computation. Consequently, the * same proposal (with same operation and same description) will have the same id if submitted on multiple governors - * accross multiple networks. This also means that in order to execute the same operation twice (on the same + * across multiple networks. This also means that in order to execute the same operation twice (on the same * governor) the proposer will have to change the description in order to avoid proposal id conflicts. */ function hashProposal( @@ -229,7 +232,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { /** * @dev Default additional encoded parameters used by castVote methods that don't include them * - * Note: Should be overriden by specific implementations to use an appropriate value, the + * Note: Should be overridden by specific implementations to use an appropriate value, the * meaning of the additional params, in the context of that implementation */ function _defaultParams() internal view virtual returns (bytes memory) { @@ -308,7 +311,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { } /** - * @dev Internal execution mechanism. Can be overriden to implement different execution mechanism + * @dev Internal execution mechanism. Can be overridden to implement different execution mechanism */ function _execute( uint256, /* proposalId */ @@ -325,7 +328,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { } /** - * @dev Hook before execution is trigerred. + * @dev Hook before execution is triggered. */ function _beforeExecute( uint256, /* proposalId */ @@ -344,7 +347,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { } /** - * @dev Hook after execution is trigerred. + * @dev Hook after execution is triggered. */ function _afterExecute( uint256, /* proposalId */ @@ -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; + } } diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol index eaf443a97d1..abb75d2eef1 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -158,8 +158,8 @@ abstract contract IGovernor is IERC165 { * @notice module:user-config * @dev Minimum number of cast voted required for a proposal to be successful. * - * Note: The `blockNumber` parameter corresponds to the snaphot used for counting vote. This allows to scale the - * quroum depending on values such as the totalSupply of a token at this block (see {ERC20Votes}). + * Note: The `blockNumber` parameter corresponds to the snapshot used for counting vote. This allows to scale the + * quorum depending on values such as the totalSupply of a token at this block (see {ERC20Votes}). */ function quorum(uint256 blockNumber) public view virtual returns (uint256); diff --git a/contracts/governance/README.adoc b/contracts/governance/README.adoc index 58daf56e70d..9e393e9a0ca 100644 --- a/contracts/governance/README.adoc +++ b/contracts/governance/README.adoc @@ -40,7 +40,7 @@ Other extensions can customize the behavior or interface in multiple ways. * {GovernorCompatibilityBravo}: Extends the interface to be fully `GovernorBravo`-compatible. Note that events are compatible regardless of whether this extension is included or not. -* {GovernorSettings}: Manages some of the settings (voting delay, voting period duration, and proposal threshold) in a way that can be updated through a governance proposal, without requiering an upgrade. +* {GovernorSettings}: Manages some of the settings (voting delay, voting period duration, and proposal threshold) in a way that can be updated through a governance proposal, without requiring an upgrade. * {GovernorPreventLateQuorum}: Ensures there is a minimum voting period after quorum is reached as a security protection against large voters. diff --git a/contracts/governance/TimelockController.sol b/contracts/governance/TimelockController.sol index ba3d54fe1f4..376023fe35b 100644 --- a/contracts/governance/TimelockController.sol +++ b/contracts/governance/TimelockController.sol @@ -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 @@ -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"); @@ -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. @@ -185,11 +194,11 @@ contract TimelockController is AccessControl { function hashOperationBatch( address[] calldata targets, uint256[] calldata values, - bytes[] calldata datas, + bytes[] calldata payloads, bytes32 predecessor, bytes32 salt ) public pure virtual returns (bytes32 hash) { - return keccak256(abi.encode(targets, values, datas, predecessor, salt)); + return keccak256(abi.encode(targets, values, payloads, predecessor, salt)); } /** @@ -226,18 +235,18 @@ contract TimelockController is AccessControl { function scheduleBatch( address[] calldata targets, uint256[] calldata values, - bytes[] calldata datas, + bytes[] calldata payloads, bytes32 predecessor, bytes32 salt, uint256 delay ) public virtual onlyRole(PROPOSER_ROLE) { require(targets.length == values.length, "TimelockController: length mismatch"); - require(targets.length == datas.length, "TimelockController: length mismatch"); + require(targets.length == payloads.length, "TimelockController: length mismatch"); - bytes32 id = hashOperationBatch(targets, values, datas, predecessor, salt); + bytes32 id = hashOperationBatch(targets, values, payloads, predecessor, salt); _schedule(id, delay); for (uint256 i = 0; i < targets.length; ++i) { - emit CallScheduled(id, i, targets[i], values[i], datas[i], predecessor, delay); + emit CallScheduled(id, i, targets[i], values[i], payloads[i], predecessor, delay); } } @@ -301,17 +310,17 @@ contract TimelockController is AccessControl { function executeBatch( address[] calldata targets, uint256[] calldata values, - bytes[] calldata datas, + bytes[] calldata payloads, bytes32 predecessor, bytes32 salt ) public payable virtual onlyRoleOrOpenRole(EXECUTOR_ROLE) { require(targets.length == values.length, "TimelockController: length mismatch"); - require(targets.length == datas.length, "TimelockController: length mismatch"); + require(targets.length == payloads.length, "TimelockController: length mismatch"); - bytes32 id = hashOperationBatch(targets, values, datas, predecessor, salt); + bytes32 id = hashOperationBatch(targets, values, payloads, predecessor, salt); _beforeCall(id, predecessor); for (uint256 i = 0; i < targets.length; ++i) { - _call(id, i, targets[i], values[i], datas[i]); + _call(id, i, targets[i], values[i], payloads[i]); } _afterCall(id); } @@ -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; + } } diff --git a/contracts/governance/extensions/GovernorTimelockCompound.sol b/contracts/governance/extensions/GovernorTimelockCompound.sol index 99aea98ba7d..d14489aa098 100644 --- a/contracts/governance/extensions/GovernorTimelockCompound.sol +++ b/contracts/governance/extensions/GovernorTimelockCompound.sol @@ -105,7 +105,7 @@ abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor { } /** - * @dev Overriden version of the {Governor-state} function with added support for the `Queued` and `Expired` status. + * @dev Overridden version of the {Governor-state} function with added support for the `Queued` and `Expired` status. */ function state(uint256 proposalId) public view virtual override(IGovernor, Governor) returns (ProposalState) { ProposalState status = super.state(proposalId); @@ -167,7 +167,7 @@ abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor { } /** - * @dev Overriden execute function that run the already queued proposal through the timelock. + * @dev Overridden execute function that run the already queued proposal through the timelock. */ function _execute( uint256 proposalId, @@ -185,7 +185,7 @@ abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor { } /** - * @dev Overriden version of the {Governor-_cancel} function to cancel the timelocked proposal if it as already + * @dev Overridden version of the {Governor-_cancel} function to cancel the timelocked proposal if it as already * been queued. */ function _cancel( diff --git a/contracts/governance/extensions/GovernorTimelockControl.sol b/contracts/governance/extensions/GovernorTimelockControl.sol index 2b302232448..8ab992f2e2c 100644 --- a/contracts/governance/extensions/GovernorTimelockControl.sol +++ b/contracts/governance/extensions/GovernorTimelockControl.sol @@ -47,7 +47,7 @@ abstract contract GovernorTimelockControl is IGovernorTimelock, Governor { } /** - * @dev Overriden version of the {Governor-state} function with added support for the `Queued` status. + * @dev Overridden version of the {Governor-state} function with added support for the `Queued` status. */ function state(uint256 proposalId) public view virtual override(IGovernor, Governor) returns (ProposalState) { ProposalState status = super.state(proposalId); @@ -107,7 +107,7 @@ abstract contract GovernorTimelockControl is IGovernorTimelock, Governor { } /** - * @dev Overriden execute function that run the already queued proposal through the timelock. + * @dev Overridden execute function that run the already queued proposal through the timelock. */ function _execute( uint256, /* proposalId */ @@ -120,7 +120,7 @@ abstract contract GovernorTimelockControl is IGovernorTimelock, Governor { } /** - * @dev Overriden version of the {Governor-_cancel} function to cancel the timelocked proposal if it as already + * @dev Overridden version of the {Governor-_cancel} function to cancel the timelocked proposal if it as already * been queued. */ // This function can reenter through the external call to the timelock, but we assume the timelock is trusted and diff --git a/contracts/interfaces/IERC2981.sol b/contracts/interfaces/IERC2981.sol index f0213986957..063582401bf 100644 --- a/contracts/interfaces/IERC2981.sol +++ b/contracts/interfaces/IERC2981.sol @@ -16,7 +16,7 @@ import "../utils/introspection/IERC165.sol"; interface IERC2981 is IERC165 { /** * @dev Returns how much royalty is owed and to whom, based on a sale price that may be denominated in any unit of - * exchange. The royalty amount is denominated and should be payed in that same unit of exchange. + * exchange. The royalty amount is denominated and should be paid in that same unit of exchange. */ function royaltyInfo(uint256 tokenId, uint256 salePrice) external diff --git a/contracts/mocks/ERC20DecimalsMock.sol b/contracts/mocks/ERC20DecimalsMock.sol index 924c3af31f3..1374cbc52a0 100644 --- a/contracts/mocks/ERC20DecimalsMock.sol +++ b/contracts/mocks/ERC20DecimalsMock.sol @@ -18,4 +18,12 @@ contract ERC20DecimalsMock is ERC20 { function decimals() public view virtual override returns (uint8) { return _decimals; } + + function mint(address account, uint256 amount) public { + _mint(account, amount); + } + + function burn(address account, uint256 amount) public { + _burn(account, amount); + } } diff --git a/contracts/mocks/EnumerableMapMock.sol b/contracts/mocks/EnumerableMapMock.sol index 21c1fe362d9..7cecd0d60d7 100644 --- a/contracts/mocks/EnumerableMapMock.sol +++ b/contracts/mocks/EnumerableMapMock.sol @@ -84,4 +84,50 @@ contract AddressToUintMapMock { function get(address key) public view returns (uint256) { return _map.get(key); } + + function getWithMessage(address key, string calldata errorMessage) public view returns (uint256) { + return _map.get(key, errorMessage); + } +} + +contract Bytes32ToBytes32MapMock { + using EnumerableMap for EnumerableMap.Bytes32ToBytes32Map; + + event OperationResult(bool result); + + EnumerableMap.Bytes32ToBytes32Map private _map; + + function contains(bytes32 key) public view returns (bool) { + return _map.contains(key); + } + + function set(bytes32 key, bytes32 value) public { + bool result = _map.set(key, value); + emit OperationResult(result); + } + + function remove(bytes32 key) public { + bool result = _map.remove(key); + emit OperationResult(result); + } + + function length() public view returns (uint256) { + return _map.length(); + } + + function at(uint256 index) public view returns (bytes32 key, bytes32 value) { + return _map.at(index); + } + + function tryGet(bytes32 key) public view returns (bool, bytes32) { + return _map.tryGet(key); + } + + function get(bytes32 key) public view returns (bytes32) { + return _map.get(key); + } + + function getWithMessage(bytes32 key, string calldata errorMessage) public view returns (bytes32) { + return _map.get(key, errorMessage); + } } diff --git a/contracts/mocks/InitializableMock.sol b/contracts/mocks/InitializableMock.sol index 630e8bbfad6..bdf53991fbc 100644 --- a/contracts/mocks/InitializableMock.sol +++ b/contracts/mocks/InitializableMock.sol @@ -59,3 +59,32 @@ contract ConstructorInitializableMock is Initializable { onlyInitializingRan = true; } } + +contract ReinitializerMock is Initializable { + uint256 public counter; + + function initialize() public initializer { + doStuff(); + } + + function reinitialize(uint8 i) public reinitializer(i) { + doStuff(); + } + + function nestedReinitialize(uint8 i, uint8 j) public reinitializer(i) { + reinitialize(j); + } + + function chainReinitialize(uint8 i, uint8 j) public { + reinitialize(i); + reinitialize(j); + } + + function disableInitializers() public { + _disableInitializers(); + } + + function doStuff() public onlyInitializing { + counter++; + } +} diff --git a/contracts/proxy/Proxy.sol b/contracts/proxy/Proxy.sol index f0e74d22f85..6a358fa474c 100644 --- a/contracts/proxy/Proxy.sol +++ b/contracts/proxy/Proxy.sol @@ -45,7 +45,7 @@ abstract contract Proxy { } /** - * @dev This is a virtual function that should be overriden so it returns the address to which the fallback function + * @dev This is a virtual function that should be overridden so it returns the address to which the fallback function * and {_fallback} should delegate. */ function _implementation() internal view virtual returns (address); @@ -80,7 +80,7 @@ abstract contract Proxy { * @dev Hook that is called before falling back to the implementation. Can happen as part of a manual `_fallback` * call, or as part of the Solidity `fallback` or `receive` functions. * - * If overriden should call `super._beforeFallback()`. + * If overridden should call `super._beforeFallback()`. */ function _beforeFallback() internal virtual {} } diff --git a/contracts/proxy/utils/Initializable.sol b/contracts/proxy/utils/Initializable.sol index 1fe4583a2b9..e6b2a4423e3 100644 --- a/contracts/proxy/utils/Initializable.sol +++ b/contracts/proxy/utils/Initializable.sol @@ -11,6 +11,26 @@ import "../../utils/Address.sol"; * external initializer function, usually called `initialize`. It then becomes necessary to protect this initializer * function so it can only be called once. The {initializer} modifier provided by this contract will have this effect. * + * The initialization functions use a version number. Once a version number is used, it is consumed and cannot be + * reused. This mechanism prevents re-execution of each "step" but allows the creation of new initialization steps in + * case an upgrade adds a module that needs to be initialized. + * + * For example: + * + * [.hljs-theme-light.nopadding] + * ``` + * contract MyToken is ERC20Upgradeable { + * function initialize() initializer public { + * __ERC20_init("MyToken", "MTK"); + * } + * } + * contract MyTokenV2 is MyToken, ERC20PermitUpgradeable { + * function initializeV2() reinitializer(2) public { + * __ERC20Permit_init("MyToken"); + * } + * } + * ``` + * * TIP: To avoid leaving the proxy in an uninitialized state, the initializer function should be called as early as * possible by providing the encoded function call as the `_data` argument to {ERC1967Proxy-constructor}. * @@ -22,21 +42,24 @@ import "../../utils/Address.sol"; * Avoid leaving a contract uninitialized. * * An uninitialized contract can be taken over by an attacker. This applies to both a proxy and its implementation - * contract, which may impact the proxy. To initialize the implementation contract, you can either invoke the - * initializer manually, or you can include a constructor to automatically mark it as initialized when it is deployed: + * contract, which may impact the proxy. To prevent the implementation contract from being used, you should invoke + * the {_disableInitializers} function in the constructor to automatically lock it when it is deployed: * * [.hljs-theme-light.nopadding] * ``` * /// @custom:oz-upgrades-unsafe-allow constructor - * constructor() initializer {} + * constructor() { + * _disableInitializers(); + * } * ``` * ==== */ abstract contract Initializable { /** * @dev Indicates that the contract has been initialized. + * @custom:oz-retyped-from bool */ - bool private _initialized; + uint8 private _initialized; /** * @dev Indicates that the contract is in the process of being initialized. @@ -44,22 +67,38 @@ abstract contract Initializable { bool private _initializing; /** - * @dev Modifier to protect an initializer function from being invoked twice. + * @dev A modifier that defines a protected initializer function that can be invoked at most once. In its scope, + * `onlyInitializing` functions can be used to initialize parent contracts. Equivalent to `reinitializer(1)`. */ modifier initializer() { - // If the contract is initializing we ignore whether _initialized is set in order to support multiple - // inheritance patterns, but we only do this in the context of a constructor, because in other contexts the - // contract may have been reentered. - require(_initializing ? _isConstructor() : !_initialized, "Initializable: contract is already initialized"); - - bool isTopLevelCall = !_initializing; + bool isTopLevelCall = _setInitializedVersion(1); if (isTopLevelCall) { _initializing = true; - _initialized = true; } - _; + if (isTopLevelCall) { + _initializing = false; + } + } + /** + * @dev A modifier that defines a protected reinitializer function that can be invoked at most once, and only if the + * contract hasn't been initialized to a greater version before. In its scope, `onlyInitializing` functions can be + * used to initialize parent contracts. + * + * `initializer` is equivalent to `reinitializer(1)`, so a reinitializer may be used after the original + * initialization step. This is essential to configure modules that are added through upgrades and that require + * initialization. + * + * Note that versions can jump in increments greater than 1; this implies that if multiple reinitializers coexist in + * a contract, executing them in the right order is up to the developer or operator. + */ + modifier reinitializer(uint8 version) { + bool isTopLevelCall = _setInitializedVersion(version); + if (isTopLevelCall) { + _initializing = true; + } + _; if (isTopLevelCall) { _initializing = false; } @@ -67,14 +106,37 @@ abstract contract Initializable { /** * @dev Modifier to protect an initialization function so that it can only be invoked by functions with the - * {initializer} modifier, directly or indirectly. + * {initializer} and {reinitializer} modifiers, directly or indirectly. */ modifier onlyInitializing() { require(_initializing, "Initializable: contract is not initializing"); _; } - function _isConstructor() private view returns (bool) { - return !Address.isContract(address(this)); + /** + * @dev Locks the contract, preventing any future reinitialization. This cannot be part of an initializer call. + * Calling this in the constructor of a contract will prevent that contract from being initialized or reinitialized + * to any version. It is recommended to use this to lock implementation contracts that are designed to be called + * through proxies. + */ + function _disableInitializers() internal virtual { + _setInitializedVersion(type(uint8).max); + } + + function _setInitializedVersion(uint8 version) private returns (bool) { + // If the contract is initializing we ignore whether _initialized is set in order to support multiple + // inheritance patterns, but we only do this in the context of a constructor, and for the lowest level + // of initializers, because in other contexts the contract may have been reentered. + if (_initializing) { + require( + version == 1 && !Address.isContract(address(this)), + "Initializable: contract is already initialized" + ); + return false; + } else { + require(_initialized < version, "Initializable: contract is already initialized"); + _initialized = version; + return true; + } } } diff --git a/contracts/token/ERC20/extensions/ERC20Wrapper.sol b/contracts/token/ERC20/extensions/ERC20Wrapper.sol index 32a3b830afe..c6dd0184a6d 100644 --- a/contracts/token/ERC20/extensions/ERC20Wrapper.sol +++ b/contracts/token/ERC20/extensions/ERC20Wrapper.sol @@ -22,6 +22,17 @@ abstract contract ERC20Wrapper is ERC20 { underlying = underlyingToken; } + /** + * @dev See {ERC20-decimals}. + */ + function decimals() public view virtual override returns (uint8) { + try IERC20Metadata(address(underlying)).decimals() returns (uint8 value) { + return value; + } catch { + return super.decimals(); + } + } + /** * @dev Allow a user to deposit underlying tokens and mint the corresponding number of wrapped tokens. */ diff --git a/contracts/utils/math/SafeMath.sol b/contracts/utils/math/SafeMath.sol index 6eb0aa6bde7..1e97d760243 100644 --- a/contracts/utils/math/SafeMath.sol +++ b/contracts/utils/math/SafeMath.sol @@ -28,7 +28,7 @@ library SafeMath { } /** - * @dev Returns the substraction of two unsigned integers, with an overflow flag. + * @dev Returns the subtraction of two unsigned integers, with an overflow flag. * * _Available since v3.4._ */ diff --git a/contracts/utils/structs/DoubleEndedQueue.sol b/contracts/utils/structs/DoubleEndedQueue.sol index 5ac2766ebe9..d1d968ef67f 100644 --- a/contracts/utils/structs/DoubleEndedQueue.sol +++ b/contracts/utils/structs/DoubleEndedQueue.sol @@ -24,7 +24,7 @@ library DoubleEndedQueue { error Empty(); /** - * @dev An operation (e.g. {at}) could't be completed due to an index being out of bounds. + * @dev An operation (e.g. {at}) couldn't be completed due to an index being out of bounds. */ error OutOfBounds(); diff --git a/contracts/utils/structs/EnumerableMap.sol b/contracts/utils/structs/EnumerableMap.sol index 1c3f4357ecf..8717c697811 100644 --- a/contracts/utils/structs/EnumerableMap.sol +++ b/contracts/utils/structs/EnumerableMap.sol @@ -30,6 +30,7 @@ import "./EnumerableSet.sol"; * * - `uint256 -> address` (`UintToAddressMap`) since v3.0.0 * - `address -> uint256` (`AddressToUintMap`) since v4.6.0 + * - `bytes32 -> bytes32` (`Bytes32ToBytes32`) since v4.6.0 */ library EnumerableMap { using EnumerableSet for EnumerableSet.Bytes32Set; @@ -43,7 +44,7 @@ library EnumerableMap { // This means that we can only create new EnumerableMaps for types that fit // in bytes32. - struct Map { + struct Bytes32ToBytes32Map { // Storage of keys EnumerableSet.Bytes32Set _keys; mapping(bytes32 => bytes32) _values; @@ -56,11 +57,11 @@ library EnumerableMap { * Returns true if the key was added to the map, that is if it was not * already present. */ - function _set( - Map storage map, + function set( + Bytes32ToBytes32Map storage map, bytes32 key, bytes32 value - ) private returns (bool) { + ) internal returns (bool) { map._values[key] = value; return map._keys.add(key); } @@ -70,7 +71,7 @@ library EnumerableMap { * * Returns true if the key was removed from the map, that is if it was present. */ - function _remove(Map storage map, bytes32 key) private returns (bool) { + function remove(Bytes32ToBytes32Map storage map, bytes32 key) internal returns (bool) { delete map._values[key]; return map._keys.remove(key); } @@ -78,14 +79,14 @@ library EnumerableMap { /** * @dev Returns true if the key is in the map. O(1). */ - function _contains(Map storage map, bytes32 key) private view returns (bool) { + function contains(Bytes32ToBytes32Map storage map, bytes32 key) internal view returns (bool) { return map._keys.contains(key); } /** * @dev Returns the number of key-value pairs in the map. O(1). */ - function _length(Map storage map) private view returns (uint256) { + function length(Bytes32ToBytes32Map storage map) internal view returns (uint256) { return map._keys.length(); } @@ -99,7 +100,7 @@ library EnumerableMap { * * - `index` must be strictly less than {length}. */ - function _at(Map storage map, uint256 index) private view returns (bytes32, bytes32) { + function at(Bytes32ToBytes32Map storage map, uint256 index) internal view returns (bytes32, bytes32) { bytes32 key = map._keys.at(index); return (key, map._values[key]); } @@ -108,10 +109,10 @@ library EnumerableMap { * @dev Tries to returns the value associated with `key`. O(1). * Does not revert if `key` is not in the map. */ - function _tryGet(Map storage map, bytes32 key) private view returns (bool, bytes32) { + function tryGet(Bytes32ToBytes32Map storage map, bytes32 key) internal view returns (bool, bytes32) { bytes32 value = map._values[key]; if (value == bytes32(0)) { - return (_contains(map, key), bytes32(0)); + return (contains(map, key), bytes32(0)); } else { return (true, value); } @@ -124,9 +125,9 @@ library EnumerableMap { * * - `key` must be in the map. */ - function _get(Map storage map, bytes32 key) private view returns (bytes32) { + function get(Bytes32ToBytes32Map storage map, bytes32 key) internal view returns (bytes32) { bytes32 value = map._values[key]; - require(value != 0 || _contains(map, key), "EnumerableMap: nonexistent key"); + require(value != 0 || contains(map, key), "EnumerableMap: nonexistent key"); return value; } @@ -136,20 +137,20 @@ library EnumerableMap { * CAUTION: This function is deprecated because it requires allocating memory for the error * message unnecessarily. For custom revert reasons use {_tryGet}. */ - function _get( - Map storage map, + function get( + Bytes32ToBytes32Map storage map, bytes32 key, string memory errorMessage - ) private view returns (bytes32) { + ) internal view returns (bytes32) { bytes32 value = map._values[key]; - require(value != 0 || _contains(map, key), errorMessage); + require(value != 0 || contains(map, key), errorMessage); return value; } // UintToAddressMap struct UintToAddressMap { - Map _inner; + Bytes32ToBytes32Map _inner; } /** @@ -164,7 +165,7 @@ library EnumerableMap { uint256 key, address value ) internal returns (bool) { - return _set(map._inner, bytes32(key), bytes32(uint256(uint160(value)))); + return set(map._inner, bytes32(key), bytes32(uint256(uint160(value)))); } /** @@ -173,21 +174,21 @@ library EnumerableMap { * Returns true if the key was removed from the map, that is if it was present. */ function remove(UintToAddressMap storage map, uint256 key) internal returns (bool) { - return _remove(map._inner, bytes32(key)); + return remove(map._inner, bytes32(key)); } /** * @dev Returns true if the key is in the map. O(1). */ function contains(UintToAddressMap storage map, uint256 key) internal view returns (bool) { - return _contains(map._inner, bytes32(key)); + return contains(map._inner, bytes32(key)); } /** * @dev Returns the number of elements in the map. O(1). */ function length(UintToAddressMap storage map) internal view returns (uint256) { - return _length(map._inner); + return length(map._inner); } /** @@ -200,7 +201,7 @@ library EnumerableMap { * - `index` must be strictly less than {length}. */ function at(UintToAddressMap storage map, uint256 index) internal view returns (uint256, address) { - (bytes32 key, bytes32 value) = _at(map._inner, index); + (bytes32 key, bytes32 value) = at(map._inner, index); return (uint256(key), address(uint160(uint256(value)))); } @@ -211,7 +212,7 @@ library EnumerableMap { * _Available since v3.4._ */ function tryGet(UintToAddressMap storage map, uint256 key) internal view returns (bool, address) { - (bool success, bytes32 value) = _tryGet(map._inner, bytes32(key)); + (bool success, bytes32 value) = tryGet(map._inner, bytes32(key)); return (success, address(uint160(uint256(value)))); } @@ -223,7 +224,7 @@ library EnumerableMap { * - `key` must be in the map. */ function get(UintToAddressMap storage map, uint256 key) internal view returns (address) { - return address(uint160(uint256(_get(map._inner, bytes32(key))))); + return address(uint160(uint256(get(map._inner, bytes32(key))))); } /** @@ -237,13 +238,13 @@ library EnumerableMap { uint256 key, string memory errorMessage ) internal view returns (address) { - return address(uint160(uint256(_get(map._inner, bytes32(key), errorMessage)))); + return address(uint160(uint256(get(map._inner, bytes32(key), errorMessage)))); } // AddressToUintMap struct AddressToUintMap { - Map _inner; + Bytes32ToBytes32Map _inner; } /** @@ -258,7 +259,7 @@ library EnumerableMap { address key, uint256 value ) internal returns (bool) { - return _set(map._inner, bytes32(uint256(uint160(key))), bytes32(value)); + return set(map._inner, bytes32(uint256(uint160(key))), bytes32(value)); } /** @@ -267,21 +268,21 @@ library EnumerableMap { * Returns true if the key was removed from the map, that is if it was present. */ function remove(AddressToUintMap storage map, address key) internal returns (bool) { - return _remove(map._inner, bytes32(uint256(uint160(key)))); + return remove(map._inner, bytes32(uint256(uint160(key)))); } /** * @dev Returns true if the key is in the map. O(1). */ function contains(AddressToUintMap storage map, address key) internal view returns (bool) { - return _contains(map._inner, bytes32(uint256(uint160(key)))); + return contains(map._inner, bytes32(uint256(uint160(key)))); } /** * @dev Returns the number of elements in the map. O(1). */ function length(AddressToUintMap storage map) internal view returns (uint256) { - return _length(map._inner); + return length(map._inner); } /** @@ -294,7 +295,7 @@ library EnumerableMap { * - `index` must be strictly less than {length}. */ function at(AddressToUintMap storage map, uint256 index) internal view returns (address, uint256) { - (bytes32 key, bytes32 value) = _at(map._inner, index); + (bytes32 key, bytes32 value) = at(map._inner, index); return (address(uint160(uint256(key))), uint256(value)); } @@ -305,7 +306,7 @@ library EnumerableMap { * _Available since v3.4._ */ function tryGet(AddressToUintMap storage map, address key) internal view returns (bool, uint256) { - (bool success, bytes32 value) = _tryGet(map._inner, bytes32(uint256(uint160(key)))); + (bool success, bytes32 value) = tryGet(map._inner, bytes32(uint256(uint160(key)))); return (success, uint256(value)); } @@ -317,6 +318,20 @@ library EnumerableMap { * - `key` must be in the map. */ function get(AddressToUintMap storage map, address key) internal view returns (uint256) { - return uint256(_get(map._inner, bytes32(uint256(uint160(key))))); + return uint256(get(map._inner, bytes32(uint256(uint160(key))))); + } + + /** + * @dev Same as {get}, with a custom error message when `key` is not in the map. + * + * CAUTION: This function is deprecated because it requires allocating memory for the error + * message unnecessarily. For custom revert reasons use {tryGet}. + */ + function get( + AddressToUintMap storage map, + address key, + string memory errorMessage + ) internal view returns (uint256) { + return uint256(get(map._inner, bytes32(uint256(uint160(key))), errorMessage)); } } diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index 8ef3b3b7a62..bd10dcd68ea 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -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; @@ -55,6 +57,7 @@ contract('Governor', function (accounts) { shouldSupportInterfaces([ 'ERC165', + 'ERC1155Receiver', 'Governor', 'GovernorWithParams', ]); @@ -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 }, + ); + }); + }); + }); }); diff --git a/test/governance/TimelockController.test.js b/test/governance/TimelockController.test.js index 84cec6c5794..4584b2252e1 100644 --- a/test/governance/TimelockController.test.js +++ b/test/governance/TimelockController.test.js @@ -1,11 +1,18 @@ -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 { + shouldSupportInterfaces, +} = require('../utils/introspection/SupportsInterface.behavior'); + 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) { @@ -25,7 +32,7 @@ function genOperation (target, value, data, predecessor, salt) { return { id, target, value, data, predecessor, salt }; } -function genOperationBatch (targets, values, datas, predecessor, salt) { +function genOperationBatch (targets, values, payloads, predecessor, salt) { const id = web3.utils.keccak256(web3.eth.abi.encodeParameters([ 'address[]', 'uint256[]', @@ -35,11 +42,11 @@ function genOperationBatch (targets, values, datas, predecessor, salt) { ], [ targets, values, - datas, + payloads, predecessor, salt, ])); - return { id, targets, values, datas, predecessor, salt }; + return { id, targets, values, payloads, predecessor, salt }; } contract('TimelockController', function (accounts) { @@ -52,40 +59,44 @@ contract('TimelockController', function (accounts) { beforeEach(async function () { // Deploy new timelock - this.timelock = await TimelockController.new( + this.mock = await TimelockController.new( MINDELAY, [ proposer ], [ executor ], { from: admin }, ); - expect(await this.timelock.hasRole(CANCELLER_ROLE, proposer)).to.be.equal(true); - await this.timelock.revokeRole(CANCELLER_ROLE, proposer, { from: admin }); - await this.timelock.grantRole(CANCELLER_ROLE, canceller, { from: admin }); + expect(await this.mock.hasRole(CANCELLER_ROLE, proposer)).to.be.equal(true); + await this.mock.revokeRole(CANCELLER_ROLE, proposer, { from: admin }); + await this.mock.grantRole(CANCELLER_ROLE, canceller, { from: admin }); // Mocks this.callreceivermock = await CallReceiverMock.new({ from: admin }); this.implementation2 = await Implementation2.new({ from: admin }); }); + shouldSupportInterfaces([ + 'ERC1155Receiver', + ]); + it('initial state', async function () { - expect(await this.timelock.getMinDelay()).to.be.bignumber.equal(MINDELAY); + expect(await this.mock.getMinDelay()).to.be.bignumber.equal(MINDELAY); - expect(await this.timelock.TIMELOCK_ADMIN_ROLE()).to.be.equal(TIMELOCK_ADMIN_ROLE); - expect(await this.timelock.PROPOSER_ROLE()).to.be.equal(PROPOSER_ROLE); - expect(await this.timelock.EXECUTOR_ROLE()).to.be.equal(EXECUTOR_ROLE); - expect(await this.timelock.CANCELLER_ROLE()).to.be.equal(CANCELLER_ROLE); + expect(await this.mock.TIMELOCK_ADMIN_ROLE()).to.be.equal(TIMELOCK_ADMIN_ROLE); + expect(await this.mock.PROPOSER_ROLE()).to.be.equal(PROPOSER_ROLE); + expect(await this.mock.EXECUTOR_ROLE()).to.be.equal(EXECUTOR_ROLE); + expect(await this.mock.CANCELLER_ROLE()).to.be.equal(CANCELLER_ROLE); expect(await Promise.all([ PROPOSER_ROLE, CANCELLER_ROLE, EXECUTOR_ROLE ].map(role => - this.timelock.hasRole(role, proposer), + this.mock.hasRole(role, proposer), ))).to.be.deep.equal([ true, false, false ]); expect(await Promise.all([ PROPOSER_ROLE, CANCELLER_ROLE, EXECUTOR_ROLE ].map(role => - this.timelock.hasRole(role, canceller), + this.mock.hasRole(role, canceller), ))).to.be.deep.equal([ false, true, false ]); expect(await Promise.all([ PROPOSER_ROLE, CANCELLER_ROLE, EXECUTOR_ROLE ].map(role => - this.timelock.hasRole(role, executor), + this.mock.hasRole(role, executor), ))).to.be.deep.equal([ false, false, true ]); }); @@ -99,7 +110,7 @@ contract('TimelockController', function (accounts) { '0xba41db3be0a9929145cfe480bd0f1f003689104d275ae912099f925df424ef94', '0x60d9109846ab510ed75c15f979ae366a8a2ace11d34ba9788c13ac296db50e6e', ); - expect(await this.timelock.hashOperation( + expect(await this.mock.hashOperation( this.operation.target, this.operation.value, this.operation.data, @@ -116,10 +127,10 @@ contract('TimelockController', function (accounts) { '0xce8f45069cc71d25f71ba05062de1a3974f9849b004de64a70998bca9d29c2e7', '0x8952d74c110f72bfe5accdf828c74d53a7dfb71235dfa8a1e8c75d8576b372ff', ); - expect(await this.timelock.hashOperationBatch( + expect(await this.mock.hashOperationBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, )).to.be.equal(this.operation.id); @@ -138,7 +149,7 @@ contract('TimelockController', function (accounts) { }); it('proposer can schedule', async function () { - const receipt = await this.timelock.schedule( + const receipt = await this.mock.schedule( this.operation.target, this.operation.value, this.operation.data, @@ -159,12 +170,12 @@ contract('TimelockController', function (accounts) { const block = await web3.eth.getBlock(receipt.receipt.blockHash); - expect(await this.timelock.getTimestamp(this.operation.id)) + expect(await this.mock.getTimestamp(this.operation.id)) .to.be.bignumber.equal(web3.utils.toBN(block.timestamp).add(MINDELAY)); }); - it('prevent overwritting active operation', async function () { - await this.timelock.schedule( + it('prevent overwriting active operation', async function () { + await this.mock.schedule( this.operation.target, this.operation.value, this.operation.data, @@ -175,7 +186,7 @@ contract('TimelockController', function (accounts) { ); await expectRevert( - this.timelock.schedule( + this.mock.schedule( this.operation.target, this.operation.value, this.operation.data, @@ -190,7 +201,7 @@ contract('TimelockController', function (accounts) { it('prevent non-proposer from commiting', async function () { await expectRevert( - this.timelock.schedule( + this.mock.schedule( this.operation.target, this.operation.value, this.operation.data, @@ -205,7 +216,7 @@ contract('TimelockController', function (accounts) { it('enforce minimum delay', async function () { await expectRevert( - this.timelock.schedule( + this.mock.schedule( this.operation.target, this.operation.value, this.operation.data, @@ -232,7 +243,7 @@ contract('TimelockController', function (accounts) { it('revert if operation is not scheduled', async function () { await expectRevert( - this.timelock.execute( + this.mock.execute( this.operation.target, this.operation.value, this.operation.data, @@ -246,7 +257,7 @@ contract('TimelockController', function (accounts) { describe('with scheduled operation', function () { beforeEach(async function () { - ({ receipt: this.receipt, logs: this.logs } = await this.timelock.schedule( + ({ receipt: this.receipt, logs: this.logs } = await this.mock.schedule( this.operation.target, this.operation.value, this.operation.data, @@ -259,7 +270,7 @@ contract('TimelockController', function (accounts) { it('revert if execution comes too early 1/2', async function () { await expectRevert( - this.timelock.execute( + this.mock.execute( this.operation.target, this.operation.value, this.operation.data, @@ -272,11 +283,11 @@ contract('TimelockController', function (accounts) { }); it('revert if execution comes too early 2/2', async function () { - const timestamp = await this.timelock.getTimestamp(this.operation.id); + const timestamp = await this.mock.getTimestamp(this.operation.id); await time.increaseTo(timestamp - 5); // -1 is too tight, test sometime fails await expectRevert( - this.timelock.execute( + this.mock.execute( this.operation.target, this.operation.value, this.operation.data, @@ -290,12 +301,12 @@ contract('TimelockController', function (accounts) { describe('on time', function () { beforeEach(async function () { - const timestamp = await this.timelock.getTimestamp(this.operation.id); + const timestamp = await this.mock.getTimestamp(this.operation.id); await time.increaseTo(timestamp); }); it('executor can reveal', async function () { - const receipt = await this.timelock.execute( + const receipt = await this.mock.execute( this.operation.target, this.operation.value, this.operation.data, @@ -314,7 +325,7 @@ contract('TimelockController', function (accounts) { it('prevent non-executor from revealing', async function () { await expectRevert( - this.timelock.execute( + this.mock.execute( this.operation.target, this.operation.value, this.operation.data, @@ -343,10 +354,10 @@ contract('TimelockController', function (accounts) { }); it('proposer can schedule', async function () { - const receipt = await this.timelock.scheduleBatch( + const receipt = await this.mock.scheduleBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, MINDELAY, @@ -358,7 +369,7 @@ contract('TimelockController', function (accounts) { index: web3.utils.toBN(i), target: this.operation.targets[i], value: web3.utils.toBN(this.operation.values[i]), - data: this.operation.datas[i], + data: this.operation.payloads[i], predecessor: this.operation.predecessor, delay: MINDELAY, }); @@ -366,15 +377,15 @@ contract('TimelockController', function (accounts) { const block = await web3.eth.getBlock(receipt.receipt.blockHash); - expect(await this.timelock.getTimestamp(this.operation.id)) + expect(await this.mock.getTimestamp(this.operation.id)) .to.be.bignumber.equal(web3.utils.toBN(block.timestamp).add(MINDELAY)); }); - it('prevent overwritting active operation', async function () { - await this.timelock.scheduleBatch( + it('prevent overwriting active operation', async function () { + await this.mock.scheduleBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, MINDELAY, @@ -382,10 +393,10 @@ contract('TimelockController', function (accounts) { ); await expectRevert( - this.timelock.scheduleBatch( + this.mock.scheduleBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, MINDELAY, @@ -397,10 +408,10 @@ contract('TimelockController', function (accounts) { it('length of batch parameter must match #1', async function () { await expectRevert( - this.timelock.scheduleBatch( + this.mock.scheduleBatch( this.operation.targets, [], - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, MINDELAY, @@ -412,7 +423,7 @@ contract('TimelockController', function (accounts) { it('length of batch parameter must match #1', async function () { await expectRevert( - this.timelock.scheduleBatch( + this.mock.scheduleBatch( this.operation.targets, this.operation.values, [], @@ -427,10 +438,10 @@ contract('TimelockController', function (accounts) { it('prevent non-proposer from commiting', async function () { await expectRevert( - this.timelock.scheduleBatch( + this.mock.scheduleBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, MINDELAY, @@ -442,10 +453,10 @@ contract('TimelockController', function (accounts) { it('enforce minimum delay', async function () { await expectRevert( - this.timelock.scheduleBatch( + this.mock.scheduleBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, MINDELAY - 1, @@ -469,10 +480,10 @@ contract('TimelockController', function (accounts) { it('revert if operation is not scheduled', async function () { await expectRevert( - this.timelock.executeBatch( + this.mock.executeBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, { from: executor }, @@ -483,10 +494,10 @@ contract('TimelockController', function (accounts) { describe('with scheduled operation', function () { beforeEach(async function () { - ({ receipt: this.receipt, logs: this.logs } = await this.timelock.scheduleBatch( + ({ receipt: this.receipt, logs: this.logs } = await this.mock.scheduleBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, MINDELAY, @@ -496,10 +507,10 @@ contract('TimelockController', function (accounts) { it('revert if execution comes too early 1/2', async function () { await expectRevert( - this.timelock.executeBatch( + this.mock.executeBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, { from: executor }, @@ -509,14 +520,14 @@ contract('TimelockController', function (accounts) { }); it('revert if execution comes too early 2/2', async function () { - const timestamp = await this.timelock.getTimestamp(this.operation.id); + const timestamp = await this.mock.getTimestamp(this.operation.id); await time.increaseTo(timestamp - 5); // -1 is to tight, test sometime fails await expectRevert( - this.timelock.executeBatch( + this.mock.executeBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, { from: executor }, @@ -527,15 +538,15 @@ contract('TimelockController', function (accounts) { describe('on time', function () { beforeEach(async function () { - const timestamp = await this.timelock.getTimestamp(this.operation.id); + const timestamp = await this.mock.getTimestamp(this.operation.id); await time.increaseTo(timestamp); }); it('executor can reveal', async function () { - const receipt = await this.timelock.executeBatch( + const receipt = await this.mock.executeBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, { from: executor }, @@ -546,17 +557,17 @@ contract('TimelockController', function (accounts) { index: web3.utils.toBN(i), target: this.operation.targets[i], value: web3.utils.toBN(this.operation.values[i]), - data: this.operation.datas[i], + data: this.operation.payloads[i], }); } }); it('prevent non-executor from revealing', async function () { await expectRevert( - this.timelock.executeBatch( + this.mock.executeBatch( this.operation.targets, this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, { from: other }, @@ -567,10 +578,10 @@ contract('TimelockController', function (accounts) { it('length mismatch #1', async function () { await expectRevert( - this.timelock.executeBatch( + this.mock.executeBatch( [], this.operation.values, - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, { from: executor }, @@ -581,10 +592,10 @@ contract('TimelockController', function (accounts) { it('length mismatch #2', async function () { await expectRevert( - this.timelock.executeBatch( + this.mock.executeBatch( this.operation.targets, [], - this.operation.datas, + this.operation.payloads, this.operation.predecessor, this.operation.salt, { from: executor }, @@ -595,7 +606,7 @@ contract('TimelockController', function (accounts) { it('length mismatch #3', async function () { await expectRevert( - this.timelock.executeBatch( + this.mock.executeBatch( this.operation.targets, this.operation.values, [], @@ -630,10 +641,10 @@ contract('TimelockController', function (accounts) { '0x8ac04aa0d6d66b8812fb41d39638d37af0a9ab11da507afd65c509f8ed079d3e', ); - await this.timelock.scheduleBatch( + await this.mock.scheduleBatch( operation.targets, operation.values, - operation.datas, + operation.payloads, operation.predecessor, operation.salt, MINDELAY, @@ -641,10 +652,10 @@ contract('TimelockController', function (accounts) { ); await time.increase(MINDELAY); await expectRevert( - this.timelock.executeBatch( + this.mock.executeBatch( operation.targets, operation.values, - operation.datas, + operation.payloads, operation.predecessor, operation.salt, { from: executor }, @@ -664,7 +675,7 @@ contract('TimelockController', function (accounts) { ZERO_BYTES32, '0xa2485763600634800df9fc9646fb2c112cf98649c55f63dd1d9c7d13a64399d9', ); - ({ receipt: this.receipt, logs: this.logs } = await this.timelock.schedule( + ({ receipt: this.receipt, logs: this.logs } = await this.mock.schedule( this.operation.target, this.operation.value, this.operation.data, @@ -676,20 +687,20 @@ contract('TimelockController', function (accounts) { }); it('canceller can cancel', async function () { - const receipt = await this.timelock.cancel(this.operation.id, { from: canceller }); + const receipt = await this.mock.cancel(this.operation.id, { from: canceller }); expectEvent(receipt, 'Cancelled', { id: this.operation.id }); }); it('cannot cancel invalid operation', async function () { await expectRevert( - this.timelock.cancel(constants.ZERO_BYTES32, { from: canceller }), + this.mock.cancel(constants.ZERO_BYTES32, { from: canceller }), 'TimelockController: operation cannot be cancelled', ); }); it('prevent non-canceller from canceling', async function () { await expectRevert( - this.timelock.cancel(this.operation.id, { from: other }), + this.mock.cancel(this.operation.id, { from: other }), `AccessControl: account ${other.toLowerCase()} is missing role ${CANCELLER_ROLE}`, ); }); @@ -699,7 +710,7 @@ contract('TimelockController', function (accounts) { describe('maintenance', function () { it('prevent unauthorized maintenance', async function () { await expectRevert( - this.timelock.updateDelay(0, { from: other }), + this.mock.updateDelay(0, { from: other }), 'TimelockController: caller must be timelock', ); }); @@ -707,14 +718,14 @@ contract('TimelockController', function (accounts) { it('timelock scheduled maintenance', async function () { const newDelay = time.duration.hours(6); const operation = genOperation( - this.timelock.address, + this.mock.address, 0, - this.timelock.contract.methods.updateDelay(newDelay.toString()).encodeABI(), + this.mock.contract.methods.updateDelay(newDelay.toString()).encodeABI(), ZERO_BYTES32, '0xf8e775b2c5f4d66fb5c7fa800f35ef518c262b6014b3c0aee6ea21bff157f108', ); - await this.timelock.schedule( + await this.mock.schedule( operation.target, operation.value, operation.data, @@ -724,7 +735,7 @@ contract('TimelockController', function (accounts) { { from: proposer }, ); await time.increase(MINDELAY); - const receipt = await this.timelock.execute( + const receipt = await this.mock.execute( operation.target, operation.value, operation.data, @@ -734,7 +745,7 @@ contract('TimelockController', function (accounts) { ); expectEvent(receipt, 'MinDelayChange', { newDuration: newDelay.toString(), oldDuration: MINDELAY }); - expect(await this.timelock.getMinDelay()).to.be.bignumber.equal(newDelay); + expect(await this.mock.getMinDelay()).to.be.bignumber.equal(newDelay); }); }); @@ -754,7 +765,7 @@ contract('TimelockController', function (accounts) { this.operation1.id, '0x036e1311cac523f9548e6461e29fb1f8f9196b91910a41711ea22f5de48df07d', ); - await this.timelock.schedule( + await this.mock.schedule( this.operation1.target, this.operation1.value, this.operation1.data, @@ -763,7 +774,7 @@ contract('TimelockController', function (accounts) { MINDELAY, { from: proposer }, ); - await this.timelock.schedule( + await this.mock.schedule( this.operation2.target, this.operation2.value, this.operation2.data, @@ -777,7 +788,7 @@ contract('TimelockController', function (accounts) { it('cannot execute before dependency', async function () { await expectRevert( - this.timelock.execute( + this.mock.execute( this.operation2.target, this.operation2.value, this.operation2.data, @@ -790,7 +801,7 @@ contract('TimelockController', function (accounts) { }); it('can execute after dependency', async function () { - await this.timelock.execute( + await this.mock.execute( this.operation1.target, this.operation1.value, this.operation1.data, @@ -798,7 +809,7 @@ contract('TimelockController', function (accounts) { this.operation1.salt, { from: executor }, ); - await this.timelock.execute( + await this.mock.execute( this.operation2.target, this.operation2.value, this.operation2.data, @@ -821,7 +832,7 @@ contract('TimelockController', function (accounts) { '0x8043596363daefc89977b25f9d9b4d06c3910959ef0c4d213557a903e1b555e2', ); - await this.timelock.schedule( + await this.mock.schedule( operation.target, operation.value, operation.data, @@ -831,7 +842,7 @@ contract('TimelockController', function (accounts) { { from: proposer }, ); await time.increase(MINDELAY); - await this.timelock.execute( + await this.mock.execute( operation.target, operation.value, operation.data, @@ -852,7 +863,7 @@ contract('TimelockController', function (accounts) { '0xb1b1b276fdf1a28d1e00537ea73b04d56639128b08063c1a2f70a52e38cba693', ); - await this.timelock.schedule( + await this.mock.schedule( operation.target, operation.value, operation.data, @@ -863,7 +874,7 @@ contract('TimelockController', function (accounts) { ); await time.increase(MINDELAY); await expectRevert( - this.timelock.execute( + this.mock.execute( operation.target, operation.value, operation.data, @@ -884,7 +895,7 @@ contract('TimelockController', function (accounts) { '0xe5ca79f295fc8327ee8a765fe19afb58f4a0cbc5053642bfdd7e73bc68e0fc67', ); - await this.timelock.schedule( + await this.mock.schedule( operation.target, operation.value, operation.data, @@ -895,7 +906,7 @@ contract('TimelockController', function (accounts) { ); await time.increase(MINDELAY); await expectRevert( - this.timelock.execute( + this.mock.execute( operation.target, operation.value, operation.data, @@ -916,7 +927,7 @@ contract('TimelockController', function (accounts) { '0xf3274ce7c394c5b629d5215723563a744b817e1730cca5587c567099a14578fd', ); - await this.timelock.schedule( + await this.mock.schedule( operation.target, operation.value, operation.data, @@ -927,7 +938,7 @@ contract('TimelockController', function (accounts) { ); await time.increase(MINDELAY); await expectRevert( - this.timelock.execute( + this.mock.execute( operation.target, operation.value, operation.data, @@ -948,7 +959,7 @@ contract('TimelockController', function (accounts) { '0x5ab73cd33477dcd36c1e05e28362719d0ed59a7b9ff14939de63a43073dc1f44', ); - await this.timelock.schedule( + await this.mock.schedule( operation.target, operation.value, operation.data, @@ -959,10 +970,10 @@ contract('TimelockController', function (accounts) { ); await time.increase(MINDELAY); - expect(await web3.eth.getBalance(this.timelock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); + expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); expect(await web3.eth.getBalance(this.callreceivermock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); - await this.timelock.execute( + await this.mock.execute( operation.target, operation.value, operation.data, @@ -971,7 +982,7 @@ contract('TimelockController', function (accounts) { { from: executor, value: 1 }, ); - expect(await web3.eth.getBalance(this.timelock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); + expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); expect(await web3.eth.getBalance(this.callreceivermock.address)).to.be.bignumber.equal(web3.utils.toBN(1)); }); @@ -984,7 +995,7 @@ contract('TimelockController', function (accounts) { '0xb78edbd920c7867f187e5aa6294ae5a656cfbf0dea1ccdca3751b740d0f2bdf8', ); - await this.timelock.schedule( + await this.mock.schedule( operation.target, operation.value, operation.data, @@ -995,11 +1006,11 @@ contract('TimelockController', function (accounts) { ); await time.increase(MINDELAY); - expect(await web3.eth.getBalance(this.timelock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); + expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); expect(await web3.eth.getBalance(this.callreceivermock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); await expectRevert( - this.timelock.execute( + this.mock.execute( operation.target, operation.value, operation.data, @@ -1010,7 +1021,7 @@ contract('TimelockController', function (accounts) { 'TimelockController: underlying transaction reverted', ); - expect(await web3.eth.getBalance(this.timelock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); + expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); expect(await web3.eth.getBalance(this.callreceivermock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); }); @@ -1023,7 +1034,7 @@ contract('TimelockController', function (accounts) { '0xdedb4563ef0095db01d81d3f2decf57cf83e4a72aa792af14c43a792b56f4de6', ); - await this.timelock.schedule( + await this.mock.schedule( operation.target, operation.value, operation.data, @@ -1034,11 +1045,11 @@ contract('TimelockController', function (accounts) { ); await time.increase(MINDELAY); - expect(await web3.eth.getBalance(this.timelock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); + expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); expect(await web3.eth.getBalance(this.callreceivermock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); await expectRevert( - this.timelock.execute( + this.mock.execute( operation.target, operation.value, operation.data, @@ -1049,8 +1060,60 @@ contract('TimelockController', function (accounts) { 'TimelockController: underlying transaction reverted', ); - expect(await web3.eth.getBalance(this.timelock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); + expect(await web3.eth.getBalance(this.mock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); expect(await web3.eth.getBalance(this.callreceivermock.address)).to.be.bignumber.equal(web3.utils.toBN(0)); }); }); + + 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(other, tokenId); + }); + + it('can receive an ERC721 safeTransfer', async function () { + await this.token.safeTransferFrom(other, this.mock.address, tokenId, { from: other }); + }); + }); + + 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(other, Object.keys(tokenIds), Object.values(tokenIds), '0x'); + }); + + it('can receive ERC1155 safeTransfer', async function () { + await this.token.safeTransferFrom( + other, + this.mock.address, + ...Object.entries(tokenIds)[0], // id + amount + '0x', + { from: other }, + ); + }); + + it('can receive ERC1155 safeBatchTransfer', async function () { + await this.token.safeBatchTransferFrom( + other, + this.mock.address, + Object.keys(tokenIds), + Object.values(tokenIds), + '0x', + { from: other }, + ); + }); + }); + }); }); diff --git a/test/governance/compatibility/GovernorCompatibilityBravo.test.js b/test/governance/compatibility/GovernorCompatibilityBravo.test.js index f585d88f3b9..5f12e9fac15 100644 --- a/test/governance/compatibility/GovernorCompatibilityBravo.test.js +++ b/test/governance/compatibility/GovernorCompatibilityBravo.test.js @@ -215,7 +215,7 @@ contract('GovernorCompatibilityBravo', function (accounts) { describe('should revert', function () { describe('on propose', function () { - it('if proposal doesnt meet proposalThreshold', async function () { + it('if proposal does not meet proposalThreshold', async function () { await expectRevert( this.helper.propose({ from: other }), 'Governor: proposer votes below proposal threshold', diff --git a/test/helpers/governance.js b/test/helpers/governance.js index accbf79ac22..c56ec4c89be 100644 --- a/test/helpers/governance.js +++ b/test/helpers/governance.js @@ -106,13 +106,13 @@ class GovernorHelper { opts, ))) : vote.params - // otherwize if params + // otherwise if params ? this.governor.castVoteWithReasonAndParams(...concatOpts( [ proposal.id, vote.support, vote.reason || '', vote.params ], opts, )) : vote.reason - // otherwize if reason + // otherwise if reason ? this.governor.castVoteWithReason(...concatOpts( [ proposal.id, vote.support, vote.reason ], opts, @@ -165,7 +165,7 @@ class GovernorHelper { const descriptionHash = web3.utils.keccak256(description); - // condensed version for queing end executing + // condensed version for queueing end executing const shortProposal = [ targets, values, diff --git a/test/proxy/utils/Initializable.test.js b/test/proxy/utils/Initializable.test.js index 04884a1d45e..1efb728504a 100644 --- a/test/proxy/utils/Initializable.test.js +++ b/test/proxy/utils/Initializable.test.js @@ -1,8 +1,9 @@ const { expectRevert } = require('@openzeppelin/test-helpers'); -const { assert } = require('chai'); +const { expect } = require('chai'); const InitializableMock = artifacts.require('InitializableMock'); const ConstructorInitializableMock = artifacts.require('ConstructorInitializableMock'); +const ReinitializerMock = artifacts.require('ReinitializerMock'); const SampleChild = artifacts.require('SampleChild'); contract('Initializable', function (accounts) { @@ -13,7 +14,7 @@ contract('Initializable', function (accounts) { context('before initialize', function () { it('initializer has not run', async function () { - assert.isFalse(await this.contract.initializerRan()); + expect(await this.contract.initializerRan()).to.equal(false); }); }); @@ -23,7 +24,7 @@ contract('Initializable', function (accounts) { }); it('initializer has run', async function () { - assert.isTrue(await this.contract.initializerRan()); + expect(await this.contract.initializerRan()).to.equal(true); }); it('initializer does not run again', async function () { @@ -38,7 +39,7 @@ contract('Initializable', function (accounts) { it('onlyInitializing modifier succeeds', async function () { await this.contract.onlyInitializingNested(); - assert.isTrue(await this.contract.onlyInitializingRan()); + expect(await this.contract.onlyInitializingRan()).to.equal(true); }); }); @@ -49,15 +50,69 @@ contract('Initializable', function (accounts) { it('nested initializer can run during construction', async function () { const contract2 = await ConstructorInitializableMock.new(); - assert.isTrue(await contract2.initializerRan()); - assert.isTrue(await contract2.onlyInitializingRan()); + expect(await contract2.initializerRan()).to.equal(true); + expect(await contract2.onlyInitializingRan()).to.equal(true); + }); + + describe('reinitialization', function () { + beforeEach('deploying', async function () { + this.contract = await ReinitializerMock.new(); + }); + + it('can reinitialize', async function () { + expect(await this.contract.counter()).to.be.bignumber.equal('0'); + await this.contract.initialize(); + expect(await this.contract.counter()).to.be.bignumber.equal('1'); + await this.contract.reinitialize(2); + expect(await this.contract.counter()).to.be.bignumber.equal('2'); + await this.contract.reinitialize(3); + expect(await this.contract.counter()).to.be.bignumber.equal('3'); + }); + + it('can jump multiple steps', async function () { + expect(await this.contract.counter()).to.be.bignumber.equal('0'); + await this.contract.initialize(); + expect(await this.contract.counter()).to.be.bignumber.equal('1'); + await this.contract.reinitialize(128); + expect(await this.contract.counter()).to.be.bignumber.equal('2'); + }); + + it('cannot nest reinitializers', async function () { + expect(await this.contract.counter()).to.be.bignumber.equal('0'); + await expectRevert(this.contract.nestedReinitialize(2, 3), 'Initializable: contract is already initialized'); + await expectRevert(this.contract.nestedReinitialize(3, 2), 'Initializable: contract is already initialized'); + }); + + it('can chain reinitializers', async function () { + expect(await this.contract.counter()).to.be.bignumber.equal('0'); + await this.contract.chainReinitialize(2, 3); + expect(await this.contract.counter()).to.be.bignumber.equal('2'); + }); + + describe('contract locking', function () { + it('prevents initialization', async function () { + await this.contract.disableInitializers(); + await expectRevert(this.contract.initialize(), 'Initializable: contract is already initialized'); + }); + + it('prevents re-initialization', async function () { + await this.contract.disableInitializers(); + await expectRevert(this.contract.reinitialize(255), 'Initializable: contract is already initialized'); + }); + + it('can lock contract after initialization', async function () { + await this.contract.initialize(); + await this.contract.disableInitializers(); + await expectRevert(this.contract.reinitialize(255), 'Initializable: contract is already initialized'); + }); + }); }); describe('complex testing with inheritance', function () { - const mother = 12; + const mother = '12'; const gramps = '56'; - const father = 34; - const child = 78; + const father = '34'; + const child = '78'; beforeEach('deploying', async function () { this.contract = await SampleChild.new(); @@ -68,23 +123,23 @@ contract('Initializable', function (accounts) { }); it('initializes human', async function () { - assert.equal(await this.contract.isHuman(), true); + expect(await this.contract.isHuman()).to.be.equal(true); }); it('initializes mother', async function () { - assert.equal(await this.contract.mother(), mother); + expect(await this.contract.mother()).to.be.bignumber.equal(mother); }); it('initializes gramps', async function () { - assert.equal(await this.contract.gramps(), gramps); + expect(await this.contract.gramps()).to.be.bignumber.equal(gramps); }); it('initializes father', async function () { - assert.equal(await this.contract.father(), father); + expect(await this.contract.father()).to.be.bignumber.equal(father); }); it('initializes child', async function () { - assert.equal(await this.contract.child(), child); + expect(await this.contract.child()).to.be.bignumber.equal(child); }); }); }); diff --git a/test/token/ERC20/extensions/ERC20Wrapper.test.js b/test/token/ERC20/extensions/ERC20Wrapper.test.js index ec074e1b8cf..ceb813e08d8 100644 --- a/test/token/ERC20/extensions/ERC20Wrapper.test.js +++ b/test/token/ERC20/extensions/ERC20Wrapper.test.js @@ -4,7 +4,8 @@ const { ZERO_ADDRESS, MAX_UINT256 } = constants; const { shouldBehaveLikeERC20 } = require('../ERC20.behavior'); -const ERC20Mock = artifacts.require('ERC20Mock'); +const NotAnERC20 = artifacts.require('CallReceiverMock'); +const ERC20Mock = artifacts.require('ERC20DecimalsMock'); const ERC20WrapperMock = artifacts.require('ERC20WrapperMock'); contract('ERC20', function (accounts) { @@ -16,8 +17,10 @@ contract('ERC20', function (accounts) { const initialSupply = new BN(100); beforeEach(async function () { - this.underlying = await ERC20Mock.new(name, symbol, initialHolder, initialSupply); + this.underlying = await ERC20Mock.new(name, symbol, 9); this.token = await ERC20WrapperMock.new(this.underlying.address, `Wrapped ${name}`, `W${symbol}`); + + await this.underlying.mint(initialHolder, initialSupply); }); afterEach(async function () { @@ -32,8 +35,14 @@ contract('ERC20', function (accounts) { expect(await this.token.symbol()).to.equal(`W${symbol}`); }); - it('has 18 decimals', async function () { - expect(await this.token.decimals()).to.be.bignumber.equal('18'); + it('has the same decimals as the underlying token', async function () { + expect(await this.token.decimals()).to.be.bignumber.equal('9'); + }); + + it('decimals default back to 18 if token has no metadata', async function () { + const noDecimals = await NotAnERC20.new(); + const otherToken = await ERC20WrapperMock.new(noDecimals.address, `Wrapped ${name}`, `W${symbol}`); + expect(await otherToken.decimals()).to.be.bignumber.equal('18'); }); it('has underlying', async function () { diff --git a/test/utils/cryptography/ECDSA.test.js b/test/utils/cryptography/ECDSA.test.js index d153996699b..fff3e2c30d7 100644 --- a/test/utils/cryptography/ECDSA.test.js +++ b/test/utils/cryptography/ECDSA.test.js @@ -47,7 +47,7 @@ function split (signature) { web3.utils.bytesToHex(raw.slice(32, 64)), // s ]; default: - expect.fail('Invalid siganture length, cannot split'); + expect.fail('Invalid signature length, cannot split'); } } diff --git a/test/utils/structs/DoubleEndedQueue.test.js b/test/utils/structs/DoubleEndedQueue.test.js index e10bf64e1ab..d1de09c33ea 100644 --- a/test/utils/structs/DoubleEndedQueue.test.js +++ b/test/utils/structs/DoubleEndedQueue.test.js @@ -69,7 +69,7 @@ contract('DoubleEndedQueue', function (accounts) { describe('push', function () { it('front', async function () { await this.deque.pushFront(bytesD); - this.content.unshift(bytesD); // add element at the begining + this.content.unshift(bytesD); // add element at the beginning expect(await getContent(this.deque)).to.have.ordered.members(this.content); }); diff --git a/test/utils/structs/EnumerableMap.behavior.js b/test/utils/structs/EnumerableMap.behavior.js new file mode 100644 index 00000000000..b1d0d0dceb0 --- /dev/null +++ b/test/utils/structs/EnumerableMap.behavior.js @@ -0,0 +1,181 @@ +const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); + +const zip = require('lodash.zip'); + +function shouldBehaveLikeMap (keys, values, zeroValue) { + const [keyA, keyB, keyC] = keys; + const [valueA, valueB, valueC] = values; + + async function expectMembersMatch (map, keys, values) { + expect(keys.length).to.equal(values.length); + + await Promise.all(keys.map(async key => + expect(await map.contains(key)).to.equal(true), + )); + + expect(await map.length()).to.bignumber.equal(keys.length.toString()); + + expect( + (await Promise.all(keys.map(key => map.get(key)))).map(k => k.toString()), + ).to.have.same.members( + values.map(value => value.toString()), + ); + + // To compare key-value pairs, we zip keys and values, and convert BNs to + // strings to workaround Chai limitations when dealing with nested arrays + expect(await Promise.all([...Array(keys.length).keys()].map(async (index) => { + const entry = await map.at(index); + return [entry.key.toString(), entry.value.toString()]; + }))).to.have.same.deep.members( + zip(keys.map(k => k.toString()), values.map(v => v.toString())), + ); + } + + it('starts empty', async function () { + expect(await this.map.contains(keyA)).to.equal(false); + + await expectMembersMatch(this.map, [], []); + }); + + describe('set', function () { + it('adds a key', async function () { + const receipt = await this.map.set(keyA, valueA); + expectEvent(receipt, 'OperationResult', { result: true }); + + await expectMembersMatch(this.map, [keyA], [valueA]); + }); + + it('adds several keys', async function () { + await this.map.set(keyA, valueA); + await this.map.set(keyB, valueB); + + await expectMembersMatch(this.map, [keyA, keyB], [valueA, valueB]); + expect(await this.map.contains(keyC)).to.equal(false); + }); + + it('returns false when adding keys already in the set', async function () { + await this.map.set(keyA, valueA); + + const receipt = (await this.map.set(keyA, valueA)); + expectEvent(receipt, 'OperationResult', { result: false }); + + await expectMembersMatch(this.map, [keyA], [valueA]); + }); + + it('updates values for keys already in the set', async function () { + await this.map.set(keyA, valueA); + + await this.map.set(keyA, valueB); + + await expectMembersMatch(this.map, [keyA], [valueB]); + }); + }); + + describe('remove', function () { + it('removes added keys', async function () { + await this.map.set(keyA, valueA); + + const receipt = await this.map.remove(keyA); + expectEvent(receipt, 'OperationResult', { result: true }); + + expect(await this.map.contains(keyA)).to.equal(false); + await expectMembersMatch(this.map, [], []); + }); + + it('returns false when removing keys not in the set', async function () { + const receipt = await this.map.remove(keyA); + expectEvent(receipt, 'OperationResult', { result: false }); + + expect(await this.map.contains(keyA)).to.equal(false); + }); + + it('adds and removes multiple keys', async function () { + // [] + + await this.map.set(keyA, valueA); + await this.map.set(keyC, valueC); + + // [A, C] + + await this.map.remove(keyA); + await this.map.remove(keyB); + + // [C] + + await this.map.set(keyB, valueB); + + // [C, B] + + await this.map.set(keyA, valueA); + await this.map.remove(keyC); + + // [A, B] + + await this.map.set(keyA, valueA); + await this.map.set(keyB, valueB); + + // [A, B] + + await this.map.set(keyC, valueC); + await this.map.remove(keyA); + + // [B, C] + + await this.map.set(keyA, valueA); + await this.map.remove(keyB); + + // [A, C] + + await expectMembersMatch(this.map, [keyA, keyC], [valueA, valueC]); + + expect(await this.map.contains(keyB)).to.equal(false); + }); + }); + + describe('read', function () { + beforeEach(async function () { + await this.map.set(keyA, valueA); + }); + + describe('get', function () { + it('existing value', async function () { + expect( + (await this.map.get(keyA)).toString(), + ).to.be.equal(valueA.toString()); + }); + it('missing value', async function () { + await expectRevert(this.map.get(keyB), 'EnumerableMap: nonexistent key'); + }); + }); + + describe('get with message', function () { + it('existing value', async function () { + expect( + (await this.map.getWithMessage(keyA, 'custom error string')) + .toString(), + ).to.be.equal(valueA.toString()); + }); + it('missing value', async function () { + await expectRevert(this.map.getWithMessage(keyB, 'custom error string'), 'custom error string'); + }); + }); + + describe('tryGet', function () { + it('existing value', async function () { + const result = await this.map.tryGet(keyA); + expect(result['0']).to.be.equal(true); + expect(result['1'].toString()).to.be.equal(valueA.toString()); + }); + it('missing value', async function () { + const result = await this.map.tryGet(keyB); + expect(result['0']).to.be.equal(false); + expect(result['1'].toString()).to.be.equal(zeroValue.toString()); + }); + }); + }); +} + +module.exports = { + shouldBehaveLikeMap, +}; diff --git a/test/utils/structs/EnumerableMap.test.js b/test/utils/structs/EnumerableMap.test.js new file mode 100644 index 00000000000..866ff64397a --- /dev/null +++ b/test/utils/structs/EnumerableMap.test.js @@ -0,0 +1,58 @@ +const { BN, constants } = require('@openzeppelin/test-helpers'); + +const AddressToUintMapMock = artifacts.require('AddressToUintMapMock'); +const UintToAddressMapMock = artifacts.require('UintToAddressMapMock'); +const Bytes32ToBytes32MapMock = artifacts.require('Bytes32ToBytes32MapMock'); + +const { shouldBehaveLikeMap } = require('./EnumerableMap.behavior'); + +contract('EnumerableMap', function (accounts) { + const [ accountA, accountB, accountC ] = accounts; + + const keyA = new BN('7891'); + const keyB = new BN('451'); + const keyC = new BN('9592328'); + + const bytesA = '0xdeadbeef'.padEnd(66, '0'); + const bytesB = '0x0123456789'.padEnd(66, '0'); + const bytesC = '0x42424242'.padEnd(66, '0'); + + // AddressToUintMap + describe('AddressToUintMap', function () { + beforeEach(async function () { + this.map = await AddressToUintMapMock.new(); + }); + + shouldBehaveLikeMap( + [accountA, accountB, accountC], + [keyA, keyB, keyC], + new BN('0'), + ); + }); + + // UintToAddressMap + describe('UintToAddressMap', function () { + beforeEach(async function () { + this.map = await UintToAddressMapMock.new(); + }); + + shouldBehaveLikeMap( + [keyA, keyB, keyC], + [accountA, accountB, accountC], + constants.ZERO_ADDRESS, + ); + }); + + // Bytes32ToBytes32Map + describe('Bytes32ToBytes32Map', function () { + beforeEach(async function () { + this.map = await Bytes32ToBytes32MapMock.new(); + }); + + shouldBehaveLikeMap( + [keyA, keyB, keyC].map(k => ('0x' + k.toString(16)).padEnd(66, '0')), + [bytesA, bytesB, bytesC], + constants.ZERO_BYTES32, + ); + }); +}); diff --git a/test/utils/structs/EnumerableMap/AddressToUintMap.test.js b/test/utils/structs/EnumerableMap/AddressToUintMap.test.js deleted file mode 100644 index 0a79479323e..00000000000 --- a/test/utils/structs/EnumerableMap/AddressToUintMap.test.js +++ /dev/null @@ -1,157 +0,0 @@ -const { BN, expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); -const { expect } = require('chai'); -const { expectMembersMatch } = require('./helpers'); - -const AddressToUintMapMock = artifacts.require('AddressToUintMapMock'); - -contract('AddressToUintMap', function (accounts) { - const [accountA, accountB, accountC] = accounts; - - const valueA = new BN('7891'); - const valueB = new BN('451'); - const valueC = new BN('9592328'); - - beforeEach(async function () { - this.map = await AddressToUintMapMock.new(); - }); - - it('starts empty', async function () { - expect(await this.map.contains(accountA)).to.equal(false); - - await expectMembersMatch(this.map, [], []); - }); - - describe('set', function () { - it('adds a key', async function () { - const receipt = await this.map.set(accountA, valueA); - - expectEvent(receipt, 'OperationResult', { result: true }); - - await expectMembersMatch(this.map, [accountA], [valueA]); - }); - - it('adds several keys', async function () { - await this.map.set(accountA, valueA); - await this.map.set(accountB, valueB); - - await expectMembersMatch(this.map, [accountA, accountB], [valueA, valueB]); - - expect(await this.map.contains(accountC)).to.equal(false); - }); - - it('returns false when adding keys already in the set', async function () { - await this.map.set(accountA, valueA); - - const receipt = await this.map.set(accountA, valueA); - - expectEvent(receipt, 'OperationResult', { result: false }); - - await expectMembersMatch(this.map, [accountA], [valueA]); - }); - - it('updates values for keys already in the set', async function () { - await this.map.set(accountA, valueA); - await this.map.set(accountA, valueB); - - await expectMembersMatch(this.map, [accountA], [valueB]); - }); - }); - - describe('remove', function () { - it('removes added keys', async function () { - await this.map.set(accountA, valueA); - - const receipt = await this.map.remove(accountA); - - expectEvent(receipt, 'OperationResult', { result: true }); - - expect(await this.map.contains(accountA)).to.equal(false); - - await expectMembersMatch(this.map, [], []); - }); - - it('returns false when removing keys not in the set', async function () { - const receipt = await this.map.remove(accountA); - - expectEvent(receipt, 'OperationResult', { result: false }); - - expect(await this.map.contains(accountA)).to.equal(false); - }); - - it('adds and removes multiple keys', async function () { - // [] - - await this.map.set(accountA, valueA); - await this.map.set(accountC, valueC); - - // [A, C] - - await this.map.remove(accountA); - await this.map.remove(accountB); - - // [C] - - await this.map.set(accountB, valueB); - - // [C, B] - - await this.map.set(accountA, valueA); - await this.map.remove(accountC); - - // [A, B] - - await this.map.set(accountA, valueA); - await this.map.set(accountB, valueB); - - // [A, B] - - await this.map.set(accountC, valueC); - await this.map.remove(accountA); - - // [B, C] - - await this.map.set(accountA, valueA); - await this.map.remove(accountB); - - // [A, C] - - await expectMembersMatch(this.map, [accountA, accountC], [valueA, valueC]); - - expect(await this.map.contains(accountB)).to.equal(false); - }); - }); - - describe('read', function () { - beforeEach(async function () { - await this.map.set(accountA, valueA); - }); - - describe('get', function () { - it('existing value', async function () { - expect(await this.map.get(accountA)).to.bignumber.equal(valueA); - }); - - it('missing value', async function () { - await expectRevert(this.map.get(accountB), 'EnumerableMap: nonexistent key'); - }); - }); - - describe('tryGet', function () { - const stringifyTryGetValue = ({ 0: result, 1: value }) => ({ 0: result, 1: value.toString() }); - - it('existing value', async function () { - const actual = stringifyTryGetValue(await this.map.tryGet(accountA)); - const expected = stringifyTryGetValue({ 0: true, 1: valueA }); - - expect(actual).to.deep.equal(expected); - }); - - it('missing value', async function () { - const actual = stringifyTryGetValue(await this.map.tryGet(accountB)); - const expected = stringifyTryGetValue({ 0: false, 1: new BN('0') }); - - expect(actual).to.deep.equal(expected); - }); - }); - }); -}); diff --git a/test/utils/structs/EnumerableMap/UintToAddressMap.test.js b/test/utils/structs/EnumerableMap/UintToAddressMap.test.js deleted file mode 100644 index 560ed507701..00000000000 --- a/test/utils/structs/EnumerableMap/UintToAddressMap.test.js +++ /dev/null @@ -1,157 +0,0 @@ -const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); -const { expect } = require('chai'); -const { expectMembersMatch } = require('./helpers'); - -const UintToAddressMapMock = artifacts.require('UintToAddressMapMock'); - -contract('UintToAddressMap', function (accounts) { - const [accountA, accountB, accountC] = accounts; - - const keyA = new BN('7891'); - const keyB = new BN('451'); - const keyC = new BN('9592328'); - - beforeEach(async function () { - this.map = await UintToAddressMapMock.new(); - }); - - it('starts empty', async function () { - expect(await this.map.contains(keyA)).to.equal(false); - - await expectMembersMatch(this.map, [], []); - }); - - describe('set', function () { - it('adds a key', async function () { - const receipt = await this.map.set(keyA, accountA); - expectEvent(receipt, 'OperationResult', { result: true }); - - await expectMembersMatch(this.map, [keyA], [accountA]); - }); - - it('adds several keys', async function () { - await this.map.set(keyA, accountA); - await this.map.set(keyB, accountB); - - await expectMembersMatch(this.map, [keyA, keyB], [accountA, accountB]); - expect(await this.map.contains(keyC)).to.equal(false); - }); - - it('returns false when adding keys already in the set', async function () { - await this.map.set(keyA, accountA); - - const receipt = await this.map.set(keyA, accountA); - expectEvent(receipt, 'OperationResult', { result: false }); - - await expectMembersMatch(this.map, [keyA], [accountA]); - }); - - it('updates values for keys already in the set', async function () { - await this.map.set(keyA, accountA); - - await this.map.set(keyA, accountB); - - await expectMembersMatch(this.map, [keyA], [accountB]); - }); - }); - - describe('remove', function () { - it('removes added keys', async function () { - await this.map.set(keyA, accountA); - - const receipt = await this.map.remove(keyA); - expectEvent(receipt, 'OperationResult', { result: true }); - - expect(await this.map.contains(keyA)).to.equal(false); - await expectMembersMatch(this.map, [], []); - }); - - it('returns false when removing keys not in the set', async function () { - const receipt = await this.map.remove(keyA); - expectEvent(receipt, 'OperationResult', { result: false }); - - expect(await this.map.contains(keyA)).to.equal(false); - }); - - it('adds and removes multiple keys', async function () { - // [] - - await this.map.set(keyA, accountA); - await this.map.set(keyC, accountC); - - // [A, C] - - await this.map.remove(keyA); - await this.map.remove(keyB); - - // [C] - - await this.map.set(keyB, accountB); - - // [C, B] - - await this.map.set(keyA, accountA); - await this.map.remove(keyC); - - // [A, B] - - await this.map.set(keyA, accountA); - await this.map.set(keyB, accountB); - - // [A, B] - - await this.map.set(keyC, accountC); - await this.map.remove(keyA); - - // [B, C] - - await this.map.set(keyA, accountA); - await this.map.remove(keyB); - - // [A, C] - - await expectMembersMatch(this.map, [keyA, keyC], [accountA, accountC]); - - expect(await this.map.contains(keyB)).to.equal(false); - }); - }); - - describe('read', function () { - beforeEach(async function () { - await this.map.set(keyA, accountA); - }); - - describe('get', function () { - it('existing value', async function () { - expect(await this.map.get(keyA)).to.be.equal(accountA); - }); - it('missing value', async function () { - await expectRevert(this.map.get(keyB), 'EnumerableMap: nonexistent key'); - }); - }); - - describe('get with message', function () { - it('existing value', async function () { - expect(await this.map.getWithMessage(keyA, 'custom error string')).to.be.equal(accountA); - }); - it('missing value', async function () { - await expectRevert(this.map.getWithMessage(keyB, 'custom error string'), 'custom error string'); - }); - }); - - describe('tryGet', function () { - it('existing value', async function () { - expect(await this.map.tryGet(keyA)).to.be.deep.equal({ - 0: true, - 1: accountA, - }); - }); - it('missing value', async function () { - expect(await this.map.tryGet(keyB)).to.be.deep.equal({ - 0: false, - 1: constants.ZERO_ADDRESS, - }); - }); - }); - }); -}); diff --git a/test/utils/structs/EnumerableMap/helpers.js b/test/utils/structs/EnumerableMap/helpers.js deleted file mode 100644 index 456cdf156f3..00000000000 --- a/test/utils/structs/EnumerableMap/helpers.js +++ /dev/null @@ -1,31 +0,0 @@ -const zip = require('lodash.zip'); - -const toStringArray = (array) => array.map((i) => i.toString()); - -async function expectMembersMatch (map, keys, values) { - const stringKeys = toStringArray(keys); - const stringValues = toStringArray(values); - - expect(keys.length).to.equal(values.length); - - await Promise.all(keys.map(async (key) => expect(await map.contains(key)).to.equal(true))); - - expect(await map.length()).to.bignumber.equal(keys.length.toString()); - - expect(toStringArray(await Promise.all(keys.map((key) => map.get(key))))).to.have.same.members(stringValues); - - // to compare key-value pairs, we zip keys and values - // we also stringify pairs because this helper is used for multiple types of maps - expect( - await Promise.all( - [...Array(keys.length).keys()].map(async (index) => { - const { key, value } = await map.at(index); - return [key.toString(), value.toString()]; - }), - ), - ).to.have.same.deep.members(zip(stringKeys, stringValues)); -} - -module.exports = { - expectMembersMatch, -};