[pallet-transaction-storage] Configurable RetentionPeriod

prdoc/pr_10656.prdoc

@@ -0,0 +1,13 @@
+title: '[pallet-transaction-storage] Configurable RetentionPeriod'
+doc:
+- audience: [Node Dev, Runtime Dev]
+  description: |-
+    This PR refactors `pallet-transaction-storage` and `sp-transaction-storage-proof` (the `new_data_provider` inherent provider), both of which rely on a hard-coded `DEFAULT_RETENTION_PERIOD`. This PR:
+    - adds a new configurable argument `retention_period` to the `new_data_provider`
+    - introduces the `TransactionStorageApi::retention_period` runtime API, which the runtime can specify arbitrary
+    - provides an example of using `new_data_provider`, with the node client calling the runtime API when constructing inherent provider data
+crates:
+- name: sp-transaction-storage-proof
+  bump: major
+- name: pallet-transaction-storage
+  bump: major

substrate/bin/node/cli/src/service.rs

@@ -47,6 +47,7 @@ use sc_transaction_pool_api::OffchainTransactionPoolFactory;
 use sp_api::ProvideRuntimeApi;
 use sp_core::crypto::Pair;
 use sp_runtime::{generic, traits::Block as BlockT, SaturatedConversion};
+use sp_transaction_storage_proof::runtime_api::TransactionStorageApi;
 use std::{path::Path, sync::Arc};
 
 /// Host functions required for kitchensink runtime and Substrate node.
