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

Remove the deltaCount check #99

Closed
PaulRBerg opened this issue Jul 18, 2022 · 3 comments
Closed

Remove the deltaCount check #99

PaulRBerg opened this issue Jul 18, 2022 · 3 comments
Assignees

Comments

@PaulRBerg
Copy link
Member

PaulRBerg commented Jul 18, 2022

Description

Implements what has been discussed in #96. In short: remove the check here:

https://github.com/sablierhq/v2-core/blob/5f13c560f6a3c99a10899ffc95907a8bc7414d84/src/SablierV2Pro.sol#L238-L241

Because the checkSegmentCounts function will automatically check the count of the segments is less than MAX_SEGMENT_COUNT.

What to Beware

The createWithDuration.tree tree (and consequently the tests too) will have to be refactored to account for two new testing branches:

  1. The case when the block gas limit is overflown in the for loop. This is the only scenario in the protocol where the block gas limit can be reached, but it's only when the user passes too many segment deltas that this can happen.
    2. The case when the count of segmentMilestones is greater than MAX_SEGMENT_COUNT.
@andreivladbrg
Copy link
Member

andreivladbrg commented Jul 28, 2022

2. The case when the count of segmentMilestones is greater than MAX_SEGMENT_COUNT.

There is no need to test this , because we only check if the amountCount is greater than MAX_SEGMENT_COUNT.

@PaulRBerg
Copy link
Member Author

because we only check if the amountCount is greater than MAX_SEGMENT_COUNT.

Well that's what I meant. I know that the contract doesn't directly check segmentMilestones. But the fact that it is possible to pass a higher-than-expected amountCount via segmentMilestones is an edge case that should be tested.

@andreivladbrg
Copy link
Member

This error SablierV2Pro__SegmentCountOutOfBounds will never revert for the reason that only segmentMilestones is greater than MAX_SEGMENT_COUNT but instead SablierV2Pro__SegmentCountsNotEqualwill.

This case is tested in the PR
https://github.com/sablierhq/v2-core/blob/e8f6106271528cdd2f5cfc9a62bdb8dfff9bc08e/test/unit/sablier-v2-pro/create-with-duration/createWithDuration.t.sol#L32-L62

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants