Skip to content

Commit 70d83fe

Browse files
authored
Distinct handling for N fields + 1 hasher vs N fields + N hashers (paritytech#458)
* 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
1 parent d01fdd7 commit 70d83fe

File tree

3 files changed

+183
-14
lines changed

3 files changed

+183
-14
lines changed

codegen/src/api/storage.rs

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -115,26 +115,56 @@ fn generate_storage_entry_fns(
115115
(field_name, field_type)
116116
})
117117
.collect::<Vec<_>>();
118-
// toddo: [AJ] use unzip here?
119-
let tuple_struct_fields =
120-
fields.iter().map(|(_, field_type)| field_type);
121-
let field_names = fields.iter().map(|(field_name, _)| field_name);
118+
119+
let field_names = fields.iter().map(|(n, _)| n);
120+
let field_types = fields.iter().map(|(_, t)| t);
121+
122122
let entry_struct = quote! {
123-
pub struct #entry_struct_ident( #( pub #tuple_struct_fields ),* );
123+
pub struct #entry_struct_ident( #( pub #field_types ),* );
124124
};
125125
let constructor =
126126
quote!( #entry_struct_ident( #( #field_names ),* ) );
127-
let keys = (0..tuple.fields().len()).into_iter().zip(hashers).map(
128-
|(field, hasher)| {
129-
let index = syn::Index::from(field);
130-
quote!( ::subxt::StorageMapKey::new(&self.#index, #hasher) )
131-
},
132-
);
133-
let key_impl = quote! {
134-
::subxt::StorageEntryKey::Map(
135-
vec![ #( #keys ),* ]
127+
128+
let key_impl = if hashers.len() == fields.len() {
129+
// If the number of hashers matches the number of fields, we're dealing with
130+
// something shaped like a StorageNMap, and each field should be hashed separately
131+
// according to the corresponding hasher.
132+
let keys = hashers
133+
.into_iter()
134+
.enumerate()
135+
.map(|(field_idx, hasher)| {
136+
let index = syn::Index::from(field_idx);
137+
quote!( ::subxt::StorageMapKey::new(&self.#index, #hasher) )
138+
});
139+
quote! {
140+
::subxt::StorageEntryKey::Map(
141+
vec![ #( #keys ),* ]
142+
)
143+
}
144+
} else if hashers.len() == 1 {
145+
// If there is one hasher, then however many fields we have, we want to hash a
146+
// tuple of them using the one hasher we're told about. This corresponds to a
147+
// StorageMap.
148+
let hasher = hashers.get(0).expect("checked for 1 hasher");
149+
let items = (0..fields.len()).map(|field_idx| {
150+
let index = syn::Index::from(field_idx);
151+
quote!( &self.#index )
152+
});
153+
quote! {
154+
::subxt::StorageEntryKey::Map(
155+
vec![ ::subxt::StorageMapKey::new(&(#( #items ),*), #hasher) ]
156+
)
157+
}
158+
} else {
159+
// If we hit this condition, we don't know how to handle the number of hashes vs fields
160+
// that we've been handed, so abort.
161+
abort_call_site!(
162+
"Number of hashers ({}) does not equal 1 for StorageMap, or match number of fields ({}) for StorageNMap",
163+
hashers.len(),
164+
fields.len()
136165
)
137166
};
167+
138168
(fields, entry_struct, constructor, key_impl)
139169
}
140170
_ => {

subxt/tests/integration/main.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ mod client;
2323
mod events;
2424
#[cfg(test)]
2525
mod frame;
26+
#[cfg(test)]
27+
mod storage;
2628

2729
use test_runtime::node_runtime;
2830
use utils::*;

subxt/tests/integration/storage.rs

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Copyright 2019-2022 Parity Technologies (UK) Ltd.
2+
// This file is part of subxt.
3+
//
4+
// subxt is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// subxt is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU General Public License
15+
// along with subxt. If not, see <http://www.gnu.org/licenses/>.
16+
17+
use crate::{
18+
node_runtime::{
19+
self,
20+
DispatchError,
21+
},
22+
pair_signer,
23+
test_context,
24+
};
25+
use sp_keyring::AccountKeyring;
26+
27+
#[async_std::test]
28+
async fn storage_plain_lookup() -> Result<(), subxt::Error<DispatchError>> {
29+
let ctx = test_context().await;
30+
31+
// Look up a plain value
32+
let entry = ctx.api.storage().timestamp().now(None).await?;
33+
assert!(entry > 0);
34+
35+
Ok(())
36+
}
37+
38+
#[async_std::test]
39+
async fn storage_map_lookup() -> Result<(), subxt::Error<DispatchError>> {
40+
let ctx = test_context().await;
41+
42+
let signer = pair_signer(AccountKeyring::Alice.pair());
43+
let alice = AccountKeyring::Alice.to_account_id();
44+
45+
// Do some transaction to bump the Alice nonce to 1:
46+
ctx.api
47+
.tx()
48+
.system()
49+
.remark(vec![1, 2, 3, 4, 5])
50+
.sign_and_submit_then_watch(&signer)
51+
.await?
52+
.wait_for_finalized_success()
53+
.await?;
54+
55+
// Look up the nonce for the user (we expect it to be 1).
56+
let entry = ctx.api.storage().system().account(alice, None).await?;
57+
assert_eq!(entry.nonce, 1);
58+
59+
Ok(())
60+
}
61+
62+
// This fails until the fix in https://github.com/paritytech/subxt/pull/458 is introduced.
63+
// Here we create a key that looks a bit like a StorageNMap key, but should in fact be
64+
// treated as a StorageKey (ie we should hash both values together with one hasher, rather
65+
// than hash both values separately, or ignore the second value).
66+
#[async_std::test]
67+
async fn storage_n_mapish_key_is_properly_created(
68+
) -> Result<(), subxt::Error<DispatchError>> {
69+
use codec::Encode;
70+
use node_runtime::{
71+
runtime_types::sp_core::crypto::KeyTypeId,
72+
session::storage::KeyOwner,
73+
};
74+
use subxt::{
75+
storage::StorageKeyPrefix,
76+
StorageEntry,
77+
};
78+
79+
// This is what the generated code hashes a `session().key_owner(..)` key into:
80+
let actual_key_bytes = KeyOwner(KeyTypeId([1, 2, 3, 4]), vec![5u8, 6, 7, 8])
81+
.key()
82+
.final_key(StorageKeyPrefix::new::<KeyOwner>())
83+
.0;
84+
85+
// Let's manually hash to what we assume it should be and compare:
86+
let expected_key_bytes = {
87+
// Hash the prefix to the storage entry:
88+
let mut bytes = sp_core::twox_128("Session".as_bytes()).to_vec();
89+
bytes.extend(&sp_core::twox_128("KeyOwner".as_bytes())[..]);
90+
// twox64_concat a *tuple* of the args expected:
91+
let suffix = (KeyTypeId([1, 2, 3, 4]), vec![5u8, 6, 7, 8]).encode();
92+
bytes.extend(&sp_core::twox_64(&suffix));
93+
bytes.extend(&suffix);
94+
bytes
95+
};
96+
97+
assert_eq!(actual_key_bytes, expected_key_bytes);
98+
Ok(())
99+
}
100+
101+
#[async_std::test]
102+
async fn storage_n_map_storage_lookup() -> Result<(), subxt::Error<DispatchError>> {
103+
let ctx = test_context().await;
104+
105+
// Boilerplate; we create a new asset class with ID 99, and then
106+
// we "approveTransfer" of some of this asset class. This gives us an
107+
// entry in the `Approvals` StorageNMap that we can try to look up.
108+
let signer = pair_signer(AccountKeyring::Alice.pair());
109+
let alice = AccountKeyring::Alice.to_account_id();
110+
let bob = AccountKeyring::Bob.to_account_id();
111+
ctx.api
112+
.tx()
113+
.assets()
114+
.create(99, alice.clone().into(), 1)
115+
.sign_and_submit_then_watch(&signer)
116+
.await?
117+
.wait_for_finalized_success()
118+
.await?;
119+
ctx.api
120+
.tx()
121+
.assets()
122+
.approve_transfer(99, bob.clone().into(), 123)
123+
.sign_and_submit_then_watch(&signer)
124+
.await?
125+
.wait_for_finalized_success()
126+
.await?;
127+
128+
// The actual test; look up this approval in storage:
129+
let entry = ctx
130+
.api
131+
.storage()
132+
.assets()
133+
.approvals(99, alice, bob, None)
134+
.await?;
135+
assert_eq!(entry.map(|a| a.amount), Some(123));
136+
Ok(())
137+
}

0 commit comments

Comments
 (0)