From 6101d4c0f459f5df435399fd62749096a425afff Mon Sep 17 00:00:00 2001 From: smol-ninja Date: Fri, 18 Oct 2024 12:22:08 +0100 Subject: [PATCH] fix: depletionTimeOf calculation (#317) * fix: depletionTimeOf calculation * perf: DRY'ify oneWeiScaled vriable * test: revert WARP_SOLVENCY_PERIOD and declare expectedDepletionTime in DepletionTimeOf integration test * chore: remove carry variable to avoid stack too deep error on lite profile * chore: rename oneWeiScaled to oneMVTScaled * chore: rename wei to mvt in invariant * docs: defining MVT --- src/SablierFlow.sol | 31 ++++++--- src/interfaces/ISablierFlow.sol | 4 +- .../depletion-time-of/depletionTimeOf.t.sol | 54 ++++++++++++--- .../depletion-time-of/depletionTimeOf.tree | 5 +- tests/integration/fuzz/depletionTimeOf.t.sol | 69 ++++++++++--------- tests/invariant/Flow.t.sol | 2 +- tests/utils/Constants.sol | 4 +- 7 files changed, 109 insertions(+), 60 deletions(-) diff --git a/src/SablierFlow.sol b/src/SablierFlow.sol index 5dec47a8..55c4103d 100644 --- a/src/SablierFlow.sol +++ b/src/SablierFlow.sol @@ -71,24 +71,35 @@ contract SablierFlow is uint8 tokenDecimals = _streams[streamId].tokenDecimals; uint256 balanceScaled = Helpers.scaleAmount({ amount: balance, decimals: tokenDecimals }); - uint256 snapshotDebtScaled = _streams[streamId].snapshotDebtScaled; - // If the stream has uncovered debt, return zero. - if (snapshotDebtScaled + _ongoingDebtScaledOf(streamId) > balanceScaled) { + // MVT represents Minimum Value Transferable, the smallest amount of token that can be transferred, which is + // always 1 in token's decimal. + uint256 oneMVTScaled = Helpers.scaleAmount({ amount: 1, decimals: tokenDecimals }); + + // If the total debt exceeds balance, return zero. + if (snapshotDebtScaled + _ongoingDebtScaledOf(streamId) >= balanceScaled + oneMVTScaled) { return 0; } - // Depletion time is defined as the UNIX timestamp beyond which the total debt exceeds stream balance. - // So we calculate it by solving: debt at depletion time = stream balance + 1. This ensures that we find the - // lowest timestamp at which the debt exceeds the balance. + uint256 ratePerSecond = _streams[streamId].ratePerSecond.unwrap(); + + // Depletion time is defined as the UNIX timestamp at which the total debt exceeds stream balance by 1 unit of + // token (mvt). So we calculate it by solving: total debt at depletion time = stream balance + 1. This ensures + // that we find the lowest timestamp at which the total debt exceeds the stream balance. // Safe to use unchecked because the calculations cannot overflow or underflow. unchecked { - uint256 solvencyAmount = - balanceScaled - snapshotDebtScaled + Helpers.scaleAmount({ amount: 1, decimals: tokenDecimals }); - uint256 solvencyPeriod = solvencyAmount / _streams[streamId].ratePerSecond.unwrap(); + uint256 solvencyAmount = balanceScaled - snapshotDebtScaled + oneMVTScaled; + uint256 solvencyPeriod = solvencyAmount / ratePerSecond; - depletionTime = _streams[streamId].snapshotTime + solvencyPeriod; + // If the division is exact, return the depletion time. + if (solvencyAmount % ratePerSecond == 0) { + depletionTime = _streams[streamId].snapshotTime + solvencyPeriod; + } + // Otherwise, round up before returning since the division by rate per second has round down the result. + else { + depletionTime = _streams[streamId].snapshotTime + solvencyPeriod + 1; + } } } diff --git a/src/interfaces/ISablierFlow.sol b/src/interfaces/ISablierFlow.sol index da601c9e..1e6d3b06 100644 --- a/src/interfaces/ISablierFlow.sol +++ b/src/interfaces/ISablierFlow.sol @@ -115,8 +115,8 @@ interface ISablierFlow is /// @param streamId The stream ID for the query. function coveredDebtOf(uint256 streamId) external view returns (uint128 coveredDebt); - /// @notice Returns the time at which the stream will deplete its balance and start to accumulate uncovered debt. If - /// there already is uncovered debt, it returns zero. + /// @notice Returns the time at which the total debt exceeds stream balance. If the total debt is less than + /// or equal to stream balance, it returns 0. /// @dev Reverts if `streamId` references a paused or a null stream. /// @param streamId The stream ID for the query. function depletionTimeOf(uint256 streamId) external view returns (uint256 depletionTime); diff --git a/tests/integration/concrete/depletion-time-of/depletionTimeOf.t.sol b/tests/integration/concrete/depletion-time-of/depletionTimeOf.t.sol index 35caf97d..8b52f10a 100644 --- a/tests/integration/concrete/depletion-time-of/depletionTimeOf.t.sol +++ b/tests/integration/concrete/depletion-time-of/depletionTimeOf.t.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.22; +import { UD21x18 } from "@prb/math/src/UD21x18.sol"; + import { Integration_Test } from "../../Integration.t.sol"; contract DepletionTimeOf_Integration_Concrete_Test is Integration_Test { @@ -15,21 +17,51 @@ contract DepletionTimeOf_Integration_Concrete_Test is Integration_Test { } function test_GivenBalanceZero() external view givenNotNull givenNotPaused { - // It should return 0 - uint256 depletionTime = flow.depletionTimeOf(defaultStreamId); - assertEq(depletionTime, 0, "depletion time"); + // It should return 0. + uint256 actualDepletionTime = flow.depletionTimeOf(defaultStreamId); + assertEq(actualDepletionTime, 0, "depletion time"); } function test_GivenUncoveredDebt() external givenNotNull givenNotPaused givenBalanceNotZero { - vm.warp({ newTimestamp: WARP_SOLVENCY_PERIOD + 1 }); - // It should return 0 - uint256 depletionTime = flow.depletionTimeOf(defaultStreamId); - assertEq(depletionTime, 0, "depletion time"); + uint256 depletionTimestamp = WARP_SOLVENCY_PERIOD + 1; + vm.warp({ newTimestamp: depletionTimestamp }); + + // Check that uncovered debt is greater than 0. + assertGt(flow.uncoveredDebtOf(defaultStreamId), 0); + + // It should return 0. + uint256 actualDepletionTime = flow.depletionTimeOf(defaultStreamId); + assertEq(actualDepletionTime, 0, "depletion time"); + } + + modifier givenNoUncoveredDebt() { + _; + } + + function test_WhenExactDivision() external givenNotNull givenNotPaused givenBalanceNotZero givenNoUncoveredDebt { + // Create a stream with a rate per second such that the deposit amount produces no remainder when divided by the + // rate per second. + UD21x18 rps = UD21x18.wrap(2e18); + uint256 streamId = createDefaultStream(rps, usdc); + depositDefaultAmount(streamId); + uint256 solvencyPeriod = DEPOSIT_AMOUNT_18D / rps.unwrap(); + + // It should return the time at which the total debt exceeds the balance. + uint40 actualDepletionTime = uint40(flow.depletionTimeOf(streamId)); + uint40 exptectedDepletionTime = WARP_ONE_MONTH + uint40(solvencyPeriod + 1); + assertEq(actualDepletionTime, exptectedDepletionTime, "depletion time"); } - function test_GivenNoUncoveredDebt() external givenNotNull givenNotPaused givenBalanceNotZero { - // It should return the time at which the stream depletes its balance - uint40 depletionTime = uint40(flow.depletionTimeOf(defaultStreamId)); - assertEq(depletionTime, WARP_SOLVENCY_PERIOD, "depletion time"); + function test_WhenNotExactDivision() + external + givenNotNull + givenNotPaused + givenBalanceNotZero + givenNoUncoveredDebt + { + // It should return the time at which the total debt exceeds the balance. + uint40 actualDepletionTime = uint40(flow.depletionTimeOf(defaultStreamId)); + uint256 expectedDepletionTime = WARP_SOLVENCY_PERIOD + 1; + assertEq(actualDepletionTime, expectedDepletionTime, "depletion time"); } } diff --git a/tests/integration/concrete/depletion-time-of/depletionTimeOf.tree b/tests/integration/concrete/depletion-time-of/depletionTimeOf.tree index 35c32dcf..3efa2717 100644 --- a/tests/integration/concrete/depletion-time-of/depletionTimeOf.tree +++ b/tests/integration/concrete/depletion-time-of/depletionTimeOf.tree @@ -11,4 +11,7 @@ DepletionTimeOf_Integration_Concrete_Test ├── given uncovered debt │ └── it should return 0 └── given no uncovered debt - └── it should return the time at which the stream depletes its balance \ No newline at end of file + ├── when exact division + │ └── it should return the time at which the total debt exceeds the balance + └── when not exact division + └── it should return the time at which the total debt exceeds the balance diff --git a/tests/integration/fuzz/depletionTimeOf.t.sol b/tests/integration/fuzz/depletionTimeOf.t.sol index 0d47647f..270ba2ac 100644 --- a/tests/integration/fuzz/depletionTimeOf.t.sol +++ b/tests/integration/fuzz/depletionTimeOf.t.sol @@ -5,50 +5,51 @@ import { Shared_Integration_Fuzz_Test } from "./Fuzz.t.sol"; contract DepletionTimeOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { /// @dev Checklist: - /// - It should return 0 if the time has already passed the solvency period. - /// - It should return a non-zero value if the time has not yet passed the solvency period. + /// - It should return a non-zero value if the current time is less than the depletion timestamp. + /// - It should return 0 if the current time is equal to or greater than the depletion timestamp. /// /// Given enough runs, all of the following scenarios should be fuzzed: /// - Multiple streams, each with different rate per second and decimals. - /// - Multiple points in time, both pre-depletion and post-depletion. - function testFuzz_DepletionTimeOf( - uint256 streamId, - uint40 timeJump, - uint8 decimals - ) - external - givenNotNull - givenPaused - { + function testFuzz_DepletionTimeOf(uint256 streamId, uint8 decimals) external givenNotNull givenPaused { (streamId, decimals,) = useFuzzedStreamOrCreate(streamId, decimals); // Calculate the solvency period based on the stream deposit. uint256 solvencyPeriod = getScaledAmount(flow.getBalance(streamId) + 1, decimals) / flow.getRatePerSecond(streamId).unwrap(); - - // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); - - // Simulate the passage of time. - vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); + uint256 carry = + getScaledAmount(flow.getBalance(streamId) + 1, decimals) % flow.getRatePerSecond(streamId).unwrap(); // Assert that depletion time equals expected value. uint256 actualDepletionTime = flow.depletionTimeOf(streamId); - if (getBlockTimestamp() >= OCT_1_2024 + solvencyPeriod) { - assertEq(actualDepletionTime, 0, "depletion time"); - - // Assert that uncovered debt is greater than 0. - assertGt(flow.uncoveredDebtOf(streamId), 0, "uncovered debt post depletion time"); - } else { - assertEq(actualDepletionTime, OCT_1_2024 + solvencyPeriod, "depletion time"); - - // Assert that uncovered debt is zero at depletion time. - vm.warp({ newTimestamp: actualDepletionTime }); - assertEq(flow.uncoveredDebtOf(streamId), 0, "uncovered debt before depletion time"); - - // Assert that uncovered debt is greater than 0 right after depletion time. - vm.warp({ newTimestamp: actualDepletionTime + 1 }); - assertGt(flow.uncoveredDebtOf(streamId), 0, "uncovered debt after depletion time"); - } + uint256 expectedDepletionTime = carry > 0 ? OCT_1_2024 + solvencyPeriod + 1 : OCT_1_2024 + solvencyPeriod; + assertEq(actualDepletionTime, expectedDepletionTime, "depletion time"); + + // Warp time to 1 second before the depletion timestamp. + vm.warp({ newTimestamp: actualDepletionTime - 1 }); + // Assert that total debt does not exceed the stream balance before depletion time. + assertLe( + flow.totalDebtOf(streamId), flow.getBalance(streamId), "pre-depletion period: total debt exceeds balance" + ); + assertLe(flow.depletionTimeOf(streamId), getBlockTimestamp() + 1, "depletion time 1 second in future"); + + // Warp time to the depletion timestamp. + vm.warp({ newTimestamp: actualDepletionTime }); + // Assert that total debt exceeds the stream balance at depletion time. + assertGt( + flow.totalDebtOf(streamId), + flow.getBalance(streamId), + "at depletion time: total debt does not exceed balance" + ); + assertEq(flow.depletionTimeOf(streamId), 0, "non-zero depletion time at depletion timestamp"); + + // Warp time to 1 second after the depletion timestamp. + vm.warp({ newTimestamp: actualDepletionTime + 1 }); + // Assert that total debt exceeds the stream balance after depletion time. + assertGt( + flow.totalDebtOf(streamId), + flow.getBalance(streamId), + "post-depletion time: total debt does not exceed balance" + ); + assertEq(flow.depletionTimeOf(streamId), 0, "non-zero depletion time after depletion timestamp"); } } diff --git a/tests/invariant/Flow.t.sol b/tests/invariant/Flow.t.sol index 060bd290..5983bb4a 100644 --- a/tests/invariant/Flow.t.sol +++ b/tests/invariant/Flow.t.sol @@ -333,7 +333,7 @@ contract Flow_Invariant_Test is Base_Test { } /// @dev For non-voided streams, the expected streamed amount should be greater than or equal to the sum of total - /// debt and withdrawn amount. And, the difference between the two should not exceed 10 wei. + /// debt and withdrawn amount. And, the difference between the two should not exceed 10 mvt. function invariant_TotalStreamedEqTotalDebtPlusWithdrawn() external view { uint256 lastStreamId = flowStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ++i) { diff --git a/tests/utils/Constants.sol b/tests/utils/Constants.sol index b559a7a6..35ed05e9 100644 --- a/tests/utils/Constants.sol +++ b/tests/utils/Constants.sol @@ -44,8 +44,10 @@ abstract contract Constants { // Time uint40 internal constant OCT_1_2024 = 1_727_740_800; uint40 internal constant ONE_MONTH = 30 days; // "30/360" convention - uint40 internal constant SOLVENCY_PERIOD = uint40(DEPOSIT_AMOUNT_18D / RATE_PER_SECOND_U128); // 578 days + // Solvency period is 49999999.999999 seconds. + uint40 internal constant SOLVENCY_PERIOD = uint40(DEPOSIT_AMOUNT_18D / RATE_PER_SECOND_U128); // ~578 days uint40 internal constant WARP_ONE_MONTH = OCT_1_2024 + ONE_MONTH; + // The following variable represents the timestamp at which the stream depletes all its balance. uint40 internal constant WARP_SOLVENCY_PERIOD = OCT_1_2024 + SOLVENCY_PERIOD; uint40 internal constant WITHDRAW_TIME = OCT_1_2024 + 2_500_000; }