Merge branch 'fix/sweep-only-added-inputs' of https://github.com/dashpay/rust-dashcore into fix/sweep-only-added-inputs

Author: jeanpierreroma

dash-spv/src/sync/filters/manager.rs

@@ -970,11 +970,7 @@ impl<H: BlockHeaderStorage, FH: FilterHeaderStorage, F: FilterStorage, W: Wallet
         let mut wallet_states: Vec<WalletScanState> = Vec::new();
         for wallet_id in &behind {
             let synced = wallet.wallet_synced_height(wallet_id);
-            // The scan query, not the full monitored set: spent single-use
-            // (CoinJoin) addresses are pruned so the per-filter match cost
-            // stays bounded by active UTXOs + gap lookahead instead of
-            // growing with every historical mixing round
-            // (dashpay/rust-dashcore#948).
+            // The scan query, which equals the monitored set today.
             let scripts = wallet.scan_script_pubkeys_for(wallet_id);
             // Bare owner/voting key hashes a compact filter carries beyond the
             // wallet's scriptPubKeys.
@@ -2188,8 +2184,7 @@ mod tests {
 
     /// `scan_batch` matches filters against the wallet's scan query
     /// (`scan_script_pubkeys_for`), not the full monitored set: a monitored
-    /// script pruned from the scan query — a spent single-use CoinJoin
-    /// address (dashpay/rust-dashcore#948) — must not pull its block in.
+    /// script the scan query does not carry must not pull its block in.
     #[tokio::test]
     async fn test_scan_batch_uses_pruned_scan_query() {
         let wallet_id: WalletId = [0x03; 32];

key-wallet-manager/benches/filter_scan.rs

@@ -1,16 +1,12 @@
-//! Compact-filter matching cost: full monitored set vs the pruned
-//! forward-scan set for a mixing-heavy CoinJoin wallet
-//! (dashpay/rust-dashcore#948).
+//! How compact-filter matching cost grows with the size of a mixing-heavy
+//! CoinJoin wallet's query.
 //!
 //! Mimics a wallet mid-recovery after many mixing rounds. Every CoinJoin
 //! round pays a fresh single-use address, so the account accumulates `used`
 //! spent addresses, keeps a small set of still-funded denominations
 //! ([`LIVE_UTXOS`]), and watches the usual gap-limit lookahead on top. One
 //! scan batch of BIP158 filters is then matched with
-//! `monitored_script_pubkeys_for` (the pre-#948 query, which drags every
-//! historical address through SipHash + sort per filter) and with
-//! `scan_script_pubkeys_for` (the pruned query, bounded by live UTXOs + gap
-//! lookahead).
+//! `monitored_script_pubkeys_for`, the query the scan runs.
 //!
 //! BIP158 keys each filter's SipHashes off the block hash, so the whole
 //! query set is re-hashed and re-sorted per filter — which is exactly why
@@ -142,29 +138,13 @@ fn bench_filter_scan(c: &mut Criterion) {
     for used in USED_ADDRESSES {
         let (manager, wallet_id) = wallet_with_mixing_history(used);
         let monitored = manager.monitored_script_pubkeys_for(&wallet_id);
-        let pruned = manager.scan_script_pubkeys_for(&wallet_id);
-        assert!(
-            pruned.len() < monitored.len(),
-            "the scan query must shrink once CoinJoin addresses are spent"
-        );
-        println!(
-            "used={used}: monitored query = {} scripts, pruned scan query = {} scripts",
-            monitored.len(),
-            pruned.len()
-        );
+        println!("used={used}: query = {} scripts", monitored.len());
 
-        for (name, scripts) in [("monitored", &monitored), ("pruned", &pruned)] {
-            group.bench_with_input(BenchmarkId::new(name, used), scripts, |b, scripts| {
-                b.iter(|| {
-                    check_compact_filters_for_elements(
-                        black_box(&filters),
-                        black_box(scripts),
-                        &[],
-                        0,
-                    )
-                })
-            });
-        }
+        group.bench_with_input(BenchmarkId::new("monitored", used), &monitored, |b, scripts| {
+            b.iter(|| {
+                check_compact_filters_for_elements(black_box(&filters), black_box(scripts), &[], 0)
+            })
+        });
     }
 
     group.finish();

key-wallet-manager/src/process_block.rs

@@ -592,19 +592,59 @@ impl<T: WalletInfoInterface + Send + Sync + 'static> WalletManager<T> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::matching::{check_compact_filters_for_elements, FilterMatchKey};
     use crate::test_helpers::*;
+    use dashcore::bip158::BlockFilter;
     use dashcore::block::{Header, Version};
     use dashcore::hashes::Hash;
     use dashcore::pow::CompactTarget;
     use dashcore::{
         BlockHash, Network, OutPoint, ScriptBuf, TxIn, TxMerkleNode, TxOut, Txid, Witness,
     };
+    use key_wallet::account::ManagedAccountTrait as _;
     use key_wallet::account::StandardAccountType;
     use key_wallet::mnemonic::Language;
     use key_wallet::wallet::initialization::WalletAccountCreationOptions;
     use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference;
     use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo;
     use key_wallet::{AccountType, Mnemonic};
+    use std::collections::HashMap;
+
+    fn coinjoin_account<'a>(
+        manager: &'a WalletManager<ManagedWalletInfo>,
+        wallet_id: &WalletId,
+    ) -> &'a key_wallet::managed_account::ManagedCoreFundsAccount {
+        manager
+            .get_wallet_info(wallet_id)
+            .expect("wallet info")
+            .accounts
+            .coinjoin_accounts
+            .get(&0)
+            .expect("CoinJoin account 0")
+    }
+
+    fn spend_first_output_of(tx: &Transaction) -> Transaction {
+        Transaction {
+            version: 2,
+            lock_time: 0,
+            input: vec![TxIn {
+                previous_output: OutPoint {
+                    txid: tx.txid(),
+                    vout: 0,
+                },
+                script_sig: ScriptBuf::new(),
+                sequence: u32::MAX,
+                witness: Witness::default(),
+            }],
+            output: vec![TxOut {
+                value: tx.output[0].value,
+                script_pubkey: ScriptBuf::new_p2pkh(&dashcore::PubkeyHash::from_byte_array(
+                    [0x77; 20],
+                )),
+            }],
+            special_transaction_payload: None,
+        }
+    }
 
     fn make_block(txdata: Vec<Transaction>) -> Block {
         Block {
@@ -820,32 +860,57 @@ mod tests {
         );
     }
 
+    /// A CoinJoin address used and left with no unspent output was once
+    /// dropped from the filter scan as unpayable. Mainnet pays such addresses
+    /// again, and the scan then never matched the block carrying the payment.
     #[tokio::test]
-    async fn test_scan_script_pubkeys_for_prunes_spent_coinjoin_addresses() {
-        use key_wallet::account::ManagedAccountTrait;
-
+    async fn test_filter_scan_matches_a_second_payment_to_an_emptied_coinjoin_address() {
         let (mut manager, wallet_id, _addr) = setup_manager_with_wallet();
+        let wallets = BTreeSet::from([wallet_id]);
+
+        let coinjoin_addr = coinjoin_account(&manager, &wallet_id)
+            .all_addresses()
+            .first()
+            .cloned()
+            .expect("CoinJoin address");
+
+        let received = create_tx_paying_to(&coinjoin_addr, 0x11);
+        let block = Block::dummy(100, vec![received.clone()]);
+        manager.process_block_for_wallets(&block, block.block_hash(), 100, &wallets).await;
+
+        let spend = spend_first_output_of(&received);
+        let block = Block::dummy(101, vec![spend]);
+        manager.process_block_for_wallets(&block, block.block_hash(), 101, &wallets).await;
 
-        // Untouched wallet: the scan set equals the monitored set.
-        let monitored = manager.monitored_script_pubkeys_for(&wallet_id);
-        assert_eq!(manager.scan_script_pubkeys_for(&wallet_id), monitored);
-
-        // Mark a CoinJoin address used with no unspent output — a spent
-        // single-use address. The scan query drops it; the monitored set
-        // keeps it.
-        let info = manager.get_wallet_info_mut(&wallet_id).expect("wallet info");
-        let coinjoin = info.accounts.coinjoin_accounts.get_mut(&0).expect("CoinJoin account 0");
-        let spent_addr = coinjoin.all_addresses().first().cloned().expect("CoinJoin address");
-        assert!(coinjoin.mark_address_used(&spent_addr));
-
-        let monitored = manager.monitored_script_pubkeys_for(&wallet_id);
-        let scan = manager.scan_script_pubkeys_for(&wallet_id);
-        assert!(monitored.contains(&spent_addr.script_pubkey()));
-        assert!(!scan.contains(&spent_addr.script_pubkey()));
-        assert_eq!(scan.len(), monitored.len() - 1);
-
-        // Unknown wallet id yields an empty scan set.
+        // Used, and holding nothing: the state that used to drop it.
+        assert!(coinjoin_account(&manager, &wallet_id).all_addresses().contains(&coinjoin_addr));
+        assert_eq!(manager.get_wallet_balance(&wallet_id).expect("balance").total(), 0);
+        assert_eq!(
+            manager.scan_script_pubkeys_for(&wallet_id),
+            manager.monitored_script_pubkeys_for(&wallet_id)
+        );
         assert!(manager.scan_script_pubkeys_for(&[0xff; 32]).is_empty());
+
+        // The block carrying the second payment must match the scan query.
+        let later = Block::dummy(102, vec![create_tx_paying_to(&coinjoin_addr, 0x33)]);
+        let filters = HashMap::from([(
+            FilterMatchKey::new(102, later.block_hash()),
+            BlockFilter::dummy(&later),
+        )]);
+        let matched = check_compact_filters_for_elements(
+            &filters,
+            &manager.scan_script_pubkeys_for(&wallet_id),
+            &[],
+            0,
+        );
+        assert_eq!(matched.len(), 1, "the scan query must still watch the emptied address");
+
+        manager.process_block_for_wallets(&later, later.block_hash(), 102, &wallets).await;
+        assert_eq!(
+            manager.get_wallet_balance(&wallet_id).expect("balance").confirmed(),
+            TX_AMOUNT,
+            "the second payment must be credited"
+        );
     }
 
     #[tokio::test]

key-wallet-manager/src/test_utils/mock_wallet.rs

@@ -391,8 +391,8 @@ pub struct MultiMockWallet {
     wallets: std::collections::BTreeMap<WalletId, MockWalletState>,
     /// Per-wallet override for `scan_script_pubkeys_for`. Wallets absent here
     /// fall back to the monitored set, mirroring the trait default. Lets tests
-    /// hand the filter scan a pruned query while the monitored set stays full
-    /// (dashpay/rust-dashcore#948).
+    /// hand the filter scan a query that does not cover everything the wallet
+    /// watches.
     scan_addresses: std::collections::BTreeMap<WalletId, Vec<Address>>,
     event_sender: broadcast::Sender<WalletEvent>,
     /// Track every block processed for assertions.

key-wallet-manager/src/wallet_interface.rs

@@ -91,14 +91,12 @@ pub trait WalletInterface: Send + Sync + 'static {
     /// Get the scriptPubKeys `wallet_id` wants matched during a forward
     /// compact-filter scan.
     ///
-    /// Defaults to [`Self::monitored_script_pubkeys_for`]. Implementations may
-    /// return a subset when some monitored scripts can no longer be paid in
-    /// practice — the managed-wallet implementation drops CoinJoin addresses
-    /// whose outputs are all spent, since those are single-use by protocol and
-    /// their monotonic growth dominates per-filter matching cost late in a
-    /// mixing-heavy recovery scan (dashpay/rust-dashcore#948). Block
-    /// processing still checks transactions against the full monitored set, so
-    /// pruning only narrows which blocks the filter scan downloads.
+    /// Defaults to [`Self::monitored_script_pubkeys_for`], and nothing narrows
+    /// it today — see [`scan_script_pubkeys`]. Block processing always uses
+    /// the full monitored set, so this only narrows which blocks get
+    /// downloaded.
+    ///
+    /// [`scan_script_pubkeys`]: key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface::scan_script_pubkeys
     fn scan_script_pubkeys_for(&self, wallet_id: &WalletId) -> Vec<ScriptBuf> {
         self.monitored_script_pubkeys_for(wallet_id)
     }

key-wallet/src/managed_account/managed_core_funds_account.rs

@@ -30,7 +30,7 @@ use crate::wallet::balance::WalletCoreBalance;
 use crate::{ExtendedPubKey, Network};
 use dashcore::blockdata::transaction::OutPoint;
 use dashcore::prelude::CoreBlockHeight;
-use dashcore::{Address, ScriptBuf, Transaction, Txid};
+use dashcore::{Address, Transaction, Txid};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 use std::collections::BTreeMap;
@@ -206,28 +206,6 @@ impl ManagedCoreFundsAccount {
         }
     }
 
-    /// Cached scriptPubKeys for every address that could still receive or hold
-    /// funds under a single-use address discipline: addresses not yet used
-    /// (the gap-limit lookahead, including reserved ones) plus used addresses
-    /// that still hold at least one unspent output.
-    ///
-    /// A used address whose outputs are all spent is omitted. That is only
-    /// sound for account types whose addresses are single-use by protocol
-    /// (CoinJoin — reuse would link mixing rounds), where nothing ever pays a
-    /// spent-and-emptied address again; callers must not apply this to
-    /// account types where address reuse is merely discouraged.
-    pub fn unspent_or_unused_script_pubkeys(&self) -> Vec<ScriptBuf> {
-        let funded: HashSet<&ScriptBuf> =
-            self.utxos.values().map(|utxo| &utxo.txout.script_pubkey).collect();
-        self.managed_account_type()
-            .address_pools()
-            .iter()
-            .flat_map(|pool| pool.addresses.values())
-            .filter(|info| !info.is_used() || funded.contains(&info.script_pubkey))
-            .map(|info| info.script_pubkey.clone())
-            .collect()
-    }
-
     /// Add new UTXOs for received outputs, remove spent ones.
     ///
     /// Skips any output whose outpoint is already in `observed_spent` — it is
@@ -1372,6 +1350,7 @@ mod conflict_sweep_walk_tests {
     use crate::transaction_checking::BlockInfo;
     use dashcore::ephemerealdata::instant_lock::InstantLock;
     use dashcore::hashes::Hash;
+    use dashcore::ScriptBuf;
     use dashcore::{BlockHash, TxIn, TxOut, Witness};
 
     fn outpoint(seed: u32, vout: u32) -> OutPoint {

key-wallet/src/tests/mod.rs

@@ -28,8 +28,6 @@ mod performance_tests;
 
 mod provider_key_derivation_tests;
 
-mod scan_script_pubkeys_tests;
-
 mod special_transaction_matching_tests;
 
 mod special_transaction_tests;