Skip to content

Commit

Permalink
Merge branch 'master' into EIP712-getters
Browse files Browse the repository at this point in the history
  • Loading branch information
ernestognw authored Jun 9, 2023
2 parents 64e0847 + cc04263 commit 2338b79
Show file tree
Hide file tree
Showing 68 changed files with 635 additions and 458 deletions.
5 changes: 0 additions & 5 deletions .changeset/beige-ducks-flow.md

This file was deleted.

4 changes: 4 additions & 0 deletions .changeset/big-plums-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
'openzeppelin-solidity': major
---
Use `abi.encodeCall` in place of `abi.encodeWithSelector` and `abi.encodeWithSignature` for improved type-checking of parameters
5 changes: 0 additions & 5 deletions .changeset/dirty-mangos-sort.md

This file was deleted.

5 changes: 5 additions & 0 deletions .changeset/eighty-crabs-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

Optimize `Strings.equal`
5 changes: 0 additions & 5 deletions .changeset/fluffy-gifts-build.md

This file was deleted.

5 changes: 5 additions & 0 deletions .changeset/four-adults-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

`ECDSA`: Use unchecked arithmetic for the `tryRecover` function that receives the `r` and `vs` short-signature fields separately.
5 changes: 0 additions & 5 deletions .changeset/friendly-suits-camp.md

This file was deleted.

5 changes: 0 additions & 5 deletions .changeset/hungry-impalas-perform.md

This file was deleted.

5 changes: 5 additions & 0 deletions .changeset/red-dots-fold.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': major
---

Overrides are now used internally for a number of functions that were previously hardcoded to their default implementation in certain locations: `ERC1155Supply.totalSupply`, `ERC721.ownerOf`, `ERC721.balanceOf` and `ERC721.totalSupply` in `ERC721Enumerable`, `ERC20.totalSupply` in `ERC20FlashMint`, and `ERC1967._getImplementation` in `ERC1967Proxy`.
5 changes: 0 additions & 5 deletions .changeset/selfish-queens-rest.md

This file was deleted.

5 changes: 5 additions & 0 deletions .changeset/serious-books-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

`ERC1155`: Optimize array allocation.
4 changes: 3 additions & 1 deletion .changeset/silly-bees-beam.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@
'openzeppelin-solidity': major
---

