-
Notifications
You must be signed in to change notification settings - Fork 12.1k
Implement LowLevelCall library #5094
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
base: master
Are you sure you want to change the base?
Changes from all commits
5dfb2b3
e8e8438
8e1cc9c
7c7be9a
4ad1b23
dfe5cc1
5240323
58b4a96
bbb6aa1
044fbbe
85ce078
bb1a555
cf31c38
7d4196b
0323b38
9331c0d
28889bd
39edc84
a5918de
29e0c7a
652df3f
59c2f87
9eb5f1c
60d33d4
2d397f4
2a0fb7e
a7e61c3
1aae8bb
1b2679a
d514606
14fa04e
d0d55fc
608e3cd
513f8be
e38691d
ac92bb4
6094bb7
6bb96d5
860e5a8
ecdb768
95907aa
124ccee
c3237df
444ce03
27f0a9b
e7f35cc
848fc06
a213518
4182f32
5230b2c
ad16f66
d1d6412
716cd3f
cdd58c1
b7ce6dd
2a4cc06
a05e964
45e8d66
0a3b2cc
b547cf5
cb233b9
51a3c50
d79627b
0227ab4
bd60fec
6a74179
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'openzeppelin-solidity': minor | ||
--- | ||
|
||
`LowLevelCall`: Add a library to perform low-level calls and deal with the `returndata` more granularly. |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,6 +4,7 @@ | |||||||||||||||||||||||||||||||||||||||||||
pragma solidity ^0.8.20; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
import {Errors} from "./Errors.sol"; | ||||||||||||||||||||||||||||||||||||||||||||
import {LowLevelCall} from "./LowLevelCall.sol"; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||
* @dev Collection of functions related to the address type | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -34,10 +35,13 @@ library Address { | |||||||||||||||||||||||||||||||||||||||||||
if (address(this).balance < amount) { | ||||||||||||||||||||||||||||||||||||||||||||
revert Errors.InsufficientBalance(address(this).balance, amount); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
(bool success, bytes memory returndata) = recipient.call{value: amount}(""); | ||||||||||||||||||||||||||||||||||||||||||||
if (!success) { | ||||||||||||||||||||||||||||||||||||||||||||
_revert(returndata); | ||||||||||||||||||||||||||||||||||||||||||||
if (LowLevelCall.callNoReturn(recipient, amount, "")) { | ||||||||||||||||||||||||||||||||||||||||||||
// call successful, nothing to do | ||||||||||||||||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||||||||||||||
} else if (LowLevelCall.returnDataSize() > 0) { | ||||||||||||||||||||||||||||||||||||||||||||
LowLevelCall.bubbleRevert(); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
revert Errors.FailedCall(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -76,47 +80,74 @@ library Address { | |||||||||||||||||||||||||||||||||||||||||||
if (address(this).balance < value) { | ||||||||||||||||||||||||||||||||||||||||||||
revert Errors.InsufficientBalance(address(this).balance, value); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
(bool success, bytes memory returndata) = target.call{value: value}(data); | ||||||||||||||||||||||||||||||||||||||||||||
return verifyCallResultFromTarget(target, success, returndata); | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||||||||||||||||||||||||||||||||||||||||||||
bool success = LowLevelCall.callNoReturn(target, value, data); | ||||||||||||||||||||||||||||||||||||||||||||
if (success && (LowLevelCall.returnDataSize() > 0 || target.code.length > 0)) { | ||||||||||||||||||||||||||||||||||||||||||||
return LowLevelCall.returnData(); | ||||||||||||||||||||||||||||||||||||||||||||
} else if (success) { | ||||||||||||||||||||||||||||||||||||||||||||
revert AddressEmptyCode(target); | ||||||||||||||||||||||||||||||||||||||||||||
} else if (LowLevelCall.returnDataSize() > 0) { | ||||||||||||||||||||||||||||||||||||||||||||
LowLevelCall.bubbleRevert(); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
revert Errors.FailedCall(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+83
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This saves a few more gas compared to your numbers here.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally find it less readable. How must does it save ? I want cases do we save ? Currently the "happy path" (the one that leads to |
||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||
* @dev Same as {xref-Address-functionCall-address-bytes-}[`functionCall`], | ||||||||||||||||||||||||||||||||||||||||||||
* but performing a static call. | ||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||
function functionStaticCall(address target, bytes memory data) internal view returns (bytes memory) { | ||||||||||||||||||||||||||||||||||||||||||||
(bool success, bytes memory returndata) = target.staticcall(data); | ||||||||||||||||||||||||||||||||||||||||||||
return verifyCallResultFromTarget(target, success, returndata); | ||||||||||||||||||||||||||||||||||||||||||||
bool success = LowLevelCall.staticcallNoReturn(target, data); | ||||||||||||||||||||||||||||||||||||||||||||
if (success && (LowLevelCall.returnDataSize() > 0 || target.code.length > 0)) { | ||||||||||||||||||||||||||||||||||||||||||||
return LowLevelCall.returnData(); | ||||||||||||||||||||||||||||||||||||||||||||
} else if (success) { | ||||||||||||||||||||||||||||||||||||||||||||
revert AddressEmptyCode(target); | ||||||||||||||||||||||||||||||||||||||||||||
} else if (LowLevelCall.returnDataSize() > 0) { | ||||||||||||||||||||||||||||||||||||||||||||
LowLevelCall.bubbleRevert(); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
revert Errors.FailedCall(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||
* @dev Same as {xref-Address-functionCall-address-bytes-}[`functionCall`], | ||||||||||||||||||||||||||||||||||||||||||||
* but performing a delegate call. | ||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||
function functionDelegateCall(address target, bytes memory data) internal returns (bytes memory) { | ||||||||||||||||||||||||||||||||||||||||||||
(bool success, bytes memory returndata) = target.delegatecall(data); | ||||||||||||||||||||||||||||||||||||||||||||
return verifyCallResultFromTarget(target, success, returndata); | ||||||||||||||||||||||||||||||||||||||||||||
bool success = LowLevelCall.delegatecallNoReturn(target, data); | ||||||||||||||||||||||||||||||||||||||||||||
if (success && (LowLevelCall.returnDataSize() > 0 || target.code.length > 0)) { | ||||||||||||||||||||||||||||||||||||||||||||
return LowLevelCall.returnData(); | ||||||||||||||||||||||||||||||||||||||||||||
} else if (success) { | ||||||||||||||||||||||||||||||||||||||||||||
revert AddressEmptyCode(target); | ||||||||||||||||||||||||||||||||||||||||||||
} else if (LowLevelCall.returnDataSize() > 0) { | ||||||||||||||||||||||||||||||||||||||||||||
LowLevelCall.bubbleRevert(); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
revert Errors.FailedCall(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||
* @dev Tool to verify that a low level call to smart-contract was successful, and reverts if the target | ||||||||||||||||||||||||||||||||||||||||||||
* was not a contract or bubbling up the revert reason (falling back to {Errors.FailedCall}) in case | ||||||||||||||||||||||||||||||||||||||||||||
* of an unsuccessful call. | ||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||
* NOTE: This function is DEPRECATED and may be remove in the next major release. | ||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||
function verifyCallResultFromTarget( | ||||||||||||||||||||||||||||||||||||||||||||
address target, | ||||||||||||||||||||||||||||||||||||||||||||
bool success, | ||||||||||||||||||||||||||||||||||||||||||||
bytes memory returndata | ||||||||||||||||||||||||||||||||||||||||||||
) internal view returns (bytes memory) { | ||||||||||||||||||||||||||||||||||||||||||||
if (!success) { | ||||||||||||||||||||||||||||||||||||||||||||
_revert(returndata); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
// only check if target is a contract if the call was successful and the return data is empty | ||||||||||||||||||||||||||||||||||||||||||||
// otherwise we already know that it was a contract | ||||||||||||||||||||||||||||||||||||||||||||
if (returndata.length == 0 && target.code.length == 0) { | ||||||||||||||||||||||||||||||||||||||||||||
revert AddressEmptyCode(target); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
// only check if target is a contract if the call was successful and the return data is empty | ||||||||||||||||||||||||||||||||||||||||||||
// otherwise we already know that it was a contract | ||||||||||||||||||||||||||||||||||||||||||||
if (success && (returndata.length > 0 || target.code.length > 0)) { | ||||||||||||||||||||||||||||||||||||||||||||
return returndata; | ||||||||||||||||||||||||||||||||||||||||||||
} else if (success) { | ||||||||||||||||||||||||||||||||||||||||||||
revert AddressEmptyCode(target); | ||||||||||||||||||||||||||||||||||||||||||||
} else if (returndata.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||
LowLevelCall.bubbleRevert(returndata); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
revert Errors.FailedCall(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -125,23 +156,10 @@ library Address { | |||||||||||||||||||||||||||||||||||||||||||
* revert reason or with a default {Errors.FailedCall} error. | ||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||
function verifyCallResult(bool success, bytes memory returndata) internal pure returns (bytes memory) { | ||||||||||||||||||||||||||||||||||||||||||||
if (!success) { | ||||||||||||||||||||||||||||||||||||||||||||
_revert(returndata); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
if (success) { | ||||||||||||||||||||||||||||||||||||||||||||
return returndata; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||
* @dev Reverts with returndata if present. Otherwise reverts with {Errors.FailedCall}. | ||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||
function _revert(bytes memory returndata) 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 | ||||||||||||||||||||||||||||||||||||||||||||
assembly ("memory-safe") { | ||||||||||||||||||||||||||||||||||||||||||||
revert(add(returndata, 0x20), mload(returndata)) | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
} else if (returndata.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||
LowLevelCall.bubbleRevert(returndata); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
revert Errors.FailedCall(); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about using
functionCallWithValue(..)
instead ofverifyCallResult(..)
in Governor & Timelock?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functionCallWithValue
will revert withAddressEmptyCode
if you try to do a call to an EOA. If we use it in Governor/Timelock, then they would not be able to send ETH to an EOA. We have a test for that.