From 30f3ad2eefce06fc3a1a063b57af22e9d75bb903 Mon Sep 17 00:00:00 2001
From: Adrian Catangiu <adrian@parity.io>
Date: Mon, 30 Oct 2023 15:15:36 +0200
Subject: [PATCH] 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 | 3 +-
.../transaction-storage/src/benchmarking.rs | 16 +++---
.../frame/transaction-storage/src/lib.rs | 40 +++++++++-----
.../frame/transaction-storage/src/mock.rs | 52 +++++--------------
.../frame/transaction-storage/src/tests.rs | 3 +-
5 files changed, 54 insertions(+), 60 deletions(-)
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index a2d100e1f8b..f3c24897632 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/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>;
diff --git a/substrate/frame/transaction-storage/src/benchmarking.rs b/substrate/frame/transaction-storage/src/benchmarking.rs
index fdbaeb1f951..8d485d9f3ca 100644
--- a/substrate/frame/transaction-storage/src/benchmarking.rs
+++ b/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(),
diff --git a/substrate/frame/transaction-storage/src/lib.rs b/substrate/frame/transaction-storage/src/lib.rs
index 753f5ca0c7b..fb8ada0f5f9 100644
--- a/substrate/frame/transaction-storage/src/lib.rs
+++ b/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;
-type NegativeImbalanceOf<T> = <<T as Config>::Currency as Currency<
- <T as frame_system::Config>::AccountId,
->>::NegativeImbalance;
+ <<T as Config>::Currency as FnInspect<<T as frame_system::Config>::AccountId>>::Balance;
+pub type CreditOf<T> = Credit<<T as frame_system::Config>::AccountId, <T as Config>::Currency>;
// 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.
- type Currency: ReservableCurrency<Self::AccountId>;
+ /// The fungible type for this pallet.
+ 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);
- let (credit, _) = T::Currency::slash(&sender, fee);
+ T::Currency::hold(&HoldReason::StorageFeeHold.into(), &sender, fee)?;
+ let (credit, _remainder) =
+ T::Currency::slash(&HoldReason::StorageFeeHold.into(), &sender, fee);
+ debug_assert!(_remainder.is_zero());
T::FeeDestination::on_unbalanced(credit);
Ok(())
}
diff --git a/substrate/frame/transaction-storage/src/mock.rs b/substrate/frame/transaction-storage/src/mock.rs
index 947c81c12ac..a8da19a382d 100644
--- a/substrate/frame/transaction-storage/src/mock.rs
+++ b/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 sp_core::H256;
-use sp_runtime::{
- traits::{BlakeTwo256, IdentityLookup},
- BuildStorage,
+use frame_support::{
+ derive_impl,
+ traits::{ConstU32, ConstU64, OnFinalize, OnInitialize},
};
+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 OnKilledAccount = ();
- type SystemWeightInfo = ();
- type SS58Prefix = ConstU16<42>;
- type OnSetCode = ();
- type MaxConsumers = ConstU32<16>;
+ type AccountId = u64;
+ type BlockHashCount = ConstU64<250>;
+ type Lookup = IdentityLookup<Self::AccountId>;
}
+#[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 MaxLocks = ();
- type MaxReserves = ();
- type ReserveIdentifier = ();
- type FreezeIdentifier = ();
- type MaxFreezes = ();
- type RuntimeHoldReason = ();
- type RuntimeFreezeReason = ();
- type MaxHolds = ();
+ type RuntimeHoldReason = RuntimeHoldReason;
+ type RuntimeFreezeReason = RuntimeFreezeReason;
+ type MaxHolds = ConstU32<128>;
}
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 }>;
diff --git a/substrate/frame/transaction-storage/src/tests.rs b/substrate/frame/transaction-storage/src/tests.rs
index 43dfed81f88..e17b3ca3beb 100644
--- a/substrate/frame/transaction-storage/src/tests.rs
+++ b/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(),
--
GitLab