Remove without_storage_info from pallet transaction-storage (pari…

…tytech#11668)

* Introduce BoundedVec

* Fix typos

* Add comments to the bounds

* Remove migration

* Improve bound value access syntax

bin/node/runtime/src/lib.rs

@@ -1478,6 +1478,10 @@ impl pallet_transaction_storage::Config for Runtime {
 	type Call = Call;
 	type FeeDestination = ();
 	type WeightInfo = pallet_transaction_storage::weights::SubstrateWeight<Runtime>;
+	type MaxBlockTransactions =
+		ConstU32<{ pallet_transaction_storage::DEFAULT_MAX_BLOCK_TRANSACTIONS }>;
+	type MaxTransactionSize =
+		ConstU32<{ pallet_transaction_storage::DEFAULT_MAX_TRANSACTION_SIZE }>;
 }
 
 impl pallet_whitelist::Config for Runtime {
@@ -1674,7 +1678,6 @@ pub type Executive = frame_executive::Executive<
 	frame_system::ChainContext<Runtime>,
 	Runtime,
 	AllPalletsWithSystem,
-	(),
 >;
 
 /// MMR helper types.

frame/transaction-storage/Cargo.toml

@@ -26,6 +26,7 @@ sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/
 sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" }
 sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" }
 sp-transaction-storage-proof = { version = "4.0.0-dev", default-features = false, path = "../../primitives/transaction-storage-proof" }
+log = { version = "0.4.17", default-features = false }
 
 [dev-dependencies]
 sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" }

frame/transaction-storage/src/benchmarking.rs

@@ -21,7 +21,7 @@
 
 use super::*;
 use frame_benchmarking::{benchmarks, whitelisted_caller};
-use frame_support::traits::{Currency, OnFinalize, OnInitialize};
+use frame_support::traits::{Currency, Get, OnFinalize, OnInitialize};
 use frame_system::{EventRecord, Pallet as System, RawOrigin};
 use sp_runtime::traits::{Bounded, One, Zero};
 use sp_std::*;
@@ -126,7 +126,7 @@ pub fn run_to_block<T: Config>(n: T::BlockNumber) {
 
 benchmarks! {
 	store {
-		let l in 1 .. MaxTransactionSize::<T>::get();
+		let l in 1 .. T::MaxTransactionSize::get();
 		let caller: T::AccountId = whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
 	}: _(RawOrigin::Signed(caller.clone()), vec![0u8; l as usize])
@@ -140,7 +140,7 @@ benchmarks! {
 		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
 		TransactionStorage::<T>::store(
 			RawOrigin::Signed(caller.clone()).into(),
-			vec![0u8; MaxTransactionSize::<T>::get() as usize],
+			vec![0u8; T::MaxTransactionSize::get() as usize],
 		)?;
 		run_to_block::<T>(1u32.into());
 	}: _(RawOrigin::Signed(caller.clone()), T::BlockNumber::zero(), 0)
@@ -152,10 +152,10 @@ benchmarks! {
 		run_to_block::<T>(1u32.into());
 		let caller: T::AccountId = whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
-		for _ in 0 .. MaxBlockTransactions::<T>::get() {
+		for _ in 0 .. T::MaxBlockTransactions::get() {
 			TransactionStorage::<T>::store(
 				RawOrigin::Signed(caller.clone()).into(),
-				vec![0u8; MaxTransactionSize::<T>::get() as usize],
+				vec![0u8; T::MaxTransactionSize::get() as usize],
 			)?;
 		}
 		run_to_block::<T>(StoragePeriod::<T>::get() + T::BlockNumber::one());

frame/transaction-storage/src/lib.rs

@@ -28,7 +28,7 @@ mod mock;
 #[cfg(test)]
 mod tests;
 
-use codec::{Decode, Encode};
+use codec::{Decode, Encode, MaxEncodedLen};
 use frame_support::{
 	dispatch::{Dispatchable, GetDispatchInfo},
 	traits::{Currency, OnUnbalanced, ReservableCurrency},
@@ -57,7 +57,16 @@ pub const DEFAULT_MAX_TRANSACTION_SIZE: u32 = 8 * 1024 * 1024;
 pub const DEFAULT_MAX_BLOCK_TRANSACTIONS: u32 = 512;
 
 /// State data for a stored transaction.
-#[derive(Encode, Decode, Clone, sp_runtime::RuntimeDebug, PartialEq, Eq, scale_info::TypeInfo)]
+#[derive(
+	Encode,
+	Decode,
+	Clone,
+	sp_runtime::RuntimeDebug,
+	PartialEq,
+	Eq,
+	scale_info::TypeInfo,
+	MaxEncodedLen,
+)]
 pub struct TransactionInfo {
 	/// Chunk trie root.
 	chunk_root: <BlakeTwo256 as Hash>::Output,
@@ -95,6 +104,10 @@ pub mod pallet {
 		type FeeDestination: OnUnbalanced<NegativeImbalanceOf<Self>>;
 		/// Weight information for extrinsics in this pallet.
 		type WeightInfo: WeightInfo;
+		/// Maximum number of indexed transactions in the block.
+		type MaxBlockTransactions: Get<u32>;
+		/// Maximum data set in a single transaction in bytes.
+		type MaxTransactionSize: Get<u32>;
 	}
 
 	#[pallet::error]
@@ -129,7 +142,6 @@ pub mod pallet {
 
 	#[pallet::pallet]
 	#[pallet::generate_store(pub(super) trait Store)]
-	#[pallet::without_storage_info]
 	pub struct Pallet<T>(_);
 
 	#[pallet::hooks]
@@ -180,7 +192,7 @@ pub mod pallet {
 		pub fn store(origin: OriginFor<T>, data: Vec<u8>) -> DispatchResult {
 			ensure!(data.len() > 0, Error::<T>::EmptyTransaction);
 			ensure!(
-				data.len() <= MaxTransactionSize::<T>::get() as usize,
+				data.len() <= T::MaxTransactionSize::get() as usize,
 				Error::<T>::TransactionTooLarge
 			);
 			let sender = ensure_signed(origin)?;
@@ -198,17 +210,19 @@ pub mod pallet {
 
 			let mut index = 0;
 			<BlockTransactions<T>>::mutate(|transactions| {
-				if transactions.len() + 1 > MaxBlockTransactions::<T>::get() as usize {
+				if transactions.len() + 1 > T::MaxBlockTransactions::get() as usize {
 					return Err(Error::<T>::TooManyTransactions)
 				}
 				let total_chunks = transactions.last().map_or(0, |t| t.block_chunks) + chunk_count;
 				index = transactions.len() as u32;
-				transactions.push(TransactionInfo {
-					chunk_root: root,
-					size: data.len() as u32,
-					content_hash: content_hash.into(),
-					block_chunks: total_chunks,
-				});
+				transactions
+					.try_push(TransactionInfo {
+						chunk_root: root,
+						size: data.len() as u32,
+						content_hash: content_hash.into(),
+						block_chunks: total_chunks,
+					})
+					.map_err(|_| Error::<T>::TooManyTransactions)?;
 				Ok(())
 			})?;
 			Self::deposit_event(Event::Stored { index });
@@ -238,19 +252,20 @@ pub mod pallet {
 
 			let mut index = 0;
 			<BlockTransactions<T>>::mutate(|transactions| {
-				if transactions.len() + 1 > MaxBlockTransactions::<T>::get() as usize {
+				if transactions.len() + 1 > T::MaxBlockTransactions::get() as usize {
 					return Err(Error::<T>::TooManyTransactions)
 				}
 				let chunks = num_chunks(info.size);
 				let total_chunks = transactions.last().map_or(0, |t| t.block_chunks) + chunks;
 				index = transactions.len() as u32;
-				transactions.push(TransactionInfo {
-					chunk_root: info.chunk_root,
-					size: info.size,
-					content_hash: info.content_hash,
-					block_chunks: total_chunks,
-				});
-				Ok(())
+				transactions
+					.try_push(TransactionInfo {
+						chunk_root: info.chunk_root,
+						size: info.size,
+						content_hash: info.content_hash,
+						block_chunks: total_chunks,
+					})
+					.map_err(|_| Error::<T>::TooManyTransactions)
 			})?;
 			Self::deposit_event(Event::Renewed { index });
 			Ok(().into())
@@ -324,8 +339,13 @@ pub mod pallet {
 	/// Collection of transaction metadata by block number.
 	#[pallet::storage]
 	#[pallet::getter(fn transaction_roots)]
-	pub(super) type Transactions<T: Config> =
-		StorageMap<_, Blake2_128Concat, T::BlockNumber, Vec<TransactionInfo>, OptionQuery>;
+	pub(super) type Transactions<T: Config> = StorageMap<
+		_,
+		Blake2_128Concat,
+		T::BlockNumber,
+		BoundedVec<TransactionInfo, T::MaxBlockTransactions>,
+		OptionQuery,
+	>;
 
 	/// Count indexed chunks for each block.
 	#[pallet::storage]
@@ -342,16 +362,6 @@ pub mod pallet {
 	/// Storage fee per transaction.
 	pub(super) type EntryFee<T: Config> = StorageValue<_, BalanceOf<T>>;
 
-	#[pallet::storage]
-	#[pallet::getter(fn max_transaction_size)]
-	/// Maximum data set in a single transaction in bytes.
-	pub(super) type MaxTransactionSize<T: Config> = StorageValue<_, u32, ValueQuery>;
-
-	#[pallet::storage]
-	#[pallet::getter(fn max_block_transactions)]
-	/// Maximum number of indexed transactions in the block.
-	pub(super) type MaxBlockTransactions<T: Config> = StorageValue<_, u32, ValueQuery>;
-
 	/// Storage period for data in blocks. Should match `sp_storage_proof::DEFAULT_STORAGE_PERIOD`
 	/// for block authoring.
 	#[pallet::storage]
@@ -360,7 +370,7 @@ pub mod pallet {
 	// Intermediates
 	#[pallet::storage]
 	pub(super) type BlockTransactions<T: Config> =
-		StorageValue<_, Vec<TransactionInfo>, ValueQuery>;
+		StorageValue<_, BoundedVec<TransactionInfo, T::MaxBlockTransactions>, ValueQuery>;
 
 	/// Was the proof checked in this block?
 	#[pallet::storage]
@@ -371,8 +381,6 @@ pub mod pallet {
 		pub byte_fee: BalanceOf<T>,
 		pub entry_fee: BalanceOf<T>,
 		pub storage_period: T::BlockNumber,
-		pub max_block_transactions: u32,
-		pub max_transaction_size: u32,
 	}
 
 	#[cfg(feature = "std")]
@@ -382,8 +390,6 @@ pub mod pallet {
 				byte_fee: 10u32.into(),
 				entry_fee: 1000u32.into(),
 				storage_period: sp_transaction_storage_proof::DEFAULT_STORAGE_PERIOD.into(),
-				max_block_transactions: DEFAULT_MAX_BLOCK_TRANSACTIONS,
-				max_transaction_size: DEFAULT_MAX_TRANSACTION_SIZE,
 			}
 		}
 	}
@@ -393,8 +399,6 @@ pub mod pallet {
 		fn build(&self) {
 			<ByteFee<T>>::put(&self.byte_fee);
 			<EntryFee<T>>::put(&self.entry_fee);
-			<MaxTransactionSize<T>>::put(&self.max_transaction_size);
-			<MaxBlockTransactions<T>>::put(&self.max_block_transactions);
 			<StoragePeriod<T>>::put(&self.storage_period);
 		}
 	}

frame/transaction-storage/src/mock.rs

@@ -17,8 +17,10 @@
 
 //! Test environment for transaction-storage pallet.
 
-use crate as pallet_transaction_storage;
-use crate::TransactionStorageProof;
+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::{
@@ -90,6 +92,8 @@ impl pallet_transaction_storage::Config for Test {
 	type Currency = Balances;
 	type FeeDestination = ();
 	type WeightInfo = ();
+	type MaxBlockTransactions = ConstU32<{ DEFAULT_MAX_BLOCK_TRANSACTIONS }>;
+	type MaxTransactionSize = ConstU32<{ DEFAULT_MAX_TRANSACTION_SIZE }>;
 }
 
 pub fn new_test_ext() -> sp_io::TestExternalities {
@@ -102,8 +106,6 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 			storage_period: 10,
 			byte_fee: 2,
 			entry_fee: 200,
-			max_block_transactions: crate::DEFAULT_MAX_BLOCK_TRANSACTIONS,
-			max_transaction_size: crate::DEFAULT_MAX_TRANSACTION_SIZE,
 		},
 	}
 	.build_storage()