-
Notifications
You must be signed in to change notification settings - Fork 240
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
[automations][epic] Vesting Scheduler V2 #1952
Conversation
📦 PR PackagesInstall this PR (you need to setup Github packages): yarn add @superfluid-finance/ethereum-contracts@PR1952
yarn add @superfluid-finance/sdk-core@PR1952
yarn add @superfluid-finance/sdk-redux@PR1952 Click to learn how to use Github packagesTo use the Github package registry, create a token with "read:packages" permission. See Creating a personal access token for help. Next add these lines to your
|
packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol
Outdated
Show resolved
Hide resolved
packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol
Show resolved
Hide resolved
packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unlikely that I will have time to dive deep into the code any time soon. I will just submit my trivial comments for now.
Although, I would like to review the stateful fuzz test, and let it run through scenarios, it is still in https://github.com/0xPilou/protocol-monorepo/blob/vesting-scheduler-v2-tests/packages/automation-contracts/scheduler/test/VestingSchedulerV2.StatefulFuzz.t.sol and I guess won't be merged as part of this PR.
@@ -167,7 +167,7 @@ Thanks goes to these wonderful people ([🐸](https://allcontributors.org/docs/e | |||
|
|||
<!-- ALL-CONTRIBUTORS-LIST:START - Do not remove or modify this section --> | |||
<!-- prettier-ignore-start --> | |||
<!-- markdownlint-disable --> | |||
<!-- markdownlint-disable --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enough eyes on it.
btw, please setup signed commits: |
Signed-off-by: Miao, ZhiCheng <[email protected]>
* 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]>
* 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]>
* 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]>
* 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]>
* 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]>
Signed-off-by: Miao, ZhiCheng <[email protected]>
Signed-off-by: Miao, ZhiCheng <[email protected]>
Signed-off-by: Miao, ZhiCheng <[email protected]>
Signed-off-by: Miao, ZhiCheng <[email protected]>
…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]>
Signed-off-by: Miao, ZhiCheng <[email protected]>
…hedule` functions Signed-off-by: Miao, ZhiCheng <[email protected]>
…n params Signed-off-by: Miao, ZhiCheng <[email protected]>
…ateClaimableVestingScheduleFormAmountAndDuration` functions Signed-off-by: Miao, ZhiCheng <[email protected]>
* 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]>
* 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]>
Signed-off-by: Miao, ZhiCheng <[email protected]>
b3ed481
to
c6b04e7
Compare
Used |
XKCD Comic RelifLink: https://xkcd.com/1952 |
Another PR, why?
We're releasing V2 for the vesting scheduler; the initial plan was to release it with only ergonomic improvements; but we'll release it bundled with the "claimable vesting schedule" feature. This PR is an epic feature-set which upon merging will confirm the delivery of the feature.
List of Changes
totalAmount
not being perfectly divisible by the calculatedflowRate
)remainderAmount
to the storedVestingSchedule
structremainderAmount
during the early end executioncreateVestingSchedule
overload withoutbytes memory ctx
createVestingScheduleFromAmountAndDuration
function (with overload variants)createAndExecuteVestingScheduleFromAmountAndDuration
function (with overload variants) (combinescreateVestingScheduleFromAmountAndDuration
andexecuteCliffAndFlow
)host.registerAppWithKey(configWord, registrationKey);
(deprecated) withhost.registerApp(configWord);
in the constructorstring memory registrationKey
from constructor parameters (not needed anymore)0
).startDate
is below current block timestamp.cliffAndFlowDate
to be in the current block timestamp (this enables schedule creation and execution in the same transaction).getMaximumNeededTokenAllowance
utility function.mapCreateVestingScheduleParams
utility function.Included PRs
Deployments
Diffs
Interfaces (
IVestingScheduler.sol
vsIVestingSchedulerV2.sol
)Contracts (
VestingScheduler.sol
vsVestingSchedulerV2.sol
)