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

feat: add wrapper function for low level calls #2264

Merged
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
4 changes: 4 additions & 0 deletions contracts/mocks/AddressImpl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ contract AddressImpl {
Address.sendValue(receiver, amount);
}

function functionCall(address target, bytes calldata data) external {
Address.functionCall(target, data);
}

// sendValue's tests require the contract to hold Ether
receive () external payable { }
}
15 changes: 3 additions & 12 deletions contracts/token/ERC20/SafeERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,10 @@ library SafeERC20 {
*/
function _callOptionalReturn(IERC20 token, bytes memory data) private {
// We need to perform a low level call here, to bypass Solidity's return data size checking mechanism, since
// we're implementing it ourselves.

// A Solidity high level call has three parts:
// 1. The target address is checked to verify it contains contract code
// 2. The call itself is made, and success asserted
// 3. The return value is decoded, which in turn checks the size of the returned data.
// solhint-disable-next-line max-line-length
require(address(token).isContract(), "SafeERC20: call to non-contract");

// solhint-disable-next-line avoid-low-level-calls
(bool success, bytes memory returndata) = address(token).call(data);
require(success, "SafeERC20: low-level call failed");
// we're implementing it ourselves. We use {Address.functionCall} to perform this call, which verifies that
// the target address contains contract code and also asserts for success in the low-level call.

bytes memory returndata = address(token).functionCall(data);
if (returndata.length > 0) { // Return data is optional
// solhint-disable-next-line max-line-length
require(abi.decode(returndata, (bool)), "SafeERC20: ERC20 operation did not succeed");
Expand Down
19 changes: 3 additions & 16 deletions contracts/token/ERC721/ERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -497,28 +497,15 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
if (!to.isContract()) {
return true;
}
// solhint-disable-next-line avoid-low-level-calls
(bool success, bytes memory returndata) = to.call(abi.encodeWithSelector(
bytes memory returndata = to.functionCall(abi.encodeWithSelector(
IERC721Receiver(to).onERC721Received.selector,
_msgSender(),
from,
tokenId,
_data
));
if (!success) {
if (returndata.length > 0) {
// solhint-disable-next-line no-inline-assembly
assembly {
let returndata_size := mload(returndata)
revert(add(32, returndata), returndata_size)
}
} else {
revert("ERC721: transfer to non ERC721Receiver implementer");
}
} else {
bytes4 retval = abi.decode(returndata, (bytes4));
return (retval == _ERC721_RECEIVED);
}
bytes4 retval = abi.decode(returndata, (bytes4));
return (retval == _ERC721_RECEIVED);
}

function _approve(address to, uint256 tokenId) private {
Expand Down
27 changes: 27 additions & 0 deletions contracts/utils/Address.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,31 @@ library Address {
(bool success, ) = recipient.call{ value: amount }("");
require(success, "Address: unable to send value, recipient may have reverted");
}

/**
* @dev Replacement for Solidity's low-level `call`: performs a low-level
julianmrodri marked this conversation as resolved.
Show resolved Hide resolved
* call with `data` to the target address `target`. Returns the `returndata`
* provided by the low-level call.
*
* The call is not executed if the target address is not a contract.
*/
function functionCall(address target, bytes memory data) internal returns (bytes memory) {
require(isContract(target), "Address: call to non-contract");

// solhint-disable-next-line avoid-low-level-calls
(bool success, bytes memory returndata) = target.call(data);
if (!success) {
nventuro marked this conversation as resolved.
Show resolved Hide resolved
if (returndata.length > 0) {
// solhint-disable-next-line no-inline-assembly
assembly {
let returndata_size := mload(returndata)
revert(add(32, returndata), returndata_size)
}
} else {
revert("Address: low-level call failed");
julianmrodri marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
return returndata;
}
}
}
2 changes: 1 addition & 1 deletion test/token/ERC20/SafeERC20.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('SafeERC20', function () {
this.wrapper = await SafeERC20Wrapper.new(hasNoCode);
});

shouldRevertOnAllCalls('SafeERC20: call to non-contract');
shouldRevertOnAllCalls('Address: call to non-contract');
nventuro marked this conversation as resolved.
Show resolved Hide resolved
});

describe('with token that returns false on all calls', function () {
Expand Down
4 changes: 2 additions & 2 deletions test/token/ERC721/ERC721.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ describe('ERC721', function () {
const nonReceiver = this.token;
await expectRevert(
this.token.safeTransferFrom(owner, nonReceiver.address, tokenId, { from: owner }),
'ERC721: transfer to non ERC721Receiver implementer'
'Address: low-level call failed'
);
});
});
Expand Down Expand Up @@ -463,7 +463,7 @@ describe('ERC721', function () {
const nonReceiver = this.token;
await expectRevert(
this.token.safeMint(nonReceiver.address, tokenId),
'ERC721: transfer to non ERC721Receiver implementer'
'Address: low-level call failed'
);
});
});
Expand Down
4 changes: 4 additions & 0 deletions test/utils/Address.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,8 @@ describe('Address', function () {
});
});
});

describe('functionCall', function () {
// TODO
});
});