Skip to content

Commit

Permalink
feat: disallow zero startTime in dynamic and tranched (#852)
Browse files Browse the repository at this point in the history 
test: when start time is zero in create function from dynamic and tranched
test: fuzz start time starting from 1
test: move common invariants in Lockup
test: remove no longer needed invariants
chore: update Precompiles
  • Loading branch information
@andreivladbrg
andreivladbrg authored Mar 17, 2024
1 parent ce4915f commit be1dea4
Show file tree
Hide file tree
Showing 25 changed files with 195 additions and 149 deletions.
6 changes: 3 additions & 3 deletions precompiles/Precompiles.sol
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/interfaces/ISablierV2LockupDynamic.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ interface ISablierV2LockupDynamic is ISablierV2Lockup {
/// - Must not be delegate called.
/// - `params.totalAmount` must be greater than zero.
/// - If set, `params.broker.fee` must not be greater than `MAX_BROKER_FEE`.
/// - `params.startTime` must be greater than zero and less than the first segment's timestamp
/// - `params.segments` must have at least one segment, but not more than `MAX_SEGMENT_COUNT`.
/// - `params.startTime` must be less than the first segment's timestamp.
/// - The segment timestamps must be arranged in ascending order.
/// - The last segment timestamp (i.e. the stream's end time) must be in the future.
/// - The sum of the segment amounts must equal the deposit amount.
Expand Down
6 changes: 2 additions & 4 deletions src/interfaces/ISablierV2LockupLinear.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,8 @@ interface ISablierV2LockupLinear is ISablierV2Lockup {
/// - Must not be delegate called.
/// - `params.totalAmount` must be greater than zero.
/// - If set, `params.broker.fee` must not be greater than `MAX_BROKER_FEE`.
/// - `params.range.start` must be greater than zero.
/// - `params.range.start` must be less than `params.range.end`.
/// - If set, `params.range.cliff` must be greater than `params.range.start`.
/// - If set, `params.range.cliff` must be less than `params.range.end`.
/// - `params.range.start` must be greater than zero and less than `params.range.end`.
/// - If set, `params.range.cliff` must be greater than `params.range.start` and less than `params.range.end`.
/// - `params.range.end` must be in the future.
/// - `params.recipient` must not be the zero address.
/// - `msg.sender` must have allowed this contract to spend at least `params.totalAmount` assets.
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/ISablierV2LockupTranched.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ interface ISablierV2LockupTranched is ISablierV2Lockup {
/// - Must not be delegate called.
/// - `params.totalAmount` must be greater than zero.
/// - If set, `params.broker.fee` must not be greater than `MAX_BROKER_FEE`.
/// - `params.startTime` must be greater than zero and less than the first tranche's timestamp
/// - `params.tranches` must have at least one tranche, but not more than `MAX_TRANCHE_COUNT`.
/// - `params.startTime` must be less than the first tranche's timestamp.
/// - The tranche timestamps must be arranged in ascending order.
/// - The last tranche timestamp (i.e. the stream's end time) must be in the future.
/// - The sum of the tranche amounts must equal the deposit amount.
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/Errors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ library Errors {
/// @notice Thrown when trying to withdraw an amount greater than the withdrawable amount.
error SablierV2Lockup_Overdraw(uint256 streamId, uint128 amount, uint128 withdrawableAmount);

/// @notice Thrown when trying to create a stream with a zero start time.
error SablierV2Lockup_StartTimeZero();

/// @notice Thrown when trying to cancel or renounce a canceled stream.
error SablierV2Lockup_StreamCanceled(uint256 streamId);

Expand Down Expand Up @@ -110,9 +113,6 @@ library Errors {
/// @notice Thrown when trying to create a stream with a start time greater than the end time.
error SablierV2LockupLinear_StartTimeNotLessThanEndTime(uint40 startTime, uint40 endTime);

/// @notice Thrown when trying to create a stream with a start time equal to zero.
error SablierV2LockupLinear_StartTimeZero();

/*//////////////////////////////////////////////////////////////////////////
SABLIER-V2-NFT-DESCRIPTOR
//////////////////////////////////////////////////////////////////////////*/
Expand Down
12 changes: 11 additions & 1 deletion src/libraries/Helpers.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ library Helpers {
revert Errors.SablierV2Lockup_DepositAmountZero();
}

// Checks: the start time is not zero.
if (startTime == 0) {
revert Errors.SablierV2Lockup_StartTimeZero();
}

// Checks: the segment count is not zero.
uint256 segmentCount = segments.length;
if (segmentCount == 0) {
Expand All @@ -84,7 +89,7 @@ library Helpers {

// Checks: the start time is not zero.
if (range.start == 0) {
revert Errors.SablierV2LockupLinear_StartTimeZero();
revert Errors.SablierV2Lockup_StartTimeZero();
}

// Checks: the start time is strictly less than the end time.
Expand Down Expand Up @@ -124,6 +129,11 @@ library Helpers {
revert Errors.SablierV2Lockup_DepositAmountZero();
}

// Checks: the start time is not zero.
if (startTime == 0) {
revert Errors.SablierV2Lockup_StartTimeZero();
}

// Checks: the tranche count is not zero.
uint256 trancheCount = tranches.length;
if (trancheCount == 0) {
Expand Down
2 changes: 1 addition & 1 deletion test/fork/LockupDynamic.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
checkUsers(params.sender, params.recipient, params.broker.account, address(lockupDynamic));
vm.assume(params.segments.length != 0);
params.broker.fee = _bound(params.broker.fee, 0, MAX_BROKER_FEE);
params.startTime = boundUint40(params.startTime, 0, defaults.START_TIME());
params.startTime = boundUint40(params.startTime, 1, defaults.START_TIME());

// Fuzz the segment timestamps.
fuzzSegmentTimestamps(params.segments, params.startTime);
Expand Down
2 changes: 1 addition & 1 deletion test/fork/LockupTranched.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ abstract contract LockupTranched_Fork_Test is Fork_Test {
checkUsers(params.sender, params.recipient, params.broker.account, address(lockupTranched));
vm.assume(params.tranches.length != 0);
params.broker.fee = _bound(params.broker.fee, 0, MAX_BROKER_FEE);
params.startTime = boundUint40(params.startTime, 0, defaults.START_TIME());
params.startTime = boundUint40(params.startTime, 1, defaults.START_TIME());

// Fuzz the tranche timestamps.
fuzzTrancheTimestamps(params.tranches, params.startTime);
Expand Down
22 changes: 22 additions & 0 deletions test/integration/concrete/lockup-dynamic/create-with-timestamps/createWithTimestamps.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,22 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
createDefaultStreamWithTotalAmount(totalAmount);
}

function test_RevertWhen_StartTimeZero()
external
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
{
vm.expectRevert(Errors.SablierV2Lockup_StartTimeZero.selector);
createDefaultStreamWithStartTime(0);
}

function test_RevertWhen_SegmentCountZero()
external
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
{
LockupDynamic.Segment[] memory segments;
vm.expectRevert(Errors.SablierV2LockupDynamic_SegmentCountZero.selector);
Expand All @@ -64,6 +75,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
{
uint256 segmentCount = defaults.MAX_COUNT() + 1;
Expand All @@ -79,6 +91,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
{
Expand All @@ -94,6 +107,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
whenSegmentAmountsSumDoesNotOverflow
Expand All @@ -120,6 +134,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
whenSegmentAmountsSumDoesNotOverflow
Expand All @@ -146,6 +161,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
whenSegmentAmountsSumDoesNotOverflow
Expand Down Expand Up @@ -175,6 +191,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
whenSegmentAmountsSumDoesNotOverflow
Expand All @@ -192,6 +209,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
whenSegmentAmountsSumDoesNotOverflow
Expand Down Expand Up @@ -229,6 +247,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
whenSegmentAmountsSumDoesNotOverflow
Expand All @@ -249,6 +268,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
whenSegmentAmountsSumDoesNotOverflow
Expand All @@ -272,6 +292,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
whenSegmentAmountsSumDoesNotOverflow
Expand All @@ -290,6 +311,7 @@ contract CreateWithTimestamps_LockupDynamic_Integration_Concrete_Test is
whenNotDelegateCalled
whenRecipientNonZeroAddress
whenDepositAmountNotZero
whenStartTimeNotZero
whenSegmentCountNotZero
whenSegmentCountNotTooHigh
whenSegmentAmountsSumDoesNotOverflow
Expand Down
71 changes: 37 additions & 34 deletions test/integration/concrete/lockup-dynamic/create-with-timestamps/createWithTimestamps.tree
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,46 +8,49 @@ createWithTimestamps.t.sol
├── when the deposit amount is zero
│ └── it should revert
└── when the deposit amount is not zero
├── when the segment count is zero
├── when the start time is zero
│ └── it should revert
└── when the segment count is not zero
├── when the segment count is too high
└── when the start time is not zero
├── when the segment count is zero
│ └── it should revert
└── when the segment count is not too high
├── when the segment amounts sum overflows
└── when the segment count is not zero
├── when the segment count is too high
│ └── it should revert
└── when the segment amounts sum does not overflow
├── when the start time is greater than the first segment timestamp
└── when the segment count is not too high
├── when the segment amounts sum overflows
│ └── it should revert
├── when the start time is equal to the first segment timestamp
│ └── it should revert
└── when the start time is less than the first segment timestamp
├── when the segment timestamps are not ordered
└── when the segment amounts sum does not overflow
├── when the start time is greater than the first segment timestamp
│ └── it should revert
├── when the start time is equal to the first segment timestamp
│ └── it should revert
└── when the segment timestamps are ordered
├── when the end time is not in the future
└── when the start time is less than the first segment timestamp
├── when the segment timestamps are not ordered
│ └── it should revert
└── when the end time is in the future
├── when the deposit amount is not equal to the segment amounts sum
└── when the segment timestamps are ordered
├── when the end time is not in the future
│ └── it should revert
└── when the deposit amount is equal to the segment amounts sum
├── when the broker fee is too high
└── when the end time is in the future
├── when the deposit amount is not equal to the segment amounts sum
│ └── it should revert
└── when the broker fee is not too high
├── when the asset is not a contract
└── when the deposit amount is equal to the segment amounts sum
├── when the broker fee is too high
│ └── it should revert
└── when the asset is a contract
├── when the asset misses the ERC-20 return value
│ ├── it should create the stream
│ ├── it should bump the next stream id
│ ├── it should mint the NFT
│ ├── it should emit a {MetadataUpdate} event
│ ├── it should perform the ERC-20 transfers
│ └── it should emit a {CreateLockupDynamicStream} event
└── when the asset does not miss the ERC-20 return value
├── it should create the stream
├── it should bump the next stream id
├── it should mint the NFT
├── it should emit a {MetadataUpdate} event
├── it should perform the ERC-20 transfers
└── it should emit a {CreateLockupDynamicStream} event
└── when the broker fee is not too high
├── when the asset is not a contract
│ └── it should revert
└── when the asset is a contract
├── when the asset misses the ERC-20 return value
│ ├── it should create the stream
│ ├── it should bump the next stream id
│ ├── it should mint the NFT
│ ├── it should emit a {MetadataUpdate} event
│ ├── it should perform the ERC-20 transfers
│ └── it should emit a {CreateLockupDynamicStream} event
└── when the asset does not miss the ERC-20 return value
├── it should create the stream
├── it should bump the next stream id
├── it should mint the NFT
├── it should emit a {MetadataUpdate} event
├── it should perform the ERC-20 transfers
└── it should emit a {CreateLockupDynamicStream} event
Loading

0 comments on commit be1dea4

Please sign in to comment.