Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace abi.encodeWithSelector & abi.encodeWithSignature with abi.encodeCall #4293

Merged
Show file tree
Hide file tree
Changes from 2 commits
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
41 changes: 41 additions & 0 deletions .changeset/big-plums-cover.md
balajipachai marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
'openzeppelin-solidity': major
---

Replaces abi.encodeWithSelector & abi.encodeWithSignature with abi.encodeCall

- WHAT the breaking change is

- The change replaces all occurences of abi.encodeWithSelector & abi.encodeWithSignature with abi.ecnodeCall

- WHY the change was made

- `abi.encodeWithSelector & abi.encodeWithSignature` can use with `interface.<function>.selector` to prevent `typo error`, but it `doesn't provide type checking`.

- HOW a consumer should update their code

````
interface miniERC20 {

function transfer(address to, uint256 value) external;
}

// works successfully
function transferData(uint256 to, uint256 value) public pure returns (bytes memory) {
return abi.encodeWithSelector(miniERC20.transfer.selector, to, value);
}```
````

`abi.encodeCall` provides type checking during compile time.

```
function transferData(uint256 to, uint256 value) public pure returns (bytes memory) {
return abi.encodeCall(miniERC20.transfer, (to, value));
}
```

```
error[5407]: TypeError: Cannot implicitly convert component at position 0 from "uint256" to "address".
```

`abi.encodeCall(function pointer, (function arguments...))`
2 changes: 1 addition & 1 deletion contracts/mocks/MulticallTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ contract MulticallTest {
) external {
bytes[] memory calls = new bytes[](recipients.length);
for (uint256 i = 0; i < recipients.length; i++) {
calls[i] = abi.encodeWithSignature("transfer(address,uint256)", recipients[i], amounts[i]);
calls[i] = abi.encodeCall(multicallToken.transfer, (recipients[i], amounts[i]));
}

bytes[] memory results = multicallToken.multicall(calls);
Expand Down
2 changes: 1 addition & 1 deletion contracts/mocks/ReentrancyMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ 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");
}
}
Expand Down
5 changes: 1 addition & 4 deletions contracts/mocks/proxy/UUPSLegacy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@ contract UUPSUpgradeableLegacyMock is UUPSUpgradeableMock {
if (!rollbackTesting.value) {
// Trigger rollback using upgradeTo from the new implementation
rollbackTesting.value = true;
Address.functionDelegateCall(
newImplementation,
abi.encodeWithSignature("upgradeTo(address)", oldImplementation)
);
Address.functionDelegateCall(newImplementation, abi.encodeCall(this.upgradeTo, (oldImplementation)));
rollbackTesting.value = false;
// Check rollback was effective
require(oldImplementation == _getImplementation(), "ERC1967Upgrade: upgrade breaks further upgrades");
Expand Down
2 changes: 1 addition & 1 deletion contracts/token/ERC20/extensions/ERC4626.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ abstract contract ERC4626 is ERC20, IERC4626 {
*/
function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool, uint8) {
(bool success, bytes memory encodedDecimals) = address(asset_).staticcall(
abi.encodeWithSelector(IERC20Metadata.decimals.selector)
abi.encodeCall(IERC20Metadata.decimals, ())
);
if (success && encodedDecimals.length >= 32) {
uint256 returnedDecimals = abi.decode(encodedDecimals, (uint256));
Expand Down
8 changes: 4 additions & 4 deletions contracts/token/ERC20/utils/SafeERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ library SafeERC20 {
* non-reverting calls are assumed to be successful.
*/
function safeTransfer(IERC20 token, address to, uint256 value) internal {
_callOptionalReturn(token, abi.encodeWithSelector(token.transfer.selector, to, value));
_callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value)));
}

/**
* @dev Transfer `value` amount of `token` from `from` to `to`, spending the approval given by `from` to the
* calling contract. If `token` returns no value, non-reverting calls are assumed to be successful.
*/
function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal {
_callOptionalReturn(token, abi.encodeWithSelector(token.transferFrom.selector, from, to, value));
_callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value)));
}

/**
Expand Down Expand Up @@ -62,10 +62,10 @@ library SafeERC20 {
* 0 before setting it to a non-zero value.
*/
function forceApprove(IERC20 token, address spender, uint256 value) internal {
bytes memory approvalCall = abi.encodeWithSelector(token.approve.selector, spender, value);
bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value));

if (!_callOptionalReturnBool(token, approvalCall)) {
_callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, 0));
_callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0)));
_callOptionalReturn(token, approvalCall);
}
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/utils/cryptography/SignatureChecker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ library SignatureChecker {
bytes memory signature
) internal view returns (bool) {
(bool success, bytes memory result) = signer.staticcall(
abi.encodeWithSelector(IERC1271.isValidSignature.selector, hash, signature)
abi.encodeCall(IERC1271.isValidSignature, (hash, signature))
);
return (success &&
result.length >= 32 &&
Expand Down
2 changes: 1 addition & 1 deletion contracts/utils/introspection/ERC165Checker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ library ERC165Checker {
*/
function supportsERC165InterfaceUnchecked(address account, bytes4 interfaceId) internal view returns (bool) {
// prepare call
bytes memory encodedParams = abi.encodeWithSelector(IERC165.supportsInterface.selector, interfaceId);
bytes memory encodedParams = abi.encodeCall(IERC165.supportsInterface, (interfaceId));

// perform static call
bool success;
Expand Down
2 changes: 1 addition & 1 deletion test/utils/ShortStrings.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ contract ShortStringsTest is Test {

function testRevertLong(string memory input) external {
vm.assume(!_isShort(input));
vm.expectRevert(abi.encodeWithSelector(ShortStrings.StringTooLong.selector, input));
balajipachai marked this conversation as resolved.
Show resolved Hide resolved
vm.expectRevert(abi.encodeCall(ShortStrings.StringTooLong, (input)));
balajipachai marked this conversation as resolved.
Show resolved Hide resolved
this.toShortString(input);
}

Expand Down