Merge bitcoindevkit#1395: Remove rand dependency from bdk

4bddb0d feat(wallet): add back TxBuilder finish() and sort_tx() with thread_rng() (Steve Myers)
45c0cae fix(bdk): remove rand dependency (rustaceanrob)

Pull request description:

  ### Description

  WIP towards removing `rand` fixes bitcoindevkit#871

  The `rand` dependency was imported explicitly, but `rand` is also implicitly used through the `rand-std` feature flag on `bitcoin`.

  ### Notes to he reviewers

  **Updated:**

  `rand` was used primarily in two parts of `bdk`. Particularly in signing and in building a transaction.

  Signing:
  - Used implicitly in [`sign_schnorr`](https://docs.rs/bitcoin/latest/bitcoin/key/struct.Secp256k1.html#method.sign_schnorr), but nowhere else within `signer`.

  Transaction ordering:
  - Used to shuffle the inputs and outputs of a transaction, the default
  - Used in the single random draw __as a fallback__ to branch and bound during coin selection. Branch and bound is the default coin selection option.

  See conversation for proposed solutions.

  ### Changelog notice

  - Remove the `rand` dependency from `bdk`

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [x] I've added tests for the new feature
  * [x] I've added docs for the new feature

  #### Bugfixes:

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

ACKs for top commit:
  ValuedMammal:
    ACK 4bddb0d
  notmandatory:
    ACK 4bddb0d

Tree-SHA512: 662d9bcb1e02f8195d73df16789b8c2aba8ccd7b37ba713ebb0bfd19c66163acbcb6f266b64f88347cbb1f96b88c8a150581012cbf818d1dc8b4437b3e53fc62

.github/workflows/cont_integration.yml

@@ -92,7 +92,7 @@ jobs:
         uses: Swatinem/[email protected]
       - name: Check bdk wallet
         working-directory: ./crates/wallet
-        run: cargo check --target wasm32-unknown-unknown --no-default-features --features miniscript/no-std,bdk_chain/hashbrown,dev-getrandom-wasm
+        run: cargo check --target wasm32-unknown-unknown --no-default-features --features miniscript/no-std,bdk_chain/hashbrown
       - name: Check esplora
         working-directory: ./crates/esplora
         run: cargo check --target wasm32-unknown-unknown --no-default-features --features miniscript/no-std,bdk_chain/hashbrown,async

crates/electrum/tests/test_electrum.rs

@@ -62,14 +62,11 @@ fn scan_detects_confirmed_tx() -> anyhow::Result<()> {
 
     // Sync up to tip.
     env.wait_until_electrum_sees_block()?;
-    let update = client
-        .sync(
-            SyncRequest::from_chain_tip(recv_chain.tip())
-                .chain_spks(core::iter::once(spk_to_track)),
-            5,
-            true,
-        )?
-        .with_confirmation_time_height_anchor(&client)?;
+    let update = client.sync(
+        SyncRequest::from_chain_tip(recv_chain.tip()).chain_spks(core::iter::once(spk_to_track)),
+        5,
+        true,
+    )?;
 
     let _ = recv_chain
         .apply_update(update.chain_update)
@@ -155,13 +152,11 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> {
 
     // Sync up to tip.
     env.wait_until_electrum_sees_block()?;
-    let update = client
-        .sync(
-            SyncRequest::from_chain_tip(recv_chain.tip()).chain_spks([spk_to_track.clone()]),
-            5,
-            false,
-        )?
-        .with_confirmation_time_height_anchor(&client)?;
+    let update = client.sync(
+        SyncRequest::from_chain_tip(recv_chain.tip()).chain_spks([spk_to_track.clone()]),
+        5,
+        false,
+    )?;
 
     let _ = recv_chain
         .apply_update(update.chain_update)
@@ -186,13 +181,11 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> {
         env.reorg_empty_blocks(depth)?;
 
         env.wait_until_electrum_sees_block()?;
-        let update = client
-            .sync(
-                SyncRequest::from_chain_tip(recv_chain.tip()).chain_spks([spk_to_track.clone()]),
-                5,
-                false,
-            )?
-            .with_confirmation_time_height_anchor(&client)?;
+        let update = client.sync(
+            SyncRequest::from_chain_tip(recv_chain.tip()).chain_spks([spk_to_track.clone()]),
+            5,
+            false,
+        )?;
 
         let _ = recv_chain
             .apply_update(update.chain_update)

crates/esplora/src/blocking_ext.rs

@@ -1,6 +1,5 @@
 use std::collections::BTreeSet;
 use std::thread::JoinHandle;
-use std::usize;
 
 use bdk_chain::collections::BTreeMap;
 use bdk_chain::spk_client::{FullScanRequest, FullScanResult, SyncRequest, SyncResult};

crates/wallet/Cargo.toml

@@ -13,39 +13,31 @@ edition = "2021"
 rust-version = "1.63"
 
 [dependencies]
-rand = "^0.8"
+rand_core = { version = "0.6.0" }
 miniscript = { version = "12.0.0", features = ["serde"], default-features = false }
-bitcoin = { version = "0.32.0", features = ["serde", "base64", "rand-std"], default-features = false }
+bitcoin = { version = "0.32.0", features = ["serde", "base64"], default-features = false }
 serde = { version = "^1.0", features = ["derive"] }
 serde_json = { version = "^1.0" }
 bdk_chain = { path = "../chain", version = "0.16.0", features = ["miniscript", "serde"], default-features = false }
 
 # Optional dependencies
 bip39 = { version = "2.0", optional = true }
 
-[target.'cfg(target_arch = "wasm32")'.dependencies]
-getrandom = "0.2"
-js-sys = "0.3"
-
 [features]
 default = ["std"]
-std = ["bitcoin/std", "miniscript/std", "bdk_chain/std"]
+std = ["bitcoin/std", "bitcoin/rand-std", "miniscript/std", "bdk_chain/std"]
 compiler = ["miniscript/compiler"]
 all-keys = ["keys-bip39"]
 keys-bip39 = ["bip39"]
 
-# This feature is used to run `cargo check` in our CI targeting wasm. It's not recommended
-# for libraries to explicitly include the "getrandom/js" feature, so we only do it when
-# necessary for running our CI. See: https://docs.rs/getrandom/0.2.8/getrandom/#webassembly-support
-dev-getrandom-wasm = ["getrandom/js"]
-
 [dev-dependencies]
 lazy_static = "1.4"
 assert_matches = "1.5.0"
 tempfile = "3"
 bdk_sqlite = { path = "../sqlite" }
 bdk_file_store = { path = "../file_store" }
 anyhow = "1"
+rand = "^0.8"
 
 [package.metadata.docs.rs]
 all-features = true

crates/wallet/README.md

@@ -70,30 +70,28 @@ To persist `Wallet` state data use a data store crate that reads and writes [`bd
 ```rust,no_run
 use bdk_wallet::{bitcoin::Network, KeychainKind, wallet::{ChangeSet, Wallet}};
 
-fn main() {
-    // Open or create a new file store for wallet data.
-    let mut db =
-        bdk_file_store::Store::<ChangeSet>::open_or_create_new(b"magic_bytes", "/tmp/my_wallet.db")
-            .expect("create store");
-
-    // Create a wallet with initial wallet data read from the file store.
-    let descriptor = "wpkh(tprv8ZgxMBicQKsPdcAqYBpzAFwU5yxBUo88ggoBqu1qPcHUfSbKK1sKMLmC7EAk438btHQrSdu3jGGQa6PA71nvH5nkDexhLteJqkM4dQmWF9g/84'/1'/0'/0/*)";
-    let change_descriptor = "wpkh(tprv8ZgxMBicQKsPdcAqYBpzAFwU5yxBUo88ggoBqu1qPcHUfSbKK1sKMLmC7EAk438btHQrSdu3jGGQa6PA71nvH5nkDexhLteJqkM4dQmWF9g/84'/1'/0'/1/*)";
-    let changeset = db.aggregate_changesets().expect("changeset loaded");
-    let mut wallet =
-        Wallet::new_or_load(descriptor, change_descriptor, changeset, Network::Testnet)
-            .expect("create or load wallet");
-
-    // Get a new address to receive bitcoin.
-    let receive_address = wallet.reveal_next_address(KeychainKind::External);
-    // Persist staged wallet data changes to the file store.
-    let staged_changeset = wallet.take_staged();
-    if let Some(changeset) = staged_changeset {
-        db.append_changeset(&changeset)
-            .expect("must commit changes to database");
-    }
-    println!("Your new receive address is: {}", receive_address.address);
+// Open or create a new file store for wallet data.
+let mut db =
+    bdk_file_store::Store::<ChangeSet>::open_or_create_new(b"magic_bytes", "/tmp/my_wallet.db")
+        .expect("create store");
+
+// Create a wallet with initial wallet data read from the file store.
+let descriptor = "wpkh(tprv8ZgxMBicQKsPdcAqYBpzAFwU5yxBUo88ggoBqu1qPcHUfSbKK1sKMLmC7EAk438btHQrSdu3jGGQa6PA71nvH5nkDexhLteJqkM4dQmWF9g/84'/1'/0'/0/*)";
+let change_descriptor = "wpkh(tprv8ZgxMBicQKsPdcAqYBpzAFwU5yxBUo88ggoBqu1qPcHUfSbKK1sKMLmC7EAk438btHQrSdu3jGGQa6PA71nvH5nkDexhLteJqkM4dQmWF9g/84'/1'/0'/1/*)";
+let changeset = db.aggregate_changesets().expect("changeset loaded");
+let mut wallet =
+    Wallet::new_or_load(descriptor, change_descriptor, changeset, Network::Testnet)
+        .expect("create or load wallet");
+
+// Get a new address to receive bitcoin.
+let receive_address = wallet.reveal_next_address(KeychainKind::External);
+// Persist staged wallet data changes to the file store.
+let staged_changeset = wallet.take_staged();
+if let Some(changeset) = staged_changeset {
+    db.append_changeset(&changeset)
+        .expect("must commit changes to database");
 }
+println!("Your new receive address is: {}", receive_address.address);
 ```
 
 <!-- ### Sync the balance of a descriptor -->

crates/wallet/src/keys/mod.rs

@@ -20,6 +20,8 @@ use core::marker::PhantomData;
 use core::ops::Deref;
 use core::str::FromStr;
 
+use rand_core::{CryptoRng, RngCore};
+
 use bitcoin::secp256k1::{self, Secp256k1, Signing};
 
 use bitcoin::bip32;
@@ -631,12 +633,23 @@ pub trait GeneratableKey<Ctx: ScriptContext>: Sized {
         entropy: Self::Entropy,
     ) -> Result<GeneratedKey<Self, Ctx>, Self::Error>;
 
-    /// Generate a key given the options with a random entropy
+    /// Generate a key given the options with random entropy.
+    ///
+    /// Uses the thread-local random number generator.
+    #[cfg(feature = "std")]
     fn generate(options: Self::Options) -> Result<GeneratedKey<Self, Ctx>, Self::Error> {
-        use rand::{thread_rng, Rng};
+        Self::generate_with_aux_rand(options, &mut bitcoin::key::rand::thread_rng())
+    }
 
+    /// Generate a key given the options with random entropy.
+    ///
+    /// Uses a provided random number generator (rng).
+    fn generate_with_aux_rand(
+        options: Self::Options,
+        rng: &mut (impl CryptoRng + RngCore),
+    ) -> Result<GeneratedKey<Self, Ctx>, Self::Error> {
         let mut entropy = Self::Entropy::default();
-        thread_rng().fill(entropy.as_mut());
+        rng.fill_bytes(entropy.as_mut());
         Self::generate_with_entropy(options, entropy)
     }
 }
@@ -657,8 +670,20 @@ where
     }
 
     /// Generate a key with the default options and a random entropy
+    ///
+    /// Uses the thread-local random number generator.
+    #[cfg(feature = "std")]
     fn generate_default() -> Result<GeneratedKey<Self, Ctx>, Self::Error> {
-        Self::generate(Default::default())
+        Self::generate_with_aux_rand(Default::default(), &mut bitcoin::key::rand::thread_rng())
+    }
+
+    /// Generate a key with the default options and a random entropy
+    ///
+    /// Uses a provided random number generator (rng).
+    fn generate_default_with_aux_rand(
+        rng: &mut (impl CryptoRng + RngCore),
+    ) -> Result<GeneratedKey<Self, Ctx>, Self::Error> {
+        Self::generate_with_aux_rand(Default::default(), rng)
     }
 }