Skip to content

Get keychain for script#1201

Closed
vladimirfomene wants to merge 2 commits intobitcoindevkit:masterfrom
vladimirfomene:get_keychain_for_script
Closed

Get keychain for script#1201
vladimirfomene wants to merge 2 commits intobitcoindevkit:masterfrom
vladimirfomene:get_keychain_for_script

Conversation

@vladimirfomene
Copy link
Copy Markdown
Contributor

Description

This PR fixes #1042.

Notes to the reviewers

Changelog notice

It adds a method which_keychain_derived that tells us which keychain derived a particular script. This is helpful when trying to figure out if a particular script belongs to our wallet.

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

@vladimirfomene vladimirfomene force-pushed the get_keychain_for_script branch from a6433a1 to bfd1bd4 Compare November 9, 2023 07:36
@vladimirfomene vladimirfomene self-assigned this Nov 9, 2023
@vladimirfomene vladimirfomene force-pushed the get_keychain_for_script branch from bfd1bd4 to 3674bd2 Compare November 9, 2023 07:42
Comment thread crates/bdk/src/wallet/mod.rs Outdated
@vladimirfomene vladimirfomene force-pushed the get_keychain_for_script branch from 3674bd2 to e2ec6ff Compare November 9, 2023 15:10
@notmandatory notmandatory added this to the 1.0.0-alpha.4 milestone Nov 13, 2023
@nondiremanuel nondiremanuel modified the milestones: 1.0.0-alpha.4, 1.0.0 Jan 6, 2024
@notmandatory notmandatory added the api A breaking API change label Jan 16, 2024
let addr = taproot_wallet.get_address(New);
let script = addr.script_pubkey();
let keychain = segwit_wallet.which_keychain_derived(&script);
assert!(keychain.is_none());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to checking the keychain, we could possibly also check the correct spk index is returned from which_keychain_derived.

diff
 fn test_which_keychain_derived_script() {
     let (mut segwit_wallet, _) = get_funded_wallet(get_test_wpkh());
     let addr = segwit_wallet.get_address(New);
     let script = addr.script_pubkey();
     let keychain = segwit_wallet.which_keychain_derived(&script).unwrap();
     assert_eq!(keychain.0, KeychainKind::External);
 
     let (mut taproot_wallet, _) = get_funded_wallet(get_test_tr_single_sig());
     let addr = taproot_wallet.get_address(New);
     let script = addr.script_pubkey();
     let keychain = segwit_wallet.which_keychain_derived(&script);
     assert!(keychain.is_none());
+
+    // find correct derivation index from spk
+    let desc = get_test_tr_single_sig_xprv();
+    let mut wallet = Wallet::new_no_persist(desc, None, Network::Testnet).unwrap();
+    let spk = wallet.get_address(New).address.script_pubkey();
+    let (_k, i) = wallet.which_keychain_derived(&spk).unwrap();
+    assert_eq!(i, &0);
+
+    // reveal more addresses. the last revealed index should now be 10.
+    for _ in 0..10 {
+        let _ = wallet.get_address(New);
+    }
+    let spk = wallet.get_address(Peek(10)).address.script_pubkey();
+    let (_k, i) = wallet.which_keychain_derived(&spk).unwrap();
+    assert_eq!(i, &10);
 }

@evanlinjin
Copy link
Copy Markdown
Member

This is the exact same thing as Wallet::derivation_of_spk:

/// Finds how the wallet derived the script pubkey `spk`.
///
/// Will only return `Some(_)` if the wallet has given out the spk.
pub fn derivation_of_spk(&self, spk: &Script) -> Option<(KeychainKind, u32)> {
self.indexed_graph.index.index_of_spk(spk)
}

@evanlinjin evanlinjin closed this Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api A breaking API change

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

The Wallet.is_mine() function should return K

6 participants