-
Notifications
You must be signed in to change notification settings - Fork 27
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
multiindex key definition - footgun #83
Comments
Hi! I think you're right; that's a footgun and it's begging for a better design. I'll see if we can do some type/trait magic to prevent things like that at compile time. In the meantime, I want to note it might be worth watching storey as an alternative storage library. It currently doesn't have indexing built-in, but for simpler needs, I feel it's set up to leverage the Rust type system a little better. I'm biased though! |
I think I have good news. The committed indexing data doesn't seem to be influenced by the type of the PK (it just shoves the bytes blindly), so I think just changing that type is okay without migrating the data already stored. That said, you could test this first. You could write a test that first saves a bunch of data using the unpatched |
I'll check out storey.. |
More than anything, the structure that represents the indexes, from the macro_rules! create_index_map {
(
$map_name:ident for $base:ident where PK: $pk:ident,
pk_namespace: $pk_ns:expr,
multi: [$($m_f:ident: $m_t:tt => $m_fn:expr, $m_ns:expr),*],
unique: [$($u_f:ident: $u_t:tt =>$u_fn:expr, $u_ns:expr),*]
) => {
paste::paste!{pub struct [<$base Indexes>]<'a>{
$(pub $m_f: cw_storage_plus::MultiIndex<'a, $m_t, $base, $pk>),*,
$(pub $u_f: cw_storage_plus::UniqueIndex<'a, $u_t, $base, $pk>),*,
}
impl <'a> cw_storage_plus::IndexList<$base> for [<$base Indexes>]<'a> {
fn get_indexes(&self) -> Box<dyn Iterator<Item = &'_ dyn cw_storage_plus::Index<$base>> + '_> {
let v: Vec<&dyn cw_storage_plus::Index<$base>> = vec![$(&self.$m_f,)* $(&self.$u_f,)*];
Box::new(v.into_iter())
}
}
pub const $map_name: cw_storage_plus::IndexedMap<$pk, $base, [<$base Indexes>]> = cw_storage_plus::IndexedMap::new($pk_ns, [<$base Indexes>] {
$(
$m_f: cw_storage_plus::MultiIndex::new($m_fn, $pk_ns, $m_ns),
)*
$(
$u_f: cw_storage_plus::UniqueIndex::new($u_fn, $u_ns),
)*
});}
};
}
// --- Example of usage ---
#[derive(Serialize, Deserialize, Clone)]
pub struct Bar {
pub name: String,
pub surname: String,
pub id: u64,
}
create_index_map!(
// BAR will be the name of the IndexMap
BAR for Bar where PK: u64,
pk_namespace: "bar_ns",
// Multi indexes, leave this empty if not needed
multi: [
// idx name: IK type => closure, index namespace
name: String => |_, data| data.name.clone(), "bar_name_ns",
name_surname: (String, String) => |_, data| (data.name.clone(), data.surname.clone()), "bar_name_surname_ns"
],
// Unique indexes, leave this empty if not needed
unique: [
id: u64 => |data| data.id, "bar_id_ns"
]
); |
I am having an issue with CW-storage-plus. (thanks to Eris/Phil @0xphilipp for pointing it out)
I have managed to produce an example which demonstrates it here - testcase
I have a key which is a (u64,&str) .. and it works ok, until I put in a u64 > 128, and it then gives a UTF-8 error
how do I remedy this.
is there a way to do this without migrating the index in the existing smart contract
the error returned is as follows
this was due to a multiindex signature not being defined properly.
the following patch to the test case resolves it.
the issue is here as I think this should have resulted in a compile error (the PK was badly defined in the original case), but it wasn't.
Thanks!
The text was updated successfully, but these errors were encountered: