Introduce and Implement VestedTransfer Trait

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

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},

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<AccountId> {

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

@@ -112,3 +112,52 @@ pub trait VestingSchedule<AccountId> {
 	/// 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<AccountId> {
+	/// The quantity used to denote time; usually just a `BlockNumber`.
+	type Moment;
+
+	/// The currency that this schedule applies to.
+	type Currency: Currency<AccountId>;
+
+	// 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: <Self::Currency as Currency<AccountId>>::Balance,
+		per_block: <Self::Currency as Currency<AccountId>>::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.
+impl<AccountId> VestedTransfer<AccountId> for () {
+	type Moment = ();
+	type Currency = ();
+
+	fn vested_transfer(
+		_source: &AccountId,
+		_target: &AccountId,
+		_locked: <Self::Currency as Currency<AccountId>>::Balance,
+		_per_block: <Self::Currency as Currency<AccountId>>::Balance,
+		_starting_block: Self::Moment,
+	) -> DispatchResult {
+		Err(sp_runtime::DispatchError::Unavailable.into())
+	}
+}

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,
 };
@@ -534,32 +534,25 @@ impl<T: Config> Pallet<T> {
 		if !schedule.is_valid() {
 			return Err(Error::<T>::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,
-			schedule.locked(),
-			schedule.per_block(),
-			schedule.starting_block(),
-		)?;
-
 		T::Currency::transfer(
 			&source,
 			&target,
 			schedule.locked(),
 			ExistenceRequirement::AllowDeath,
 		)?;
 
-		// We can't let this fail because the currency transfer has already happened.
-		let res = Self::add_vesting_schedule(
+		// If adding this vesting schedule fails, all storage changes are undone due to FRAME's
+		// default transactional layers.
+		Self::add_vesting_schedule(
 			&target,
 			schedule.locked(),
 			schedule.per_block(),
 			schedule.starting_block(),
-		);
-		debug_assert!(res.is_ok(), "Failed to add a schedule when we had to succeed.");
+		)?;
 
 		Ok(())
 	}
@@ -751,8 +744,7 @@ where
 		Ok(())
 	}
 
-	// Ensure we can call `add_vesting_schedule` without error. This should always
-	// be called prior to `add_vesting_schedule`.
+	/// Checks if `add_vesting_schedule` would work against `who`.
 	fn can_add_vesting_schedule(
 		who: &T::AccountId,
 		locked: BalanceOf<T>,
@@ -784,3 +776,24 @@ where
 		Ok(())
 	}
 }
+
+impl<T: Config> VestedTransfer<T::AccountId> for Pallet<T>
+where
+	BalanceOf<T>: MaybeSerializeDeserialize + Debug,
+{
+	type Currency = T::Currency;
+	type Moment = BlockNumberFor<T>;
+
+	fn vested_transfer(
+		source: &T::AccountId,
+		target: &T::AccountId,
+		locked: BalanceOf<T>,
+		per_block: BalanceOf<T>,
+		starting_block: BlockNumberFor<T>,
+	) -> DispatchResult {
+		let schedule = VestingInfo::new(locked, per_block, starting_block);
+		let source = <T::Lookup as StaticLookup>::unlookup(source.clone());
+		let target = <T::Lookup as StaticLookup>::unlookup(target.clone());
+		Self::do_vested_transfer(source, target, schedule)
+	}
+}

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::<Test>::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::<Test>::get(&2).unwrap(), vec![sched0, sched1]);