From a0dab5bb18ebb2e0923482a16d04c8412cc4743e Mon Sep 17 00:00:00 2001 From: smol-ninja Date: Mon, 9 Sep 2024 14:54:50 +0100 Subject: [PATCH] fix: bug in snapshot time in the presence of low rps (#220) * fix: discrepancy in time value during withdrawAt test: lower rps bound in Utils fix: precision loss in time due to denormalization in ongoing debt test: consistencies in assert messages in fork tests testL fix bugs in fork tests refactor: remove redundant internal functions perf: check the corrected time is less than snapshot time test: add an invariant that snapshot debt should always increase if updated chore: fix typos refactor: remove unneeded struct refactor: remove redundant correctedTime checks fix fuzz tests rename originalTime to time fix bug in depletion time docs: polish chore: remove SafeCast from Helpers Co-authored-by: Andrei Vlad Birgaoanu <99738872+andreivladbrg@users.noreply.github.com> * test: declare struct to fix stack too deep error * refactor: rename vars * docs: polish * docs: polish natspec for _ongoingDebt Co-authored-by: Paul Razvan Berg * docs: polish natspec in depletionTimeOf Co-authored-by: Andrei Vlad Birgaoanu <99738872+andreivladbrg@users.noreply.github.com> * fix: return ongoing debt as 0 when renormalizedOngoingDebt < ratePerSecond Co-authored-by: Andrei Vlad Birgaoanu <99738872+andreivladbrg@users.noreply.github.com> --------- Co-authored-by: Andrei Vlad Birgaoanu <99738872+andreivladbrg@users.noreply.github.com> Co-authored-by: Paul Razvan Berg --- .github/workflows/ci.yml | 4 +- README.md | 2 + foundry.toml | 2 +- src/SablierFlow.sol | 122 +++++++++---- src/interfaces/ISablierFlow.sol | 4 +- src/libraries/Helpers.sol | 37 ---- test/fork/Flow.t.sol | 164 +++++++++++------- test/integration/Integration.t.sol | 8 +- test/integration/concrete/batch/batch.t.sol | 4 +- .../ongoing-debt-of/ongoingDebtOf.t.sol | 4 +- .../concrete/withdraw-at/withdrawAt.t.sol | 6 +- .../concrete/withdraw-max/withdrawMax.t.sol | 2 +- test/integration/fuzz/coveredDebtOf.t.sol | 10 +- test/integration/fuzz/depletionTimeOf.t.sol | 16 +- test/integration/fuzz/ongoingDebtOf.t.sol | 8 +- test/integration/fuzz/refund.t.sol | 6 +- .../integration/fuzz/refundableAmountOf.t.sol | 10 +- test/integration/fuzz/withdrawAt.t.sol | 103 +++++++---- test/integration/fuzz/withdrawMax.t.sol | 24 ++- test/invariant/Flow.t.sol | 13 ++ test/invariant/handlers/FlowHandler.sol | 4 + test/utils/Events.sol | 2 +- test/utils/Utils.sol | 21 ++- 23 files changed, 358 insertions(+), 218 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01f9b4cc..6e9b09d6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,7 @@ jobs: needs: ["lint", "build"] uses: "sablier-labs/reusable-workflows/.github/workflows/forge-test.yml@main" with: - foundry-fuzz-runs: 5000 + foundry-fuzz-runs: 50000 foundry-profile: "test-optimized" match-path: "test/integration/**/*.sol" name: "Integration tests" @@ -39,7 +39,7 @@ jobs: uses: "sablier-labs/reusable-workflows/.github/workflows/forge-test.yml@main" with: foundry-invariant-depth: 100 - foundry-invariant-runs: 100 + foundry-invariant-runs: 500 foundry-profile: "test-optimized" match-path: "test/invariant/**/*.sol" name: "Invariant tests" diff --git a/README.md b/README.md index 4d005d6e..6d146c82 100644 --- a/README.md +++ b/README.md @@ -178,3 +178,5 @@ Currently, it's not possible to address this precision problem entirely. 12. if $isPaused = true \implies rps = 0$ 13. if $isVoided = true \implies isPaused = true$, $ra = 0$ and $ud = 0$ + +14. snapshot time should never decrease diff --git a/foundry.toml b/foundry.toml index eb0d1e7c..04a6bce9 100644 --- a/foundry.toml +++ b/foundry.toml @@ -20,7 +20,7 @@ [profile.default.fuzz] max_test_rejects = 1_000_000 # Number of times `vm.assume` can fail - runs = 20 + runs = 5000 [profile.default.invariant] call_override = false # Override unsafe external calls to perform reentrancy checks diff --git a/src/SablierFlow.sol b/src/SablierFlow.sol index cafcbf16..0198e982 100644 --- a/src/SablierFlow.sol +++ b/src/SablierFlow.sol @@ -5,6 +5,7 @@ import { IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/I import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol"; +import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import { ud21x18, UD21x18 } from "@prb/math/src/UD21x18.sol"; import { UD60x18, ZERO } from "@prb/math/src/UD60x18.sol"; @@ -25,6 +26,7 @@ contract SablierFlow is ISablierFlow, // 4 inherited components SablierFlowBase // 8 inherited components { + using SafeCast for uint256; using SafeERC20 for IERC20; /*////////////////////////////////////////////////////////////////////////// @@ -67,26 +69,36 @@ contract SablierFlow is return 0; } - // Calculate here the total debt for gas efficiency. - uint128 totalDebt = _streams[streamId].snapshotDebt + _ongoingDebtOf(streamId, uint40(block.timestamp)); + (uint128 ongoingDebt,) = _ongoingDebtOf(streamId, uint40(block.timestamp)); + uint128 snapshotDebt = _streams[streamId].snapshotDebt; + uint128 totalDebt = snapshotDebt + ongoingDebt; // If the stream has debt, return zero. if (totalDebt >= balance) { return 0; } + uint8 tokenDecimals = _streams[streamId].tokenDecimals; + uint128 solvencyAmount; + + // 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. // Safe to use unchecked because the calculations cannot overflow or underflow. unchecked { - // The solvency amount is normalized to 18 decimals since it is divided by the rate per second. - uint128 solvencyAmount = Helpers.normalizeAmount(balance - totalDebt, _streams[streamId].tokenDecimals); + if (tokenDecimals == 18) { + solvencyAmount = (balance - snapshotDebt + 1); + } else { + solvencyAmount = ((balance - snapshotDebt + 1) * (10 ** (18 - tokenDecimals))).toUint128(); + } uint128 solvencyPeriod = solvencyAmount / _streams[streamId].ratePerSecond.unwrap(); - depletionTime = uint40(block.timestamp + solvencyPeriod); + return _streams[streamId].snapshotTime + uint40(solvencyPeriod); } } /// @inheritdoc ISablierFlow function ongoingDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint128 ongoingDebt) { - ongoingDebt = _ongoingDebtOf(streamId, uint40(block.timestamp)); + (ongoingDebt,) = _ongoingDebtOf(streamId, uint40(block.timestamp)); } /// @inheritdoc ISablierFlow @@ -439,13 +451,33 @@ contract SablierFlow is return totalDebt; } - /// @dev Calculates the ongoing debt accrued since last snapshot. Return 0 if the stream is paused. - function _ongoingDebtOf(uint256 streamId, uint40 time) internal view returns (uint128) { + /// @dev When token decimal is less than 18, the `_ongoingDebtOf` function can be non-injective with respect to its + /// input parameter `time` due to the denormalization. This results into a time range [t, t+δ] during which it + /// would produce the same value. Thus, to minimise any loss to the users, this function returns a corrected time + /// which is the lower bound, t in the range [t, t+δ]. The corrected time is the nearest Unix timestamp that + /// produces the smallest remainder greater than the minimum transferable value. This corrected time is then stored + /// as the snapshot time. + /// + /// @param streamId The ID of the stream. + /// @param time The time to calculate the ongoing debt. + /// + /// @return ongoingDebt The denormalized ongoing debt accrued since the last snapshot. Return 0 if the stream is + /// paused or `time` is less than the snapshot time. + /// @return correctedTime The corrected time derived from the denormalized ongoing debt. Return `time` if + /// the stream is paused or is less than the snapshot time. + function _ongoingDebtOf( + uint256 streamId, + uint40 time + ) + internal + view + returns (uint128 ongoingDebt, uint40 correctedTime) + { uint40 snapshotTime = _streams[streamId].snapshotTime; - // If the stream is paused or the time is less than the `snapshotTime`, return zero. + // Check: if the stream is paused or the `time` is less than the `snapshotTime`. if (_streams[streamId].isPaused || time <= snapshotTime) { - return 0; + return (0, time); } uint128 elapsedTime; @@ -456,11 +488,37 @@ contract SablierFlow is elapsedTime = time - snapshotTime; } + uint128 ratePerSecond = _streams[streamId].ratePerSecond.unwrap(); + uint8 tokenDecimals = _streams[streamId].tokenDecimals; + // Calculate the ongoing debt accrued by multiplying the elapsed time by the rate per second. - uint128 normalizedAmount = elapsedTime * _streams[streamId].ratePerSecond.unwrap(); + uint128 normalizedOngoingDebt = elapsedTime * ratePerSecond; + + // If the token decimals are 18, return the normalized ongoing debt and the time. + if (tokenDecimals == 18) { + return (normalizedOngoingDebt, time); + } + + // Safe to use unchecked because we use {SafeCast}. + unchecked { + uint8 factor = 18 - tokenDecimals; + // Since debt is denoted in token decimals, denormalize the amount. + ongoingDebt = (normalizedOngoingDebt / (10 ** factor)).toUint128(); + + // Renormalize the ongoing debt for the calculation of corrected time. + uint128 renormalizedOngoingDebt = (ongoingDebt * (10 ** factor)).toUint128(); - // Since amount values are represented in token decimals, denormalize the amount before returning. - return Helpers.denormalizeAmount(normalizedAmount, _streams[streamId].tokenDecimals); + // Return 0 if renormalized ongoing debt is less than `ratePerSecond`. This eliminates the risk of leaking + // funds when renormalized debt is less than rate per second. + if (renormalizedOngoingDebt < ratePerSecond) { + return (0, snapshotTime); + } + + // Derive the corrected time from the renormalized ongoing debt. + correctedTime = uint40(snapshotTime + renormalizedOngoingDebt / ratePerSecond); + } + + return (ongoingDebt, correctedTime); } /// @dev Calculates the refundable amount. @@ -473,7 +531,7 @@ contract SablierFlow is /// stream's balance. function _totalDebtOf(uint256 streamId, uint40 time) internal view returns (uint128) { // Calculate the ongoing debt streamed since last snapshot. - uint128 ongoingDebt = _ongoingDebtOf(streamId, time); + (uint128 ongoingDebt,) = _ongoingDebtOf(streamId, time); // Calculate the total debt. return _streams[streamId].snapshotDebt + ongoingDebt; @@ -510,15 +568,18 @@ contract SablierFlow is revert Errors.SablierFlow_RatePerSecondNotDifferent(streamId, newRatePerSecond); } + // Calculate the ongoing debt and corrected time. + (uint128 ongoingDebt, uint40 correctedTime) = _ongoingDebtOf(streamId, uint40(block.timestamp)); + // Effect: update the snapshot debt. - _updateSnapshotDebt(streamId); + _streams[streamId].snapshotDebt += ongoingDebt; + + // Effect: update the snapshot time. + _streams[streamId].snapshotTime = correctedTime; // Effect: set the new rate per second. _streams[streamId].ratePerSecond = newRatePerSecond; - // Effect: update the stream time. - _updateSnapshotTime({ streamId: streamId, time: uint40(block.timestamp) }); - // Log the adjustment. emit ISablierFlow.AdjustFlowStream({ streamId: streamId, @@ -627,7 +688,8 @@ contract SablierFlow is /// @dev See the documentation for the user-facing functions that call this internal function. function _pause(uint256 streamId) internal { // Effect: update the snapshot debt. - _updateSnapshotDebt(streamId); + (uint128 ongoingDebt,) = _ongoingDebtOf(streamId, uint40(block.timestamp)); + _streams[streamId].snapshotDebt += ongoingDebt; // Effect: set the rate per second to zero. _streams[streamId].ratePerSecond = ud21x18(0); @@ -696,23 +758,12 @@ contract SablierFlow is _streams[streamId].isPaused = false; // Effect: update the snapshot time. - _updateSnapshotTime(streamId, uint40(block.timestamp)); + _streams[streamId].snapshotTime = uint40(block.timestamp); // Log the restart. emit ISablierFlow.RestartFlowStream(streamId, msg.sender, ratePerSecond); } - /// @dev Update the snapshot debt by adding the ongoing debt streamed since the snapshot time. - function _updateSnapshotDebt(uint256 streamId) internal { - // Effect: update the snapshot debt. - _streams[streamId].snapshotDebt += _ongoingDebtOf(streamId, uint40(block.timestamp)); - } - - /// @dev Updates the `snapshotTime` to the specified time. - function _updateSnapshotTime(uint256 streamId, uint40 time) internal { - _streams[streamId].snapshotTime = time; - } - /// @dev Voids a stream that has uncovered debt. function _void(uint256 streamId) internal { uint128 debtToWriteOff = _uncoveredDebtOf(streamId); @@ -772,7 +823,8 @@ contract SablierFlow is revert Errors.SablierFlow_WithdrawNoFundsAvailable(streamId); } - uint128 totalDebt = _totalDebtOf(streamId, time); + (uint128 ongoingDebt, uint40 correctedTime) = _ongoingDebtOf(streamId, time); + uint128 totalDebt = _streams[streamId].snapshotDebt + ongoingDebt; // If there is debt, the withdraw amount is the balance, and the snapshot debt is updated so that we // don't lose track of the debt. @@ -794,7 +846,7 @@ contract SablierFlow is } // Effect: update the stream time. - _updateSnapshotTime(streamId, time); + _streams[streamId].snapshotTime = correctedTime; // Load the variables in memory. IERC20 token = _streams[streamId].token; @@ -818,7 +870,7 @@ contract SablierFlow is _streams[streamId].balance -= totalWithdrawAmount; // Interaction: perform the ERC-20 transfer. - _streams[streamId].token.safeTransfer({ to: to, value: withdrawAmount }); + token.safeTransfer({ to: to, value: withdrawAmount }); // Log the withdrawal. emit ISablierFlow.WithdrawFromFlowStream({ @@ -828,7 +880,7 @@ contract SablierFlow is caller: msg.sender, protocolFeeAmount: feeAmount, withdrawAmount: withdrawAmount, - withdrawTime: time + snapshotTime: correctedTime }); } } diff --git a/src/interfaces/ISablierFlow.sol b/src/interfaces/ISablierFlow.sol index 3904a4f9..c5df7837 100644 --- a/src/interfaces/ISablierFlow.sol +++ b/src/interfaces/ISablierFlow.sol @@ -97,7 +97,7 @@ interface ISablierFlow is /// decimals. /// @param withdrawAmount The amount withdrawn to the recipient after subtracting the protocol fee, denoted in /// token's decimals. - /// @param withdrawTime The Unix timestamp up to which ongoing debt was calculated from the snapshot time. + /// @param snapshotTime The Unix timestamp representing the updated snapshot time. event WithdrawFromFlowStream( uint256 indexed streamId, address indexed to, @@ -105,7 +105,7 @@ interface ISablierFlow is address caller, uint128 protocolFeeAmount, uint128 withdrawAmount, - uint40 withdrawTime + uint40 snapshotTime ); /*////////////////////////////////////////////////////////////////////////// diff --git a/src/libraries/Helpers.sol b/src/libraries/Helpers.sol index 8afdb1e6..5c252dc9 100644 --- a/src/libraries/Helpers.sol +++ b/src/libraries/Helpers.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity >=0.8.22; -import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import { ud, UD60x18 } from "@prb/math/src/UD60x18.sol"; import { Broker } from "./../types/DataTypes.sol"; @@ -10,8 +9,6 @@ import { Errors } from "./Errors.sol"; /// @title Helpers /// @notice Library with helper functions in {SablierFlow} contract. library Helpers { - using SafeCast for uint256; - /// @dev Calculate the fee amount and the net amount after subtracting the fee, based on the `fee` percentage. function calculateAmountsFromFee( uint128 totalAmount, @@ -52,38 +49,4 @@ library Helpers { // Calculate the broker fee amount that is going to be transferred to the `broker.account`. (brokerFeeAmount, depositAmount) = calculateAmountsFromFee(totalAmount, broker.fee); } - - /// @notice Denormalizes the provided amount to be denoted in the token's decimals. - /// @dev The following logic is used to denormalize the amount: - /// - If the token has exactly 18 decimals, the amount is returned as is. - /// - If the token has fewer than 18 decimals, the amount is divided by $10^(18 - tokenDecimals)$. - function denormalizeAmount(uint128 normalizedAmount, uint8 tokenDecimals) internal pure returns (uint128 amount) { - // Return the original amount if token's decimals is 18. - if (tokenDecimals == 18) { - return normalizedAmount; - } - - // Safe to use unchecked because we use {SafeCast}. - unchecked { - uint8 factor = 18 - tokenDecimals; - amount = (normalizedAmount / (10 ** factor)).toUint128(); - } - } - - /// @notice Normalizes the provided amount to be denoted in 18 decimals. - /// @dev The following logic is used to normalize the amount: - /// - If the token has exactly 18 decimals, the amount is returned as is. - /// - If the token has fewer than 18 decimals, the amount is multiplied by $10^(18 - tokenDecimals)$. - function normalizeAmount(uint128 amount, uint8 tokenDecimals) internal pure returns (uint128 normalizedAmount) { - // Return the original amount if token's decimals is 18. - if (tokenDecimals == 18) { - return amount; - } - - // Safe to use unchecked because we use {SafeCast}. - unchecked { - uint8 factor = 18 - tokenDecimals; - normalizedAmount = (amount * (10 ** factor)).toUint128(); - } - } } diff --git a/test/fork/Flow.t.sol b/test/fork/Flow.t.sol index 332f57e6..9fcdcdc2 100644 --- a/test/fork/Flow.t.sol +++ b/test/fork/Flow.t.sol @@ -13,6 +13,8 @@ contract Flow_Fork_Test is Fork_Test { /// @dev Total number of streams to create for each token. uint256 internal constant TOTAL_STREAMS = 20; + Vars internal vars; + /// @dev An enum to represent functions from the Flow contract. enum FlowFunc { adjustRatePerSecond, @@ -38,6 +40,26 @@ contract Flow_Fork_Test is Fork_Test { uint40 withdrawAtTime; } + /// @dev A struct to hold the actual and expected values, this prevents stack overflow. + struct Vars { + // Actual values. + UD21x18 actualRatePerSecond; + uint40 actualSnapshotTime; + uint128 actualSnapshotDebt; + uint128 actualStreamBalance; + uint256 actualStreamId; + uint256 actualTokenBalance; + uint128 actualTotalDebt; + // Expected values. + UD21x18 expectedRatePerSecond; + uint40 expectedSnapshotTime; + uint128 expectedSnapshotDebt; + uint128 expectedStreamBalance; + uint256 expectedStreamId; + uint256 expectedTokenBalance; + uint128 expectedTotalDebt; + } + /*////////////////////////////////////////////////////////////////////////// FORK TEST //////////////////////////////////////////////////////////////////////////*/ @@ -218,20 +240,30 @@ contract Flow_Fork_Test is Fork_Test { if (flow.isPaused(streamId)) { flow.restart(streamId, RATE_PER_SECOND); } - if (newRatePerSecond.unwrap() == flow.getRatePerSecond(streamId).unwrap()) { + + UD21x18 oldRatePerSecond = flow.getRatePerSecond(streamId); + if (newRatePerSecond.unwrap() == oldRatePerSecond.unwrap()) { newRatePerSecond = ud21x18(newRatePerSecond.unwrap() + 1); } uint128 beforeSnapshotAmount = flow.getSnapshotDebt(streamId); uint128 totalDebt = flow.totalDebtOf(streamId); uint128 ongoingDebt = flow.ongoingDebtOf(streamId); + if (ongoingDebt != 0) { + vars.expectedSnapshotTime = uint40( + flow.getSnapshotTime(streamId) + + getNormalizedAmount(ongoingDebt, flow.getTokenDecimals(streamId)) / oldRatePerSecond.unwrap() + ); + } else { + vars.expectedSnapshotTime = getBlockTimestamp(); + } // It should emit 1 {AdjustFlowStream}, 1 {MetadataUpdate} events. vm.expectEmit({ emitter: address(flow) }); emit AdjustFlowStream({ streamId: streamId, totalDebt: totalDebt, - oldRatePerSecond: flow.getRatePerSecond(streamId), + oldRatePerSecond: oldRatePerSecond, newRatePerSecond: newRatePerSecond }); @@ -241,19 +273,18 @@ contract Flow_Fork_Test is Fork_Test { flow.adjustRatePerSecond({ streamId: streamId, newRatePerSecond: newRatePerSecond }); // It should update snapshot debt. - uint128 actualSnapshotDebt = flow.getSnapshotDebt(streamId); - uint128 expectedSnapshotDebt = ongoingDebt + beforeSnapshotAmount; - assertEq(actualSnapshotDebt, expectedSnapshotDebt, "snapshot debt"); + vars.actualSnapshotDebt = flow.getSnapshotDebt(streamId); + vars.expectedSnapshotDebt = ongoingDebt + beforeSnapshotAmount; + assertEq(vars.actualSnapshotDebt, vars.expectedSnapshotDebt, "AdjustRatePerSecond: snapshot debt"); // It should set the new rate per second - UD21x18 actualRatePerSecond = flow.getRatePerSecond(streamId); - UD21x18 expectedRatePerSecond = newRatePerSecond; - assertEq(actualRatePerSecond, expectedRatePerSecond, "rate per second"); + vars.actualRatePerSecond = flow.getRatePerSecond(streamId); + vars.expectedRatePerSecond = newRatePerSecond; + assertEq(vars.actualRatePerSecond, vars.expectedRatePerSecond, "AdjustRatePerSecond: rate per second"); // It should update snapshot time - uint128 actualSnapshotTime = flow.getSnapshotTime(streamId); - uint128 expectedSnapshotTime = getBlockTimestamp(); - assertEq(actualSnapshotTime, expectedSnapshotTime, "snapshot time"); + vars.actualSnapshotTime = flow.getSnapshotTime(streamId); + assertEq(vars.actualSnapshotTime, vars.expectedSnapshotTime, "AdjustRatePerSecond: snapshot time"); } /*////////////////////////////////////////////////////////////////////////// @@ -261,17 +292,17 @@ contract Flow_Fork_Test is Fork_Test { //////////////////////////////////////////////////////////////////////////*/ function _test_Create(address recipient, address sender, UD21x18 ratePerSecond, bool transferable) private { - uint256 expectedStreamId = flow.nextStreamId(); + vars.expectedStreamId = flow.nextStreamId(); vm.expectEmit({ emitter: address(flow) }); - emit Transfer({ from: address(0), to: recipient, tokenId: expectedStreamId }); + emit Transfer({ from: address(0), to: recipient, tokenId: vars.expectedStreamId }); vm.expectEmit({ emitter: address(flow) }); - emit MetadataUpdate({ _tokenId: expectedStreamId }); + emit MetadataUpdate({ _tokenId: vars.expectedStreamId }); vm.expectEmit({ emitter: address(flow) }); emit CreateFlowStream({ - streamId: expectedStreamId, + streamId: vars.expectedStreamId, token: token, sender: sender, recipient: recipient, @@ -279,7 +310,7 @@ contract Flow_Fork_Test is Fork_Test { transferable: transferable }); - uint256 actualStreamId = flow.create({ + vars.actualStreamId = flow.create({ recipient: recipient, sender: sender, ratePerSecond: ratePerSecond, @@ -287,7 +318,7 @@ contract Flow_Fork_Test is Fork_Test { transferable: transferable }); - Flow.Stream memory actualStream = flow.getStream(actualStreamId); + Flow.Stream memory actualStream = flow.getStream(vars.actualStreamId); Flow.Stream memory expectedStream = Flow.Stream({ balance: 0, isPaused: false, @@ -303,16 +334,16 @@ contract Flow_Fork_Test is Fork_Test { }); // It should create the stream. - assertEq(actualStreamId, expectedStreamId, "stream ID"); + assertEq(vars.actualStreamId, vars.expectedStreamId, "Create: stream ID"); assertEq(actualStream, expectedStream); // It should bump the next stream id. - assertEq(flow.nextStreamId(), expectedStreamId + 1, "next stream ID"); + assertEq(flow.nextStreamId(), vars.expectedStreamId + 1, "Create: next stream ID"); // It should mint the NFT. - address actualNFTOwner = flow.ownerOf({ tokenId: actualStreamId }); + address actualNFTOwner = flow.ownerOf({ tokenId: vars.actualStreamId }); address expectedNFTOwner = recipient; - assertEq(actualNFTOwner, expectedNFTOwner, "NFT owner"); + assertEq(actualNFTOwner, expectedNFTOwner, "Create: NFT owner"); } /*////////////////////////////////////////////////////////////////////////// @@ -352,14 +383,14 @@ contract Flow_Fork_Test is Fork_Test { flow.deposit(streamId, depositAmount); // Assert that the token balance of stream has been updated. - uint256 actualTokenBalance = token.balanceOf(address(flow)); - uint256 expectedTokenBalance = initialTokenBalance + depositAmount; - assertEq(actualTokenBalance, expectedTokenBalance, "token balanceOf"); + vars.actualTokenBalance = token.balanceOf(address(flow)); + vars.expectedTokenBalance = initialTokenBalance + depositAmount; + assertEq(vars.actualTokenBalance, vars.expectedTokenBalance, "Deposit: token balance"); // Assert that stored balance in stream has been updated. - uint256 actualStreamBalance = flow.getBalance(streamId); - uint256 expectedStreamBalance = initialStreamBalance + depositAmount; - assertEq(actualStreamBalance, expectedStreamBalance, "stream balance"); + vars.actualStreamBalance = flow.getBalance(streamId); + vars.expectedStreamBalance = initialStreamBalance + depositAmount; + assertEq(vars.actualStreamBalance, vars.expectedStreamBalance, "Deposit: stream balance"); } /*////////////////////////////////////////////////////////////////////////// @@ -389,10 +420,10 @@ contract Flow_Fork_Test is Fork_Test { flow.pause(streamId); // Assert that the stream is paused. - assertTrue(flow.isPaused(streamId), "paused"); + assertTrue(flow.isPaused(streamId), "Pause: paused"); // Assert that the rate per second is 0. - assertEq(flow.getRatePerSecond(streamId), 0, "rate per second"); + assertEq(flow.getRatePerSecond(streamId), 0, "Pause: rate per second"); } /*////////////////////////////////////////////////////////////////////////// @@ -431,14 +462,14 @@ contract Flow_Fork_Test is Fork_Test { flow.refund(streamId, refundAmount); // Assert that the token balance of stream has been updated. - uint256 actualTokenBalance = token.balanceOf(address(flow)); - uint256 expectedTokenBalance = initialTokenBalance - refundAmount; - assertEq(actualTokenBalance, expectedTokenBalance, "token balanceOf"); + vars.actualTokenBalance = token.balanceOf(address(flow)); + vars.expectedTokenBalance = initialTokenBalance - refundAmount; + assertEq(vars.actualTokenBalance, vars.expectedTokenBalance, "Refund: token balance"); // Assert that stored balance in stream has been updated. - uint256 actualStreamBalance = flow.getBalance(streamId); - uint256 expectedStreamBalance = initialStreamBalance - refundAmount; - assertEq(actualStreamBalance, expectedStreamBalance, "stream balance"); + vars.actualStreamBalance = flow.getBalance(streamId); + vars.expectedStreamBalance = initialStreamBalance - refundAmount; + assertEq(vars.actualStreamBalance, vars.expectedStreamBalance, "Refund: stream balance"); } /*////////////////////////////////////////////////////////////////////////// @@ -469,12 +500,12 @@ contract Flow_Fork_Test is Fork_Test { assertFalse(flow.isPaused(streamId)); // It should update rate per second. - UD21x18 actualRatePerSecond = flow.getRatePerSecond(streamId); - assertEq(actualRatePerSecond, ratePerSecond, "ratePerSecond"); + vars.actualRatePerSecond = flow.getRatePerSecond(streamId); + assertEq(vars.actualRatePerSecond, ratePerSecond, "Restart: rate per second"); // It should update snapshot time. - uint40 actualSnapshotTime = flow.getSnapshotTime(streamId); - assertEq(actualSnapshotTime, getBlockTimestamp(), "snapshotTime"); + vars.actualSnapshotTime = flow.getSnapshotTime(streamId); + assertEq(vars.actualSnapshotTime, getBlockTimestamp(), "Restart: snapshot time"); } /*////////////////////////////////////////////////////////////////////////// @@ -528,13 +559,13 @@ contract Flow_Fork_Test is Fork_Test { flow.void(streamId); // It should set the rate per second to zero. - assertEq(flow.getRatePerSecond(streamId), 0, "rate per second"); + assertEq(flow.getRatePerSecond(streamId), 0, "Void: rate per second"); // It should pause the stream. - assertTrue(flow.isPaused(streamId), "paused"); + assertTrue(flow.isPaused(streamId), "Void: paused"); // It should set the total debt to stream balance. - assertEq(flow.totalDebtOf(streamId), beforeVoidBalance, "total debt"); + assertEq(flow.totalDebtOf(streamId), beforeVoidBalance, "Void: total debt"); } /*////////////////////////////////////////////////////////////////////////// @@ -549,25 +580,30 @@ contract Flow_Fork_Test is Fork_Test { ); uint8 tokenDecimals = flow.getTokenDecimals(streamId); - + uint128 ratePerSecond = flow.getRatePerSecond(streamId).unwrap(); uint128 streamBalance = flow.getBalance(streamId); if (streamBalance == 0) { - uint128 depositAmount = getDefaultDepositAmount(tokenDecimals); - depositOnStream(streamId, depositAmount); + depositOnStream(streamId, getDefaultDepositAmount(tokenDecimals)); streamBalance = flow.getBalance(streamId); } - uint128 totalDebt = flow.getSnapshotDebt(streamId) - + getDenormalizedAmount( - flow.getRatePerSecond(streamId).unwrap() * (withdrawTime - flow.getSnapshotTime(streamId)), - flow.getTokenDecimals(streamId) - ); + uint128 ongoingDebt = + getDenormalizedAmount(ratePerSecond * (withdrawTime - flow.getSnapshotTime(streamId)), tokenDecimals); + uint128 totalDebt = flow.getSnapshotDebt(streamId) + ongoingDebt; uint128 withdrawAmount = streamBalance < totalDebt ? streamBalance : totalDebt; (, address caller,) = vm.readCallers(); address recipient = flow.getRecipient(streamId); - uint256 tokenBalance = token.balanceOf(address(flow)); + uint256 initialTokenBalance = token.balanceOf(address(flow)); + + // Compute the snapshot time that will be stored post withdraw. + if (!flow.isPaused(streamId) && withdrawTime > flow.getSnapshotTime(streamId)) { + vars.expectedSnapshotTime = + uint40(getNormalizedAmount(ongoingDebt, tokenDecimals) / ratePerSecond + flow.getSnapshotTime(streamId)); + } else { + vars.expectedSnapshotTime = withdrawTime; + } // Expect the relevant events to be emitted. vm.expectEmit({ emitter: address(token) }); @@ -581,7 +617,7 @@ contract Flow_Fork_Test is Fork_Test { caller: caller, protocolFeeAmount: 0, withdrawAmount: withdrawAmount, - withdrawTime: withdrawTime + snapshotTime: vars.expectedSnapshotTime }); vm.expectEmit({ emitter: address(flow) }); @@ -591,25 +627,25 @@ contract Flow_Fork_Test is Fork_Test { flow.withdrawAt(streamId, recipient, withdrawTime); // It should update snapshot time. - assertEq(flow.getSnapshotTime(streamId), withdrawTime, "snapshot time"); + vars.actualSnapshotTime = flow.getSnapshotTime(streamId); + assertEq(vars.actualSnapshotTime, vars.expectedSnapshotTime, "WithdrawAt: snapshot time"); // It should decrease the total debt by withdrawn amount. - uint128 actualTotalDebt = flow.getSnapshotDebt(streamId) + vars.actualTotalDebt = flow.getSnapshotDebt(streamId) + getDenormalizedAmount( - flow.getRatePerSecond(streamId).unwrap() * (withdrawTime - flow.getSnapshotTime(streamId)), - flow.getTokenDecimals(streamId) + ratePerSecond * (vars.expectedSnapshotTime - flow.getSnapshotTime(streamId)), tokenDecimals ); - uint128 expectedTotalDebt = totalDebt - withdrawAmount; - assertEq(actualTotalDebt, expectedTotalDebt, "total debt"); + vars.expectedTotalDebt = totalDebt - withdrawAmount; + assertEq(vars.actualTotalDebt, vars.expectedTotalDebt, "WithdrawAt: total debt"); // It should reduce the stream balance by the withdrawn amount. - uint128 actualStreamBalance = flow.getBalance(streamId); - uint128 expectedStreamBalance = streamBalance - withdrawAmount; - assertEq(actualStreamBalance, expectedStreamBalance, "stream balance"); + vars.actualStreamBalance = flow.getBalance(streamId); + vars.expectedStreamBalance = streamBalance - withdrawAmount; + assertEq(vars.actualStreamBalance, vars.expectedStreamBalance, "WithdrawAt: stream balance"); // It should reduce the token balance of stream. - uint256 actualTokenBalance = token.balanceOf(address(flow)); - uint256 expectedTokenBalance = tokenBalance - withdrawAmount; - assertEq(actualTokenBalance, expectedTokenBalance, "token balance"); + vars.actualTokenBalance = token.balanceOf(address(flow)); + vars.expectedTokenBalance = initialTokenBalance - withdrawAmount; + assertEq(vars.actualTokenBalance, vars.expectedTokenBalance, "WithdrawAt: token balance"); } } diff --git a/test/integration/Integration.t.sol b/test/integration/Integration.t.sol index 4e1ea21b..1839cb4d 100644 --- a/test/integration/Integration.t.sol +++ b/test/integration/Integration.t.sol @@ -127,8 +127,8 @@ abstract contract Integration_Test is Base_Test { deposit(defaultStreamId, DEPOSIT_AMOUNT_6D); } - /// @dev Update the `snapshotTime` of a stream to the current block timestamp. - function updateSnapshotTimeToBlockTimestamp(uint256 streamId) internal { + /// @dev Update the snapshot using `adjustRatePerSecond` and then warp block timestamp to it. + function updateSnapshotTimeAndWarp(uint256 streamId) internal { resetPrank(users.sender); UD21x18 ratePerSecond = flow.getRatePerSecond(streamId); @@ -137,6 +137,10 @@ abstract contract Integration_Test is Base_Test { // Restores the rate per second. flow.adjustRatePerSecond(streamId, ratePerSecond); + + // Warp to make block.timestamp match the snapshot time. This is necessary as the correct snapshot time can + // deviate from the block.timestamp due to precision loss. + vm.warp({ newTimestamp: flow.getSnapshotTime(streamId) }); } /*////////////////////////////////////////////////////////////////////////// diff --git a/test/integration/concrete/batch/batch.t.sol b/test/integration/concrete/batch/batch.t.sol index c8a6120e..d7c21081 100644 --- a/test/integration/concrete/batch/batch.t.sol +++ b/test/integration/concrete/batch/batch.t.sol @@ -350,7 +350,7 @@ contract Batch_Integration_Concrete_Test is Integration_Test { caller: users.sender, protocolFeeAmount: 0, withdrawAmount: WITHDRAW_AMOUNT_6D, - withdrawTime: WITHDRAW_TIME + snapshotTime: WITHDRAW_TIME }); vm.expectEmit({ emitter: address(flow) }); @@ -368,7 +368,7 @@ contract Batch_Integration_Concrete_Test is Integration_Test { protocolFeeAmount: 0, caller: users.sender, withdrawAmount: WITHDRAW_AMOUNT_6D, - withdrawTime: WITHDRAW_TIME + snapshotTime: WITHDRAW_TIME }); vm.expectEmit({ emitter: address(flow) }); diff --git a/test/integration/concrete/ongoing-debt-of/ongoingDebtOf.t.sol b/test/integration/concrete/ongoing-debt-of/ongoingDebtOf.t.sol index bda77588..20642b86 100644 --- a/test/integration/concrete/ongoing-debt-of/ongoingDebtOf.t.sol +++ b/test/integration/concrete/ongoing-debt-of/ongoingDebtOf.t.sol @@ -18,8 +18,8 @@ contract OngoingDebtOf_Integration_Concrete_Test is Integration_Test { } function test_WhenSnapshotTimeInPresent() external givenNotNull givenNotPaused { - // Update the last time to the current block timestamp. - updateSnapshotTimeToBlockTimestamp(defaultStreamId); + // Update the snapshot time and warp the current block timestamp to it. + updateSnapshotTimeAndWarp(defaultStreamId); // It should return zero. uint128 ongoingDebt = flow.ongoingDebtOf(defaultStreamId); diff --git a/test/integration/concrete/withdraw-at/withdrawAt.t.sol b/test/integration/concrete/withdraw-at/withdrawAt.t.sol index fb7f79f3..8a847193 100644 --- a/test/integration/concrete/withdraw-at/withdrawAt.t.sol +++ b/test/integration/concrete/withdraw-at/withdrawAt.t.sol @@ -27,8 +27,8 @@ contract WithdrawAt_Integration_Concrete_Test is Integration_Test { } function test_RevertWhen_TimeLessThanSnapshotTime() external whenNoDelegateCall givenNotNull { - // Set the snapshot time to the current block timestamp. - updateSnapshotTimeToBlockTimestamp(defaultStreamId); + // Update the snapshot time and warp the current block timestamp to it. + updateSnapshotTimeAndWarp(defaultStreamId); uint40 snapshotTime = flow.getSnapshotTime(defaultStreamId); @@ -269,7 +269,7 @@ contract WithdrawAt_Integration_Concrete_Test is Integration_Test { caller: users.recipient, protocolFeeAmount: feeAmount, withdrawAmount: withdrawAmount, - withdrawTime: WITHDRAW_TIME + snapshotTime: WITHDRAW_TIME }); vm.expectEmit({ emitter: address(flow) }); diff --git a/test/integration/concrete/withdraw-max/withdrawMax.t.sol b/test/integration/concrete/withdraw-max/withdrawMax.t.sol index 303fef64..2dc10c58 100644 --- a/test/integration/concrete/withdraw-max/withdrawMax.t.sol +++ b/test/integration/concrete/withdraw-max/withdrawMax.t.sol @@ -51,7 +51,7 @@ contract WithdrawMax_Integration_Concrete_Test is Integration_Test { caller: users.sender, protocolFeeAmount: 0, withdrawAmount: withdrawAmount, - withdrawTime: getBlockTimestamp() + snapshotTime: getBlockTimestamp() }); vm.expectEmit({ emitter: address(flow) }); diff --git a/test/integration/fuzz/coveredDebtOf.t.sol b/test/integration/fuzz/coveredDebtOf.t.sol index ba2eb701..07908776 100644 --- a/test/integration/fuzz/coveredDebtOf.t.sol +++ b/test/integration/fuzz/coveredDebtOf.t.sol @@ -14,8 +14,8 @@ contract CoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { uint40 depletionPeriod = flow.depletionTimeOf(streamId); - // Bound the time jump so that it exceeds depletion timestamp. - timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod); + // Bound the time jump so that it is less than the depletion timestamp. + timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod - 1); // Simulate the passage of time. vm.warp({ newTimestamp: timeJump }); @@ -51,8 +51,8 @@ contract CoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { uint40 depletionPeriod = flow.depletionTimeOf(streamId); - // Bound the time jump so that it exceeds depletion timestamp. - timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod); + // Bound the time jump so that it is less than the depletion timestamp. + timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod - 1); // Simulate the passage of time. vm.warp({ newTimestamp: timeJump }); @@ -73,7 +73,7 @@ contract CoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { function testFuzz_PostDepletion(uint256 streamId, uint40 timeJump, uint8 decimals) external givenNotNull { (streamId,, depositedAmount) = useFuzzedStreamOrCreate(streamId, decimals); - // Bound the time jump so that it exceeds depletion timestamp. + // Bound the time jump so it is greater than depletion timestamp. uint40 depletionPeriod = flow.depletionTimeOf(streamId); timeJump = boundUint40(timeJump, depletionPeriod + 1, UINT40_MAX); diff --git a/test/integration/fuzz/depletionTimeOf.t.sol b/test/integration/fuzz/depletionTimeOf.t.sol index 8baf1b4f..9a28bcc5 100644 --- a/test/integration/fuzz/depletionTimeOf.t.sol +++ b/test/integration/fuzz/depletionTimeOf.t.sol @@ -23,8 +23,9 @@ contract DepletionTimeOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { (streamId, decimals,) = useFuzzedStreamOrCreate(streamId, decimals); // Calculate the solvency period based on the stream deposit. - uint40 solvencyPeriod = - uint40(getNormalizedAmount(flow.getBalance(streamId), decimals) / flow.getRatePerSecond(streamId).unwrap()); + uint40 solvencyPeriod = uint40( + getNormalizedAmount(flow.getBalance(streamId) + 1, decimals) / flow.getRatePerSecond(streamId).unwrap() + ); // Bound the time jump to provide a realistic time frame. timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); @@ -36,8 +37,19 @@ contract DepletionTimeOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { uint40 actualDepletionTime = flow.depletionTimeOf(streamId); if (getBlockTimestamp() > MAY_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, MAY_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"); } } } diff --git a/test/integration/fuzz/ongoingDebtOf.t.sol b/test/integration/fuzz/ongoingDebtOf.t.sol index 99ccb510..4841b357 100644 --- a/test/integration/fuzz/ongoingDebtOf.t.sol +++ b/test/integration/fuzz/ongoingDebtOf.t.sol @@ -52,8 +52,8 @@ contract OngoingDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); - // Update the last time to block timestamp. - updateSnapshotTimeToBlockTimestamp(streamId); + // Update the snapshot time and warp the current block timestamp to it. + updateSnapshotTimeAndWarp(streamId); // Assert that ongoing debt is zero. uint128 actualOngoingDebt = flow.ongoingDebtOf(streamId); @@ -76,8 +76,8 @@ contract OngoingDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { { (streamId, decimals,) = useFuzzedStreamOrCreate(streamId, decimals); - // Update the last time to block timestamp. - updateSnapshotTimeToBlockTimestamp(streamId); + // Update the snapshot time and warp the current block timestamp to it. + updateSnapshotTimeAndWarp(streamId); // Bound the time jump to provide a realistic time frame. timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); diff --git a/test/integration/fuzz/refund.t.sol b/test/integration/fuzz/refund.t.sol index 53f3439c..9172a7c4 100644 --- a/test/integration/fuzz/refund.t.sol +++ b/test/integration/fuzz/refund.t.sol @@ -29,7 +29,7 @@ contract Refund_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); - // Bound the time jump so that it exceeds depletion timestamp. + // Bound the time jump so it is greater than depletion timestamp. uint40 depletionPeriod = flow.depletionTimeOf(streamId); timeJump = boundUint40(timeJump, depletionPeriod + 1, UINT40_MAX); @@ -63,9 +63,9 @@ contract Refund_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); - // Bound the time jump to provide a realistic time frame and not exceeding depletion timestamp. + // Bound the time jump so that it is less than the depletion timestamp. uint40 depletionPeriod = flow.depletionTimeOf(streamId); - timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod); + timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod - 1); // Simulate the passage of time. vm.warp({ newTimestamp: timeJump }); diff --git a/test/integration/fuzz/refundableAmountOf.t.sol b/test/integration/fuzz/refundableAmountOf.t.sol index 57c7021d..43bf7b7a 100644 --- a/test/integration/fuzz/refundableAmountOf.t.sol +++ b/test/integration/fuzz/refundableAmountOf.t.sol @@ -19,8 +19,8 @@ contract RefundableAmountOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Tes uint128 previousStreamBalance = flow.getBalance(streamId); - // Bound the time jump so that it exceeds depletion timestamp. - timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod); + // Bound the time jump so that it is less than the depletion timestamp. + timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod - 1); // Simulate the passage of time. vm.warp({ newTimestamp: timeJump }); @@ -49,9 +49,9 @@ contract RefundableAmountOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Tes { (streamId, decimals, depositedAmount) = useFuzzedStreamOrCreate(streamId, decimals); - // Bound the time jump so that it exceeds depletion timestamp. + // Bound the time jump so that it is less than the depletion timestamp. uint40 depletionPeriod = flow.depletionTimeOf(streamId); - timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod); + timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod - 1); // Simulate the passage of time. vm.warp({ newTimestamp: timeJump }); @@ -71,7 +71,7 @@ contract RefundableAmountOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Tes function testFuzz_PostDepletion(uint256 streamId, uint40 timeJump, uint8 decimals) external givenNotNull { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); - // Bound the time jump so that it exceeds depletion timestamp. + // Bound the time jump so it is greater than depletion timestamp. uint40 depletionPeriod = flow.depletionTimeOf(streamId); timeJump = boundUint40(timeJump, depletionPeriod + 1, UINT40_MAX); diff --git a/test/integration/fuzz/withdrawAt.t.sol b/test/integration/fuzz/withdrawAt.t.sol index 25e9a00e..c254ab81 100644 --- a/test/integration/fuzz/withdrawAt.t.sol +++ b/test/integration/fuzz/withdrawAt.t.sol @@ -7,6 +7,25 @@ import { ud, UD60x18, ZERO } from "@prb/math/src/UD60x18.sol"; import { Shared_Integration_Fuzz_Test } from "./Fuzz.t.sol"; contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { + struct Vars { + uint40 actualSnapshotTime; + uint256 actualStreamBalance; + uint256 actualTokenBalance; + uint128 actualTotalDebt; + uint128 expectedProtocolRevenue; + uint40 expectedSnapshotTime; + uint128 expectedStreamBalance; + uint256 expectedTokenBalance; + uint128 expectedTotalDebt; + uint128 ongoingDebt; + uint128 streamBalance; + uint256 tokenBalance; + uint128 totalDebt; + uint128 withdrawAmount; + } + + Vars internal vars; + /// @dev It should withdraw 0 amount from a stream. function testFuzz_Paused_Withdraw( address caller, @@ -50,32 +69,32 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { caller: caller, protocolFeeAmount: 0, withdrawAmount: 0, - withdrawTime: withdrawTime + snapshotTime: withdrawTime }); vm.expectEmit({ emitter: address(flow) }); emit MetadataUpdate({ _tokenId: streamId }); - uint128 expectedTotalDebt = flow.totalDebtOf(streamId); - uint128 expectedStreamBalance = flow.getBalance(streamId); - uint256 expectedTokenBalance = token.balanceOf(address(flow)); + vars.expectedTotalDebt = flow.totalDebtOf(streamId); + vars.expectedStreamBalance = flow.getBalance(streamId); + vars.expectedTokenBalance = token.balanceOf(address(flow)); // Change prank to caller and withdraw the tokens. resetPrank(caller); flow.withdrawAt(streamId, users.recipient, withdrawTime); // Assert that all states are unchanged except for snapshotTime. - uint128 actualSnapshotTime = flow.getSnapshotTime(streamId); - assertEq(actualSnapshotTime, withdrawTime, "snapshot time"); + vars.actualSnapshotTime = flow.getSnapshotTime(streamId); + assertEq(vars.actualSnapshotTime, withdrawTime, "snapshot time"); - uint128 actualTotalDebt = flow.totalDebtOf(streamId); - assertEq(actualTotalDebt, expectedTotalDebt, "total debt"); + vars.actualTotalDebt = flow.totalDebtOf(streamId); + assertEq(vars.actualTotalDebt, vars.expectedTotalDebt, "total debt"); - uint128 actualStreamBalance = flow.getBalance(streamId); - assertEq(actualStreamBalance, expectedStreamBalance, "stream balance"); + vars.actualStreamBalance = flow.getBalance(streamId); + assertEq(vars.actualStreamBalance, vars.expectedStreamBalance, "stream balance"); - uint256 actualTokenBalance = token.balanceOf(address(flow)); - assertEq(actualTokenBalance, expectedTokenBalance, "token balance"); + vars.actualTokenBalance = token.balanceOf(address(flow)); + assertEq(vars.actualTokenBalance, vars.expectedTokenBalance, "token balance"); } /// @dev Checklist: @@ -202,7 +221,7 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { // Bound the time jump to provide a realistic time frame. timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); - uint40 warpTimestamp = getBlockTimestamp() + timeJump; + uint40 warpTimestamp = getBlockTimestamp() + boundUint40(timeJump, 1 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: warpTimestamp }); @@ -210,27 +229,33 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { // Bound the withdraw time between the allowed range. withdrawTime = boundUint40(withdrawTime, MAY_1_2024, warpTimestamp); - uint256 tokenBalance = token.balanceOf(address(flow)); - uint128 totalDebt = flow.getSnapshotDebt(streamId) - + getDenormalizedAmount({ - amount: flow.getRatePerSecond(streamId).unwrap() * (withdrawTime - flow.getSnapshotTime(streamId)), - decimals: flow.getTokenDecimals(streamId) - }); - uint128 streamBalance = flow.getBalance(streamId); - uint128 withdrawAmount = streamBalance < totalDebt ? streamBalance : totalDebt; + vars.tokenBalance = token.balanceOf(address(flow)); + vars.ongoingDebt = getDenormalizedAmount({ + amount: flow.getRatePerSecond(streamId).unwrap() * (withdrawTime - flow.getSnapshotTime(streamId)), + decimals: flow.getTokenDecimals(streamId) + }); + vars.totalDebt = flow.getSnapshotDebt(streamId) + vars.ongoingDebt; + vars.streamBalance = flow.getBalance(streamId); + vars.withdrawAmount = vars.streamBalance < vars.totalDebt ? vars.streamBalance : vars.totalDebt; - uint128 expectedProtocolRevenue = flow.protocolRevenue(token); + vars.expectedProtocolRevenue = flow.protocolRevenue(token); uint128 feeAmount; if (flow.protocolFee(token) > ZERO) { - feeAmount = uint128(ud(withdrawAmount).mul(flow.protocolFee(token)).unwrap()); - withdrawAmount -= feeAmount; - expectedProtocolRevenue += feeAmount; + feeAmount = uint128(ud(vars.withdrawAmount).mul(flow.protocolFee(token)).unwrap()); + vars.withdrawAmount -= feeAmount; + vars.expectedProtocolRevenue += feeAmount; } + // Compute the snapshot time that will be stored post withdraw. + vars.expectedSnapshotTime = uint40( + getNormalizedAmount(vars.ongoingDebt, flow.getTokenDecimals(streamId)) + / flow.getRatePerSecond(streamId).unwrap() + flow.getSnapshotTime(streamId) + ); + // Expect the relevant events to be emitted. vm.expectEmit({ emitter: address(token) }); - emit IERC20.Transfer({ from: address(flow), to: to, value: withdrawAmount }); + emit IERC20.Transfer({ from: address(flow), to: to, value: vars.withdrawAmount }); vm.expectEmit({ emitter: address(flow) }); emit WithdrawFromFlowStream({ @@ -239,8 +264,8 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { token: token, caller: caller, protocolFeeAmount: feeAmount, - withdrawAmount: withdrawAmount, - withdrawTime: withdrawTime + withdrawAmount: vars.withdrawAmount, + snapshotTime: vars.expectedSnapshotTime }); vm.expectEmit({ emitter: address(flow) }); @@ -250,26 +275,28 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { flow.withdrawAt(streamId, to, withdrawTime); // Assert the protocol revenue. - assertEq(flow.protocolRevenue(token), expectedProtocolRevenue, "protocol revenue"); + assertEq(flow.protocolRevenue(token), vars.expectedProtocolRevenue, "protocol revenue"); + + uint40 snapshotTime = flow.getSnapshotTime(streamId); // It should update snapshot time. - assertEq(flow.getSnapshotTime(streamId), withdrawTime, "snapshot time"); + assertEq(snapshotTime, vars.expectedSnapshotTime, "snapshot time"); // It should decrease the full total debt by withdrawn amount and fee amount. - uint128 actualTotalDebt = flow.getSnapshotDebt(streamId) + vars.actualTotalDebt = flow.getSnapshotDebt(streamId) + getDenormalizedAmount({ - amount: flow.getRatePerSecond(streamId).unwrap() * (withdrawTime - flow.getSnapshotTime(streamId)), + amount: flow.getRatePerSecond(streamId).unwrap() * (vars.expectedSnapshotTime - snapshotTime), decimals: flow.getTokenDecimals(streamId) }); - uint128 expectedTotalDebt = totalDebt - withdrawAmount - feeAmount; - assertEq(actualTotalDebt, expectedTotalDebt, "total debt"); + vars.expectedTotalDebt = vars.totalDebt - vars.withdrawAmount - feeAmount; + assertEq(vars.actualTotalDebt, vars.expectedTotalDebt, "total debt"); // It should reduce the stream balance by the withdrawn amount and fee amount. - uint128 expectedStreamBalance = streamBalance - withdrawAmount - feeAmount; - assertEq(flow.getBalance(streamId), expectedStreamBalance, "stream balance"); + vars.expectedStreamBalance = vars.streamBalance - vars.withdrawAmount - feeAmount; + assertEq(flow.getBalance(streamId), vars.expectedStreamBalance, "stream balance"); // It should reduce the token balance of stream by net withdrawn amount. - uint256 expectedTokenBalance = tokenBalance - withdrawAmount; - assertEq(token.balanceOf(address(flow)), expectedTokenBalance, "token balance"); + vars.expectedTokenBalance = vars.tokenBalance - vars.withdrawAmount; + assertEq(token.balanceOf(address(flow)), vars.expectedTokenBalance, "token balance"); } } diff --git a/test/integration/fuzz/withdrawMax.t.sol b/test/integration/fuzz/withdrawMax.t.sol index 83a000e3..d24174aa 100644 --- a/test/integration/fuzz/withdrawMax.t.sol +++ b/test/integration/fuzz/withdrawMax.t.sol @@ -136,6 +136,16 @@ contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { uint128 streamBalance = flow.getBalance(streamId); uint128 withdrawAmount = flow.coveredDebtOf(streamId); + uint40 expectedCorrectedAmount = uint40( + getNormalizedAmount(flow.ongoingDebtOf(streamId), flow.getTokenDecimals(streamId)) + / flow.getRatePerSecond(streamId).unwrap() + flow.getSnapshotTime(streamId) + ); + + uint128 residualDebt = getDenormalizedAmount({ + amount: flow.getRatePerSecond(streamId).unwrap() * (getBlockTimestamp() - expectedCorrectedAmount), + decimals: flow.getTokenDecimals(streamId) + }); + // Expect the relevant events to be emitted. vm.expectEmit({ emitter: address(token) }); emit IERC20.Transfer({ from: address(flow), to: withdrawTo, value: withdrawAmount }); @@ -148,7 +158,7 @@ contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { caller: caller, protocolFeeAmount: 0, withdrawAmount: withdrawAmount, - withdrawTime: getBlockTimestamp() + snapshotTime: expectedCorrectedAmount }); vm.expectEmit({ emitter: address(flow) }); @@ -158,13 +168,21 @@ contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { flow.withdrawMax(streamId, withdrawTo); // It should update snapshot time. - assertEq(flow.getSnapshotTime(streamId), getBlockTimestamp(), "snapshot time"); + assertEq(flow.getSnapshotTime(streamId), expectedCorrectedAmount, "snapshot time"); // It should decrease the total debt by the withdrawn value. uint128 actualTotalDebt = flow.totalDebtOf(streamId); - uint128 expectedTotalDebt = totalDebt - withdrawAmount; + uint128 expectedTotalDebt = totalDebt - withdrawAmount + residualDebt; assertEq(actualTotalDebt, expectedTotalDebt, "total debt"); + // Assert that snapshot time is updated correctly. + assertEq(flow.getSnapshotTime(streamId), expectedCorrectedAmount, "snapshot time"); + + // Assert that total debt equals snapshot debt and ongoing debt + assertEq( + flow.totalDebtOf(streamId), flow.getSnapshotDebt(streamId) + flow.ongoingDebtOf(streamId), "snapshot debt" + ); + // It should reduce the stream balance by the withdrawn amount. uint128 actualStreamBalance = flow.getBalance(streamId); uint128 expectedStreamBalance = streamBalance - withdrawAmount; diff --git a/test/invariant/Flow.t.sol b/test/invariant/Flow.t.sol index 2a95bffd..5994c192 100644 --- a/test/invariant/Flow.t.sol +++ b/test/invariant/Flow.t.sol @@ -102,6 +102,19 @@ contract Flow_Invariant_Test is Base_Test { ); } + /// @dev For any stream, the snapshot time should be greater than or equal to the previous snapshot time. + function invariant_SnapshotTimeAlwaysIncreases() external view { + uint256 lastStreamId = flowStore.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ++i) { + uint256 streamId = flowStore.streamIds(i); + assertGe( + flow.getSnapshotTime(streamId), + flowHandler.previousSnapshotTime(streamId), + "Invariant violation: snapshot time should never decrease" + ); + } + } + /// @dev For any stream, if uncovered debt > 0, then the covered debt should equal the stream balance. function invariant_UncoveredDebt_CoveredDebtEqBalance() external view { uint256 lastStreamId = flowStore.lastStreamId(); diff --git a/test/invariant/handlers/FlowHandler.sol b/test/invariant/handlers/FlowHandler.sol index be374f95..2a24b2ab 100644 --- a/test/invariant/handlers/FlowHandler.sol +++ b/test/invariant/handlers/FlowHandler.sol @@ -19,6 +19,9 @@ contract FlowHandler is BaseHandler { address internal currentSender; uint256 internal currentStreamId; + /// @dev Snapshot times mapped by stream IDs. + mapping(uint256 streamId => uint40 snapshotTime) public previousSnapshotTime; + /// @dev Total debts mapped by stream IDs. mapping(uint256 streamId => uint128 amount) public previousTotalDebtOf; @@ -37,6 +40,7 @@ contract FlowHandler is BaseHandler { /// @dev Updates the states of handler right before calling each Flow function. modifier updateFlowHandlerStates() { + previousSnapshotTime[currentStreamId] = flow.getSnapshotTime(currentStreamId); previousTotalDebtOf[currentStreamId] = flow.totalDebtOf(currentStreamId); previousUncoveredDebtOf[currentStreamId] = flow.uncoveredDebtOf(currentStreamId); _; diff --git a/test/utils/Events.sol b/test/utils/Events.sol index 3740c083..cd7e8e1c 100644 --- a/test/utils/Events.sol +++ b/test/utils/Events.sol @@ -77,6 +77,6 @@ abstract contract Events { address caller, uint128 protocolFeeAmount, uint128 withdrawAmount, - uint40 withdrawTime + uint40 snapshotTime ); } diff --git a/test/utils/Utils.sol b/test/utils/Utils.sol index f2d3460d..6fa025e6 100644 --- a/test/utils/Utils.sol +++ b/test/utils/Utils.sol @@ -6,7 +6,6 @@ import { ud21x18, UD21x18 } from "@prb/math/src/UD21x18.sol"; import { PRBMathUtils } from "@prb/math/test/utils/Utils.sol"; import { CommonBase } from "forge-std/src/Base.sol"; import { SafeCastLib } from "solady/src/utils/SafeCastLib.sol"; -import { Helpers } from "src/libraries/Helpers.sol"; import { Constants } from "./Constants.sol"; abstract contract Utils is CommonBase, Constants, PRBMathUtils { @@ -33,7 +32,7 @@ abstract contract Utils is CommonBase, Constants, PRBMathUtils { /// @dev Bounds the rate per second between a realistic range. function boundRatePerSecond(UD21x18 ratePerSecond) internal pure returns (UD21x18) { - return ud21x18(boundUint128(ratePerSecond.unwrap(), 0.00001e18, 10e18)); + return ud21x18(boundUint128(ratePerSecond.unwrap(), 0.0000001e18, 10e18)); } /// @dev Bounds a `uint128` number. @@ -61,14 +60,24 @@ abstract contract Utils is CommonBase, Constants, PRBMathUtils { return TRANSFER_VALUE * (10 ** decimals).toUint128(); } - /// @dev Mirror function for {Helpers.denormalizeAmount}. + /// @dev Denormalizes the amount to denote it in token's decimals. function getDenormalizedAmount(uint128 amount, uint8 decimals) internal pure returns (uint128) { - return Helpers.denormalizeAmount(amount, decimals); + if (decimals == 18) { + return amount; + } + + uint8 factor = 18 - decimals; + return (amount / (10 ** factor)).toUint128(); } - /// @dev Mirror function for {Helpers.normalizeAmount}. + /// @dev Normalizes the amount to denote it in 18 decimals. function getNormalizedAmount(uint128 amount, uint8 decimals) internal pure returns (uint128) { - return Helpers.normalizeAmount(amount, decimals); + if (decimals == 18) { + return amount; + } + + uint8 factor = 18 - decimals; + return (amount * (10 ** factor)).toUint128(); } /// @dev Checks if the Foundry profile is "benchmark".