From dd97143463111b49126b4e6100f5b80cb323df11 Mon Sep 17 00:00:00 2001 From: Steve Myers Date: Wed, 18 Jun 2025 21:44:25 -0500 Subject: [PATCH] feat!: Add #[non_exhaustive] to ChangeSet structs in tx_graph, keychain_txout, local_chain Add #[non_exhaustive] attribute to the ChangeSet structs to prevent downstream crates from exhaustively matching or constructing instances using struct literal syntax, allowing future additions without breaking compatibility. The change improves forward compatibility by allowing new fields to be added without breaking downstream code in future releases. --- crates/bitcoind_rpc/tests/test_emitter.rs | 9 +- crates/chain/src/indexed_tx_graph.rs | 1 + crates/chain/src/indexer/keychain_txout.rs | 1 + crates/chain/src/local_chain.rs | 1 + crates/chain/src/tx_graph.rs | 2 + crates/chain/tests/test_indexed_tx_graph.rs | 60 ++++---- .../chain/tests/test_keychain_txout_index.rs | 38 ++--- crates/chain/tests/test_tx_graph.rs | 141 +++++++----------- 8 files changed, 108 insertions(+), 145 deletions(-) diff --git a/crates/bitcoind_rpc/tests/test_emitter.rs b/crates/bitcoind_rpc/tests/test_emitter.rs index 67cbb329fb..9bd5450d40 100644 --- a/crates/bitcoind_rpc/tests/test_emitter.rs +++ b/crates/bitcoind_rpc/tests/test_emitter.rs @@ -103,11 +103,10 @@ pub fn test_sync_local_chain() -> anyhow::Result<()> { assert_eq!( local_chain.apply_update(emission.checkpoint,)?, if exp_height == exp_hashes.len() - reorged_blocks.len() { - bdk_chain::local_chain::ChangeSet { - blocks: core::iter::once((height, Some(hash))) - .chain((height + 1..exp_hashes.len() as u32).map(|h| (h, None))) - .collect(), - } + bdk_chain::local_chain::ChangeSet::from( + core::iter::once((height, Some(hash))) + .chain((height + 1..exp_hashes.len() as u32).map(|h| (h, None))), + ) } else { [(height, Some(hash))].into() }, diff --git a/crates/chain/src/indexed_tx_graph.rs b/crates/chain/src/indexed_tx_graph.rs index 6933784017..095de314d7 100644 --- a/crates/chain/src/indexed_tx_graph.rs +++ b/crates/chain/src/indexed_tx_graph.rs @@ -471,6 +471,7 @@ impl AsRef> for IndexedTxGraph { )) )] #[must_use] +#[non_exhaustive] pub struct ChangeSet { /// [`TxGraph`] changeset. pub tx_graph: tx_graph::ChangeSet, diff --git a/crates/chain/src/indexer/keychain_txout.rs b/crates/chain/src/indexer/keychain_txout.rs index 4c40ce7344..b54d45db08 100644 --- a/crates/chain/src/indexer/keychain_txout.rs +++ b/crates/chain/src/indexer/keychain_txout.rs @@ -1047,6 +1047,7 @@ impl core::error::Error for InsertDesc #[derive(Clone, Debug, Default, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[must_use] +#[non_exhaustive] pub struct ChangeSet { /// Maps each `DescriptorId` to its last revealed derivation index. pub last_revealed: BTreeMap, diff --git a/crates/chain/src/local_chain.rs b/crates/chain/src/local_chain.rs index 5bfff3aa91..c51ed36b8c 100644 --- a/crates/chain/src/local_chain.rs +++ b/crates/chain/src/local_chain.rs @@ -432,6 +432,7 @@ where /// The [`ChangeSet`] represents changes to [`LocalChain`]. #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] +#[non_exhaustive] pub struct ChangeSet { /// Changes to the [`LocalChain`] blocks. /// diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index 72f8f4876f..a2cf824c62 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -169,6 +169,7 @@ impl From> for TxGraph { /// /// [module-level documentation]: crate::tx_graph #[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] pub struct TxGraph { txs: HashMap, spends: BTreeMap>, @@ -1056,6 +1057,7 @@ impl TxGraph { )) )] #[must_use] +#[non_exhaustive] pub struct ChangeSet { /// Added transactions. pub txs: BTreeSet>, diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 18d1ff1bf4..deaef892f6 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -261,26 +261,24 @@ fn insert_relevant_txs() { let txs = [tx_c, tx_b, tx_a]; - let changeset = indexed_tx_graph::ChangeSet { - tx_graph: tx_graph::ChangeSet { - txs: txs.iter().cloned().map(Arc::new).collect(), - ..Default::default() - }, - indexer: keychain_txout::ChangeSet { - last_revealed: [(descriptor.descriptor_id(), 9_u32)].into(), - spk_cache: [(descriptor.descriptor_id(), { - let index_after_spk_1 = 9 /* index of spk_1 */ + 1; - SpkIterator::new_with_range( - &descriptor, - // This will also persist the staged spk cache inclusions from prev call to - // `.insert_descriptor`. - 0..index_after_spk_1 + lookahead, - ) - .collect() - })] - .into(), - }, - }; + let mut tx_graph_changeset = tx_graph::ChangeSet::default(); + tx_graph_changeset.txs = txs.iter().cloned().map(Arc::new).collect(); + + let mut indexer_changeset = keychain_txout::ChangeSet::default(); + indexer_changeset.last_revealed = [(descriptor.descriptor_id(), 9_u32)].into(); + indexer_changeset.spk_cache = [(descriptor.descriptor_id(), { + let index_after_spk_1 = 9 /* index of spk_1 */ + 1; + SpkIterator::new_with_range( + &descriptor, + // This will also persist the staged spk cache inclusions from prev call to + // `.insert_descriptor`. + 0..index_after_spk_1 + lookahead, + ) + .collect() + })] + .into(); + + let changeset = indexed_tx_graph::ChangeSet::from((tx_graph_changeset, indexer_changeset)); assert_eq!( graph.batch_insert_relevant(txs.iter().cloned().map(|tx| (tx, None))), @@ -288,18 +286,16 @@ fn insert_relevant_txs() { ); // The initial changeset will also contain info about the keychain we added - let initial_changeset = indexed_tx_graph::ChangeSet { - tx_graph: changeset.tx_graph, - indexer: keychain_txout::ChangeSet { - last_revealed: changeset.indexer.last_revealed, - spk_cache: [( - descriptor.descriptor_id(), - SpkIterator::new_with_range(&descriptor, 0..=9 /* index of spk_1*/ + lookahead) - .collect(), - )] - .into(), - }, - }; + let mut initial_indexer_changeset = keychain_txout::ChangeSet::default(); + initial_indexer_changeset.last_revealed = changeset.indexer.last_revealed; + initial_indexer_changeset.spk_cache = [( + descriptor.descriptor_id(), + SpkIterator::new_with_range(&descriptor, 0..=9 /* index of spk_1*/ + lookahead).collect(), + )] + .into(); + + let initial_changeset = + indexed_tx_graph::ChangeSet::from((changeset.tx_graph, initial_indexer_changeset)); assert_eq!(graph.initial_changeset(), initial_changeset); } diff --git a/crates/chain/tests/test_keychain_txout_index.rs b/crates/chain/tests/test_keychain_txout_index.rs index 263a0fa86f..8eaf34c966 100644 --- a/crates/chain/tests/test_keychain_txout_index.rs +++ b/crates/chain/tests/test_keychain_txout_index.rs @@ -89,14 +89,12 @@ fn merge_changesets_check_last_revealed() { rhs_di.insert(descriptor_ids[1], 5); // value more than lhs desc 1 lhs_di.insert(descriptor_ids[3], 4); // key doesn't exist in lhs - let mut lhs = ChangeSet { - last_revealed: lhs_di, - ..Default::default() - }; - let rhs = ChangeSet { - last_revealed: rhs_di, - ..Default::default() - }; + let mut lhs = ChangeSet::default(); + lhs.last_revealed = lhs_di; + + let mut rhs = ChangeSet::default(); + rhs.last_revealed = rhs_di; + lhs.merge(rhs); // Existing index doesn't update if the new index in `other` is lower than `self`. @@ -138,12 +136,13 @@ fn test_set_all_derivation_indices() { ), ] .into(); + + let mut expected_changeset = ChangeSet::default(); + expected_changeset.last_revealed = last_revealed.clone(); + expected_changeset.spk_cache = spk_cache.clone(); assert_eq!( txout_index.reveal_to_target_multi(&derive_to), - ChangeSet { - last_revealed: last_revealed.clone(), - spk_cache: spk_cache.clone(), - } + expected_changeset ); assert_eq!(txout_index.last_revealed_indices(), derive_to); assert_eq!( @@ -638,16 +637,11 @@ fn lookahead_to_target() { #[test] fn applying_changesets_one_by_one_vs_aggregate_must_have_same_result() { let desc = parse_descriptor(DESCRIPTORS[0]); - let changesets: &[ChangeSet] = &[ - ChangeSet { - last_revealed: [(desc.descriptor_id(), 10)].into(), - ..Default::default() - }, - ChangeSet { - last_revealed: [(desc.descriptor_id(), 12)].into(), - ..Default::default() - }, - ]; + let mut changeset_1 = ChangeSet::default(); + changeset_1.last_revealed = [(desc.descriptor_id(), 10)].into(); + let mut changeset_2 = ChangeSet::default(); + changeset_2.last_revealed = [(desc.descriptor_id(), 12)].into(); + let changesets: &[ChangeSet] = &[changeset_1, changeset_2]; let mut indexer_a = KeychainTxOutIndex::::new(0, true); let _ = indexer_a diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index b2a3596085..7e2351a093 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -78,12 +78,11 @@ fn insert_txouts() { let mut graph = { let mut graph = TxGraph::::default(); for (outpoint, txout) in &original_ops { + let mut expected_changeset = ChangeSet::default(); + expected_changeset.txouts.insert(*outpoint, txout.clone()); assert_eq!( graph.insert_txout(*outpoint, txout.clone()), - ChangeSet { - txouts: [(*outpoint, txout.clone())].into(), - ..Default::default() - } + expected_changeset ); } graph @@ -110,18 +109,17 @@ fn insert_txouts() { // Check the resulting addition. let changeset = graph.apply_update(update); - - assert_eq!( - changeset, - ChangeSet { - txs: [Arc::new(update_tx.clone())].into(), - txouts: update_ops.clone().into(), - anchors: [(conf_anchor, update_tx.compute_txid()),].into(), - first_seen: [(hash!("tx2"), 1000000)].into(), - last_seen: [(hash!("tx2"), 1000000)].into(), - last_evicted: [].into(), - } - ); + let mut expected_changeset = ChangeSet::default(); + expected_changeset.txs.insert(Arc::new(update_tx.clone())); + expected_changeset.txouts.extend(update_ops.iter().cloned()); + expected_changeset + .anchors + .insert((conf_anchor, update_tx.compute_txid())); + expected_changeset + .first_seen + .insert(hash!("tx2"), 1_000_000); + expected_changeset.last_seen.insert(hash!("tx2"), 1_000_000); + assert_eq!(changeset, expected_changeset,); // Apply changeset and check the new graph counts. graph.apply_changeset(changeset); @@ -166,17 +164,20 @@ fn insert_txouts() { ); // Check that the initial_changeset is correct - assert_eq!( - graph.initial_changeset(), - ChangeSet { - txs: [Arc::new(update_tx.clone())].into(), - txouts: update_ops.into_iter().chain(original_ops).collect(), - anchors: [(conf_anchor, update_tx.compute_txid()),].into(), - first_seen: [(hash!("tx2"), 1000000)].into(), - last_seen: [(hash!("tx2"), 1000000)].into(), - last_evicted: [].into(), - } - ); + let mut expected_changeset = ChangeSet::default(); + expected_changeset.txs.insert(Arc::new(update_tx.clone())); + expected_changeset + .txouts + .extend(update_ops.into_iter().chain(original_ops)); + expected_changeset + .anchors + .insert((conf_anchor, update_tx.compute_txid())); + expected_changeset + .first_seen + .insert(hash!("tx2"), 1_000_000); + expected_changeset.last_seen.insert(hash!("tx2"), 1_000_000); + + assert_eq!(graph.initial_changeset(), expected_changeset,); } #[test] @@ -324,20 +325,12 @@ fn insert_tx_witness_precedence() { let mut tx_graph = TxGraph::::default(); let changeset_insert_unsigned = tx_graph.insert_tx(unsigned_tx.clone()); let changeset_insert_signed = tx_graph.insert_tx(signed_tx.clone()); - assert_eq!( - changeset_insert_unsigned, - ChangeSet { - txs: [Arc::new(unsigned_tx.clone())].into(), - ..Default::default() - } - ); - assert_eq!( - changeset_insert_signed, - ChangeSet { - txs: [Arc::new(signed_tx.clone())].into(), - ..Default::default() - } - ); + let mut expected_changeset = ChangeSet::default(); + expected_changeset.txs.insert(Arc::new(unsigned_tx.clone())); + assert_eq!(changeset_insert_unsigned, expected_changeset,); + expected_changeset = ChangeSet::default(); + expected_changeset.txs.insert(Arc::new(signed_tx.clone())); + assert_eq!(changeset_insert_signed, expected_changeset); } // Unsigned tx must not displace signed. @@ -345,13 +338,9 @@ fn insert_tx_witness_precedence() { let mut tx_graph = TxGraph::::default(); let changeset_insert_signed = tx_graph.insert_tx(signed_tx.clone()); let changeset_insert_unsigned = tx_graph.insert_tx(unsigned_tx.clone()); - assert_eq!( - changeset_insert_signed, - ChangeSet { - txs: [Arc::new(signed_tx)].into(), - ..Default::default() - } - ); + let mut expected_changeset = ChangeSet::default(); + expected_changeset.txs.insert(Arc::new(signed_tx)); + assert_eq!(changeset_insert_signed, expected_changeset); assert!(changeset_insert_unsigned.is_empty()); } @@ -402,13 +391,9 @@ fn insert_tx_witness_precedence() { let mut tx_graph = TxGraph::::default(); let changeset_small = tx_graph.insert_tx(tx_small.clone()); let changeset_large = tx_graph.insert_tx(tx_large); - assert_eq!( - changeset_small, - ChangeSet { - txs: [Arc::new(tx_small.clone())].into(), - ..Default::default() - } - ); + let mut expected_changeset = ChangeSet::default(); + expected_changeset.txs.insert(Arc::new(tx_small.clone())); + assert_eq!(changeset_small, expected_changeset,); assert!(changeset_large.is_empty()); let tx = tx_graph .get_tx(tx_small.compute_txid()) @@ -1161,15 +1146,11 @@ fn test_changeset_last_seen_merge() { ]; for (original_ls, update_ls) in test_cases { - let mut original = ChangeSet::<()> { - last_seen: original_ls.map(|ls| (txid, ls)).into_iter().collect(), - ..Default::default() - }; + let mut original = ChangeSet::<()>::default(); + original.last_seen = original_ls.map(|ls| (txid, ls)).into_iter().collect(); assert!(!original.is_empty() || original_ls.is_none()); - let update = ChangeSet::<()> { - last_seen: update_ls.map(|ls| (txid, ls)).into_iter().collect(), - ..Default::default() - }; + let mut update = ChangeSet::<()>::default(); + update.last_seen = update_ls.map(|ls| (txid, ls)).into_iter().collect(); assert!(!update.is_empty() || update_ls.is_none()); original.merge(update); @@ -1470,39 +1451,27 @@ fn tx_graph_update_conversion() { #[test] fn test_seen_at_updates() { // Update both first_seen and last_seen - let seen_at = 1000000_u64; + let seen_at = 1_000_000_u64; let mut graph = TxGraph::::default(); let mut changeset = graph.insert_seen_at(hash!("tx1"), seen_at); - assert_eq!( - changeset, - ChangeSet { - first_seen: [(hash!("tx1"), 1000000)].into(), - last_seen: [(hash!("tx1"), 1000000)].into(), - ..Default::default() - } - ); + let mut expected_changeset = ChangeSet::default(); + expected_changeset.first_seen = [(hash!("tx1"), 1_000_000)].into(); + expected_changeset.last_seen = [(hash!("tx1"), 1_000_000)].into(); + assert_eq!(changeset, expected_changeset,); // Update first_seen but not last_seen let earlier_seen_at = 999_999_u64; changeset = graph.insert_seen_at(hash!("tx1"), earlier_seen_at); - assert_eq!( - changeset, - ChangeSet { - first_seen: [(hash!("tx1"), 999999)].into(), - ..Default::default() - } - ); + let mut expected_changeset = ChangeSet::default(); + expected_changeset.first_seen = [(hash!("tx1"), 999_999)].into(); + assert_eq!(changeset, expected_changeset); // Update last_seen but not first_seen let later_seen_at = 1_000_001_u64; changeset = graph.insert_seen_at(hash!("tx1"), later_seen_at); - assert_eq!( - changeset, - ChangeSet { - last_seen: [(hash!("tx1"), 1000001)].into(), - ..Default::default() - } - ); + let mut expected_changeset = ChangeSet::default(); + expected_changeset.last_seen = [(hash!("tx1"), 1000001)].into(); + assert_eq!(changeset, expected_changeset); // Should not change anything changeset = graph.insert_seen_at(hash!("tx1"), 1000000);