Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove getter from vesting pallet #4902

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()`.
Aideepakchaudhary marked this conversation as resolved.
Show resolved Hide resolved

crates:
- name: pallet-vesting
bump: minor
Aideepakchaudhary marked this conversation as resolved.
Show resolved Hide resolved
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)
}
Aideepakchaudhary marked this conversation as resolved.
Show resolved Hide resolved

// 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
Loading