Skip to content

Commit

Permalink
Merge branch 'master' into structure/circular-buffer
Browse files Browse the repository at this point in the history
  • Loading branch information
Amxx committed Apr 23, 2024
2 parents f6d92b4 + 4032b42 commit 5775190
Show file tree
Hide file tree
Showing 18 changed files with 120 additions and 33 deletions.
5 changes: 5 additions & 0 deletions .changeset/lucky-crews-eat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`Votes`: Set `_moveDelegateVotes` visibility to internal instead of private.
5 changes: 5 additions & 0 deletions .changeset/spotty-falcons-explain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`Math`, `SignedMath`: Add a branchless `ternary` function that computes`cond ? a : b` in constant gas cost.
5 changes: 5 additions & 0 deletions .changeset/thin-walls-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`AccessManager`, `VestingWallet`, `TimelockController` and `ERC2771Forwarder`: Added a public `initializer` function in their corresponding upgradeable variants.
7 changes: 4 additions & 3 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ jobs:
uses: ./.github/actions/setup
- name: Run coverage
run: npm run coverage
- uses: codecov/codecov-action@v3
with:
token: ${{ secrets.CODECOV_TOKEN }}
- uses: codecov/codecov-action@v4
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

harnesses:
runs-on: ubuntu-latest
Expand All @@ -118,6 +118,7 @@ jobs:
- uses: crytic/slither-action@v0.3.2
with:
node-version: 18.15
slither-version: 0.10.1

codespell:
runs-on: ubuntu-latest
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

### Breaking changes

- `ERC1967Utils`: Removed duplicate declaration of the `Upgraded`, `AdminChanged` and `BeaconUpgraded` events. These events are still available through the `IERC1967` interface located under the `contracts/interfaces/` directory. Minimum pragma version is now 0.8.21.

### Custom error changes

