Refactor transaction storage pallet to use fungible traits (#1800)

Partial https://github.com/paritytech/polkadot-sdk/issues/226

`frame/transaction-storage`: replace `Currency` with `fungible::*`
traits

---------

Signed-off-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: georgepisaltu <52418509+georgepisaltu@users.noreply.github.com>

substrate/bin/node/runtime/src/lib.rs

@@ -526,7 +526,7 @@ impl pallet_balances::Config for Runtime {
 	type WeightInfo = pallet_balances::weights::SubstrateWeight<Runtime>;
 	type FreezeIdentifier = RuntimeFreezeReason;
 	type MaxFreezes = ConstU32<1>;
-	type MaxHolds = ConstU32<5>;
+	type MaxHolds = ConstU32<6>;
 }
 
 parameter_types! {
@@ -1833,6 +1833,7 @@ impl pallet_nfts::Config for Runtime {
 impl pallet_transaction_storage::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
 	type Currency = Balances;
+	type RuntimeHoldReason = RuntimeHoldReason;
 	type RuntimeCall = RuntimeCall;
 	type FeeDestination = ();
 	type WeightInfo = pallet_transaction_storage::weights::SubstrateWeight<Runtime>;

substrate/frame/transaction-storage/src/benchmarking.rs

@@ -21,9 +21,9 @@
 
 use super::*;
 use frame_benchmarking::v1::{benchmarks, whitelisted_caller};
-use frame_support::traits::{Currency, Get, OnFinalize, OnInitialize};
+use frame_support::traits::{Get, OnFinalize, OnInitialize};
 use frame_system::{pallet_prelude::BlockNumberFor, EventRecord, Pallet as System, RawOrigin};
-use sp_runtime::traits::{Bounded, One, Zero};
+use sp_runtime::traits::{Bounded, CheckedDiv, One, Zero};
 use sp_std::*;
 use sp_transaction_storage_proof::TransactionStorageProof;
 
@@ -103,9 +103,6 @@ fn proof() -> Vec<u8> {
 	array_bytes::hex2bytes_unchecked(PROOF)
 }
 
-type BalanceOf<T> =
-	<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
-
 fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
 	let events = System::<T>::events();
 	let system_event: <T as frame_system::Config>::RuntimeEvent = generic_event.into();
@@ -129,7 +126,8 @@ benchmarks! {
 	store {
 		let l in 1 .. T::MaxTransactionSize::get();
 		let caller: T::AccountId = whitelisted_caller();
-		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
+		let initial_balance = BalanceOf::<T>::max_value().checked_div(&2u32.into()).unwrap();
+		T::Currency::set_balance(&caller, initial_balance);
 	}: _(RawOrigin::Signed(caller.clone()), vec![0u8; l as usize])
 	verify {
 		assert!(!BlockTransactions::<T>::get().is_empty());
@@ -138,7 +136,8 @@ benchmarks! {
 
 	renew {
 		let caller: T::AccountId = whitelisted_caller();
-		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
+		let initial_balance = BalanceOf::<T>::max_value().checked_div(&2u32.into()).unwrap();
+		T::Currency::set_balance(&caller, initial_balance);
 		TransactionStorage::<T>::store(
 			RawOrigin::Signed(caller.clone()).into(),
 			vec![0u8; T::MaxTransactionSize::get() as usize],
@@ -152,7 +151,8 @@ benchmarks! {
 	check_proof_max {
 		run_to_block::<T>(1u32.into());
 		let caller: T::AccountId = whitelisted_caller();
-		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
+		let initial_balance = BalanceOf::<T>::max_value().checked_div(&2u32.into()).unwrap();
+		T::Currency::set_balance(&caller, initial_balance);
 		for _ in 0 .. T::MaxBlockTransactions::get() {
 			TransactionStorage::<T>::store(
 				RawOrigin::Signed(caller.clone()).into(),

substrate/frame/transaction-storage/src/lib.rs

@@ -31,7 +31,14 @@ mod tests;
 use codec::{Decode, Encode, MaxEncodedLen};
 use frame_support::{
 	dispatch::GetDispatchInfo,
-	traits::{Currency, OnUnbalanced, ReservableCurrency},
+	traits::{
+		fungible::{
+			hold::Balanced as FnBalanced, Inspect as FnInspect, Mutate as FnMutate,
+			MutateHold as FnMutateHold,
+		},
+		tokens::fungible::Credit,
+		OnUnbalanced,
+	},
 };
 use sp_runtime::traits::{BlakeTwo256, Dispatchable, Hash, One, Saturating, Zero};
 use sp_std::{prelude::*, result};
@@ -42,10 +49,8 @@ use sp_transaction_storage_proof::{
 
 /// A type alias for the balance type from this pallet's point of view.
 type BalanceOf<T> =
-	<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
+	<<T as Config>::Currency as FnInspect<<T as frame_system::Config>::AccountId>>::Balance;
-type NegativeImbalanceOf<T> = <<T as Config>::Currency as Currency<
+pub type CreditOf<T> = Credit<<T as frame_system::Config>::AccountId, <T as Config>::Currency>;
-	<T as frame_system::Config>::AccountId,
->>::NegativeImbalance;
 
 // Re-export pallet items so that they can be accessed from the crate namespace.
 pub use pallet::*;
@@ -89,6 +94,13 @@ pub mod pallet {
 	use frame_support::pallet_prelude::*;
 	use frame_system::pallet_prelude::*;
 
+	/// A reason for this pallet placing a hold on funds.
+	#[pallet::composite_enum]
+	pub enum HoldReason {
+		/// The funds are held as deposit for the used storage.
+		StorageFeeHold,
+	}
+
 	#[pallet::config]
 	pub trait Config: frame_system::Config {
 		/// The overarching event type.
@@ -98,10 +110,14 @@ pub mod pallet {
 			+ Dispatchable<RuntimeOrigin = Self::RuntimeOrigin>
 			+ GetDispatchInfo
 			+ From<frame_system::Call<Self>>;
-		/// The currency trait.
+		/// The fungible type for this pallet.
-		type Currency: ReservableCurrency<Self::AccountId>;
+		type Currency: FnMutate<Self::AccountId>
+			+ FnMutateHold<Self::AccountId, Reason = Self::RuntimeHoldReason>
+			+ FnBalanced<Self::AccountId>;
+		/// The overarching runtime hold reason.
+		type RuntimeHoldReason: From<HoldReason>;
 		/// Handler for the unbalanced decrease when fees are burned.
-		type FeeDestination: OnUnbalanced<NegativeImbalanceOf<Self>>;
+		type FeeDestination: OnUnbalanced<CreditOf<Self>>;
 		/// Weight information for extrinsics in this pallet.
 		type WeightInfo: WeightInfo;
 		/// Maximum number of indexed transactions in the block.
@@ -112,8 +128,6 @@ pub mod pallet {
 
 	#[pallet::error]
 	pub enum Error<T> {
-		/// Insufficient account balance.
-		InsufficientFunds,
 		/// Invalid configuration.
 		NotConfigured,
 		/// Renewed extrinsic is not found.
@@ -432,8 +446,10 @@ pub mod pallet {
 			let byte_fee = ByteFee::<T>::get().ok_or(Error::<T>::NotConfigured)?;
 			let entry_fee = EntryFee::<T>::get().ok_or(Error::<T>::NotConfigured)?;
 			let fee = byte_fee.saturating_mul(size.into()).saturating_add(entry_fee);
-			ensure!(T::Currency::can_slash(&sender, fee), Error::<T>::InsufficientFunds);
+			T::Currency::hold(&HoldReason::StorageFeeHold.into(), &sender, fee)?;
-			let (credit, _) = T::Currency::slash(&sender, fee);
+			let (credit, _remainder) =
+				T::Currency::slash(&HoldReason::StorageFeeHold.into(), &sender, fee);
+			debug_assert!(_remainder.is_zero());
 			T::FeeDestination::on_unbalanced(credit);
 			Ok(())
 		}

substrate/frame/transaction-storage/src/mock.rs

@@ -21,12 +21,11 @@ use crate::{
 	self as pallet_transaction_storage, TransactionStorageProof, DEFAULT_MAX_BLOCK_TRANSACTIONS,
 	DEFAULT_MAX_TRANSACTION_SIZE,
 };
-use frame_support::traits::{ConstU16, ConstU32, ConstU64, OnFinalize, OnInitialize};
+use frame_support::{
-use sp_core::H256;
+	derive_impl,
-use sp_runtime::{
+	traits::{ConstU32, ConstU64, OnFinalize, OnInitialize},
-	traits::{BlakeTwo256, IdentityLookup},
-	BuildStorage,
 };
+use sp_runtime::{traits::IdentityLookup, BuildStorage};
 
 pub type Block = frame_system::mocking::MockBlock<Test>;
 
@@ -37,58 +36,35 @@ frame_support::construct_runtime!(
 		System: frame_system::{Pallet, Call, Config<T>, Storage, Event<T>},
 		Balances: pallet_balances::{Pallet, Call, Config<T>, Storage, Event<T>},
 		TransactionStorage: pallet_transaction_storage::{
-			Pallet, Call, Storage, Config<T>, Inherent, Event<T>
+			Pallet, Call, Storage, Config<T>, Inherent, Event<T>, HoldReason
 		},
 	}
 );
 
+#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)]
 impl frame_system::Config for Test {
-	type BaseCallFilter = frame_support::traits::Everything;
-	type BlockWeights = ();
-	type BlockLength = ();
-	type RuntimeOrigin = RuntimeOrigin;
-	type RuntimeCall = RuntimeCall;
-	type Nonce = u64;
-	type Hash = H256;
-	type Hashing = BlakeTwo256;
-	type AccountId = u64;
-	type Lookup = IdentityLookup<Self::AccountId>;
 	type Block = Block;
-	type RuntimeEvent = RuntimeEvent;
-	type BlockHashCount = ConstU64<250>;
-	type DbWeight = ();
-	type Version = ();
-	type PalletInfo = PalletInfo;
 	type AccountData = pallet_balances::AccountData<u64>;
-	type OnNewAccount = ();
+	type AccountId = u64;
-	type OnKilledAccount = ();
+	type BlockHashCount = ConstU64<250>;
-	type SystemWeightInfo = ();
+	type Lookup = IdentityLookup<Self::AccountId>;
-	type SS58Prefix = ConstU16<42>;
-	type OnSetCode = ();
-	type MaxConsumers = ConstU32<16>;
 }
 
+#[derive_impl(pallet_balances::config_preludes::TestDefaultConfig as pallet_balances::DefaultConfig)]
 impl pallet_balances::Config for Test {
 	type Balance = u64;
-	type DustRemoval = ();
-	type RuntimeEvent = RuntimeEvent;
 	type ExistentialDeposit = ConstU64<1>;
 	type AccountStore = System;
-	type WeightInfo = ();
+	type RuntimeHoldReason = RuntimeHoldReason;
-	type MaxLocks = ();
+	type RuntimeFreezeReason = RuntimeFreezeReason;
-	type MaxReserves = ();
+	type MaxHolds = ConstU32<128>;
-	type ReserveIdentifier = ();
-	type FreezeIdentifier = ();
-	type MaxFreezes = ();
-	type RuntimeHoldReason = ();
-	type RuntimeFreezeReason = ();
-	type MaxHolds = ();
 }
 
 impl pallet_transaction_storage::Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type RuntimeCall = RuntimeCall;
 	type Currency = Balances;
+	type RuntimeHoldReason = RuntimeHoldReason;
 	type FeeDestination = ();
 	type WeightInfo = ();
 	type MaxBlockTransactions = ConstU32<{ DEFAULT_MAX_BLOCK_TRANSACTIONS }>;

substrate/frame/transaction-storage/src/tests.rs

@@ -21,6 +21,7 @@ use super::{Pallet as TransactionStorage, *};
 use crate::mock::*;
 use frame_support::{assert_noop, assert_ok};
 use frame_system::RawOrigin;
+use sp_runtime::{DispatchError, TokenError::FundsUnavailable};
 use sp_transaction_storage_proof::registration::build_proof;
 
 const MAX_DATA_SIZE: u32 = DEFAULT_MAX_TRANSACTION_SIZE;
@@ -71,7 +72,7 @@ fn burns_fee() {
 				RawOrigin::Signed(5).into(),
 				vec![0u8; 2000 as usize]
 			),
-			Error::<Test>::InsufficientFunds,
+			DispatchError::Token(FundsUnavailable),
 		);
 		assert_ok!(TransactionStorage::<Test>::store(
 			RawOrigin::Signed(caller).into(),