Skip to content

wip: "imprint" fn #541

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

Closed
wants to merge 4 commits into from
Closed

wip: "imprint" fn #541

wants to merge 4 commits into from

Conversation

hko-s
Copy link
Contributor

@hko-s hko-s commented Apr 20, 2025

This is just a sketch for discussion.

We could consider having an "imprint" function roughly like this, and implementing "fingerprint" in terms of it.
I think the cost of that would mainly be that the Hasher is Boxed in this more generic approach?

@link2xt
Copy link
Contributor

link2xt commented Apr 21, 2025

Implmenting fingerprints in terms of imprint is good for test coverage, so we can be sure imprints always contain the same data as fingerprints. I don't think a Box is a problem.

@hko-s
Copy link
Contributor Author

hko-s commented Apr 21, 2025

I've pushed a commit to see how fingerprint looks when based on imprint

@hko-s
Copy link
Contributor Author

hko-s commented Apr 21, 2025

It's a bit sad how much expecting is currently going on in this PR. I think it would be possible to make imprint nicer by linking the hash algorithm parameter and returning an array with a corresponding output size.

But it seems that neither the rustcrypto types nor the current rpgp hashing mechanism have suitable facilities for this. We could make them for this PR?

@link2xt
Copy link
Contributor

link2xt commented Apr 21, 2025

But it seems that neither the rustcrypto types nor the current rpgp hashing mechanism have suitable facilities for this. We could make them for this PR?

Is it going to use generic-array? We already have generic-array dependency and it does not need to appear in the public API unless we expose the imprint function with a generic hash algorithm.

@hko-s hko-s changed the title sketch an "imprint" fn wip: "imprint" fn Apr 22, 2025
@hko-s
Copy link
Contributor Author

hko-s commented Apr 22, 2025

Is it going to use generic-array? We already have generic-array dependency and it does not need to appear in the public API unless we expose the imprint function with a generic hash algorithm.

I didn't have a concrete idea, but making this nicer on the type-level would seem potentially worthwhile.

Hoping for @dignifiedquire to weigh in with thoughts/preferences :)

@link2xt
Copy link
Contributor

link2xt commented Apr 22, 2025

I'm also fine with .expect(), it's not really going to crash the app. Especially if alternative with generic types and boxes and so on is much uglier.

@link2xt link2xt mentioned this pull request Apr 30, 2025
11 tasks
@hko-s
Copy link
Contributor Author

hko-s commented May 8, 2025

Closing in favor of #549

@hko-s hko-s closed this May 8, 2025
@hko-s hko-s deleted the imprint branch May 18, 2025 19:40
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

Successfully merging this pull request may close these issues.

3 participants