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

Issue querying storage with tuple key #446

Closed
wzli opened this issue Feb 14, 2022 · 6 comments · Fixed by #458
Closed

Issue querying storage with tuple key #446

wzli opened this issue Feb 14, 2022 · 6 comments · Fixed by #458
Assignees

Comments

@wzli
Copy link

wzli commented Feb 14, 2022

I'm on subxt release 0.17, generated metadata on the same version, my runtime is based on node-template 4.0.0
The storage is defined as:

#[pallet::storage]
#[pallet::getter(fn my_storage_map)]
pub(super) type MyStorageMap<T: Config> =
        StorageMap<_, Twox64Concat, (T::AccountId, T::Index), MyStruct<T>, OptionQuery>;

In the substrate unit tests, inserting and retrieving from the same tuple key there is no problems, but in subxt the same query returns None, even though the insert is successful when I check by iterating all storage entries.

let entry = api
        .storage()
        .my_pallet()
        .my_storage_map(acc_id, idx, None)
        .await?;
println!("{entry:?}"); // -> None

Anything else I can provide to help debug?

@jsdw
Copy link
Collaborator

jsdw commented Feb 15, 2022

Hey, thanks for raising an issue!

Would it be possible for you to provide some steps for us to be able to reproduce this issue ourselves?

@wzli
Copy link
Author

wzli commented Feb 15, 2022

Okay so here is simple substrate pallet with a storage map that takes a tuple for key. I also wrote unit tests that ensure insert and retrieval works as expected.

wzli/substrate-node-template@aa9e5f6

on the subxt side, using v0.17, here's the code

use sp_keyring::AccountKeyring;
use subxt::{ClientBuilder, DefaultConfig, DefaultExtra, PairSigner};

#[subxt::subxt(runtime_metadata_path = "metadata.scale")]
pub mod runtime {}

#[async_std::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    env_logger::init();

    let signer = PairSigner::<DefaultConfig, DefaultExtra<DefaultConfig>, _>::new(
        AccountKeyring::Alice.pair(),
    );
    let api = ClientBuilder::new()
        .set_url("ws://localhost:9944")
        .build()
        .await?
        .to_runtime_api::<runtime::RuntimeApi<DefaultConfig, DefaultExtra<DefaultConfig>>>();

    let test_val = 500;
    let tx_result = api
        .tx()
        .template_module()
        .insert(test_val)
        .sign_and_submit_then_watch(&signer)
        .await?
        .wait_for_finalized_success()
        .await;

    let inserted_event = tx_result?
        .find_first_event::<runtime::template_module::events::SomethingInserted>()?
        .unwrap();
    let runtime::template_module::events::SomethingInserted((acc_id, nonce), _) = inserted_event;
    let val = api
        .storage()
        .template_module()
        .tuple_map(acc_id, nonce, None)
        .await?;

    println!("sent {test_val:?} got {val:?}");

    let mut map = api.storage().template_module().tuple_map_iter(None).await?;
    while let Some(x) = map.next().await? {
        println!("{x:?}");
    }
    
    Ok(())
}  

Think you guys can pick up from here. In the example code, the print saids "sent 500 got None", even though iterating the map proves insert was successful.

@jsdw
Copy link
Collaborator

jsdw commented Feb 15, 2022

Thanks @wzli, that's very handy! I'll try to get to this in the next few days.

@jsdw jsdw self-assigned this Feb 15, 2022
@jsdw
Copy link
Collaborator

jsdw commented Feb 17, 2022

Ok, so I've dug into this a little. I haven't fully wrapped my head around it, but the issue seems to stem from the fact that the generated code for that TupleMap entry looks like:

pub struct TupleMap(
    pub ::subxt::sp_core::crypto::AccountId32,
    pub ::core::primitive::u32,
);
impl ::subxt::StorageEntry for TupleMap {
    const PALLET: &'static str = "TemplateModule";
    const STORAGE: &'static str = "TupleMap";
    type Value = ::core::primitive::u32;
    fn key(&self) -> ::subxt::StorageEntryKey {
        ::subxt::StorageEntryKey::Map(vec![::subxt::StorageMapKey::new(
            &self.0,
            ::subxt::StorageHasher::Twox64Concat,
        )])
    }
}

But the key fn is ignoring self.1 entirely. I think it should more like:

pub struct TupleMap(
    pub ::subxt::sp_core::crypto::AccountId32,
    pub ::core::primitive::u32,
);
impl ::subxt::StorageEntry for TupleMap {
    const PALLET: &'static str = "TemplateModule";
    const STORAGE: &'static str = "TupleMap";
    type Value = ::core::primitive::u32;
    fn key(&self) -> ::subxt::StorageEntryKey {
        ::subxt::StorageEntryKey::Map(vec![::subxt::StorageMapKey::new(
            &(&self.0, self.1),
            ::subxt::StorageHasher::Twox64Concat,
        )])
    }
}

When I point your example code at my generated code with said change applied, we get back Some(500) as you'd expect.

I'll need to wrap my head around the storage generation code to work out what's going on here, and why this is the case. It does appear to be a bug in subxt though in any case!

@ascjones
Copy link
Contributor

The bug is because it is incorrectly assuming that a tuple key belongs to a StorageNMap entry: https://github.com/paritytech/substrate/blob/27b8806ed82844bb5283a4dadf853ee518fd042f/frame/support/src/storage/types/nmap.rs#L455.

What it should do is check how many hashers there are, if there is 1 then it is a regular StorageMap so hash the whole key (as in this case). If hashers_count == tuple_field_count then hash each individual field with their respective hashers.

This is the kind of thing that will need a test, as demonstrated by this bug...

@jsdw
Copy link
Collaborator

jsdw commented Feb 18, 2022

@wzli hopefully this will resolve your bug, if you'd like to point to that branch for now until it merges: #458

@jsdw jsdw closed this as completed in #458 Feb 21, 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.

3 participants