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 13 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
14 changes: 14 additions & 0 deletions contracts/mocks/AddressImpl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ pragma solidity ^0.6.0;
import "../utils/Address.sol";

contract AddressImpl {
event CallReturnValue(string data);

function isContract(address account) external view returns (bool) {
return Address.isContract(account);
}
Expand All @@ -13,6 +15,18 @@ contract AddressImpl {
Address.sendValue(receiver, amount);
}

function functionCall(address target, bytes calldata data) external {
bytes memory returnData = Address.functionCall(target, data);

emit CallReturnValue(abi.decode(returnData, (string)));
}

function functionCallWithValue(address target, bytes calldata data, uint256 value) external payable {
bytes memory returnData = Address.functionCallWithValue(target, data, value);

emit CallReturnValue(abi.decode(returnData, (string)));
}

// sendValue's tests require the contract to hold Ether
receive () external payable { }
}
40 changes: 40 additions & 0 deletions contracts/mocks/CallReceiverMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.6.0;

contract CallReceiverMock {

event MockFunctionCalled();

uint256[] private _array;

function mockFunction() public payable returns (string memory) {
emit MockFunctionCalled();

return "0x1234";
}

function mockFunctionNonPayable() public returns (string memory) {
emit MockFunctionCalled();

return "0x1234";
}

function mockFunctionRevertsNoReason() public payable {
revert();
}

function mockFunctionRevertsReason() public payable {
revert("CallReceiverMock: reverting");
}

function mockFunctionThrows() public payable {
assert(false);
}

function mockFunctionOutOfGas() public payable {
for (uint256 i = 0; ; ++i) {
_array.push(i);
}
}
}
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, "SafeERC20: low-level call failed");
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
21 changes: 4 additions & 17 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);
}
), "ERC721: transfer to non ERC721Receiver implementer");
bytes4 retval = abi.decode(returndata, (bytes4));
return (retval == _ERC721_RECEIVED);
}

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

/**
* @dev Performs a Solidity function call using a low level `call`. A
* plain`call` is an unsafe replacement for a function call: use this
* function instead.
*
* If `target` reverts with a revert reason, it is bubbled up by this
* function (like regular Solidity function calls).
*
* Requirements:
*
* - `target` must be a contract.
* - calling `target` with `data` must not revert.
*/
function functionCall(address target, bytes memory data) internal returns (bytes memory) {
return functionCall(target, data, "Address: low-level call failed");
}

/**
* @dev Same as {Address-functionCall-address-bytes-}, but with
* `errorMessage` as a fallback revert reason when `target` reverts.
*/
function functionCall(address target, bytes memory data, string memory errorMessage) internal returns (bytes memory) {
return _functionCallWithValue(target, data, 0, errorMessage);
}

/**
* @dev Performs a Solidity function call using a low level `call`,
* transferring `value` wei. A plain`call` is an unsafe replacement for a
* function call: use this function instead.
*
* If `target` reverts with a revert reason, it is bubbled up by this
* function (like regular Solidity function calls).
*
* Requirements:
*
* - `target` must be a contract.
* - the calling contract must have an ETH balance of at least `value`.
* - calling `target` with `data` must not revert.
*/
function functionCallWithValue(address target, bytes memory data, uint256 value) internal returns (bytes memory) {
return functionCallWithValue(target, data, value, "Address: low-level call with value failed");
}

/**
* @dev Same as {Address-functionCallWithValue-address-bytes-uint256-}, but
* with `errorMessage` as a fallback revert reason when `target` reverts.
*/
function functionCallWithValue(address target, bytes memory data, uint256 value, string memory errorMessage) internal returns (bytes memory) {
require(address(this).balance >= value, "Address: insufficient balance for call");
frangio marked this conversation as resolved.
Show resolved Hide resolved
return _functionCallWithValue(target, data, value, errorMessage);
}

function _functionCallWithValue(address target, bytes memory data, uint256 weiValue, string memory errorMessage) private 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{ value: weiValue }(data);
if (success) {
return returndata;
} else {
// Look for revert reason and bubble it up if present
if (returndata.length > 0) {
// The easiest way to bubble the revert reason is using memory via assembly

// solhint-disable-next-line no-inline-assembly
assembly {
let returndata_size := mload(returndata)
revert(add(32, returndata), returndata_size)
}
} else {
revert(errorMessage);
}
}
}
}
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
Loading