[pallet-transaction-storage] Configurable RetentionPeriod

prdoc/pr_10656.prdoc

@@ -0,0 +1,33 @@
+title: '[pallet-transaction-storage] Configurable StoragePeriod'
+doc:
+- audience:
+  - Runtime Dev
+  - Node Dev
+  description: "Relates to: https://github.com/paritytech/polkadot-sdk/pull/10494\n\
+    Relates to: https://github.com/paritytech/polkadot-bulletin-chain/issues/82\n\
+    Relates to: https://github.com/paritytech/polkadot-bulletin-chain/issues/60\n\n\
+    ## Context/Problem\n\nThis 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_STORAGE_PERIOD`.\n\nThe `new_data_provider` uses a hard-coded `pub\
+    \ const DEFAULT_STORAGE_PERIOD: u32 = 100800;`, which forces the runtime to use\
+    \ the same value. This value corresponds to 7 days for 6-second blocks. We want\
+    \ to make this constant configurable and driven by the runtime \u2014 exposed\
+    \ via a runtime API. For example, if we switch to 2-second blocks but still want\
+    \ to keep the 7-day period, we won\u2019t need to coordinate and restart the collators\
+    \ with a new hard-coded constant and so on. Basically, we want to have possibility\
+    \ to set this constant with arbitrary value without affecting the collators or\
+    \ anything.\n\n## Solution\n\nThis PR:\n* adds a new argument `storage_period`\
+    \ to the `new_data_provider`\n* introduces the `TransactionStorageApi::storage_period`\
+    \ runtime API, which the runtime can specify arbitrary\n* provides an example\
+    \ of using `new_data_provider`, with the node client calling the runtime API when\
+    \ constructing inherent provider data\n  * I didn't want to complicate the node\
+    \ client traits, so `retrieve_storage_period` uses just direct `call_api_at(\"\
+    TransactionStorageApi_storage_period\", at_block)`\n\n## TODO\n- [ ] Is there\
+    \ any alternative to `call_api_at` for how a node\u2019s custom inherent data\
+    \ provider can call a runtime API declared via `decl_runtime_apis`?\n\n## Follow-up\n\
+    \nIt will be used for Bulletin setup here: https://github.com/paritytech/polkadot-sdk/pull/10494"
+crates:
+- name: sp-transaction-storage-proof
+  bump: patch
+- name: pallet-transaction-storage
+  bump: patch

substrate/bin/node/cli/Cargo.toml

@@ -169,6 +169,7 @@ wat = { workspace = true }
 node-testing = { workspace = true }
 sc-service-test = { workspace = true }
 substrate-cli-test-utils = { workspace = true }
+substrate-rpc-client = { workspace = true }
 
 [build-dependencies]
 clap = { optional = true, workspace = true }

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

@@ -627,11 +627,11 @@ pub fn new_full_base<N: NetworkBackend<Block, <Block as BlockT>::Hash>>(
 							slot_duration,
 						);
 
-					let storage_proof =
-						sp_transaction_storage_proof::registration::new_data_provider(
-							&*client_clone,
-							&parent,
-						)?;
+					let storage_proof = sp_transaction_storage_proof::registration::new_data_provider(
+						&*client_clone,
+						&parent,
+						sp_transaction_storage_proof::runtime_api::client::retrieve_storage_period(&client_clone, parent)?,
+					)?;
 
 					Ok((slot, timestamp, storage_proof))
 				}

substrate/bin/node/cli/tests/runtime_api_works.rs

