Skip to content

Commit

Permalink
Distinct handling for N fields + 1 hasher vs N fields + N hashers (#458)
Browse files Browse the repository at this point in the history
* Distinct handling for N fields + 1 hasher vs N fields + N hashers

* tweak comment

* cargo fmt

* fix typo

* Add a few storage specific tests

* clippy fixes

* cargo fmt

* Add a test to specifically address this fix

* comment typo

* Address niggles

* slgihtly nicer iter code
  • Loading branch information
jsdw authored Feb 21, 2022
1 parent d01fdd7 commit 70d83fe
Show file tree
Hide file tree
Showing 3 changed files with 183 additions and 14 deletions.
58 changes: 44 additions & 14 deletions codegen/src/api/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,26 +115,56 @@ fn generate_storage_entry_fns(
(field_name, field_type)
})
.collect::<Vec<_>>();
// toddo: [AJ] use unzip here?
let tuple_struct_fields =
fields.iter().map(|(_, field_type)| field_type);
let field_names = fields.iter().map(|(field_name, _)| field_name);

let field_names = fields.iter().map(|(n, _)| n);
let field_types = fields.iter().map(|(_, t)| t);

let entry_struct = quote! {
pub struct #entry_struct_ident( #( pub #tuple_struct_fields ),* );
pub struct #entry_struct_ident( #( pub #field_types ),* );
};
let constructor =
quote!( #entry_struct_ident( #( #field_names ),* ) );
let keys = (0..tuple.fields().len()).into_iter().zip(hashers).map(
|(field, hasher)| {
let index = syn::Index::from(field);
quote!( ::subxt::StorageMapKey::new(&self.#index, #hasher) )
},
);
let key_impl = quote! {
::subxt::StorageEntryKey::Map(
vec![ #( #keys ),* ]

let key_impl = if hashers.len() == fields.len() {
// If the number of hashers matches the number of fields, we're dealing with
// something shaped like a StorageNMap, and each field should be hashed separately
// according to the corresponding hasher.
let keys = hashers
.into_iter()
.enumerate()
.map(|(field_idx, hasher)| {
let index = syn::Index::from(field_idx);
quote!( ::subxt::StorageMapKey::new(&self.#index, #hasher) )
});
quote! {
::subxt::StorageEntryKey::Map(
vec![ #( #keys ),* ]
)
}
} else if hashers.len() == 1 {
// If there is one hasher, then however many fields we have, we want to hash a
// tuple of them using the one hasher we're told about. This corresponds to a
// StorageMap.
let hasher = hashers.get(0).expect("checked for 1 hasher");
let items = (0..fields.len()).map(|field_idx| {
let index = syn::Index::from(field_idx);
quote!( &self.#index )
});
quote! {
::subxt::StorageEntryKey::Map(
vec![ ::subxt::StorageMapKey::new(&(#( #items ),*), #hasher) ]
)
}
} else {
// If we hit this condition, we don't know how to handle the number of hashes vs fields
// that we've been handed, so abort.
abort_call_site!(
"Number of hashers ({}) does not equal 1 for StorageMap, or match number of fields ({}) for StorageNMap",
hashers.len(),
fields.len()
)
};

(fields, entry_struct, constructor, key_impl)
}
_ => {
Expand Down
2 changes: 2 additions & 0 deletions subxt/tests/integration/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ mod client;
mod events;
#[cfg(test)]
mod frame;
#[cfg(test)]
mod storage;

use test_runtime::node_runtime;
use utils::*;
137 changes: 137 additions & 0 deletions subxt/tests/integration/storage.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
// Copyright 2019-2022 Parity Technologies (UK) Ltd.
// This file is part of subxt.
//
// subxt is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// subxt is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with subxt. If not, see <http://www.gnu.org/licenses/>.

use crate::{
node_runtime::{
self,
DispatchError,
},
pair_signer,
test_context,
};
use sp_keyring::AccountKeyring;

#[async_std::test]
async fn storage_plain_lookup() -> Result<(), subxt::Error<DispatchError>> {
let ctx = test_context().await;

// Look up a plain value
let entry = ctx.api.storage().timestamp().now(None).await?;
assert!(entry > 0);

Ok(())
}

#[async_std::test]
async fn storage_map_lookup() -> Result<(), subxt::Error<DispatchError>> {
let ctx = test_context().await;

let signer = pair_signer(AccountKeyring::Alice.pair());
let alice = AccountKeyring::Alice.to_account_id();

// Do some transaction to bump the Alice nonce to 1:
ctx.api
.tx()
.system()
.remark(vec![1, 2, 3, 4, 5])
.sign_and_submit_then_watch(&signer)
.await?
.wait_for_finalized_success()
.await?;

// Look up the nonce for the user (we expect it to be 1).
let entry = ctx.api.storage().system().account(alice, None).await?;
assert_eq!(entry.nonce, 1);

Ok(())
}

// This fails until the fix in https://github.com/paritytech/subxt/pull/458 is introduced.
// Here we create a key that looks a bit like a StorageNMap key, but should in fact be
// treated as a StorageKey (ie we should hash both values together with one hasher, rather
// than hash both values separately, or ignore the second value).
#[async_std::test]
async fn storage_n_mapish_key_is_properly_created(
) -> Result<(), subxt::Error<DispatchError>> {
use codec::Encode;
use node_runtime::{
runtime_types::sp_core::crypto::KeyTypeId,
session::storage::KeyOwner,
};
use subxt::{
storage::StorageKeyPrefix,
StorageEntry,
};

// This is what the generated code hashes a `session().key_owner(..)` key into:
let actual_key_bytes = KeyOwner(KeyTypeId([1, 2, 3, 4]), vec![5u8, 6, 7, 8])
.key()
.final_key(StorageKeyPrefix::new::<KeyOwner>())
.0;

// Let's manually hash to what we assume it should be and compare:
let expected_key_bytes = {
// Hash the prefix to the storage entry:
let mut bytes = sp_core::twox_128("Session".as_bytes()).to_vec();
bytes.extend(&sp_core::twox_128("KeyOwner".as_bytes())[..]);
// twox64_concat a *tuple* of the args expected:
let suffix = (KeyTypeId([1, 2, 3, 4]), vec![5u8, 6, 7, 8]).encode();
bytes.extend(&sp_core::twox_64(&suffix));
bytes.extend(&suffix);
bytes
};

assert_eq!(actual_key_bytes, expected_key_bytes);
Ok(())
}

#[async_std::test]
async fn storage_n_map_storage_lookup() -> Result<(), subxt::Error<DispatchError>> {
let ctx = test_context().await;

// Boilerplate; we create a new asset class with ID 99, and then
// we "approveTransfer" of some of this asset class. This gives us an
// entry in the `Approvals` StorageNMap that we can try to look up.
let signer = pair_signer(AccountKeyring::Alice.pair());
let alice = AccountKeyring::Alice.to_account_id();
let bob = AccountKeyring::Bob.to_account_id();
ctx.api
.tx()
.assets()
.create(99, alice.clone().into(), 1)
.sign_and_submit_then_watch(&signer)
.await?
.wait_for_finalized_success()
.await?;
ctx.api
.tx()
.assets()
.approve_transfer(99, bob.clone().into(), 123)
.sign_and_submit_then_watch(&signer)
.await?
.wait_for_finalized_success()
.await?;

// The actual test; look up this approval in storage:
let entry = ctx
.api
.storage()
.assets()
.approvals(99, alice, bob, None)
.await?;
assert_eq!(entry.map(|a| a.amount), Some(123));
Ok(())
}

0 comments on commit 70d83fe

Please sign in to comment.