NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Optimize Address.functionCall removing redundant isContract check (#3469)

Browse Source 
Co-authored-by: Francisco <frangio.1@gmail.com>
This commit is contained in:
Mikhail Melnik
2022-06-28 04:02:33 +08:00
committed by GitHub
parent e8c60f92e3
commit 6f88199db9
4 changed files with 48 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -20,6 +20,7 @@
* `PaymentSplitter`: add `releasable` getters. ([#3350](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3350))
* `Initializable`: refactored implementation of modifiers for easier understanding. ([#3450](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3450))
* `Proxies`: remove runtime check of ERC1967 storage slots. ([#3455](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3455))
* `Address`: optimize `functionCall` functions by checking contract size only if there is no returned data. ([#3469](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3469))
### Breaking changes

2
contracts/mocks/crosschain/bridges.sol
View File

@ -22,7 +22,7 @@ abstract contract BaseRelayMock {
_currentSender = sender;
(bool success, bytes memory returndata) = target.call(data);
Address.verifyCallResult(success, returndata, "low-level call reverted");
Address.verifyCallResultFromTarget(target, success, returndata, "low-level call reverted");
_currentSender = previousSender;
}

64
contracts/utils/Address.sol
View File

@ -132,10 +132,8 @@ library Address {
string memory errorMessage
) internal returns (bytes memory) {
require(address(this).balance >= value, "Address: insufficient balance for call");
require(isContract(target), "Address: call to non-contract");
(bool success, bytes memory returndata) = target.call{value: value}(data);
return verifyCallResult(success, returndata, errorMessage);
return verifyCallResultFromTarget(target, success, returndata, errorMessage);
}
/**
@ -159,10 +157,8 @@ library Address {
bytes memory data,
string memory errorMessage
) internal view returns (bytes memory) {
require(isContract(target), "Address: static call to non-contract");
(bool success, bytes memory returndata) = target.staticcall(data);
return verifyCallResult(success, returndata, errorMessage);
return verifyCallResultFromTarget(target, success, returndata, errorMessage);
}
/**
@ -186,15 +182,37 @@ library Address {
bytes memory data,
string memory errorMessage
) internal returns (bytes memory) {
require(isContract(target), "Address: delegate call to non-contract");
(bool success, bytes memory returndata) = target.delegatecall(data);
return verifyCallResult(success, returndata, errorMessage);
return verifyCallResultFromTarget(target, success, returndata, errorMessage);
}
/**
* @dev Tool to verifies that a low level call was successful, and revert if it wasn't, either by bubbling the
* revert reason using the provided one.
* @dev Tool to verify that a low level call to smart-contract was successful, and revert (either by bubbling
* the revert reason or using the provided one) in case of unsuccessful call or if target was not a contract.
*
* _Available since v4.8._
*/
function verifyCallResultFromTarget(
address target,
bool success,
bytes memory returndata,
string memory errorMessage
) internal view returns (bytes memory) {
if (success) {
if (returndata.length == 0) {
// only check isContract if the call was successful and the return data is empty
// otherwise we already know that it was a contract
require(isContract(target), "Address: call to non-contract");
}
return returndata;
} else {
_revert(returndata, errorMessage);
}
}
/**
* @dev Tool to verify that a low level call was successful, and revert if it wasn't, either by bubbling the
* revert reason or using the provided one.
*
* _Available since v4.3._
*/
@ -206,17 +224,21 @@ library Address {
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
/// @solidity memory-safe-assembly
assembly {
let returndata_size := mload(returndata)
revert(add(32, returndata), returndata_size)
}
} else {
revert(errorMessage);
_revert(returndata, errorMessage);
}
}
function _revert(bytes memory returndata, string memory errorMessage) private pure {
// 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
/// @solidity memory-safe-assembly
assembly {
let returndata_size := mload(returndata)
revert(add(32, returndata), returndata_size)
}
} else {
revert(errorMessage);
}
}
}

6
test/utils/Address.test.js
View File

@ -143,7 +143,7 @@ contract('Address', function (accounts) {
}, []);
await expectRevert(
this.mock.functionCall(this.contractRecipient.address, abiEncodedCall, { gas: '100000' }),
this.mock.functionCall(this.contractRecipient.address, abiEncodedCall, { gas: '120000' }),
'Address: low-level call failed',
);
});
@ -329,7 +329,7 @@ contract('Address', function (accounts) {
}, []);
await expectRevert(
this.mock.functionStaticCall(recipient, abiEncodedCall),
'Address: static call to non-contract',
'Address: call to non-contract',
);
});
});
@ -375,7 +375,7 @@ contract('Address', function (accounts) {
}, []);
await expectRevert(
this.mock.functionDelegateCall(recipient, abiEncodedCall),
'Address: delegate call to non-contract',
'Address: call to non-contract',
);
});
});