Skip to content

Conversation

@nresare
Copy link

@nresare nresare commented Feb 19, 2025

The method signatures in RandomizedPrehashSigner and RandomizedDigestSigner have updated in the signature crate. This change makes the relevant changes to have the implementations match the updated traits

The method signatures in RandomizedPrehashSigner and
RandomizedDigestSigner have updated in the signature crate. This
change makes the relevant changes to have the implementations
match the updated traits
@nresare nresare marked this pull request as draft February 19, 2025 17:09
@nresare
Copy link
Author

nresare commented Feb 19, 2025

This PR contains the changes I had to make to be able to build against the unreleased changes that have recently been merged to HEAD in the https://github.com/RustCrypto/traits repo. When a new pre-release of the signature create is released I expect to update this change and have build with released artefacts.

I went with using try_fill_bytes().expect() as this reflects the old behaviour (panic on failure to read random data). One could possibly argue that it should be possible to modify the returned Result such that we can recover from this failure, but I think that in practice failure to obtain random data should be a terminal error.

This might be of interest to @baloo, and hopefully I haven't duplicated too much effort

@baloo
Copy link
Member

baloo commented Feb 19, 2025

If you're consuming those pre-release crates, you should be able to just pin versions.
If you pin signature =0.2.3-pre.4 then work backwards from there, you should be able to make things work.
I would recommend that.

Bumping the ecosystem to rand_core 0.9.0 is a bit convonluted.
I've done some of it, you can see the number of PR I've opened in the last week, they all relates to it.

This otherwise duplicates work done in #901, but that can't merge before RustCrypto/traits#1751 gets merged (and probably RustCrypto/traits#1755).

Once this is done, we will need to bump the downstream crates of ecdsa (p256, p384, ...).

@nresare
Copy link
Author

nresare commented Feb 19, 2025

Oh, I feel a bit silly now, completely missed #901 and it turns out it is indeed duplicated effort.

I'm not consuming pre-release crates, but rather looking at some unrelated experimentation that required changes in tandem to crates ecdsa and signature and as such it seemed logical to use the HEAD versions in the respective repositories. As a first step, I realised that one wouldn't compile with the other and it seemed prudent to open a PR with my fixes.

If you would like any help propagating the rand version bump into all the nooks and crannies of the code base once the core parts have landed, I'm happy to help.

Closing this now, as as it duplicates #901

@nresare nresare closed this Feb 19, 2025
@baloo
Copy link
Member

baloo commented Feb 19, 2025

If you would like any help propagating the rand version bump into all the nooks and crannies of the code base once the core parts have landed, I'm happy to help.

For now, I think the "core" should be mostly done (except dsa, which we need to bump to crypto-bigint first (#906)).
But afterwards there might be some things to fix over in RustCrypto/formats but I haven't looked at that just yet (dependency trree will get easier once the base traits have merged, no need to pull git dependencies everywhere).

In any case, help is always appreciated ;)

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.

2 participants