Skip to content

Commit

Permalink
test: add a new invariant for streamed amount
Browse files Browse the repository at this point in the history 
test: invariant to compare total debt, streamed amount and withdrawn amount

test: calculate streamed amount from ongoing debt before and after

Co-authored-by: Andrei Vlad Birgaoanu <99738872+andreivladbrg@users.noreply.github.com>

test(invariant): update function calls made by stream ID
test(invariant): move depositAmount to CreateParams struct to avoid stack too deep error

Co-authored-by: Andrei Vlad Birgaoanu <99738872+andreivladbrg@users.noreply.github.com>

test: remove incorrect invariant
  • Loading branch information
@smol-ninja
smol-ninja committed Sep 17, 2024
1 parent acb6e84 commit 54e505b
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 81 deletions.
18 changes: 8 additions & 10 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,26 +157,24 @@ Currently, it's not possible to address this precision problem entirely.
`flow.collectProtocolRevenue` handlers. In real life, someone can transfer tokens to the contract or admin can
withdraw revenue from the contract.

3. for any stream, if $ud > 0 \implies cd = bal$
3. snapshot time should never decrease

4. if $rps \gt 0$ and no deposits are made $\implies \frac{d(ud)}{dt} \ge 0$
4. for any stream, if $ud > 0 \implies cd = bal$

5. if $rps \gt 0$, and no withdraw is made $\implies \frac{d(td)}{dt} \ge 0$
5. if $rps \gt 0$ and no deposits are made $\implies \frac{d(ud)}{dt} \ge 0$

6. for any stream, sum of deposited amounts $\ge$ sum of withdrawn amounts + sum of refunded
6. if $rps \gt 0$, and no withdraw is made $\implies \frac{d(td)}{dt} \ge 0$

7. sum of all deposited amounts $\ge$ sum of all withdrawn amounts + sum of all refunded
7. for any stream, sum of deposited amounts $\ge$ sum of withdrawn amounts + sum of refunded

8. next stream id = current stream id + 1
8. sum of all deposited amounts $\ge$ sum of all withdrawn amounts + sum of all refunded

9. if $ud = 0$ and $isPaused = true \implies cd = sa$
9. next stream id = current stream id + 1

10. if $ud = 0$ and $isPaused = false \implies cd = sa + oa$
10. if $ud = 0$ \implies cd = td$

11. $bal = ra + cd$

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
3 changes: 3 additions & 0 deletions src/SablierFlow.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,9 @@ contract SablierFlow is
// Interaction: perform the ERC-20 transfer.
token.safeTransfer({ to: to, value: amount });

// Protocol Invariant: the difference in total debt should be equal to the difference in the stream balance.
assert(totalDebt - _totalDebtOf(streamId) == balance - _streams[streamId].balance);

