Skip to content

Commit

Permalink
Merge branch 'master' into fix/improve-error-messages-#3120
Browse files Browse the repository at this point in the history
  • Loading branch information
frangio authored May 27, 2022
2 parents 2fd23b9 + 61294a6 commit 1b4b3ac
Show file tree
Hide file tree
Showing 170 changed files with 10,538 additions and 6,891 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
28 changes: 28 additions & 0 deletions .github/workflows/slither.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: Slither Analysis
on:
push:
branches:
- master
- release-v*
pull_request: {}
workflow_dispatch: {}

jobs:
analyze:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 12.x
- uses: actions/cache@v3
id: cache
with:
path: '**/node_modules'
key: npm-v2-${{ hashFiles('**/package-lock.json') }}
restore-keys: npm-v2-
- run: npm ci
if: steps.cache.outputs.cache-hit != 'true'
- name: Clean project
run: npm run clean
- uses: crytic/[email protected]
30 changes: 4 additions & 26 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -30,6 +30,7 @@ jobs:
FORCE_COLOR: 1
ENABLE_GAS_REPORT: true
- run: npm run test:inheritance
- run: npm run test:generation
- name: Print gas report
run: cat gas-report.txt

Expand All @@ -42,7 +43,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'
Expand All @@ -53,27 +54,4 @@ jobs:
- run: npm run coverage
env:
NODE_OPTIONS: --max_old_space_size=4096
- uses: codecov/codecov-action@v2

slither:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 12.x
- uses: actions/cache@v2
id: cache
with:
path: '**/node_modules'
key: npm-v2-${{ hashFiles('**/package-lock.json') }}
restore-keys: npm-v2-
- run: npm ci
if: steps.cache.outputs.cache-hit != 'true'
- name: Set up Python
uses: actions/setup-python@v2

- name: Install dependencies
run: pip3 install slither-analyzer
- name: Summary of static analysis
run: npm run slither
- uses: codecov/codecov-action@v3
34 changes: 31 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,42 @@

## Unreleased

* `Clones`: optimize clone creation ([#3329](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3329))
* `TimelockController`: Migrate `_call` to `_execute` and allow inheritance and overriding similar to `Governor`. ([#3317](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3317))
* `CrossChainEnabledPolygonChild`: replace the `require` statement with the custom error `NotCrossChainCall`. ([#3380](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3380))
* `ERC20FlashMint`: Add customizable flash fee receiver. ([#3327](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3327))
* `Strings`: add a new overloaded function `toHexString` that converts an `address` with fixed length of 20 bytes to its not checksummed ASCII `string` hexadecimal representation. ([#3403](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3403))
* `EnumerableMap`: add new `UintToUintMap` map type. ([#3338](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3338))
* `EnumerableMap`: add new `Bytes32ToUintMap` map type. ([#3416](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3416))
* `SafeCast`: add support for many more types, using procedural code generation. ([#3245](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3245))
* `MerkleProof`: add `multiProofVerify` to prove multiple values are part of a Merkle tree. ([#3276](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3276))

## 4.6.0 (2022-04-26)

* `crosschain`: Add a new set of contracts for cross-chain applications. `CrossChainEnabled` is a base contract with instantiations for several chains and bridges, and `AccessControlCrossChain` is an extension of access control that allows cross-chain operation. ([#3183](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3183))
* `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))
* `ERC20FlashMint`: support infinite allowance when paying back a flash loan. ([#3226](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3226))
* `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))
* `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))
* `ERC1155`: Add a `_afterTokenTransfer` hook for improved extensibility. ([#3166](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3166))
* `ERC1155URIStorage`: add a new extension that implements a `_setURI` behavior similar to ERC721's `_setTokenURI`. ([#3210](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3210))
* `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))
* `ERC20FlashMint`: support infinite allowance when paying back a flash loan. ([#3226](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3226))
* `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))
* `Governor`: fix an inconsistency in data locations that could lead to invalid bytecode being produced. ([#3295](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3295))
* `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))
* `TimelockController`: Add a separate canceller role for the ability to cancel. ([#3165](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3165))
* `Initializable`: add a reinitializer modifier that enables the initialization of new modules, added to already initialized contracts through upgradeability. ([#3232](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3232))
* `Initializable`: add an Initialized event that tracks initialized version numbers. ([#3294](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3294))
* `ERC2981`: make `royaltyInfo` public to allow super call in overrides. ([#3305](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3305))

### Upgradeability notice

* `TimelockController`: **(Action needed)** The upgrade from <4.6 to >=4.6 introduces a new `CANCELLER_ROLE` that requires set up to be assignable. After the upgrade, only addresses with this role will have the ability to cancel. Proposers will no longer be able to cancel. Assigning cancellers can be done by an admin (including the timelock itself) once the role admin is set up. To do this, we recommend upgrading to the `TimelockControllerWith46MigrationUpgradeable` contract and then calling the `migrateTo46` function.

### Breaking changes

Expand Down Expand Up @@ -455,7 +483,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))
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!

Expand Down
2 changes: 1 addition & 1 deletion audit/2017-03.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion certora/applyHarness.patch
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions certora/specs/GovernorBase.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions certora/specs/GovernorCountingSimple.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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){
Expand All @@ -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";
}
14 changes: 13 additions & 1 deletion contracts/access/AccessControl.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v4.5.0) (access/AccessControl.sol)
// OpenZeppelin Contracts (last updated v4.6.0) (access/AccessControl.sol)

pragma solidity ^0.8.0;

Expand Down Expand Up @@ -138,6 +138,8 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
* Requirements:
*
* - the caller must have ``role``'s admin role.
*
* May emit a {RoleGranted} event.
*/
function grantRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) {
_grantRole(role, account);
Expand All @@ -151,6 +153,8 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
* Requirements:
*
* - the caller must have ``role``'s admin role.
*
* May emit a {RoleRevoked} event.
*/
function revokeRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) {
_revokeRole(role, account);
Expand All @@ -169,6 +173,8 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
* Requirements:
*
* - the caller must be `account`.
*
* May emit a {RoleRevoked} event.
*/
function renounceRole(bytes32 role, address account) public virtual override {
require(account == _msgSender(), "AccessControl: can only renounce roles for self");
Expand All @@ -183,6 +189,8 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
* event. Note that unlike {grantRole}, this function doesn't perform any
* checks on the calling account.
*
* May emit a {RoleGranted} event.
*
* [WARNING]
* ====
* This function should only be called from the constructor when setting
Expand Down Expand Up @@ -213,6 +221,8 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
* @dev Grants `role` to `account`.
*
* Internal function without access restriction.
*
* May emit a {RoleGranted} event.
*/
function _grantRole(bytes32 role, address account) internal virtual {
if (!hasRole(role, account)) {
Expand All @@ -225,6 +235,8 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
* @dev Revokes `role` from `account`.
*
* Internal function without access restriction.
*
* May emit a {RoleRevoked} event.
*/
function _revokeRole(bytes32 role, address account) internal virtual {
if (hasRole(role, account)) {
Expand Down
Loading

0 comments on commit 1b4b3ac

Please sign in to comment.