Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Use one blake2 implementation #12260

Closed
nazar-pc opened this issue Sep 13, 2022 · 3 comments · Fixed by #12266
Closed

Use one blake2 implementation #12260

nazar-pc opened this issue Sep 13, 2022 · 3 comments · Fixed by #12266

Comments

@nazar-pc
Copy link
Contributor

nazar-pc commented Sep 13, 2022

I noticed that Substrate uses both blake2 and blake2-rfc crates directly.

If there a specific reason for doing so or can we unify usage to one of them (presumably blake2 as it seems to be more common in general and is shared by indirect dependencies like snow) instead and remove extra dependency?

NOTE: blake2-rfc is also used in parity-db, so to remove it from dependency tree updating parity-db will be necessary as well.

UPD: blake2 doesn't support keyed hashing, so maybe blake2-rfc is the better option for now 🤷‍♂️

@davxy
Copy link
Member

davxy commented Sep 13, 2022

@nazar-pc
Copy link
Contributor Author

Hm, I was referring to RustCrypto/hashes#360

I'm not sure it is used in Substrate (honestly didn't check), but if one is superior in terms of feature set and comparable in performance, there is a higher chance of pulling the other dependency indirectly anyway. Still beneficial to have one dependency in Substrate, but both will be compiled in the end.

@davxy
Copy link
Member

davxy commented Sep 14, 2022

I've made a small recognition and looks like (as I was expecting) the keyed hash is not used within the Substrate repo:

let hash_result = blake2_rfc::blake2b::blake2b(JUNCTION_ID_LEN, &[], data);

^ This is a blake2-256

fn ss58hash(data: &[u8]) -> blake2_rfc::blake2b::Blake2bResult {
let mut context = blake2_rfc::blake2b::Blake2b::new(64);
context.update(PREFIX);
context.update(data);
context.finalize()
}

^ This is a blake2-512


Parity-db is using the keyed version in one place:

https://github.com/paritytech/parity-db/blob/ab6e6bfd76e1646aa9ee54bc950c6e8b0abfbdbb/src/column.rs#L140

But I think that the interface of blake2 crate allows to get the same result... I will do a couple of experiments.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants