Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion key-wallet-manager/src/matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ mod tests {
use super::*;
use dashcore::address::Payload;
use dashcore::bip158::BlockFilterWriter;
use dashcore::{Address, Block, Transaction};
use dashcore::{Address, Block, OutPoint, Transaction, Txid};
use key_wallet::Network;

fn scripts_for(addresses: &[Address]) -> Vec<ScriptBuf> {
Expand Down Expand Up @@ -256,4 +256,49 @@ mod tests {
let with_element = check_compact_filters_for_elements(&input, &[], &[owner_hash], 0);
assert!(with_element.contains(&key), "bare-element query must match");
}

/// A wallet that owns a masternode's 1000-DASH collateral output but not
/// its owner/voting keys sees a compact filter that carries the
/// `ProRegTx`'s `collateralOutpoint` as a bare 36-byte consensus-serialized
/// element (the way Dash Core's `ExtractSpecialTxFilterElements` inserts
/// it), not as one of the block's scriptPubKeys. The scripts-only query
/// must miss it and the query carrying the serialized outpoint must hit it.
#[test]
fn test_collateral_outpoint_requires_extra_element() {
// The collateral outpoint a peer inserts as a bare element.
let outpoint = OutPoint::new(Txid::from([7u8; 32]), 1);
let serialized = dashcore::consensus::encode::serialize(&outpoint);
assert_eq!(serialized.len(), 36, "outpoint serializes to txid ++ le-vout");

// An unrelated output so the filter is realistic.
let unrelated = Address::dummy(Network::Regtest, 99);
let tx = Transaction::dummy(&unrelated, 0..0, &[1]);
let block = Block::dummy(100, vec![tx]);

// Build the filter like a Dash Core peer: block output scripts plus the
// collateral outpoint as a bare element.
let mut content = Vec::new();
{
let mut writer = BlockFilterWriter::new(&mut content, &block);
writer.add_output_scripts();
writer.add_element(&serialized);
writer.finish().expect("finish filter");
}
let filter = BlockFilter::new(&content);
let key = FilterMatchKey::new(100, block.block_hash());

let mut input = HashMap::new();
input.insert(key.clone(), filter);

// A wallet watching only its own addresses (none of them in this block)
// does not carry the bare 36-byte outpoint element and misses.
let watched = Address::dummy(Network::Regtest, 7);
let scripts_only =
check_compact_filters_for_elements(&input, &scripts_for(&[watched]), &[], 0);
assert!(!scripts_only.contains(&key), "scripts-only query must miss the outpoint");

// Carrying the serialized outpoint in `extra_elements` matches.
let with_element = check_compact_filters_for_elements(&input, &[], &[serialized], 0);
assert!(with_element.contains(&key), "serialized-outpoint query must match");
}
}
12 changes: 12 additions & 0 deletions key-wallet/src/managed_account/managed_account_collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ impl ManagedAccountCollection {
}

/// Check if a managed account type exists in the collection
/// Whether the wallet holds any masternode provider account (owner,
/// voting, operator, or platform keys).
///
/// A wallet with none of these can never own or operate a masternode, so it
/// has no reason to watch for masternode special transactions.
pub(crate) fn has_provider_accounts(&self) -> bool {
self.provider_owner_keys.is_some()
|| self.provider_voting_keys.is_some()
|| self.provider_operator_keys.is_some()
|| self.provider_platform_keys.is_some()
}
Comment on lines 98 to +109

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Stale doc comment on line 98 now attaches to has_provider_accounts.

Line 98 (/// Check if a managed account type exists in the collection) was the doc comment for contains_managed_account_type, but the new method was inserted between them. In Rust, consecutive /// lines form one doc block, so has_provider_accounts now carries an incorrect first line in its rustdoc.

📝 Proposed fix: move the stale comment to `contains_managed_account_type`
     /// Check if a managed account type exists in the collection
+    pub fn contains_managed_account_type(&self, managed_type: &ManagedAccountType) -> bool {
+
+    /// Whether the wallet holds any masternode provider account (owner,
+    /// voting, operator, or platform keys).
+    ///
+    /// A wallet with none of these can never own or operate a masternode, so it
+    /// has no reason to watch for masternode special transactions.
+    pub(crate) fn has_provider_accounts(&self) -> bool {

Reordering so contains_managed_account_type's doc stays with it:

+    /// Whether the wallet holds any masternode provider account (owner,
+    /// voting, operator, or platform keys).
+    ///
+    /// A wallet with none of these can never own or operate a masternode, so it
+    /// has no reason to watch for masternode special transactions.
+    pub(crate) fn has_provider_accounts(&self) -> bool {
+        self.provider_owner_keys.is_some()
+            || self.provider_voting_keys.is_some()
+            || self.provider_operator_keys.is_some()
+            || self.provider_platform_keys.is_some()
+    }
+
     /// Check if a managed account type exists in the collection
     pub fn contains_managed_account_type(&self, managed_type: &ManagedAccountType) -> bool {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@key-wallet/src/managed_account/managed_account_collection.rs` around lines 98
- 109, Move the stale “Check if a managed account type exists in the collection”
rustdoc block so it directly precedes contains_managed_account_type, and ensure
has_provider_accounts retains only documentation describing provider account
detection.


pub fn contains_managed_account_type(&self, managed_type: &ManagedAccountType) -> bool {
use crate::account::StandardAccountType;

Expand Down
10 changes: 8 additions & 2 deletions key-wallet/src/test_utils/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@ impl TestWalletContext {
/// Creates a new random testnet wallet with a BIP44 account and one
/// pre-derived receive address.
pub fn new_random() -> Self {
let wallet = Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::Default)
.expect("Should create wallet");
Self::new_random_with_options(WalletAccountCreationOptions::Default)
}

/// Like [`new_random`](Self::new_random) but with explicit account-creation
/// options, so a test can build e.g. a funding-only wallet with no provider
/// accounts.
pub fn new_random_with_options(options: WalletAccountCreationOptions) -> Self {
let wallet = Wallet::new_random(Network::Testnet, options).expect("Should create wallet");
let mut managed_wallet =
ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0);

Expand Down
76 changes: 76 additions & 0 deletions key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,31 @@ impl WalletInfoInterface for ManagedWalletInfo {
}));
}
}
// Dash Core inserts a `ProRegTx`'s `collateralOutpoint` into the block's
// compact filter serialized the consensus way — 32-byte txid followed by
// the 4-byte little-endian vout, 36 bytes total (see
// `ExtractSpecialTxFilterElements`). A wallet that owns the collateral
// output but not the masternode's owner/voting keys would otherwise never
// match that `ProRegTx` on the scriptPubKey path, so watch each UTXO's
// outpoint directly.
//
// This is gated on the wallet holding provider accounts because the
// element is only ever useful to a masternode owner: it discovers a
// `ProRegTx` that *references* one of the wallet's UTXOs as collateral.
// The common paths are already covered without it — an owner-created
// `ProRegTx` is matched by its fee input's script, and a key-holding
// owner by the owner/voting hashes above — so the collateral element
// only adds the narrow case where the wallet owns the collateral but
// holds neither the keys nor the funding inputs. A funding-only wallet
// can never hit that case, so it skips these elements rather than pay
// the per-UTXO false-positive cost on every compact-filter query. Within
// a provider wallet every UTXO is watched (not just collateral-sized
// ones) to avoid hardcoding network-specific collateral amounts.
if self.accounts.has_provider_accounts() {
for utxo in self.utxos() {
elements.push(dashcore::consensus::encode::serialize(&utxo.outpoint));
}
}
elements
}

Expand Down Expand Up @@ -505,3 +530,54 @@ impl WalletInfoInterface for ManagedWalletInfo {
self.accounts.all_accounts().iter().map(|a| a.monitor_revision()).sum()
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::test_utils::TestWalletContext;
use crate::wallet::initialization::WalletAccountCreationOptions;

/// A wallet that owns a UTXO must surface that UTXO's outpoint as a bare
/// filter element, consensus-serialized to the 36-byte form Dash Core
/// inserts for a `ProRegTx`'s `collateralOutpoint`, so a compact-filter
/// scan matches a masternode registration against the wallet's collateral
/// even when the wallet holds none of the masternode's keys.
#[tokio::test]
async fn test_watched_utxo_outpoint_is_filter_element() {
let (ctx, _tx) = TestWalletContext::new_random().with_mempool_funding(200_000).await;
let outpoint = ctx.first_utxo().outpoint;
let serialized = dashcore::consensus::encode::serialize(&outpoint);
assert_eq!(serialized.len(), 36, "outpoint serializes to txid ++ le-vout");

let elements = ctx.managed_wallet.monitored_filter_elements();
assert!(
elements.contains(&serialized),
"monitored_filter_elements must carry the watched UTXO's serialized outpoint"
);
}

/// A funding-only wallet (no provider accounts) must not surface its UTXO
/// outpoints as filter elements. The collateral outpoint only helps a
/// masternode owner discover a `ProRegTx` referencing its collateral, so a
/// plain payment wallet skips that per-UTXO cost entirely.
#[tokio::test]
async fn test_funding_only_wallet_omits_collateral_outpoints() {
let (ctx, _tx) = TestWalletContext::new_random_with_options(
WalletAccountCreationOptions::BIP44AccountsOnly([0].into()),
)
.with_mempool_funding(200_000)
.await;

// The wallet is funded, so it owns a UTXO...
let serialized = dashcore::consensus::encode::serialize(&ctx.first_utxo().outpoint);
assert_eq!(serialized.len(), 36);

// ...but with no provider accounts it emits no filter elements at all,
// in particular none of its UTXO outpoints.
let elements = ctx.managed_wallet.monitored_filter_elements();
assert!(
elements.is_empty(),
"funding-only wallet must not emit collateral outpoint elements"
);
}
}
Loading