`ERC20Votes`: Changed internal vote accounting to reusable `Votes` module previously used by `ERC721Votes`. Removed implicit `ERC20Permit` inheritance. [#3816](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3816)
`ERC20Votes`: Changed internal vote accounting to reusable `Votes` module previously used by `ERC721Votes`. Removed implicit `ERC20Permit` inheritance. Note that the `DOMAIN_SEPARATOR` getter was previously guaranteed to be available for `ERC20Votes` contracts, but is no longer available unless `ERC20Permit` is explicitly used; ERC-5267 support is included in `ERC20Votes` with `EIP712` and is recommended as an alternative.

pr: #3816
5 changes: 0 additions & 5 deletions .changeset/spicy-ducks-cough.md

This file was deleted.

5 changes: 5 additions & 0 deletions .changeset/swift-bags-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

`Governor`: Add a mechanism to restrict the address of the proposer using a suffix in the description.
5 changes: 0 additions & 5 deletions .changeset/swift-berries-sort.md

This file was deleted.

5 changes: 0 additions & 5 deletions .changeset/tame-geckos-search.md

This file was deleted.

5 changes: 0 additions & 5 deletions .changeset/three-weeks-double.md

This file was deleted.

5 changes: 0 additions & 5 deletions .changeset/unlucky-snakes-drive.md

This file was deleted.

5 changes: 5 additions & 0 deletions .changeset/violet-dancers-cough.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

Remove the `override` specifier from functions that only override a single interface function.
8 changes: 3 additions & 5 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ concurrency:
group: checks-${{ github.ref }}
cancel-in-progress: true

env:
NODE_OPTIONS: --max_old_space_size=5120

jobs:
lint:
runs-on: ubuntu-latest
Expand All @@ -26,7 +29,6 @@ jobs:
runs-on: ubuntu-latest
env:
FORCE_COLOR: 1
NODE_OPTIONS: --max_old_space_size=4096
GAS: true
steps:
- uses: actions/checkout@v3
Expand Down Expand Up @@ -57,8 +59,6 @@ jobs:
run: bash scripts/upgradeable/transpile.sh
- name: Run tests
run: npm run test
env:
NODE_OPTIONS: --max_old_space_size=4096
- name: Check linearisation of the inheritance graph
run: npm run test:inheritance
- name: Check storage layout
Expand Down Expand Up @@ -86,8 +86,6 @@ jobs:
- name: Set up environment
uses: ./.github/actions/setup
- run: npm run coverage
env:
NODE_OPTIONS: --max_old_space_size=4096
- uses: codecov/codecov-action@v3
with:
token: ${{ secrets.CODECOV_TOKEN }}
Expand Down
19 changes: 17 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,29 @@

### Removals

The following contracts and libraries were removed:
The following contracts, libraries and functions were removed:

- `Address.isContract` (because of its ambiguous nature and potential for misuse)
- `Checkpoints.History`
- `Counters`
- `ERC20Snapshot`
- `ERC20VotesComp`
- `ERC165Storage` (in favor of inheritance based approach)
- `ERC777`
- `ERC1820Implementer`
- `GovernorVotesComp`
- `GovernorProposalThreshold` (deprecated since 4.4)
- `PaymentSplitter`
- `TokenTimelock` (removed in favor of `VestingWallet`)
- `PullPayment`
- `SafeMath`
- `SignedSafeMath`
- `Timers`
- `TokenTimelock` (in favor of `VestingWallet`)
- All escrow contracts (`Escrow`, `ConditionalEscrow` and `RefundEscrow`)
- All cross-chain contracts, including `AccessControlCrossChain` and all the vendored bridge interfaces
- All presets in favor of [OpenZeppelin Contracts Wizard](https://wizard.openzeppelin.com/)

These removals were implemented in the following PRs: [#3637](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3637), [#3880](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3880), [#3945](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3945), [#4258](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4258), [#4276](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4276), [#4289](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4289)

### How to upgrade from 4.x

Expand Down
10 changes: 5 additions & 5 deletions contracts/access/AccessControl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
/**
* @dev Returns `true` if `account` has been granted `role`.
*/
function hasRole(bytes32 role, address account) public view virtual override returns (bool) {
function hasRole(bytes32 role, address account) public view virtual returns (bool) {
return _roles[role].members[account];
}

Expand Down Expand Up @@ -126,7 +126,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
*
* To change a role's admin, use {_setRoleAdmin}.
*/
function getRoleAdmin(bytes32 role) public view virtual override returns (bytes32) {
function getRoleAdmin(bytes32 role) public view virtual returns (bytes32) {
return _roles[role].adminRole;
}

Expand All @@ -142,7 +142,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
*
* May emit a {RoleGranted} event.
*/
function grantRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) {
function grantRole(bytes32 role, address account) public virtual onlyRole(getRoleAdmin(role)) {
_grantRole(role, account);
}

Expand All @@ -157,7 +157,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
*
* May emit a {RoleRevoked} event.
*/
function revokeRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) {
function revokeRole(bytes32 role, address account) public virtual onlyRole(getRoleAdmin(role)) {
_revokeRole(role, account);
}

Expand All @@ -177,7 +177,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
*
* May emit a {RoleRevoked} event.
*/
function renounceRole(bytes32 role, address account) public virtual override {
function renounceRole(bytes32 role, address account) public virtual {
require(account == _msgSender(), "AccessControl: can only renounce roles for self");

_revokeRole(role, account);
Expand Down
4 changes: 2 additions & 2 deletions contracts/access/AccessControlEnumerable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ abstract contract AccessControlEnumerable is IAccessControlEnumerable, AccessCon
* https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post]
* for more information.
*/
function getRoleMember(bytes32 role, uint256 index) public view virtual override returns (address) {
function getRoleMember(bytes32 role, uint256 index) public view virtual returns (address) {
return _roleMembers[role].at(index);
}

/**
* @dev Returns the number of accounts that have `role`. Can be used
* together with {getRoleMember} to enumerate all bearers of a role.
*/
function getRoleMemberCount(bytes32 role) public view virtual override returns (uint256) {
function getRoleMemberCount(bytes32 role) public view virtual returns (uint256) {
return _roleMembers[role].length();
}

Expand Down
102 changes: 91 additions & 11 deletions contracts/governance/Governor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
}

/**
* @dev See {IGovernor-propose}.
* @dev See {IGovernor-propose}. This function has opt-in frontrunning protection, described in {_isValidDescriptionForProposer}.
*/
function propose(
address[] memory targets,
Expand All @@ -272,8 +272,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
string memory description
) public virtual override returns (uint256) {
address proposer = _msgSender();
uint256 currentTimepoint = clock();
require(_isValidDescriptionForProposer(proposer, description), "Governor: proposer restricted");

uint256 currentTimepoint = clock();
require(
getVotes(proposer, currentTimepoint - 1) >= proposalThreshold(),
"Governor: proposer votes below proposal threshold"
Expand Down Expand Up @@ -605,20 +606,14 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
/**
* @dev See {IERC721Receiver-onERC721Received}.
*/
function onERC721Received(address, address, uint256, bytes memory) public virtual override returns (bytes4) {
function onERC721Received(address, address, uint256, bytes memory) public virtual returns (bytes4) {
return this.onERC721Received.selector;
}

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

Expand All @@ -631,7 +626,92 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
uint256[] memory,
uint256[] memory,
bytes memory
) public virtual override returns (bytes4) {
) public virtual returns (bytes4) {
return this.onERC1155BatchReceived.selector;
}

/**
* @dev Check if the proposer is authorized to submit a proposal with the given description.
*
* If the proposal description ends with `#proposer=0x???`, where `0x???` is an address written as a hex string
* (case insensitive), then the submission of this proposal will only be authorized to said address.
*
* This is used for frontrunning protection. By adding this pattern at the end of their proposal, one can ensure
* that no other address can submit the same proposal. An attacker would have to either remove or change that part,
* which would result in a different proposal id.
*
* If the description does not match this pattern, it is unrestricted and anyone can submit it. This includes:
* - If the `0x???` part is not a valid hex string.
* - If the `0x???` part is a valid hex string, but does not contain exactly 40 hex digits.
* - If it ends with the expected suffix followed by newlines or other whitespace.
* - If it ends with some other similar suffix, e.g. `#other=abc`.
* - If it does not end with any such suffix.
*/
function _isValidDescriptionForProposer(
address proposer,
string memory description
) internal view virtual returns (bool) {
uint256 len = bytes(description).length;

// Length is too short to contain a valid proposer suffix
if (len < 52) {
return true;
}

// Extract what would be the `#proposer=0x` marker beginning the suffix
bytes12 marker;
assembly {
// - Start of the string contents in memory = description + 32
// - First character of the marker = len - 52
// - Length of "#proposer=0x0000000000000000000000000000000000000000" = 52
// - We read the memory word starting at the first character of the marker:
// - (description + 32) + (len - 52) = description + (len - 20)
// - Note: Solidity will ignore anything past the first 12 bytes
marker := mload(add(description, sub(len, 20)))
}

// If the marker is not found, there is no proposer suffix to check
if (marker != bytes12("#proposer=0x")) {
return true;
}

// Parse the 40 characters following the marker as uint160
uint160 recovered = 0;
for (uint256 i = len - 40; i < len; ++i) {
(bool isHex, uint8 value) = _tryHexToUint(bytes(description)[i]);
// If any of the characters is not a hex digit, ignore the suffix entirely
if (!isHex) {
return true;
}
recovered = (recovered << 4) | value;
}

return recovered == uint160(proposer);
}

/**
* @dev Try to parse a character from a string as a hex value. Returns `(true, value)` if the char is in
* `[0-9a-fA-F]` and `(false, 0)` otherwise. Value is guaranteed to be in the range `0 <= value < 16`
*/
function _tryHexToUint(bytes1 char) private pure returns (bool, uint8) {
uint8 c = uint8(char);
unchecked {
// Case 0-9
if (47 < c && c < 58) {
return (true, c - 48);
}
// Case A-F
else if (64 < c && c < 71) {
return (true, c - 55);
}
// Case a-f
else if (96 < c && c < 103) {
return (true, c - 87);
}
// Else: not a hex char
else {
return (false, 0);
}
}
}
}
4 changes: 2 additions & 2 deletions contracts/governance/IGovernor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,14 @@ abstract contract IGovernor is IERC165, IERC6372 {
* @notice module:core
* @dev See {IERC6372}
*/
function clock() public view virtual override returns (uint48);
function clock() public view virtual returns (uint48);

/**
* @notice module:core
* @dev See EIP-6372.
*/
// solhint-disable-next-line func-name-mixedcase
function CLOCK_MODE() public view virtual override returns (string memory);
function CLOCK_MODE() public view virtual returns (string memory);

/**
* @notice module:voting
Expand Down
Loading

0 comments on commit 2338b79

Please sign in to comment.