Skip to content

Commit

Permalink
remove getter from vesting pallet (paritytech#4902)
Browse files Browse the repository at this point in the history
### ISSUE
Link to the issue:
paritytech#3326

cc @muraca 

Deliverables
 - [Deprecation] remove pallet::getter usage from all pallet-vesting
 

### Test Outcomes
___
Successful tests by running `cargo test -p pallet-vesting --features
runtime-benchmarks`

running 45 tests
test benchmarking::bench_force_vested_transfer ... ok
test benchmarking::bench_vest_other_locked ... ok
test mock::__construct_runtime_integrity_test::runtime_integrity_tests
... ok
test benchmarking::bench_not_unlocking_merge_schedules ... ok
test benchmarking::bench_unlocking_merge_schedules ... ok
test mock::test_genesis_config_builds ... ok
test tests::build_genesis_has_storage_version_v1 ... ok
test tests::check_vesting_status ... ok
test benchmarking::bench_force_remove_vesting_schedule ... ok
test tests::check_vesting_status_for_multi_schedule_account ... ok
test benchmarking::bench_vest_locked ... ok
test tests::extra_balance_should_transfer ... ok
test tests::generates_multiple_schedules_from_genesis_config ... ok
test tests::force_vested_transfer_allows_max_schedules ... ok
test tests::force_vested_transfer_correctly_fails ... ok
test tests::force_vested_transfer_works ... ok
test tests::liquid_funds_should_transfer_with_delayed_vesting ... ok
test tests::merge_finished_and_ongoing_schedules ... ok
test benchmarking::bench_vest_unlocked ... ok
test tests::merge_finished_and_yet_to_be_started_schedules ... ok
test tests::merge_finishing_schedules_does_not_create_a_new_one ... ok
test tests::merge_ongoing_and_yet_to_be_started_schedules ... ok
test benchmarking::bench_vest_other_unlocked ... ok
test tests::merge_ongoing_schedules ... ok
test tests::merge_schedules_that_have_not_started ... ok
test tests::merge_vesting_handles_per_block_0 ... ok
test tests::per_block_works ... ok
test tests::merge_schedules_throws_proper_errors ... ok
test tests::multiple_schedules_from_genesis_config_errors - should panic
... ok
test tests::merging_shifts_other_schedules_index ... ok
test tests::non_vested_cannot_vest_other ... ok
test tests::unvested_balance_should_not_transfer ... ok
test tests::non_vested_cannot_vest ... ok
test tests::vested_balance_should_transfer ... ok
test tests::remove_vesting_schedule ... ok
test tests::vested_transfer_correctly_fails ... ok
test tests::vested_balance_should_transfer_with_multi_sched ... ok
test tests::vested_balance_should_transfer_using_vest_other ... ok
test tests::vested_transfer_less_than_existential_deposit_fails ... ok
test tests::vesting_info_ending_block_as_balance_works ... ok
test tests::vesting_info_validate_works ... ok
test
tests::vested_balance_should_transfer_using_vest_other_with_multi_sched
... ok
test tests::vested_transfer_works ... ok
test tests::vested_transfer_allows_max_schedules ... ok
test benchmarking::bench_vested_transfer ... ok

test result: ok. 45 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
out; finished in 0.10s

---

Polkadot Address: 16htXkeVhfroBhL6nuqiwknfXKcT6WadJPZqEi2jRf9z4XPY
  • Loading branch information
Aideepakchaudhary authored Jul 5, 2024
1 parent 95c7e4f commit 2b17cb8
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 60 deletions.
14 changes: 14 additions & 0 deletions prdoc/pr_4902.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Removed `pallet::getter` usage from the pallet-vesting

doc:
- audience: Runtime Dev
description: |
This PR removed `pallet::getter`s from `pallet-vesting`s storage items.
When accessed inside the pallet, use the syntax `StorageItem::<T>::get()`.

crates:
- name: pallet-vesting
bump: minor
18 changes: 9 additions & 9 deletions substrate/frame/vesting/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use frame_support::assert_ok;
use frame_system::{pallet_prelude::BlockNumberFor, Pallet as System, RawOrigin};
use sp_runtime::traits::{Bounded, CheckedDiv, CheckedMul};

use super::*;
use super::{Vesting as VestingStorage, *};
use crate::Pallet as Vesting;

const SEED: u32 = 0;
Expand Down Expand Up @@ -291,7 +291,7 @@ benchmarks! {
"Vesting balance should equal sum locked of all schedules",
);
assert_eq!(
Vesting::<T>::vesting(&caller).unwrap().len(),
VestingStorage::<T>::get(&caller).unwrap().len(),
s as usize,
"There should be exactly max vesting schedules"
);
Expand All @@ -304,7 +304,7 @@ benchmarks! {
);
let expected_index = (s - 2) as usize;
assert_eq!(
Vesting::<T>::vesting(&caller).unwrap()[expected_index],
VestingStorage::<T>::get(&caller).unwrap()[expected_index],
expected_schedule
);
assert_eq!(
Expand All @@ -313,7 +313,7 @@ benchmarks! {
"Vesting balance should equal total locked of all schedules",
);
assert_eq!(
Vesting::<T>::vesting(&caller).unwrap().len(),
VestingStorage::<T>::get(&caller).unwrap().len(),
(s - 1) as usize,
"Schedule count should reduce by 1"
);
Expand Down Expand Up @@ -344,7 +344,7 @@ benchmarks! {
"Vesting balance should reflect that we are half way through all schedules duration",
);
assert_eq!(
Vesting::<T>::vesting(&caller).unwrap().len(),
VestingStorage::<T>::get(&caller).unwrap().len(),
s as usize,
"There should be exactly max vesting schedules"
);
Expand All @@ -359,12 +359,12 @@ benchmarks! {
);
let expected_index = (s - 2) as usize;
assert_eq!(
Vesting::<T>::vesting(&caller).unwrap()[expected_index],
VestingStorage::<T>::get(&caller).unwrap()[expected_index],
expected_schedule,
"New schedule is properly created and placed"
);
assert_eq!(
Vesting::<T>::vesting(&caller).unwrap()[expected_index],
VestingStorage::<T>::get(&caller).unwrap()[expected_index],
expected_schedule
);
assert_eq!(
Expand All @@ -373,7 +373,7 @@ benchmarks! {
"Vesting balance should equal half total locked of all schedules",
);
assert_eq!(
Vesting::<T>::vesting(&caller).unwrap().len(),
VestingStorage::<T>::get(&caller).unwrap().len(),
(s - 1) as usize,
"Schedule count should reduce by 1"
);
Expand Down Expand Up @@ -404,7 +404,7 @@ force_remove_vesting_schedule {
}: _(RawOrigin::Root, target_lookup, schedule_index)
verify {
assert_eq!(
Vesting::<T>::vesting(&target).unwrap().len(),
VestingStorage::<T>::get(&target).unwrap().len(),
schedule_index as usize,
"Schedule count should reduce by 1"
);
Expand Down
19 changes: 13 additions & 6 deletions substrate/frame/vesting/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ pub mod pallet {

/// Information regarding the vesting of a given account.
#[pallet::storage]
#[pallet::getter(fn vesting)]
pub type Vesting<T: Config> = StorageMap<
_,
Blake2_128Concat,
Expand Down Expand Up @@ -419,7 +418,7 @@ pub mod pallet {
let schedule1_index = schedule1_index as usize;
let schedule2_index = schedule2_index as usize;

let schedules = Self::vesting(&who).ok_or(Error::<T>::NotVesting)?;
let schedules = Vesting::<T>::get(&who).ok_or(Error::<T>::NotVesting)?;
let merge_action =
VestingAction::Merge { index1: schedule1_index, index2: schedule2_index };

Expand Down Expand Up @@ -464,6 +463,14 @@ pub mod pallet {
}

impl<T: Config> Pallet<T> {
// Public function for accessing vesting storage
pub fn vesting(
account: T::AccountId,
) -> Option<BoundedVec<VestingInfo<BalanceOf<T>, BlockNumberFor<T>>, MaxVestingSchedulesGet<T>>>
{
Vesting::<T>::get(account)
}

// Create a new `VestingInfo`, based off of two other `VestingInfo`s.
// NOTE: We assume both schedules have had funds unlocked up through the current block.
fn merge_vesting_info(
Expand Down Expand Up @@ -622,7 +629,7 @@ impl<T: Config> Pallet<T> {

/// Unlock any vested funds of `who`.
fn do_vest(who: T::AccountId) -> DispatchResult {
let schedules = Self::vesting(&who).ok_or(Error::<T>::NotVesting)?;
let schedules = Vesting::<T>::get(&who).ok_or(Error::<T>::NotVesting)?;

let (schedules, locked_now) =
Self::exec_action(schedules.to_vec(), VestingAction::Passive)?;
Expand Down Expand Up @@ -687,7 +694,7 @@ where

/// Get the amount that is currently being vested and cannot be transferred out of this account.
fn vesting_balance(who: &T::AccountId) -> Option<BalanceOf<T>> {
if let Some(v) = Self::vesting(who) {
if let Some(v) = Vesting::<T>::get(who) {
let now = T::BlockNumberProvider::current_block_number();
let total_locked_now = v.iter().fold(Zero::zero(), |total, schedule| {
schedule.locked_at::<T::BlockNumberToBalance>(now).saturating_add(total)
Expand Down Expand Up @@ -726,7 +733,7 @@ where
return Err(Error::<T>::InvalidScheduleParams.into())
};

let mut schedules = Self::vesting(who).unwrap_or_default();
let mut schedules = Vesting::<T>::get(who).unwrap_or_default();

// NOTE: we must push the new schedule so that `exec_action`
// will give the correct new locked amount.
Expand Down Expand Up @@ -764,7 +771,7 @@ where

/// Remove a vesting schedule for a given account.
fn remove_vesting_schedule(who: &T::AccountId, schedule_index: u32) -> DispatchResult {
let schedules = Self::vesting(who).ok_or(Error::<T>::NotVesting)?;
let schedules = Vesting::<T>::get(who).ok_or(Error::<T>::NotVesting)?;
let remove_action = VestingAction::Remove { index: schedule_index as usize };

let (schedules, locked_now) = Self::exec_action(schedules.to_vec(), remove_action)?;
Expand Down
Loading

0 comments on commit 2b17cb8

Please sign in to comment.