Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Replace some uses of abi.encodePacked with more explicit alternatives
  • Loading branch information
Amxx committed Jun 1, 2023
commit 0e16a4ab6a82d6ef7605b2a9b2b307f8920caecc
5 changes: 5 additions & 0 deletions .changeset/flat-bottles-wonder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

Replace some uses of `abi.encodePacked` with clearer alternatives
12 changes: 5 additions & 7 deletions contracts/access/AccessControl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions contracts/mocks/ReentrancyAttack.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
5 changes: 2 additions & 3 deletions contracts/mocks/ReentrancyMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions contracts/token/ERC1155/extensions/ERC1155URIStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion contracts/token/ERC721/ERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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()) : "";
}

/**
Expand Down
4 changes: 2 additions & 2 deletions contracts/token/ERC721/extensions/ERC721URIStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion contracts/utils/Strings.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}

/**
Expand Down