// Log the withdrawal.
emit ISablierFlow.WithdrawFromFlowStream({
streamId: streamId,
Expand Down
4 changes: 0 additions & 4 deletions test/fork/Flow.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -584,10 +584,6 @@ contract Flow_Fork_Test is Fork_Test {
uint256 initialTokenBalance = token.balanceOf(address(flow));
uint128 totalDebt = flow.totalDebtOf(streamId);

uint128 ongoingDebtNormalized = getNormalizedAmount(flow.ongoingDebtOf(streamId), tokenDecimals);
uint128 ratePerSecond = flow.getRatePerSecond(streamId).unwrap();
uint40 snapshotTime = flow.getSnapshotTime(streamId);

vars.expectedSnapshotTime = getBlockTimestamp();

(, address caller,) = vm.readCallers();
Expand Down
60 changes: 22 additions & 38 deletions test/invariant/Flow.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ contract Flow_Invariant_Test is Base_Test {
}

/// @dev If rps > 0, and no additional deposits are made, then the uncovered debt should never decrease.
function invariant_UncoveredDebtGt0_RpsGt0_UncoveredDebtIncrease() external view {
function invariant_RpsGt0_UncoveredDebtGt0_UncoveredDebtIncrease() external view {
uint256 lastStreamId = flowStore.lastStreamId();
for (uint256 i = 0; i < lastStreamId; ++i) {
uint256 streamId = flowStore.streamIds(i);
if (flow.getRatePerSecond(streamId).unwrap() > 0 && flowHandler.calls("deposit") == 0) {
if (flow.getRatePerSecond(streamId).unwrap() > 0 && flowHandler.calls(streamId, "deposit") == 0) {
assertGe(
flow.uncoveredDebtOf(streamId),
flowHandler.previousUncoveredDebtOf(streamId),
Expand All @@ -145,6 +145,21 @@ contract Flow_Invariant_Test is Base_Test {
}
}

/// @dev If rps > 0, no withdraw is made, the total debt should always increase.
function invariant_RpsGt0_TotalDebtAlwaysIncreases() external view {
uint256 lastStreamId = flowStore.lastStreamId();
for (uint256 i = 0; i < lastStreamId; ++i) {
uint256 streamId = flowStore.streamIds(i);
if (flow.getRatePerSecond(streamId).unwrap() != 0 && flowHandler.calls(streamId, "withdraw") == 0) {
assertGe(
flow.totalDebtOf(streamId),
flowHandler.previousTotalDebtOf(streamId),
"Invariant violation: total debt should be monotonically increasing"
);
}
}
}

/// @dev For any stream, the sum of all deposited amounts should always be greater than or equal to the sum of all
/// withdrawn and refunded amounts.
function invariant_InflowGeOutflow() external view {
Expand Down Expand Up @@ -183,48 +198,17 @@ contract Flow_Invariant_Test is Base_Test {
}
}

/// @dev If there is no uncovered debt and the stream is paused, the covered debt should always be equal to
/// the snapshot debt.
function invariant_NoUncoveredDebt_StreamedPaused_CoveredDebtEqSnapshotAmount() external view {
/// @dev If there is no uncovered debt, the covered debt should always be equal to
/// the total debt.
function invariant_NoUncoveredDebt_StreamedPaused_CoveredDebtEqTotalDebt() external view {
uint256 lastStreamId = flowStore.lastStreamId();
for (uint256 i = 0; i < lastStreamId; ++i) {
uint256 streamId = flowStore.streamIds(i);
if (flow.isPaused(streamId) && flow.uncoveredDebtOf(streamId) == 0) {
if (flow.uncoveredDebtOf(streamId) == 0) {
assertEq(
flow.coveredDebtOf(streamId),
flow.getSnapshotDebt(streamId),
"Invariant violation: paused stream covered debt != snapshot debt"
);
}
}
}

/// @dev If there is no uncovered debt and the stream is not paused, the covered debt should always be equal
/// to the sum of snapshot debt and ongoing debt.
function invariant_NoDebt_CoveredDebtEqOngoingDebtPlusSnapshotAmount() external view {
uint256 lastStreamId = flowStore.lastStreamId();
for (uint256 i = 0; i < lastStreamId; ++i) {
uint256 streamId = flowStore.streamIds(i);
if (!flow.isPaused(streamId) && flow.uncoveredDebtOf(streamId) == 0) {
assertEq(
flow.coveredDebtOf(streamId),
flow.ongoingDebtOf(streamId) + flow.getSnapshotDebt(streamId),
"Invariant violation: covered debt != ongoing debt + snapshot debt"
);
}
}
}

/// @dev If rps > 0, no withdraw is made, the total debt should always increase.
function invariant_RpsGt0_TotalDebtAlwaysIncreases() external view {
uint256 lastStreamId = flowStore.lastStreamId();
for (uint256 i = 0; i < lastStreamId; ++i) {
uint256 streamId = flowStore.streamIds(i);
if (flow.getRatePerSecond(streamId).unwrap() != 0 && flowHandler.calls("withdraw") == 0) {
assertGe(
flow.totalDebtOf(streamId),
flowHandler.previousTotalDebtOf(streamId),
"Invariant violation: total debt should be monotonically increasing"
"Invariant violation: paused stream covered debt != snapshot debt"
);
}
}
Expand Down
8 changes: 4 additions & 4 deletions test/invariant/handlers/BaseHandler.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ abstract contract BaseHandler is StdCheats, Utils {
/// @dev Maximum number of streams that can be created during an invariant campaign.
uint256 internal constant MAX_STREAM_COUNT = 100;

/// @dev Maps function names to the number of times they have been called.
mapping(string func => uint256 calls) public calls;
/// @dev Maps function names and the number of times they have been called by the stream ID.
mapping(uint256 streamId => mapping(string func => uint256 calls)) public calls;

/// @dev The total number of calls made to this contract.
uint256 public totalCalls;
Expand Down Expand Up @@ -52,8 +52,8 @@ abstract contract BaseHandler is StdCheats, Utils {
}

/// @dev Records a function call for instrumentation purposes.
modifier instrument(string memory functionName) {
calls[functionName]++;
modifier instrument(uint256 streamId, string memory functionName) {
calls[streamId][functionName]++;
totalCalls++;
_;
}
Expand Down
31 changes: 15 additions & 16 deletions test/invariant/handlers/FlowCreateHandler.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ contract FlowCreateHandler is BaseHandler {

/// @dev Struct to prevent stack too deep error.
struct CreateParams {
uint128 depositAmount;
uint256 timeJump;
uint256 tokenIndex;
address sender;
Expand All @@ -70,10 +71,10 @@ contract FlowCreateHandler is BaseHandler {

function create(CreateParams memory params)
public
instrument("create")
checkUsers(params)
useFuzzedToken(params.tokenIndex)
adjustTimestamp(params.timeJump)
instrument(flow.nextStreamId(), "create")
{
vm.assume(flowStore.lastStreamId() < MAX_STREAM_COUNT);

Expand All @@ -88,46 +89,44 @@ contract FlowCreateHandler is BaseHandler {
flowStore.pushStreamId(streamId, params.sender, params.recipient);
}

function createAndDeposit(
CreateParams memory params,
uint128 depositAmount
)
function createAndDeposit(CreateParams memory params)
public
instrument("createAndDeposit")
checkUsers(params)
useFuzzedToken(params.tokenIndex)
adjustTimestamp(params.timeJump)
instrument(flow.nextStreamId(), "createAndDeposit")
{
vm.assume(flowStore.lastStreamId() < MAX_STREAM_COUNT);

uint8 decimals = IERC20Metadata(address(currentToken)).decimals();

// Calculate the upper bound, based on the token decimals, for the deposit amount.
uint128 upperBound = getDenormalizedAmount(1_000_000e18, decimals);

// Bound the stream parameters.
vm.assume(depositAmount >= 100 && depositAmount <= upperBound);
uint128 upperBound = getDenormalizedAmount(1_000_000e18, IERC20Metadata(address(currentToken)).decimals());
vm.assume(params.depositAmount >= 100 && params.depositAmount <= upperBound);
vm.assume(params.ratePerSecond.unwrap() >= 0.0000000001e18 && params.ratePerSecond.unwrap() <= 10e18);

// Mint enough tokens to the Sender.
deal({
token: address(currentToken),
to: params.sender,
give: currentToken.balanceOf(params.sender) + depositAmount
give: currentToken.balanceOf(params.sender) + params.depositAmount
});

// Approve {SablierFlow} to spend the tokens.
currentToken.approve({ spender: address(flow), value: depositAmount });
currentToken.approve({ spender: address(flow), value: params.depositAmount });

// Create the stream.
uint256 streamId = flow.createAndDeposit(
params.sender, params.recipient, params.ratePerSecond, currentToken, params.transferable, depositAmount
params.sender,
params.recipient,
params.ratePerSecond,
currentToken,
params.transferable,
params.depositAmount
);

// Store the stream id.
flowStore.pushStreamId(streamId, params.sender, params.recipient);

// Store the deposited amount.
flowStore.updateStreamDepositedAmountsSum(streamId, depositAmount);
flowStore.updateStreamDepositedAmountsSum(streamId, params.depositAmount);
}
}
16 changes: 8 additions & 8 deletions test/invariant/handlers/FlowHandler.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@ contract FlowHandler is BaseHandler {
UD21x18 newRatePerSecond
)
external
instrument("adjustRatePerSecond")
useFuzzedStream(streamIndex)
useFuzzedStreamSender
adjustTimestamp(timeJump)
updateFlowHandlerStates
instrument(currentStreamId, "adjustRatePerSecond")
{
// Only non paused streams can have their rate per second adjusted.
vm.assume(!flow.isPaused(currentStreamId));
Expand All @@ -104,11 +104,11 @@ contract FlowHandler is BaseHandler {
uint128 depositAmount
)
external
instrument("deposit")
useFuzzedStream(streamIndex)
useFuzzedStreamSender
adjustTimestamp(timeJump)
updateFlowHandlerStates
instrument(currentStreamId, "deposit")
{
// Voided streams cannot be deposited on.
vm.assume(!flow.isVoided(currentStreamId));
Expand All @@ -135,18 +135,18 @@ contract FlowHandler is BaseHandler {
}

/// @dev A function that does nothing but warp the time into the future.
function passTime(uint256 timeJump) external instrument("passTime") adjustTimestamp(timeJump) { }
function passTime(uint256 timeJump) external adjustTimestamp(timeJump) { }

function pause(
uint256 timeJump,
uint256 streamIndex
)
external
instrument("pause")
useFuzzedStream(streamIndex)
useFuzzedStreamSender
adjustTimestamp(timeJump)
updateFlowHandlerStates
instrument(currentStreamId, "pause")
{
// Paused streams cannot be paused again.
vm.assume(!flow.isPaused(currentStreamId));
Expand All @@ -161,11 +161,11 @@ contract FlowHandler is BaseHandler {
uint128 refundAmount
)
external
instrument("refund")
useFuzzedStream(streamIndex)
useFuzzedStreamSender
adjustTimestamp(timeJump)
updateFlowHandlerStates
instrument(currentStreamId, "refund")
{
// Voided streams cannot be refunded.
vm.assume(!flow.isVoided(currentStreamId));
Expand All @@ -191,11 +191,11 @@ contract FlowHandler is BaseHandler {
UD21x18 ratePerSecond
)
external
instrument("restart")
useFuzzedStream(streamIndex)
useFuzzedStreamSender
adjustTimestamp(timeJump)
updateFlowHandlerStates
instrument(currentStreamId, "restart")
{
// Voided streams cannot be restarted.
vm.assume(!flow.isVoided(currentStreamId));
Expand All @@ -215,11 +215,11 @@ contract FlowHandler is BaseHandler {
uint256 streamIndex
)
external
instrument("void")
useFuzzedStream(streamIndex)
useFuzzedStreamRecipient
adjustTimestamp(timeJump)
updateFlowHandlerStates
instrument(currentStreamId, "void")
{
// Voided streams cannot be voided again.
vm.assume(!flow.isVoided(currentStreamId));
Expand All @@ -238,11 +238,11 @@ contract FlowHandler is BaseHandler {
uint128 amount
)
external
instrument("withdraw")
useFuzzedStream(streamIndex)
useFuzzedStreamRecipient
adjustTimestamp(timeJump)
updateFlowHandlerStates
instrument(currentStreamId, "withdraw")
{
// The protocol doesn't allow the withdrawal address to be the zero address.
vm.assume(to != address(0));
Expand Down
5 changes: 5 additions & 0 deletions test/invariant/stores/FlowStore.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ contract FlowStore {
mapping(uint256 streamId => address sender) public senders;
mapping(uint256 streamId => uint128 depositedAmount) public depositedAmounts;
mapping(uint256 streamId => uint128 refundedAmount) public refundedAmounts;
mapping(uint256 streamId => uint128 streamedAmount) public streamedAmount;
mapping(uint256 streamId => uint128 withdrawnAmount) public withdrawnAmounts;
uint256[] public streamIds;
uint256 public streamDepositedAmountsSum;
Expand All @@ -37,6 +38,10 @@ contract FlowStore {
streamDepositedAmountsSum += amount;
}

function updateStreamedAmount(uint256 streamId, uint128 amount) external {
streamedAmount[streamId] += amount;
}

function updateStreamRefundedAmountsSum(uint256 streamId, uint128 amount) external {
refundedAmounts[streamId] += amount;
streamRefundedAmountsSum += amount;
Expand Down
1 change: 0 additions & 1 deletion test/utils/Utils.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ abstract contract Utils is CommonBase, Constants, PRBMathUtils {
returns (uint128 depositAmount)
{
uint128 maxDepositAmount = (UINT128_MAX - balance);

if (decimals < 18) {
maxDepositAmount = maxDepositAmount / uint128(10 ** (18 - decimals));
}
Expand Down

0 comments on commit 54e505b

Please sign in to comment.