From f433e6d1e5d6a993e2d336200e227811cde4544c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 11 Oct 2024 23:47:17 +0200 Subject: [PATCH] making unsafeReadBytesOffset private --- .changeset/rude-cougars-look.md | 5 ----- contracts/governance/Governor.sol | 16 ++++++++++++++-- contracts/utils/Bytes.sol | 18 ------------------ contracts/utils/Strings.sol | 23 +++++++++++++++++------ 4 files changed, 31 insertions(+), 31 deletions(-) delete mode 100644 .changeset/rude-cougars-look.md delete mode 100644 contracts/utils/Bytes.sol diff --git a/.changeset/rude-cougars-look.md b/.changeset/rude-cougars-look.md deleted file mode 100644 index 8f0604041a3..00000000000 --- a/.changeset/rude-cougars-look.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': minor ---- - -`Bytes`: Add a library for common operations on `bytes` objects. diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 6f807a37d59..6d7d046a824 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -11,7 +11,6 @@ import {IERC165, ERC165} from "../utils/introspection/ERC165.sol"; import {SafeCast} from "../utils/math/SafeCast.sol"; import {DoubleEndedQueue} from "../utils/structs/DoubleEndedQueue.sol"; import {Address} from "../utils/Address.sol"; -import {Bytes} from "../utils/Bytes.sol"; import {Context} from "../utils/Context.sol"; import {Nonces} from "../utils/Nonces.sol"; import {Strings} from "../utils/Strings.sol"; @@ -770,7 +769,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72 } // Extract what would be the `#proposer=` marker beginning the suffix - bytes10 marker = bytes10(Bytes.unsafeReadBytesOffset(bytes(description), length - 52)); + bytes10 marker = bytes10(_unsafeReadBytesOffset(bytes(description), length - 52)); // If the marker is not found, there is no proposer suffix to check if (marker != bytes10("#proposer=")) { @@ -807,4 +806,17 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72 * @inheritdoc IGovernor */ function quorum(uint256 timepoint) public view virtual returns (uint256); + + /** + * @dev Reads a bytes32 from a bytes array without bounds checking. + * + * NOTE: making this function internal would mean it could be used with memory unsafe offset, and marking the + * assembly block as such would prevent some optimizations. + */ + function _unsafeReadBytesOffset(bytes memory buffer, uint256 offset) private pure returns (bytes32 value) { + // This is not memory safe in the general case, but all calls to this private function are within bounds. + assembly ("memory-safe") { + value := mload(add(buffer, add(0x20, offset))) + } + } } diff --git a/contracts/utils/Bytes.sol b/contracts/utils/Bytes.sol deleted file mode 100644 index f67cd69c006..00000000000 --- a/contracts/utils/Bytes.sol +++ /dev/null @@ -1,18 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.20; - -import {Math} from "./math/Math.sol"; - -/** - * @dev Bytes operations. - */ -library Bytes { - /// @dev Reads a bytes32 from a bytes array without bounds checking. - function unsafeReadBytesOffset(bytes memory buffer, uint256 offset) internal pure returns (bytes32 value) { - // This is not memory safe in the general case - assembly { - value := mload(add(buffer, add(0x20, offset))) - } - } -} diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 23fa9a7cde7..de7649b969c 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -6,13 +6,11 @@ pragma solidity ^0.8.20; import {Math} from "./math/Math.sol"; import {SafeCast} from "./math/SafeCast.sol"; import {SignedMath} from "./math/SignedMath.sol"; -import {Bytes} from "./Bytes.sol"; /** * @dev String operations. */ library Strings { - using Bytes for bytes; using SafeCast for *; bytes16 private constant HEX_DIGITS = "0123456789abcdef"; @@ -237,8 +235,8 @@ library Strings { bytes memory buffer = bytes(input); // Check presence of a negative sign. - bytes1 sign = bytes1(buffer.unsafeReadBytesOffset(begin)); - bool positiveSign = sign == bytes1("+"); + bytes1 sign = bytes1(_unsafeReadBytesOffset(buffer, begin)); + bool positiveSign = sign == bytes1("+"); bool negativeSign = sign == bytes1("-"); uint256 offset = (positiveSign || negativeSign).toUint(); @@ -299,7 +297,7 @@ library Strings { bytes memory buffer = bytes(input); // skip 0x prefix if present - bool hasPrefix = bytes2(buffer.unsafeReadBytesOffset(begin)) == bytes2("0x"); + bool hasPrefix = bytes2(_unsafeReadBytesOffset(buffer, begin)) == bytes2("0x"); uint256 offset = hasPrefix.toUint() * 2; uint256 result = 0; @@ -357,7 +355,7 @@ library Strings { uint256 end ) internal pure returns (bool success, address value) { // check that input is the correct length - bool hasPrefix = bytes2(bytes(input).unsafeReadBytesOffset(begin)) == bytes2("0x"); + bool hasPrefix = bytes2(_unsafeReadBytesOffset(bytes(input), begin)) == bytes2("0x"); uint256 expectedLength = 40 + hasPrefix.toUint() * 2; if (end - begin == expectedLength) { @@ -386,4 +384,17 @@ library Strings { return value; } + + /** + * @dev Reads a bytes32 from a bytes array without bounds checking. + * + * NOTE: making this function internal would mean it could be used with memory unsafe offset, and marking the + * assembly block as such would prevent some optimizations. + */ + function _unsafeReadBytesOffset(bytes memory buffer, uint256 offset) private pure returns (bytes32 value) { + // This is not memory safe in the general case, but all calls to this private function are within bounds. + assembly ("memory-safe") { + value := mload(add(buffer, add(0x20, offset))) + } + } }