From 0e16a4ab6a82d6ef7605b2a9b2b307f8920caecc Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 1 Jun 2023 13:52:29 +0200 Subject: [PATCH 1/2] Replace some uses of abi.encodePacked with more explicit alternatives --- .changeset/flat-bottles-wonder.md | 5 +++++ contracts/access/AccessControl.sol | 12 +++++------- .../compatibility/GovernorCompatibilityBravo.sol | 2 +- contracts/mocks/ReentrancyAttack.sol | 4 ++-- contracts/mocks/ReentrancyMock.sol | 5 ++--- .../token/ERC1155/extensions/ERC1155URIStorage.sol | 4 ++-- contracts/token/ERC721/ERC721.sol | 2 +- .../token/ERC721/extensions/ERC721URIStorage.sol | 4 ++-- contracts/utils/Strings.sol | 2 +- 9 files changed, 21 insertions(+), 19 deletions(-) create mode 100644 .changeset/flat-bottles-wonder.md diff --git a/.changeset/flat-bottles-wonder.md b/.changeset/flat-bottles-wonder.md new file mode 100644 index 00000000000..f70bba1d8d7 --- /dev/null +++ b/.changeset/flat-bottles-wonder.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +Replace some uses of `abi.encodePacked` with clearer alternatives diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol index 82a43933d72..c8d812f8317 100644 --- a/contracts/access/AccessControl.sol +++ b/contracts/access/AccessControl.sol @@ -108,13 +108,11 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 { function _checkRole(bytes32 role, address account) internal view virtual { if (!hasRole(role, account)) { revert( - string( - abi.encodePacked( - "AccessControl: account ", - Strings.toHexString(account), - " is missing role ", - Strings.toHexString(uint256(role), 32) - ) + string.concat( + "AccessControl: account ", + Strings.toHexString(account), + " is missing role ", + Strings.toHexString(uint256(role), 32) ) ); } diff --git a/contracts/governance/compatibility/GovernorCompatibilityBravo.sol b/contracts/governance/compatibility/GovernorCompatibilityBravo.sol index 425ecad0963..acdae99c68c 100644 --- a/contracts/governance/compatibility/GovernorCompatibilityBravo.sol +++ b/contracts/governance/compatibility/GovernorCompatibilityBravo.sol @@ -152,7 +152,7 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp for (uint256 i = 0; i < fullcalldatas.length; ++i) { fullcalldatas[i] = bytes(signatures[i]).length == 0 ? calldatas[i] - : abi.encodePacked(bytes4(keccak256(bytes(signatures[i]))), calldatas[i]); + : bytes.concat(abi.encodeWithSignature(signatures[i]), calldatas[i]); } return fullcalldatas; diff --git a/contracts/mocks/ReentrancyAttack.sol b/contracts/mocks/ReentrancyAttack.sol index 2da8b1f1ae8..df292430133 100644 --- a/contracts/mocks/ReentrancyAttack.sol +++ b/contracts/mocks/ReentrancyAttack.sol @@ -5,8 +5,8 @@ pragma solidity ^0.8.19; import "../utils/Context.sol"; contract ReentrancyAttack is Context { - function callSender(bytes4 data) public { - (bool success, ) = _msgSender().call(abi.encodeWithSelector(data)); + function callSender(bytes calldata data) public { + (bool success, ) = _msgSender().call(data); require(success, "ReentrancyAttack: failed call"); } } diff --git a/contracts/mocks/ReentrancyMock.sol b/contracts/mocks/ReentrancyMock.sol index 104d4f42aea..053e53d779e 100644 --- a/contracts/mocks/ReentrancyMock.sol +++ b/contracts/mocks/ReentrancyMock.sol @@ -26,15 +26,14 @@ contract ReentrancyMock is ReentrancyGuard { function countThisRecursive(uint256 n) public nonReentrant { if (n > 0) { _count(); - (bool success, ) = address(this).call(abi.encodeWithSignature("countThisRecursive(uint256)", n - 1)); + (bool success, ) = address(this).call(abi.encodeCall(this.countThisRecursive, (n - 1))); require(success, "ReentrancyMock: failed call"); } } function countAndCall(ReentrancyAttack attacker) public nonReentrant { _count(); - bytes4 func = bytes4(keccak256("callback()")); - attacker.callSender(func); + attacker.callSender(abi.encodeCall(this.callback, ())); } function _count() private { diff --git a/contracts/token/ERC1155/extensions/ERC1155URIStorage.sol b/contracts/token/ERC1155/extensions/ERC1155URIStorage.sol index 5b0da2b33a2..79782a42b4e 100644 --- a/contracts/token/ERC1155/extensions/ERC1155URIStorage.sol +++ b/contracts/token/ERC1155/extensions/ERC1155URIStorage.sol @@ -42,8 +42,8 @@ abstract contract ERC1155URIStorage is ERC1155 { function uri(uint256 tokenId) public view virtual override returns (string memory) { string memory tokenURI = _tokenURIs[tokenId]; - // If token URI is set, concatenate base URI and tokenURI (via abi.encodePacked). - return bytes(tokenURI).length > 0 ? string(abi.encodePacked(_baseURI, tokenURI)) : super.uri(tokenId); + // If token URI is set, concatenate base URI and tokenURI (via string.concat). + return bytes(tokenURI).length > 0 ? string.concat(_baseURI, tokenURI) : super.uri(tokenId); } /** diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 58a626cdad0..550bf53a5e3 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -92,7 +92,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { _requireMinted(tokenId); string memory baseURI = _baseURI(); - return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, tokenId.toString())) : ""; + return bytes(baseURI).length > 0 ? string.concat(baseURI, tokenId.toString()) : ""; } /** diff --git a/contracts/token/ERC721/extensions/ERC721URIStorage.sol b/contracts/token/ERC721/extensions/ERC721URIStorage.sol index 6350a0952ad..5d51cc8d6e3 100644 --- a/contracts/token/ERC721/extensions/ERC721URIStorage.sol +++ b/contracts/token/ERC721/extensions/ERC721URIStorage.sol @@ -35,9 +35,9 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 { if (bytes(base).length == 0) { return _tokenURI; } - // If both are set, concatenate the baseURI and tokenURI (via abi.encodePacked). + // If both are set, concatenate the baseURI and tokenURI (via string.concat). if (bytes(_tokenURI).length > 0) { - return string(abi.encodePacked(base, _tokenURI)); + return string.concat(base, _tokenURI); } return super.tokenURI(tokenId); diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 33d7bbf590e..ead778c956c 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -42,7 +42,7 @@ library Strings { * @dev Converts a `int256` to its ASCII `string` decimal representation. */ function toString(int256 value) internal pure returns (string memory) { - return string(abi.encodePacked(value < 0 ? "-" : "", toString(SignedMath.abs(value)))); + return string.concat(value < 0 ? "-" : "", toString(SignedMath.abs(value))); } /** From d23dfff90d12b1c9628b4d107d5e728bf26ff2a3 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 14 Jun 2023 15:59:43 +0200 Subject: [PATCH 2/2] Update .changeset/flat-bottles-wonder.md Co-authored-by: Francisco --- .changeset/flat-bottles-wonder.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/flat-bottles-wonder.md b/.changeset/flat-bottles-wonder.md index f70bba1d8d7..099ea833983 100644 --- a/.changeset/flat-bottles-wonder.md +++ b/.changeset/flat-bottles-wonder.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -Replace some uses of `abi.encodePacked` with clearer alternatives +Replace some uses of `abi.encodePacked` with clearer alternatives (e.g. `bytes.concat`, `string.concat`).