Add docs and use BoundedVec

pallets/relay-storage-roots/src/lib.rs

@@ -29,6 +29,24 @@
 //! This pallet SHOULD NOT be used for data that can change quickly, because we allow the user to
 //! submit a proof of an old state. Therefore a valid proof does not imply that the current relay
 //! state is the expected one.
+//!
+//! ### Design
+//!
+//! The relay storage roots are inserted in the `on_finalize` hook, so the storage root of the
+//! current relay block will not be available in the mapping until the next block, but it can be
+//! read using the `RelaychainStateProvider` at any point after the `on_initialize` of
+//! `cumulus_pallet_parachain_system`.
+//!
+//! One storage root is inserted per parachain block, but there may be more than one relay block in
+//! between two parachain blocks. In that case, there will be a gap in the `RelayStorageRoot`
+//! mapping. When creating a proof, users should ensure that they are using the latest storage root
+//! available in the mapping, otherwise it may not be possible to validate their proof.
+//!
+//! The `RelayStorageRoot` mapping is bounded by `MaxStorageRoots`. To ensure that oldest storage
+//! roots are removed first, there is an additional `RelayStorageRootKeys` storage item that stores
+//! a sorted list of all the keys. This is needed because it is not possible to iterate over a
+//! mapping in order (unless if using an `Identity` hash). The `MaxStorageRoots` limit applies to
+//! the number of items, not to their age.
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
@@ -57,7 +75,6 @@ pub mod pallet {
 	use super::*;
 
 	#[pallet::pallet]
-	#[pallet::without_storage_info]
 	pub struct Pallet<T>(PhantomData<T>);
 
 	/// Configuration trait of this pallet.
@@ -79,7 +96,7 @@ pub mod pallet {
 	/// Used to remove the oldest key without having to iterate over all of them.
 	#[pallet::storage]
 	pub type RelayStorageRootKeys<T: Config> =
-		StorageValue<_, VecDeque<RelayBlockNumber>, ValueQuery>;
+		StorageValue<_, BoundedVec<RelayBlockNumber, T::MaxStorageRoots>, ValueQuery>;
 
 	impl<T: Config> Pallet<T> {
 		/// Populates `RelayStorageRoot` using `RelaychainStateProvider`.
@@ -92,14 +109,18 @@ pub mod pallet {
 			}
 
 			<RelayStorageRoot<T>>::insert(relay_state.number, relay_state.state_root);
-			let mut keys = <RelayStorageRootKeys<T>>::get();
+			let mut keys: VecDeque<_> = <RelayStorageRootKeys<T>>::get().into_inner().into();
 			keys.push_back(relay_state.number);
 			// Delete the oldest stored root if the total number is greater than MaxStorageRoots
 			if u32::try_from(keys.len()).unwrap() > T::MaxStorageRoots::get() {
 				let first_key = keys.pop_front().unwrap();
 				<RelayStorageRoot<T>>::remove(first_key);
 			}
 
+			// If `MaxStorageRoots` has decreased, we need to delete more than one storage root.
+			// But that would make this function have an unbounded amount of writes. So instead, we
+			// will only delete the first one, and leak the rest.
+			let keys = BoundedVec::truncate_from(keys.into());
 			<RelayStorageRootKeys<T>>::put(keys);
 		}
 	}