diff --git a/SECURITY.md b/SECURITY.md index 65c04ee9..51f758c5 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -57,10 +57,10 @@ it must adhere to the following assumptions: trust that the sender will not abuse the refund function to reclaim tokens. - The `depletionTimeOf` function depends on the stream's rate per second. Therefore, any change in the rate per second will result in a new depletion time. -- As explained in the [Technical Documentation](https://github.com/sablier-labs/flow/blob/main/TECHNICAL-DOC.md), - recipients cannot withdraw the exact amount of debt streamed due to precision errors. This discrepancy is minor, with - the maximum potential loss (in USDC) being just $0.01 per withdraw. Typically, this loss ranges from 0 to 1 unit of - the token (in its native decimal format), depending on the timing of the recipient's withdrawal. +- As explained in the [Technical Documentation](https://github.com/sablier-labs/flow/blob/main/TECHNICAL-DOC.md), there + could be a minor discrepancy between the actual streamed amount and the expected amount. This is due to `rps` being an + 18-decimal number, while users provide the amount per interval in the UI. If `rps` had infinite decimals, this + discrepancy would not occur. ### Rewards diff --git a/TECHNICAL-DOC.md b/TECHNICAL-DOC.md index 55b65c7c..08ad5156 100644 --- a/TECHNICAL-DOC.md +++ b/TECHNICAL-DOC.md @@ -92,8 +92,7 @@ can only withdraw the available balance. 16. if $isVoided = true \implies isPaused = true$ and $ud = 0$ -17. if $isVoided = false \implies \text{expected amount streamed} \ge td + \text{amount withdrawn}$ and - $\text{expected amount streamed} - (td + \text{amount withdrawn}) \le 10$ +17. if $isVoided = false \implies \text{expected amount streamed} = td + \text{amount withdrawn}$ ## Limitation diff --git a/package.json b/package.json index 7ce1bd43..ce075778 100644 --- a/package.json +++ b/package.json @@ -63,8 +63,8 @@ "clean": "rm -rf artifacts broadcast cache docs out out-optimized out-svg", "lint": "bun run lint:sol && bun run prettier:check", "lint:fix": "bun run lint:sol:fix && forge fmt", - "lint:sol": "forge fmt --check && bun solhint \"{benchmark,precompiles,script,src,test}/**/*.sol\"", - "lint:sol:fix": "bun solhint \"{benchmark,precompiles,script,src,test}/**/*.sol\" --fix --noPrompt", + "lint:sol": "forge fmt --check && bun solhint \"{benchmark,precompiles,script,src,tests}/**/*.sol\"", + "lint:sol:fix": "bun solhint \"{benchmark,precompiles,script,src,tests}/**/*.sol\" --fix --noPrompt", "prepack": "bun install && bash ./shell/prepare-artifacts.sh", "prepare": "husky", "prettier:check": "prettier --check \"**/*.{json,md,svg,yml}\"", diff --git a/tests/integration/fuzz/create.t.sol b/tests/integration/fuzz/create.t.sol index bc9d5a3c..fdd41ef3 100644 --- a/tests/integration/fuzz/create.t.sol +++ b/tests/integration/fuzz/create.t.sol @@ -3,7 +3,7 @@ pragma solidity >=0.8.22; import { IERC4906 } from "@openzeppelin/contracts/interfaces/IERC4906.sol"; import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; -import { ud21x18, UD21x18 } from "@prb/math/src/UD21x18.sol"; +import { UD21x18 } from "@prb/math/src/UD21x18.sol"; import { ISablierFlow } from "src/interfaces/ISablierFlow.sol"; diff --git a/tests/integration/fuzz/restart.t.sol b/tests/integration/fuzz/restart.t.sol index 5332bad1..aad223d3 100644 --- a/tests/integration/fuzz/restart.t.sol +++ b/tests/integration/fuzz/restart.t.sol @@ -2,7 +2,7 @@ pragma solidity >=0.8.22; import { IERC4906 } from "@openzeppelin/contracts/interfaces/IERC4906.sol"; -import { ud21x18, UD21x18 } from "@prb/math/src/UD21x18.sol"; +import { UD21x18 } from "@prb/math/src/UD21x18.sol"; import { ISablierFlow } from "src/interfaces/ISablierFlow.sol"; diff --git a/tests/integration/fuzz/withdrawMultiple.t.sol b/tests/integration/fuzz/withdrawMultiple.t.sol index bb49a501..55ac56ed 100644 --- a/tests/integration/fuzz/withdrawMultiple.t.sol +++ b/tests/integration/fuzz/withdrawMultiple.t.sol @@ -3,20 +3,18 @@ pragma solidity >=0.8.22; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { ud21x18 } from "@prb/math/src/UD21x18.sol"; -import { ISablierFlow } from "src/interfaces/ISablierFlow.sol"; import { Shared_Integration_Fuzz_Test } from "./Fuzz.t.sol"; -contract WithdrawMultiple_Delay_Fuzz_Test is Shared_Integration_Fuzz_Test { +contract WithdrawMaxMultiple_NoDelay_Fuzz_Test is Shared_Integration_Fuzz_Test { /// @dev Checklist: /// - It should test multiple withdrawals from the stream using `withdrawMax`. - /// - It should assert that the actual withdrawn amount is less than the desired amount. - /// - It should check that stream delay and deviation are within acceptable limits for realistic values of rps. + /// - It should assert that the actual withdrawn amount is equal to the desired amount. /// /// Given enough runs, all of the following scenarios should be fuzzed for USDC: /// - Multiple values for realistic rps. /// - Multiple withdrawal counts on the same stream at multiple points in time. - function testFuzz_WithdrawMultiple_Usdc_SmallDelay(uint128 rps, uint256 withdrawCount, uint40 timeJump) external { + function testFuzz_WithdrawMaxMultiple_Usdc_NoDelay(uint128 rps, uint256 withdrawCount, uint40 timeJump) external { rps = boundRatePerSecond(ud21x18(rps)).unwrap(); IERC20 token = createToken(DECIMALS); @@ -50,72 +48,25 @@ contract WithdrawMultiple_Delay_Fuzz_Test is Shared_Integration_Fuzz_Test { // Calculate the desired amount. uint256 desiredTotalWithdrawnAmount = (rps * totalStreamPeriod) / SCALE_FACTOR; - // Calculate the deviation. - uint256 deviationAmount = desiredTotalWithdrawnAmount - actualTotalWithdrawnAmount; - - // Calculate the stream delay. - uint256 streamDelay = (deviationAmount * SCALE_FACTOR) / rps; - - // Assert that the stream delay is within 5 second for the given fuzzed rps. - assertLe(streamDelay, 5 seconds); - - // Assert that the deviation is less than 0.01e6 USDC. - assertLe(deviationAmount, 0.01e6); - - // Assert that actual withdrawn amount is always less than the desired amount. - assertLe(actualTotalWithdrawnAmount, desiredTotalWithdrawnAmount); + // Assert that actual sum of withdrawn amount equals the total desired amount. + assertEq(actualTotalWithdrawnAmount, desiredTotalWithdrawnAmount); } /// @dev Checklist: /// - It should test multiple withdrawals from the stream using `withdrawMax`. - /// - It should assert that the actual withdrawn amount is always less than the desired amount. - /// - /// Given enough runs, all of the following scenarios should be fuzzed: - /// - Multiple values for decimals - /// - Multiple values for wide ranged rps. - /// - Multiple withdrawal counts on the same stream at multiple points in time. - function testFuzz_WithdrawMaxMultiple_RpsWideRange( - uint128 rps, - uint256 withdrawCount, - uint40 timeJump, - uint8 decimals - ) - external - { - _test_WithdrawMultiple(rps, withdrawCount, timeJump, decimals, ISablierFlow.withdrawMax.selector, 0); - } - - /// @dev Checklist: - /// - It should test multiple withdrawals from the stream using `withdraw`. - /// - It should assert that the actual withdrawn amount is always less than the desired amount. + /// - It should assert that the actual withdrawn amount is equal to the desired amount. /// /// Given enough runs, all of the following scenarios should be fuzzed: /// - Multiple values for decimals /// - Multiple values for wide ranged rps. - /// - Multiple amounts to withdraw. /// - Multiple withdrawal counts on the same stream at multiple points in time. - function testFuzz_WithdrawMultiple_RpsWideRange( + function testFuzz_WithdrawMaxMultiple_RpsWideRange_NoDelay( uint128 rps, - uint128 withdrawAmount, uint256 withdrawCount, uint40 timeJump, uint8 decimals ) external - { - _test_WithdrawMultiple(rps, withdrawCount, timeJump, decimals, ISablierFlow.withdraw.selector, withdrawAmount); - } - - // Private helper function. - function _test_WithdrawMultiple( - uint128 rps, - uint256 withdrawCount, - uint40 timeJump, - uint8 decimals, - bytes4 selector, - uint128 withdrawAmount - ) - private { decimals = boundUint8(decimals, 0, 18); IERC20 token = createToken(decimals); @@ -147,16 +98,7 @@ contract WithdrawMultiple_Delay_Fuzz_Test is Shared_Integration_Fuzz_Test { timeJump = boundUint40(timeJump, 1 hours, 1 days); vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); - // Withdraw the tokens based on the selector value. - // ISablierFlow.withdraw - if (selector == ISablierFlow.withdraw.selector) { - withdrawAmount = boundUint128(withdrawAmount, 1, flow.withdrawableAmountOf(streamId)); - flow.withdraw(streamId, users.recipient, withdrawAmount); - } - // ISablierFlow.withdrawMax - else if (selector == ISablierFlow.withdrawMax.selector) { - (withdrawAmount,) = flow.withdrawMax(streamId, users.recipient); - } + (uint128 withdrawAmount,) = flow.withdrawMax(streamId, users.recipient); // Update the actual total amount withdrawn. actualTotalWithdrawnAmount += withdrawAmount; @@ -165,7 +107,7 @@ contract WithdrawMultiple_Delay_Fuzz_Test is Shared_Integration_Fuzz_Test { uint40 totalStreamPeriod = getBlockTimestamp() - timeBeforeFirstWithdraw; uint256 desiredTotalWithdrawnAmount = getDescaledAmount(rps * totalStreamPeriod, decimals); - // Assert that actual withdrawn amount is always less than the desired amount. - assertLe(actualTotalWithdrawnAmount, desiredTotalWithdrawnAmount); + // Assert that actual sum of withdrawn amounts equal the desired amount. + assertEq(actualTotalWithdrawnAmount, desiredTotalWithdrawnAmount); } } diff --git a/tests/invariant/Flow.t.sol b/tests/invariant/Flow.t.sol index 5983bb4a..8650f357 100644 --- a/tests/invariant/Flow.t.sol +++ b/tests/invariant/Flow.t.sol @@ -332,8 +332,8 @@ 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 mvt. + /// @dev For non-voided streams, the expected streamed amount should equal to the sum of withdrawn + /// amount and total debt. function invariant_TotalStreamedEqTotalDebtPlusWithdrawn() external view { uint256 lastStreamId = flowStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ++i) { @@ -345,16 +345,10 @@ contract Flow_Invariant_Test is Base_Test { calculateExpectedStreamedAmount(flowStore.streamIds(i), flow.getTokenDecimals(streamId)); uint256 actualTotalStreamed = flow.totalDebtOf(streamId) + flowStore.withdrawnAmounts(streamId); - assertGe( + assertEq( expectedTotalStreamed, actualTotalStreamed, - "Invariant violation: expected streamed amount >= total debt + withdrawn amount" - ); - - assertLe( - expectedTotalStreamed - actualTotalStreamed, - 10, - "Invariant violation: expected streamed amount - total debt + withdrawn amount <= 10" + "Invariant violation: expected total streamed amount != total debt + withdrawn amount" ); } }