Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Multiple vesting schedules per account #7699

Conversation

raoulmillais
Copy link

@raoulmillais raoulmillais commented Dec 9, 2020

This implements allowing multiple vesting schedules per account in the vesting pallet. This will ultimately fix #7101

Please be gentle I am new to both rust and substrate! :)

Task List

Support multiple vestings per account:

  • switch storage from map of account → VestingInfo to account → Vec of VestingInfo
  • update places where we add a schedule to append
  • test summing of vesting schedules when calling vest
  • change existing schedule error to too many schedule error
  • make maximum vesting schedules per account configurable
  • test specifying multiple vesting schedules for one account in genesis config
  • test adding more than max vesting schedules errors
  • update weights to account for change from single VestingInfo to Vec of VestingInfo
  • review events in light of new functionality
  • ensure we have tests for events firing are correct and are fully covered
  • create a migration runtime upgrade following this PR as as atemplate and ensuring new pallet versioning is followed
  • test migrations using Substrate Debug Kit
  • sanity check coding style conforms to house style
  • update module documentation
  • bump minor version of pallet to 2.1.0
  • bump runtime version
  • create a CHANGELOG
  • create a PR
    • labels for pr: A0-pleasereview A7-needspolkadotpr D1-runtime-migration B3-apinoteworthy l8-enhancement
    • who to tag for review
    • run benchmarks

Support consolidating vestings in account

  • PLAN: enumerate all possible combinations of vesting schedules to define
    valid/invalid combinations for algorithm
  • new extrinsic accepting two indexes into existing vesting schedules
  • new event for consolidating vesting schedules
  • test for extrinsic happy path
  • test for invalid index returning error
  • tests for all possible invalid consolidations
  • calculate weight for new consolidate extrinsic
  • update module documentation

Companion polkadot PR

See companion PR for substrate#7040

  • Bump vestings pallet dependency version to 2.1.0
  • Ensure vestings on_runtime_upgrade is run
  • Update runtime to configure max vestings allowed per account

@raoulmillais raoulmillais added A3-in_progress Pull request is in progress. No review needed at this stage. J0-enhancement An additional feature request. labels Dec 9, 2020
@raoulmillais raoulmillais self-assigned this Dec 9, 2020
@raoulmillais raoulmillais force-pushed the multiple-vesting-schedules-per-account branch 2 times, most recently from 7a0f325 to 7fb4540 Compare December 9, 2020 16:36
@kianenigma kianenigma removed the J0-enhancement An additional feature request. label Dec 11, 2020
@raoulmillais raoulmillais force-pushed the multiple-vesting-schedules-per-account branch 5 times, most recently from 1728aca to b76844e Compare December 14, 2020 15:15
@kianenigma kianenigma self-requested a review January 12, 2021 09:43
@raoulmillais
Copy link
Author

raoulmillais commented Jan 13, 2021

Needs a companion PR and merge with master

@raoulmillais raoulmillais added the A0-please_review Pull request needs code review. label Jan 13, 2021
@raoulmillais raoulmillais marked this pull request as ready for review January 13, 2021 12:35
- Add config value for max vesting schedules (currently not honoured)
- Check for too many schedules when adding schedules instead of erroring if there is one
- Update storage with only remaining schedules (incomplete ones) after updating the lock
- Update module genesis building to support multiple vesting schedules in genesis config (sum
  unvested balances so far before adding vesing schedule and set lock to total unvested balance
  across all schedules)
- Update vesting_balance to return None if there are no vesting schedules, or sum up the vested_at
  for all vesting schedules
- Update tests for api change
- Add new tests for more than one schedule (disjoint and concurrent)
- ADds missing docs for `vested_transfer` and `force_vested_transfer`
@kianenigma
Copy link
Contributor

I suppose unfortunately this will not be finished, correct?

@kianenigma kianenigma added A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. and removed A0-please_review Pull request needs code review. A3-in_progress Pull request is in progress. No review needed at this stage. labels Feb 19, 2021
@gnunicorn gnunicorn closed this Mar 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Multiple Vesting Schedules in Vesting Pallet
3 participants