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

[BUG] Wrong code generated for StorageEntry when the Key is () #552

Closed
shekohex opened this issue May 27, 2022 · 1 comment · Fixed by #565
Closed

[BUG] Wrong code generated for StorageEntry when the Key is () #552

shekohex opened this issue May 27, 2022 · 1 comment · Fixed by #565
Milestone

Comments

@shekohex
Copy link

Hello There, I guess I found a bug in the codegen for when the parameters of the StorageEntry is empty.

Error Message

error[E0392]: parameter `'a` is never used
     --> src/substrate/protocol_substrate_runtime.rs:29742:39
      |
29742 |             pub struct LocationAssets<'a>();
      |                                       ^^ unused parameter
      |
      = help: consider removing `'a`, referring to it in a field, or using a marker such as `PhantomData`

Bug Overview

We have a pallet called AssetRegistry that looks like the following (simplified):

// ...snip...
#[pallet::config]
pub trait Config: frame_system::Config {
    type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
    /// Asset type
    type AssetId: Parameter
        + Member
        + Default
        + Copy
        + BaseArithmetic
        + MaybeSerializeDeserialize
        + MaxEncodedLen;

    /// Asset location type
    type AssetNativeLocation: Parameter + Member + Default;

    /// Native Asset Id
    #[pallet::constant]
    type NativeAssetId: Get<Self::AssetId>;
}

// ...snip...

#[pallet::storage]
#[pallet::getter(fn locations)]
/// Native location of an asset.
pub type AssetLocations<T: Config> =
    StorageMap<_, Twox64Concat, T::AssetId, T::AssetNativeLocation, OptionQuery>;

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

// ...snip...

and in our runtime, we are using it like this (simplified):

impl pallet_asset_registry::Config for Runtime {
    type AssetId = u32;
    type AssetNativeLocation = (); // **NOTICE THIS HERE**
    type Event = Event;
    type NativeAssetId = GetNativeCurrencyId;
}

The Bug

When we generate the code, from the metadata we noticed the following code:

  • For AssetLocations:
pub struct AssetLocations<'a>(pub &'a ::core::primitive::u32);
impl ::subxt::StorageEntry for AssetLocations<'_> {
    const PALLET: &'static str = "AssetRegistry";
    const STORAGE: &'static str = "AssetLocations";
    type Value = ();
    fn key(&self) -> ::subxt::StorageEntryKey {
        ::subxt::StorageEntryKey::Map(vec![::subxt::StorageMapKey::new(
            &self.0,
            ::subxt::StorageHasher::Twox64Concat,
        )])
    }
}

Which looks correct (semantically and syntactically)

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

Which is also looking correct semantically but not syntactically

Ideas

So, this value is considered a tuple, but an empty one and I guess the code hits this path here:

pub struct #entry_struct_ident <'a>( #( pub &'a #field_types ),* );

but it does not take into account if the field_types is empty, we should check for that, and if so we should use core::marker::PhantomData for example.

Metadata

rustc version

rustc 1.60.0-nightly (498eeb72f 2022-01-31)
binary: rustc
commit-hash: 498eeb72f590e518e19746b346be53713689e207
commit-date: 2022-01-31
host: x86_64-unknown-linux-gnu
release: 1.60.0-nightly
LLVM version: 13.0.0

Substrate Runtime version/branch

version: 6.0.0
branch: polkadot-v0.9.22

subxt version

v0.21.0
@jsdw
Copy link
Collaborator

jsdw commented Jun 7, 2022

Thanks for reporting this and for the detailed writeup! Apologies; I've been on holiday and then abroad, but I'll have a look at this when I next get a chance!

@jsdw jsdw added this to the Release 0.22 milestone Jun 14, 2022
@jsdw jsdw closed this as completed in #565 Jun 16, 2022
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 a pull request may close this issue.

2 participants