diff --git a/Cargo.lock b/Cargo.lock index a5e431c7a4aa0..d08a177b84b5a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6375,6 +6375,7 @@ dependencies = [ "frame-support", "frame-system", "hex-literal", + "log", "pallet-balances", "parity-scale-codec", "scale-info", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 8e854b770a4ff..520c7e380f3dc 100644 --- a/bin/node/runtime/src/lib.rs +++ b/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; + 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 { @@ -1672,7 +1676,6 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, - (), >; /// MMR helper types. diff --git a/frame/transaction-storage/Cargo.toml b/frame/transaction-storage/Cargo.toml index a5195c4f75974..f001ef6acd468 100644 --- a/frame/transaction-storage/Cargo.toml +++ b/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" } diff --git a/frame/transaction-storage/src/benchmarking.rs b/frame/transaction-storage/src/benchmarking.rs index 285b5cba7ad22..83dd37922a31f 100644 --- a/frame/transaction-storage/src/benchmarking.rs +++ b/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(n: T::BlockNumber) { benchmarks! { store { - let l in 1 .. MaxTransactionSize::::get(); + let l in 1 .. T::MaxTransactionSize::get(); let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); }: _(RawOrigin::Signed(caller.clone()), vec![0u8; l as usize]) @@ -140,7 +140,7 @@ benchmarks! { T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); TransactionStorage::::store( RawOrigin::Signed(caller.clone()).into(), - vec![0u8; MaxTransactionSize::::get() as usize], + vec![0u8; T::MaxTransactionSize::get() as usize], )?; run_to_block::(1u32.into()); }: _(RawOrigin::Signed(caller.clone()), T::BlockNumber::zero(), 0) @@ -152,10 +152,10 @@ benchmarks! { run_to_block::(1u32.into()); let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - for _ in 0 .. MaxBlockTransactions::::get() { + for _ in 0 .. T::MaxBlockTransactions::get() { TransactionStorage::::store( RawOrigin::Signed(caller.clone()).into(), - vec![0u8; MaxTransactionSize::::get() as usize], + vec![0u8; T::MaxTransactionSize::get() as usize], )?; } run_to_block::(StoragePeriod::::get() + T::BlockNumber::one()); diff --git a/frame/transaction-storage/src/lib.rs b/frame/transaction-storage/src/lib.rs index a63b31f2f1aac..f16b8f029662b 100644 --- a/frame/transaction-storage/src/lib.rs +++ b/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: ::Output, @@ -95,6 +104,10 @@ pub mod pallet { type FeeDestination: OnUnbalanced>; /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; + /// Maximum number of indexed transactions in the block. + type MaxBlockTransactions: Get; + /// Maximum data set in a single transaction in bytes. + type MaxTransactionSize: Get; } #[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(_); #[pallet::hooks] @@ -180,7 +192,7 @@ pub mod pallet { pub fn store(origin: OriginFor, data: Vec) -> DispatchResult { ensure!(data.len() > 0, Error::::EmptyTransaction); ensure!( - data.len() <= MaxTransactionSize::::get() as usize, + data.len() <= T::MaxTransactionSize::get() as usize, Error::::TransactionTooLarge ); let sender = ensure_signed(origin)?; @@ -198,17 +210,19 @@ pub mod pallet { let mut index = 0; >::mutate(|transactions| { - if transactions.len() + 1 > MaxBlockTransactions::::get() as usize { + if transactions.len() + 1 > T::MaxBlockTransactions::get() as usize { return Err(Error::::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::::TooManyTransactions)?; Ok(()) })?; Self::deposit_event(Event::Stored { index }); @@ -238,19 +252,20 @@ pub mod pallet { let mut index = 0; >::mutate(|transactions| { - if transactions.len() + 1 > MaxBlockTransactions::::get() as usize { + if transactions.len() + 1 > T::MaxBlockTransactions::get() as usize { return Err(Error::::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::::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 = - StorageMap<_, Blake2_128Concat, T::BlockNumber, Vec, OptionQuery>; + pub(super) type Transactions = StorageMap< + _, + Blake2_128Concat, + T::BlockNumber, + BoundedVec, + OptionQuery, + >; /// Count indexed chunks for each block. #[pallet::storage] @@ -342,16 +362,6 @@ pub mod pallet { /// Storage fee per transaction. pub(super) type EntryFee = StorageValue<_, BalanceOf>; - #[pallet::storage] - #[pallet::getter(fn max_transaction_size)] - /// Maximum data set in a single transaction in bytes. - pub(super) type MaxTransactionSize = StorageValue<_, u32, ValueQuery>; - - #[pallet::storage] - #[pallet::getter(fn max_block_transactions)] - /// Maximum number of indexed transactions in the block. - pub(super) type MaxBlockTransactions = 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 = - StorageValue<_, Vec, ValueQuery>; + StorageValue<_, BoundedVec, ValueQuery>; /// Was the proof checked in this block? #[pallet::storage] @@ -371,8 +381,6 @@ pub mod pallet { pub byte_fee: BalanceOf, pub entry_fee: BalanceOf, 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) { >::put(&self.byte_fee); >::put(&self.entry_fee); - >::put(&self.max_transaction_size); - >::put(&self.max_block_transactions); >::put(&self.storage_period); } } diff --git a/frame/transaction-storage/src/mock.rs b/frame/transaction-storage/src/mock.rs index 753d4baaf00e2..771387ef705be 100644 --- a/frame/transaction-storage/src/mock.rs +++ b/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()