From ef103f37e4eb18514f0d1d8511c38e9fffe2afce Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 14 Jun 2023 21:11:12 +0200 Subject: [PATCH] Replace some uses of abi.encodePacked with more explicit alternatives (#4296) Co-authored-by: Francisco --- .changeset/flat-bottles-wonder.md | 5 +++++ .../governance/compatibility/GovernorCompatibilityBravo.sol | 2 +- contracts/mocks/ReentrancyAttack.sol | 4 ++-- contracts/mocks/ReentrancyMock.sol | 3 +-- contracts/token/ERC1155/extensions/ERC1155URIStorage.sol | 4 ++-- contracts/token/ERC721/ERC721.sol | 2 +- contracts/token/ERC721/extensions/ERC721URIStorage.sol | 4 ++-- contracts/utils/Strings.sol | 2 +- 8 files changed, 15 insertions(+), 11 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 000000000..099ea8339 --- /dev/null +++ b/.changeset/flat-bottles-wonder.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +Replace some uses of `abi.encodePacked` with clearer alternatives (e.g. `bytes.concat`, `string.concat`). diff --git a/contracts/governance/compatibility/GovernorCompatibilityBravo.sol b/contracts/governance/compatibility/GovernorCompatibilityBravo.sol index 670be9591..8d6d9ccb4 100644 --- a/contracts/governance/compatibility/GovernorCompatibilityBravo.sol +++ b/contracts/governance/compatibility/GovernorCompatibilityBravo.sol @@ -155,7 +155,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 2da8b1f1a..df2924301 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 b4819dd59..053e53d77 100644 --- a/contracts/mocks/ReentrancyMock.sol +++ b/contracts/mocks/ReentrancyMock.sol @@ -33,8 +33,7 @@ contract ReentrancyMock is ReentrancyGuard { 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 5b0da2b33..79782a42b 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 a26255c7c..21ed95813 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -97,7 +97,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er _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 ae058122d..ae625fc28 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 65c8c8753..574717f86 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -47,7 +47,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))); } /**