@@ -0,0 +1,56 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
+
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+#![cfg(unix)]
+use codec::Decode;
+use polkadot_sdk::sp_runtime::traits::NumberFor;
+use std::time::Duration;
+use substrate_cli_test_utils as common;
+use substrate_rpc_client::{ws_client, StateApi};
+
+#[tokio::test]
+async fn transaction_storage_runtime_api_call_works() {
+	common::run_with_timeout(Duration::from_secs(60 * 10), async move {
+		// Run the node.
+		let mut node = common::KillChildOnDrop(common::start_node());
+		let stderr = node.stderr.take().unwrap();
+		let ws_url = common::extract_info_from_output(stderr).0.ws_url;
+		common::wait_n_finalized_blocks(1, &ws_url).await;
+		let block_hash = common::block_hash(1, &ws_url).await.unwrap();
+		node.assert_still_running();
+
+		// Call the runtime API.
+		let rpc = ws_client(ws_url).await.unwrap();
+		let result = rpc
+			.call(
+				String::from("TransactionStorageApi_storage_period"),
+				vec![].into(),
+				Some(block_hash),
+			)
+			.await
+			.unwrap();
+
+		// Decode and assert the received value.
+		let storage_period: NumberFor<kitchensink_runtime::Block> =
+			Decode::decode(&mut &result.0[..]).unwrap();
+		assert_eq!(storage_period, 100800);
+
+		node.stop();
+	})
+	.await;
+}

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 storage_period() -> NumberFor<Block> {
+			TransactionStorage::storage_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/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>::storage_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 storage period for data (in blocks).
+pub const DEFAULT_STORAGE_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::storage_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::storage_period();
 					let target_number = number.saturating_sub(period);
 
 					target_number.is_zero() || {
@@ -264,7 +267,7 @@ 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 default_period = DEFAULT_STORAGE_PERIOD.into();
 			let storage_period = GenesisConfig::<T>::default().storage_period;
 			assert_eq!(
 				storage_period, default_period,
@@ -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::storage_period();
 			let target_number = number.saturating_sub(period);
 			ensure!(!target_number.is_zero(), Error::<T>::UnexpectedProof);
 			let transactions =
@@ -456,8 +459,7 @@ 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.
+	/// Storage period for data in blocks.
 	#[pallet::storage]
 	pub type StoragePeriod<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;
 
@@ -482,7 +484,7 @@ pub mod pallet {
 			Self {
 				byte_fee: 10u32.into(),
 				entry_fee: 1000u32.into(),
-				storage_period: sp_transaction_storage_proof::DEFAULT_STORAGE_PERIOD.into(),
+				storage_period: DEFAULT_STORAGE_PERIOD.into(),
 			}
 		}
 	}
@@ -525,14 +527,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 StoragePeriod storage information from the outside of this pallet.
+		pub fn storage_period() -> BlockNumberFor<T> {
+			StoragePeriod::<T>::get()
+		}
 
 		fn apply_fee(sender: T::AccountId, size: u32) -> DispatchResult {
 			let byte_fee = ByteFee::<T>::get().ok_or(Error::<T>::NotConfigured)?;

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

@@ -20,6 +20,7 @@ async-trait = { optional = true, workspace = true }
 codec = { features = ["derive"], workspace = true }
 scale-info = { features = ["derive"], workspace = true }
 sp-core = { optional = true, workspace = true }
+sp-api = { workspace = true }
 sp-inherents = { workspace = true }
 sp-runtime = { workspace = true }
 sp-trie = { optional = true, 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,
+		storage_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(storage_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,77 @@
+// 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 storage period in blocks.
+		fn storage_period() -> NumberFor<Block>;
+	}
+}
+
+#[cfg(feature = "std")]
+pub mod client {
+	use codec::Decode;
+	use sp_api::{ApiError, CallApiAt, CallApiAtParams};
+	use sp_core::traits::CallContext;
+	use sp_runtime::traits::Block as BlockT;
+	use sp_runtime::traits::NumberFor;
+
+	/// An expected state call key.
+	pub const TRANSACTION_STORAGE_API_STORAGE_PERIOD: &'static str =
+		"TransactionStorageApi_storage_period";
+
+	/// Fetches the storage period value for a specific block from the runtime API.
+	///
+	/// This function interacts with the `TRANSACTION_STORAGE_API_STORAGE_PERIOD` API
+	/// provided by the runtime.
+	///
+	/// # Arguments
+	/// - `client`: A reference to an object implementing the `CallApiAt` trait,
+	///   used to interact with the runtime API.
+	/// - `at_block`: The hash of the specific block for which the storage period is queried.
+	pub fn retrieve_storage_period<B, C>(
+		client: &C,
+		at_block: B::Hash,
+	) -> Result<NumberFor<B>, ApiError>
+	where
+		B: BlockT,
+		C: CallApiAt<B>,
+	{
+		// Call the expected runtime API.
+		let result = client.call_api_at(CallApiAtParams {
+			at: at_block,
+			function: TRANSACTION_STORAGE_API_STORAGE_PERIOD,
+			arguments: vec![],
+			overlayed_changes: &Default::default(),
+			call_context: CallContext::Onchain,
+			recorder: &None,
+			extensions: &Default::default(),
+		})?;
+
+		// Decode to `NumberFor<B>`.
+		Decode::decode(&mut &result[..]).map_err(|e| ApiError::FailedToDecodeReturnValue {
+			function: TRANSACTION_STORAGE_API_STORAGE_PERIOD,
+			error: e,
+			raw: result,
+		})
+	}
+}