Align StoragePeriod between Bulletin and SDK

pallets/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 producing block `N`.
+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
 
@@ -16,7 +16,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 changes at the moment.
+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.
 
@@ -32,7 +32,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.
+`--keep-blocks=100800` enables block pruning. The value here should be 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.
@@ -75,7 +75,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 transction, and index is the index of that transaction in the block.
 
 

pallets/transaction-storage/src/benchmarking.rs

@@ -157,7 +157,7 @@ mod benchmarks {
 				vec![0u8; T::MaxTransactionSize::get() as usize],
 			)?;
 		}
-		run_to_block::<T>(T::StoragePeriod::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();
 

pallets/transaction-storage/src/lib.rs

@@ -24,19 +24,26 @@
 // Ensure we're `no_std` when compiling for Wasm.
 #![cfg_attr(not(feature = "std"), no_std)]
 
+extern crate alloc;
+
 mod benchmarking;
 pub mod weights;
 
+pub mod migrations;
 #[cfg(test)]
 mod mock;
 #[cfg(test)]
 mod tests;
 
 use codec::{Decode, Encode, MaxEncodedLen};
+use core::fmt::Debug;
 use polkadot_sdk_frame::{
 	deps::{sp_core::sp_std::prelude::*, *},
 	prelude::*,
-	traits::fungible::{Balanced, Credit, Inspect, Mutate, MutateHold},
+	traits::{
+		fungible::{Balanced, Credit, Inspect, Mutate, MutateHold},
+		parameter_types,
+	},
 };
 use sp_transaction_storage_proof::{
 	encode_index, num_chunks, random_chunk, ChunkIndex, InherentError, TransactionStorageProof,
@@ -54,12 +61,16 @@ pub use weights::WeightInfo;
 
 const LOG_TARGET: &str = "runtime::transaction-storage";
 
+/// Default retention period for data (in blocks).
+pub const DEFAULT_RETENTION_PERIOD: u32 = 100800;
+parameter_types! {
+	pub const DefaultRetentionPeriod: u32 = DEFAULT_RETENTION_PERIOD;
+}
+
+// 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.
-// TODO: find out what is "allocator" and "allocator limit"
 // Setting higher limit also requires raising the allocator limit.
-// TODO: not used, can we remove or use?
 pub const DEFAULT_MAX_TRANSACTION_SIZE: u32 = 8 * 1024 * 1024;
-// TODO: not used, can we remove or use?
 pub const DEFAULT_MAX_BLOCK_TRANSACTIONS: u32 = 512;
 
 /// Encountered an impossible situation, implies a bug.
@@ -74,7 +85,7 @@ pub const AUTHORIZATION_NOT_FOUND: InvalidTransaction = InvalidTransaction::Cust
 pub const AUTHORIZATION_NOT_EXPIRED: InvalidTransaction = InvalidTransaction::Custom(4);
 
 /// Number of transactions and bytes covered by an authorization.
-#[derive(PartialEq, Eq, RuntimeDebug, Encode, Decode, scale_info::TypeInfo, MaxEncodedLen)]
+#[derive(PartialEq, Eq, Debug, Encode, Decode, scale_info::TypeInfo, MaxEncodedLen)]
 pub struct AuthorizationExtent {
 	/// Number of transactions.
 	pub transactions: u32,
@@ -108,9 +119,7 @@ struct Authorization<BlockNumber> {
 type AuthorizationFor<T> = Authorization<BlockNumberFor<T>>;
 
 /// State data for a stored transaction.
-#[derive(
-	Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, scale_info::TypeInfo, MaxEncodedLen,
-)]
+#[derive(Encode, Decode, Clone, Debug, PartialEq, Eq, scale_info::TypeInfo, MaxEncodedLen)]
 pub struct TransactionInfo {
 	/// Chunk trie root.
 	chunk_root: <BlakeTwo256 as Hash>::Output,
@@ -122,7 +131,7 @@ pub struct TransactionInfo {
 	/// is used to find transaction info by block chunk index using binary search.
 	///
 	/// Cumulative value of all previous transactions in the block; the last transaction holds the
-	/// total chunk value.
+	/// total chunks.
 	block_chunks: ChunkIndex,
 }
 
@@ -194,11 +203,6 @@ pub mod pallet {
 		/// Maximum data set in a single transaction in bytes.
 		#[pallet::constant]
 		type MaxTransactionSize: Get<u32>;
-		/// Storage period for data in blocks. Should match
-		/// [`DEFAULT_STORAGE_PERIOD`](sp_transaction_storage_proof::DEFAULT_STORAGE_PERIOD) for
-		/// block authoring.
-		#[pallet::constant]
-		type StoragePeriod: Get<BlockNumberFor<Self>>;
 		/// Authorizations expire after this many blocks.
 		#[pallet::constant]
 		type AuthorizationPeriod: Get<BlockNumberFor<Self>>;
@@ -258,13 +262,14 @@ pub mod pallet {
 	#[pallet::hooks]
 	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
 		fn on_initialize(n: BlockNumberFor<T>) -> Weight {
+			// TODO: https://github.com/paritytech/polkadot-sdk/issues/10203 - Replace this with benchmarked weights.
 			let mut weight = Weight::zero();
 			let db_weight = T::DbWeight::get();
 
 			// 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 = T::StoragePeriod::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(2));
@@ -282,7 +287,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 = T::StoragePeriod::get();
+					let period = Self::retention_period();
 					let target_number = number.saturating_sub(period);
 
 					target_number.is_zero() || {
@@ -294,7 +299,7 @@ pub mod pallet {
 				"Storage proof must be checked once in the block"
 			);
 
-			// Insert new transactions
+			// Insert new transactions, iff they have chunks.
 			let transactions = <BlockTransactions<T>>::take();
 			let total_chunks = transactions.last().map_or(0, |t| t.block_chunks);
 			if total_chunks != 0 {
@@ -305,22 +310,30 @@ pub mod pallet {
 		fn integrity_test() {
 			assert!(
 				!T::MaxBlockTransactions::get().is_zero(),
-				"Not useful if data cannot be stored"
+				"MaxTransactionSize must be greater than zero"
+			);
+			assert!(
+				!T::MaxTransactionSize::get().is_zero(),
+				"MaxTransactionSize must be greater than zero"
+			);
+			let default_period = DEFAULT_RETENTION_PERIOD.into();
+			let retention_period = GenesisConfig::<T>::default().retention_period;
+			assert_eq!(
+				retention_period, default_period,
+				"GenesisConfig.retention_period must match DEFAULT_RETENTION_PERIOD"
 			);
-			assert!(!T::MaxTransactionSize::get().is_zero(), "Not useful if data cannot be stored");
-			assert!(!T::StoragePeriod::get().is_zero(), "Not useful if data is not stored");
 			assert!(
 				!T::AuthorizationPeriod::get().is_zero(),
-				"Not useful if authorizations are never valid"
+				"AuthorizationPeriod must be greater than zero"
 			);
 		}
 	}
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
-		/// Index and store data off chain. Minimum data size is 1 bytes, maximum is
-		/// `MaxTransactionSize`. Data will be removed after `StoragePeriod` blocks, unless `renew`
-		/// is called.
+		/// Index and store data off chain. Minimum data size is 1 byte, maximum is
+		/// `MaxTransactionSize`. Data will be removed after `RetentionPeriod` blocks, unless
+		/// `renew` is called.
 		///
 		/// Authorization is required to store data using regular signed/unsigned transactions.
 		/// Regular signed transactions require account authorization (see
@@ -346,9 +359,9 @@ pub mod pallet {
 			debug_assert_eq!(chunk_count, num_chunks(data.len() as u32) as usize);
 			let root = sp_io::trie::blake2_256_ordered_root(chunks, sp_runtime::StateVersion::V1);
 
+			let content_hash = sp_io::hashing::blake2_256(&data);
 			let extrinsic_index =
 				<frame_system::Pallet<T>>::extrinsic_index().ok_or(Error::<T>::BadContext)?;
-			let content_hash = sp_io::hashing::blake2_256(&data);
 			sp_io::transaction_index::index(extrinsic_index, data.len() as u32, content_hash);
 
 			let mut index = 0;
@@ -418,7 +431,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
@@ -436,7 +449,7 @@ pub mod pallet {
 
 			// Get the target block metadata.
 			let number = <frame_system::Pallet<T>>::block_number();
-			let period = T::StoragePeriod::get();
+			let period = Self::retention_period();
 			let target_number = number.saturating_sub(period);
 			ensure!(!target_number.is_zero(), Error::<T>::UnexpectedProof);
 			let transactions =
@@ -639,6 +652,14 @@ pub mod pallet {
 	/// Storage fee per transaction.
 	pub type EntryFee<T: Config> = StorageValue<_, BalanceOf<T>>;
 
+	/// 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 RetentionPeriod<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;
+
 	// Intermediates
 	#[pallet::storage]
 	pub(super) type BlockTransactions<T: Config> =
@@ -648,6 +669,32 @@ pub mod pallet {
 	#[pallet::storage]
 	pub(super) type ProofChecked<T: Config> = StorageValue<_, bool, ValueQuery>;
 
+	#[pallet::genesis_config]
+	pub struct GenesisConfig<T: Config> {
+		pub byte_fee: BalanceOf<T>,
+		pub entry_fee: BalanceOf<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(),
+				retention_period: DEFAULT_RETENTION_PERIOD.into(),
+			}
+		}
+	}
+
+	#[pallet::genesis_build]
+	impl<T: Config> BuildGenesisConfig for GenesisConfig<T> {
+		fn build(&self) {
+			ByteFee::<T>::put(self.byte_fee);
+			EntryFee::<T>::put(self.entry_fee);
+			RetentionPeriod::<T>::put(self.retention_period);
+		}
+	}
+
 	#[pallet::inherent]
 	impl<T: Config> ProvideInherent for Pallet<T> {
 		type Call = Call<T>;
@@ -835,6 +882,11 @@ pub mod pallet {
 			Self::check_signed(who, call, CheckContext::PreDispatch).map(|_| ())
 		}
 
+		/// Get RetentionPeriod storage information from the outside of this pallet.
+		pub fn retention_period() -> BlockNumberFor<T> {
+			RetentionPeriod::<T>::get()
+		}
+
 		/// Returns `true` if a blob of the given size can be stored.
 		fn data_size_ok(size: usize) -> bool {
 			(size > 0) && (size <= T::MaxTransactionSize::get() as usize)

pallets/transaction-storage/src/migrations.rs

@@ -0,0 +1,61 @@
+// 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.
+
+use crate::{Config, RetentionPeriod, LOG_TARGET};
+use core::marker::PhantomData;
+use polkadot_sdk_frame::{
+	prelude::{BlockNumberFor, Weight},
+	traits::{Get, OnRuntimeUpgrade, Zero},
+};
+
+/// Runtime migration that sets the `RetentionPeriod` storage item to a
+/// non-zero `NewValue` value **only if it is currently zero**.
+///
+/// Idempotent migration: safe to run multiple times
+pub struct SetRetentionPeriodIfZero<T, NewValue>(PhantomData<(T, NewValue)>);
+impl<T: Config, NewValue: Get<BlockNumberFor<T>>> OnRuntimeUpgrade
+	for SetRetentionPeriodIfZero<T, NewValue>
+{
+	fn on_runtime_upgrade() -> Weight {
+		let mut weight = T::DbWeight::get().reads(1);
+
+		// If zero, let's reset.
+		if RetentionPeriod::<T>::get().is_zero() {
+			RetentionPeriod::<T>::set(NewValue::get());
+			weight.saturating_accrue(T::DbWeight::get().writes(1));
+
+			tracing::warn!(
+				target: LOG_TARGET,
+				new_value = ?NewValue::get(),
+				"[SetRetentionPeriodIfZero] RetentionPeriod was zero, resetting to:",
+			);
+		}
+
+		weight
+	}
+
+	#[cfg(feature = "try-runtime")]
+	fn post_upgrade(
+		_state: alloc::vec::Vec<u8>,
+	) -> Result<(), polkadot_sdk_frame::deps::sp_runtime::DispatchError> {
+		polkadot_sdk_frame::prelude::ensure!(
+			!RetentionPeriod::<T>::get().is_zero(),
+			"must be migrate to the `NewValue`."
+		);
+
+		tracing::info!(target: LOG_TARGET, "SetRetentionPeriodIfZero is OK!");
+		Ok(())
+	}
+}