From 5599e4309a77ac7fc72cfdf58da24cc0466eb199 Mon Sep 17 00:00:00 2001 From: Quentin Garchery Date: Thu, 17 Aug 2023 11:53:47 +0200 Subject: [PATCH 1/3] refactor: remove interface and code length check --- src/interfaces/IERC20.sol | 6 ++---- src/libraries/SafeTransferLib.sol | 19 +++++++++---------- test/forge/SafeTransferLib.t.sol | 7 ------- 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/interfaces/IERC20.sol b/src/interfaces/IERC20.sol index c976ff288..2d5816d50 100644 --- a/src/interfaces/IERC20.sol +++ b/src/interfaces/IERC20.sol @@ -4,7 +4,5 @@ pragma solidity >=0.5.0; /// @title IERC20 /// @author Morpho Labs /// @custom:contact security@morpho.xyz -interface IERC20 { - function transfer(address to, uint256 value) external returns (bool); - function transferFrom(address from, address to, uint256 value) external returns (bool); -} +/// @dev Empty because we only call library functions. It prevents calling transfer (transferFrom) instead of safeTransfer (safeTransferFrom). +interface IERC20 {} diff --git a/src/libraries/SafeTransferLib.sol b/src/libraries/SafeTransferLib.sol index cfb7c2ab9..4fb877efc 100644 --- a/src/libraries/SafeTransferLib.sol +++ b/src/libraries/SafeTransferLib.sol @@ -5,6 +5,11 @@ import {ErrorsLib} from "../libraries/ErrorsLib.sol"; import {IERC20} from "../interfaces/IERC20.sol"; +interface ERC20 { + function transfer(address to, uint256 value) external returns (bool); + function transferFrom(address from, address to, uint256 value) external returns (bool); +} + /// @title SafeTransferLib /// @author Morpho Labs /// @custom:contact security@morpho.xyz @@ -12,19 +17,13 @@ import {IERC20} from "../interfaces/IERC20.sol"; /// not returning a boolean for `transfer` and `transferFrom` functions. library SafeTransferLib { function safeTransfer(IERC20 token, address to, uint256 value) internal { - (bool success, bytes memory returndata) = address(token).call(abi.encodeCall(token.transfer, (to, value))); - require( - success && address(token).code.length > 0 && (returndata.length == 0 || abi.decode(returndata, (bool))), - ErrorsLib.TRANSFER_FAILED - ); + (bool success, bytes memory returndata) = address(token).call(abi.encodeCall(ERC20.transfer, (to, value))); + require(success && (returndata.length == 0 || abi.decode(returndata, (bool))), ErrorsLib.TRANSFER_FAILED); } function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { (bool success, bytes memory returndata) = - address(token).call(abi.encodeCall(token.transferFrom, (from, to, value))); - require( - success && address(token).code.length > 0 && (returndata.length == 0 || abi.decode(returndata, (bool))), - ErrorsLib.TRANSFER_FROM_FAILED - ); + address(token).call(abi.encodeCall(ERC20.transferFrom, (from, to, value))); + require(success && (returndata.length == 0 || abi.decode(returndata, (bool))), ErrorsLib.TRANSFER_FROM_FAILED); } } diff --git a/test/forge/SafeTransferLib.t.sol b/test/forge/SafeTransferLib.t.sol index 1d6454b31..e947f5456 100644 --- a/test/forge/SafeTransferLib.t.sol +++ b/test/forge/SafeTransferLib.t.sol @@ -59,13 +59,6 @@ contract SafeTransferLibTest is Test { tokenWithBooleanAlwaysFalse = new ERC20WithBooleanAlwaysFalse(); } - function testSafeTransferShouldRevertOnTokenWithEmptyCode(address noCode) public { - vm.assume(noCode.code.length == 0); - - vm.expectRevert(bytes(ErrorsLib.TRANSFER_FAILED)); - this.safeTransfer(noCode, address(0), 0); - } - function testSafeTransfer(address to, uint256 amount) public { tokenWithoutBoolean.setBalance(address(this), amount); From 4c08fe6c0344463ca74ec55ac711948bafe40b51 Mon Sep 17 00:00:00 2001 From: Quentin Garchery Date: Thu, 17 Aug 2023 15:14:31 +0200 Subject: [PATCH 2/3] docs: add a comment about non-zero code --- src/libraries/SafeTransferLib.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/SafeTransferLib.sol b/src/libraries/SafeTransferLib.sol index 4fb877efc..468d6eff3 100644 --- a/src/libraries/SafeTransferLib.sol +++ b/src/libraries/SafeTransferLib.sol @@ -15,6 +15,7 @@ interface ERC20 { /// @custom:contact security@morpho.xyz /// @notice Library to manage tokens not fully ERC20 compliant: /// not returning a boolean for `transfer` and `transferFrom` functions. +/// @dev It is the responsibility of the market creator to make sure that the address of the token has non-zero code. library SafeTransferLib { function safeTransfer(IERC20 token, address to, uint256 value) internal { (bool success, bytes memory returndata) = address(token).call(abi.encodeCall(ERC20.transfer, (to, value))); From 7d983831a99e013fb5fce08f14a4360facd9eb9f Mon Sep 17 00:00:00 2001 From: Quentin Garchery Date: Thu, 17 Aug 2023 18:38:56 +0200 Subject: [PATCH 3/3] style: rename internal interface in safe transfer lib --- src/libraries/SafeTransferLib.sol | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/SafeTransferLib.sol b/src/libraries/SafeTransferLib.sol index 468d6eff3..4efeb7fa9 100644 --- a/src/libraries/SafeTransferLib.sol +++ b/src/libraries/SafeTransferLib.sol @@ -5,7 +5,7 @@ import {ErrorsLib} from "../libraries/ErrorsLib.sol"; import {IERC20} from "../interfaces/IERC20.sol"; -interface ERC20 { +interface IERC20Internal { function transfer(address to, uint256 value) external returns (bool); function transferFrom(address from, address to, uint256 value) external returns (bool); } @@ -18,13 +18,14 @@ interface ERC20 { /// @dev It is the responsibility of the market creator to make sure that the address of the token has non-zero code. library SafeTransferLib { function safeTransfer(IERC20 token, address to, uint256 value) internal { - (bool success, bytes memory returndata) = address(token).call(abi.encodeCall(ERC20.transfer, (to, value))); + (bool success, bytes memory returndata) = + address(token).call(abi.encodeCall(IERC20Internal.transfer, (to, value))); require(success && (returndata.length == 0 || abi.decode(returndata, (bool))), ErrorsLib.TRANSFER_FAILED); } function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { (bool success, bytes memory returndata) = - address(token).call(abi.encodeCall(ERC20.transferFrom, (from, to, value))); + address(token).call(abi.encodeCall(IERC20Internal.transferFrom, (from, to, value))); require(success && (returndata.length == 0 || abi.decode(returndata, (bool))), ErrorsLib.TRANSFER_FROM_FAILED); } }