From dbcae10f4ebf340ed7bc0c0efb64079b6f2edf02 Mon Sep 17 00:00:00 2001 From: z0r0z <92001561+z0r0z@users.noreply.github.com> Date: Tue, 4 Oct 2022 18:17:01 +0900 Subject: [PATCH 1/6] Update ERC20Votes.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ✍️ minor typo fix in comments 🧮 use unchecked {} for simple arithmetic where underflow is obvious in preceding statement --- contracts/token/ERC20/extensions/ERC20Votes.sol | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC20Votes.sol b/contracts/token/ERC20/extensions/ERC20Votes.sol index da16cbada52..f1259954460 100644 --- a/contracts/token/ERC20/extensions/ERC20Votes.sol +++ b/contracts/token/ERC20/extensions/ERC20Votes.sol @@ -63,7 +63,9 @@ abstract contract ERC20Votes is IVotes, ERC20Permit { */ function getVotes(address account) public view virtual override returns (uint256) { uint256 pos = _checkpoints[account].length; - return pos == 0 ? 0 : _checkpoints[account][pos - 1].votes; + unchecked { + return pos == 0 ? 0 : _checkpoints[account][pos - 1].votes; + } } /** @@ -129,8 +131,10 @@ abstract contract ERC20Votes is IVotes, ERC20Permit { low = mid + 1; } } - - return high == 0 ? 0 : _unsafeAccess(ckpts, high - 1).votes; + + unchecked { + return high == 0 ? 0 : _unsafeAccess(ckpts, high - 1).votes; + } } /** @@ -242,8 +246,10 @@ abstract contract ERC20Votes is IVotes, ERC20Permit { uint256 delta ) private returns (uint256 oldWeight, uint256 newWeight) { uint256 pos = ckpts.length; - - Checkpoint memory oldCkpt = pos == 0 ? Checkpoint(0, 0) : _unsafeAccess(ckpts, pos - 1); + + unchecked { + Checkpoint memory oldCkpt = pos == 0 ? Checkpoint(0, 0) : _unsafeAccess(ckpts, pos - 1); + } oldWeight = oldCkpt.votes; newWeight = op(oldWeight, delta); From 912b609348f02def92ee890e109c4343e884ea7e Mon Sep 17 00:00:00 2001 From: z0r0z <92001561+z0r0z@users.noreply.github.com> Date: Tue, 4 Oct 2022 18:18:41 +0900 Subject: [PATCH 2/6] Update ERC20Votes.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ✍️ typo fix --- contracts/token/ERC20/extensions/ERC20Votes.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/ERC20/extensions/ERC20Votes.sol b/contracts/token/ERC20/extensions/ERC20Votes.sol index f1259954460..6c23a46cd42 100644 --- a/contracts/token/ERC20/extensions/ERC20Votes.sol +++ b/contracts/token/ERC20/extensions/ERC20Votes.sol @@ -82,7 +82,7 @@ abstract contract ERC20Votes is IVotes, ERC20Permit { /** * @dev Retrieve the `totalSupply` at the end of `blockNumber`. Note, this value is the sum of all balances. - * It is but NOT the sum of all the delegated votes! + * It is NOT the sum of all the delegated votes! * * Requirements: * From fb009ea10811850b72ba93e8fd92395690163a96 Mon Sep 17 00:00:00 2001 From: z0r0z <92001561+z0r0z@users.noreply.github.com> Date: Tue, 4 Oct 2022 18:32:28 +0900 Subject: [PATCH 3/6] Update ERC20Votes.sol fix memo --- contracts/token/ERC20/extensions/ERC20Votes.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC20Votes.sol b/contracts/token/ERC20/extensions/ERC20Votes.sol index 6c23a46cd42..3c27270955b 100644 --- a/contracts/token/ERC20/extensions/ERC20Votes.sol +++ b/contracts/token/ERC20/extensions/ERC20Votes.sol @@ -249,15 +249,15 @@ abstract contract ERC20Votes is IVotes, ERC20Permit { unchecked { Checkpoint memory oldCkpt = pos == 0 ? Checkpoint(0, 0) : _unsafeAccess(ckpts, pos - 1); - } - - oldWeight = oldCkpt.votes; - newWeight = op(oldWeight, delta); + + oldWeight = oldCkpt.votes; + newWeight = op(oldWeight, delta); - if (pos > 0 && oldCkpt.fromBlock == block.number) { - _unsafeAccess(ckpts, pos - 1).votes = SafeCast.toUint224(newWeight); - } else { - ckpts.push(Checkpoint({fromBlock: SafeCast.toUint32(block.number), votes: SafeCast.toUint224(newWeight)})); + if (pos > 0 && oldCkpt.fromBlock == block.number) { + _unsafeAccess(ckpts, pos - 1).votes = SafeCast.toUint224(newWeight); + } else { + ckpts.push(Checkpoint({fromBlock: SafeCast.toUint32(block.number), votes: SafeCast.toUint224(newWeight)})); + } } } From 56cc5efad53a17ece56b594c2e42f8516940ccff Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 5 Oct 2022 17:24:29 -0300 Subject: [PATCH 4/6] lint --- contracts/token/ERC20/extensions/ERC20Votes.sol | 10 ++++++---- contracts/utils/cryptography/MerkleProof.sol | 4 +++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/contracts/token/ERC20/extensions/ERC20Votes.sol b/contracts/token/ERC20/extensions/ERC20Votes.sol index 3c27270955b..581d4f0f3b8 100644 --- a/contracts/token/ERC20/extensions/ERC20Votes.sol +++ b/contracts/token/ERC20/extensions/ERC20Votes.sol @@ -131,7 +131,7 @@ abstract contract ERC20Votes is IVotes, ERC20Permit { low = mid + 1; } } - + unchecked { return high == 0 ? 0 : _unsafeAccess(ckpts, high - 1).votes; } @@ -246,17 +246,19 @@ abstract contract ERC20Votes is IVotes, ERC20Permit { uint256 delta ) private returns (uint256 oldWeight, uint256 newWeight) { uint256 pos = ckpts.length; - + unchecked { Checkpoint memory oldCkpt = pos == 0 ? Checkpoint(0, 0) : _unsafeAccess(ckpts, pos - 1); - + oldWeight = oldCkpt.votes; newWeight = op(oldWeight, delta); if (pos > 0 && oldCkpt.fromBlock == block.number) { _unsafeAccess(ckpts, pos - 1).votes = SafeCast.toUint224(newWeight); } else { - ckpts.push(Checkpoint({fromBlock: SafeCast.toUint32(block.number), votes: SafeCast.toUint224(newWeight)})); + ckpts.push( + Checkpoint({fromBlock: SafeCast.toUint32(block.number), votes: SafeCast.toUint224(newWeight)}) + ); } } } diff --git a/contracts/utils/cryptography/MerkleProof.sol b/contracts/utils/cryptography/MerkleProof.sol index 19b2500203f..bcae05c2c1b 100644 --- a/contracts/utils/cryptography/MerkleProof.sol +++ b/contracts/utils/cryptography/MerkleProof.sol @@ -138,7 +138,9 @@ library MerkleProof { // `proof` array. for (uint256 i = 0; i < totalHashes; i++) { bytes32 a = leafPos < leavesLen ? leaves[leafPos++] : hashes[hashPos++]; - bytes32 b = proofFlags[i] ? leafPos < leavesLen ? leaves[leafPos++] : hashes[hashPos++] : proof[proofPos++]; + bytes32 b = proofFlags[i] + ? (leafPos < leavesLen ? leaves[leafPos++] : hashes[hashPos++]) + : proof[proofPos++]; hashes[i] = _hashPair(a, b); } From d02bdff723edea4e6c91f8061d6f84fd8e806441 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 5 Oct 2022 17:25:39 -0300 Subject: [PATCH 5/6] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b8f0e56d0f..68e21375bf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + + * `ERC20Votes`: optimize by using unchecked arithmetic. ([#3748](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3748)) + ## Unreleased * `TimelockController`: Added a new `admin` constructor parameter that is assigned the admin role instead of the deployer account. ([#3722](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3722)) From 7d4c6f545337dd90b605b1d95a25d6a66a6c2111 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 5 Oct 2022 17:26:56 -0300 Subject: [PATCH 6/6] revert unrelated lint --- contracts/utils/cryptography/MerkleProof.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/utils/cryptography/MerkleProof.sol b/contracts/utils/cryptography/MerkleProof.sol index bcae05c2c1b..19b2500203f 100644 --- a/contracts/utils/cryptography/MerkleProof.sol +++ b/contracts/utils/cryptography/MerkleProof.sol @@ -138,9 +138,7 @@ library MerkleProof { // `proof` array. for (uint256 i = 0; i < totalHashes; i++) { bytes32 a = leafPos < leavesLen ? leaves[leafPos++] : hashes[hashPos++]; - bytes32 b = proofFlags[i] - ? (leafPos < leavesLen ? leaves[leafPos++] : hashes[hashPos++]) - : proof[proofPos++]; + bytes32 b = proofFlags[i] ? leafPos < leavesLen ? leaves[leafPos++] : hashes[hashPos++] : proof[proofPos++]; hashes[i] = _hashPair(a, b); }