-
Notifications
You must be signed in to change notification settings - Fork 281
Description
An old issue discussing this is #1669, but I want to raise a new issue to note that it is still a problem.
The storage Address trait looks like:
pub trait Address {
/// The target type of the value that lives at this address.
type Target: DecodeWithMetadata;
/// The keys type used to construct this address.
type Keys: StorageKey;
/// Can an entry be fetched from this address?
/// Set this type to [`Yes`] to enable the corresponding calls to be made.
type IsFetchable;
/// Can a default entry be obtained from this address?
/// Set this type to [`Yes`] to enable the corresponding calls to be made.
type IsDefaultable;
/// Can this address be iterated over?
/// Set this type to [`Yes`] to enable the corresponding calls to be made.
type IsIterable;
/// The name of the pallet that the entry lives under.
fn pallet_name(&self) -> &str;
/// The name of the entry in a given pallet that the item is at.
fn entry_name(&self) -> &str;
/// Output the non-prefix bytes; that is, any additional bytes that need
/// to be appended to the key to dig into maps.
fn append_entry_bytes(&self, metadata: &Metadata, bytes: &mut Vec<u8>) -> Result<(), Error>;
/// An optional hash which, if present, will be checked against
/// the node metadata to confirm that the return type matches what
/// we are expecting.
fn validation_hash(&self) -> Option<[u8; 32]> {
None
}
}The Keys type here is used to denote the type of the keys that we store in a concrete Address impl. These keys will be encoded when we append_entry_bytes ie here:
impl<Keys, ReturnTy, Fetchable, Defaultable, Iterable> Address
for DefaultAddress<Keys, ReturnTy, Fetchable, Defaultable, Iterable>
where
Keys: StorageKey,
ReturnTy: DecodeWithMetadata,
{
type Target = ReturnTy;
type Keys = Keys;
type IsFetchable = Fetchable;
type IsDefaultable = Defaultable;
type IsIterable = Iterable;
fn pallet_name(&self) -> &str {
&self.pallet_name
}
fn entry_name(&self) -> &str {
&self.entry_name
}
fn append_entry_bytes(&self, metadata: &Metadata, bytes: &mut Vec<u8>) -> Result<(), Error> {
let pallet = metadata.pallet_by_name_err(self.pallet_name())?;
let storage = pallet
.storage()
.ok_or_else(|| MetadataError::StorageNotFoundInPallet(self.pallet_name().to_owned()))?;
let entry = storage
.entry_by_name(self.entry_name())
.ok_or_else(|| MetadataError::StorageEntryNotFound(self.entry_name().to_owned()))?;
let hashers = StorageHashers::new(entry.entry_type(), metadata.types())?;
self.keys
.encode_storage_key(bytes, &mut hashers.iter(), metadata.types())?; // <-- here we encode keys which are of type `Keys`.
Ok(())
}
fn validation_hash(&self) -> Option<[u8; 32]> {
self.validation_hash
}
}The problem is that we also rely on this same Keys type for decoding keys, so while _iter() methods set keys to () because there are no keys provided to the struct (no keys need encoding), we still get back all of the keys for decoding.
I think that we should break Keys into EncodeKeys and DecodeKeys. This way, we no longer conflate the two things, and can accept different sets of keys from that which we will ultimately be decoding.
We should perhaps also take this opportunity to more thoroughly rework the storage APIs:
- Let's rely on the
StorageKeystrait (being renamed to justEncodableValuesin an upcoming PR because it's being used for Runtime APIs and View Functions too) instead of subxt'sStorageKeytrait. - Can we make the APis a little more simialr to
subxt-historicie in this example. We want static typing insteadof dynamic, but perhaps static typing could basically assert that types are iterable or whatever but otherwise use similar sort of interface. Needs exploring a bit.
It might be that we do step 1 sooner and step 2 later, depending on the effort of a more comprehensive rework.