From e0f4630954e6e9a6b52966964f6369a9f96a4319 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Tue, 9 Dec 2025 14:15:26 +0000 Subject: [PATCH] Simplify storage APIs --- new/src/storage.rs | 31 ++-- new/src/storage/prefix_of.rs | 76 ---------- new/src/storage/storage_entry.rs | 242 +++++++++++-------------------- 3 files changed, 98 insertions(+), 251 deletions(-) diff --git a/new/src/storage.rs b/new/src/storage.rs index 918fb0fb13..d0cfc6681e 100644 --- a/new/src/storage.rs +++ b/new/src/storage.rs @@ -74,17 +74,17 @@ impl> StorageClient { pub fn entry( &self, address: Addr, - ) -> Result, StorageError> { + ) -> Result, StorageError> { self.validate(&address)?; StorageEntry::new(&self.client, address) } /// Iterate over all of the storage entries listed in the metadata for the current block. This does **not** include well known /// storage entries like `:code` which are not listed in the metadata. - pub fn entries(&self) -> impl Iterator> { + pub fn entries(&self) -> impl Iterator> { let metadata = self.client.metadata_ref(); Entry::tuples_of(metadata.storage_entries()).map(|(pallet_name, entry_name)| { - StorageEntries { + StorageEntryRef { pallet_name: pallet_name.clone(), entry_name, client: &self.client, @@ -102,7 +102,7 @@ impl> StorageClient { key_parts: Addr::KeyParts, ) -> Result, StorageError> { let entry = self.entry(addr)?; - entry.internal_fetch(key_parts).await + entry.fetch(key_parts).await } /// This is essentially a shorthand for `client.entry(addr)?.try_fetch(key_parts)`. See [`StorageEntry::try_fetch()`]. @@ -112,7 +112,7 @@ impl> StorageClient { key_parts: Addr::KeyParts, ) -> Result>, StorageError> { let entry = self.entry(addr)?; - entry.internal_try_fetch(key_parts).await + entry.try_fetch(key_parts).await } /// This is essentially a shorthand for `client.entry(addr)?.iter(key_parts)`. See [`StorageEntry::iter()`]. @@ -126,7 +126,7 @@ impl> StorageClient { StorageError, > { let entry = self.entry(addr)?; - entry.internal_iter(key_parts).await + entry.iter(key_parts).await } /// In rare cases, you may wish to fetch a storage value that does not live at a typical address. This method @@ -152,11 +152,11 @@ impl> StorageClient { pub async fn storage_version(&self, pallet_name: impl AsRef) -> Result { // construct the storage key. This is done similarly in // `frame_support::traits::metadata::StorageVersion::storage_key()`: - let mut key_bytes: Vec = vec![]; - key_bytes.extend(&sp_crypto_hashing::twox_128( - pallet_name.as_ref().as_bytes(), - )); - key_bytes.extend(&sp_crypto_hashing::twox_128(b":__STORAGE_VERSION__:")); + let key_bytes = frame_decode::storage::encode_storage_key_prefix( + pallet_name.as_ref(), + ":__STORAGE_VERSION__:", + ) + .to_vec(); // fetch the raw bytes and decode them into the StorageVersion struct: let storage_version_bytes = self.fetch_raw(key_bytes).await?; @@ -173,14 +173,14 @@ impl> StorageClient { } /// Working with a specific storage entry. -pub struct StorageEntries<'atblock, Client, T> { +pub struct StorageEntryRef<'atblock, Client, T> { pallet_name: Cow<'atblock, str>, entry_name: Cow<'atblock, str>, client: &'atblock Client, marker: std::marker::PhantomData, } -impl<'atblock, Client, T> StorageEntries<'atblock, Client, T> +impl<'atblock, Client, T> StorageEntryRef<'atblock, Client, T> where T: Config, Client: OfflineClientAtBlockT, @@ -198,10 +198,7 @@ where /// Extract the relevant storage information so that we can work with this entry. pub fn entry( &self, - ) -> Result< - StorageEntry<'_, T, Client, address::DynamicAddress, crate::utils::Maybe>, - StorageError, - > { + ) -> Result, StorageError> { let addr = address::dynamic(self.pallet_name.to_owned(), self.entry_name.to_owned()); StorageEntry::new(self.client, addr) } diff --git a/new/src/storage/prefix_of.rs b/new/src/storage/prefix_of.rs index 4f9244fadf..e6f30abecf 100644 --- a/new/src/storage/prefix_of.rs +++ b/new/src/storage/prefix_of.rs @@ -76,46 +76,6 @@ array_impl!(4: 3 2 1 0); array_impl!(5: 4 3 2 1 0); array_impl!(6: 5 4 3 2 1 0); -/// This is much like [`PrefixOf`] except that it also includes `Self` as an allowed type, -/// where `Self` must impl [`IntoEncodableValues`] just as every [`PrefixOf`] does. -pub trait EqualOrPrefixOf: IntoEncodableValues {} - -// Tuples -macro_rules! tuple_impl_eq { - ($($t:ident)+) => { - // Any T that is a PrefixOf impls EqualOrPrefixOf too - impl <$($t,)+ T: PrefixOf<($($t,)+)>> EqualOrPrefixOf<($($t,)+)> for T {} - // Keys impls EqualOrPrefixOf - impl <$($t),+> EqualOrPrefixOf<($($t,)+)> for ($($t,)+) where ($($t,)+): IntoEncodableValues {} - // &'a Keys impls EqualOrPrefixOf - impl <'a, $($t),+> EqualOrPrefixOf<($($t,)+)> for &'a ($($t,)+) where ($($t,)+): IntoEncodableValues {} - } -} - -tuple_impl_eq!(A); -tuple_impl_eq!(A B); -tuple_impl_eq!(A B C); -tuple_impl_eq!(A B C D); -tuple_impl_eq!(A B C D E); -tuple_impl_eq!(A B C D E F); - -// Vec -impl EqualOrPrefixOf> for Vec {} -impl EqualOrPrefixOf> for &Vec {} - -// Arrays -macro_rules! array_impl_eq { - ($($n:literal)+) => { - $( - impl EqualOrPrefixOf<[A; $n]> for [A; $n] {} - impl <'a, A: EncodeAsType> EqualOrPrefixOf<[A; $n]> for &'a [A; $n] {} - )+ - } -} - -impl EqualOrPrefixOf<[A; N]> for T where T: PrefixOf<[A; N]> {} -array_impl_eq!(1 2 3 4 5 6); - #[cfg(test)] mod test { use super::*; @@ -129,9 +89,6 @@ mod test { fn accepts_prefix_of>(&self, keys: P) { let _encoder = keys.into_encodable_values(); } - fn accepts_eq_or_prefix_of>(&self, keys: P) { - let _encoder = keys.into_encodable_values(); - } } #[test] @@ -158,37 +115,4 @@ mod test { t.accepts_prefix_of([0]); t.accepts_prefix_of([]); } - - #[test] - fn test_eq_or_prefix_of() { - // In real life we'd have a struct a bit like this: - let t = Test::<(bool, String, u64)>::new(); - - // And we'd want to be able to call some method like this: - t.accepts_eq_or_prefix_of(&(true, String::from("hi"), 0)); - t.accepts_eq_or_prefix_of(&(true, String::from("hi"))); - t.accepts_eq_or_prefix_of((true,)); - t.accepts_eq_or_prefix_of(()); - - t.accepts_eq_or_prefix_of((true, String::from("hi"), 0)); - t.accepts_eq_or_prefix_of((true, String::from("hi"))); - t.accepts_eq_or_prefix_of((true,)); - t.accepts_eq_or_prefix_of(()); - - let t = Test::<[u64; 5]>::new(); - - t.accepts_eq_or_prefix_of([0, 1, 2, 3, 4]); - t.accepts_eq_or_prefix_of([0, 1, 2, 3]); - t.accepts_eq_or_prefix_of([0, 1, 2]); - t.accepts_eq_or_prefix_of([0, 1]); - t.accepts_eq_or_prefix_of([0]); - t.accepts_eq_or_prefix_of([]); - - t.accepts_eq_or_prefix_of([0, 1, 2, 3, 4]); - t.accepts_eq_or_prefix_of([0, 1, 2, 3]); - t.accepts_eq_or_prefix_of([0, 1, 2]); - t.accepts_eq_or_prefix_of([0, 1]); - t.accepts_eq_or_prefix_of([0]); - t.accepts_eq_or_prefix_of([]); - } } diff --git a/new/src/storage/storage_entry.rs b/new/src/storage/storage_entry.rs index dd3fa97c31..dbf7cd0f73 100644 --- a/new/src/storage/storage_entry.rs +++ b/new/src/storage/storage_entry.rs @@ -4,7 +4,7 @@ use crate::config::Config; use crate::error::StorageError; use crate::storage::address::Address; use crate::storage::{PrefixOf, StorageKeyValue, StorageValue}; -use crate::utils::{Maybe, Yes, YesMaybe}; +use crate::utils::YesMaybe; use core::marker::PhantomData; use frame_decode::storage::{IntoEncodableValues, StorageInfo, StorageTypeInfo}; use futures::StreamExt; @@ -13,14 +13,12 @@ use std::sync::Arc; /// This represents a single storage entry (be it a plain value or map) /// and the operations that can be performed on it. #[derive(Debug)] -pub struct StorageEntry<'atblock, T: Config, Client, Addr, IsPlain> { +pub struct StorageEntry<'atblock, T: Config, Client, Addr> { inner: Arc>, - marker: PhantomData<(T, IsPlain)>, + marker: PhantomData, } -impl<'atblock, T: Config, Client, Addr, IsPlain> Clone - for StorageEntry<'atblock, T, Client, Addr, IsPlain> -{ +impl<'atblock, T: Config, Client, Addr> Clone for StorageEntry<'atblock, T, Client, Addr> { fn clone(&self) -> Self { Self { inner: self.inner.clone(), @@ -36,7 +34,7 @@ struct StorageEntryInner<'atblock, Addr, Client> { client: &'atblock Client, } -impl<'atblock, T, Client, Addr, IsPlain> StorageEntry<'atblock, T, Client, Addr, IsPlain> +impl<'atblock, T, Client, Addr> StorageEntry<'atblock, T, Client, Addr> where T: Config, Addr: Address, @@ -94,14 +92,50 @@ where }) } - /// Create the bytes for a storage key given the key parts. - /// - /// **Warning:** This provides no safety around the provided keys in order that it can be used - /// behind the scenes in several places. - pub(crate) fn internal_key_bytes( + /// The keys for plain storage values are always 32 byte hashes. + pub fn key_prefix(&self) -> [u8; 32] { + frame_decode::storage::encode_storage_key_prefix(self.pallet_name(), self.entry_name()) + } + + /// This returns a full key to a single value in this storage entry. + pub fn fetch_key(&self, key_parts: Addr::KeyParts) -> Result, StorageError> { + self.key_from_any_parts(key_parts) + } + + /// This returns a valid key suitable for iterating over the values in this storage entry. + pub fn iter_key>( &self, - key_parts: Keys, + key_parts: KeyParts, ) -> Result, StorageError> { + let num_keys = self.inner.info.keys.len(); + if Addr::IsPlain::is_yes() { + Err(StorageError::CannotIterPlainEntry { + pallet_name: self.pallet_name().into(), + entry_name: self.entry_name().into(), + }) + } else if key_parts.num_encodable_values() >= num_keys { + Err(StorageError::WrongNumberOfKeyPartsProvidedForIterating { + max_expected: num_keys - 1, + got: key_parts.num_encodable_values(), + }) + } else { + self.key_from_any_parts(key_parts) + } + } + + // This has a more lax type signature than `.key` and so can be used in a couple of places internally. + fn key_from_any_parts( + &self, + key_parts: impl IntoEncodableValues, + ) -> Result, StorageError> { + let num_keys = self.inner.info.keys.len(); + if key_parts.num_encodable_values() != num_keys { + return Err(StorageError::WrongNumberOfKeyPartsProvidedForFetching { + expected: num_keys, + got: key_parts.num_encodable_values(), + }); + } + let key = frame_decode::storage::encode_storage_key_with_info( self.pallet_name(), self.entry_name(), @@ -115,23 +149,28 @@ where } } -impl<'atblock, T, Client, Addr, IsPlain> StorageEntry<'atblock, T, Client, Addr, IsPlain> +impl<'atblock, T, Client, Addr> StorageEntry<'atblock, T, Client, Addr> where T: Config, Addr: Address, Client: OnlineClientAtBlockT, { - /// Fetch a value, using the default value if none can be found, or returning an error - /// if no value exists at this location and there is no default value. + /// Fetch a storage value within this storage entry. /// - /// **Warning:** This provides no safety around the provided keys in order that it can be used - /// behind the scenes in several places. - pub(crate) async fn internal_fetch( + /// If the entry is a map, you'll need to provide the relevant values for each part of the storage + /// key. If the entry is a plain value, you must provide an empty list of key parts, ie `()`. + /// + /// The type of these key parts is determined by the [`Address`] of this storage entry. If the address + /// is generated via the `#[subxt]` macro then it will ensure you provide a valid type. + /// + /// If no value is found, the default value will be returned for this entry if one exists. If no value is + /// found and no default value exists, an error will be returned. + pub async fn fetch( &self, - key_parts: impl IntoEncodableValues, + key_parts: Addr::KeyParts, ) -> Result, StorageError> { let value = self - .internal_try_fetch(key_parts) + .try_fetch(key_parts) .await? .or_else(|| self.default_value()) .ok_or(StorageError::NoValueFound)?; @@ -139,15 +178,20 @@ where Ok(value) } - /// Fetch a value, returning `None` if no value exists at that location. + /// Fetch a storage value within this storage entry. /// - /// **Warning:** This provides no safety around the provided keys in order that it can be used - /// behind the scenes in several places. - pub(crate) async fn internal_try_fetch( + /// If the entry is a map, you'll need to provide the relevant values for each part of the storage + /// key. If the entry is a plain value, you must provide an empty list of key parts, ie `()`. + /// + /// The type of these key parts is determined by the [`Address`] of this storage entry. If the address + /// is generated via the `#[subxt]` macro then it will ensure you provide a valid type. + /// + /// If no value is found, `None` will be returned. + pub async fn try_fetch( &self, - key_parts: impl IntoEncodableValues, + key_parts: Addr::KeyParts, ) -> Result>, StorageError> { - let key = self.internal_key_bytes(key_parts)?; + let key = self.fetch_key(key_parts)?; let block_hash = self.inner.client.block_hash(); let value = self @@ -169,21 +213,28 @@ where Ok(value) } - /// Iterate over the values under the provided key. + /// Iterate over storage values within this storage entry. /// - /// **Warning:** This provides no safety around the provided keys in order that it can be used - /// behind the scenes in several places. - pub(crate) async fn internal_iter>( + /// You'll need to provide a prefix of the key parts required to point to a single value in the map. + /// Normally you will provide `()` to iterate over _everything_, `(first_key,)` to iterate over everything underneath + /// `first_key` in the map, `(first_key, second_key)` to iterate over everything underneath `first_key` + /// and `second_key` in the map, and so on, up to the actual depth of the map - 1. + /// + /// The possible types of these key parts is determined by the [`Address`] of this storage entry. + /// If the address is generated via the `#[subxt]` macro then it will ensure you provide a valid type. + /// + /// For plain values, there is no valid type, since they cannot be iterated over. + pub async fn iter>( &self, key_parts: KeyParts, ) -> Result< impl futures::Stream, StorageError>> - + use<'atblock, Addr, Client, T, KeyParts, IsPlain>, + + use<'atblock, Addr, Client, T, KeyParts>, StorageError, > { let info = self.inner.info.clone(); let types = self.inner.client.metadata_ref().types(); - let key_bytes = self.internal_key_bytes(key_parts)?; + let key_bytes = self.key_from_any_parts(key_parts)?; let block_hash = self.inner.client.block_hash(); let stream = self @@ -209,128 +260,3 @@ where Ok(Box::pin(stream)) } } - -// Plain values get a fetch method with no extra arguments. -impl<'atblock, T, Client, Addr> StorageEntry<'atblock, T, Client, Addr, Yes> -where - T: Config, - Addr: Address, - Client: OnlineClientAtBlockT, -{ - /// Fetch the storage value at this location. If no value is found, the default value will be returned - /// for this entry if one exists. If no value is found and no default value exists, an error will be returned. - pub async fn fetch(&self) -> Result, StorageError> { - self.internal_fetch(()).await - } - - /// Fetch the storage value at this location. If no value is found, `None` will be returned. - pub async fn try_fetch( - &self, - ) -> Result>, StorageError> { - self.internal_try_fetch(()).await - } - - /// This is identical to [`StorageEntry::key_prefix()`] and is the full - /// key for this storage entry. - pub fn key(&self) -> [u8; 32] { - self.key_prefix() - } - - /// The keys for plain storage values are always 32 byte hashes. - pub fn key_prefix(&self) -> [u8; 32] { - frame_decode::storage::encode_storage_key_prefix(self.pallet_name(), self.entry_name()) - } -} - -// When HasDefaultValue = Yes, we expect there to exist a valid default value and will use that -// if we fetch an entry and get nothing back. -impl<'atblock, T, Client, Addr> StorageEntry<'atblock, T, Client, Addr, Maybe> -where - T: Config, - Addr: Address, - Client: OnlineClientAtBlockT, -{ - /// Fetch a storage value within this storage entry. - /// - /// This entry may be a map, and so you must provide the relevant values for each part of the storage - /// key that is required in order to point to a single value. - /// - /// If no value is found, the default value will be returned for this entry if one exists. If no value is - /// found and no default value exists, an error will be returned. - pub async fn fetch( - &self, - key_parts: Addr::KeyParts, - ) -> Result, StorageError> { - self.internal_fetch(key_parts).await - } - - /// Fetch a storage value within this storage entry. - /// - /// This entry may be a map, and so you must provide the relevant values for each part of the storage - /// key that is required in order to point to a single value. - /// - /// If no value is found, `None` will be returned. - pub async fn try_fetch( - &self, - key_parts: Addr::KeyParts, - ) -> Result>, StorageError> { - self.internal_try_fetch(key_parts).await - } - - /// Iterate over storage values within this storage entry. - /// - /// You may provide any prefix of the values needed to point to a single value. Normally you will - /// provide `()` to iterate over _everything_, or `(first_key,)` to iterate over everything underneath - /// `first_key` in the map, or `(first_key, second_key)` to iterate over everything underneath `first_key` - /// and `second_key` in the map, and so on, up to the actual depth of the map - 1. - pub async fn iter>( - &self, - key_parts: KeyParts, - ) -> Result< - impl futures::Stream, StorageError>> - + use<'atblock, Addr, Client, T, KeyParts>, - StorageError, - > { - self.internal_iter(key_parts).await - } - - /// This returns a full key to a single value in this storage entry. - pub fn key(&self, key_parts: Addr::KeyParts) -> Result, StorageError> { - let num_keys = self.inner.info.keys.len(); - if key_parts.num_encodable_values() != num_keys { - Err(StorageError::WrongNumberOfKeyPartsProvidedForFetching { - expected: num_keys, - got: key_parts.num_encodable_values(), - }) - } else { - self.internal_key_bytes(key_parts) - } - } - - /// This returns valid keys to iterate over the storage entry at the available levels. - pub fn iter_key>( - &self, - key_parts: KeyParts, - ) -> Result, StorageError> { - let num_keys = self.inner.info.keys.len(); - if Addr::IsPlain::is_yes() { - Err(StorageError::CannotIterPlainEntry { - pallet_name: self.pallet_name().into(), - entry_name: self.entry_name().into(), - }) - } else if key_parts.num_encodable_values() >= num_keys { - Err(StorageError::WrongNumberOfKeyPartsProvidedForIterating { - max_expected: num_keys - 1, - got: key_parts.num_encodable_values(), - }) - } else { - self.internal_key_bytes(key_parts) - } - } - - /// The first 32 bytes of the storage entry key, which points to the entry but not necessarily - /// a single storage value (unless the entry is a plain value). - pub fn key_prefix(&self) -> [u8; 32] { - frame_decode::storage::encode_storage_key_prefix(self.pallet_name(), self.entry_name()) - } -}