Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: use equality in the withdraw multiple test and in invariant #320

Merged
merged 7 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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}\"",
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/fuzz/create.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/fuzz/restart.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
65 changes: 10 additions & 55 deletions tests/integration/fuzz/withdrawMultiple.t.sol
andreivladbrg marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -50,60 +48,27 @@ 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.
andreivladbrg marked this conversation as resolved.
Show resolved Hide resolved
assertLe(actualTotalWithdrawnAmount, desiredTotalWithdrawnAmount);
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);
_test_WithdrawMultiple(rps, withdrawCount, timeJump, decimals, 0);
smol-ninja marked this conversation as resolved.
Show resolved Hide resolved
andreivladbrg marked this conversation as resolved.
Show resolved Hide resolved
}

// Private helper function.
Expand All @@ -112,7 +77,6 @@ contract WithdrawMultiple_Delay_Fuzz_Test is Shared_Integration_Fuzz_Test {
uint256 withdrawCount,
uint40 timeJump,
uint8 decimals,
bytes4 selector,
uint128 withdrawAmount
)
private
Expand Down Expand Up @@ -147,16 +111,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);
}
(withdrawAmount,) = flow.withdrawMax(streamId, users.recipient);

// Update the actual total amount withdrawn.
actualTotalWithdrawnAmount += withdrawAmount;
Expand All @@ -165,7 +120,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);
}
}
14 changes: 4 additions & 10 deletions tests/invariant/Flow.t.sol
andreivladbrg marked this conversation as resolved.
Show resolved Hide resolved
andreivladbrg marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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"
);
}
}
Expand Down
Loading