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.

Sign up for GitHub

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

Jump to bottom

[automations] Vesting Scheduler V2 final clean-up #1973

Merged
merged 13 commits into from
Jul 2, 2024
Merged

[automations] Vesting Scheduler V2 final clean-up #1973

merged 13 commits into from
Jul 2, 2024

Conversation

kasparkallas
Copy link
Contributor

@kasparkallas kasparkallas commented Jul 2, 2024 •
edited
Loading

  • remove a confusing overload
  • add cliffPeriod to createAndExecute function for completeness sake
  • unify & restrict visibility modifiers
  • slight refactor with start date normalization to achieve pure mapper function
  • slightly change log event semantics when doing single transfer for a claim
  • some renamings
  • more tests
  • instead of ctx, pass sender
  • reduce duplication with _getId

0xPilou and others added 13 commits July 1, 2024 15:02
@0xPilou
refactor: unify createClaimableVestingSchedule and `createVestingSc…
16610bf 
…hedule` functions
@0xPilou
refactor: reoder createVestingScheduleFromAmountAndDuration functio…
919b9b4 
…n params
@0xPilou
refactor: unify createVestingScheduleFormAmountAndDuration and `cre…
55ed778 
…ateClaimableVestingScheduleFormAmountAndDuration` functions
@kasparkallas
Merge branch 'feature-set-vesting-scheduler-v2' of https://github.com…
dd14636 
…/superfluid-finance/protocol-monorepo into feature-set-vesting-scheduler-v2
@kasparkallas
refactor: remove confusing overloads
b0e2e29
@kasparkallas
feat: add cliffPeriod to createAndExecute functions
f4cb431
@kasparkallas
unify modifiers & remove a helper function
644ee35
@kasparkallas
refactor: use normalizeStartDate function to get a function to be pure
70b3396
@kasparkallas
refactor: remove unnecessary passing of ctx
92ba9cc
@kasparkallas
refactor: rename
db5f259
@kasparkallas
add more claim fuzz tests
6e65094
@kasparkallas
change log event semantics slightly for single transfer
c6b5373
@kasparkallas
remove version
078c3ea
@kasparkallas kasparkallas self-assigned this Jul 2, 2024
@kasparkallas kasparkallas requested a review from a team as a code owner July 2, 2024 07:47
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 2, 2024

Changelog Reminder

Reminder to update the CHANGELOG.md for any of the modified packages in this PR.

  • CHANGELOG.md modified
  • Double check before merge

kasparkallas
kasparkallas commented Jul 2, 2024
View reviewed changes
packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol
);

emit VestingEndExecuted(
agg.superToken,
agg.sender,
agg.receiver,
schedule.endDate,
totalVestedAmount,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided it's better to show the value under the "cliff and flow" event's "flow delay compensation" along with the "cliff amount".

packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol
@@ -398,6 +357,22 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase {
}
}

function _validateBeforeCliffAndFlow(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved slightly

packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol
ScheduleCreationParams memory params,
bytes memory ctx
) private returns (bytes memory newCtx) {
newCtx = ctx;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx was only used for the sender

packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol
address sender = _getSender(ctx);

// Default to current block timestamp if no start date is provided.
if (params.startDate == 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

startDate normalization is moved to a single place and it's expected to pass in correctly here

@kasparkallas kasparkallas merged commit 5f42f63 into feature-set-vesting-scheduler-v2 Jul 2, 2024
18 checks passed
@kasparkallas kasparkallas deleted the vesting-scheduler-v2-more-overload-cleanup branch July 2, 2024 08:24
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 2, 2024

XKCD Comic Relif

Link: https://xkcd.com/1973
https://xkcd.com/1973

@kasparkallas kasparkallas mentioned this pull request Jul 2, 2024
Merged
hellwolf pushed a commit that referenced this pull request Jul 23, 2024
@kasparkallas @0xPilou @hellwolf
[automations] Vesting Scheduler V2 final clean-up (#1973)
f46e79a 
* 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 <76021631+0xPilou@users.noreply.github.com>
Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>
github-merge-queue bot pushed a commit that referenced this pull request Jul 23, 2024
@kasparkallas @0xPilou
[automations][epic] Vesting Scheduler V2 (#1952)
5106c44 
* chore: no-op for pull request diff

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* [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 <zhicheng.miao@gmail.com>

* [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 <kaspar@superfluid.finance>
Co-authored-by: Kaspar Kallas <git@kasparkallas.com>
Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* [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 <zhicheng.miao@gmail.com>

* [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 <zhicheng.miao@gmail.com>

* [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 <git@kasparkallas.com>
Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* fix: check if schedule is claimed on executeEndVesting

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* test: increased `getMaximumNeededTokenAllowance` coverage

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* feat: added remainderAmount in `VestingScheduleUpdated`

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* add tests & fixes

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* [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 <zhicheng.miao@gmail.com>

* chore: add optimism hardhat config

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* refactor: unify `createClaimableVestingSchedule` and `createVestingSchedule` functions

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* refactor: reoder `createVestingScheduleFromAmountAndDuration` function params

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* refactor: unify `createVestingScheduleFormAmountAndDuration` and `createClaimableVestingScheduleFormAmountAndDuration` functions

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* [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 <76021631+0xPilou@users.noreply.github.com>
Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

* [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 <zhicheng.miao@gmail.com>

* refactor: re-order functions for better readability

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>

---------

Signed-off-by: Miao, ZhiCheng <zhicheng.miao@gmail.com>
Co-authored-by: Pilou <76021631+0xPilou@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xPilou 0xPilou 0xPilou approved these changes

Assignees

@kasparkallas kasparkallas

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kasparkallas @0xPilou