@@ -631,6 +632,7 @@ pub fn new_full_base<N: NetworkBackend<Block, <Block as BlockT>::Hash>>(
 						sp_transaction_storage_proof::registration::new_data_provider(
 							&*client_clone,
 							&parent,
+							client_clone.runtime_api().retention_period(parent)?,
 						)?;
 
 					Ok((slot, timestamp, storage_proof))

substrate/bin/node/cli/tests/res/default_genesis_config.json

@@ -100,7 +100,7 @@
   "transactionStorage": {
     "byteFee": 10,
     "entryFee": 1000,
-    "storagePeriod": 100800
+    "retentionPeriod": 100800
   },
   "allianceMotion": {
     "members": []

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

@@ -3776,6 +3776,12 @@ pallet_revive::impl_runtime_apis_plus_revive_traits!(
 		}
 	}
 
+	impl sp_transaction_storage_proof::runtime_api::TransactionStorageApi<Block> for Runtime {
+		fn retention_period() -> NumberFor<Block> {
+			TransactionStorage::retention_period()
+		}
+	}
+
 	#[cfg(feature = "try-runtime")]
 	impl frame_try_runtime::TryRuntime<Block> for Runtime {
 		fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) {

substrate/frame/transaction-storage/Cargo.toml

@@ -46,6 +46,7 @@ std = [
 	"sp-inherents/std",
 	"sp-io/std",
 	"sp-runtime/std",
+	"sp-transaction-storage-proof/std",
 	"tracing/std",
 ]
 runtime-benchmarks = [

substrate/frame/transaction-storage/README.md

@@ -2,8 +2,8 @@
 
 Indexes transactions and manages storage proofs.
 
-Allows storing arbitrary data on the chain. Data is automatically removed after `StoragePeriod` blocks, unless the
-storage is renewed. Validators must submit proof of storing a random chunk of data for block `N - StoragePeriod` when
+Allows storing arbitrary data on the chain. Data is automatically removed after `RetentionPeriod` blocks, unless the
+storage is renewed. Validators must submit proof of storing a random chunk of data for block `N - RetentionPeriod` when
 producing block `N`.
 
 # Running a chain
@@ -17,7 +17,7 @@ cargo run --release -- build-spec --chain=local > sc_init.json
 ```
 
 Edit the json chain spec file to customise the chain. The storage chain genesis params are configured in the
-`transactionStorage` section. Note that `storagePeriod` is specified in blocks and changing it also requires code
+`transactionStorage` section. Note that `retentionPeriod` is specified in blocks and changing it also requires code
 changes at the moment.
 
 Build a raw spec from the init spec.
@@ -34,7 +34,7 @@ cargo run --release -- --chain=sc.json -d /tmp/bob --storage-chain --keep-blocks
 ```
 
 `--storage-chain` enables transaction indexing. `--keep-blocks=100800` enables block pruning. The value here should be
-greater or equal than the storage period. `--ipfs-server` enables serving stored content over IPFS.
+greater or equal than the retention period. `--ipfs-server` enables serving stored content over IPFS.
 
 Once the network is started, any other joining nodes need to sync with `--sync=fast`. Regular sync will fail because
 block pruning removes old blocks. The chain does not keep full block history.
@@ -78,7 +78,7 @@ ipfs swarm connect <substrate peer address>
 ipfs block get /ipfs/<CID> > kitten.jpeg
 ```
 
-To renew data and prevent it from being disposed after the storage period, use `transactionStorage.renew(block, index)`
+To renew data and prevent it from being disposed after the retention period, use `transactionStorage.renew(block, index)`
 where `block` is the block number of the previous store or renew transaction, and index is the index of that transaction
 in the block.
 

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

@@ -169,7 +169,7 @@ mod benchmarks {
 				vec![0u8; T::MaxTransactionSize::get() as usize],
 			)?;
 		}
-		run_to_block::<T>(StoragePeriod::<T>::get() + BlockNumberFor::<T>::one());
+		run_to_block::<T>(crate::Pallet::<T>::retention_period() + BlockNumberFor::<T>::one());
 		let encoded_proof = proof();
 		let proof = TransactionStorageProof::decode(&mut &*encoded_proof).unwrap();
 

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

@@ -57,6 +57,9 @@ pub type CreditOf<T> = Credit<<T as frame_system::Config>::AccountId, <T as Conf
 pub use pallet::*;
 pub use weights::WeightInfo;
 
+/// Default retention period for data (in blocks).
+pub const DEFAULT_RETENTION_PERIOD: u32 = 100800;
+
 // TODO: https://github.com/paritytech/polkadot-bulletin-chain/issues/139 - Clarify purpose of allocator limits and decide whether to remove or use these constants.
 /// Maximum bytes that can be stored in one transaction.
 // Setting higher limit also requires raising the allocator limit.
@@ -218,7 +221,7 @@ pub mod pallet {
 			// Drop obsolete roots. The proof for `obsolete` will be checked later
 			// in this block, so we drop `obsolete` - 1.
 			weight.saturating_accrue(db_weight.reads(1));
-			let period = StoragePeriod::<T>::get();
+			let period = Self::retention_period();
 			let obsolete = n.saturating_sub(period.saturating_add(One::one()));
 			if obsolete > Zero::zero() {
 				weight.saturating_accrue(db_weight.writes(1));
@@ -235,7 +238,7 @@ pub mod pallet {
 				ProofChecked::<T>::take() || {
 					// Proof is not required for early or empty blocks.
 					let number = frame_system::Pallet::<T>::block_number();
-					let period = StoragePeriod::<T>::get();
+					let period = Self::retention_period();
 					let target_number = number.saturating_sub(period);
 
 					target_number.is_zero() || {
@@ -264,20 +267,20 @@ pub mod pallet {
 				!T::MaxTransactionSize::get().is_zero(),
 				"MaxTransactionSize must be greater than zero"
 			);
-			let default_period = sp_transaction_storage_proof::DEFAULT_STORAGE_PERIOD.into();
-			let storage_period = GenesisConfig::<T>::default().storage_period;
+			let default_period = DEFAULT_RETENTION_PERIOD.into();
+			let retention_period = GenesisConfig::<T>::default().retention_period;
 			assert_eq!(
-				storage_period, default_period,
-				"GenesisConfig.storage_period must match DEFAULT_STORAGE_PERIOD"
+				retention_period, default_period,
+				"GenesisConfig.retention_period must match DEFAULT_RETENTION_PERIOD"
 			);
 		}
 	}
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		/// Index and store data off chain. Minimum data size is 1 byte, maximum is
-		/// `MaxTransactionSize`. Data will be removed after `StoragePeriod` blocks, unless `renew`
-		/// is called.
+		/// `MaxTransactionSize`. Data will be removed after `RetentionPeriod` blocks, unless
+		/// `renew` is called.
 		///
 		/// Emits [`Stored`](Event::Stored) when successful.
 		///
@@ -376,7 +379,7 @@ pub mod pallet {
 			Ok(().into())
 		}
 
-		/// Check storage proof for block number `block_number() - StoragePeriod`. If such a block
+		/// Check storage proof for block number `block_number() - RetentionPeriod`. If such a block
 		/// does not exist, the proof is expected to be `None`.
 		///
 		/// ## Complexity
@@ -394,7 +397,7 @@ pub mod pallet {
 
 			// Get the target block metadata.
 			let number = frame_system::Pallet::<T>::block_number();
-			let period = StoragePeriod::<T>::get();
+			let period = Self::retention_period();
 			let target_number = number.saturating_sub(period);
 			ensure!(!target_number.is_zero(), Error::<T>::UnexpectedProof);
 			let transactions =
@@ -456,10 +459,13 @@ pub mod pallet {
 	/// Storage fee per transaction.
 	pub type EntryFee<T: Config> = StorageValue<_, BalanceOf<T>>;
 
-	/// Storage period for data in blocks. Should match `sp_storage_proof::DEFAULT_STORAGE_PERIOD`
-	/// for block authoring.
+	/// Number of blocks for which stored data must be retained.
+	///
+	/// Data older than `RetentionPeriod` blocks is eligible for removal unless it
+	/// has been explicitly renewed. Validators are required to prove possession of
+	/// data corresponding to block `N - RetentionPeriod` when producing block `N`.
 	#[pallet::storage]
-	pub type StoragePeriod<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;
+	pub type RetentionPeriod<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;
 
 	// Intermediates
 	#[pallet::storage]
@@ -474,15 +480,15 @@ pub mod pallet {
 	pub struct GenesisConfig<T: Config> {
 		pub byte_fee: BalanceOf<T>,
 		pub entry_fee: BalanceOf<T>,
-		pub storage_period: BlockNumberFor<T>,
+		pub retention_period: BlockNumberFor<T>,
 	}
 
 	impl<T: Config> Default for GenesisConfig<T> {
 		fn default() -> Self {
 			Self {
 				byte_fee: 10u32.into(),
 				entry_fee: 1000u32.into(),
-				storage_period: sp_transaction_storage_proof::DEFAULT_STORAGE_PERIOD.into(),
+				retention_period: DEFAULT_RETENTION_PERIOD.into(),
 			}
 		}
 	}
@@ -492,7 +498,7 @@ pub mod pallet {
 		fn build(&self) {
 			ByteFee::<T>::put(self.byte_fee);
 			EntryFee::<T>::put(self.entry_fee);
-			StoragePeriod::<T>::put(self.storage_period);
+			RetentionPeriod::<T>::put(self.retention_period);
 		}
 	}
 
@@ -525,14 +531,18 @@ pub mod pallet {
 		) -> Option<BoundedVec<TransactionInfo, T::MaxBlockTransactions>> {
 			Transactions::<T>::get(block)
 		}
-		/// Get ByteFee storage information from outside of this pallet.
+		/// Get ByteFee storage information from the outside of this pallet.
 		pub fn byte_fee() -> Option<BalanceOf<T>> {
 			ByteFee::<T>::get()
 		}
-		/// Get EntryFee storage information from outside of this pallet.
+		/// Get EntryFee storage information from the outside of this pallet.
 		pub fn entry_fee() -> Option<BalanceOf<T>> {
 			EntryFee::<T>::get()
 		}
+		/// Get RetentionPeriod storage information from the outside of this pallet.
+		pub fn retention_period() -> BlockNumberFor<T> {
+			RetentionPeriod::<T>::get()
+		}
 
 		fn apply_fee(sender: T::AccountId, size: u32) -> DispatchResult {
 			let byte_fee = ByteFee::<T>::get().ok_or(Error::<T>::NotConfigured)?;

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

@@ -68,7 +68,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 			..Default::default()
 		},
 		transaction_storage: pallet_transaction_storage::GenesisConfig::<Test> {
-			storage_period: 10,
+			retention_period: 10,
 			byte_fee: 2,
 			entry_fee: 200,
 		},

substrate/primitives/transaction-storage-proof/Cargo.toml

@@ -19,6 +19,7 @@ targets = ["x86_64-unknown-linux-gnu"]
 async-trait = { optional = true, workspace = true }
 codec = { features = ["derive"], workspace = true }
 scale-info = { features = ["derive"], workspace = true }
+sp-api = { workspace = true }
 sp-core = { optional = true, workspace = true }
 sp-inherents = { workspace = true }
 sp-runtime = { workspace = true }
@@ -30,6 +31,7 @@ std = [
 	"async-trait",
 	"codec/std",
 	"scale-info/std",
+	"sp-api/std",
 	"sp-core/std",
 	"sp-inherents/std",
 	"sp-runtime/std",

substrate/primitives/transaction-storage-proof/src/lib.rs

@@ -20,6 +20,8 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
+pub mod runtime_api;
+
 extern crate alloc;
 
 use core::result::Result;
@@ -33,8 +35,6 @@ pub use sp_inherents::Error;
 
 /// The identifier for the proof inherent.
 pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"tx_proof";
-/// Storage period for data.
-pub const DEFAULT_STORAGE_PERIOD: u32 = 100800;
 /// Proof trie value size.
 pub const CHUNK_SIZE: usize = 256;
 
@@ -168,15 +168,14 @@ pub mod registration {
 	pub fn new_data_provider<B, C>(
 		client: &C,
 		parent: &B::Hash,
+		retention_period: NumberFor<B>,
 	) -> Result<InherentDataProvider, Error>
 	where
 		B: BlockT,
 		C: IndexedBody<B>,
 	{
 		let parent_number = client.number(*parent)?.unwrap_or(Zero::zero());
-		let number = parent_number
-			.saturating_add(One::one())
-			.saturating_sub(DEFAULT_STORAGE_PERIOD.into());
+		let number = parent_number.saturating_add(One::one()).saturating_sub(retention_period);
 		if number.is_zero() {
 			// Too early to collect proofs.
 			return Ok(InherentDataProvider::new(None));

substrate/primitives/transaction-storage-proof/src/runtime_api.rs

@@ -0,0 +1,28 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Runtime API definition for the transaction storage proof processing.
+
+use sp_runtime::traits::NumberFor;
+
+sp_api::decl_runtime_apis! {
+	/// Runtime API trait for transaction storage support.
+	pub trait TransactionStorageApi {
+		/// Get the actual value of a retention period in blocks.
+		fn retention_period() -> NumberFor<Block>;
+	}
+}