Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CandidateHash newtype #5224

Open
eskimor opened this issue Aug 3, 2024 · 2 comments
Open

Fix CandidateHash newtype #5224

eskimor opened this issue Aug 3, 2024 · 2 comments
Labels
C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK.

Comments

@eskimor
Copy link
Member

eskimor commented Aug 3, 2024

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.
@eskimor eskimor added the C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. label Aug 3, 2024
@programskillforverification
Copy link
Contributor

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
Copy link
Contributor

eagr commented Aug 19, 2024

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

  • code
  • collation
  • proof-of-validity
  • storage root
  • erasure coding root

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK.
Projects
Status: Backlog
Development

No branches or pull requests

3 participants