From 058bb266cc45b96d8724183f0817c030715d609a Mon Sep 17 00:00:00 2001 From: TuDo1403 Date: Mon, 20 Nov 2023 15:55:25 +0700 Subject: [PATCH 1/2] 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 2/2] 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; - } }