Using blake2_128_concat instead of blake2_256 for storage key in pallet-contracts child storage

[atenjin]

Currently, pallet-contracts module using blake2_256 rather than blake2_128_concat for hashing a contract storage key:

substrate/frame/contracts/src/storage.rs

Lines 47 to 49 in 445fa2d

	pub fn read(trie_id: &TrieId, key: &StorageKey) -> Option<Vec<u8>> {
	child::get_raw(&crate::child_trie_info(&trie_id), &blake2_256(key))
	}

and

substrate/frame/contracts/src/storage.rs

Lines 71 to 72 in 445fa2d

	let hashed_key = blake2_256(key);
	let child_trie_info = &crate::child_trie_info(&trie_id);

then in runtime, all storages already have used xxx_concat(like blake2_128_concat) hash to replace xxx (like blake2_256) to let a storage key contains the source information. In this way, the third parties could parse the hashed key to get the source key information.

I think pallet-contracts also needs this feature, thus, I advice maybe we need to change the blake2_256 hash to blake2_128_concat, or design another hash way to let the hash contains the source key information.

Maybe using blake2_128_concat instead of blake2_256 is a simple way.

Thanks.

[bkchr]

CC @athei

[athei]

I agree with you. Contracts should also make use of transparent hashing. We re-hash the key to prevent attackers to unbalance the trie. But transparent hashing should achieve the same goal because the attacker cannot choose the prefix.

In this way, the third parties could parse the hashed key to get the source key information.

I would assume that the source is already a hash. To discover the "real" source would require that this is also a transparent hash, right? But the keys supplied to pallet contracts must be 32 byte in size. So I don't know how this could work out.

[atenjin]

Emmm, you are right. I forget the ink! storage key is just a 32 bytes value. I do not know how #[ink::storage] part converts to a 32 bytes value yet.
Currenlty, through contract metadata, I think there must be a way to use an existed value to convert to the storage key(e.g. for erc20 contract, when I know an AccountId then If I what to query the balance of this account, I could use this AccountId to calculate the storage key.)
But I don't know whether there is a way if provide a raw contract storage key(32 bytes value), I could parse the raw storage key to Erc20::balances plus AccountId by contract metadata. (e.g. export a 32 bytes key for current state directly, like using export-state or something else to get the 32 bytes key, then convert to Erc20::balances and AccountId)
(It seems be impossible, for the key is just 32 bytes length, not a variable-length value....)
If this way is existed, then change blake2_256 to blake2_128_concatis useful, or else, it seems be useless...

All in all, the 3rd parites client requirement is that, if using some method to get a raw key, then the 3rd parties could parse the raw way through metadata to get the source key.

Like in runtime, when export the states for a block height, it's all raw keys and values, then we could parse the raw key to get the information which module and storage the key belongs to. This is very useful for substrate-archive to parse data and something else like statistics application.

But it seems be impossible under current pallet-contracts and ink model... for ink! storage key just a 32 bytes length value, it must lose the source information for this key in contract.

[athei]

But it seems be impossible under current pallet-contracts and ink model... for ink! storage key just a 32 bytes length value, it must lose the source information for this key in contract.

Yes. In order to make this work we also need to change seal_set_storage to take an arbitrary long key that we then hash with blake_128_concat. ink! of course needs to use that to supply a properly constructed prefix and encode that in the metadata to make that usable in the UI.

That would also make seal_restore_to more complicated when the keys can have different sizes.

[athei]

@Robbepop I think we should make this happen. Having transparent hashing for contract storage would allow for a much better UX when using a generic block explorer.

[stale]

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

[athei]

Closed in favor of #11029