This version comes with changes to the custom error identifiers. Contracts previously depending on the following errors should be replaced accordingly:
Expand Down
11 changes: 6 additions & 5 deletions contracts/access/manager/AccessManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ import {Time} from "../../utils/types/Time.sol";
* will be {AccessManager} itself.
*
* WARNING: When granting permissions over an {Ownable} or {AccessControl} contract to an {AccessManager}, be very
* mindful of the danger associated with functions such as {{Ownable-renounceOwnership}} or
* {{AccessControl-renounceRole}}.
* mindful of the danger associated with functions such as {Ownable-renounceOwnership} or
* {AccessControl-renounceRole}.
*/
contract AccessManager is Context, Multicall, IAccessManager {
using Time for *;
Expand Down Expand Up @@ -109,8 +109,8 @@ contract AccessManager is Context, Multicall, IAccessManager {
bytes32 private _executionId;

/**
* @dev Check that the caller is authorized to perform the operation, following the restrictions encoded in
* {_getAdminRestrictions}.
* @dev Check that the caller is authorized to perform the operation.
* See {AccessManager} description for a detailed breakdown of the authorization logic.
*/
modifier onlyAuthorized() {
_checkAuthorized();
Expand Down Expand Up @@ -470,7 +470,8 @@ contract AccessManager is Context, Multicall, IAccessManager {

/**
* @dev Reverts if the operation is currently scheduled and has not expired.
* (Note: This function was introduced due to stack too deep errors in schedule.)
*
* NOTE: This function was introduced due to stack too deep errors in schedule.
*/
function _checkNotScheduled(bytes32 operationId) private view {
uint48 prevTimepoint = _schedules[operationId].timepoint;
Expand Down
2 changes: 1 addition & 1 deletion contracts/access/manager/IAccessManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ interface IAccessManager {
* to identify the indirect workflow, and will consider calls that require a delay to be forbidden.
*
* NOTE: This function does not report the permissions of this manager itself. These are defined by the
* {_canCallSelf} function instead.
* {AccessManager} documentation.
*/
function canCall(
address caller,
Expand Down
2 changes: 1 addition & 1 deletion contracts/governance/utils/Votes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 {
/**
* @dev Moves delegated votes from one delegate to another.
*/
function _moveDelegateVotes(address from, address to, uint256 amount) private {
function _moveDelegateVotes(address from, address to, uint256 amount) internal virtual {
if (from != to && amount > 0) {
if (from != address(0)) {
(uint256 oldValue, uint256 newValue) = _push(
Expand Down
2 changes: 1 addition & 1 deletion contracts/proxy/ERC1967/ERC1967Utils.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v5.0.0) (proxy/ERC1967/ERC1967Utils.sol)

pragma solidity ^0.8.20;
pragma solidity ^0.8.21;

import {IBeacon} from "../beacon/IBeacon.sol";
import {IERC1967} from "../../interfaces/IERC1967.sol";
Expand Down
2 changes: 1 addition & 1 deletion contracts/token/ERC20/extensions/ERC20FlashMint.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender {
error ERC3156ExceededMaxLoan(uint256 maxLoan);

/**
* @dev The receiver of a flashloan is not a valid {onFlashLoan} implementer.
* @dev The receiver of a flashloan is not a valid {IERC3156FlashBorrower-onFlashLoan} implementer.
*/
error ERC3156InvalidReceiver(address receiver);

Expand Down
8 changes: 4 additions & 4 deletions contracts/token/ERC20/extensions/ERC20Votes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import {Checkpoints} from "../../../utils/structs/Checkpoints.sol";
* NOTE: This contract does not provide interface compatibility with Compound's COMP token.
*
* This extension keeps a history (checkpoints) of each account's vote power. Vote power can be delegated either
* by calling the {delegate} function directly, or by providing a signature to be used with {delegateBySig}. Voting
* power can be queried through the public accessors {getVotes} and {getPastVotes}.
* by calling the {Votes-delegate} function directly, or by providing a signature to be used with {Votes-delegateBySig}. Voting
* power can be queried through the public accessors {Votes-getVotes} and {Votes-getPastVotes}.
*
* By default, token balance does not account for voting power. This makes transfers cheaper. The downside is that it
* requires users to delegate to themselves in order to activate checkpoints and have their voting power tracked.
Expand All @@ -30,9 +30,9 @@ abstract contract ERC20Votes is ERC20, Votes {
* @dev Maximum token supply. Defaults to `type(uint208).max` (2^208^ - 1).
*
* This maximum is enforced in {_update}. It limits the total supply of the token, which is otherwise a uint256,
* so that checkpoints can be stored in the Trace208 structure used by {{Votes}}. Increasing this value will not
* so that checkpoints can be stored in the Trace208 structure used by {Votes}. Increasing this value will not
* remove the underlying limitation, and will cause {_update} to fail because of a math overflow in
* {_transferVotingUnits}. An override could be used to further restrict the total supply (to a lower value) if
* {Votes-_transferVotingUnits}. An override could be used to further restrict the total supply (to a lower value) if
* additional logic requires it. When resolving override conflicts on this function, the minimum should be
* returned.
*/
Expand Down
26 changes: 21 additions & 5 deletions contracts/utils/math/Math.sol
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,34 @@ library Math {
}
}

/**
* @dev Branchless ternary evaluation for `a ? b : c`. Gas costs are constant.
*
* IMPORTANT: This function may reduce bytecode size and consume less gas when used standalone.
* However, the compiler may optimize Solidity ternary operations (i.e. `a ? b : c`) to only compute
* one branch when needed, making this function more expensive.
*/
function ternary(bool condition, uint256 a, uint256 b) internal pure returns (uint256) {
unchecked {
// branchless ternary works because:
// b ^ (a ^ b) == a
// b ^ 0 == b
return b ^ ((a ^ b) * SafeCast.toUint(condition));
}
}

/**
* @dev Returns the largest of two numbers.
*/
function max(uint256 a, uint256 b) internal pure returns (uint256) {
return a > b ? a : b;
return ternary(a > b, a, b);
}

/**
* @dev Returns the smallest of two numbers.
*/
function min(uint256 a, uint256 b) internal pure returns (uint256) {
return a < b ? a : b;
return ternary(a < b, a, b);
}

/**
Expand Down Expand Up @@ -114,7 +130,7 @@ library Math {
// but the largest value we can obtain is type(uint256).max - 1, which happens
// when a = type(uint256).max and b = 1.
unchecked {
return a == 0 ? 0 : (a - 1) / b + 1;
return SafeCast.toUint(a > 0) * ((a - 1) / b + 1);
}
}

Expand Down Expand Up @@ -147,7 +163,7 @@ library Math {

// Make sure the result is less than 2²⁵⁶. Also prevents denominator == 0.
if (denominator <= prod1) {
Panic.panic(denominator == 0 ? Panic.DIVISION_BY_ZERO : Panic.UNDER_OVERFLOW);
Panic.panic(ternary(denominator == 0, Panic.DIVISION_BY_ZERO, Panic.UNDER_OVERFLOW));
}

///////////////////////////////////////////////
Expand Down Expand Up @@ -268,7 +284,7 @@ library Math {
}

if (gcd != 1) return 0; // No inverse exists.
return x < 0 ? (n - uint256(-x)) : uint256(x); // Wrap the result if it's negative.
return ternary(x < 0, n - uint256(-x), uint256(x)); // Wrap the result if it's negative.
}
}

Expand Down
22 changes: 20 additions & 2 deletions contracts/utils/math/SignedMath.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,40 @@

pragma solidity ^0.8.20;

import {SafeCast} from "./SafeCast.sol";

/**
* @dev Standard signed math utilities missing in the Solidity language.
*/
library SignedMath {
/**
* @dev Branchless ternary evaluation for `a ? b : c`. Gas costs are constant.
*
* IMPORTANT: This function may reduce bytecode size and consume less gas when used standalone.
* However, the compiler may optimize Solidity ternary operations (i.e. `a ? b : c`) to only compute
* one branch when needed, making this function more expensive.
*/
function ternary(bool condition, int256 a, int256 b) internal pure returns (int256) {
unchecked {
// branchless terinary works because:
// b ^ (a ^ b) == a
// b ^ 0 == b
return b ^ ((a ^ b) * int256(SafeCast.toUint(condition)));
}
}

/**
* @dev Returns the largest of two signed numbers.
*/
function max(int256 a, int256 b) internal pure returns (int256) {
return a > b ? a : b;
return ternary(a > b, a, b);
}

/**
* @dev Returns the smallest of two signed numbers.
*/
function min(int256 a, int256 b) internal pure returns (int256) {
return a < b ? a : b;
return ternary(a < b, a, b);
}

/**
Expand Down
25 changes: 17 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
"solidity-ast": "^0.4.50",
"solidity-coverage": "^0.8.5",
"solidity-docgen": "^0.6.0-beta.29",
"undici": "^5.28.4",
"undici": "^6.11.1",
"yargs": "^17.0.0"
}
}
5 changes: 4 additions & 1 deletion scripts/upgradeable/transpile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ npx @openzeppelin/upgrade-safe-transpiler -D \
-x '!contracts/proxy/ERC1967/ERC1967Utils.sol' \
-x '!contracts/proxy/utils/UUPSUpgradeable.sol' \
-x '!contracts/proxy/beacon/IBeacon.sol' \
-p 'contracts/**/presets/**/*' \
-p 'contracts/access/manager/AccessManager.sol' \
-p 'contracts/finance/VestingWallet.sol' \
-p 'contracts/governance/TimelockController.sol' \
-p 'contracts/metatx/ERC2771Forwarder.sol' \
-n \
-N 'contracts/mocks/**/*' \
-q '@openzeppelin/'
Expand Down
10 changes: 10 additions & 0 deletions test/utils/math/Math.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ import {Test, stdError} from "forge-std/Test.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";

contract MathTest is Test {
function testSelect(bool f, uint256 a, uint256 b) public {
assertEq(Math.ternary(f, a, b), f ? a : b);
}

// MIN & MAX
function testMinMax(uint256 a, uint256 b) public {
assertEq(Math.min(a, b), a < b ? a : b);
assertEq(Math.max(a, b), a > b ? a : b);
}

// CEILDIV
function testCeilDiv(uint256 a, uint256 b) public {
vm.assume(b > 0);
Expand Down
10 changes: 10 additions & 0 deletions test/utils/math/SignedMath.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ import {Math} from "../../../contracts/utils/math/Math.sol";
import {SignedMath} from "../../../contracts/utils/math/SignedMath.sol";

contract SignedMathTest is Test {
function testSelect(bool f, int256 a, int256 b) public {
assertEq(SignedMath.ternary(f, a, b), f ? a : b);
}

// MIN & MAX
function testMinMax(int256 a, int256 b) public {
assertEq(SignedMath.min(a, b), a < b ? a : b);
assertEq(SignedMath.max(a, b), a > b ? a : b);
}

// MIN
function testMin(int256 a, int256 b) public {
int256 result = SignedMath.min(a, b);
Expand Down

0 comments on commit 5775190

Please # to comment.