paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.com/
1.91k stars 704 forks source link

CoreDescriptors and CoreSchedules do not use a transparent hasher for keys #6563

Open seadanda opened 1 day ago

seadanda commented 1 day ago

Both use Twox256 instead of e.g. Twox64Concat, obfuscating the value of the keys.

from a conversation here

Suggest using either Twox64Concat or Blake2_128Concat for the keys

bkchr commented 1 day ago

If you don't need to iterate the keys, there is no need to use a reconstructable hash.

seadanda commented 36 minutes ago

I think about it the other way round: why use one that isn't reconstructable without reason? In this case I get that we don't need to iterate them, but I think UIs and anybody reading the state would find it useful to have readable keys.

Happy either way, especially since this is already live and would be a breaking change to change it. I could easily be convinced it's not worth the change, but it's definitely a minor inconvenience.

eskimor commented 4 minutes ago

Good catch @seadanda ! Will be fixed as part of #1312 . ( I am changing the storage there anyway)