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

Prevent setting end time in the past #396

Merged
merged 9 commits into from
Apr 4, 2023
1 change: 1 addition & 0 deletions src/interfaces/ISablierV2LockupDynamic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ interface ISablierV2LockupDynamic is ISablierV2Lockup {
/// - The first segment's milestone must be greater than or equal to `params.startTime`.
/// - The segment milestones must be arranged in ascending order.
/// - `params.startTime` must not be greater than the last segment's milestone.
/// - The current time must not be greater than or equal to `params.range.end`.
/// - The sum of the segment amounts must be equal to the deposit amount.
/// - `params.recipient` must not be the zero address.
/// - `msg.sender` must have allowed this contract to spend at least `params.totalAmount` assets.
Expand Down
3 changes: 2 additions & 1 deletion src/interfaces/ISablierV2LockupLinear.sol
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ interface ISablierV2LockupLinear is ISablierV2Lockup {
/// - `params.totalAmount` must not be zero.
/// - If set, `params.broker.fee` must not be greater than `MAX_FEE`.
/// - `params.range.start` must not be greater than `params.range.cliff`.
/// - `params.range.cliff` must not be greater than `params.range.end`.
/// - `params.range.cliff` must not be greater than or equal to `params.range.end`.
/// - The current time must not be greater than or equal to `params.range.end`.
/// - `params.recipient` must not be the zero address.
/// - `msg.sender` must have allowed this contract to spend at least `params.totalAmount` assets.
///
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ library Errors {
/// @notice Thrown when attempting to create a stream with a zero deposit amount.
error SablierV2Lockup_DepositAmountZero();

/// @notice Thrown when attempting to create a stream with the end time in the past.
error SablierV2Lockup_EndTimeInThePast(uint40 currentTime, uint40 endTime);

/// @notice Thrown when the protocol fee is greater than the maximum fee permitted.
error SablierV2Lockup_ProtocolFeeTooHigh(UD60x18 protocolFee, UD60x18 maxFee);

Expand Down
19 changes: 16 additions & 3 deletions src/libraries/Helpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ library Helpers {
uint40 startTime
)
internal
pure
view
{
// Checks: the deposit amount is not zero.
if (depositAmount == 0) {
Expand All @@ -87,7 +87,7 @@ library Helpers {
}

/// @dev Checks the parameters of the {SablierV2LockupLinear-_createWithRange} function.
function checkCreateLinearParams(uint128 depositAmount, LockupLinear.Range memory range) internal pure {
function checkCreateLinearParams(uint128 depositAmount, LockupLinear.Range memory range) internal view {
// Checks: the deposit amount is not zero.
if (depositAmount == 0) {
revert Errors.SablierV2Lockup_DepositAmountZero();
Expand All @@ -102,6 +102,12 @@ library Helpers {
if (range.cliff >= range.end) {
revert Errors.SablierV2LockupLinear_CliffTimeNotLessThanEndTime(range.cliff, range.end);
}

// Checks: the end time is not in the past.
uint40 currentTime = uint40(block.timestamp);
if (currentTime >= range.end) {
revert Errors.SablierV2Lockup_EndTimeInThePast(currentTime, range.end);
}
}

/// @dev Checks that the segment array counts match, and then adjusts the segments by calculating the milestones.
Expand Down Expand Up @@ -153,7 +159,7 @@ library Helpers {
uint40 startTime
)
private
pure
view
{
// Checks: the start time is strictly less than the first segment milestone.
if (startTime >= segments[0].milestone) {
Expand Down Expand Up @@ -191,6 +197,13 @@ library Helpers {
}
}

// Checks: the end time is not in the past.
// When the loop exits, the current milestone is the last milestone, i.e. the end time of the stream.
uint40 currentTime = uint40(block.timestamp);
if (currentTime >= currentMilestone) {
revert Errors.SablierV2Lockup_EndTimeInThePast(currentTime, currentMilestone);
}

// Check that the deposit amount is equal to the segment amounts sum.
if (depositAmount != segmentAmountsSum) {
revert Errors.SablierV2LockupDynamic_DepositAmountNotEqualToSegmentAmountsSum(
Expand Down
3 changes: 3 additions & 0 deletions test/Base.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ abstract contract Base_Test is Assertions, Calculations, Events, Fuzzers, StdChe
recipient: createUser("Recipient"),
sender: createUser("Sender")
});

// Warp to March 1, 2023 at 00:00 GMT to provide a more realistic testing environment.
vm.warp({ timestamp: DEFAULT_START_TIME });
}

/*//////////////////////////////////////////////////////////////////////////
Expand Down
14 changes: 10 additions & 4 deletions test/fork/lockup/linear/Linear.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,18 @@ abstract contract Linear_Fork_Test is Fork_Test {
/// - Multiple values for the withdraw amount, including zero.
function testForkFuzz_Linear_CreateWithdrawCancel(Params memory params) external {
checkUsers(params.sender, params.recipient, params.broker.account, address(linear));

// Bound the parameters.
uint40 currentTime = getBlockTimestamp();
params.broker.fee = bound(params.broker.fee, 0, MAX_FEE);
params.range.start = boundUint40(
params.range.start, uint40(block.timestamp - 1000 seconds), uint40(block.timestamp + 10_000 seconds)
);
params.range.start = boundUint40(params.range.start, currentTime - 1000 seconds, currentTime + 10_000 seconds);
params.range.cliff = boundUint40(params.range.cliff, params.range.start, params.range.start + 52 weeks);
params.range.end = boundUint40(params.range.end, params.range.cliff + 1, MAX_UNIX_TIMESTAMP);
// Fuzz the end time so that it is always after the cliff time, and always greater than the current time.
params.range.end = boundUint40(
params.range.end,
(params.range.cliff <= currentTime ? currentTime : params.range.cliff) + 1,
MAX_UNIX_TIMESTAMP
);
params.protocolFee = bound(params.protocolFee, 0, MAX_FEE);
params.totalAmount = boundUint128(params.totalAmount, 1, uint128(initialHolderBalance));

Expand Down
16 changes: 8 additions & 8 deletions test/fuzz/lockup/dynamic/streamed-amount-of/streamedAmountOf.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ contract StreamedAmountOf_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test {
{
timeWarp = boundUint40(timeWarp, DEFAULT_CLIFF_DURATION, DEFAULT_TOTAL_DURATION * 2);

// Warp into the future.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Create a single-element segment array.
LockupDynamic.Segment[] memory segments = new LockupDynamic.Segment[](1);
segments[0] = LockupDynamic.Segment({
Expand All @@ -47,6 +43,10 @@ contract StreamedAmountOf_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test {
// Create the stream with the one-segment array.
uint256 streamId = createDefaultStreamWithSegments(segments);

// Warp into the future.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Run the test.
uint128 actualStreamedAmount = dynamic.streamedAmountOf(streamId);
uint128 expectedStreamedAmount =
Expand Down Expand Up @@ -76,15 +76,15 @@ contract StreamedAmountOf_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test {
whenMultipleSegments
whenCurrentMilestoneNot1st
{
timeWarp = boundUint40(timeWarp, MAX_SEGMENTS[0].milestone, DEFAULT_TOTAL_DURATION * 2);
timeWarp = boundUint40(timeWarp, MAX_SEGMENT_DURATION, DEFAULT_TOTAL_DURATION * 2);

// Create the stream with the multiple-segment array.
uint256 streamId = createDefaultStreamWithSegments(MAX_SEGMENTS);

// Warp into the future.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Create the stream with the multiple-segment array.
uint256 streamId = createDefaultStreamWithSegments(MAX_SEGMENTS);

// Run the test.
uint128 actualStreamedAmount = dynamic.streamedAmountOf(streamId);
uint128 expectedStreamedAmount =
Expand Down
19 changes: 14 additions & 5 deletions test/fuzz/lockup/dynamic/withdraw/withdraw.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ contract Withdraw_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test, Withdraw_Fuzz_Test {
address funder;
uint256 streamId;
uint128 totalAmount;
uint40 totalDuration;
uint128 withdrawAmount;
uint128 withdrawableAmount;
}
Expand Down Expand Up @@ -60,10 +61,8 @@ contract Withdraw_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test, Withdraw_Fuzz_Test {
(vars.totalAmount, vars.createAmounts) = fuzzSegmentAmountsAndCalculateCreateAmounts(params.segments);

// Bound the time warp.
params.timeWarp = bound(params.timeWarp, 1, params.segments[params.segments.length - 1].milestone);

// Warp into the future.
vm.warp({ timestamp: DEFAULT_START_TIME + params.timeWarp });
vars.totalDuration = params.segments[params.segments.length - 1].milestone - DEFAULT_START_TIME;
params.timeWarp = bound(params.timeWarp, 1, vars.totalDuration);

// Mint enough ERC-20 assets to the sender.
deal({ token: address(DEFAULT_ASSET), to: vars.funder, give: vars.totalAmount });
Expand All @@ -82,8 +81,18 @@ contract Withdraw_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test, Withdraw_Fuzz_Test {
})
);

// Bound the withdraw amount.
// Warp into the future.
vm.warp({ timestamp: DEFAULT_START_TIME + params.timeWarp });

// Query the withdrawable amount.
vars.withdrawableAmount = dynamic.withdrawableAmountOf(vars.streamId);

// Halt the test if the withdraw amount is zero.
if (vars.withdrawableAmount == 0) {
return;
}

// Bound the withdraw amount.
vars.withdrawAmount = boundUint128(vars.withdrawAmount, 1, vars.withdrawableAmount);

// Expect the ERC-20 assets to be transferred to the recipient.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,17 @@ contract WithdrawableAmountOf_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test {
{
timeWarp = boundUint40(timeWarp, DEFAULT_CLIFF_DURATION, DEFAULT_TOTAL_DURATION * 2);

// Warp into the future.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Create the stream with a custom total amount. The broker fee is disabled so that it doesn't interfere with
// the calculations.
LockupDynamic.CreateWithMilestones memory params = defaultParams.createWithMilestones;
params.totalAmount = DEFAULT_DEPOSIT_AMOUNT;
params.broker = Broker({ account: address(0), fee: ZERO });
uint256 streamId = dynamic.createWithMilestones(params);

// Warp into the future.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Run the test.
uint128 actualWithdrawableAmount = dynamic.withdrawableAmountOf(streamId);
uint128 expectedWithdrawableAmount =
Expand All @@ -68,7 +68,7 @@ contract WithdrawableAmountOf_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test {
/// - Current time < end time
/// - Current time = end time
/// - Current time > end time
/// - WithdrawFromLockupStream amount equal to deposit amount and not
/// - Withdraw amount equal to deposit amount and not
function testFuzz_WithdrawableAmountOf(
uint40 timeWarp,
uint128 withdrawAmount
Expand All @@ -80,9 +80,8 @@ contract WithdrawableAmountOf_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test {
{
timeWarp = boundUint40(timeWarp, DEFAULT_CLIFF_DURATION, DEFAULT_TOTAL_DURATION * 2);

// Warp into the future.
// Define the current time.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
PaulRBerg marked this conversation as resolved.
Show resolved Hide resolved
vm.warp({ timestamp: currentTime });

// Bound the withdraw amount.
uint128 streamedAmount =
Expand All @@ -96,6 +95,9 @@ contract WithdrawableAmountOf_Dynamic_Fuzz_Test is Dynamic_Fuzz_Test {
params.broker = Broker({ account: address(0), fee: ZERO });
uint256 streamId = dynamic.createWithMilestones(params);

// Warp into the future.
vm.warp({ timestamp: currentTime });

// Make the withdrawal.
dynamic.withdraw({ streamId: streamId, to: users.recipient, amount: withdrawAmount });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ contract StreamedAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test {
/// - Current time < end time
/// - Current time = end time
/// - Current time > end time
PaulRBerg marked this conversation as resolved.
Show resolved Hide resolved
/// - Multiple values for the deposit amount
function testFuzz_StreamedAmountOf(
uint40 timeWarp,
uint128 depositAmount
Expand All @@ -52,10 +51,6 @@ contract StreamedAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test {
vm.assume(depositAmount != 0);
timeWarp = boundUint40(timeWarp, DEFAULT_CLIFF_DURATION, DEFAULT_TOTAL_DURATION * 2);

// Warp into the future.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Mint enough ERC-20 assets to the sender.
deal({ token: address(DEFAULT_ASSET), to: users.sender, give: depositAmount });

Expand All @@ -65,6 +60,10 @@ contract StreamedAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test {
params.broker = Broker({ account: address(0), fee: ZERO });
uint256 streamId = linear.createWithRange(params);

// Warp into the future.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Run the test.
uint128 actualStreamedAmount = linear.streamedAmountOf(streamId);
uint128 expectedStreamedAmount = calculateStreamedAmount(currentTime, depositAmount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ contract WithdrawableAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test {
/// - Current time < end time
/// - Current time = end time
/// - Current time > end time
PaulRBerg marked this conversation as resolved.
Show resolved Hide resolved
/// - Multiple values for the deposit amount
function testFuzz_WithdrawableAmountOf_NoWithdrawals(
uint40 timeWarp,
uint128 depositAmount
Expand All @@ -52,10 +51,6 @@ contract WithdrawableAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test {
vm.assume(depositAmount != 0);
timeWarp = boundUint40(timeWarp, DEFAULT_CLIFF_DURATION, DEFAULT_TOTAL_DURATION * 2);

// Warp into the future.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Mint enough ERC-20 assets to the sender.
deal({ token: address(DEFAULT_ASSET), to: users.sender, give: depositAmount });

Expand All @@ -65,6 +60,10 @@ contract WithdrawableAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test {
params.broker = Broker({ account: address(0), fee: ZERO });
uint256 streamId = linear.createWithRange(params);

// Warp into the future.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Run the test.
uint128 actualWithdrawableAmount = linear.withdrawableAmountOf(streamId);
uint128 expectedWithdrawableAmount = calculateStreamedAmount(currentTime, depositAmount);
Expand All @@ -79,7 +78,7 @@ contract WithdrawableAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test {
/// - Current time = end time
/// - Current time > end time
PaulRBerg marked this conversation as resolved.
Show resolved Hide resolved
/// - Multiple values for the deposit amount
/// - WithdrawFromLockupStream amount equal to deposit amount and not
/// - Withdraw amount equal to deposit amount and not
function testFuzz_WithdrawableAmountOf_WithWithdrawals(
uint40 timeWarp,
uint128 depositAmount,
Expand All @@ -92,9 +91,8 @@ contract WithdrawableAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test {
timeWarp = boundUint40(timeWarp, DEFAULT_CLIFF_DURATION, DEFAULT_TOTAL_DURATION * 2);
depositAmount = boundUint128(depositAmount, 10_000, UINT128_MAX);

// Warp into the future.
// Define the current time.
uint40 currentTime = DEFAULT_START_TIME + timeWarp;
vm.warp({ timestamp: currentTime });

// Bound the withdraw amount.
uint128 streamedAmount = calculateStreamedAmount(currentTime, depositAmount);
Expand All @@ -109,6 +107,9 @@ contract WithdrawableAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test {
params.broker = Broker({ account: address(0), fee: ZERO });
uint256 streamId = linear.createWithRange(params);

// Warp into the future.
vm.warp({ timestamp: currentTime });

// Make the withdrawal.
linear.withdraw({ streamId: streamId, to: users.recipient, amount: withdrawAmount });

Expand Down
12 changes: 6 additions & 6 deletions test/fuzz/lockup/shared/cancel-multiple/cancelMultiple.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,15 @@ abstract contract CancelMultiple_Fuzz_Test is Fuzz_Test, Lockup_Shared_Test {
timeWarp = bound(timeWarp, 0 seconds, DEFAULT_TOTAL_DURATION * 2);
endTime = boundUint40(endTime, DEFAULT_CLIFF_TIME + 1, DEFAULT_END_TIME + DEFAULT_TOTAL_DURATION / 2);

// Warp into the future.
vm.warp({ timestamp: DEFAULT_START_TIME + timeWarp });

// Make the sender the caller in this test.
changePrank({ msgSender: users.sender });

// Create a new stream with a different end time.
uint256 streamId = createDefaultStreamWithEndTime(endTime);

// Warp into the future.
vm.warp({ timestamp: DEFAULT_START_TIME + timeWarp });

// Create the stream ids array.
uint256[] memory streamIds = Solarray.uint256s(defaultStreamIds[0], streamId);

Expand Down Expand Up @@ -148,15 +148,15 @@ abstract contract CancelMultiple_Fuzz_Test is Fuzz_Test, Lockup_Shared_Test {
timeWarp = bound(timeWarp, 0 seconds, DEFAULT_TOTAL_DURATION * 2);
endTime = boundUint40(endTime, DEFAULT_CLIFF_TIME + 1, DEFAULT_END_TIME + DEFAULT_TOTAL_DURATION / 2);

// Warp into the future.
vm.warp({ timestamp: DEFAULT_START_TIME + timeWarp });

// Make the recipient the caller in this test.
changePrank({ msgSender: users.recipient });

// Create a new stream with a different end time.
uint256 streamId = createDefaultStreamWithEndTime(endTime);

// Warp into the future.
vm.warp({ timestamp: DEFAULT_START_TIME + timeWarp });

// Create the stream ids array.
uint256[] memory streamIds = Solarray.uint256s(defaultStreamIds[0], streamId);

Expand Down
Loading