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

Client should not require Block hash to be H256 for everything useful #3624

Closed
rphmeier opened this issue Sep 16, 2019 · 10 comments
Closed

Client should not require Block hash to be H256 for everything useful #3624

rphmeier opened this issue Sep 16, 2019 · 10 comments
Assignees
Labels
I7-refactor Code needs refactoring. U2-some_time_soon Issue is worth doing soon.

Comments

@rphmeier
Copy link
Contributor

rphmeier commented Sep 16, 2019

This pollutes everything.

When we need to use hashes as keys on disk, I think it would be fine to truncate hashes to 256 bits silently and internally (as that is more than enough entropy), but not to require it as a type parameter everywhere.

@rphmeier rphmeier added I7-refactor Code needs refactoring. U2-some_time_soon Issue is worth doing soon. labels Sep 16, 2019
@rphmeier
Copy link
Contributor Author

@andresilva pointed out that somewhere we need a bound proving that Backend::Hasher::Out == Block::Hash.

We should aim to make this more flexible with some kind of conversion parameter or drop the requirement entirely.

@burdges
Copy link

burdges commented Sep 16, 2019

I'm confused.. Isn't H256 exactly what avoid the type parameter everywhere?

@bkchr
Copy link
Member

bkchr commented Sep 17, 2019

@burdges This line

Block: BlockT<Hash=H256>,
is the one that should be dropped.

@rphmeier
Copy link
Contributor Author

Well, I suppose in some sense it's avoiding a concrete specification of the hash algorithm, but Substrate in some places says "we're generic over the hash output length" and in most (important places) it says "the hash length must be 256 bits".

@burdges
Copy link

burdges commented Sep 17, 2019

I see.. We need 256 bit hashes for 128 bits of security anyplace where birthday attacks pose a threat, like in Merkle trees. We can safely neglect that we technically loose some security bits from the Merkle tree itself, so we do not afaik require more than 256 bits anyplace.

As an aside, if we identify places that only require fewer bits of security, like MACs, then we could make them use some MacTag128 or H192 types, but right I think being generic over hash size everywhere seems unnecessary.

@rphmeier
Copy link
Contributor Author

rphmeier commented Sep 17, 2019

It should either be fully-generic or fully-non-generic, not some weird halfway-house.

@burdges
Copy link

burdges commented Sep 17, 2019

I'm saying non-generic H256 is always fine. :)

@rphmeier
Copy link
Contributor Author

Right - and just to clarify, any cryptographic non-safety caused by too-small hashes is not something that would arise as part of the implementation of this issue. That would happen in the chain configuration, where somebody might choose a hash size which is too small.

@Demi-Marie
Copy link
Contributor

IMO, a better option would be to have a type SubstrateHash = H256 and then use SubstrateHash everywhere. That way, we can still change the type if we need to.

@bkchr
Copy link
Member

bkchr commented Jan 23, 2020

DOne

@bkchr bkchr closed this as completed Jan 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I7-refactor Code needs refactoring. U2-some_time_soon Issue is worth doing soon.
Projects
None yet
Development

No branches or pull requests

4 participants