diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 54020bb90..5a91de04e 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -51,7 +51,7 @@ jobs: with: github-token: ${{ secrets.GITHUB_TOKEN }} - name: Upload artifact - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: coverage-report path: coverage-report.html diff --git a/.github/workflows/nightly_docs.yml b/.github/workflows/nightly_docs.yml index 5fa4ecb5d..59b19a591 100644 --- a/.github/workflows/nightly_docs.yml +++ b/.github/workflows/nightly_docs.yml @@ -22,7 +22,7 @@ jobs: env: RUSTDOCFLAGS: '--cfg docsrs -Dwarnings' - name: Upload artifact - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: built-docs path: ./target/doc/* @@ -44,7 +44,7 @@ jobs: - name: Remove old latest run: rm -rf ./docs/.vuepress/public/docs-rs/bdk/nightly/latest - name: Download built docs - uses: actions/download-artifact@v1 + uses: actions/download-artifact@v4 with: name: built-docs path: ./docs/.vuepress/public/docs-rs/bdk/nightly/latest diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 9f12fac4e..25803f139 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -88,7 +88,7 @@ use crate::descriptor::{ use crate::psbt::PsbtUtils; use crate::signer::SignerError; use crate::types::*; -use crate::wallet::coin_selection::Excess::{Change, NoChange}; +use crate::wallet::coin_selection::Excess::{self, Change, NoChange}; use crate::wallet::error::{BuildFeeBumpError, CreateTxError, MiniscriptPsbtError}; use self::coin_selection::Error; @@ -1468,17 +1468,28 @@ impl Wallet { self.preselect_utxos(¶ms, Some(current_height.to_consensus_u32())); // get drain script + let mut drain_index = Option::<(KeychainKind, u32)>::None; let drain_script = match params.drain_to { Some(ref drain_recipient) => drain_recipient.clone(), None => { let change_keychain = self.map_keychain(KeychainKind::Internal); - let ((index, spk), index_changeset) = self + let (index, spk) = self .indexed_graph .index - .next_unused_spk(change_keychain) - .expect("keychain must exist"); - self.indexed_graph.index.mark_used(change_keychain, index); - self.stage.merge(index_changeset.into()); + .unused_keychain_spks(change_keychain) + .next() + .unwrap_or_else(|| { + let (next_index, _) = self + .indexed_graph + .index + .next_index(change_keychain) + .expect("keychain must exist"); + let spk = self + .peek_address(change_keychain, next_index) + .script_pubkey(); + (next_index, spk) + }); + drain_index = Some((change_keychain, index)); spk } }; @@ -1577,6 +1588,18 @@ impl Wallet { params.ordering.sort_tx_with_aux_rand(&mut tx, rng); let psbt = self.complete_transaction(tx, coin_selection.selected, params)?; + + // recording changes to the change keychain + if let (Excess::Change { .. }, Some((keychain, index))) = (excess, drain_index) { + let (_, index_changeset) = self + .indexed_graph + .index + .reveal_to_target(keychain, index) + .expect("must not be None"); + self.stage.merge(index_changeset.into()); + self.mark_used(keychain, index); + } + Ok(psbt) } diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 21f391ca6..2a2a68fa6 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -1392,6 +1392,127 @@ fn test_create_tx_global_xpubs_with_origin() { assert_eq!(psbt.xpub.get(&key), Some(&(fingerprint, path))); } +#[test] +fn test_create_tx_increment_change_index() { + // Test derivation index and unused index of change keychain when creating a transaction + // Cases include wildcard and non-wildcard descriptors with and without an internal keychain + // note the test assumes that the first external address is revealed since we're using + // `receive_output` + struct TestCase { + name: &'static str, + descriptor: &'static str, + change_descriptor: Option<&'static str>, + // amount to send + to_send: u64, + // (derivation index, next unused index) of *change keychain* + expect: (Option, u32), + } + // total wallet funds + let amount = 10_000; + let recipient = Address::from_str("bcrt1q3qtze4ys45tgdvguj66zrk4fu6hq3a3v9pfly5") + .unwrap() + .assume_checked() + .script_pubkey(); + let (desc, change_desc) = get_test_tr_single_sig_xprv_with_change_desc(); + [ + TestCase { + name: "two wildcard, builder error", + descriptor: desc, + change_descriptor: Some(change_desc), + to_send: amount + 1, + // should not use or derive change index + expect: (None, 0), + }, + TestCase { + name: "two wildcard, create change", + descriptor: desc, + change_descriptor: Some(change_desc), + to_send: 5_000, + // should use change index + expect: (Some(0), 1), + }, + TestCase { + name: "two wildcard, no change", + descriptor: desc, + change_descriptor: Some(change_desc), + to_send: 9_850, + // should not use change index + expect: (None, 0), + }, + TestCase { + name: "one wildcard, create change", + descriptor: desc, + change_descriptor: None, + to_send: 5_000, + // should use change index of external keychain + expect: (Some(1), 2), + }, + TestCase { + name: "one wildcard, no change", + descriptor: desc, + change_descriptor: None, + to_send: 9_850, + // should not use change index + expect: (Some(0), 1), + }, + TestCase { + name: "single key, create change", + descriptor: get_test_tr_single_sig(), + change_descriptor: None, + to_send: 5_000, + // single key only has one derivation index (0) + expect: (Some(0), 0), + }, + TestCase { + name: "single key, no change", + descriptor: get_test_tr_single_sig(), + change_descriptor: None, + to_send: 9_850, + expect: (Some(0), 0), + }, + ] + .into_iter() + .for_each(|test| { + // create wallet + let (params, change_keychain) = match test.change_descriptor { + Some(change_desc) => ( + Wallet::create(test.descriptor, change_desc), + KeychainKind::Internal, + ), + None => ( + Wallet::create_single(test.descriptor), + KeychainKind::External, + ), + }; + let mut wallet = params + .network(Network::Regtest) + .create_wallet_no_persist() + .unwrap(); + // fund wallet + receive_output(&mut wallet, amount, ConfirmationTime::unconfirmed(0)); + // create tx + let mut builder = wallet.build_tx(); + builder.add_recipient(recipient.clone(), Amount::from_sat(test.to_send)); + let res = builder.finish(); + if !test.name.contains("error") { + assert!(res.is_ok()); + } + let (exp_derivation_index, exp_next_unused) = test.expect; + assert_eq!( + wallet.derivation_index(change_keychain), + exp_derivation_index, + "derivation index test {}", + test.name, + ); + assert_eq!( + wallet.next_unused_address(change_keychain).index, + exp_next_unused, + "next unused index test {}", + test.name, + ); + }); +} + #[test] fn test_add_foreign_utxo() { let (mut wallet1, _) = get_funded_wallet_wpkh();