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

Some algorithms don't implement Sum #120

Open
coryschwartz opened this issue Dec 25, 2019 · 2 comments
Open

Some algorithms don't implement Sum #120

coryschwartz opened this issue Dec 25, 2019 · 2 comments

Comments

@coryschwartz
Copy link

These algorighms are found in multiformats.Names, but are never registered in sum.go. As a result, attempting to use multiformats.Sum with these algorighms raises ErrSumNotSupported.

I have no need for these, myself, but it seems inconsistent for Sum to work on some algorithms and not others. Is there any reason these are not registered?

keccak-224
x11
keccak-384
@coryschwartz
Copy link
Author

I'd like to take a stab at this PR -- do let me know if something needs changed here.

x11 was added last year, it seems just not registered at the time.

As for the keccac functions I could imagine these just being aliased to the sha3 variant, although that's not quite right. Maybe the variants that lack a NewLegacyKeccak* method can just be removed -- I am not sure.

@Stebalien Stebalien transferred this issue from multiformats/multihash Dec 31, 2019
@Stebalien
Copy link
Member

Hm. I'm not sure what to do about these.

  • We added a definition for the x11 code to this package but never intended to implement it in this package (too many additional dependencies).
  • We removed support for keccak-224 and keccak-384 to remove an external dependency.

My concern is that these hashes shouldn't be rejected as invalid just because this implementation of multihash doesn't implement them.

We should consider simply removing the ValidCode check and registering the code in the Names and Codes maps inside RegisterHashFunc. But this could have other implications (some error cases will no longer be caught) so I don't want to rush any decisions here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants