Skip to content

signature: use Fn(&mut D) for *DigestSigner/*DigestVerifier#2004

Merged
tarcieri merged 1 commit intoRustCrypto:masterfrom
daxpedda:signature-digest-fn
Sep 13, 2025
Merged

signature: use Fn(&mut D) for *DigestSigner/*DigestVerifier#2004
tarcieri merged 1 commit intoRustCrypto:masterfrom
daxpedda:signature-digest-fn

Conversation

@daxpedda
Copy link
Copy Markdown
Contributor

@daxpedda daxpedda commented Sep 12, 2025

I honestly tried to find the right words for the documentation, but it all sounds rather off to me.

My suspicion is that we will actually need FnMut here if users intend to use some Reader API inside.

Companion PR: RustCrypto/signatures#1064.
Resolves #2003.

Comment thread signature/src/signer.rs Outdated
@daxpedda daxpedda force-pushed the signature-digest-fn branch 2 times, most recently from 90bda63 to 322b948 Compare September 12, 2025 20:14
Comment thread signature/src/verifier.rs
@tarcieri
Copy link
Copy Markdown
Member

I think it would be good to add some comments about the expected contract of this function, namely that it should expect to perform all of its updates in a single invocation, and if called repeatedly it should perform the same sequence of updates

@daxpedda
Copy link
Copy Markdown
Contributor Author

Update for the signatures repo is up: RustCrypto/signatures#1064.

Comment thread signature/src/signer.rs
@daxpedda
Copy link
Copy Markdown
Contributor Author

daxpedda commented Sep 12, 2025

FYI because Digest isn't a supertrait of Update I kinda did the implementation with FixedOutput + Update . But I could also write Digest + Update.

Also just noticing that if we actually do FixedOutput + Update we should also add HashMarker? Should I add HashMarker in the trait here?

EDIT: Went with Digest + Update for now.

@tarcieri tarcieri changed the title Use Fn(&mut D) for Async/Randomized/Digest/Signer/Verifier signature: use Fn(&mut D) for *DigestSigner/*DigestVerifier Sep 13, 2025
@tarcieri tarcieri merged commit a9ccac1 into RustCrypto:master Sep 13, 2025
11 checks passed
tarcieri pushed a commit to RustCrypto/signatures that referenced this pull request Sep 13, 2025
@tarcieri
Copy link
Copy Markdown
Member

@daxpedda this is kind of annoying re: Digest + Update:

error[E0034]: multiple applicable items in scope
   --> k256/src/schnorr/verifying.rs:119:53
    |
119 |                 msg.iter().for_each(|&slice| digest.update(slice));
    |                                                     ^^^^^^ multiple `update` found
    |
    = note: candidate #1 is defined in an impl of the trait `Digest` for the type `D`
    = note: candidate #2 is defined in an impl of the trait `Update` for the type `Sha256`

Maybe it should just be Default + Update + FixedOutput? (which AFAICT would also work for Shake256`)

@daxpedda
Copy link
Copy Markdown
Contributor Author

Yeah, we can just use Default + FixedOutput + Update, but in implementations. I don't believe Shake supports FixedOutput, where we can just skip it.

I mentioned this before, but we might want to add HashMarker as well?

tarcieri added a commit to RustCrypto/elliptic-curves that referenced this pull request Sep 13, 2025
Also bumps `ecdsa` to v0.17.0-rc.7

This brings `*DigestSigner`/`*DigestVerifier` changes introduced in
RustCrypto/traits#2004
@tarcieri
Copy link
Copy Markdown
Member

Aah, I see, it looks like it did in sha3 v0.10 but not in v0.11. Sure, HashMarker sounds good.

tarcieri added a commit to RustCrypto/elliptic-curves that referenced this pull request Sep 13, 2025
Also bumps `ecdsa` to v0.17.0-rc.7

This brings `*DigestSigner`/`*DigestVerifier` changes introduced in
RustCrypto/traits#2004
tarcieri added a commit to RustCrypto/RSA that referenced this pull request Sep 14, 2025
This brings `*DigestSigner`/`*DigestVerifier` changes introduced in
RustCrypto/traits#2004
tarcieri added a commit to RustCrypto/RSA that referenced this pull request Sep 14, 2025
This brings `*DigestSigner`/`*DigestVerifier` changes introduced in
RustCrypto/traits#2004
takumi-earth pushed a commit to earthlings-dev/RSA that referenced this pull request Jan 27, 2026
This brings `*DigestSigner`/`*DigestVerifier` changes introduced in
RustCrypto/traits#2004
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.

signature: change DigestSigner/DigestVerifier to be ML-DSA "external mu" compatible?

2 participants