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 67cbb329f..9bd5450d4 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 693378401..095de314d 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 4c40ce734..b54d45db0 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 5bfff3aa9..c51ed36b8 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 72f8f4876..a2cf824c6 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 18d1ff1bf..deaef892f 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 263a0fa86..8eaf34c96 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 b2a359608..7e2351a09 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);