search

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.78k stars 635 forks source link

Fix CandidateHash newtype #5224

Open eskimor opened 1 month ago

eskimor commented 1 month ago

In general Hash is way to overloaded in the codebase, CandidateHash was a glorious exception, being a newtype around Hash, but due to this DeRef instance, it is still interchangeable, limiting the added type safety.

We should:

  1. Remove the CandidateHash DeRef instance.
  2. Introduce more proper newtype wrappers. E.g. for the head data hash of a parachain, also relay block hashes should not be plain Hash ... although that change will be massive and probably overkill if we at least use newtypes for all other hashes.
programskillforverification commented 1 month ago

We can use macro to wrapper newtype around a [Hash] type. Not only for CandidateHash, but also all defined hashtype. I'd like to work on this issue, but maybe that's a breaking change.

eagr commented 3 weeks ago

Stuff whose hash I could think of that might use a newtype (besides relay chain block)

But I'm not sure coz I don't see the entire picture. Anything you'd like to add? :) @eskimor