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

Update default hamt hash function to sha256 and make algo generic #624

Merged
merged 8 commits into from
Aug 20, 2020

Conversation

austinabell
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • ref: https://github.com/ipfs/go-hamt-ipld/pull/49
    • Done for security reasons (hash size bumped to 32 bytes) but also made generic to allow swapping this hash function more easily (and I didn't necessarily like the feature gating that I did before).
  • Fixed bugs in using wrong bit length or type (key types matter as they get serialized, even if they hash the same)

The defaults for the generic types required that I swap the order of the generics, which unfortunately required to touch a bunch of files, but is necessary as all defaulted must be the last parameters. And if you see Hamt::<_>::.. it's just because you need to explicitly default the types, and can't leave out.

Reference issue to close (if applicable)

Closes #614

Other information and links

@austinabell austinabell added the Spec Change Updates to implementation required due to a Filecoin spec change after implementation label Aug 19, 2020
Copy link
Member

@ec2 ec2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@austinabell austinabell merged commit 2b54a87 into main Aug 20, 2020
@austinabell austinabell deleted the austin/hamt/hashing branch August 20, 2020 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IPLD Spec Change Updates to implementation required due to a Filecoin spec change after implementation Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change to using sha2-256 hashing instead of blake2b
3 participants