diff --git a/prdoc/pr_5630.prdoc b/prdoc/pr_5630.prdoc new file mode 100644 index 000000000000..bafb9e746d40 --- /dev/null +++ b/prdoc/pr_5630.prdoc @@ -0,0 +1,15 @@ +# 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: Introduce and Implement the `VestedTransfer` Trait + +doc: + - audience: Runtime Dev + description: | + This PR introduces a new trait `VestedTransfer` which is implemented by `pallet_vesting`. With this, other pallets can easily introduce vested transfers into their logic. + +crates: + - name: frame-support + bump: minor + - name: pallet-vesting + bump: minor diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 6e59ff9d030b..631225b9a327 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -23,7 +23,8 @@ pub mod tokens; pub use tokens::{ currency::{ ActiveIssuanceOf, Currency, InspectLockableCurrency, LockIdentifier, LockableCurrency, - NamedReservableCurrency, ReservableCurrency, TotalIssuanceOf, VestingSchedule, + NamedReservableCurrency, ReservableCurrency, TotalIssuanceOf, VestedTransfer, + VestingSchedule, }, fungible, fungibles, imbalance::{Imbalance, OnUnbalanced, SignedImbalance}, diff --git a/substrate/frame/support/src/traits/tokens/currency.rs b/substrate/frame/support/src/traits/tokens/currency.rs index b3db4c98001d..ea2c66a32cb0 100644 --- a/substrate/frame/support/src/traits/tokens/currency.rs +++ b/substrate/frame/support/src/traits/tokens/currency.rs @@ -30,7 +30,9 @@ use sp_runtime::{traits::MaybeSerializeDeserialize, DispatchError}; mod reservable; pub use reservable::{NamedReservableCurrency, ReservableCurrency}; mod lockable; -pub use lockable::{InspectLockableCurrency, LockIdentifier, LockableCurrency, VestingSchedule}; +pub use lockable::{ + InspectLockableCurrency, LockIdentifier, LockableCurrency, VestedTransfer, VestingSchedule, +}; /// Abstraction over a fungible assets system. pub trait Currency { diff --git a/substrate/frame/support/src/traits/tokens/currency/lockable.rs b/substrate/frame/support/src/traits/tokens/currency/lockable.rs index 51a48dd15ce8..4ec45c908e68 100644 --- a/substrate/frame/support/src/traits/tokens/currency/lockable.rs +++ b/substrate/frame/support/src/traits/tokens/currency/lockable.rs @@ -112,3 +112,56 @@ pub trait VestingSchedule { /// NOTE: This doesn't alter the free balance of the account. fn remove_vesting_schedule(who: &AccountId, schedule_index: u32) -> DispatchResult; } + +/// A vested transfer over a currency. This allows a transferred amount to vest over time. +pub trait VestedTransfer { + /// The quantity used to denote time; usually just a `BlockNumber`. + type Moment; + + /// The currency that this schedule applies to. + type Currency: Currency; + + /// Execute a vested transfer from `source` to `target` with the given schedule: + /// - `locked`: The amount to be transferred and for the vesting schedule to apply to. + /// - `per_block`: The amount to be unlocked each block. (linear vesting) + /// - `starting_block`: The block where the vesting should start. This block can be in the past + /// or future, and should adjust when the tokens become available to the user. + /// + /// Example: Assume we are on block 100. If `locked` amount is 100, and `per_block` is 1: + /// - If `starting_block` is 0, then the whole 100 tokens will be available right away as the + /// vesting schedule started in the past and has fully completed. + /// - If `starting_block` is 50, then 50 tokens are made available right away, and 50 more + /// tokens will unlock one token at a time until block 150. + /// - If `starting_block` is 100, then each block, 1 token will be unlocked until the whole + /// balance is unlocked at block 200. + /// - If `starting_block` is 200, then the 100 token balance will be completely locked until + /// block 200, and then start to unlock one token at a time until block 300. + fn vested_transfer( + source: &AccountId, + target: &AccountId, + locked: >::Balance, + per_block: >::Balance, + starting_block: Self::Moment, + ) -> DispatchResult; +} + +// An no-op implementation of `VestedTransfer` for pallets that require this trait, but users may +// not want to implement this functionality +pub struct NoVestedTransfers { + phantom: core::marker::PhantomData, +} + +impl> VestedTransfer for NoVestedTransfers { + type Moment = (); + type Currency = C; + + fn vested_transfer( + _source: &AccountId, + _target: &AccountId, + _locked: >::Balance, + _per_block: >::Balance, + _starting_block: Self::Moment, + ) -> DispatchResult { + Err(sp_runtime::DispatchError::Unavailable.into()) + } +} diff --git a/substrate/frame/vesting/src/benchmarking.rs b/substrate/frame/vesting/src/benchmarking.rs index 68214c4f47cc..736dd6eac1a8 100644 --- a/substrate/frame/vesting/src/benchmarking.rs +++ b/substrate/frame/vesting/src/benchmarking.rs @@ -42,7 +42,7 @@ fn add_locks(who: &T::AccountId, n: u8) { } fn add_vesting_schedules( - target: AccountIdLookupOf, + target: &T::AccountId, n: u32, ) -> Result, &'static str> { let min_transfer = T::MinVestedTransfer::get(); @@ -52,7 +52,6 @@ fn add_vesting_schedules( let starting_block = 1u32; let source: T::AccountId = account("source", 0, SEED); - let source_lookup = T::Lookup::unlookup(source.clone()); T::Currency::make_free_balance_be(&source, BalanceOf::::max_value()); T::BlockNumberProvider::set_block_number(BlockNumberFor::::zero()); @@ -62,11 +61,7 @@ fn add_vesting_schedules( total_locked += locked; let schedule = VestingInfo::new(locked, per_block, starting_block.into()); - assert_ok!(Vesting::::do_vested_transfer( - source_lookup.clone(), - target.clone(), - schedule - )); + assert_ok!(Vesting::::do_vested_transfer(&source, target, schedule)); // Top up to guarantee we can always transfer another schedule. T::Currency::make_free_balance_be(&source, BalanceOf::::max_value()); @@ -81,11 +76,10 @@ benchmarks! { let s in 1 .. T::MAX_VESTING_SCHEDULES; let caller: T::AccountId = whitelisted_caller(); - let caller_lookup = T::Lookup::unlookup(caller.clone()); T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance()); add_locks::(&caller, l as u8); - let expected_balance = add_vesting_schedules::(caller_lookup, s)?; + let expected_balance = add_vesting_schedules::(&caller, s)?; // At block zero, everything is vested. assert_eq!(System::::block_number(), BlockNumberFor::::zero()); @@ -109,11 +103,10 @@ benchmarks! { let s in 1 .. T::MAX_VESTING_SCHEDULES; let caller: T::AccountId = whitelisted_caller(); - let caller_lookup = T::Lookup::unlookup(caller.clone()); T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance()); add_locks::(&caller, l as u8); - add_vesting_schedules::(caller_lookup, s)?; + add_vesting_schedules::(&caller, s)?; // At block 21, everything is unlocked. T::BlockNumberProvider::set_block_number(21u32.into()); @@ -141,7 +134,7 @@ benchmarks! { T::Currency::make_free_balance_be(&other, T::Currency::minimum_balance()); add_locks::(&other, l as u8); - let expected_balance = add_vesting_schedules::(other_lookup.clone(), s)?; + let expected_balance = add_vesting_schedules::(&other, s)?; // At block zero, everything is vested. assert_eq!(System::::block_number(), BlockNumberFor::::zero()); @@ -171,7 +164,7 @@ benchmarks! { T::Currency::make_free_balance_be(&other, T::Currency::minimum_balance()); add_locks::(&other, l as u8); - add_vesting_schedules::(other_lookup.clone(), s)?; + add_vesting_schedules::(&other, s)?; // At block 21 everything is unlocked. T::BlockNumberProvider::set_block_number(21u32.into()); @@ -206,7 +199,7 @@ benchmarks! { add_locks::(&target, l as u8); // Add one vesting schedules. let orig_balance = T::Currency::free_balance(&target); - let mut expected_balance = add_vesting_schedules::(target_lookup.clone(), s)?; + let mut expected_balance = add_vesting_schedules::(&target, s)?; let transfer_amount = T::MinVestedTransfer::get(); let per_block = transfer_amount.checked_div(&20u32.into()).unwrap(); @@ -246,7 +239,7 @@ benchmarks! { add_locks::(&target, l as u8); // Add one less than max vesting schedules let orig_balance = T::Currency::free_balance(&target); - let mut expected_balance = add_vesting_schedules::(target_lookup.clone(), s)?; + let mut expected_balance = add_vesting_schedules::(&target, s)?; let transfer_amount = T::MinVestedTransfer::get(); let per_block = transfer_amount.checked_div(&20u32.into()).unwrap(); @@ -281,7 +274,7 @@ benchmarks! { T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance()); add_locks::(&caller, l as u8); // Add max vesting schedules. - let expected_balance = add_vesting_schedules::(caller_lookup, s)?; + let expected_balance = add_vesting_schedules::(&caller, s)?; // Schedules are not vesting at block 0. assert_eq!(System::::block_number(), BlockNumberFor::::zero()); @@ -332,7 +325,7 @@ benchmarks! { T::Currency::make_free_balance_be(&caller, T::Currency::minimum_balance()); add_locks::(&caller, l as u8); // Add max vesting schedules. - let total_transferred = add_vesting_schedules::(caller_lookup, s)?; + let total_transferred = add_vesting_schedules::(&caller, s)?; // Go to about half way through all the schedules duration. (They all start at 1, and have a duration of 20 or 21). T::BlockNumberProvider::set_block_number(11u32.into()); @@ -397,7 +390,7 @@ force_remove_vesting_schedule { // Give target existing locks. add_locks::(&target, l as u8); - let _ = add_vesting_schedules::(target_lookup.clone(), s)?; + add_vesting_schedules::(&target, s)?; // The last vesting schedule. let schedule_index = s - 1; diff --git a/substrate/frame/vesting/src/lib.rs b/substrate/frame/vesting/src/lib.rs index bfc10efeed79..15f8d397f81c 100644 --- a/substrate/frame/vesting/src/lib.rs +++ b/substrate/frame/vesting/src/lib.rs @@ -66,8 +66,8 @@ use frame_support::{ ensure, storage::bounded_vec::BoundedVec, traits::{ - Currency, ExistenceRequirement, Get, LockIdentifier, LockableCurrency, VestingSchedule, - WithdrawReasons, + Currency, ExistenceRequirement, Get, LockIdentifier, LockableCurrency, VestedTransfer, + VestingSchedule, WithdrawReasons, }, weights::Weight, }; @@ -351,8 +351,8 @@ pub mod pallet { schedule: VestingInfo, BlockNumberFor>, ) -> DispatchResult { let transactor = ensure_signed(origin)?; - let transactor = ::unlookup(transactor); - Self::do_vested_transfer(transactor, target, schedule) + let target = T::Lookup::lookup(target)?; + Self::do_vested_transfer(&transactor, &target, schedule) } /// Force a vested transfer. @@ -380,7 +380,9 @@ pub mod pallet { schedule: VestingInfo, BlockNumberFor>, ) -> DispatchResult { ensure_root(origin)?; - Self::do_vested_transfer(source, target, schedule) + let target = T::Lookup::lookup(target)?; + let source = T::Lookup::lookup(source)?; + Self::do_vested_transfer(&source, &target, schedule) } /// Merge two vesting schedules together, creating a new vesting schedule that unlocks over @@ -525,8 +527,8 @@ impl Pallet { // Execute a vested transfer from `source` to `target` with the given `schedule`. fn do_vested_transfer( - source: AccountIdLookupOf, - target: AccountIdLookupOf, + source: &T::AccountId, + target: &T::AccountId, schedule: VestingInfo, BlockNumberFor>, ) -> DispatchResult { // Validate user inputs. @@ -534,27 +536,22 @@ impl Pallet { if !schedule.is_valid() { return Err(Error::::InvalidScheduleParams.into()) }; - let target = T::Lookup::lookup(target)?; - let source = T::Lookup::lookup(source)?; // Check we can add to this account prior to any storage writes. Self::can_add_vesting_schedule( - &target, + target, schedule.locked(), schedule.per_block(), schedule.starting_block(), )?; - T::Currency::transfer( - &source, - &target, - schedule.locked(), - ExistenceRequirement::AllowDeath, - )?; + T::Currency::transfer(source, target, schedule.locked(), ExistenceRequirement::AllowDeath)?; // We can't let this fail because the currency transfer has already happened. + // Must be successful as it has been checked before. + // Better to return error on failure anyway. let res = Self::add_vesting_schedule( - &target, + target, schedule.locked(), schedule.per_block(), schedule.starting_block(), @@ -751,8 +748,8 @@ where Ok(()) } - // Ensure we can call `add_vesting_schedule` without error. This should always - // be called prior to `add_vesting_schedule`. + /// Ensure we can call `add_vesting_schedule` without error. This should always + /// be called prior to `add_vesting_schedule`. fn can_add_vesting_schedule( who: &T::AccountId, locked: BalanceOf, @@ -784,3 +781,32 @@ where Ok(()) } } + +/// An implementation that allows the Vesting Pallet to handle a vested transfer +/// on behalf of another Pallet. +impl VestedTransfer for Pallet +where + BalanceOf: MaybeSerializeDeserialize + Debug, +{ + type Currency = T::Currency; + type Moment = BlockNumberFor; + + fn vested_transfer( + source: &T::AccountId, + target: &T::AccountId, + locked: BalanceOf, + per_block: BalanceOf, + starting_block: BlockNumberFor, + ) -> DispatchResult { + use frame_support::storage::{with_transaction, TransactionOutcome}; + let schedule = VestingInfo::new(locked, per_block, starting_block); + with_transaction(|| -> TransactionOutcome { + let result = Self::do_vested_transfer(source, target, schedule); + + match &result { + Ok(()) => TransactionOutcome::Commit(result), + _ => TransactionOutcome::Rollback(result), + } + }) + } +} diff --git a/substrate/frame/vesting/src/tests.rs b/substrate/frame/vesting/src/tests.rs index 57cb59f27a4d..0dd7133d930a 100644 --- a/substrate/frame/vesting/src/tests.rs +++ b/substrate/frame/vesting/src/tests.rs @@ -280,13 +280,14 @@ 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!(Balances::transfer_allow_death(Some(1).into(), 3, 155)); // Account 1 can send extra units gained + // Account 1 can send extra units gained + assert_ok!(Balances::transfer_allow_death(Some(1).into(), 3, 155)); // 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!(Balances::transfer_allow_death(Some(2).into(), 3, 100)); // Account 2 can send extra - // units gained + // Account 2 can send extra units gained + assert_ok!(Balances::transfer_allow_death(Some(2).into(), 3, 100)); }); } @@ -295,14 +296,16 @@ fn liquid_funds_should_transfer_with_delayed_vesting() { ExtBuilder::default().existential_deposit(256).build().execute_with(|| { let user12_free_balance = Balances::free_balance(&12); - assert_eq!(user12_free_balance, 2560); // Account 12 has free balance - // Account 12 has liquid funds + // Account 12 has free balance + assert_eq!(user12_free_balance, 2560); + // Account 12 has liquid funds assert_eq!(Vesting::vesting_balance(&12), Some(user12_free_balance - 256 * 5)); // Account 12 has delayed vesting let user12_vesting_schedule = VestingInfo::new( 256 * 5, - 64, // Vesting over 20 blocks + // Vesting over 20 blocks + 64, 10, ); assert_eq!(VestingStorage::::get(&12).unwrap(), vec![user12_vesting_schedule]); @@ -630,8 +633,10 @@ fn merge_ongoing_schedules() { let sched1 = VestingInfo::new( ED * 10, - ED, // Vest over 10 blocks. - sched0.starting_block() + 5, // Start at block 15. + // Vest over 10 blocks. + ED, + // Start at block 15. + sched0.starting_block() + 5, ); assert_ok!(Vesting::vested_transfer(Some(4).into(), 2, sched1)); assert_eq!(VestingStorage::::get(&2).unwrap(), vec![sched0, sched1]); @@ -1191,3 +1196,43 @@ fn remove_vesting_schedule() { ); }); } + +#[test] +fn vested_transfer_impl_works() { + ExtBuilder::default().existential_deposit(ED).build().execute_with(|| { + assert_eq!(Balances::free_balance(&3), 256 * 30); + assert_eq!(Balances::free_balance(&4), 256 * 40); + // Account 4 should not have any vesting yet. + assert_eq!(VestingStorage::::get(&4), None); + + // Basic working scenario + assert_ok!(>::vested_transfer( + &3, + &4, + ED * 5, + ED * 5 / 20, + 10 + )); + // Now account 4 should have vesting. + let new_vesting_schedule = VestingInfo::new( + ED * 5, + (ED * 5) / 20, // Vesting over 20 blocks + 10, + ); + assert_eq!(VestingStorage::::get(&4).unwrap(), vec![new_vesting_schedule]); + // Account 4 has 5 * 256 locked. + assert_eq!(Vesting::vesting_balance(&4), Some(256 * 5)); + + // If the transfer fails (because they don't have enough balance), no storage is changed. + assert_noop!( + >::vested_transfer(&3, &4, ED * 9999, ED * 5 / 20, 10), + TokenError::FundsUnavailable + ); + + // If applying the vesting schedule fails (per block is 0), no storage is changed. + assert_noop!( + >::vested_transfer(&3, &4, ED * 5, 0, 10), + Error::::InvalidScheduleParams + ); + }); +}