[automations] VestingSchedulerV2 add helpful view functions #1965
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changelog ReminderReminder to update the CHANGELOG.md for any of the modified packages in this PR.
|
hellwolf
reviewed
Jun 21, 2024
| // Assume | ||
| vm.assume(randomizer != 0); | ||
|
|
||
| vm.assume(startDate == 0 || startDate >= block.timestamp); |
There was a problem hiding this comment.
generally vm.bound is preferred over the vm.assume. check their documentation on why.
hellwolf
reviewed
Jun 21, 2024
|
|
||
|
|
||
| /// @dev IVestingScheduler.getCreateVestingScheduleParamsFromAmountAndDuration implementation. | ||
| function getCreateVestingScheduleParamsFromAmountAndDuration( |
There was a problem hiding this comment.
a minor thing, a trick to make this function "pure", is to take block.timestamp as a input value, e.g. "timestampNow"
0xPilou
approved these changes
Jun 21, 2024
hellwolf
approved these changes
Jun 21, 2024
kasparkallas
merged commit 5a4a980
into
feature-set-vesting-scheduler-v2
16 of 20 checks passed
XKCD Comic RelifLink: https://xkcd.com/1965 |
hellwolf
pushed a commit
that referenced
this pull request
Jul 23, 2024
* refactor: use uint96 for remainderAmount & change packing order * feat: add `getMaximumNeededTokenAllowance` helper function with a test * refactor: converge on view function usage * chore: add comments * chore: clean-up * chore: reset whitespace Signed-off-by: Miao, ZhiCheng <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 23, 2024
* chore: no-op for pull request diff Signed-off-by: Miao, ZhiCheng <[email protected]> * [automations] Vesting Scheduler V2 ergonomic improvements (#1904) * allow creation and execution of the vesting schedule in the current block * add createVestingSchedule function which works with totalAmount and totalDuration * add overloads without ctx * need to improve testing coverage * add more overloads with fewer parameters * reorganize the functions * add create and execute schedule mvp * work in progress, needs proper testing * remove try-catch from early end * prefer reverting the early end until stream can be closed without needing the transfer (i.e. it will slightly overflow in that case) * add dust amount fix (wip) * needs proper test cover * consider the log events * rename from dustFixAmount to remainderAmount * add to log as well * fix test issues * tiny comment rename * remove functions create and execute functions with cliff period * add a comprehensive fuzzed test for createScheduleFromAmountAndDuration * slightly change end compensation & remainder handling * use greater or equal handling for case when only remainder needs to be transferred * assert transferFrom success result * add todo-s, improve tests * keep V1 contract, separate V2 explicitly * update deploy script for v2 * unify deploy scripts * use newer host.registerApp & unify deploy scripts - add base-mainnet option * clean-up * add diff generation script & completely revert VestingScheduler.sol Signed-off-by: Miao, ZhiCheng <[email protected]> * [AUTOMATIONS] Vesting Scheduler - add claimable schedule feature (#1944) * added claimable vesting feature * add check on `_executeCliffAndFlow` for claimable schedules * updated time window condition on schedule claim * fix typo * add some unit tests for claiming schedules * increased test coverage * added claimValidityDate feature * updated tests * add claimValidityDate param to createSchedules function * updated unit tests * refactor internal function params (stack too deep) + add claimValidityDate to schedule creation event * removed `isClaimable` boolean from VestingSchedule data structure * remove internal function creating dupplication * updated claim validity date check logic * refactor: re-order the claimValidityDate in the event - keep it as one of the last for backwards compatibility * refactor: rename error CannotClaimFlowOnBehalf to CannotClaimScheduleOnBehalf * fix: remove merge issues from hardhat configs * fix: remove duplication from hardhat config * fix: moved & rename params struct into VestingSchedulerV2 contract --------- Co-authored-by: Kaspar Kallas <[email protected]> Co-authored-by: Kaspar Kallas <[email protected]> Signed-off-by: Miao, ZhiCheng <[email protected]> * [AUTOMATIONS] VestingSchdulerV2 improvements (#1963) * update: change claimValidityDate to claimPeriod in function that takes amount and duration as params * fix: clear claimValidityDate on claim + add checks to execute* functions * update: add VestingClaimed event to `_executeCliffAndFlow` function Signed-off-by: Miao, ZhiCheng <[email protected]> * [automations] VestingSchedulerV2 add helpful view functions (#1965) * refactor: use uint96 for remainderAmount & change packing order * feat: add `getMaximumNeededTokenAllowance` helper function with a test * refactor: converge on view function usage * chore: add comments * chore: clean-up * chore: reset whitespace Signed-off-by: Miao, ZhiCheng <[email protected]> * [AUTOMATIONS] VestingSchedulerV2 claim after end date (#1964) * update: change claimValidityDate to claimPeriod in function that takes amount and duration as params * fix: clear claimValidityDate on claim + add checks to execute* functions * update: add VestingClaimed event to `_executeCliffAndFlow` function * feature: add capabilities to have claimValidityDate after endDate * fix: rearrange `_executeCliffAndFlow` logic * test: increased coverage for executeCliffAndFlow * test: added revert check on `executeEndVesting` test * refactor: clean-up - add additional asserts - change log event order (to match other situation) --------- Co-authored-by: Kaspar Kallas <[email protected]> Signed-off-by: Miao, ZhiCheng <[email protected]> * fix: check if schedule is claimed on executeEndVesting Signed-off-by: Miao, ZhiCheng <[email protected]> * test: increased `getMaximumNeededTokenAllowance` coverage Signed-off-by: Miao, ZhiCheng <[email protected]> * feat: added remainderAmount in `VestingScheduleUpdated` Signed-off-by: Miao, ZhiCheng <[email protected]> * add tests & fixes Signed-off-by: Miao, ZhiCheng <[email protected]> * [automations] Vesting Scheduler V2 refactoring after single-transfer feature (#1969) * refactor: explicit functions - _claim - _exececuteAtSingleTransfer - _getTotalVestedAmount * chore: test that schedule is deleted in more places * refactor: use more foundry bound in tests * chore: test better the scenario where the schedule is not ended on time * refactor: refactor to using aggregate object - make executeCliffAndFlow public * chore: improve the test further * refactor: use aggregate in all places * refactor: re-order some functions based on visibility * chore: add small comment * refactor: small whitespace fix * refactor: use named parameters when using structs * refactor: remove unnecessary comments * fix: change type in log event * chore: test claim event * chore: add version Signed-off-by: Miao, ZhiCheng <[email protected]> * chore: add optimism hardhat config Signed-off-by: Miao, ZhiCheng <[email protected]> * refactor: unify `createClaimableVestingSchedule` and `createVestingSchedule` functions Signed-off-by: Miao, ZhiCheng <[email protected]> * refactor: reoder `createVestingScheduleFromAmountAndDuration` function params Signed-off-by: Miao, ZhiCheng <[email protected]> * refactor: unify `createVestingScheduleFormAmountAndDuration` and `createClaimableVestingScheduleFormAmountAndDuration` functions Signed-off-by: Miao, ZhiCheng <[email protected]> * [automations] Vesting Scheduler V2 final clean-up (#1973) * refactor: unify `createClaimableVestingSchedule` and `createVestingSchedule` functions * refactor: reoder `createVestingScheduleFromAmountAndDuration` function params * refactor: unify `createVestingScheduleFormAmountAndDuration` and `createClaimableVestingScheduleFormAmountAndDuration` functions * refactor: remove confusing overloads * feat: add cliffPeriod to createAndExecute functions * unify modifiers & remove a helper function * refactor: use normalizeStartDate function to get a function to be pure * refactor: remove unnecessary passing of ctx * refactor: rename * add more claim fuzz tests * change log event semantics slightly for single transfer * remove version --------- Co-authored-by: Pilou <[email protected]> Signed-off-by: Miao, ZhiCheng <[email protected]> * [automations] Vesting scheduler v2 - fix & v1 compatibility (#1977) * refactor: unify `createClaimableVestingSchedule` and `createVestingSchedule` functions * refactor: reoder `createVestingScheduleFromAmountAndDuration` function params * refactor: unify `createVestingScheduleFormAmountAndDuration` and `createClaimableVestingScheduleFormAmountAndDuration` functions * chore: add `createVestingSchedule` v1 overload for backward compatibility * fix: remove `cliffPeriod` parameter in `createAndExecuteVestingScheduleFromAmountAndDuration` function * refactor: replace `_getSender(bytes(""))` by `msg.sender` Signed-off-by: Miao, ZhiCheng <[email protected]> * refactor: re-order functions for better readability Signed-off-by: Miao, ZhiCheng <[email protected]> --------- Signed-off-by: Miao, ZhiCheng <[email protected]> Co-authored-by: Pilou <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added and exposed
getCreateVestingScheduleParamsFromAmountAndDurationThe view function can be used to know up front what will be the stored
VestingSchedulevalues. Useful for UI-s. It's effectively the previous_createVestingScheduleFromAmountAndDurationexposed as a view function without any logic changed.Exposed
ScheduleCreationParamsThe struct holds all relevant values to a vesting schedule in addition to just the ones being stored. Useful for UI-s. Separated "superfluid context" out of it as it's a technical requirement of the smart contract to enable batching transaction, not necessarily relevant to the core data of the vesting schedule.
Deleted
ScheduleCreationFromAmountAndDurationParamsLess is more.
Added and exposed
getMaximumNeededTokenAllowanceAdded a helpful utility function to calculate what is the maximum needed ERC-20 allowance for the vesting schedule to execute successfully under all conditions. Not used by the core contract logic as allowance is meant to be handled outside, so no added risk, but useful for UI-s (that otherwise would need to calculate all of this on their own).
Used
uint96forremainderAmountInitially I used
uint256for simplicity sake butuint96is plenty enough.Re-ordered
VestingSchedulepackingAs there's no difference in the storage size used, I opted to order it in a more backwards compatible way to V1 vesting scheduler.
Added more tests