pallet-vesting: Support multiple, merge-able vesting schedules (paritytech#9202)

* Support multiple, mergable vesting schedules

* Update node runtime

* Remove some TODO design questions and put them as commennts

* Update frame/vesting/src/benchmarking.rs

* Syntax and comment clean up

* Create filter enum for removing schedules

* Dry vesting calls with do_vest

* Improve old benchmarks to account for max schedules

* Update WeightInfo trait and make dummy fns

* Add merge_schedule weights

* Explicitly test multiple vesting scheudles

* Make new vesting tests more more clear

* Apply suggestions from code review

* Update remove_vesting_schedule to error with no index

* Try reduce spacing diff

* Apply suggestions from code review

* Use get on vesting for bounds check; check origin first

* No filter tuple; various simplifications

* unwrap or default when getting user schedules

* spaces be gone

* ReadMe fixes

* Update frame/vesting/src/lib.rs

Co-authored-by: Peter Goodspeed-Niklaus <[email protected]>

* address some comments for docs

* merge sched docs

* Apply suggestions from code review

Co-authored-by: Shawn Tabrizi <[email protected]>

* log error when trying to push to vesting vec

* use let Some, not is_some

* remove_vesting_schedule u32, not optin

* new not try_new, create validate builder; VestingInfo

* Merge prep: break out tests and mock

* Add files forgot to include in merge

* revert some accidental changes to merged files

* Revert remaining accidental file changes

* More revert of accidental file change

* Try to reduce diff on tests

* namespace Vesting; check key when key should not exist;

* ending_block throws error on per_block of 0

* Try improve merge vesting info comment

* Update frame/vesting/src/lib.rs

Co-authored-by: Kian Paimani <[email protected]>

* add validate + correct; handle duration > blocknumber

* Move vesting_info module to its own file

* Seperate Vesting/locks updates from writing

* Add can_add_vesting schedule

* Adjust min vested transfer to be greater than all ED

* Initial integrity test impl

* merge_finished_and_yet_to_be_started_schedules

* Make sure to assert storage items are cleaned up

* Migration initial impl (not tested)

* Correct try-runtime hooks

* Apply suggestions from code review

Co-authored-by: Shawn Tabrizi <[email protected]>

* header

* WIP: improve benchmarks

* Benchmarking working

* benchmarking: step over max schedules

* cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_vesting --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/vesting/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* Simplify APIs by accepting vec; convert to bounded on write

* Test:  build_genesis_has_storage_version_v1

* Test more error cases

* Hack to get polkadot weights to work; should revert later

* Improve benchmarking; works on polkadot

* cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_vesting --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/vesting/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* WIP override storage

* Set storage not working example

* Remove unused tests

* VestingInfo: make public, derive MaxEndcodedLen

* Rename ending_block to ending_block_as_balance

* Superificial improvements

* Check for end block infinite, not just duration

* More superficial update

* Update tests

* Test vest with multi schedule

* Don't use half max balance in benchmarks

* Use debug_assert when locked is unexpected 0

* Implement exec_action

* Simplify per_block calc in vesting_info

* VestingInfo.validate in add_vesting_schedule & can_add_vesting_schedule

* Simplify post migrate check

* Remove merge event

* Minor benchmarking updates

* Remove VestingInfo.correct

* per_block accesor max with 1

* Improve comment

* Remoe debug

* Fix add schedule comment

* Apply suggestions from code review

Co-authored-by: Peter Goodspeed-Niklaus <[email protected]>

* no ref for should_remove param

* Remove unused vestingaction derive

* Asserts to show balance unlock in merge benchmark

* Remove unused imports

* trivial

* Fix benchmark asserts to handle non-multiple of 20 locked

* Add generate_storage_info

* migration :facepalm

* Remove per_block 0 logic

* Update frame/vesting/src/lib.rs

* Do not check for ending later than greatest block

* Apply suggestions from code review

* Benchmarks: simplify vesting schedule creation

* Add log back for migration

* Add note in ext docs explaining that all schedules will vest

* Make integrity test work

* Improve integrity test

* Remove unnescary type param from VestingInfo::new

* Remove unnescary resut for ending_block_as_balance

* Remove T param from ending_block_as_balance

* Reduce visibility of raw_per_block

* Remove unused type param for validate

* update old comment

* Make log a dep; log warn in migrate

* VestingInfo.validate returns Err(()), no T type param

* Try improve report_schedule_updates

* is_valid, not validate

* revert node runtime reorg;

* change schedule validity check to just warning

* Simplify merge_vesting_info return type

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Kian Paimani <[email protected]>

* Add warning for migration

* Fix indentation

* Delete duplicate warnings

* Reduce diff in node runtime

* Fix benchmark build

* Upgrade cargo.toml to use 4.0.0-dev

* Cleanup

* MaxVestingSchedulesGetter initial impl

* MinVestedTransfer getter inintial impl

* Test MaxVestingSchedules & MinVestedTransfer getters; use getters in benchmarks

* Run cargo fmt

* Revert MinVestedTransfer & MaxVestingSchedules getters; Add integrity test

* Make MAX_VESTING_SCHEDULES a const

* fmt

* WIP: benchmark improvements

* Finish benchmark update

* Add test for transfer to account with less than ed

* Rm min_new_account_transfer; move sp-io to dev-dep

* Reduce cargo.toml diff

* Explain MAX_VESTING_SCHEDULES choice

* Fix after merge

* Try fix CI complaints

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_vesting --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/vesting/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_vesting --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/vesting/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* fmt

* trigger

* fmt

Co-authored-by: Parity Bot <[email protected]>
Co-authored-by: Peter Goodspeed-Niklaus <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: kianenigma <[email protected]>

Author: 5 people

Cargo.lock

@@ -1062,6 +1062,9 @@ impl pallet_vesting::Config for Runtime {
 	type BlockNumberToBalance = ConvertInto;
 	type MinVestedTransfer = MinVestedTransfer;
 	type WeightInfo = pallet_vesting::weights::SubstrateWeight<Runtime>;
+	// `VestingInfo` encode length is 36bytes. 28 schedules gets encoded as 1009 bytes, which is the
+	// highest number of schedules that encodes less than 2^10.
+	const MAX_VESTING_SCHEDULES: u32 = 28;
 }
 
 impl pallet_mmr::Config for Runtime {

bin/node/runtime/src/lib.rs

@@ -80,8 +80,8 @@ pub trait VestingSchedule<AccountId> {
 
 	/// Adds a vesting schedule to a given account.
 	///
-	/// If there already exists a vesting schedule for the given account, an `Err` is returned
-	/// and nothing is updated.
+	/// If the account has `MaxVestingSchedules`, an Error is returned and nothing
+	/// is updated.
 	///
 	/// Is a no-op if the amount to be vested is zero.
 	///
@@ -93,8 +93,16 @@ pub trait VestingSchedule<AccountId> {
 		starting_block: Self::Moment,
 	) -> DispatchResult;
 
+	/// Checks if `add_vesting_schedule` would work against `who`.
+	fn can_add_vesting_schedule(
+		who: &AccountId,
+		locked: <Self::Currency as Currency<AccountId>>::Balance,
+		per_block: <Self::Currency as Currency<AccountId>>::Balance,
+		starting_block: Self::Moment,
+	) -> DispatchResult;
+
 	/// Remove a vesting schedule for a given account.
 	///
 	/// NOTE: This doesn't alter the free balance of the account.
-	fn remove_vesting_schedule(who: &AccountId);
+	fn remove_vesting_schedule(who: &AccountId, schedule_index: u32) -> DispatchResult;
 }

frame/support/src/traits/tokens/currency/lockable.rs

@@ -21,9 +21,10 @@ sp-runtime = { version = "4.0.0-dev", default-features = false, path = "../../pr
 frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" }
 frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" }
 frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../benchmarking", optional = true }
+log = { version = "0.4.0", default-features = false  }
 
 [dev-dependencies]
-sp-io = { version = "4.0.0-dev", path = "../../primitives/io" }
+sp-io = { version = "4.0.0-dev", default-features = false, path = "../../primitives/io" }
 sp-core = { version = "4.0.0-dev", path = "../../primitives/core" }
 pallet-balances = { version = "4.0.0-dev", path = "../balances" }