Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle StorageEntry empty keys #565

Merged
merged 2 commits into from
Jun 16, 2022
Merged

Handle StorageEntry empty keys #565

merged 2 commits into from
Jun 16, 2022

Conversation

lexnv
Copy link
Collaborator

@lexnv lexnv commented Jun 14, 2022

There are cases when the StorageEntry has an empty key.

For example, defining a custom pallet

#[pallet::config]
pub trait Config: frame_system::Config {
   type AssetNativeLocation: Parameter + Member + Default;
}

#[pallet::storage]
#[pallet::getter(fn location_assets)]
/// Local asset for native location.
pub type LocationAssets<T: Config> =
    StorageMap<_, Blake2_128Concat, T::AssetNativeLocation, bool, OptionQuery>;

and implementing it

impl pallet_asset_registry::Config for Runtime {
    type AssetNativeLocation = ();

results in subxt generating an extra lifetime parameter for a parameter that does not exist.
For more details see #552.

This PR ensures that no lifetimes are added when dealing with empty keys.

Testing Done

Added the following patch to the substrate-node-template.

+++ b/pallets/template/src/lib.rs
@@ -24,6 +24,9 @@ pub mod pallet {
        pub trait Config: frame_system::Config {
                /// Because this pallet emits events, it depends on the runtime's definition of an event.
                type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
+
+               /// AssetNativeLocation for () storage.
+               type AssetNativeLocation: Parameter + Member + Default + MaxEncodedLen;
        }

        #[pallet::pallet]
@@ -38,6 +41,12 @@ pub mod pallet {
        // https://docs.substrate.io/v3/runtime/storage#declaring-storage-items
        pub type Something<T> = StorageValue<_, u32>;

+       #[pallet::storage]
+       #[pallet::getter(fn location_assets)]
+       /// Local asset for native location.
+       pub type LocationAssets<T: Config> =
+               StorageMap<_, Blake2_128Concat, T::AssetNativeLocation, bool, OptionQuery>;
+
        // Pallets use events to inform users when important changes are made.
        // https://docs.substrate.io/v3/runtime/events-and-errors
        #[pallet::event]
diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs
index 8a00054..4781e95 100644
--- a/runtime/src/lib.rs
+++ b/runtime/src/lib.rs
@@ -266,6 +266,7 @@ impl pallet_sudo::Config for Runtime {
 /// Configure the pallet-template in pallets/template.
 impl pallet_template::Config for Runtime {
        type Event = Event;
+       type AssetNativeLocation = ();

Before

Generated code does not compile

pub struct LocationAssets<'a>();

impl ::subxt::StorageEntry for LocationAssets<'_> {
    const PALLET: &'static str = "TemplateModule";
    const STORAGE: &'static str = "LocationAssets";
    type Value = ::core::primitive::bool;
    fn key(&self) -> ::subxt::StorageEntryKey {
        ::subxt::StorageEntryKey::Map(vec![::subxt::StorageMapKey::new(
            &(),
            ::subxt::StorageHasher::Blake2_128Concat,
        )])
    }
}

// Constructed as:
let entry = LocationAssets();

After

There is no extra lifetime parameter added.
Also, the constructor is modified to reflect the changes.

pub struct LocationAssets;
impl ::subxt::StorageEntry for LocationAssets {
    const PALLET: &'static str = "TemplateModule";
    const STORAGE: &'static str = "LocationAssets";
    type Value = ::core::primitive::bool;
    fn key(&self) -> ::subxt::StorageEntryKey {
        ::subxt::StorageEntryKey::Map(vec![::subxt::StorageMapKey::new(
            &(),
            ::subxt::StorageHasher::Blake2_128Concat,
        )])
    }
}

// Constructed as:
let entry = LocationAssets;

Closes #552.

lexnv added 2 commits June 14, 2022 21:35
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv self-assigned this Jun 15, 2022
@jsdw
Copy link
Collaborator

jsdw commented Jun 15, 2022

Looks good to me! The more I look at that code for generating storage entries, the more I think we need tohave a good pass over it to tidy it all up; it's all a bit of a mess nowadays as it keeps growing in complexity.

I'm working on a UI test for this too, but I think this code is OK to merge in isolation.

@jsdw
Copy link
Collaborator

jsdw commented Jun 15, 2022

Added a UI test in #567, so if that passes with this fix then this is good to go I think :)

@lexnv
Copy link
Collaborator Author

lexnv commented Jun 15, 2022

@jsdw Yep, the ui tests seem to pass with this PR.

The doc-tests seem to be failing currently, I ll have a look over those 😄

@jsdw jsdw merged commit 7621d71 into master Jun 16, 2022
@jsdw jsdw deleted the 552_storage_entry_empty_keys branch June 16, 2022 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Wrong code generated for StorageEntry when the Key is ()
3 participants