From dd3c1ff2f37e3ba451f86b761342b7df3f91c44b Mon Sep 17 00:00:00 2001 From: smol-ninja Date: Mon, 11 Nov 2024 20:18:42 +0000 Subject: [PATCH] feat: round up protocol fee if it rounds down to 0 --- src/interfaces/ISablierFlow.sol | 4 +- src/libraries/Helpers.sol | 13 ++++++- .../concrete/withdraw/withdraw.t.sol | 39 ++++++++++++++++++- .../concrete/withdraw/withdraw.tree | 28 +++++++------ tests/integration/fuzz/withdraw.t.sol | 1 + tests/utils/Modifiers.sol | 4 ++ 6 files changed, 73 insertions(+), 16 deletions(-) diff --git a/src/interfaces/ISablierFlow.sol b/src/interfaces/ISablierFlow.sol index 41feb991..2e2ce15d 100644 --- a/src/interfaces/ISablierFlow.sol +++ b/src/interfaces/ISablierFlow.sol @@ -413,7 +413,9 @@ interface ISablierFlow is /// /// Notes: /// - It sets the snapshot time to the `block.timestamp` if `amount` is greater than snapshot debt. - /// - A protocol fee may be charged on the withdrawn amount if the protocol fee is enabled for the streaming token. + /// - If the protocol fee is enabled for the streaming token: + /// - A percentage based protocol fee will be charged on the withdrawn amount. + /// - If withdrawn amount is so small that the protocol fee rounds down to zero, a fee of 1 token will be charged. /// /// Requirements: /// - Must not be delegate called. diff --git a/src/libraries/Helpers.sol b/src/libraries/Helpers.sol index eac02995..1e4f96df 100644 --- a/src/libraries/Helpers.sol +++ b/src/libraries/Helpers.sol @@ -21,6 +21,12 @@ library Helpers { // Calculate the fee amount based on the fee percentage. feeAmount = ud(totalAmount).mul(fee).intoUint128(); + // If `totalAmount` is so small that the `feeAmount` is rounded down to zero, set the fee to 1. This avoids + // exploiting the protocol fee by withdrawing very small amounts. + if (feeAmount == 0) { + feeAmount = 1; + } + // Calculate the net amount after subtracting the fee from the total amount. netAmount = totalAmount - feeAmount; } @@ -46,8 +52,11 @@ library Helpers { revert Errors.SablierFlow_BrokerAddressZero(); } - // Calculate the broker fee amount that is going to be transferred to the `broker.account`. - (brokerFeeAmount, depositAmount) = calculateAmountsFromFee(totalAmount, broker.fee); + // Calculate the broker fee amount. + brokerFeeAmount = ud(totalAmount).mul(broker.fee).intoUint128(); + + // Calculate the net amount after subtracting the broker fee from the total amount. + depositAmount = totalAmount - brokerFeeAmount; } /// @dev Descales the provided `amount` from 18 decimals fixed-point number to token's decimals number. diff --git a/tests/integration/concrete/withdraw/withdraw.t.sol b/tests/integration/concrete/withdraw/withdraw.t.sol index 41899723..993d255a 100644 --- a/tests/integration/concrete/withdraw/withdraw.t.sol +++ b/tests/integration/concrete/withdraw/withdraw.t.sol @@ -179,7 +179,7 @@ contract Withdraw_Integration_Concrete_Test is Integration_Test { }); } - function test_GivenProtocolFeeNotZero() + function test_WhenFeeAmountRoundsDownToZero() external whenNoDelegateCall givenNotNull @@ -188,6 +188,43 @@ contract Withdraw_Integration_Concrete_Test is Integration_Test { whenWithdrawalAddressOwner whenAmountNotOverdraw whenAmountEqualTotalDebt + givenProtocolFeeNotZero + { + // Go back to the starting point. + vm.warp({ newTimestamp: OCT_1_2024 }); + + resetPrank({ msgSender: users.sender }); + + // Create the stream and make a deposit. + uint256 streamId = createDefaultStream(tokenWithProtocolFee); + deposit(streamId, DEPOSIT_AMOUNT_6D); + + // Simulate the one month of streaming. + vm.warp({ newTimestamp: WARP_ONE_MONTH }); + + // Make recipient the caller test. + resetPrank({ msgSender: users.recipient }); + + // It should deduct 1 token as protocol fee + _test_Withdraw({ + streamId: streamId, + to: users.recipient, + depositAmount: DEPOSIT_AMOUNT_6D, + protocolFeeAmount: 1, + withdrawAmount: 99 + }); + } + + function test_WhenFeeAmountNotRoundDownToZero() + external + whenNoDelegateCall + givenNotNull + whenAmountNotZero + whenWithdrawalAddressNotZero + whenWithdrawalAddressOwner + whenAmountNotOverdraw + whenAmountEqualTotalDebt + givenProtocolFeeNotZero { // Go back to the starting point. vm.warp({ newTimestamp: OCT_1_2024 }); diff --git a/tests/integration/concrete/withdraw/withdraw.tree b/tests/integration/concrete/withdraw/withdraw.tree index 8b9601ed..e4a76858 100644 --- a/tests/integration/concrete/withdraw/withdraw.tree +++ b/tests/integration/concrete/withdraw/withdraw.tree @@ -34,16 +34,20 @@ Withdraw_Integration_Concrete_Test │ └── it should make the withdrawal └── when amount exceed snapshot debt ├── given protocol fee not zero - │ ├── it should update the protocol revenue - │ └── it should withdraw the net amount + │ ├── when fee amount rounds down to zero + │ │ ├── it should update the protocol revenue by 1 + │ │ └── it should make the withdrawal + │ └── when fee amount not round down to zero + │ ├── it should update the protocol revenue + │ └── it should withdraw the net amount └── given protocol fee zero - ├── given token has 18 decimals - │ └── it should make the withdrawal - └── given token not have 18 decimals - ├── it should withdraw the withdrawable amount - ├── it should reduce the stream balance by the withdrawn amount - ├── it should reduce the aggregate amount by the withdrawn amount - ├── it should update snapshot debt to zero - ├── it should update snapshot time - ├── it should emit 1 {Transfer}, 1 {WithdrawFromFlowStream} and 1 {MetadataUpdated} events - └── it should return the withdrawn amount + ├── given token has 18 decimals + │ └── it should make the withdrawal + └── given token not have 18 decimals + ├── it should withdraw the withdrawable amount + ├── it should reduce the stream balance by the withdrawn amount + ├── it should reduce the aggregate amount by the withdrawn amount + ├── it should update snapshot debt to zero + ├── it should update snapshot time + ├── it should emit 1 {Transfer}, 1 {WithdrawFromFlowStream} and 1 {MetadataUpdated} events + └── it should return the withdrawn amount diff --git a/tests/integration/fuzz/withdraw.t.sol b/tests/integration/fuzz/withdraw.t.sol index 52f1f13b..ace33739 100644 --- a/tests/integration/fuzz/withdraw.t.sol +++ b/tests/integration/fuzz/withdraw.t.sol @@ -156,6 +156,7 @@ contract Withdraw_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { vars.expectedProtocolRevenue = flow.protocolRevenue(token); if (flow.protocolFee(token) > ZERO) { vars.protocolFeeAmount = ud(withdrawAmount).mul(flow.protocolFee(token)).intoUint128(); + if (vars.protocolFeeAmount == 0) vars.protocolFeeAmount = 1; vars.expectedProtocolRevenue += vars.protocolFeeAmount; } diff --git a/tests/utils/Modifiers.sol b/tests/utils/Modifiers.sol index 694b8a49..ab431b7b 100644 --- a/tests/utils/Modifiers.sol +++ b/tests/utils/Modifiers.sol @@ -130,6 +130,10 @@ abstract contract Modifiers { WITHDRAW //////////////////////////////////////////////////////////////////////////*/ + modifier givenProtocolFeeNotZero() { + _; + } + modifier givenProtocolFeeZero() { _; }