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

Low level call and memory management #5091

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
24 changes: 15 additions & 9 deletions contracts/access/manager/AuthorityUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
pragma solidity ^0.8.20;

import {IAuthority} from "./IAuthority.sol";
import {Memory} from "../../utils/Memory.sol";

library AuthorityUtils {
/**
Expand All @@ -17,16 +18,21 @@ library AuthorityUtils {
address target,
bytes4 selector
) internal view returns (bool immediate, uint32 delay) {
(bool success, bytes memory data) = authority.staticcall(
abi.encodeCall(IAuthority.canCall, (caller, target, selector))
);
if (success) {
if (data.length >= 0x40) {
(immediate, delay) = abi.decode(data, (bool, uint32));
} else if (data.length >= 0x20) {
immediate = abi.decode(data, (bool));
Memory.FreePtr ptr = Memory.save();

bytes memory params = abi.encodeCall(IAuthority.canCall, (caller, target, selector));
assembly ("memory-safe") {
let success := staticcall(not(0), authority, add(params, 0x20), mload(params), 0, 0x40)
if success {
if gt(returndatasize(), 0x1F) {
immediate := gt(mload(0x00), 0)
}
if gt(returndatasize(), 0x3F) {
delay := and(mload(0x20), 0xFFFFFFFF)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this revert if delay is too large ?

Suggested change
delay := and(mload(0x20), 0xFFFFFFFF)
delay := mload(0x20)
if gt(delay, 0xFFFFFFFF) { revert(0,0) }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

abi.decode reverts, so I guess we should revert too to keep backwards compatibility

ernestognw marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
return (immediate, delay);

Memory.load(ptr);
}
}
30 changes: 18 additions & 12 deletions contracts/token/ERC20/extensions/ERC4626.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {IERC20, IERC20Metadata, ERC20} from "../ERC20.sol";
import {SafeERC20} from "../utils/SafeERC20.sol";
import {IERC4626} from "../../../interfaces/IERC4626.sol";
import {Math} from "../../../utils/math/Math.sol";
import {Memory} from "../../../utils/Memory.sol";

/**
* @dev Implementation of the ERC-4626 "Tokenized Vault Standard" as defined in
Expand Down Expand Up @@ -75,25 +76,30 @@ abstract contract ERC4626 is ERC20, IERC4626 {
* @dev Set the underlying asset contract. This must be an ERC20-compatible contract (ERC-20 or ERC-777).
*/
constructor(IERC20 asset_) {
(bool success, uint8 assetDecimals) = _tryGetAssetDecimals(asset_);
_underlyingDecimals = success ? assetDecimals : 18;
_underlyingDecimals = _tryGetAssetDecimalsWithFallback(asset_, 18);
_asset = asset_;
}

/**
* @dev Attempts to fetch the asset decimals. A return value of false indicates that the attempt failed in some way.
*/
function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool, uint8) {
(bool success, bytes memory encodedDecimals) = address(asset_).staticcall(
abi.encodeCall(IERC20Metadata.decimals, ())
);
if (success && encodedDecimals.length >= 32) {
uint256 returnedDecimals = abi.decode(encodedDecimals, (uint256));
if (returnedDecimals <= type(uint8).max) {
return (true, uint8(returnedDecimals));
}
function _tryGetAssetDecimalsWithFallback(IERC20 asset_, uint8 defaultValue) private view returns (uint8) {
Memory.FreePtr ptr = Memory.save();

bool success;
uint256 length;
uint256 value;

bytes memory params = abi.encodeCall(IERC20Metadata.decimals, ());
assembly ("memory-safe") {
success := staticcall(not(0), asset_, add(params, 0x20), mload(params), 0, 0x20)
length := returndatasize()
value := mload(0)
}
return (false, 0);

Memory.load(ptr);

return uint8(Math.ternary(success && length >= 0x20 && value <= type(uint8).max, value, defaultValue));
}

/**
Expand Down
8 changes: 7 additions & 1 deletion contracts/token/ERC20/utils/SafeERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pragma solidity ^0.8.20;
import {IERC20} from "../IERC20.sol";
import {IERC1363} from "../../../interfaces/IERC1363.sol";
import {Address} from "../../../utils/Address.sol";
import {Memory} from "../../../utils/Memory.sol";

/**
* @title SafeERC20
Expand Down Expand Up @@ -34,15 +35,19 @@ library SafeERC20 {
* non-reverting calls are assumed to be successful.
*/
function safeTransfer(IERC20 token, address to, uint256 value) internal {
Memory.FreePtr ptr = Memory.save();
_callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value)));
Memory.load(ptr);
}

/**
* @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 {
Memory.FreePtr ptr = Memory.save();
_callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value)));
Memory.load(ptr);
}

/**
Expand Down Expand Up @@ -74,12 +79,13 @@ library SafeERC20 {
* to be set to zero before setting it to a non-zero value, such as USDT.
*/
function forceApprove(IERC20 token, address spender, uint256 value) internal {
Memory.FreePtr ptr = Memory.save();
bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value));

if (!_callOptionalReturnBool(token, approvalCall)) {
_callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0)));
_callOptionalReturn(token, approvalCall);
}
Memory.load(ptr);
}

/**
Expand Down
22 changes: 22 additions & 0 deletions contracts/utils/Memory.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.20;

/**
* @dev Helper library for deallocating memory reserved by abi.encode or low level calls.
*/
library Memory {
type FreePtr is bytes32;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be just Pointer, it doesn't have anything to do with the free memory pointer strictly talking (e.g. the library may add functions for dealing with pointers)

Suggested change
type FreePtr is bytes32;
type Pointer is bytes32;


function save() internal pure returns (FreePtr ptr) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, I think this should be very explicit, otherwise reading Memory.save() doesn't tell much.

Suggested change
function save() internal pure returns (FreePtr ptr) {
function saveFreePointer() internal pure returns (Pointer ptr) {

assembly ("memory-safe") {
ptr := mload(0x40)
}
}

function load(FreePtr ptr) internal pure {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accordingly

Suggested change
function load(FreePtr ptr) internal pure {
function loadFreePointer(Pointer ptr) internal pure {

assembly ("memory-safe") {
mstore(0x40, ptr)
}
}
}
21 changes: 14 additions & 7 deletions contracts/utils/cryptography/SignatureChecker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma solidity ^0.8.20;

import {ECDSA} from "./ECDSA.sol";
import {IERC1271} from "../../interfaces/IERC1271.sol";
import {Memory} from "../Memory.sol";

/**
* @dev Signature verification helper that can be used instead of `ECDSA.recover` to seamlessly support both ECDSA
Expand Down Expand Up @@ -39,12 +40,18 @@ library SignatureChecker {
address signer,
bytes32 hash,
bytes memory signature
) internal view returns (bool) {
(bool success, bytes memory result) = signer.staticcall(
abi.encodeCall(IERC1271.isValidSignature, (hash, signature))
);
return (success &&
result.length >= 32 &&
abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector));
) internal view returns (bool success) {
bytes4 magic = IERC1271.isValidSignature.selector;

Memory.FreePtr ptr = Memory.save();

bytes memory params = abi.encodeCall(IERC1271.isValidSignature, (hash, signature));
assembly ("memory-safe") {
if staticcall(not(0), signer, add(params, 0x20), mload(params), 0, 0x20) {
success := and(gt(returndatasize(), 0x1F), eq(mload(0), magic))
}
}

Memory.load(ptr);
}
}
Loading