Skip to content

Generated storage _iter() methods don't allow proper decoding of keys. #2091

@jsdw

Description

@jsdw

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:

  1. Let's rely on the StorageKeys trait (being renamed to just EncodableValues in an upcoming PR because it's being used for Runtime APIs and View Functions too) instead of subxt's StorageKey trait.
  2. Can we make the APis a little more simialr to subxt-historic ie 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions