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.

Sign up for GitHub

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

Jump to bottom

Use XXH32 instead of sha256 for const hashing #1393

Merged
merged 5 commits into from
Sep 19, 2022
Merged

Use XXH32 instead of sha256 for const hashing #1393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fbfd4e7
use much simpler fnv instead of sha256 for const hashing
xermicus Sep 13, 2022
46beecb
Merge branch 'master' into cl/const-storage-key-with-stable-rust
xermicus Sep 16, 2022
df85b09
update key tests for fnv1a
xermicus Sep 16, 2022
aae9523
switch to xxhash32
xermicus Sep 16, 2022
3256c28
update UI tests with new keys
xermicus Sep 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions crates/ink/tests/ui/storage_item/fail/collections_only_packed_1.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ error[E0277]: the trait bound `Vec<NonPacked>: parity_scale_codec::Decode` is no
= help: the trait `parity_scale_codec::Decode` is implemented for `Vec<T>`
= note: required because of the requirements on the impl of `Packed` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `Vec<NonPacked>`

error[E0277]: the trait bound `[NonPacked]: Encode` is not satisfied
--> tests/ui/storage_item/fail/collections_only_packed_1.rs:11:8
Expand All @@ -21,7 +21,7 @@ error[E0277]: the trait bound `[NonPacked]: Encode` is not satisfied
= note: required because of the requirements on the impl of `Encode` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `Packed` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `Vec<NonPacked>`

error[E0277]: the trait bound `Vec<NonPacked>: parity_scale_codec::Decode` is not satisfied
--> tests/ui/storage_item/fail/collections_only_packed_1.rs:9:1
Expand All @@ -32,7 +32,7 @@ error[E0277]: the trait bound `Vec<NonPacked>: parity_scale_codec::Decode` is no
= help: the trait `parity_scale_codec::Decode` is implemented for `Vec<T>`
= note: required because of the requirements on the impl of `Packed` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `Vec<NonPacked>`
note: required because it appears within the type `Contract`
--> tests/ui/storage_item/fail/collections_only_packed_1.rs:10:8
|
Expand All @@ -57,7 +57,7 @@ error[E0277]: the trait bound `[NonPacked]: Encode` is not satisfied
= note: required because of the requirements on the impl of `Encode` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `Packed` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `Vec<NonPacked>`
note: required because it appears within the type `Contract`
--> tests/ui/storage_item/fail/collections_only_packed_1.rs:10:8
|
Expand All @@ -79,7 +79,7 @@ error[E0277]: the trait bound `Vec<NonPacked>: parity_scale_codec::Decode` is no
= help: the trait `parity_scale_codec::Decode` is implemented for `Vec<T>`
= note: required because of the requirements on the impl of `Packed` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `Vec<NonPacked>`
note: required because it appears within the type `Contract`
--> tests/ui/storage_item/fail/collections_only_packed_1.rs:10:8
|
Expand All @@ -104,7 +104,7 @@ error[E0277]: the trait bound `[NonPacked]: Encode` is not satisfied
= note: required because of the requirements on the impl of `Encode` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `Packed` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `Vec<NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `Vec<NonPacked>`
note: required because it appears within the type `Contract`
--> tests/ui/storage_item/fail/collections_only_packed_1.rs:10:8
|
Expand Down
12 changes: 6 additions & 6 deletions crates/ink/tests/ui/storage_item/fail/collections_only_packed_2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ error[E0277]: the trait bound `BTreeMap<u128, NonPacked>: parity_scale_codec::De
= help: the trait `parity_scale_codec::Decode` is implemented for `BTreeMap<K, V>`
= note: required because of the requirements on the impl of `Packed` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `BTreeMap<u128, NonPacked>`

error[E0277]: the trait bound `BTreeMap<u128, NonPacked>: Encode` is not satisfied
--> tests/ui/storage_item/fail/collections_only_packed_2.rs:11:8
Expand All @@ -18,7 +18,7 @@ error[E0277]: the trait bound `BTreeMap<u128, NonPacked>: Encode` is not satisfi
= help: the trait `Encode` is implemented for `BTreeMap<K, V>`
= note: required because of the requirements on the impl of `Packed` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `BTreeMap<u128, NonPacked>`

error[E0277]: the trait bound `BTreeMap<u128, NonPacked>: parity_scale_codec::Decode` is not satisfied
--> tests/ui/storage_item/fail/collections_only_packed_2.rs:9:1
Expand All @@ -29,7 +29,7 @@ error[E0277]: the trait bound `BTreeMap<u128, NonPacked>: parity_scale_codec::De
= help: the trait `parity_scale_codec::Decode` is implemented for `BTreeMap<K, V>`
= note: required because of the requirements on the impl of `Packed` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `BTreeMap<u128, NonPacked>`
note: required because it appears within the type `Contract`
--> tests/ui/storage_item/fail/collections_only_packed_2.rs:10:8
|
Expand All @@ -51,7 +51,7 @@ error[E0277]: the trait bound `BTreeMap<u128, NonPacked>: Encode` is not satisfi
= help: the trait `Encode` is implemented for `BTreeMap<K, V>`
= note: required because of the requirements on the impl of `Packed` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `BTreeMap<u128, NonPacked>`
note: required because it appears within the type `Contract`
--> tests/ui/storage_item/fail/collections_only_packed_2.rs:10:8
|
Expand All @@ -73,7 +73,7 @@ error[E0277]: the trait bound `BTreeMap<u128, NonPacked>: parity_scale_codec::De
= help: the trait `parity_scale_codec::Decode` is implemented for `BTreeMap<K, V>`
= note: required because of the requirements on the impl of `Packed` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `BTreeMap<u128, NonPacked>`
note: required because it appears within the type `Contract`
--> tests/ui/storage_item/fail/collections_only_packed_2.rs:10:8
|
Expand All @@ -95,7 +95,7 @@ error[E0277]: the trait bound `BTreeMap<u128, NonPacked>: Encode` is not satisfi
= help: the trait `Encode` is implemented for `BTreeMap<K, V>`
= note: required because of the requirements on the impl of `Packed` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `StorableHint<()>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<2085512762_u32>>` for `BTreeMap<u128, NonPacked>`
= note: required because of the requirements on the impl of `AutoStorableHint<ManualKey<453539032_u32>>` for `BTreeMap<u128, NonPacked>`
note: required because it appears within the type `Contract`
--> tests/ui/storage_item/fail/collections_only_packed_2.rs:10:8
|
Expand Down
2 changes: 1 addition & 1 deletion crates/primitives/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ derive_more = { version = "0.99", default-features = false, features = ["from",
ink_prelude = { version = "4.0.0-alpha.1", path = "../prelude/", default-features = false }
scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive", "full"] }
scale-info = { version = "2", default-features = false, features = ["derive"], optional = true }
sha2-const = { version = "0.1.2", default-features = false }
xxhash-rust = { version = "0.8", features = ["const_xxh32"] }

[features]
default = ["std"]
Expand Down
35 changes: 15 additions & 20 deletions crates/primitives/src/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
// limitations under the License.

use ink_prelude::vec;
use sha2_const::Sha256;
use xxhash_rust::const_xxh32::xxh32;

/// The value 0 is a valid seed.
const XXH32_SEED: u32 = 0;

/// A key into the smart contract storage.
///
Expand All @@ -33,18 +36,12 @@ impl KeyComposer {
/// Concatenate two `Key` into one during compilation.
pub const fn concat(left: Key, right: Key) -> Key {
// If one of the keys is zero, then return another without hashing.
// If both keys are non-zero, return the hash of both keys.
// If both keys are non-zero, return the hash of the XOR difference of both keys.
match (left, right) {
(0, 0) => 0,
(0, _) => right,
(_, 0) => left,
(left, right) => {
let hash = Sha256::new()
.update(&left.to_be_bytes())
.update(&right.to_be_bytes())
.finalize();
Key::from_be_bytes([hash[0], hash[1], hash[2], hash[3]])
}
(left, right) => xxh32(&(left ^ right).to_be_bytes(), XXH32_SEED),
}
}

Expand All @@ -59,8 +56,7 @@ impl KeyComposer {
return 0
}

let hash = Sha256::new().update(bytes).finalize();
Key::from_be_bytes([hash[0], hash[1], hash[2], hash[3]])
xxh32(bytes, XXH32_SEED)
}

/// Evaluates the storage key of the field in the structure, variant or union.
Expand All @@ -69,8 +65,7 @@ impl KeyComposer {
/// 1. If `variant_name` is not empty then computes the ASCII byte representation and call it `V`.
/// 1. Compute the ASCII byte representation of `field_name` and call it `F`.
/// 1. Concatenate (`S` and `F`) or (`S`, `V` and `F`) using `::` as separator and call it `C`.
/// 1. Apply the `SHA2` 256-bit hash `H` of `C`.
/// 1. The first 4 bytes of `H` make up the storage key.
/// 1. The `FNV1A` 32-bit hash of `C` is the storage key.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xermicus this should be XXHASH instead, eh?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! 🙈

///
/// # Note
///
Expand Down Expand Up @@ -119,33 +114,33 @@ mod tests {
fn concat_works_correct() {
assert_eq!(KeyComposer::concat(0, 13), 13);
assert_eq!(KeyComposer::concat(31, 0), 31);
assert_eq!(KeyComposer::concat(31, 13), 0xD83A5CD8);
assert_eq!(KeyComposer::concat(31, 13), 0x9ab19a67);
assert_eq!(KeyComposer::concat(0, 0), 0);
}

#[test]
fn from_str_works_correct() {
assert_eq!(KeyComposer::from_str(""), 0);
assert_eq!(KeyComposer::from_str("123"), 0xa665a459);
assert_eq!(KeyComposer::from_str("Hello world"), 0x64ec88ca);
assert_eq!(KeyComposer::from_str("123"), 0xb6855437);
assert_eq!(KeyComposer::from_str("Hello world"), 0x9705d437);
}

#[test]
fn from_bytes_works_correct() {
assert_eq!(KeyComposer::from_bytes(b""), 0);
assert_eq!(KeyComposer::from_bytes(b"123"), 0xa665a459);
assert_eq!(KeyComposer::from_bytes(b"Hello world"), 0x64ec88ca);
assert_eq!(KeyComposer::from_bytes(b"123"), 0xb6855437);
assert_eq!(KeyComposer::from_bytes(b"Hello world"), 0x9705d437);
}

#[test]
fn compute_key_works_correct() {
assert_eq!(
KeyComposer::compute_key("Contract", "", "balances"),
Ok(0x05e859ec)
Ok(0xf820ff02)
);
assert_eq!(
KeyComposer::compute_key("Enum", "Variant", "0"),
Ok(0x9d029590)
Ok(0x14786b51)
);
assert_eq!(
KeyComposer::compute_key("", "Variant", "0"),
Expand Down