From 98c9c6f078bb74865fc3954125aa7f9ddff229d7 Mon Sep 17 00:00:00 2001 From: "tu-do.ron" <18521578@gm.uit.edu.vn> Date: Mon, 20 Nov 2023 14:45:24 +0700 Subject: [PATCH 1/5] Revert "feat(lib-error-handler): implement `LibErrorHandler`" --- src/LibErrorHandler.sol | 40 ---------------------------------------- 1 file changed, 40 deletions(-) delete mode 100644 src/LibErrorHandler.sol diff --git a/src/LibErrorHandler.sol b/src/LibErrorHandler.sol deleted file mode 100644 index 8202fca..0000000 --- a/src/LibErrorHandler.sol +++ /dev/null @@ -1,40 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; - -library LibErrorHandler { - /// @dev Reserves error definition to upload to signature database. - error ExternalCallFailed(bytes4 msgSig, bytes4 callSig); - - /// @notice handle low level call revert if call failed, - /// If extcall return empty bytes, reverts with custom error. - /// @param status Status of external call - /// @param callSig function signature of the calldata - /// @param returnOrRevertData bytes result from external call - function handleRevert(bool status, bytes4 callSig, bytes memory returnOrRevertData) internal pure { - // Get the function signature of current context - bytes4 msgSig = msg.sig; - assembly ("memory-safe") { - if iszero(status) { - // Load the length of bytes array - let revertLength := mload(returnOrRevertData) - // Check if length != 0 => revert following reason from external call - if iszero(iszero(revertLength)) { - // Start of revert data bytes. The 0x20 offset is always the same. - revert(add(returnOrRevertData, 0x20), revertLength) - } - - // Load free memory pointer - let ptr := mload(0x40) - // Store 4 bytes the function selector of ExternalCallFailed(msg.sig, callSig) - // Equivalent to revert ExternalCallFailed(bytes4,bytes4) - mstore(ptr, 0x49bf4104) - // Store 4 bytes of msgSig parameter in the next slot - mstore(add(ptr, 0x20), msgSig) - // Store 4 bytes of callSig parameter in the next slot - mstore(add(ptr, 0x40), callSig) - // Revert 68 bytes of error starting from 0x1c - revert(add(ptr, 0x1c), 0x44) - } - } - } -} From 88358f5d6b226356a593f46985354ee842ac0130 Mon Sep 17 00:00:00 2001 From: "tu-do.ron" <18521578@gm.uit.edu.vn> Date: Mon, 20 Nov 2023 14:46:19 +0700 Subject: [PATCH 2/5] Revert "feat: add `LibErrorHandler`" --- src/LibErrorHandler.sol | 40 ---------------------------------------- 1 file changed, 40 deletions(-) delete mode 100644 src/LibErrorHandler.sol diff --git a/src/LibErrorHandler.sol b/src/LibErrorHandler.sol deleted file mode 100644 index 8202fca..0000000 --- a/src/LibErrorHandler.sol +++ /dev/null @@ -1,40 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; - -library LibErrorHandler { - /// @dev Reserves error definition to upload to signature database. - error ExternalCallFailed(bytes4 msgSig, bytes4 callSig); - - /// @notice handle low level call revert if call failed, - /// If extcall return empty bytes, reverts with custom error. - /// @param status Status of external call - /// @param callSig function signature of the calldata - /// @param returnOrRevertData bytes result from external call - function handleRevert(bool status, bytes4 callSig, bytes memory returnOrRevertData) internal pure { - // Get the function signature of current context - bytes4 msgSig = msg.sig; - assembly ("memory-safe") { - if iszero(status) { - // Load the length of bytes array - let revertLength := mload(returnOrRevertData) - // Check if length != 0 => revert following reason from external call - if iszero(iszero(revertLength)) { - // Start of revert data bytes. The 0x20 offset is always the same. - revert(add(returnOrRevertData, 0x20), revertLength) - } - - // Load free memory pointer - let ptr := mload(0x40) - // Store 4 bytes the function selector of ExternalCallFailed(msg.sig, callSig) - // Equivalent to revert ExternalCallFailed(bytes4,bytes4) - mstore(ptr, 0x49bf4104) - // Store 4 bytes of msgSig parameter in the next slot - mstore(add(ptr, 0x20), msgSig) - // Store 4 bytes of callSig parameter in the next slot - mstore(add(ptr, 0x40), callSig) - // Revert 68 bytes of error starting from 0x1c - revert(add(ptr, 0x1c), 0x44) - } - } - } -} From 012efddb0f0e54938c26fe38006bda0013b37154 Mon Sep 17 00:00:00 2001 From: TuDo1403 Date: Mon, 20 Nov 2023 15:12:33 +0700 Subject: [PATCH 3/5] feat: add LibErrorHandler --- src/LibErrorHandler.sol | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 src/LibErrorHandler.sol diff --git a/src/LibErrorHandler.sol b/src/LibErrorHandler.sol new file mode 100644 index 0000000..8202fca --- /dev/null +++ b/src/LibErrorHandler.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +library LibErrorHandler { + /// @dev Reserves error definition to upload to signature database. + error ExternalCallFailed(bytes4 msgSig, bytes4 callSig); + + /// @notice handle low level call revert if call failed, + /// If extcall return empty bytes, reverts with custom error. + /// @param status Status of external call + /// @param callSig function signature of the calldata + /// @param returnOrRevertData bytes result from external call + function handleRevert(bool status, bytes4 callSig, bytes memory returnOrRevertData) internal pure { + // Get the function signature of current context + bytes4 msgSig = msg.sig; + assembly ("memory-safe") { + if iszero(status) { + // Load the length of bytes array + let revertLength := mload(returnOrRevertData) + // Check if length != 0 => revert following reason from external call + if iszero(iszero(revertLength)) { + // Start of revert data bytes. The 0x20 offset is always the same. + revert(add(returnOrRevertData, 0x20), revertLength) + } + + // Load free memory pointer + let ptr := mload(0x40) + // Store 4 bytes the function selector of ExternalCallFailed(msg.sig, callSig) + // Equivalent to revert ExternalCallFailed(bytes4,bytes4) + mstore(ptr, 0x49bf4104) + // Store 4 bytes of msgSig parameter in the next slot + mstore(add(ptr, 0x20), msgSig) + // Store 4 bytes of callSig parameter in the next slot + mstore(add(ptr, 0x40), callSig) + // Revert 68 bytes of error starting from 0x1c + revert(add(ptr, 0x1c), 0x44) + } + } + } +} From 058bb266cc45b96d8724183f0817c030715d609a Mon Sep 17 00:00:00 2001 From: TuDo1403 Date: Mon, 20 Nov 2023 15:55:25 +0700 Subject: [PATCH 4/5] feat: add and happy case unit tests --- foundry.toml | 1 + src/transfers/LibNativeTransfer.sol | 66 ++++++++++++++++++++++++++ test/transfers/LibNativeTransfer.t.sol | 41 ++++++++++++++++ 3 files changed, 108 insertions(+) create mode 100644 src/transfers/LibNativeTransfer.sol create mode 100644 test/transfers/LibNativeTransfer.t.sol diff --git a/foundry.toml b/foundry.toml index 596d69b..db742ed 100644 --- a/foundry.toml +++ b/foundry.toml @@ -7,6 +7,7 @@ ffi = true # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options remappings = [ + 'contract-libs/=src/', 'forge-std/=lib/forge-std/src/', 'ds-test/=lib/forge-std/lib/ds-test/src/', '@openzeppelin/=lib/openzeppelin-contracts/', diff --git a/src/transfers/LibNativeTransfer.sol b/src/transfers/LibNativeTransfer.sol new file mode 100644 index 0000000..ff467c3 --- /dev/null +++ b/src/transfers/LibNativeTransfer.sol @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; +import { LibErrorHandler } from "../LibErrorHandler.sol"; + +using { toAmount } for Gas global; + +enum Gas { + Strictly, + NoGriefing, + ForwardAll, + NoStorageWrite +} + +/// @dev see: https://github.com/axieinfinity/ronin-dpos-contracts/pull/195 +uint256 constant GAS_STIPEND_STRICT = 0; + +/// @dev Suggested gas stipend for contract receiving NATIVE +/// that disallows any storage writes. +uint256 constant GAS_STIPEND_NO_STORAGE_WRITES = 2_300; + +/// @dev Suggested gas stipend for contract receiving NATIVE to perform a few +/// storage reads and writes, but low enough to prevent griefing. +/// Multiply by a small constant (e.g. 2), if needed. +uint256 constant GAS_STIPEND_NO_GRIEF = 100_000; + +function toAmount(Gas gas) view returns (uint256) { + if (gas == Gas.ForwardAll) return gasleft(); + if (gas == Gas.NoGriefing) return GAS_STIPEND_NO_GRIEF; + if (gas == Gas.NoStorageWrite) return GAS_STIPEND_NO_STORAGE_WRITES; + return GAS_STIPEND_STRICT; +} + +/** + * @title NativeTransferHelper + */ +library LibNativeTransfer { + using Strings for *; + using LibErrorHandler for bool; + + /** + * @dev Transfers Native Coin and wraps result for the method caller to a recipient. + */ + function safeTransfer(address to, uint256 value, Gas gas) internal { + (bool success, bytes memory returnOrRevertData) = trySendValue(to, value, gas.toAmount()); + success.handleRevert(bytes4(0x0), returnOrRevertData); + } + + /** + * @dev Unsafe send `amount` Native to the address `to`. If the sender's balance is insufficient, + * the call does not revert. + * + * Note: + * - Does not assert whether the balance of sender is sufficient. + * - Does not assert whether the recipient accepts NATIVE. + * - Consider using `ReentrancyGuard` before calling this function. + * + */ + function trySendValue(address to, uint256 value, uint256 gasAmount) + internal + returns (bool success, bytes memory returnOrRevertData) + { + (success, returnOrRevertData) = to.call{ value: value, gas: gasAmount }(""); + } +} diff --git a/test/transfers/LibNativeTransfer.t.sol b/test/transfers/LibNativeTransfer.t.sol new file mode 100644 index 0000000..5d7aa02 --- /dev/null +++ b/test/transfers/LibNativeTransfer.t.sol @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.23; + +import { Test } from "forge-std/Test.sol"; +import { Gas, LibNativeTransfer } from "contract-libs/transfers/LibNativeTransfer.sol"; + +contract LibNativeTransferTest is Test { + function testFork_RevertWhen_TransferNativeToContractWithoutFallback_safeTransfer( + address any, + uint256 amount, + uint8 v + ) external { + vm.deal(any, amount); + vm.expectRevert(); + vm.prank(any); + LibNativeTransfer.safeTransfer(address(this), amount, _toGas(v)); + } + + function testConcrete_TransferNative(uint8 v) external { + LibNativeTransfer.safeTransfer(address(0xBEEF), 1e18, _toGas(v)); + assertEq(address(0xBEEF).balance, 1e18); + } + + function testFork_TransferNativeToRecipient(address recipient, uint256 amount, uint8 v) external { + // Transferring to msg.sender can fail because it's possible to overflow their ETH balance as it begins non-zero. + if (recipient.code.length > 0 || uint256(uint160(recipient)) <= 18 || recipient == msg.sender) return; + + amount = bound(amount, 0, address(this).balance); + LibNativeTransfer.safeTransfer(recipient, amount, _toGas(v)); + + assertEq(recipient.balance, amount); + } + + function _toGas(uint8 v) internal view returns (Gas gas) { + v = uint8(bound(v, 0, 3)); + if (v == uint8(Gas.Strictly)) gas = Gas.Strictly; + if (v == uint8(Gas.NoGriefing)) gas = Gas.NoGriefing; + if (v == uint8(Gas.ForwardAll)) gas = Gas.ForwardAll; + if (v == uint8(Gas.NoStorageWrite)) gas = Gas.NoStorageWrite; + } +} From 30b5cf64471fab09d973f72d1d5ba055b55e9e87 Mon Sep 17 00:00:00 2001 From: TuDo1403 Date: Mon, 20 Nov 2023 16:13:49 +0700 Subject: [PATCH 5/5] format: reduce complex code --- src/transfers/LibNativeTransfer.sol | 34 ++------------------------ test/transfers/LibNativeTransfer.t.sol | 22 ++++++----------- 2 files changed, 9 insertions(+), 47 deletions(-) diff --git a/src/transfers/LibNativeTransfer.sol b/src/transfers/LibNativeTransfer.sol index ff467c3..0a0dee3 100644 --- a/src/transfers/LibNativeTransfer.sol +++ b/src/transfers/LibNativeTransfer.sol @@ -1,49 +1,19 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { LibErrorHandler } from "../LibErrorHandler.sol"; -using { toAmount } for Gas global; - -enum Gas { - Strictly, - NoGriefing, - ForwardAll, - NoStorageWrite -} - -/// @dev see: https://github.com/axieinfinity/ronin-dpos-contracts/pull/195 -uint256 constant GAS_STIPEND_STRICT = 0; - -/// @dev Suggested gas stipend for contract receiving NATIVE -/// that disallows any storage writes. -uint256 constant GAS_STIPEND_NO_STORAGE_WRITES = 2_300; - -/// @dev Suggested gas stipend for contract receiving NATIVE to perform a few -/// storage reads and writes, but low enough to prevent griefing. -/// Multiply by a small constant (e.g. 2), if needed. -uint256 constant GAS_STIPEND_NO_GRIEF = 100_000; - -function toAmount(Gas gas) view returns (uint256) { - if (gas == Gas.ForwardAll) return gasleft(); - if (gas == Gas.NoGriefing) return GAS_STIPEND_NO_GRIEF; - if (gas == Gas.NoStorageWrite) return GAS_STIPEND_NO_STORAGE_WRITES; - return GAS_STIPEND_STRICT; -} - /** * @title NativeTransferHelper */ library LibNativeTransfer { - using Strings for *; using LibErrorHandler for bool; /** * @dev Transfers Native Coin and wraps result for the method caller to a recipient. */ - function safeTransfer(address to, uint256 value, Gas gas) internal { - (bool success, bytes memory returnOrRevertData) = trySendValue(to, value, gas.toAmount()); + function transfer(address to, uint256 value, uint256 gasAmount) internal { + (bool success, bytes memory returnOrRevertData) = trySendValue(to, value, gasAmount); success.handleRevert(bytes4(0x0), returnOrRevertData); } diff --git a/test/transfers/LibNativeTransfer.t.sol b/test/transfers/LibNativeTransfer.t.sol index 5d7aa02..0a24d69 100644 --- a/test/transfers/LibNativeTransfer.t.sol +++ b/test/transfers/LibNativeTransfer.t.sol @@ -2,40 +2,32 @@ pragma solidity ^0.8.23; import { Test } from "forge-std/Test.sol"; -import { Gas, LibNativeTransfer } from "contract-libs/transfers/LibNativeTransfer.sol"; +import { LibNativeTransfer } from "contract-libs/transfers/LibNativeTransfer.sol"; contract LibNativeTransferTest is Test { function testFork_RevertWhen_TransferNativeToContractWithoutFallback_safeTransfer( address any, uint256 amount, - uint8 v + uint256 gas ) external { vm.deal(any, amount); vm.expectRevert(); vm.prank(any); - LibNativeTransfer.safeTransfer(address(this), amount, _toGas(v)); + LibNativeTransfer.transfer(address(this), amount, gas); } - function testConcrete_TransferNative(uint8 v) external { - LibNativeTransfer.safeTransfer(address(0xBEEF), 1e18, _toGas(v)); + function testConcrete_TransferNative(uint256 gas) external { + LibNativeTransfer.transfer(address(0xBEEF), 1e18, gas); assertEq(address(0xBEEF).balance, 1e18); } - function testFork_TransferNativeToRecipient(address recipient, uint256 amount, uint8 v) external { + function testFork_TransferNativeToRecipient(address recipient, uint256 amount, uint256 gas) external { // Transferring to msg.sender can fail because it's possible to overflow their ETH balance as it begins non-zero. if (recipient.code.length > 0 || uint256(uint160(recipient)) <= 18 || recipient == msg.sender) return; amount = bound(amount, 0, address(this).balance); - LibNativeTransfer.safeTransfer(recipient, amount, _toGas(v)); + LibNativeTransfer.transfer(recipient, amount, gas); assertEq(recipient.balance, amount); } - - function _toGas(uint8 v) internal view returns (Gas gas) { - v = uint8(bound(v, 0, 3)); - if (v == uint8(Gas.Strictly)) gas = Gas.Strictly; - if (v == uint8(Gas.NoGriefing)) gas = Gas.NoGriefing; - if (v == uint8(Gas.ForwardAll)) gas = Gas.ForwardAll; - if (v == uint8(Gas.NoStorageWrite)) gas = Gas.NoStorageWrite; - } }