From fcaf225fb2c78305b8277d7ef1279a8bce69eac6 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Thu, 2 Mar 2023 16:49:08 +0100 Subject: [PATCH 01/15] vesting --- frame/society/src/mock.rs | 7 +++++-- frame/vesting/src/lib.rs | 24 ++++++++++++++++++++++++ frame/vesting/src/mock.rs | 5 ++++- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/frame/society/src/mock.rs b/frame/society/src/mock.rs index 24544bf9e82dd..61d17d1d96607 100644 --- a/frame/society/src/mock.rs +++ b/frame/society/src/mock.rs @@ -142,7 +142,7 @@ impl EnvBuilder { } } - pub fn execute R>(mut self, f: F) -> R { + pub fn execute(mut self, f: impl FnOnce() -> ()) { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); self.balances.push((Society::account_id(), self.balance.max(self.pot))); pallet_balances::GenesisConfig:: { balances: self.balances } @@ -156,8 +156,11 @@ impl EnvBuilder { .assimilate_storage(&mut t) .unwrap(); let mut ext: sp_io::TestExternalities = t.into(); - ext.execute_with(f) + ext.execute_with(|| { + f(); + }) } + #[allow(dead_code)] pub fn with_members(mut self, m: Vec) -> Self { self.members = m; diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index 6cde546f387b4..f8ef959a5d472 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -192,6 +192,11 @@ pub mod pallet { fn integrity_test() { assert!(T::MAX_VESTING_SCHEDULES > 0, "`MaxVestingSchedules` must ge greater than 0"); } + + #[cfg(feature = "try-runtime")] + fn try_state(_n: BlockNumberFor) -> Result<(), &'static str> { + Self::do_try_state() + } } /// Information regarding the vesting of a given account. @@ -649,6 +654,25 @@ impl Pallet { Ok((schedules, locked_now)) } + + #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] + pub fn do_try_state() -> Result<(), &'static str> { + + Vesting::::iter().for_each(|(_, d)| { + let vesting = d.to_vec(); + let mut total_per_block: BalanceOf = Zero::zero(); + let mut total_locked_now: BalanceOf = Zero::zero(); + for info in vesting.iter() { + // get the number of remaining vesting blocks<>balance + let payments_left: BalanceOf = info.ending_block_as_balance::(); + total_per_block += info.per_block().saturating_mul(payments_left); + total_locked_now += info.locked_at::(>::block_number()); + }; + let one_extra = total_per_block.saturating_sub(total_locked_now); + assert_eq!(total_per_block, total_locked_now.saturating_add(one_extra)); + }); + Ok(()) + } } impl VestingSchedule for Pallet diff --git a/frame/vesting/src/mock.rs b/frame/vesting/src/mock.rs index da9490bea66c0..02601a96f785e 100644 --- a/frame/vesting/src/mock.rs +++ b/frame/vesting/src/mock.rs @@ -149,7 +149,10 @@ impl ExtBuilder { .assimilate_storage(&mut t) .unwrap(); let mut ext = sp_io::TestExternalities::new(t); - ext.execute_with(|| System::set_block_number(1)); + ext.execute_with(|| { + System::set_block_number(1); + Vesting::do_try_state().unwrap(); + }); ext } } From 70e6c00635f7f1ea78e0f93698db5256aa33fa53 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sat, 11 Mar 2023 09:40:55 +0100 Subject: [PATCH 02/15] Tips --- frame/tips/src/lib.rs | 16 ++++++++++++++++ frame/tips/src/tests.rs | 13 +++++++++++++ 2 files changed, 29 insertions(+) diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index 175fc3ac39d02..bd0afd008d2f1 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -453,6 +453,14 @@ pub mod pallet { Ok(()) } } + + #[pallet::hooks] + impl, I: 'static> Hooks> for Pallet { + #[cfg(feature = "try-runtime")] + fn try_state(_n: BlockNumberFor) -> Result<(), &'static str> { + Self::do_try_state() + } + } } impl, I: 'static> Pallet { @@ -599,4 +607,12 @@ impl, I: 'static> Pallet { Tips::::insert(hash, new_tip) } } + + #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] + pub fn do_try_state() -> Result<(), &'static str> { + let reasons = Reasons::::iter_keys().collect::>(); + let tips = Tips::::iter_keys().collect::>(); + assert_eq!(reasons.len(), tips.len()); + Ok(()) + } } diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index cb0b4458c7fba..e0bad8c8f2658 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -235,6 +235,7 @@ fn tip_new_cannot_be_used_twice() { Tips::tip_new(RuntimeOrigin::signed(11), b"awesome.dot".to_vec(), 3, 10), Error::::AlreadyKnown ); + Tips::do_try_state().unwrap(); }); } @@ -262,6 +263,7 @@ fn report_awesome_and_tip_works() { assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 102); assert_eq!(Balances::free_balance(3), 8); + Tips::do_try_state().unwrap(); }); } @@ -280,6 +282,7 @@ fn report_awesome_from_beneficiary_and_tip_works() { assert_ok!(Tips::close_tip(RuntimeOrigin::signed(100), h.into())); assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 110); + Tips::do_try_state().unwrap(); }); } @@ -318,6 +321,8 @@ fn close_tip_works() { Tips::close_tip(RuntimeOrigin::signed(100), h.into()), Error::::UnknownTip ); + + Tips::do_try_state().unwrap(); }); } @@ -349,6 +354,8 @@ fn slash_tip_works() { // tipper slashed assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 88); + + Tips::do_try_state().unwrap(); }); } @@ -383,6 +390,8 @@ fn retract_tip_works() { Tips::close_tip(RuntimeOrigin::signed(10), h.into()), Error::::UnknownTip ); + + Tips::do_try_state().unwrap(); }); } @@ -397,6 +406,7 @@ fn tip_median_calculation_works() { System::set_block_number(2); assert_ok!(Tips::close_tip(RuntimeOrigin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); + Tips::do_try_state().unwrap(); }); } @@ -416,6 +426,7 @@ fn tip_changing_works() { System::set_block_number(2); assert_ok!(Tips::close_tip(RuntimeOrigin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); + Tips::do_try_state().unwrap(); }); } @@ -613,5 +624,7 @@ fn report_awesome_and_tip_works_second_instance() { assert_eq!(Balances::free_balance(&Treasury::account_id()), 101); // Treasury 2 gave the funds assert_eq!(Balances::free_balance(&Treasury1::account_id()), 191); + + Tips::do_try_state().unwrap(); }); } From 7f01672af05eb66fa4009a599192fcfc8b878fb3 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Mon, 20 Mar 2023 07:06:18 +0100 Subject: [PATCH 03/15] documentation, revert society --- frame/society/src/mock.rs | 6 ++---- frame/tips/src/lib.rs | 7 +++++++ frame/vesting/src/lib.rs | 8 ++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/frame/society/src/mock.rs b/frame/society/src/mock.rs index 61d17d1d96607..57ff3aa8ab9fc 100644 --- a/frame/society/src/mock.rs +++ b/frame/society/src/mock.rs @@ -142,7 +142,7 @@ impl EnvBuilder { } } - pub fn execute(mut self, f: impl FnOnce() -> ()) { + pub fn execute R>(mut self, f: F) -> R { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); self.balances.push((Society::account_id(), self.balance.max(self.pot))); pallet_balances::GenesisConfig:: { balances: self.balances } @@ -156,9 +156,7 @@ impl EnvBuilder { .assimilate_storage(&mut t) .unwrap(); let mut ext: sp_io::TestExternalities = t.into(); - ext.execute_with(|| { - f(); - }) + ext.execute_with(f) } #[allow(dead_code)] diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index bd0afd008d2f1..4fafacbce0cb6 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -608,6 +608,13 @@ impl, I: 'static> Pallet { } } + /// Ensure the correctness of the state of this pallet. + /// + /// This should be valid before or after each state transition of this pallet. + /// + /// ## Invariants: + /// + /// * `Reasons` and `Tips` must have the same length of keys #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] pub fn do_try_state() -> Result<(), &'static str> { let reasons = Reasons::::iter_keys().collect::>(); diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index f8ef959a5d472..4a7d7b0d31c83 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -655,6 +655,14 @@ impl Pallet { Ok((schedules, locked_now)) } + /// Ensure the correctness of the state of this pallet. + /// + /// This should be valid before or after each state transition of this pallet. + /// + /// ## Invariants: + /// + /// * The `per_block` payments left of every account currently vesting + /// * is equal to the total balance currently locked. #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] pub fn do_try_state() -> Result<(), &'static str> { From 34800d89779dd87473154362a2e8b59f839a9bfe Mon Sep 17 00:00:00 2001 From: doordashcon Date: Mon, 20 Mar 2023 07:08:39 +0100 Subject: [PATCH 04/15] revert society --- frame/society/src/mock.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/society/src/mock.rs b/frame/society/src/mock.rs index 57ff3aa8ab9fc..24544bf9e82dd 100644 --- a/frame/society/src/mock.rs +++ b/frame/society/src/mock.rs @@ -158,7 +158,6 @@ impl EnvBuilder { let mut ext: sp_io::TestExternalities = t.into(); ext.execute_with(f) } - #[allow(dead_code)] pub fn with_members(mut self, m: Vec) -> Self { self.members = m; From e3194a10da25cdd3e7cc960953f656461049c1b7 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Mon, 20 Mar 2023 18:14:07 +0100 Subject: [PATCH 05/15] cargo fmt --- frame/tips/src/lib.rs | 2 +- frame/tips/src/tests.rs | 8 ++++---- frame/vesting/src/lib.rs | 11 ++++++----- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index 4fafacbce0cb6..1915b0201144c 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -613,7 +613,7 @@ impl, I: 'static> Pallet { /// This should be valid before or after each state transition of this pallet. /// /// ## Invariants: - /// + /// /// * `Reasons` and `Tips` must have the same length of keys #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] pub fn do_try_state() -> Result<(), &'static str> { diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index e0bad8c8f2658..b1a3fe98b0b4d 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -321,7 +321,7 @@ fn close_tip_works() { Tips::close_tip(RuntimeOrigin::signed(100), h.into()), Error::::UnknownTip ); - + Tips::do_try_state().unwrap(); }); } @@ -354,7 +354,7 @@ fn slash_tip_works() { // tipper slashed assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 88); - + Tips::do_try_state().unwrap(); }); } @@ -390,7 +390,7 @@ fn retract_tip_works() { Tips::close_tip(RuntimeOrigin::signed(10), h.into()), Error::::UnknownTip ); - + Tips::do_try_state().unwrap(); }); } @@ -624,7 +624,7 @@ fn report_awesome_and_tip_works_second_instance() { assert_eq!(Balances::free_balance(&Treasury::account_id()), 101); // Treasury 2 gave the funds assert_eq!(Balances::free_balance(&Treasury1::account_id()), 191); - + Tips::do_try_state().unwrap(); }); } diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index 4a7d7b0d31c83..08077daec0dab 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -660,22 +660,23 @@ impl Pallet { /// This should be valid before or after each state transition of this pallet. /// /// ## Invariants: - /// + /// /// * The `per_block` payments left of every account currently vesting /// * is equal to the total balance currently locked. #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] pub fn do_try_state() -> Result<(), &'static str> { - Vesting::::iter().for_each(|(_, d)| { let vesting = d.to_vec(); let mut total_per_block: BalanceOf = Zero::zero(); let mut total_locked_now: BalanceOf = Zero::zero(); for info in vesting.iter() { // get the number of remaining vesting blocks<>balance - let payments_left: BalanceOf = info.ending_block_as_balance::(); + let payments_left: BalanceOf = + info.ending_block_as_balance::(); total_per_block += info.per_block().saturating_mul(payments_left); - total_locked_now += info.locked_at::(>::block_number()); - }; + total_locked_now += info + .locked_at::(>::block_number()); + } let one_extra = total_per_block.saturating_sub(total_locked_now); assert_eq!(total_per_block, total_locked_now.saturating_add(one_extra)); }); From bed7f694f0f57e130d7a71b163623c845837d23d Mon Sep 17 00:00:00 2001 From: doordashcon Date: Thu, 6 Apr 2023 22:27:21 +0100 Subject: [PATCH 06/15] attribute fix --- frame/tips/src/lib.rs | 2 +- frame/vesting/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index 1915b0201144c..c6f80eaedeea9 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -615,7 +615,7 @@ impl, I: 'static> Pallet { /// ## Invariants: /// /// * `Reasons` and `Tips` must have the same length of keys - #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] + #[cfg(any(feature = "try-runtime", test))] pub fn do_try_state() -> Result<(), &'static str> { let reasons = Reasons::::iter_keys().collect::>(); let tips = Tips::::iter_keys().collect::>(); diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index 08077daec0dab..a1b1ee4a4d977 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -663,7 +663,7 @@ impl Pallet { /// /// * The `per_block` payments left of every account currently vesting /// * is equal to the total balance currently locked. - #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] + #[cfg(any(feature = "try-runtime", test))] pub fn do_try_state() -> Result<(), &'static str> { Vesting::::iter().for_each(|(_, d)| { let vesting = d.to_vec(); From 786f375873edc8bc16ba394f9ce0bd0516826723 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Thu, 4 May 2023 09:11:22 +0100 Subject: [PATCH 07/15] improve vesting docs & Tips try-state --- frame/tips/src/lib.rs | 17 +++++++++++------ frame/tips/src/tests.rs | 18 ++++++++---------- frame/vesting/src/lib.rs | 38 +++++++++++++++++++++++--------------- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index 27ce0bdf2af7d..b2fc803781a87 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -253,8 +253,8 @@ pub mod pallet { let hash = T::Hashing::hash_of(&(&reason_hash, &who)); ensure!(!Tips::::contains_key(&hash), Error::::AlreadyKnown); - let deposit = T::TipReportDepositBase::get() + - T::DataDepositPerByte::get() * (reason.len() as u32).into(); + let deposit = T::TipReportDepositBase::get() + + T::DataDepositPerByte::get() * (reason.len() as u32).into(); T::Currency::reserve(&finder, deposit)?; Reasons::::insert(&reason_hash, &reason); @@ -508,9 +508,9 @@ impl, I: 'static> Pallet { Some(m) => { member = members_iter.next(); if m < a { - continue + continue; } else { - break true + break true; } }, } @@ -613,12 +613,17 @@ impl, I: 'static> Pallet { /// /// ## Invariants: /// - /// * `Reasons` and `Tips` must have the same length of keys + /// * There should be a corresponding `OpenTip.reason` for each key in `Reasons`. #[cfg(any(feature = "try-runtime", test))] pub fn do_try_state() -> Result<(), &'static str> { let reasons = Reasons::::iter_keys().collect::>(); let tips = Tips::::iter_keys().collect::>(); - assert_eq!(reasons.len(), tips.len()); + + for reason in reasons { + for tip in &tips { + assert_eq!(reason, Tips::::get(tip).unwrap().reason); + } + } Ok(()) } } diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 20fc4cc0781da..825e50df9b368 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -263,11 +263,11 @@ fn report_awesome_and_tip_works() { assert_ok!(Tips::tip(RuntimeOrigin::signed(12), h, 10)); assert_noop!(Tips::tip(RuntimeOrigin::signed(9), h, 10), BadOrigin); System::set_block_number(2); + Tips::do_try_state().unwrap(); assert_ok!(Tips::close_tip(RuntimeOrigin::signed(100), h.into())); assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 102); assert_eq!(Balances::free_balance(3), 8); - Tips::do_try_state().unwrap(); }); } @@ -283,10 +283,10 @@ fn report_awesome_from_beneficiary_and_tip_works() { assert_ok!(Tips::tip(RuntimeOrigin::signed(11), h, 10)); assert_ok!(Tips::tip(RuntimeOrigin::signed(12), h, 10)); System::set_block_number(2); + Tips::do_try_state().unwrap(); assert_ok!(Tips::close_tip(RuntimeOrigin::signed(100), h.into())); assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 110); - Tips::do_try_state().unwrap(); }); } @@ -315,6 +315,7 @@ fn close_tip_works() { assert_noop!(Tips::close_tip(RuntimeOrigin::signed(0), h.into()), Error::::Premature); System::set_block_number(2); + Tips::do_try_state().unwrap(); assert_noop!(Tips::close_tip(RuntimeOrigin::none(), h.into()), BadOrigin); assert_ok!(Tips::close_tip(RuntimeOrigin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); @@ -325,8 +326,6 @@ fn close_tip_works() { Tips::close_tip(RuntimeOrigin::signed(100), h.into()), Error::::UnknownTip ); - - Tips::do_try_state().unwrap(); }); } @@ -352,14 +351,13 @@ fn slash_tip_works() { assert_noop!(Tips::slash_tip(RuntimeOrigin::signed(0), h), BadOrigin); // can remove from root. + Tips::do_try_state().unwrap(); assert_ok!(Tips::slash_tip(RuntimeOrigin::root(), h)); assert_eq!(last_event(), TipEvent::TipSlashed { tip_hash: h, finder: 0, deposit: 12 }); // tipper slashed assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 88); - - Tips::do_try_state().unwrap(); }); } @@ -373,6 +371,7 @@ fn retract_tip_works() { assert_ok!(Tips::tip(RuntimeOrigin::signed(10), h, 10)); assert_ok!(Tips::tip(RuntimeOrigin::signed(11), h, 10)); assert_ok!(Tips::tip(RuntimeOrigin::signed(12), h, 10)); + Tips::do_try_state().unwrap(); assert_noop!(Tips::retract_tip(RuntimeOrigin::signed(10), h), Error::::NotFinder); assert_ok!(Tips::retract_tip(RuntimeOrigin::signed(0), h)); System::set_block_number(2); @@ -387,6 +386,7 @@ fn retract_tip_works() { let h = tip_hash(); assert_ok!(Tips::tip(RuntimeOrigin::signed(11), h, 10)); assert_ok!(Tips::tip(RuntimeOrigin::signed(12), h, 10)); + Tips::do_try_state().unwrap(); assert_noop!(Tips::retract_tip(RuntimeOrigin::signed(0), h), Error::::NotFinder); assert_ok!(Tips::retract_tip(RuntimeOrigin::signed(10), h)); System::set_block_number(2); @@ -394,8 +394,6 @@ fn retract_tip_works() { Tips::close_tip(RuntimeOrigin::signed(10), h.into()), Error::::UnknownTip ); - - Tips::do_try_state().unwrap(); }); } @@ -408,9 +406,9 @@ fn tip_median_calculation_works() { assert_ok!(Tips::tip(RuntimeOrigin::signed(11), h, 10)); assert_ok!(Tips::tip(RuntimeOrigin::signed(12), h, 1000000)); System::set_block_number(2); + Tips::do_try_state().unwrap(); assert_ok!(Tips::close_tip(RuntimeOrigin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); - Tips::do_try_state().unwrap(); }); } @@ -428,9 +426,9 @@ fn tip_changing_works() { assert_ok!(Tips::tip(RuntimeOrigin::signed(11), h, 100)); assert_ok!(Tips::tip(RuntimeOrigin::signed(10), h, 10)); System::set_block_number(2); + Tips::do_try_state().unwrap(); assert_ok!(Tips::close_tip(RuntimeOrigin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); - Tips::do_try_state().unwrap(); }); } diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index e893390f943ad..39ec1ecedd355 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -421,7 +421,7 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; if schedule1_index == schedule2_index { - return Ok(()) + return Ok(()); }; let schedule1_index = schedule1_index as usize; let schedule2_index = schedule2_index as usize; @@ -499,7 +499,7 @@ impl Pallet { // Validate user inputs. ensure!(schedule.locked() >= T::MinVestedTransfer::get(), Error::::AmountLow); if !schedule.is_valid() { - return Err(Error::::InvalidScheduleParams.into()) + return Err(Error::::InvalidScheduleParams.into()); }; let target = T::Lookup::lookup(target)?; let source = T::Lookup::lookup(source)?; @@ -647,8 +647,8 @@ impl Pallet { }; debug_assert!( - locked_now > Zero::zero() && schedules.len() > 0 || - locked_now == Zero::zero() && schedules.len() == 0 + locked_now > Zero::zero() && schedules.len() > 0 + || locked_now == Zero::zero() && schedules.len() == 0 ); Ok((schedules, locked_now)) @@ -660,24 +660,32 @@ impl Pallet { /// /// ## Invariants: /// - /// * The `per_block` payments left of every account currently vesting - /// * is equal to the total balance currently locked. + /// For each account currently vesting: + /// * The `per_block` amount left to be claimed is equal to the + /// total balance currently locked. + /// + /// *`total_per_block` is the total amount left to be unlocked after the starting block. + /// *`total_locked` is the total amount locked. + /// + /// * `one_extra` accounts for the extra block that + /// will be added to the vesting schedule if `per_block` is bigger than `locked`. #[cfg(any(feature = "try-runtime", test))] pub fn do_try_state() -> Result<(), &'static str> { Vesting::::iter().for_each(|(_, d)| { let vesting = d.to_vec(); let mut total_per_block: BalanceOf = Zero::zero(); - let mut total_locked_now: BalanceOf = Zero::zero(); + let mut total_locked: BalanceOf = Zero::zero(); for info in vesting.iter() { // get the number of remaining vesting blocks<>balance - let payments_left: BalanceOf = + let amount_left: BalanceOf = info.ending_block_as_balance::(); - total_per_block += info.per_block().saturating_mul(payments_left); - total_locked_now += info + total_per_block += info.per_block().saturating_mul(amount_left); + // get the total block<>balance still locked. + total_locked += info .locked_at::(>::block_number()); } - let one_extra = total_per_block.saturating_sub(total_locked_now); - assert_eq!(total_per_block, total_locked_now.saturating_add(one_extra)); + let one_extra = total_per_block.saturating_sub(total_locked); + assert_eq!(total_per_block, total_locked.saturating_add(one_extra)); }); Ok(()) } @@ -722,13 +730,13 @@ where starting_block: T::BlockNumber, ) -> DispatchResult { if locked.is_zero() { - return Ok(()) + return Ok(()); } let vesting_schedule = VestingInfo::new(locked, per_block, starting_block); // Check for `per_block` or `locked` of 0. if !vesting_schedule.is_valid() { - return Err(Error::::InvalidScheduleParams.into()) + return Err(Error::::InvalidScheduleParams.into()); }; let mut schedules = Self::vesting(who).unwrap_or_default(); @@ -756,7 +764,7 @@ where ) -> DispatchResult { // Check for `per_block` or `locked` of 0. if !VestingInfo::new(locked, per_block, starting_block).is_valid() { - return Err(Error::::InvalidScheduleParams.into()) + return Err(Error::::InvalidScheduleParams.into()); } ensure!( From d9e0f3a8487038a0d5a644ce5be059b5b67ec243 Mon Sep 17 00:00:00 2001 From: Doordashcon Date: Mon, 15 May 2023 15:27:05 +0000 Subject: [PATCH 08/15] pepper conditional --- frame/vesting/src/lib.rs | 12 ++++++------ frame/vesting/src/tests.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index 39ec1ecedd355..a5cf187bedf2d 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -664,8 +664,8 @@ impl Pallet { /// * The `per_block` amount left to be claimed is equal to the /// total balance currently locked. /// - /// *`total_per_block` is the total amount left to be unlocked after the starting block. - /// *`total_locked` is the total amount locked. + /// *`total_to_vest` the total amount left to vest over all remaining blocks. + /// *`total_locked` the total amount locked. /// /// * `one_extra` accounts for the extra block that /// will be added to the vesting schedule if `per_block` is bigger than `locked`. @@ -673,19 +673,19 @@ impl Pallet { pub fn do_try_state() -> Result<(), &'static str> { Vesting::::iter().for_each(|(_, d)| { let vesting = d.to_vec(); - let mut total_per_block: BalanceOf = Zero::zero(); + let mut total_to_vest: BalanceOf = Zero::zero(); let mut total_locked: BalanceOf = Zero::zero(); for info in vesting.iter() { // get the number of remaining vesting blocks<>balance let amount_left: BalanceOf = info.ending_block_as_balance::(); - total_per_block += info.per_block().saturating_mul(amount_left); + total_to_vest += info.per_block().saturating_mul(amount_left); // get the total block<>balance still locked. total_locked += info .locked_at::(>::block_number()); } - let one_extra = total_per_block.saturating_sub(total_locked); - assert_eq!(total_per_block, total_locked.saturating_add(one_extra)); + let one_extra = total_to_vest.saturating_sub(total_locked); + assert_eq!(total_to_vest, total_locked.saturating_add(one_extra)); }); Ok(()) } diff --git a/frame/vesting/src/tests.rs b/frame/vesting/src/tests.rs index 46afe895f6fcc..da93ff3909716 100644 --- a/frame/vesting/src/tests.rs +++ b/frame/vesting/src/tests.rs @@ -75,6 +75,7 @@ fn check_vesting_status() { // Account 12 has only their illiquid funds locked assert_eq!(Vesting::vesting_balance(&12), Some(user12_free_balance - ED * 5)); + assert_ok!(Vesting::do_try_state()); System::set_block_number(10); assert_eq!(System::block_number(), 10); @@ -85,6 +86,7 @@ fn check_vesting_status() { // Account 12 has started vesting by block 10 assert_eq!(Vesting::vesting_balance(&12), Some(user12_free_balance - ED * 5)); + assert_ok!(Vesting::do_try_state()); System::set_block_number(30); assert_eq!(System::block_number(), 30); @@ -139,6 +141,7 @@ fn check_vesting_status_for_multi_schedule_account() { ); assert_ok!(Vesting::vested_transfer(Some(4).into(), 2, sched2)); + assert_ok!(Vesting::do_try_state()); System::set_block_number(9); // Free balance is equal to the 3 existing schedules total amount. let free_balance = Balances::free_balance(&2); @@ -149,6 +152,7 @@ fn check_vesting_status_for_multi_schedule_account() { Some(free_balance - sched1.per_block() * 9 - sched2.per_block() * 4) ); + assert_ok!(Vesting::do_try_state()); System::set_block_number(20); // At block #20 sched1 is fully unlocked while sched2 and sched0 are partially unlocked. assert_eq!( @@ -158,6 +162,7 @@ fn check_vesting_status_for_multi_schedule_account() { ) ); + assert_ok!(Vesting::do_try_state()); System::set_block_number(30); // At block #30 sched0 and sched1 are fully unlocked while sched2 is partially unlocked. assert_eq!( @@ -166,6 +171,7 @@ fn check_vesting_status_for_multi_schedule_account() { ); // At block #35 sched2 fully unlocks and thus all schedules funds are unlocked. + assert_ok!(Vesting::do_try_state()); System::set_block_number(35); assert_eq!(Vesting::vesting_balance(&2), Some(0)); // Since we have not called any extrinsics that would unlock funds the schedules @@ -337,12 +343,14 @@ fn vested_transfer_works() { // Account 4 has 5 * 256 locked. assert_eq!(Vesting::vesting_balance(&4), Some(256 * 5)); + assert_ok!(Vesting::do_try_state()); System::set_block_number(20); assert_eq!(System::block_number(), 20); // Account 4 has 5 * 64 units vested by block 20. assert_eq!(Vesting::vesting_balance(&4), Some(10 * 64)); + assert_ok!(Vesting::do_try_state()); System::set_block_number(30); assert_eq!(System::block_number(), 30); @@ -432,6 +440,7 @@ fn vested_transfer_allows_max_schedules() { assert_eq!(Balances::free_balance(&4), user_4_free_balance); // Account 4 has fully vested when all the schedules end, + assert_ok!(Vesting::do_try_state()); System::set_block_number( ::MinVestedTransfer::get() + sched.starting_block(), ); @@ -478,12 +487,14 @@ fn force_vested_transfer_works() { // Account 4 has 5 * ED locked. assert_eq!(Vesting::vesting_balance(&4), Some(ED * 5)); + assert_ok!(Vesting::do_try_state()); System::set_block_number(20); assert_eq!(System::block_number(), 20); // Account 4 has 5 * 64 units vested by block 20. assert_eq!(Vesting::vesting_balance(&4), Some(10 * 64)); + assert_ok!(Vesting::do_try_state()); System::set_block_number(30); assert_eq!(System::block_number(), 30); @@ -577,6 +588,7 @@ fn force_vested_transfer_allows_max_schedules() { assert_eq!(Balances::free_balance(&4), user_4_free_balance); // Account 4 has fully vested when all the schedules end, + assert_ok!(Vesting::do_try_state()); System::set_block_number(::MinVestedTransfer::get() + 10); assert_eq!(Vesting::vesting_balance(&4), Some(0)); // and after unlocking its schedules are removed from storage. @@ -601,6 +613,7 @@ fn merge_schedules_that_have_not_started() { assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched0]); assert_eq!(Balances::usable_balance(&2), 0); assert_ok!(Vesting::merge_schedules(Some(2).into(), 0, 1)); + assert_ok!(Vesting::do_try_state()); // Since we merged identical schedules, the new schedule finishes at the same // time as the original, just with double the amount. @@ -644,6 +657,7 @@ fn merge_ongoing_schedules() { assert_eq!(Balances::usable_balance(&2), 0); assert_ok!(Vesting::merge_schedules(Some(2).into(), 0, 1)); + assert_ok!(Vesting::do_try_state()); // Merging schedules un-vests all pre-existing schedules prior to merging, which is // reflected in account 2's updated usable balance. @@ -668,6 +682,7 @@ fn merge_ongoing_schedules() { assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched2]); // And just to double check, we assert the new merged schedule we be cleaned up as expected. + assert_ok!(Vesting::do_try_state()); System::set_block_number(30); vest_and_assert_no_vesting::(2); }); @@ -714,6 +729,7 @@ fn merging_shifts_other_schedules_index() { assert_eq!(usable_balance, Balances::usable_balance(&3)); assert_ok!(Vesting::merge_schedules(Some(3).into(), 0, 2)); + assert_ok!(Vesting::do_try_state()); // Create the merged schedule of sched0 & sched2. // The merged schedule will have the max possible starting block, @@ -767,6 +783,7 @@ fn merge_ongoing_and_yet_to_be_started_schedules() { assert_eq!(Balances::usable_balance(&2), usable_balance); // Go forward a block. + assert_ok!(Vesting::do_try_state()); cur_block += 1; System::set_block_number(cur_block); @@ -780,6 +797,7 @@ fn merge_ongoing_and_yet_to_be_started_schedules() { // Merge the schedules before sched1 starts. assert_ok!(Vesting::merge_schedules(Some(2).into(), 0, 1)); + assert_ok!(Vesting::do_try_state()); // After merging, the usable balance only changes by the amount sched0 vested since we // last called `vest` (which is just 1 block). The usable balance is not affected by // sched1 because it has not started yet. @@ -799,6 +817,7 @@ fn merge_ongoing_and_yet_to_be_started_schedules() { let sched2_per_block = sched2_locked / sched2_duration; let sched2 = VestingInfo::new(sched2_locked, sched2_per_block, sched2_start); + assert_ok!(Vesting::do_try_state()); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched2]); }); } @@ -839,11 +858,13 @@ fn merge_finished_and_ongoing_schedules() { let cur_block = sched0.ending_block_as_balance::(); System::set_block_number(cur_block); assert_eq!(System::block_number(), 30); + assert_ok!(Vesting::do_try_state()); // Prior to `merge_schedules` and with no vest/vest_other called the user has no usable // balance. assert_eq!(Balances::usable_balance(&2), 0); assert_ok!(Vesting::merge_schedules(Some(2).into(), 0, 1)); + assert_ok!(Vesting::do_try_state()); // sched2 is now the first, since sched0 & sched1 get filtered out while "merging". // sched1 gets treated like the new merged schedule by getting pushed onto back @@ -893,6 +914,7 @@ fn merge_finishing_schedules_does_not_create_a_new_one() { assert_eq!(all_scheds_end, 40); System::set_block_number(all_scheds_end); + assert_ok!(Vesting::do_try_state()); // Prior to merge_schedules and with no vest/vest_other called the user has no usable // balance. @@ -900,6 +922,7 @@ fn merge_finishing_schedules_does_not_create_a_new_one() { // Merge schedule 0 and 1. assert_ok!(Vesting::merge_schedules(Some(2).into(), 0, 1)); + assert_ok!(Vesting::do_try_state()); // The user no longer has any more vesting schedules because they both ended at the // block they where merged, assert!(!>::contains_key(&2)); @@ -927,6 +950,7 @@ fn merge_finished_and_yet_to_be_started_schedules() { ); assert_ok!(Vesting::vested_transfer(Some(13).into(), 2, sched1)); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1]); + assert_ok!(Vesting::do_try_state()); let sched2 = VestingInfo::new( ED * 40, @@ -936,8 +960,10 @@ fn merge_finished_and_yet_to_be_started_schedules() { // Add a 3rd schedule to demonstrate how sched1 shifts. assert_ok!(Vesting::vested_transfer(Some(13).into(), 2, sched2)); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1, sched2]); + assert_ok!(Vesting::do_try_state()); System::set_block_number(30); + assert_ok!(Vesting::do_try_state()); // At block 30, sched0 has finished unlocking while sched1 and sched2 are still fully // locked, @@ -947,6 +973,7 @@ fn merge_finished_and_yet_to_be_started_schedules() { // Merge schedule 0 and 1. assert_ok!(Vesting::merge_schedules(Some(2).into(), 0, 1)); + assert_ok!(Vesting::do_try_state()); // sched0 is removed since it finished, and sched1 is removed and then pushed on the back // because it is treated as the merged schedule From 42410fe5f94c9eed8fa96065ea8ecf1a4e832b5a Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 30 Jul 2023 22:56:55 +0100 Subject: [PATCH 09/15] WIP --- frame/vesting/src/lib.rs | 76 +++++++++++++------- frame/vesting/src/mock.rs | 1 + frame/vesting/src/tests.rs | 138 +++++++++++++++++++++++++++++-------- 3 files changed, 164 insertions(+), 51 deletions(-) diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index 8a565f35f8c52..a0f3c4162e4e9 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -194,7 +194,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> Result<(), &'static str> { + fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { Self::do_try_state() } } @@ -244,6 +244,7 @@ pub mod pallet { let locked = balance.saturating_sub(liquid); let length_as_balance = T::BlockNumberToBalance::convert(length); let per_block = locked / length_as_balance.max(sp_runtime::traits::One::one()); + println!("{:?}", per_block); let vesting_info = VestingInfo::new(locked, per_block, begin); if !vesting_info.is_valid() { panic!("Invalid VestingInfo params at genesis") @@ -650,36 +651,65 @@ impl Pallet { /// Ensure the correctness of the state of this pallet. /// - /// This should be valid before or after each state transition of this pallet. + /// The following expectations must always apply. /// - /// ## Invariants: + /// ## Expectations: /// - /// For each account currently vesting: - /// * The `per_block` amount left to be claimed is equal to the - /// total balance currently locked. + /// Before schedules begin: + /// * the locked amount(`still_vesting`) of a vesting schedule must be equal to the + /// product of the duration(`schedules_left` - `starting_block`) and the per block amount when the locked + /// amount is divisible by the per block amount. /// - /// *`total_to_vest` the total amount left to vest over all remaining blocks. - /// *`total_locked` the total amount locked. - /// - /// * `one_extra` accounts for the extra block that - /// will be added to the vesting schedule if `per_block` is bigger than `locked`. + /// During schedule timeframe: + /// * the amount `still_vesting` must be equal to the product of the remaining blocks to vest(`schedules_left` - `current_block`) + /// and per block amount when the locked amount is divisible by the per block amount. + /// * However, if the locked amount is not divisible by the per block amount, + /// at the final vesting block the remainder must equal the amount `still_vesting`. #[cfg(any(feature = "try-runtime", test))] - pub fn do_try_state() -> Result<(), &'static str> { + pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { Vesting::::iter().for_each(|(_, d)| { - let vesting = d.to_vec(); - let mut total_to_vest: BalanceOf = Zero::zero(); - let mut total_locked: BalanceOf = Zero::zero(); - for info in vesting.iter() { - // get the number of remaining vesting blocks<>balance - let amount_left: BalanceOf = + let infos = d.to_vec(); + + for info in infos.iter() { + let schedules_left: BalanceOf = info.ending_block_as_balance::(); - total_to_vest += info.per_block().saturating_mul(amount_left); - // get the total block<>balance still locked. - total_locked += info + let starting_block = T::BlockNumberToBalance::convert(info.starting_block()); + + let still_vesting = info .locked_at::(>::block_number()); + let current_block = + T::BlockNumberToBalance::convert(>::block_number()); + + if current_block < starting_block { + let count = schedules_left.saturating_sub(starting_block); + if (info.locked() % info.per_block()).is_zero() { + assert!(still_vesting == (count * info.per_block()), "uninitiated schedule, the vesting balance is unequal with the total raw per block releases."); + } else { + let re = info.locked() % info.per_block(); + assert!(info.per_block() > re, "uinitiated schedule, the raw per block is greater than remainder"); + } + } else { + if (info.locked() % info.per_block()).is_zero() { + assert!( + still_vesting + == (schedules_left.saturating_sub(current_block) + * info.per_block()), + "Schedule started, the vesting balance is unequal with the total per block releases" + ); + } else { + let re = info.locked() % info.per_block(); + assert!(info.per_block() > re, "Schedule started, per block is less than remainder"); + + if current_block == schedules_left.saturating_sub(One::one()) { + assert!(still_vesting == re, "Schedule started, at the final vesting block the vesting balance is unequal to the remainder"); + } + + if current_block == schedules_left { + assert!(still_vesting == Zero::zero(), "Schedule ended, no more vesting balance"); + } + } + } } - let one_extra = total_to_vest.saturating_sub(total_locked); - assert_eq!(total_to_vest, total_locked.saturating_add(one_extra)); }); Ok(()) } diff --git a/frame/vesting/src/mock.rs b/frame/vesting/src/mock.rs index 598e8aab0ecaf..f5e5c570552b3 100644 --- a/frame/vesting/src/mock.rs +++ b/frame/vesting/src/mock.rs @@ -154,6 +154,7 @@ impl ExtBuilder { .unwrap(); let mut ext = sp_io::TestExternalities::new(t); ext.execute_with(|| { + // sp_tracing::try_init_simple(); // Add this line System::set_block_number(1); Vesting::do_try_state().unwrap(); }); diff --git a/frame/vesting/src/tests.rs b/frame/vesting/src/tests.rs index da93ff3909716..de172a0d18240 100644 --- a/frame/vesting/src/tests.rs +++ b/frame/vesting/src/tests.rs @@ -37,6 +37,7 @@ where { // Its ok for this to fail because the user may already have no schedules. let _result = Vesting::vest(Some(account).into()); + assert_ok!(Vesting::do_try_state()); assert!(!>::contains_key(account)); } @@ -52,17 +53,17 @@ fn check_vesting_status() { let user1_vesting_schedule = VestingInfo::new( ED * 5, 128, // Vesting over 10 blocks - 0, + 0, // amount_left = 9 (we are already at block=1) ); let user2_vesting_schedule = VestingInfo::new( ED * 20, ED, // Vesting over 20 blocks - 10, + 10, // amount_left = 30 ); let user12_vesting_schedule = VestingInfo::new( ED * 5, 64, // Vesting over 20 blocks - 10, + 10, // amount_left = 30 ); assert_eq!(Vesting::vesting(&1).unwrap(), vec![user1_vesting_schedule]); // Account 1 has a vesting schedule assert_eq!(Vesting::vesting(&2).unwrap(), vec![user2_vesting_schedule]); // Account 2 has a vesting schedule @@ -70,14 +71,17 @@ fn check_vesting_status() { // Account 1 has only 128 units vested from their illiquid ED * 5 units at block 1 assert_eq!(Vesting::vesting_balance(&1), Some(128 * 9)); + println!("{:?}, {:?}", Vesting::vesting_balance(&1), Some(128 * 9)); // Account 2 has their full balance locked assert_eq!(Vesting::vesting_balance(&2), Some(user2_free_balance)); + println!("{:?}, {:?}", Vesting::vesting_balance(&2), Some(user2_free_balance)); // Account 12 has only their illiquid funds locked assert_eq!(Vesting::vesting_balance(&12), Some(user12_free_balance - ED * 5)); + println!("{:?}, {:?}", Vesting::vesting_balance(&12), Some(user12_free_balance - ED * 5)); - assert_ok!(Vesting::do_try_state()); System::set_block_number(10); assert_eq!(System::block_number(), 10); + assert_ok!(Vesting::do_try_state()); // Account 1 has fully vested by block 10 assert_eq!(Vesting::vesting_balance(&1), Some(0)); @@ -86,9 +90,9 @@ fn check_vesting_status() { // Account 12 has started vesting by block 10 assert_eq!(Vesting::vesting_balance(&12), Some(user12_free_balance - ED * 5)); - assert_ok!(Vesting::do_try_state()); System::set_block_number(30); assert_eq!(System::block_number(), 30); + assert_ok!(Vesting::do_try_state()); assert_eq!(Vesting::vesting_balance(&1), Some(0)); // Account 1 is still fully vested, and not negative assert_eq!(Vesting::vesting_balance(&2), Some(0)); // Account 2 has fully vested by block 30 @@ -125,6 +129,7 @@ fn check_vesting_status_for_multi_schedule_account() { 0, ); assert_ok!(Vesting::vested_transfer(Some(4).into(), 2, sched1)); + assert_ok!(Vesting::do_try_state()); // Free balance is equal to the two existing schedules total amount. let free_balance = Balances::free_balance(&2); assert_eq!(free_balance, ED * (10 + 20)); @@ -140,9 +145,10 @@ fn check_vesting_status_for_multi_schedule_account() { 5, ); assert_ok!(Vesting::vested_transfer(Some(4).into(), 2, sched2)); - assert_ok!(Vesting::do_try_state()); + System::set_block_number(9); + assert_ok!(Vesting::do_try_state()); // Free balance is equal to the 3 existing schedules total amount. let free_balance = Balances::free_balance(&2); assert_eq!(free_balance, ED * (10 + 20 + 30)); @@ -152,7 +158,6 @@ fn check_vesting_status_for_multi_schedule_account() { Some(free_balance - sched1.per_block() * 9 - sched2.per_block() * 4) ); - assert_ok!(Vesting::do_try_state()); System::set_block_number(20); // At block #20 sched1 is fully unlocked while sched2 and sched0 are partially unlocked. assert_eq!( @@ -161,19 +166,20 @@ fn check_vesting_status_for_multi_schedule_account() { free_balance - sched1.locked() - sched2.per_block() * 15 - sched0.per_block() * 10 ) ); - assert_ok!(Vesting::do_try_state()); + System::set_block_number(30); // At block #30 sched0 and sched1 are fully unlocked while sched2 is partially unlocked. assert_eq!( Vesting::vesting_balance(&2), Some(free_balance - sched1.locked() - sched2.per_block() * 25 - sched0.locked()) ); + assert_ok!(Vesting::do_try_state()); // At block #35 sched2 fully unlocks and thus all schedules funds are unlocked. - assert_ok!(Vesting::do_try_state()); System::set_block_number(35); assert_eq!(Vesting::vesting_balance(&2), Some(0)); + assert_ok!(Vesting::do_try_state()); // Since we have not called any extrinsics that would unlock funds the schedules // are still in storage, assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1, sched2]); @@ -202,6 +208,7 @@ fn vested_balance_should_transfer() { // Account 1 has only 5 units vested at block 1 (plus 50 unvested) assert_eq!(Vesting::vesting_balance(&1), Some(45)); assert_ok!(Vesting::vest(Some(1).into())); + assert_ok!(Vesting::do_try_state()); assert_ok!(Balances::transfer_allow_death(Some(1).into(), 2, 55)); }); } @@ -211,6 +218,7 @@ fn vested_balance_should_transfer_with_multi_sched() { ExtBuilder::default().existential_deposit(ED).build().execute_with(|| { let sched0 = VestingInfo::new(5 * ED, 128, 0); assert_ok!(Vesting::vested_transfer(Some(13).into(), 1, sched0)); + assert_ok!(Vesting::do_try_state()); // Total 10*ED locked for all the schedules. assert_eq!(Vesting::vesting(&1).unwrap(), vec![sched0, sched0]); @@ -220,6 +228,7 @@ fn vested_balance_should_transfer_with_multi_sched() { // Account 1 has only 256 units unlocking at block 1 (plus 1280 already fee). assert_eq!(Vesting::vesting_balance(&1), Some(2304)); assert_ok!(Vesting::vest(Some(1).into())); + assert_ok!(Vesting::do_try_state()); assert_ok!(Balances::transfer_allow_death(Some(1).into(), 2, 1536)); }); } @@ -240,6 +249,7 @@ fn vested_balance_should_transfer_using_vest_other() { // Account 1 has only 5 units vested at block 1 (plus 50 unvested) assert_eq!(Vesting::vesting_balance(&1), Some(45)); assert_ok!(Vesting::vest_other(Some(2).into(), 1)); + assert_ok!(Vesting::do_try_state()); assert_ok!(Balances::transfer_allow_death(Some(1).into(), 2, 55)); }); } @@ -249,6 +259,7 @@ fn vested_balance_should_transfer_using_vest_other_with_multi_sched() { ExtBuilder::default().existential_deposit(ED).build().execute_with(|| { let sched0 = VestingInfo::new(5 * ED, 128, 0); assert_ok!(Vesting::vested_transfer(Some(13).into(), 1, sched0)); + assert_ok!(Vesting::do_try_state()); // Total of 10*ED of locked for all the schedules. assert_eq!(Vesting::vesting(&1).unwrap(), vec![sched0, sched0]); @@ -285,11 +296,13 @@ fn extra_balance_should_transfer() { // Account 1 has only 5 units vested at block 1 (plus 150 unvested) assert_eq!(Vesting::vesting_balance(&1), Some(45)); assert_ok!(Vesting::vest(Some(1).into())); + assert_ok!(Vesting::do_try_state()); assert_ok!(Balances::transfer_allow_death(Some(1).into(), 3, 155)); // Account 1 can send extra units gained // Account 2 has no units vested at block 1, but gained 100 assert_eq!(Vesting::vesting_balance(&2), Some(200)); assert_ok!(Vesting::vest(Some(2).into())); + assert_ok!(Vesting::do_try_state()); assert_ok!(Balances::transfer_allow_death(Some(2).into(), 3, 100)); // Account 2 can send extra // units gained }); @@ -297,7 +310,7 @@ fn extra_balance_should_transfer() { #[test] fn liquid_funds_should_transfer_with_delayed_vesting() { - ExtBuilder::default().existential_deposit(256).build().execute_with(|| { + ExtBuilder::default().existential_deposit(ED).build().execute_with(|| { let user12_free_balance = Balances::free_balance(&12); assert_eq!(user12_free_balance, 2560); // Account 12 has free balance @@ -319,7 +332,7 @@ fn liquid_funds_should_transfer_with_delayed_vesting() { #[test] fn vested_transfer_works() { - ExtBuilder::default().existential_deposit(256).build().execute_with(|| { + ExtBuilder::default().existential_deposit(ED).build().execute_with(|| { let user3_free_balance = Balances::free_balance(&3); let user4_free_balance = Balances::free_balance(&4); assert_eq!(user3_free_balance, 256 * 30); @@ -333,6 +346,7 @@ fn vested_transfer_works() { 10, ); assert_ok!(Vesting::vested_transfer(Some(3).into(), 4, new_vesting_schedule)); + assert_ok!(Vesting::do_try_state()); // Now account 4 should have vesting. assert_eq!(Vesting::vesting(&4).unwrap(), vec![new_vesting_schedule]); // Ensure the transfer happened correctly. @@ -343,16 +357,16 @@ fn vested_transfer_works() { // Account 4 has 5 * 256 locked. assert_eq!(Vesting::vesting_balance(&4), Some(256 * 5)); - assert_ok!(Vesting::do_try_state()); System::set_block_number(20); assert_eq!(System::block_number(), 20); + assert_ok!(Vesting::do_try_state()); // Account 4 has 5 * 64 units vested by block 20. assert_eq!(Vesting::vesting_balance(&4), Some(10 * 64)); - assert_ok!(Vesting::do_try_state()); System::set_block_number(30); assert_eq!(System::block_number(), 30); + assert_ok!(Vesting::do_try_state()); // Account 4 has fully vested, assert_eq!(Vesting::vesting_balance(&4), Some(0)); @@ -422,6 +436,7 @@ fn vested_transfer_allows_max_schedules() { // Add max amount schedules to user 4. for _ in 0..max_schedules { assert_ok!(Vesting::vested_transfer(Some(13).into(), 4, sched)); + assert_ok!(Vesting::do_try_state()); } // The schedules count towards vesting balance @@ -440,7 +455,6 @@ fn vested_transfer_allows_max_schedules() { assert_eq!(Balances::free_balance(&4), user_4_free_balance); // Account 4 has fully vested when all the schedules end, - assert_ok!(Vesting::do_try_state()); System::set_block_number( ::MinVestedTransfer::get() + sched.starting_block(), ); @@ -476,6 +490,7 @@ fn force_vested_transfer_works() { 4, new_vesting_schedule )); + assert_ok!(Vesting::do_try_state()); // Now account 4 should have vesting. assert_eq!(Vesting::vesting(&4).unwrap()[0], new_vesting_schedule); assert_eq!(Vesting::vesting(&4).unwrap().len(), 1); @@ -487,16 +502,16 @@ fn force_vested_transfer_works() { // Account 4 has 5 * ED locked. assert_eq!(Vesting::vesting_balance(&4), Some(ED * 5)); - assert_ok!(Vesting::do_try_state()); System::set_block_number(20); assert_eq!(System::block_number(), 20); + assert_ok!(Vesting::do_try_state()); // Account 4 has 5 * 64 units vested by block 20. assert_eq!(Vesting::vesting_balance(&4), Some(10 * 64)); - assert_ok!(Vesting::do_try_state()); System::set_block_number(30); assert_eq!(System::block_number(), 30); + assert_ok!(Vesting::do_try_state()); // Account 4 has fully vested, assert_eq!(Vesting::vesting_balance(&4), Some(0)); @@ -570,6 +585,7 @@ fn force_vested_transfer_allows_max_schedules() { // Add max amount schedules to user 4. for _ in 0..max_schedules { assert_ok!(Vesting::force_vested_transfer(RawOrigin::Root.into(), 13, 4, sched)); + assert_ok!(Vesting::do_try_state()); } // The schedules count towards vesting balance. @@ -588,8 +604,8 @@ fn force_vested_transfer_allows_max_schedules() { assert_eq!(Balances::free_balance(&4), user_4_free_balance); // Account 4 has fully vested when all the schedules end, - assert_ok!(Vesting::do_try_state()); System::set_block_number(::MinVestedTransfer::get() + 10); + assert_ok!(Vesting::do_try_state()); assert_eq!(Vesting::vesting_balance(&4), Some(0)); // and after unlocking its schedules are removed from storage. vest_and_assert_no_vesting::(4); @@ -610,6 +626,7 @@ fn merge_schedules_that_have_not_started() { // Add a schedule that is identical to the one that already exists. assert_ok!(Vesting::vested_transfer(Some(3).into(), 2, sched0)); + assert_ok!(Vesting::do_try_state()); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched0]); assert_eq!(Balances::usable_balance(&2), 0); assert_ok!(Vesting::merge_schedules(Some(2).into(), 0, 1)); @@ -646,11 +663,13 @@ fn merge_ongoing_schedules() { sched0.starting_block() + 5, // Start at block 15. ); assert_ok!(Vesting::vested_transfer(Some(4).into(), 2, sched1)); + assert_ok!(Vesting::do_try_state()); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1]); // Got to half way through the second schedule where both schedules are actively vesting. let cur_block = 20; System::set_block_number(cur_block); + assert_ok!(Vesting::do_try_state()); // Account 2 has no usable balances prior to the merge because they have not unlocked // with `vest` yet. @@ -681,9 +700,9 @@ fn merge_ongoing_schedules() { let sched2 = VestingInfo::new(sched2_locked, sched2_per_block, cur_block); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched2]); - // And just to double check, we assert the new merged schedule we be cleaned up as expected. - assert_ok!(Vesting::do_try_state()); + // And just to double check, we assert the new merged schedule will be cleaned up as expected. System::set_block_number(30); + assert_ok!(Vesting::do_try_state()); vest_and_assert_no_vesting::(2); }); } @@ -720,8 +739,11 @@ fn merging_shifts_other_schedules_index() { // Transfer the above 3 schedules to account 3. assert_ok!(Vesting::vested_transfer(Some(4).into(), 3, sched0)); + assert_ok!(Vesting::do_try_state()); assert_ok!(Vesting::vested_transfer(Some(4).into(), 3, sched1)); + assert_ok!(Vesting::do_try_state()); assert_ok!(Vesting::vested_transfer(Some(4).into(), 3, sched2)); + assert_ok!(Vesting::do_try_state()); // With no schedules vested or merged they are in the order they are created assert_eq!(Vesting::vesting(&3).unwrap(), vec![sched0, sched1, sched2]); @@ -770,12 +792,14 @@ fn merge_ongoing_and_yet_to_be_started_schedules() { (sched0.starting_block() + sched0.ending_block_as_balance::()) / 2; assert_eq!(cur_block, 20); System::set_block_number(cur_block); + assert_ok!(Vesting::do_try_state()); // Prior to vesting there is no usable balance. let mut usable_balance = 0; assert_eq!(Balances::usable_balance(&2), usable_balance); // Vest the current schedules (which is just sched0 now). Vesting::vest(Some(2).into()).unwrap(); + assert_ok!(Vesting::do_try_state()); // After vesting the usable balance increases by the unlocked amount. let sched0_vested_now = sched0.locked() - sched0.locked_at::(cur_block); @@ -783,9 +807,9 @@ fn merge_ongoing_and_yet_to_be_started_schedules() { assert_eq!(Balances::usable_balance(&2), usable_balance); // Go forward a block. - assert_ok!(Vesting::do_try_state()); cur_block += 1; System::set_block_number(cur_block); + assert_ok!(Vesting::do_try_state()); // And add a schedule that starts after this block, but before sched0 finishes. let sched1 = VestingInfo::new( @@ -794,6 +818,7 @@ fn merge_ongoing_and_yet_to_be_started_schedules() { cur_block + 1, ); assert_ok!(Vesting::vested_transfer(Some(4).into(), 2, sched1)); + assert_ok!(Vesting::do_try_state()); // Merge the schedules before sched1 starts. assert_ok!(Vesting::merge_schedules(Some(2).into(), 0, 1)); @@ -817,7 +842,6 @@ fn merge_ongoing_and_yet_to_be_started_schedules() { let sched2_per_block = sched2_locked / sched2_duration; let sched2 = VestingInfo::new(sched2_locked, sched2_per_block, sched2_start); - assert_ok!(Vesting::do_try_state()); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched2]); }); } @@ -841,6 +865,7 @@ fn merge_finished_and_ongoing_schedules() { 10, ); assert_ok!(Vesting::vested_transfer(Some(4).into(), 2, sched1)); + assert_ok!(Vesting::do_try_state()); // Transfer a 3rd schedule, so we can demonstrate how schedule indices change. // (We are not merging this schedule.) @@ -850,6 +875,7 @@ fn merge_finished_and_ongoing_schedules() { 10, ); assert_ok!(Vesting::vested_transfer(Some(3).into(), 2, sched2)); + assert_ok!(Vesting::do_try_state()); // The schedules are in expected order prior to merging. assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1, sched2]); @@ -858,7 +884,6 @@ fn merge_finished_and_ongoing_schedules() { let cur_block = sched0.ending_block_as_balance::(); System::set_block_number(cur_block); assert_eq!(System::block_number(), 30); - assert_ok!(Vesting::do_try_state()); // Prior to `merge_schedules` and with no vest/vest_other called the user has no usable // balance. @@ -906,6 +931,7 @@ fn merge_finishing_schedules_does_not_create_a_new_one() { 10, ); assert_ok!(Vesting::vested_transfer(Some(3).into(), 2, sched1)); + assert_ok!(Vesting::do_try_state()); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1]); let all_scheds_end = sched0 @@ -949,8 +975,8 @@ fn merge_finished_and_yet_to_be_started_schedules() { 35, ); assert_ok!(Vesting::vested_transfer(Some(13).into(), 2, sched1)); - assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1]); assert_ok!(Vesting::do_try_state()); + assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1]); let sched2 = VestingInfo::new( ED * 40, @@ -959,8 +985,8 @@ fn merge_finished_and_yet_to_be_started_schedules() { ); // Add a 3rd schedule to demonstrate how sched1 shifts. assert_ok!(Vesting::vested_transfer(Some(13).into(), 2, sched2)); - assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1, sched2]); assert_ok!(Vesting::do_try_state()); + assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched1, sched2]); System::set_block_number(30); assert_ok!(Vesting::do_try_state()); @@ -1007,6 +1033,7 @@ fn merge_schedules_throws_proper_errors() { // There are enough schedules to merge but an index is non-existent. Vesting::vested_transfer(Some(3).into(), 2, sched0).unwrap(); + assert_ok!(Vesting::do_try_state()); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched0, sched0]); assert_noop!( Vesting::merge_schedules(Some(2).into(), 0, 2), @@ -1160,16 +1187,16 @@ fn vested_transfer_less_than_existential_deposit_fails() { ExtBuilder::default().existential_deposit(4 * ED).build().execute_with(|| { // MinVestedTransfer is less the ED. assert!( - ::Currency::minimum_balance() > - ::MinVestedTransfer::get() + ::Currency::minimum_balance() + > ::MinVestedTransfer::get() ); let sched = VestingInfo::new(::MinVestedTransfer::get() as u64, 1u64, 10u64); // The new account balance with the schedule's locked amount would be less than ED. assert!( - Balances::free_balance(&99) + sched.locked() < - ::Currency::minimum_balance() + Balances::free_balance(&99) + sched.locked() + < ::Currency::minimum_balance() ); // vested_transfer fails. @@ -1181,3 +1208,58 @@ fn vested_transfer_less_than_existential_deposit_fails() { ); }); } + +#[test] +fn plain_lack_stain() { + let vesting_config = vec![ + // 5 * existential deposit locked. + (1, 0, 10, 5 * ED), + // 1 * existential deposit locked. + (2, 10, 20, 19 * ED), + // 2 * existential deposit locked. + (2, 10, 0, 18 * ED), + // 1 * existential deposit locked. + (12, 0, 20, 9 * ED), + // 2 * existential deposit locked. + (12, 10, 0, 8 * ED), + // 3 * existential deposit locked. + (12, 10, 20, 7 * ED), + ]; + ExtBuilder::default() + .existential_deposit(ED) + .vesting_genesis_config(vesting_config) + .build() + .execute_with(|| { + let user1_sched1 = VestingInfo::new(5 * ED, 128, 0u64); + assert_eq!(Vesting::vesting(&1).unwrap(), vec![user1_sched1]); + + let user2_sched1 = VestingInfo::new(1 * ED, 12, 10u64); + let user2_sched2 = VestingInfo::new(2 * ED, 512, 10u64); + assert_eq!(Vesting::vesting(&2).unwrap(), vec![user2_sched1, user2_sched2]); + + let user12_sched1 = VestingInfo::new(1 * ED, 12, 0u64); + let user12_sched2 = VestingInfo::new(2 * ED, 512, 10u64); + let user12_sched3 = VestingInfo::new(3 * ED, 38, 10u64); + assert_eq!( + Vesting::vesting(&12).unwrap(), + vec![user12_sched1, user12_sched2, user12_sched3] + ); + + // get the max ending block + let mut v: Vec = Vec::new(); + + v.push(user1_sched1.ending_block_as_balance::()); + v.push(user2_sched1.ending_block_as_balance::()); + v.push(user2_sched2.ending_block_as_balance::()); + v.push(user12_sched1.ending_block_as_balance::()); + v.push(user12_sched2.ending_block_as_balance::()); + v.push(user12_sched3.ending_block_as_balance::()); + + // Loop through all schedules + let max_value = v.iter().max().unwrap_or(&500); + for i in 0..*max_value { + System::set_block_number(i); + assert_ok!(Vesting::do_try_state()); + } + }) +} From 6a5a764b58eac0067f995496c595a4af0ac18b7f Mon Sep 17 00:00:00 2001 From: doordashcon Date: Fri, 4 Aug 2023 21:30:37 +0100 Subject: [PATCH 10/15] vesting docs, tips result --- frame/tips/src/lib.rs | 2 +- frame/vesting/src/lib.rs | 28 ++++++++++++++++++++-------- frame/vesting/src/tests.rs | 2 +- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index b2fc803781a87..9adadf25b0c3c 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -615,7 +615,7 @@ impl, I: 'static> Pallet { /// /// * There should be a corresponding `OpenTip.reason` for each key in `Reasons`. #[cfg(any(feature = "try-runtime", test))] - pub fn do_try_state() -> Result<(), &'static str> { + pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { let reasons = Reasons::::iter_keys().collect::>(); let tips = Tips::::iter_keys().collect::>(); diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index a0f3c4162e4e9..38ea02ea56c4a 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -73,6 +73,7 @@ use sp_runtime::{ AtLeast32BitUnsigned, Bounded, Convert, MaybeSerializeDeserialize, One, Saturating, StaticLookup, Zero, }, + SaturatedConversion, RuntimeDebug, }; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; @@ -659,12 +660,15 @@ impl Pallet { /// * the locked amount(`still_vesting`) of a vesting schedule must be equal to the /// product of the duration(`schedules_left` - `starting_block`) and the per block amount when the locked /// amount is divisible by the per block amount. + /// * However, If the locked amount is not divisible by the per block amount, the remainder (`locked` % `per_block`) + /// should be less than the per block amount and at the final vesting block (`schedules_left` - 1), + /// the vesting balance should be equal to the remainder. /// /// During schedule timeframe: /// * the amount `still_vesting` must be equal to the product of the remaining blocks to vest(`schedules_left` - `current_block`) /// and per block amount when the locked amount is divisible by the per block amount. - /// * However, if the locked amount is not divisible by the per block amount, - /// at the final vesting block the remainder must equal the amount `still_vesting`. + /// * However, If the locked amount is not divisible by the per block amount, then at the final vesting block of the + /// current schedule (`schedules_left` - 1), the vesting balance should be equal to the remainder. #[cfg(any(feature = "try-runtime", test))] pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { Vesting::::iter().for_each(|(_, d)| { @@ -673,7 +677,7 @@ impl Pallet { for info in infos.iter() { let schedules_left: BalanceOf = info.ending_block_as_balance::(); - let starting_block = T::BlockNumberToBalance::convert(info.starting_block()); + let starting_block = T::BlockNumberToBalance::convert(info.starting_block()); let still_vesting = info .locked_at::(>::block_number()); @@ -683,10 +687,18 @@ impl Pallet { if current_block < starting_block { let count = schedules_left.saturating_sub(starting_block); if (info.locked() % info.per_block()).is_zero() { - assert!(still_vesting == (count * info.per_block()), "uninitiated schedule, the vesting balance is unequal with the total raw per block releases."); + assert!(still_vesting == (count * info.per_block()), "Before schedule starts, the vesting balance should be equal to the total per block releases."); } else { let re = info.locked() % info.per_block(); - assert!(info.per_block() > re, "uinitiated schedule, the raw per block is greater than remainder"); + assert!(info.per_block() > re, "Before schedule starts, the per block should be greater than the remainder"); + + let final_vest_block = schedules_left.saturating_sub(One::one()).saturated_into::(); + let final_vest_amount = info.locked_at::(final_vest_block.saturated_into()); + assert!(final_vest_amount == re, "Before schedule starts, the final vest amount should be equal to the remainder"); + + let no_schedules_left = schedules_left.saturated_into::(); + let no_locks = info.locked_at::(no_schedules_left.saturated_into()); + assert!(no_locks == Zero::zero(), "After all schedules, all amounts should be unlocked"); } } else { if (info.locked() % info.per_block()).is_zero() { @@ -694,14 +706,14 @@ impl Pallet { still_vesting == (schedules_left.saturating_sub(current_block) * info.per_block()), - "Schedule started, the vesting balance is unequal with the total per block releases" + "during schedules, the vesting balance should be equal to the total per block releases" ); } else { let re = info.locked() % info.per_block(); - assert!(info.per_block() > re, "Schedule started, per block is less than remainder"); + assert!(info.per_block() > re, "per block should be greater than the remainder"); if current_block == schedules_left.saturating_sub(One::one()) { - assert!(still_vesting == re, "Schedule started, at the final vesting block the vesting balance is unequal to the remainder"); + assert!(still_vesting == re, "At the final vesting block, the vesting balance should be equal to the remainder"); } if current_block == schedules_left { diff --git a/frame/vesting/src/tests.rs b/frame/vesting/src/tests.rs index de172a0d18240..180114cbabba4 100644 --- a/frame/vesting/src/tests.rs +++ b/frame/vesting/src/tests.rs @@ -1210,7 +1210,7 @@ fn vested_transfer_less_than_existential_deposit_fails() { } #[test] -fn plain_lack_stain() { +fn play_out_all_schedules() { let vesting_config = vec![ // 5 * existential deposit locked. (1, 0, 10, 5 * ED), From 8eb23d08431dcf77a9bb6d5116281580fb3afcfe Mon Sep 17 00:00:00 2001 From: doordashcon Date: Fri, 4 Aug 2023 23:18:58 +0100 Subject: [PATCH 11/15] oversight print macro --- frame/tips/src/lib.rs | 10 +++++----- frame/vesting/src/lib.rs | 38 ++++++++++++++++++++------------------ frame/vesting/src/tests.rs | 11 ++++++----- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index 553286724d56c..8a3fb172ddc86 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -254,8 +254,8 @@ pub mod pallet { let hash = T::Hashing::hash_of(&(&reason_hash, &who)); ensure!(!Tips::::contains_key(&hash), Error::::AlreadyKnown); - let deposit = T::TipReportDepositBase::get() - + T::DataDepositPerByte::get() * (reason.len() as u32).into(); + let deposit = T::TipReportDepositBase::get() + + T::DataDepositPerByte::get() * (reason.len() as u32).into(); T::Currency::reserve(&finder, deposit)?; Reasons::::insert(&reason_hash, &reason); @@ -457,7 +457,7 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> Result<(), &'static str> { + fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { Self::do_try_state() } } @@ -509,9 +509,9 @@ impl, I: 'static> Pallet { Some(m) => { member = members_iter.next(); if m < a { - continue; + continue } else { - break true; + break true } }, } diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index b7835b42d6715..a93e345f0ce92 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -74,8 +74,7 @@ use sp_runtime::{ AtLeast32BitUnsigned, Bounded, Convert, MaybeSerializeDeserialize, One, Saturating, StaticLookup, Zero, }, - SaturatedConversion, - RuntimeDebug, + RuntimeDebug, SaturatedConversion, }; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; @@ -246,7 +245,6 @@ pub mod pallet { let locked = balance.saturating_sub(liquid); let length_as_balance = T::BlockNumberToBalance::convert(length); let per_block = locked / length_as_balance.max(sp_runtime::traits::One::one()); - println!("{:?}", per_block); let vesting_info = VestingInfo::new(locked, per_block, begin); if !vesting_info.is_valid() { panic!("Invalid VestingInfo params at genesis") @@ -418,7 +416,7 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; if schedule1_index == schedule2_index { - return Ok(()); + return Ok(()) }; let schedule1_index = schedule1_index as usize; let schedule2_index = schedule2_index as usize; @@ -496,7 +494,7 @@ impl Pallet { // Validate user inputs. ensure!(schedule.locked() >= T::MinVestedTransfer::get(), Error::::AmountLow); if !schedule.is_valid() { - return Err(Error::::InvalidScheduleParams.into()); + return Err(Error::::InvalidScheduleParams.into()) }; let target = T::Lookup::lookup(target)?; let source = T::Lookup::lookup(source)?; @@ -644,8 +642,8 @@ impl Pallet { }; debug_assert!( - locked_now > Zero::zero() && schedules.len() > 0 - || locked_now == Zero::zero() && schedules.len() == 0 + locked_now > Zero::zero() && schedules.len() > 0 || + locked_now == Zero::zero() && schedules.len() == 0 ); Ok((schedules, locked_now)) @@ -659,17 +657,21 @@ impl Pallet { /// /// Before schedules begin: /// * the locked amount(`still_vesting`) of a vesting schedule must be equal to the - /// product of the duration(`schedules_left` - `starting_block`) and the per block amount when the locked - /// amount is divisible by the per block amount. - /// * However, If the locked amount is not divisible by the per block amount, the remainder (`locked` % `per_block`) - /// should be less than the per block amount and at the final vesting block (`schedules_left` - 1), - /// the vesting balance should be equal to the remainder. + /// product of the duration(`schedules_left` - `starting_block`) and the per block amount when + /// the locked amount is divisible by the per block amount. + /// * However, If the locked amount is not divisible by the per block amount, the remainder + /// (`locked` % `per_block`) + /// should be less than the per block amount and at the final vesting block (`schedules_left` - + /// 1), the vesting balance should be equal to the remainder. /// /// During schedule timeframe: - /// * the amount `still_vesting` must be equal to the product of the remaining blocks to vest(`schedules_left` - `current_block`) + /// * the amount `still_vesting` must be equal to the product of the remaining blocks to + /// vest(`schedules_left` - `current_block`) /// and per block amount when the locked amount is divisible by the per block amount. - /// * However, If the locked amount is not divisible by the per block amount, then at the final vesting block of the - /// current schedule (`schedules_left` - 1), the vesting balance should be equal to the remainder. + /// * However, If the locked amount is not divisible by the per block amount, then at the final + /// vesting block of the + /// current schedule (`schedules_left` - 1), the vesting balance should be equal to the + /// remainder. #[cfg(any(feature = "try-runtime", test))] pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { Vesting::::iter().for_each(|(_, d)| { @@ -767,13 +769,13 @@ where starting_block: BlockNumberFor, ) -> DispatchResult { if locked.is_zero() { - return Ok(()); + return Ok(()) } let vesting_schedule = VestingInfo::new(locked, per_block, starting_block); // Check for `per_block` or `locked` of 0. if !vesting_schedule.is_valid() { - return Err(Error::::InvalidScheduleParams.into()); + return Err(Error::::InvalidScheduleParams.into()) }; let mut schedules = Self::vesting(who).unwrap_or_default(); @@ -801,7 +803,7 @@ where ) -> DispatchResult { // Check for `per_block` or `locked` of 0. if !VestingInfo::new(locked, per_block, starting_block).is_valid() { - return Err(Error::::InvalidScheduleParams.into()); + return Err(Error::::InvalidScheduleParams.into()) } ensure!( diff --git a/frame/vesting/src/tests.rs b/frame/vesting/src/tests.rs index 180114cbabba4..46502d20af030 100644 --- a/frame/vesting/src/tests.rs +++ b/frame/vesting/src/tests.rs @@ -700,7 +700,8 @@ fn merge_ongoing_schedules() { let sched2 = VestingInfo::new(sched2_locked, sched2_per_block, cur_block); assert_eq!(Vesting::vesting(&2).unwrap(), vec![sched2]); - // And just to double check, we assert the new merged schedule will be cleaned up as expected. + // And just to double check, we assert the new merged schedule will be cleaned up as + // expected. System::set_block_number(30); assert_ok!(Vesting::do_try_state()); vest_and_assert_no_vesting::(2); @@ -1187,16 +1188,16 @@ fn vested_transfer_less_than_existential_deposit_fails() { ExtBuilder::default().existential_deposit(4 * ED).build().execute_with(|| { // MinVestedTransfer is less the ED. assert!( - ::Currency::minimum_balance() - > ::MinVestedTransfer::get() + ::Currency::minimum_balance() > + ::MinVestedTransfer::get() ); let sched = VestingInfo::new(::MinVestedTransfer::get() as u64, 1u64, 10u64); // The new account balance with the schedule's locked amount would be less than ED. assert!( - Balances::free_balance(&99) + sched.locked() - < ::Currency::minimum_balance() + Balances::free_balance(&99) + sched.locked() < + ::Currency::minimum_balance() ); // vested_transfer fails. From dc502eee253410c9ee51a9991efa7ed4ddb7d220 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 6 Aug 2023 10:59:40 +0100 Subject: [PATCH 12/15] feature flag SaturatedConversion --- frame/tips/src/tests.rs | 5 ++++- frame/vesting/src/lib.rs | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 8bd4fec1b94d5..5bb2d4e97ea52 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -197,7 +197,10 @@ pub fn new_test_ext() -> sp_io::TestExternalities { .build_storage() .unwrap() .into(); - ext.execute_with(|| System::set_block_number(1)); + ext.execute_with(|| { + System::set_block_number(1); + Tips::do_try_state().unwrap(); + }); ext } diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index a93e345f0ce92..6f4dc21ac50d2 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -74,10 +74,13 @@ use sp_runtime::{ AtLeast32BitUnsigned, Bounded, Convert, MaybeSerializeDeserialize, One, Saturating, StaticLookup, Zero, }, - RuntimeDebug, SaturatedConversion, + RuntimeDebug, }; use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; +#[cfg(any(feature = "try-runtime", test))] +use sp_runtime::SaturatedConversion; + pub use pallet::*; pub use vesting_info::*; pub use weights::WeightInfo; From 52d40d4df4939b9264edfac8304428fb919a52ba Mon Sep 17 00:00:00 2001 From: doordashcon Date: Sun, 6 Aug 2023 18:08:47 +0100 Subject: [PATCH 13/15] fully Q --- frame/tips/src/lib.rs | 2 +- frame/vesting/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index 8a3fb172ddc86..7c18983c74ae0 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -457,7 +457,7 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { + fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { Self::do_try_state() } } diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index 6f4dc21ac50d2..05e774ec6fda4 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -198,7 +198,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { + fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { Self::do_try_state() } } From fda9110f76e72a39be1ee46f2579c0b5ab50ffa9 Mon Sep 17 00:00:00 2001 From: doordashcon Date: Tue, 8 Aug 2023 12:51:56 +0100 Subject: [PATCH 14/15] ensure --- frame/tips/src/lib.rs | 13 ++++++--- frame/tips/src/tests.rs | 26 +++++++++++++----- frame/vesting/src/lib.rs | 54 +++++++++++++++++++++++++------------- frame/vesting/src/tests.rs | 2 +- 4 files changed, 67 insertions(+), 28 deletions(-) diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index 7c18983c74ae0..32738934b669d 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -68,6 +68,7 @@ use sp_std::prelude::*; use codec::{Decode, Encode}; use frame_support::{ + ensure, traits::{ ContainsLengthBound, Currency, EnsureOrigin, ExistenceRequirement::KeepAlive, Get, OnUnbalanced, ReservableCurrency, SortedMembers, @@ -76,6 +77,9 @@ use frame_support::{ }; use frame_system::pallet_prelude::BlockNumberFor; +#[cfg(any(feature = "try-runtime", test))] +use sp_runtime::TryRuntimeError; + pub use pallet::*; pub use weights::WeightInfo; @@ -457,7 +461,7 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { + fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { Self::do_try_state() } } @@ -616,13 +620,16 @@ impl, I: 'static> Pallet { /// /// * There should be a corresponding `OpenTip.reason` for each key in `Reasons`. #[cfg(any(feature = "try-runtime", test))] - pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { + pub fn do_try_state() -> Result<(), TryRuntimeError> { let reasons = Reasons::::iter_keys().collect::>(); let tips = Tips::::iter_keys().collect::>(); for reason in reasons { for tip in &tips { - assert_eq!(reason, Tips::::get(tip).unwrap().reason); + ensure!( + reason == Tips::::get(tip).unwrap().reason, + TryRuntimeError::Other("reasons don't match") + ); } } Ok(()) diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 5bb2d4e97ea52..1a078caa55388 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -197,10 +197,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities { .build_storage() .unwrap() .into(); - ext.execute_with(|| { - System::set_block_number(1); - Tips::do_try_state().unwrap(); - }); + ext.execute_with(|| System::set_block_number(1)); ext } @@ -311,6 +308,7 @@ fn close_tip_works() { System::set_block_number(2); Tips::do_try_state().unwrap(); + assert_noop!(Tips::close_tip(RuntimeOrigin::none(), h.into()), BadOrigin); assert_ok!(Tips::close_tip(RuntimeOrigin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); @@ -367,9 +365,14 @@ fn retract_tip_works() { assert_ok!(Tips::tip(RuntimeOrigin::signed(11), h, 10)); assert_ok!(Tips::tip(RuntimeOrigin::signed(12), h, 10)); Tips::do_try_state().unwrap(); + assert_noop!(Tips::retract_tip(RuntimeOrigin::signed(10), h), Error::::NotFinder); assert_ok!(Tips::retract_tip(RuntimeOrigin::signed(0), h)); + Tips::do_try_state().unwrap(); + System::set_block_number(2); + Tips::do_try_state().unwrap(); + assert_noop!( Tips::close_tip(RuntimeOrigin::signed(0), h.into()), Error::::UnknownTip @@ -382,9 +385,14 @@ fn retract_tip_works() { assert_ok!(Tips::tip(RuntimeOrigin::signed(11), h, 10)); assert_ok!(Tips::tip(RuntimeOrigin::signed(12), h, 10)); Tips::do_try_state().unwrap(); + assert_noop!(Tips::retract_tip(RuntimeOrigin::signed(0), h), Error::::NotFinder); assert_ok!(Tips::retract_tip(RuntimeOrigin::signed(10), h)); + Tips::do_try_state().unwrap(); + System::set_block_number(2); + Tips::do_try_state().unwrap(); + assert_noop!( Tips::close_tip(RuntimeOrigin::signed(10), h.into()), Error::::UnknownTip @@ -400,8 +408,11 @@ fn tip_median_calculation_works() { let h = tip_hash(); assert_ok!(Tips::tip(RuntimeOrigin::signed(11), h, 10)); assert_ok!(Tips::tip(RuntimeOrigin::signed(12), h, 1000000)); + Tips::do_try_state().unwrap(); + System::set_block_number(2); Tips::do_try_state().unwrap(); + assert_ok!(Tips::close_tip(RuntimeOrigin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); }); @@ -420,8 +431,11 @@ fn tip_changing_works() { assert_ok!(Tips::tip(RuntimeOrigin::signed(12), h, 1000)); assert_ok!(Tips::tip(RuntimeOrigin::signed(11), h, 100)); assert_ok!(Tips::tip(RuntimeOrigin::signed(10), h, 10)); + Tips::do_try_state().unwrap(); + System::set_block_number(2); Tips::do_try_state().unwrap(); + assert_ok!(Tips::close_tip(RuntimeOrigin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); }); @@ -615,15 +629,15 @@ fn report_awesome_and_tip_works_second_instance() { assert_ok!(Tips1::tip(RuntimeOrigin::signed(11), h, 10)); assert_ok!(Tips1::tip(RuntimeOrigin::signed(12), h, 10)); assert_noop!(Tips1::tip(RuntimeOrigin::signed(9), h, 10), BadOrigin); + Tips::do_try_state().unwrap(); System::set_block_number(2); + Tips::do_try_state().unwrap(); assert_ok!(Tips1::close_tip(RuntimeOrigin::signed(100), h.into())); // Treasury 1 unchanged assert_eq!(Balances::free_balance(&Treasury::account_id()), 101); // Treasury 2 gave the funds assert_eq!(Balances::free_balance(&Treasury1::account_id()), 191); - - Tips::do_try_state().unwrap(); }); } diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index 05e774ec6fda4..100c9322f8862 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -79,7 +79,7 @@ use sp_runtime::{ use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; #[cfg(any(feature = "try-runtime", test))] -use sp_runtime::SaturatedConversion; +use sp_runtime::{SaturatedConversion, TryRuntimeError}; pub use pallet::*; pub use vesting_info::*; @@ -198,7 +198,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { + fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { Self::do_try_state() } } @@ -676,14 +676,14 @@ impl Pallet { /// current schedule (`schedules_left` - 1), the vesting balance should be equal to the /// remainder. #[cfg(any(feature = "try-runtime", test))] - pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { - Vesting::::iter().for_each(|(_, d)| { + pub fn do_try_state() -> Result<(), TryRuntimeError> { + for (_, d) in Vesting::::iter() { let infos = d.to_vec(); for info in infos.iter() { let schedules_left: BalanceOf = info.ending_block_as_balance::(); - let starting_block = T::BlockNumberToBalance::convert(info.starting_block()); + let starting_block = T::BlockNumberToBalance::convert(info.starting_block()); let still_vesting = info .locked_at::(>::block_number()); @@ -693,42 +693,60 @@ impl Pallet { if current_block < starting_block { let count = schedules_left.saturating_sub(starting_block); if (info.locked() % info.per_block()).is_zero() { - assert!(still_vesting == (count * info.per_block()), "Before schedule starts, the vesting balance should be equal to the total per block releases."); + ensure!(still_vesting == (count * info.per_block()), TryRuntimeError::Other("Before schedule starts, the vesting balance should be equal to the total per block releases")) } else { let re = info.locked() % info.per_block(); - assert!(info.per_block() > re, "Before schedule starts, the per block should be greater than the remainder"); + ensure!(info.per_block() > re, TryRuntimeError::Other("Before schedule starts, the per block should be greater than the remainder")); - let final_vest_block = schedules_left.saturating_sub(One::one()).saturated_into::(); - let final_vest_amount = info.locked_at::(final_vest_block.saturated_into()); - assert!(final_vest_amount == re, "Before schedule starts, the final vest amount should be equal to the remainder"); + let final_vest_block = + schedules_left.saturating_sub(One::one()).saturated_into::(); + let final_vest_amount = info.locked_at::( + final_vest_block.saturated_into(), + ); + ensure!(final_vest_amount == re, TryRuntimeError::Other("Before schedule starts, the final vest amount should be equal to the remainder")); let no_schedules_left = schedules_left.saturated_into::(); - let no_locks = info.locked_at::(no_schedules_left.saturated_into()); - assert!(no_locks == Zero::zero(), "After all schedules, all amounts should be unlocked"); + let no_locks = info.locked_at::( + no_schedules_left.saturated_into(), + ); + ensure!( + no_locks == Zero::zero(), + TryRuntimeError::Other( + "After all schedules, all amounts should be unlocked" + ) + ); } } else { if (info.locked() % info.per_block()).is_zero() { - assert!( + ensure!( still_vesting == (schedules_left.saturating_sub(current_block) * info.per_block()), - "during schedules, the vesting balance should be equal to the total per block releases" + TryRuntimeError::Other("during schedules, the vesting balance should be equal to the total per block releases") ); } else { let re = info.locked() % info.per_block(); - assert!(info.per_block() > re, "per block should be greater than the remainder"); + ensure!( + info.per_block() > re, + TryRuntimeError::Other( + "per block should be greater than the remainder" + ) + ); if current_block == schedules_left.saturating_sub(One::one()) { - assert!(still_vesting == re, "At the final vesting block, the vesting balance should be equal to the remainder"); + ensure!(still_vesting == re, TryRuntimeError::Other("At the final vesting block, the vesting balance should be equal to the remainder")); } if current_block == schedules_left { - assert!(still_vesting == Zero::zero(), "Schedule ended, no more vesting balance"); + ensure!( + still_vesting == Zero::zero(), + TryRuntimeError::Other("Schedule ended, no more vesting balance") + ); } } } } - }); + } Ok(()) } } diff --git a/frame/vesting/src/tests.rs b/frame/vesting/src/tests.rs index 46502d20af030..4ee43406eefbd 100644 --- a/frame/vesting/src/tests.rs +++ b/frame/vesting/src/tests.rs @@ -1257,7 +1257,7 @@ fn play_out_all_schedules() { v.push(user12_sched3.ending_block_as_balance::()); // Loop through all schedules - let max_value = v.iter().max().unwrap_or(&500); + let max_value = v.iter().max().unwrap_or(&380); for i in 0..*max_value { System::set_block_number(i); assert_ok!(Vesting::do_try_state()); From b235a18c56c1455184f8d0beb9817b5020332c2b Mon Sep 17 00:00:00 2001 From: doordashcon Date: Thu, 24 Aug 2023 15:46:57 +0100 Subject: [PATCH 15/15] slit vesting logic into seperate fn --- frame/tips/src/lib.rs | 13 +-- frame/vesting/src/lib.rs | 177 +++++++++++++++++++++------------------ 2 files changed, 100 insertions(+), 90 deletions(-) diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index 32738934b669d..6b7ac3cf5c6b7 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -622,15 +622,10 @@ impl, I: 'static> Pallet { #[cfg(any(feature = "try-runtime", test))] pub fn do_try_state() -> Result<(), TryRuntimeError> { let reasons = Reasons::::iter_keys().collect::>(); - let tips = Tips::::iter_keys().collect::>(); - - for reason in reasons { - for tip in &tips { - ensure!( - reason == Tips::::get(tip).unwrap().reason, - TryRuntimeError::Other("reasons don't match") - ); - } + + for tip in Tips::::iter_keys() { + let tip_reason = Tips::::get(&tip).unwrap().reason; + ensure!(reasons.contains(&tip_reason), TryRuntimeError::Other("reasons don't match")); } Ok(()) } diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index 100c9322f8862..a756898d5cd66 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -651,31 +651,31 @@ impl Pallet { Ok((schedules, locked_now)) } +} - /// Ensure the correctness of the state of this pallet. - /// - /// The following expectations must always apply. - /// - /// ## Expectations: - /// - /// Before schedules begin: - /// * the locked amount(`still_vesting`) of a vesting schedule must be equal to the - /// product of the duration(`schedules_left` - `starting_block`) and the per block amount when - /// the locked amount is divisible by the per block amount. - /// * However, If the locked amount is not divisible by the per block amount, the remainder - /// (`locked` % `per_block`) - /// should be less than the per block amount and at the final vesting block (`schedules_left` - - /// 1), the vesting balance should be equal to the remainder. - /// - /// During schedule timeframe: - /// * the amount `still_vesting` must be equal to the product of the remaining blocks to - /// vest(`schedules_left` - `current_block`) - /// and per block amount when the locked amount is divisible by the per block amount. - /// * However, If the locked amount is not divisible by the per block amount, then at the final - /// vesting block of the - /// current schedule (`schedules_left` - 1), the vesting balance should be equal to the - /// remainder. - #[cfg(any(feature = "try-runtime", test))] +/// Ensure the correctness of the state of this pallet. +/// +/// The following expectations must always apply. +/// +/// ## Expectations: +/// +/// `handle_before_schedule_starts`: +/// * the locked amount of a vesting schedule must be equal to the +/// product of the duration(`schedules_left` - `starting_block`) and the per block amount when +/// the locked amount is divisible by the per block amount. +/// * However, If the locked amount is not divisible by the per block amount, the final vesting blo +/// (`schedules_left` - 1), the vesting balance should be equal to the remainder. +/// +/// `handle_during_schedule`: +/// * the amount `still_vesting` must be equal to the product of the remaining blocks to +/// vest(`schedules_left` - `current_block`) +/// and per block amount when the locked amount is divisible by the per block amount. +/// * However, If the locked amount is not divisible by the per block amount, then at the final +/// vesting block of the +/// current schedule (`schedules_left` - 1), the vesting balance should be equal to the +/// remainder. +#[cfg(any(feature = "try-runtime", test))] +impl Pallet { pub fn do_try_state() -> Result<(), TryRuntimeError> { for (_, d) in Vesting::::iter() { let infos = d.to_vec(); @@ -684,71 +684,86 @@ impl Pallet { let schedules_left: BalanceOf = info.ending_block_as_balance::(); let starting_block = T::BlockNumberToBalance::convert(info.starting_block()); - - let still_vesting = info - .locked_at::(>::block_number()); - let current_block = + let current_block_to_balance = T::BlockNumberToBalance::convert(>::block_number()); - if current_block < starting_block { - let count = schedules_left.saturating_sub(starting_block); - if (info.locked() % info.per_block()).is_zero() { - ensure!(still_vesting == (count * info.per_block()), TryRuntimeError::Other("Before schedule starts, the vesting balance should be equal to the total per block releases")) - } else { - let re = info.locked() % info.per_block(); - ensure!(info.per_block() > re, TryRuntimeError::Other("Before schedule starts, the per block should be greater than the remainder")); - - let final_vest_block = - schedules_left.saturating_sub(One::one()).saturated_into::(); - let final_vest_amount = info.locked_at::( - final_vest_block.saturated_into(), - ); - ensure!(final_vest_amount == re, TryRuntimeError::Other("Before schedule starts, the final vest amount should be equal to the remainder")); - - let no_schedules_left = schedules_left.saturated_into::(); - let no_locks = info.locked_at::( - no_schedules_left.saturated_into(), - ); - ensure!( - no_locks == Zero::zero(), - TryRuntimeError::Other( - "After all schedules, all amounts should be unlocked" - ) - ); - } + if current_block_to_balance < starting_block { + Self::handle_before_schedule_starts(info, starting_block, schedules_left)?; } else { - if (info.locked() % info.per_block()).is_zero() { - ensure!( - still_vesting - == (schedules_left.saturating_sub(current_block) - * info.per_block()), - TryRuntimeError::Other("during schedules, the vesting balance should be equal to the total per block releases") - ); - } else { - let re = info.locked() % info.per_block(); - ensure!( - info.per_block() > re, - TryRuntimeError::Other( - "per block should be greater than the remainder" - ) - ); - - if current_block == schedules_left.saturating_sub(One::one()) { - ensure!(still_vesting == re, TryRuntimeError::Other("At the final vesting block, the vesting balance should be equal to the remainder")); - } - - if current_block == schedules_left { - ensure!( - still_vesting == Zero::zero(), - TryRuntimeError::Other("Schedule ended, no more vesting balance") - ); - } - } + Self::handle_during_schedule(info, current_block_to_balance, schedules_left)?; } } } Ok(()) } + + fn handle_before_schedule_starts( + info: &VestingInfo, BlockNumberFor>, + starting_block: BalanceOf, + schedules_left: BalanceOf, + ) -> Result<(), TryRuntimeError> { + let count = schedules_left.saturating_sub(starting_block); + + if (info.locked() % info.per_block()).is_zero() { + ensure!( + info.locked_at::(frame_system::Pallet::::block_number()) == (count * info.per_block()), + TryRuntimeError::Other("Before schedule starts, the vesting balance should be equal to the total per block releases") + ); + } else { + let re = info.locked() % info.per_block(); + + let final_vest_block = + schedules_left.saturating_sub(One::one()).saturated_into::(); + + let final_vest_amount = + info.locked_at::(final_vest_block.saturated_into()); + + ensure!(final_vest_amount == re, TryRuntimeError::Other("Before schedule starts, the final vest amount should be equal to the remainder")); + + let no_schedules_left = schedules_left.saturated_into::(); + + let no_locks = + info.locked_at::(no_schedules_left.saturated_into()); + + ensure!( + no_locks == Zero::zero(), + TryRuntimeError::Other("After all schedules, all amounts should be unlocked") + ); + } + + Ok(()) + } + + fn handle_during_schedule( + info: &VestingInfo, BlockNumberFor>, + current_block_to_balance: BalanceOf, + schedules_left: BalanceOf, + ) -> Result<(), TryRuntimeError> { + let current_block = frame_system::Pallet::::block_number(); + + let still_vesting = info.locked_at::(current_block); + + if (info.locked() % info.per_block()).is_zero() { + ensure!( + still_vesting == (schedules_left.saturating_sub(current_block_to_balance) * info.per_block()), + TryRuntimeError::Other("during schedules, the vesting balance should be equal to the total per block releases") + ); + } else { + let re = info.locked() % info.per_block(); + + if current_block_to_balance == schedules_left.saturating_sub(One::one()) { + ensure!(still_vesting == re, TryRuntimeError::Other("At the final vesting block, the vesting balance should be equal to the remainder")); + } + + if current_block_to_balance == schedules_left { + ensure!( + still_vesting == Zero::zero(), + TryRuntimeError::Other("Schedule ended, no more vesting balance") + ); + } + } + Ok(()) + } } impl VestingSchedule for Pallet