frost/bdk
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(keychain): Fix KeychainTxOutIndex range queries

Browse Source 
The underlying SpkTxOutIndex should not use DescriptorIds to index
because this loses the ordering relationship of the spks so queries on
subranges of keychains work.

Along with that we enforce that there is a strict 1-to-1 relationship
between descriptors and keychains. Violating this leads to an error in
insert_descriptor now.

In general I try to make the translation layer between the SpkTxOutIndex
and the KeychainTxOutIndex thinner. Ergonomics of this will be improved
in next commit.

The test from the previous commit passes.
This commit is contained in:
LLFourn
2024-06-06 10:17:55 +10:00
committed by 志宇
parent 3b2ff0cc95
commit bc2a8be979
12 changed files with 441 additions and 467 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
crates/chain/tests/test_indexed_tx_graph.rs
View File

@@ -10,7 +10,7 @@ use bdk_chain::{
indexed_tx_graph::{self, IndexedTxGraph},
keychain::{self, Balance, KeychainTxOutIndex},
local_chain::LocalChain,
tx_graph, ChainPosition, ConfirmationHeightAnchor, DescriptorExt,
tx_graph, Append, ChainPosition, ConfirmationHeightAnchor, DescriptorExt,
};
use bitcoin::{
secp256k1::Secp256k1, Amount, OutPoint, Script, ScriptBuf, Transaction, TxIn, TxOut,
@@ -140,8 +140,16 @@ fn test_list_owned_txouts() {
KeychainTxOutIndex::new(10),
);
let _ = graph.index.insert_descriptor("keychain_1".into(), desc_1);
let _ = graph.index.insert_descriptor("keychain_2".into(), desc_2);
assert!(!graph
.index
.insert_descriptor("keychain_1".into(), desc_1)
.unwrap()
.is_empty());
assert!(!graph
.index
.insert_descriptor("keychain_2".into(), desc_2)
.unwrap()
.is_empty());
// Get trusted and untrusted addresses
@@ -257,18 +265,26 @@ fn test_list_owned_txouts() {
.unwrap_or_else(|| panic!("block must exist at {}", height));
let txouts = graph
.graph()
.filter_chain_txouts(&local_chain, chain_tip, graph.index.outpoints())
.filter_chain_txouts(
&local_chain,
chain_tip,
graph.index.outpoints().iter().cloned(),
)
.collect::<Vec<_>>();
let utxos = graph
.graph()
.filter_chain_unspents(&local_chain, chain_tip, graph.index.outpoints())
.filter_chain_unspents(
&local_chain,
chain_tip,
graph.index.outpoints().iter().cloned(),
)
.collect::<Vec<_>>();
let balance = graph.graph().balance(
&local_chain,
chain_tip,
graph.index.outpoints(),
graph.index.outpoints().iter().cloned(),
|_, spk: &Script| trusted_spks.contains(&spk.to_owned()),
);

143
crates/chain/tests/test_keychain_txout_index.rs
View File

@@ -98,7 +98,7 @@ fn append_changesets_check_last_revealed() {
}
#[test]
fn test_apply_changeset_with_different_descriptors_to_same_keychain() {
fn when_apply_contradictory_changesets_they_are_ignored() {
let external_descriptor = parse_descriptor(DESCRIPTORS[0]);
let internal_descriptor = parse_descriptor(DESCRIPTORS[1]);
let mut txout_index =
@@ -120,7 +120,7 @@ fn test_apply_changeset_with_different_descriptors_to_same_keychain() {
assert_eq!(
txout_index.keychains().collect::<Vec<_>>(),
vec![
(&TestKeychain::External, &internal_descriptor),
(&TestKeychain::External, &external_descriptor),
(&TestKeychain::Internal, &internal_descriptor)
]
);
@@ -134,8 +134,8 @@ fn test_apply_changeset_with_different_descriptors_to_same_keychain() {
assert_eq!(
txout_index.keychains().collect::<Vec<_>>(),
vec![
(&TestKeychain::External, &internal_descriptor),
(&TestKeychain::Internal, &external_descriptor)
(&TestKeychain::External, &external_descriptor),
(&TestKeychain::Internal, &internal_descriptor)
]
);
}
@@ -156,7 +156,7 @@ fn test_set_all_derivation_indices() {
]
.into();
assert_eq!(
txout_index.reveal_to_target_multi(&derive_to).1,
txout_index.reveal_to_target_multi(&derive_to),
ChangeSet {
keychains_added: BTreeMap::new(),
last_revealed: last_revealed.clone()
@@ -164,7 +164,7 @@ fn test_set_all_derivation_indices() {
);
assert_eq!(txout_index.last_revealed_indices(), derive_to);
assert_eq!(
txout_index.reveal_to_target_multi(&derive_to).1,
txout_index.reveal_to_target_multi(&derive_to),
keychain::ChangeSet::default(),
"no changes if we set to the same thing"
);
@@ -190,7 +190,7 @@ fn test_lookahead() {
.reveal_to_target(&TestKeychain::External, index)
.unwrap();
assert_eq!(
revealed_spks.collect::<Vec<_>>(),
revealed_spks,
vec![(index, spk_at_index(&external_descriptor, index))],
);
assert_eq!(
@@ -241,7 +241,7 @@ fn test_lookahead() {
.reveal_to_target(&TestKeychain::Internal, 24)
.unwrap();
assert_eq!(
revealed_spks.collect::<Vec<_>>(),
revealed_spks,
(0..=24)
.map(|index| (index, spk_at_index(&internal_descriptor, index)))
.collect::<Vec<_>>(),
@@ -511,7 +511,7 @@ fn test_non_wildcard_derivations() {
let (revealed_spks, revealed_changeset) = txout_index
.reveal_to_target(&TestKeychain::External, 200)
.unwrap();
assert_eq!(revealed_spks.count(), 0);
assert_eq!(revealed_spks.len(), 0);
assert!(revealed_changeset.is_empty());
// we check that spks_of_keychain returns a SpkIterator with just one element
@@ -591,19 +591,17 @@ fn lookahead_to_target() {
let keychain_test_cases = [
(
external_descriptor.descriptor_id(),
TestKeychain::External,
t.external_last_revealed,
t.external_target,
),
(
internal_descriptor.descriptor_id(),
TestKeychain::Internal,
t.internal_last_revealed,
t.internal_target,
),
];
for (descriptor_id, keychain, last_revealed, target) in keychain_test_cases {
for (keychain, last_revealed, target) in keychain_test_cases {
if let Some(target) = target {
let original_last_stored_index = match last_revealed {
Some(last_revealed) => Some(last_revealed + t.lookahead),
@@ -619,10 +617,10 @@ fn lookahead_to_target() {
let keys = index
.inner()
.all_spks()
.range((descriptor_id, 0)..=(descriptor_id, u32::MAX))
.map(|(k, _)| *k)
.range((keychain.clone(), 0)..=(keychain.clone(), u32::MAX))
.map(|(k, _)| k.clone())
.collect::<Vec<_>>();
let exp_keys = core::iter::repeat(descriptor_id)
let exp_keys = core::iter::repeat(keychain)
.zip(0_u32..=exp_last_stored_index)
.collect::<Vec<_>>();
assert_eq!(keys, exp_keys);
@@ -631,50 +629,6 @@ fn lookahead_to_target() {
}
}
/// `::index_txout` should still index txouts with spks derived from descriptors without keychains.
/// This includes properly refilling the lookahead for said descriptors.
#[test]
fn index_txout_after_changing_descriptor_under_keychain() {
let secp = bdk_chain::bitcoin::secp256k1::Secp256k1::signing_only();
let (desc_a, _) = Descriptor::<DescriptorPublicKey>::parse_descriptor(&secp, DESCRIPTORS[0])
.expect("descriptor 0 must be valid");
let (desc_b, _) = Descriptor::<DescriptorPublicKey>::parse_descriptor(&secp, DESCRIPTORS[1])
.expect("descriptor 1 must be valid");
let desc_id_a = desc_a.descriptor_id();
let mut txout_index = bdk_chain::keychain::KeychainTxOutIndex::<()>::new(10);
// Introduce `desc_a` under keychain `()` and replace the descriptor.
let _ = txout_index.insert_descriptor((), desc_a.clone());
let _ = txout_index.insert_descriptor((), desc_b.clone());
// Loop through spks in intervals of `lookahead` to create outputs with. We should always be
// able to index these outputs if `lookahead` is respected.
let spk_indices = [9, 19, 29, 39];
for i in spk_indices {
let spk_at_index = desc_a
.at_derivation_index(i)
.expect("must derive")
.script_pubkey();
let index_changeset = txout_index.index_txout(
// Use spk derivation index as vout as we just want an unique outpoint.
OutPoint::new(h!("mock_tx"), i as _),
&TxOut {
value: Amount::from_sat(10_000),
script_pubkey: spk_at_index,
},
);
assert_eq!(
index_changeset,
bdk_chain::keychain::ChangeSet {
keychains_added: BTreeMap::default(),
last_revealed: [(desc_id_a, i)].into(),
},
"must always increase last active if impl respects lookahead"
);
}
}
#[test]
fn insert_descriptor_no_change() {
let secp = Secp256k1::signing_only();
@@ -683,19 +637,20 @@ fn insert_descriptor_no_change() {
let mut txout_index = KeychainTxOutIndex::<()>::default();
assert_eq!(
txout_index.insert_descriptor((), desc.clone()),
keychain::ChangeSet {
Ok(keychain::ChangeSet {
keychains_added: [((), desc.clone())].into(),
last_revealed: Default::default()
},
}),
);
assert_eq!(
txout_index.insert_descriptor((), desc.clone()),
keychain::ChangeSet::default(),
Ok(keychain::ChangeSet::default()),
"inserting the same descriptor for keychain should return an empty changeset",
);
}
#[test]
#[cfg(not(debug_assertions))]
fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() {
let desc = parse_descriptor(DESCRIPTORS[0]);
let changesets: &[ChangeSet<TestKeychain>] = &[
@@ -743,39 +698,25 @@ fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() {
);
}
// When the same descriptor is associated with various keychains,
// index methods only return the highest keychain by Ord
#[test]
fn test_only_highest_ord_keychain_is_returned() {
fn assigning_same_descriptor_to_multiple_keychains_should_error() {
let desc = parse_descriptor(DESCRIPTORS[0]);
let mut indexer = KeychainTxOutIndex::<TestKeychain>::new(0);
let _ = indexer.insert_descriptor(TestKeychain::Internal, desc.clone());
let _ = indexer.insert_descriptor(TestKeychain::External, desc);
assert!(indexer
.insert_descriptor(TestKeychain::External, desc)
.is_err())
}
// reveal_next_spk will work with either keychain
let spk0: ScriptBuf = indexer
.reveal_next_spk(&TestKeychain::External)
.unwrap()
.0
.1
.into();
let spk1: ScriptBuf = indexer
.reveal_next_spk(&TestKeychain::Internal)
.unwrap()
.0
.1
.into();
// index_of_spk will always return External
assert_eq!(
indexer.index_of_spk(&spk0),
Some((TestKeychain::External, 0))
);
assert_eq!(
indexer.index_of_spk(&spk1),
Some((TestKeychain::External, 1))
);
#[test]
fn reassigning_keychain_to_a_new_descriptor_should_error() {
let desc1 = parse_descriptor(DESCRIPTORS[0]);
let desc2 = parse_descriptor(DESCRIPTORS[1]);
let mut indexer = KeychainTxOutIndex::<TestKeychain>::new(0);
let _ = indexer.insert_descriptor(TestKeychain::Internal, desc1);
assert!(indexer
.insert_descriptor(TestKeychain::Internal, desc2)
.is_err());
}
#[test]
@@ -786,27 +727,29 @@ fn when_querying_over_a_range_of_keychains_the_utxos_should_show_up() {
for (i, descriptor) in DESCRIPTORS.iter().enumerate() {
let descriptor = parse_descriptor(descriptor);
let _ = indexer.insert_descriptor(i, descriptor.clone());
indexer.reveal_next_spk(&i);
if i != 4 {
// skip one in the middle to see if uncovers any bugs
indexer.reveal_next_spk(&i);
}
tx.output.push(TxOut {
script_pubkey: descriptor.at_derivation_index(0).unwrap().script_pubkey(),
value: Amount::from_sat(10_000),
});
}
let _ = indexer.index_tx(&tx);
assert_eq!(indexer.outpoints().count(), DESCRIPTORS.len());
let n_spks = DESCRIPTORS.len() - /*we skipped one*/ 1;
assert_eq!(
indexer.revealed_spks(0..DESCRIPTORS.len()).count(),
DESCRIPTORS.len()
);
let _ = indexer.index_tx(&tx);
assert_eq!(indexer.outpoints().len(), n_spks);
assert_eq!(indexer.revealed_spks(0..DESCRIPTORS.len()).count(), n_spks);
assert_eq!(indexer.revealed_spks(1..4).count(), 4 - 1);
assert_eq!(
indexer.net_value(&tx, 0..DESCRIPTORS.len()).to_sat(),
(10_000 * DESCRIPTORS.len()) as i64
(10_000 * n_spks) as i64
);
assert_eq!(
indexer.net_value(&tx, 3..5).to_sat(),
(10_000 * (5 - 3)) as i64
indexer.net_value(&tx, 3..6).to_sat(),
(10_000 * (6 - 3 - /*the skipped one*/ 1)) as i64
);
}