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 rand to 0.9 #1862

Closed
wants to merge 1 commit into from
Closed

Conversation

getong
Copy link

@getong getong commented Mar 1, 2025

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@YuriQueirozAndrade
Copy link

WIP ?

@getong
Copy link
Author

getong commented Mar 3, 2025

WIP ?

forgot to update comment, sorry

@YuriQueirozAndrade
Copy link

ok, good, but when I compile (I run cargo check and cargo build, I get the same errors), I get these errors, my question about WIP is because of these errors, in your environment the compile is clean ?

  • Errors:

    error[E0277]: the trait bound `ThreadRng: rand_core::CryptoRng` is not satisfied
       --> crates/wallet/src/keys/mod.rs:647:47
        |
    647 |         Self::generate_with_aux_rand(options, &mut bitcoin::key::rand::thread_rng())
        |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `DerefMut` is not implemented for `ThreadRng`
        |
    note: there are multiple versions of `rand_core` in the dependency graph
       --> /home/yuri/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rand_core-0.9.3/src/lib.rs:204:1
        |
    204 | pub trait CryptoRng: RngCore {}
        |
    note: required by a bound in `GeneratableKey::generate_with_aux_rand`
       --> crates/wallet/src/keys/mod.rs:655:25
        |
    655 |     rng: &mut (impl CryptoRng + RngCore),
        |                         ^^^^^^^^^ required by this bound in `GeneratableKey::generate_with_aux_rand`
    error[E0277]: the trait bound `ThreadRng: rand_core::RngCore` is not satisfied
       --> crates/wallet/src/wallet/tx_builder.rs:660:35
        |
    660 |         self.finish_with_aux_rand(&mut bitcoin::key::rand::thread_rng())
        |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `DerefMut` is not implemented for `ThreadRng`
        |
    note: there are multiple versions of `rand_core` in the dependency graph
       --> /home/yuri/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rand_core-0.9.3/src/lib.rs:130:1
        |
    130 | pub trait RngCore {}
        |
    note: required by a bound in `TxBuilder::<'_, Cs>::finish_with_aux_rand`
       --> crates/wallet/src/wallet/tx_builder.rs:673:54
        |
    673 |     pub fn finish_with_aux_rand(self, rng: &mut impl RngCore) -> Result<Psbt, CreateTxError> {
        |                                               ^^^^^^^ required by this bound in `TxBuilder::<'_, Cs>::finish_with_aux_rand`

Another thing, try using conventional commits, I believe you can gain more visibility from reviewers that way

@getong
Copy link
Author

getong commented Mar 3, 2025

oh, i see the error too.
And I make a pr to this, rust-bitcoin/rust-secp256k1#783
wait for this.

@thunderbiscuit
Copy link
Member

@getong please consider writing good descriptions for your PRs, particularly as a first-time contributor. Why the bump? Did you run into issues? Is it a security fix? Are you just trying to keep all our dependencies up to date one at a time?

BDK is a big project with tons of users and contributors. If you'd like to see your PR merged, it needs to meet the bar for high quality open source contributions. Cheers!

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

Successfully merging this pull request may close these issues.

4 participants