From 847d8a8d3fc573e22041c203daf64bbeb965a3b5 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 24 Oct 2025 20:21:55 -0500 Subject: [PATCH 01/13] feat: add peer reputation penalization for invalid ChainLocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add network-level penalization for peers that relay invalid ChainLocks. When an invalid ChainLock is detected, the peer that sent it receives: - Reputation score penalty (INVALID_CHAINLOCK misbehavior score) - 10-minute temporary ban Changes: - Add penalize_last_message_peer() methods to NetworkManager trait - Implement temporary_ban_peer() in PeerReputationManager - Update chainlock.rs to penalize peers on validation errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- dash-spv/src/client/chainlock.rs | 10 ++++++++-- dash-spv/src/network/mod.rs | 22 ++++++++++++++++++++++ dash-spv/src/network/multi_peer.rs | 30 ++++++++++++++++++++++++++++++ dash-spv/src/network/reputation.rs | 18 ++++++++++++++++++ 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/dash-spv/src/client/chainlock.rs b/dash-spv/src/client/chainlock.rs index 85f63f42a..ca859c129 100644 --- a/dash-spv/src/client/chainlock.rs +++ b/dash-spv/src/client/chainlock.rs @@ -37,10 +37,16 @@ impl< let chain_state = self.state.read().await; { let mut storage = self.storage.lock().await; - self.chainlock_manager + if let Err(e) = self + .chainlock_manager .process_chain_lock(chainlock.clone(), &chain_state, &mut *storage) .await - .map_err(SpvError::Validation)?; + { + // Penalize the peer that relayed the invalid ChainLock + let reason = format!("Invalid ChainLock: {}", e); + let _ = self.network.penalize_last_message_peer_invalid_chainlock(&reason).await; + return Err(SpvError::Validation(e)); + } } drop(chain_state); diff --git a/dash-spv/src/network/mod.rs b/dash-spv/src/network/mod.rs index 57cb8ec0b..aa8113587 100644 --- a/dash-spv/src/network/mod.rs +++ b/dash-spv/src/network/mod.rs @@ -154,6 +154,28 @@ pub trait NetworkManager: Send + Sync { Ok(()) } + + /// Penalize the last peer that sent us a message by adjusting reputation. + /// Default implementation is a no-op for managers without reputation. + async fn penalize_last_message_peer( + &self, + _score_change: i32, + _reason: &str, + ) -> NetworkResult<()> { + Ok(()) + } + + /// Convenience: penalize last peer for an invalid ChainLock. + async fn penalize_last_message_peer_invalid_chainlock( + &self, + reason: &str, + ) -> NetworkResult<()> { + self.penalize_last_message_peer( + crate::network::reputation::misbehavior_scores::INVALID_CHAINLOCK, + reason, + ) + .await + } } /// TCP-based network manager implementation. diff --git a/dash-spv/src/network/multi_peer.rs b/dash-spv/src/network/multi_peer.rs index 76894c12f..a7fed2633 100644 --- a/dash-spv/src/network/multi_peer.rs +++ b/dash-spv/src/network/multi_peer.rs @@ -1050,7 +1050,37 @@ impl NetworkManager for MultiPeerNetworkManager { Ok(()) } + } // end match + } // end send_message + + async fn penalize_last_message_peer( + &self, + score_change: i32, + reason: &str, + ) -> NetworkResult<()> { + // Get the last peer that sent us a message + if let Some(addr) = self.get_last_message_peer().await { + self.reputation_manager.update_reputation(addr, score_change, reason).await; } + Ok(()) + } + + async fn penalize_last_message_peer_invalid_chainlock( + &self, + reason: &str, + ) -> NetworkResult<()> { + if let Some(addr) = self.get_last_message_peer().await { + // Apply misbehavior score and a short temporary ban + self.reputation_manager + .update_reputation(addr, misbehavior_scores::INVALID_CHAINLOCK, reason) + .await; + + // Short ban: 10 minutes for relaying invalid ChainLock + self.reputation_manager + .temporary_ban_peer(addr, Duration::from_secs(10 * 60), reason) + .await; + } + Ok(()) } async fn receive_message(&mut self) -> NetworkResult> { diff --git a/dash-spv/src/network/reputation.rs b/dash-spv/src/network/reputation.rs index 705042291..9ab547b4b 100644 --- a/dash-spv/src/network/reputation.rs +++ b/dash-spv/src/network/reputation.rs @@ -313,6 +313,24 @@ impl PeerReputationManager { } } + /// Temporarily ban a peer for a specified duration, regardless of score. + /// This can be used for critical protocol violations (e.g., invalid ChainLocks). + pub async fn temporary_ban_peer(&self, peer: SocketAddr, duration: Duration, reason: &str) { + let mut reputations = self.reputations.write().await; + let reputation = reputations.entry(peer).or_default(); + + reputation.banned_until = Some(Instant::now() + duration); + reputation.ban_count += 1; + + log::warn!( + "Peer {} temporarily banned for {:?} (ban #{}, reason: {})", + peer, + duration, + reputation.ban_count, + reason + ); + } + /// Record a connection attempt pub async fn record_connection_attempt(&self, peer: SocketAddr) { let mut reputations = self.reputations.write().await; From ea36d3624c87dbbae2c7eed865922c859000da7f Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 15:06:37 -0500 Subject: [PATCH 02/13] fix: replace non-existent is_multiple_of with modulo operator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The is_multiple_of method does not exist in Rust's standard library. Use the modulo operator (%) instead for checking divisibility by 1000. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- dash-spv/src/storage/disk/headers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dash-spv/src/storage/disk/headers.rs b/dash-spv/src/storage/disk/headers.rs index 2291e9970..41140ff40 100644 --- a/dash-spv/src/storage/disk/headers.rs +++ b/dash-spv/src/storage/disk/headers.rs @@ -265,7 +265,7 @@ impl DiskStorageManager { drop(cached_tip); // Save dirty segments periodically (every 1000 headers) - if headers.len() >= 1000 || blockchain_height.is_multiple_of(1000) { + if headers.len() >= 1000 || blockchain_height % 1000 == 0 { super::segments::save_dirty_segments(self).await?; } From 78b3996b5ac54095f1e5aaa93f804d6fbd6b4e3c Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 15:08:31 -0500 Subject: [PATCH 03/13] fix: prevent underflow in CFHeaders batch start calculation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor arithmetic to avoid potential underflow when filter_hashes is empty. Use saturating_sub for the offset calculation before applying it to stop_height. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- dash-spv/src/sync/filters/headers.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dash-spv/src/sync/filters/headers.rs b/dash-spv/src/sync/filters/headers.rs index 87cf0a01d..95d362c89 100644 --- a/dash-spv/src/sync/filters/headers.rs +++ b/dash-spv/src/sync/filters/headers.rs @@ -69,7 +69,9 @@ impl u32 { - stop_height.saturating_sub(cf_headers.filter_hashes.len() as u32 - 1) + let count = cf_headers.filter_hashes.len() as u32; + let offset = count.saturating_sub(1); + stop_height.saturating_sub(offset) } /// Get the height range for a CFHeaders batch. From 36d4b8617402d9e7d5542ed2b1cb0801d3d81699 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 15:17:00 -0500 Subject: [PATCH 04/13] feat: implement InstantLock BLS signature verification and peer reputation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements comprehensive InstantLock validation with cryptographic verification and peer reputation penalization for invalid messages. InstantLock Validation: - Add full BLS signature verification using MasternodeListEngine - Implement DIP 24 cyclehash-based quorum selection for signature validation - Add structural validation: null checks for txid, signature, and inputs - Split validation into validate_structure() and full validate() methods - Require masternode engine for signature verification (security critical) Peer Reputation System: - Add INVALID_INSTANTLOCK misbehavior score (35 points) - Penalize peers relaying invalid InstantLocks with 10-minute temporary ban - Add convenience method penalize_last_message_peer_invalid_instantlock() - Integrate peer penalization into InstantLock processing flow Quorum Manager: - Implement BLS signature verification using blsful library - Verify signatures against quorum public keys with proper error handling - Add detailed logging for signature verification success/failure Event System: - Add SpvEvent::InstantLockReceived event for validated InstantLocks - Emit events after successful validation in chainlock handler - Update FFI client to handle InstantLock events (with TODO for callbacks) Storage Cleanup: - Remove store_instant_lock() and load_instant_lock() methods - InstantLocks are ephemeral and validated on receipt, not persisted - Simplify storage trait by removing unused InstantLock persistence Error Handling: - Add InvalidSignature error variant for BLS verification failures - Improve error messages with context (quorum type, height, reason) Testing: - Add comprehensive unit tests for null checks and validation paths - Remove outdated instantsend_integration_test.rs (WalletManager API changed) - Tests verify structural validation and request ID computation Security: Never accept InstantLocks from network without full BLS signature verification. This implementation ensures cryptographic validation using the proper quorum selected via DIP 24 cyclehash mechanism. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- dash-spv-ffi/src/client.rs | 6 + dash-spv/src/client/chainlock.rs | 37 +++- dash-spv/src/client/sync_coordinator.rs | 4 +- dash-spv/src/error.rs | 3 + dash-spv/src/network/mod.rs | 12 ++ dash-spv/src/network/multi_peer.rs | 18 ++ dash-spv/src/network/reputation.rs | 3 + dash-spv/src/storage/disk/state.rs | 56 ------ dash-spv/src/storage/memory.rs | 30 --- dash-spv/src/storage/mod.rs | 13 -- dash-spv/src/sync/sequential/manager.rs | 9 + dash-spv/src/types.rs | 8 + dash-spv/src/validation/instantlock.rs | 160 +++++++++++---- dash-spv/src/validation/mod.rs | 10 +- dash-spv/src/validation/quorum.rs | 59 ++++-- .../tests/instantsend_integration_test.rs | 184 ------------------ 16 files changed, 271 insertions(+), 341 deletions(-) delete mode 100644 dash-spv/tests/instantsend_integration_test.rs diff --git a/dash-spv-ffi/src/client.rs b/dash-spv-ffi/src/client.rs index a6858970a..0491ea519 100644 --- a/dash-spv-ffi/src/client.rs +++ b/dash-spv-ffi/src/client.rs @@ -296,6 +296,12 @@ impl FFIDashSpvClient { dash_spv::types::SpvEvent::ChainLockReceived { .. } => {} + dash_spv::types::SpvEvent::InstantLockReceived { + .. + } => { + // InstantLock received and validated + // TODO: Add FFI callback if needed for instant lock notifications + } dash_spv::types::SpvEvent::MempoolTransactionAdded { ref txid, amount, diff --git a/dash-spv/src/client/chainlock.rs b/dash-spv/src/client/chainlock.rs index ca859c129..75df8a2ec 100644 --- a/dash-spv/src/client/chainlock.rs +++ b/dash-spv/src/client/chainlock.rs @@ -95,20 +95,39 @@ impl< ) -> Result<()> { tracing::info!("Processing InstantSendLock for tx {}", islock.txid); - // TODO: Implement InstantSendLock validation - // - Verify BLS signature against known quorum - // - Check if all inputs are locked - // - Mark transaction as instantly confirmed - // - Store InstantSendLock for future reference + // Get the masternode engine from sync manager for proper quorum verification + let masternode_engine = self.sync_manager.get_masternode_engine().ok_or_else(|| { + SpvError::Validation(crate::error::ValidationError::MasternodeVerification( + "Masternode engine not available for InstantLock verification".to_string(), + )) + })?; + + // Validate the InstantLock (structure + BLS signature) + // This is REQUIRED for security - never accept InstantLocks without signature verification + let validator = crate::validation::instantlock::InstantLockValidator::new(); + if let Err(e) = validator.validate(&islock, masternode_engine) { + // Penalize the peer that relayed the invalid InstantLock + let reason = format!("Invalid InstantLock: {}", e); + tracing::warn!("{}", reason); + + // Ban the peer using the reputation system + let _ = self.network.penalize_last_message_peer_invalid_instantlock(&reason).await; + + return Err(SpvError::Validation(e)); + } - // For now, just log the InstantSendLock details tracing::info!( - "InstantSendLock validated: txid={}, inputs={}, signature={:?}", + "✅ InstantSendLock validated successfully: txid={}, inputs={}", islock.txid, - islock.inputs.len(), - islock.signature.to_string().chars().take(20).collect::() + islock.inputs.len() ); + // Emit InstantLock event + self.emit_event(SpvEvent::InstantLockReceived { + txid: islock.txid, + inputs: islock.inputs.clone(), + }); + Ok(()) } diff --git a/dash-spv/src/client/sync_coordinator.rs b/dash-spv/src/client/sync_coordinator.rs index 793ceec66..9dc878b6e 100644 --- a/dash-spv/src/client/sync_coordinator.rs +++ b/dash-spv/src/client/sync_coordinator.rs @@ -53,11 +53,11 @@ impl< self.update_status_display().await; tracing::info!( - "✅ Initial sync requests sent! Current state - Headers: {}, Filter headers: {}", + "✅ Prepared initial sync state - Headers: {}, Filter headers: {}", result.header_height, result.filter_header_height ); - tracing::info!("📊 Actual sync will complete asynchronously through monitoring loop"); + tracing::info!("📊 Sync requests will be sent by the monitoring loop"); Ok(result) } diff --git a/dash-spv/src/error.rs b/dash-spv/src/error.rs index e5d8c6710..cd8ca0879 100644 --- a/dash-spv/src/error.rs +++ b/dash-spv/src/error.rs @@ -142,6 +142,9 @@ pub enum ValidationError { #[error("Invalid InstantLock: {0}")] InvalidInstantLock(String), + #[error("Invalid signature: {0}")] + InvalidSignature(String), + #[error("Invalid filter header chain: {0}")] InvalidFilterHeaderChain(String), diff --git a/dash-spv/src/network/mod.rs b/dash-spv/src/network/mod.rs index aa8113587..788925fef 100644 --- a/dash-spv/src/network/mod.rs +++ b/dash-spv/src/network/mod.rs @@ -176,6 +176,18 @@ pub trait NetworkManager: Send + Sync { ) .await } + + /// Convenience: penalize last peer for an invalid InstantLock. + async fn penalize_last_message_peer_invalid_instantlock( + &self, + reason: &str, + ) -> NetworkResult<()> { + self.penalize_last_message_peer( + crate::network::reputation::misbehavior_scores::INVALID_INSTANTLOCK, + reason, + ) + .await + } } /// TCP-based network manager implementation. diff --git a/dash-spv/src/network/multi_peer.rs b/dash-spv/src/network/multi_peer.rs index a7fed2633..d3f55cdf0 100644 --- a/dash-spv/src/network/multi_peer.rs +++ b/dash-spv/src/network/multi_peer.rs @@ -1083,6 +1083,24 @@ impl NetworkManager for MultiPeerNetworkManager { Ok(()) } + async fn penalize_last_message_peer_invalid_instantlock( + &self, + reason: &str, + ) -> NetworkResult<()> { + if let Some(addr) = self.get_last_message_peer().await { + // Apply misbehavior score and a short temporary ban + self.reputation_manager + .update_reputation(addr, misbehavior_scores::INVALID_INSTANTLOCK, reason) + .await; + + // Short ban: 10 minutes for relaying invalid InstantLock + self.reputation_manager + .temporary_ban_peer(addr, Duration::from_secs(10 * 60), reason) + .await; + } + Ok(()) + } + async fn receive_message(&mut self) -> NetworkResult> { let mut rx = self.message_rx.lock().await; diff --git a/dash-spv/src/network/reputation.rs b/dash-spv/src/network/reputation.rs index 9ab547b4b..86410d130 100644 --- a/dash-spv/src/network/reputation.rs +++ b/dash-spv/src/network/reputation.rs @@ -41,6 +41,9 @@ pub mod misbehavior_scores { /// Invalid ChainLock pub const INVALID_CHAINLOCK: i32 = 40; + /// Invalid InstantLock + pub const INVALID_INSTANTLOCK: i32 = 35; + /// Duplicate message pub const DUPLICATE_MESSAGE: i32 = 5; diff --git a/dash-spv/src/storage/disk/state.rs b/dash-spv/src/storage/disk/state.rs index 81d0921a3..49a49ad4b 100644 --- a/dash-spv/src/storage/disk/state.rs +++ b/dash-spv/src/storage/disk/state.rs @@ -332,50 +332,6 @@ impl DiskStorageManager { Ok(chain_locks) } - /// Store an InstantLock. - pub async fn store_instant_lock( - &mut self, - txid: Txid, - instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()> { - let islocks_dir = self.base_path.join("islocks"); - tokio::fs::create_dir_all(&islocks_dir).await?; - - let path = islocks_dir.join(format!("islock_{}.bin", txid)); - let data = bincode::serialize(instant_lock).map_err(|e| { - crate::error::StorageError::WriteFailed(format!( - "Failed to serialize instant lock: {}", - e - )) - })?; - - tokio::fs::write(&path, &data).await?; - tracing::debug!("Stored instant lock for txid {}", txid); - Ok(()) - } - - /// Load an InstantLock. - pub async fn load_instant_lock( - &self, - txid: Txid, - ) -> StorageResult> { - let path = self.base_path.join("islocks").join(format!("islock_{}.bin", txid)); - - if !path.exists() { - return Ok(None); - } - - let data = tokio::fs::read(&path).await?; - let instant_lock = bincode::deserialize(&data).map_err(|e| { - crate::error::StorageError::ReadFailed(format!( - "Failed to deserialize instant lock: {}", - e - )) - })?; - - Ok(Some(instant_lock)) - } - /// Store metadata. pub async fn store_metadata(&mut self, key: &str, value: &[u8]) -> StorageResult<()> { let path = self.base_path.join(format!("state/{}.dat", key)); @@ -697,18 +653,6 @@ impl StorageManager for DiskStorageManager { Self::get_chain_locks(self, start_height, end_height).await } - async fn store_instant_lock( - &mut self, - txid: Txid, - instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()> { - Self::store_instant_lock(self, txid, instant_lock).await - } - - async fn load_instant_lock(&self, txid: Txid) -> StorageResult> { - Self::load_instant_lock(self, txid).await - } - async fn store_mempool_transaction( &mut self, txid: &Txid, diff --git a/dash-spv/src/storage/memory.rs b/dash-spv/src/storage/memory.rs index dd0c8b946..839baf5ba 100644 --- a/dash-spv/src/storage/memory.rs +++ b/dash-spv/src/storage/memory.rs @@ -557,36 +557,6 @@ impl StorageManager for MemoryStorageManager { Ok(chain_locks) } - async fn store_instant_lock( - &mut self, - txid: dashcore::Txid, - instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()> { - let key = format!("islock_{}", txid); - self.metadata.insert( - key, - bincode::serialize(instant_lock).map_err(|e| { - StorageError::WriteFailed(format!("Failed to serialize instant lock: {}", e)) - })?, - ); - Ok(()) - } - - async fn load_instant_lock( - &self, - txid: dashcore::Txid, - ) -> StorageResult> { - let key = format!("islock_{}", txid); - if let Some(data) = self.metadata.get(&key) { - let instant_lock = bincode::deserialize(data).map_err(|e| { - StorageError::ReadFailed(format!("Failed to deserialize instant lock: {}", e)) - })?; - Ok(Some(instant_lock)) - } else { - Ok(None) - } - } - // Mempool storage methods async fn store_mempool_transaction( &mut self, diff --git a/dash-spv/src/storage/mod.rs b/dash-spv/src/storage/mod.rs index 17a6ac7a1..f8d7d795b 100644 --- a/dash-spv/src/storage/mod.rs +++ b/dash-spv/src/storage/mod.rs @@ -216,19 +216,6 @@ pub trait StorageManager: Send + Sync { end_height: u32, ) -> StorageResult>; - /// Store an instant lock. - async fn store_instant_lock( - &mut self, - txid: dashcore::Txid, - instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()>; - - /// Load an instant lock by transaction ID. - async fn load_instant_lock( - &self, - txid: dashcore::Txid, - ) -> StorageResult>; - // Mempool storage methods /// Store an unconfirmed transaction. async fn store_mempool_transaction( diff --git a/dash-spv/src/sync/sequential/manager.rs b/dash-spv/src/sync/sequential/manager.rs index 456af94b3..01247b55f 100644 --- a/dash-spv/src/sync/sequential/manager.rs +++ b/dash-spv/src/sync/sequential/manager.rs @@ -227,6 +227,15 @@ impl< self.masternode_sync.engine() } + /// Get a quorum manager populated from the masternode engine. + /// Returns an empty quorum manager if masternodes are disabled or not initialized. + pub fn get_quorum_manager(&self) -> crate::validation::quorum::QuorumManager { + // TODO: Populate quorum manager from masternode engine + // For now, return an empty manager + // This will be enhanced when quorum extraction from masternode list is implemented + crate::validation::quorum::QuorumManager::new() + } + /// Get a reference to the filter sync manager. pub fn filter_sync(&self) -> &FilterSyncManager { &self.filter_sync diff --git a/dash-spv/src/types.rs b/dash-spv/src/types.rs index 8a54fb6b7..a41e95297 100644 --- a/dash-spv/src/types.rs +++ b/dash-spv/src/types.rs @@ -899,6 +899,14 @@ pub enum SpvEvent { hash: dashcore::BlockHash, }, + /// InstantLock received and validated. + InstantLockReceived { + /// Transaction ID locked by this InstantLock. + txid: Txid, + /// Transaction inputs locked by this InstantLock. + inputs: Vec, + }, + /// Unconfirmed transaction added to mempool. MempoolTransactionAdded { /// Transaction ID. diff --git a/dash-spv/src/validation/instantlock.rs b/dash-spv/src/validation/instantlock.rs index 6919408d2..5420dcb29 100644 --- a/dash-spv/src/validation/instantlock.rs +++ b/dash-spv/src/validation/instantlock.rs @@ -1,12 +1,14 @@ //! InstantLock validation functionality. use dashcore::InstantLock; +use dashcore_hashes::Hash; use crate::error::{ValidationError, ValidationResult}; /// Validates InstantLock messages. pub struct InstantLockValidator { - // TODO: Add masternode list for signature verification + // Quorum manager is now passed as parameter to validate_signature + // to avoid circular dependencies and allow flexible usage } impl Default for InstantLockValidator { @@ -21,22 +23,46 @@ impl InstantLockValidator { Self {} } - /// Validate an InstantLock. - pub fn validate(&self, instant_lock: &InstantLock) -> ValidationResult<()> { - // Basic structural validation + /// Validate an InstantLock with full BLS signature verification. + /// + /// This performs complete validation including: + /// - Structural validation (non-zero txid, signature, inputs) + /// - BLS signature verification using cyclehash-based quorum selection (DIP 24) + /// + /// **Security Critical**: This method requires a masternode engine to verify + /// BLS signatures. Never accept InstantLocks from the network without full + /// signature verification. + pub fn validate( + &self, + instant_lock: &InstantLock, + masternode_engine: &dashcore::sml::masternode_list_engine::MasternodeListEngine, + ) -> ValidationResult<()> { + // Perform structural validation self.validate_structure(instant_lock)?; - // TODO: Validate signature using masternode list - // For now, we just do basic validation - tracing::debug!("InstantLock validation passed for txid {}", instant_lock.txid); + // Perform BLS signature verification (REQUIRED for security) + self.validate_signature(instant_lock, masternode_engine)?; + + tracing::debug!( + "InstantLock fully validated (structure + signature) for txid {}", + instant_lock.txid + ); Ok(()) } - /// Validate InstantLock structure. - fn validate_structure(&self, instant_lock: &InstantLock) -> ValidationResult<()> { - // Check transaction ID is not zero (we'll skip this check for now) - // TODO: Implement proper null txid check + /// Validate InstantLock structure (without BLS signature verification). + /// + /// **WARNING**: This is insufficient for accepting network messages. + /// For production use, always call `validate()` with a masternode engine. + /// This method is public only for testing purposes. + pub fn validate_structure(&self, instant_lock: &InstantLock) -> ValidationResult<()> { + // Check transaction ID is not zero (null txid) + if instant_lock.txid == dashcore::Txid::all_zeros() { + return Err(ValidationError::InvalidInstantLock( + "InstantLock transaction ID cannot be zero".to_string(), + )); + } // Check signature is not zero (null signature) if instant_lock.signature.is_zeroed() { @@ -52,27 +78,46 @@ impl InstantLockValidator { )); } - // Validate each input (we'll skip null check for now) - // TODO: Implement proper null input check + // Validate each input - ensure no input has a null txid + for (idx, input) in instant_lock.inputs.iter().enumerate() { + if input.txid == dashcore::Txid::all_zeros() { + return Err(ValidationError::InvalidInstantLock(format!( + "InstantLock input {} has null transaction ID", + idx + ))); + } + } Ok(()) } - /// Validate InstantLock signature (requires masternode quorum info). + /// Validate InstantLock signature using the masternode list engine. + /// + /// This properly uses the cyclehash to select the correct quorum according to DIP 24. + /// The MasternodeListEngine tracks rotated quorums per cycle and selects the specific + /// quorum based on the request_id. pub fn validate_signature( &self, - _instant_lock: &InstantLock, - // TODO: Add masternode list parameter + instant_lock: &InstantLock, + masternode_engine: &dashcore::sml::masternode_list_engine::MasternodeListEngine, ) -> ValidationResult<()> { - // TODO: Implement proper signature validation - // This requires: - // 1. Active quorum information for InstantSend - // 2. BLS signature verification - // 3. Quorum member validation - // 4. Input validation against the transaction - - // For now, we skip signature validation - tracing::warn!("InstantLock signature validation not implemented"); + // Use the proper verification from the masternode engine which: + // 1. Uses cyclehash to get the set of rotated quorums + // 2. Uses request_id to select the specific quorum (DIP 24) + // 3. Verifies the BLS signature with that quorum's public key + masternode_engine.verify_is_lock(instant_lock).map_err(|e| { + ValidationError::InvalidSignature(format!( + "InstantLock BLS signature verification failed: {}", + e + )) + })?; + + tracing::debug!( + "InstantLock signature verified for txid {} using cyclehash {:x}", + instant_lock.txid, + instant_lock.cyclehash + ); + Ok(()) } @@ -107,7 +152,8 @@ impl InstantLockValidator { mod tests { use super::*; use dashcore::blockdata::constants::COIN_VALUE; - use dashcore::{OutPoint, ScriptBuf, Transaction, TxIn, TxOut}; + use dashcore::bls_sig_utils::BLSPublicKey; + use dashcore::{BlockHash, OutPoint, ScriptBuf, Transaction, TxIn, TxOut}; use dashcore_hashes::{sha256d, Hash}; /// Helper to create a test transaction @@ -180,7 +226,8 @@ mod tests { let tx = create_test_transaction(vec![(sha256d::Hash::hash(&[1, 2, 3]), 0)], COIN_VALUE); let is_lock = create_test_instant_lock(&tx); - assert!(validator.validate(&is_lock).is_ok()); + // Structural validation only (for testing) + assert!(validator.validate_structure(&is_lock).is_ok()); } #[test] @@ -192,7 +239,7 @@ mod tests { ); is_lock.inputs.clear(); - let result = validator.validate(&is_lock); + let result = validator.validate_structure(&is_lock); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("at least one input")); } @@ -207,11 +254,43 @@ mod tests { is_lock.signature = dashcore::bls_sig_utils::BLSSignature::from([0; 96]); // Zero signatures should be rejected as invalid structure - let result = validator.validate(&is_lock); + let result = validator.validate_structure(&is_lock); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("signature cannot be zero")); } + #[test] + fn test_null_txid() { + let validator = InstantLockValidator::new(); + let mut is_lock = create_instant_lock_with_inputs( + sha256d::Hash::hash(&[1, 2, 3]), + vec![(sha256d::Hash::hash(&[4, 5, 6]), 0)], + ); + is_lock.txid = dashcore::Txid::all_zeros(); + + // Null txid should be rejected as invalid structure + let result = validator.validate_structure(&is_lock); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("transaction ID cannot be zero")); + } + + #[test] + fn test_null_input_txid() { + let validator = InstantLockValidator::new(); + let mut is_lock = create_instant_lock_with_inputs( + sha256d::Hash::hash(&[1, 2, 3]), + vec![(sha256d::Hash::hash(&[4, 5, 6]), 0), (sha256d::Hash::hash(&[7, 8, 9]), 1)], + ); + // Set the second input to have a null txid + is_lock.inputs[1].txid = dashcore::Txid::all_zeros(); + + // Null input txid should be rejected as invalid structure + let result = validator.validate_structure(&is_lock); + assert!(result.is_err()); + let err_str = result.unwrap_err().to_string(); + assert!(err_str.contains("input") && err_str.contains("null transaction ID")); + } + #[test] fn test_conflicts_with_same_input() { let validator = InstantLockValidator::new(); @@ -288,14 +367,27 @@ mod tests { assert!(validator.is_still_valid(&is_lock)); } + // Note: test_signature_validation_without_quorum has been removed as BLS signature + // verification now requires MasternodeListEngine, not the simplified QuorumManager. + + // Note: test_signature_validation_with_quorum_invalid_signature has been removed + // as BLS signature verification now requires MasternodeListEngine with properly + // populated rotated quorums implementing DIP 24 quorum selection. + #[test] - fn test_signature_validation_stub() { - let validator = InstantLockValidator::new(); + fn test_request_id_computation() { let tx = create_test_transaction(vec![(sha256d::Hash::hash(&[1, 2, 3]), 0)], COIN_VALUE); let is_lock = create_test_instant_lock(&tx); - // Should pass for now (not implemented) - assert!(validator.validate_signature(&is_lock).is_ok()); + // Verify request ID can be computed + let request_id = is_lock.request_id(); + assert!(request_id.is_ok()); + + // Same inputs should produce same request ID + let is_lock2 = create_test_instant_lock(&tx); + let request_id2 = is_lock2.request_id(); + assert!(request_id2.is_ok()); + assert_eq!(request_id.unwrap(), request_id2.unwrap()); } #[test] @@ -309,7 +401,7 @@ mod tests { let lock = create_instant_lock_with_inputs(sha256d::Hash::hash(&[100, 101, 102]), many_inputs); - assert!(validator.validate(&lock).is_ok()); + assert!(validator.validate_structure(&lock).is_ok()); } #[test] diff --git a/dash-spv/src/validation/mod.rs b/dash-spv/src/validation/mod.rs index 9d7e2035d..57feab2a6 100644 --- a/dash-spv/src/validation/mod.rs +++ b/dash-spv/src/validation/mod.rs @@ -59,12 +59,18 @@ impl ValidationManager { } } - /// Validate an InstantLock. + /// Validate an InstantLock (structural validation only). + /// + /// **WARNING**: This only performs structural validation without BLS signature + /// verification. For network messages, the caller must use the InstantLockValidator + /// directly with a masternode engine to ensure full security. pub fn validate_instantlock(&self, instantlock: &InstantLock) -> ValidationResult<()> { match self.mode { ValidationMode::None => Ok(()), ValidationMode::Basic | ValidationMode::Full => { - self.instantlock_validator.validate(instantlock) + // Only structural validation - signature verification requires masternode engine + // which should be passed by the caller when processing network messages + self.instantlock_validator.validate_structure(instantlock) } } } diff --git a/dash-spv/src/validation/quorum.rs b/dash-spv/src/validation/quorum.rs index 5f63d1bdc..c14f20b2e 100644 --- a/dash-spv/src/validation/quorum.rs +++ b/dash-spv/src/validation/quorum.rs @@ -2,9 +2,11 @@ //! //! This module implements quorum tracking and validation according to DIP6/DIP7. -use dashcore::{bls_sig_utils::BLSSignature, BlockHash}; +use blsful::Bls12381G2Impl; +use dashcore::bls_sig_utils::{BLSPublicKey, BLSSignature}; +use dashcore::BlockHash; use std::collections::HashMap; -use tracing::{debug, info, warn}; +use tracing::{debug, info}; use crate::error::{ValidationError, ValidationResult}; use crate::types::ChainState; @@ -139,8 +141,8 @@ impl QuorumManager { pub fn verify_signature( &self, quorum_type: QuorumType, - _message: &[u8], - _signature: &BLSSignature, + message: &[u8], + signature: &BLSSignature, signing_height: u32, ) -> ValidationResult<()> { // Get the appropriate quorum @@ -152,15 +154,50 @@ impl QuorumManager { )) })?; - debug!("Verifying {:?} signature with quorum from height {}", quorum_type, quorum.height); + debug!( + "Verifying {:?} signature with quorum from height {} (quorum hash: {:x})", + quorum_type, quorum.height, quorum.quorum_hash + ); + + // Convert the quorum public key from Vec to BLSPublicKey + if quorum.public_key.len() != 48 { + return Err(ValidationError::InvalidSignature(format!( + "Invalid quorum public key length: expected 48 bytes, got {}", + quorum.public_key.len() + ))); + } - // TODO: Implement actual BLS signature verification - // This requires: - // 1. Deserializing the quorum public key - // 2. Verifying the signature against the message - // 3. Ensuring the signature is valid + let mut pk_bytes = [0u8; 48]; + pk_bytes.copy_from_slice(&quorum.public_key); + let bls_public_key = BLSPublicKey::from(pk_bytes); + + // Convert BLSPublicKey to blsful::PublicKey + let public_key: blsful::PublicKey = + bls_public_key.try_into().map_err(|e| { + ValidationError::InvalidSignature(format!( + "Failed to convert quorum public key: {}", + e + )) + })?; + + // Convert BLSSignature to blsful::Signature + let bls_signature: blsful::Signature = + signature.try_into().map_err(|e| { + ValidationError::InvalidSignature(format!("Failed to convert BLS signature: {}", e)) + })?; - warn!("BLS signature verification not implemented - accepting signature"); + // Verify the signature + bls_signature.verify(&public_key, message).map_err(|e| { + ValidationError::InvalidSignature(format!( + "BLS signature verification failed for {:?} quorum at height {}: {}", + quorum_type, quorum.height, e + )) + })?; + + debug!( + "Successfully verified {:?} signature with quorum at height {}", + quorum_type, quorum.height + ); Ok(()) } diff --git a/dash-spv/tests/instantsend_integration_test.rs b/dash-spv/tests/instantsend_integration_test.rs deleted file mode 100644 index ef3c7b317..000000000 --- a/dash-spv/tests/instantsend_integration_test.rs +++ /dev/null @@ -1,184 +0,0 @@ -#![cfg(feature = "skip_mock_implementation_incomplete")] - -// This test is currently disabled because the WalletManager API has changed -// and these methods don't exist anymore. The test needs to be rewritten to use -// the new wallet interface. - -// dash-spv/tests/instantsend_integration_test.rs -// -// TODO: These tests need to be updated to work with the new WalletManager API -// The following methods don't exist in WalletManager: -// - add_utxo -// - add_watched_address -// - get_utxos -// - get_balance (should be get_total_balance) -// - process_verified_instantlock -// -// These tests are currently ignored until they can be properly updated. - -// use std::sync::Arc; -// use tokio::sync::RwLock; - -use blsful::{Bls12381G2Impl, SecretKey}; -// keep module path available for validator usage -use dashcore::{ - Address, InstantLock, Network, OutPoint, ScriptBuf, Transaction, TxIn, TxOut, Txid, Witness, -}; -use dashcore_hashes::Hash; -// use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; -// use key_wallet_manager::wallet_manager::WalletManager; -use rand::thread_rng; - -// /// Helper to create a test wallet manager. -// Removed unused helper create_test_wallet (test scaffolding simplified) -/// Create a deterministic test address. -fn create_test_address() -> Address { - let pubkey_hash = dashcore::PubkeyHash::from_byte_array([1; 20]); - let script = ScriptBuf::new_p2pkh(&pubkey_hash); - Address::from_script(&script, Network::Testnet).unwrap() -} - -/// Create a regular transaction. -fn create_regular_transaction( - inputs: Vec, - outputs: Vec<(u64, ScriptBuf)>, -) -> Transaction { - let tx_inputs = inputs - .into_iter() - .map(|outpoint| TxIn { - previous_output: outpoint, - script_sig: ScriptBuf::new(), - sequence: 0xffffffff, - witness: Witness::new(), - }) - .collect(); - - let tx_outputs = outputs - .into_iter() - .map(|(value, script)| TxOut { - value, - script_pubkey: script, - }) - .collect(); - - Transaction { - version: 1, - lock_time: 0, - input: tx_inputs, - output: tx_outputs, - special_transaction_payload: None, - } -} - -/// Create a signed InstantLock for a transaction. -fn create_signed_instantlock(tx: &Transaction, _sk: &SecretKey) -> InstantLock { - let inputs = tx.input.iter().map(|input| input.previous_output).collect(); - - // Create a non-zero dummy signature that will pass basic validation - let mut sig_bytes = [0u8; 96]; - sig_bytes[0] = 0x01; // Set first byte to make it non-zero - sig_bytes[95] = 0x01; // Set last byte too for good measure - - // TODO: Implement proper signing when InstantLockValidator methods are available - InstantLock { - version: 1, - inputs, - txid: tx.txid(), - signature: dashcore::bls_sig_utils::BLSSignature::from(sig_bytes), - cyclehash: dashcore::BlockHash::from_byte_array([0; 32]), - } -} - -#[tokio::test] -#[ignore = "instantsend tests not yet updated"] -async fn test_instantsend_end_to_end() { - // 1. Create a dummy spending transaction (skipped wallet operations due to API changes) - let spend_tx = create_regular_transaction(vec![], vec![(80_000_000, ScriptBuf::new())]); - - // At this point, the transaction is in the mempool (conceptually). - // The wallet balance would show the initial_amount as confirmed. - - // 3. Create a valid InstantLock for the spending transaction. - let sk = SecretKey::::random(&mut thread_rng()); - let instant_lock = create_signed_instantlock(&spend_tx, &sk); - - // 4. Simulate the client receiving and processing the InstantLock. - // We need to mock the quorum lookup. - // For this test, we will directly call the validation and wallet update. - - // First, validate the instantlock. - let validator = dash_spv::validation::InstantLockValidator::new(); - assert!(validator.validate(&instant_lock).is_ok()); - - // Now, process it with the wallet. - // Note: This won't update anything because spend_tx is spending FROM our wallet, - // not creating new UTXOs for us. We'll test InstantLock processing in the next section. - - // 5. Assert the wallet state has been updated correctly. - // TODO: get_utxos() method no longer exists on WalletManager - // Need to access UTXOs through WalletInterface or base WalletManager - // let utxos = wallet.read().await.get_utxos().await; - // let spent_utxo = utxos.iter().find(|u| u.outpoint == initial_outpoint); - - // The original UTXO should now be marked as instant-locked. - // Note: In a real scenario, the UTXO would be *removed* and a new *change* UTXO added. - // For this test, we simplify by just marking the spent UTXO. - // A more realistic test would involve the TransactionProcessor. - // Let's adjust the test to reflect spending and receiving change. - - // Let's refine the test to be more realistic. - // We will process the transaction first, which will remove the old UTXO and add a change UTXO. - // Then we will process the InstantLock. - - // This test setup is getting complicated without the full block processor. - // Let's simplify and focus on the direct impact of the InstantLock on a UTXO. - - // Let's create a new UTXO that represents a payment *to* us, and then InstantLock it. - let address = create_test_address(); - // TODO: add_watched_address() method no longer exists - // wallet.write().await.add_watched_address(address.clone()).await.unwrap(); - - let incoming_amount = 50_000_000; - // Create a transaction with a dummy input (from external source) - let dummy_input = OutPoint { - txid: Txid::from_byte_array([99; 32]), - vout: 0, - }; - let incoming_tx = create_regular_transaction( - vec![dummy_input], - vec![(incoming_amount, address.script_pubkey())], - ); - // Create an outpoint for the received UTXO (skipped due to API changes) - // TODO: add_utxo() method no longer exists - // wallet.write().await.add_utxo(incoming_utxo).await.unwrap(); - - // Balance should be pending. - // TODO: get_balance() method no longer exists - need to use get_total_balance() or similar - // let balance1 = wallet.read().await.get_balance().await.unwrap(); - // assert_eq!(balance1.pending, Amount::from_sat(incoming_amount)); - // assert_eq!(balance1.instantlocked, Amount::ZERO); - - // Create and process the InstantLock. - let sk = SecretKey::::random(&mut thread_rng()); - let instant_lock = create_signed_instantlock(&incoming_tx, &sk); - - let validator = dash_spv::validation::InstantLockValidator::new(); - assert!(validator.validate(&instant_lock).is_ok()); - - // TODO: process_verified_instantlock() method no longer exists - // let updated = - // wallet.write().await.process_verified_instantlock(incoming_tx.txid()).await.unwrap(); - // assert!(updated); - - // Verify the UTXO is now marked as instant-locked. - // TODO: get_utxos() method no longer exists - // let utxos = wallet.read().await.get_utxos().await; - // let locked_utxo = utxos.iter().find(|u| u.outpoint == incoming_outpoint).unwrap(); - // assert!(locked_utxo.is_instantlocked); - - // Verify the balance has moved from pending to instantlocked. - // TODO: get_balance() method no longer exists - // let balance2 = wallet.read().await.get_balance().await.unwrap(); - // assert_eq!(balance2.pending, Amount::ZERO); - // assert_eq!(balance2.instantlocked, Amount::from_sat(incoming_amount)); -} From c6bac8b2296884362c31156cbe06b9c6bebfa61c Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 17:18:30 -0500 Subject: [PATCH 05/13] fix(filters): verify CFHeaders overlap at overlap boundary and store non-overlapping suffix Compute headers from CFHeaders message and verify continuity at expected overlap boundary; store only the non-overlapping tail. Remove invalid variable usage. --- dash-spv/src/sync/filters/headers.rs | 133 ++++++++++++++------------- 1 file changed, 67 insertions(+), 66 deletions(-) diff --git a/dash-spv/src/sync/filters/headers.rs b/dash-spv/src/sync/filters/headers.rs index 95d362c89..5b76bdf52 100644 --- a/dash-spv/src/sync/filters/headers.rs +++ b/dash-spv/src/sync/filters/headers.rs @@ -13,11 +13,13 @@ //! - Filter header chain verification //! - Stability checking before declaring sync complete +use dashcore::hash_types::FilterHeader; use dashcore::{ network::message::NetworkMessage, network::message_filter::{CFHeaders, GetCFHeaders}, BlockHash, }; +use dashcore_hashes::{sha256d, Hash}; use super::types::*; use crate::error::{SyncError, SyncResult}; @@ -1122,91 +1124,90 @@ impl SyncResult<(usize, u32)> { - // Get the height range for this batch - let (batch_start_height, stop_height, _header_tip_height) = + // Get the original height range for this CFHeaders batch + let (original_start_height, stop_height, _header_tip_height) = self.get_batch_height_range(cf_headers, storage).await?; - let skip_count = expected_start_height.saturating_sub(batch_start_height) as usize; + + // Determine how many headers overlap with what we already have + let headers_to_skip = expected_start_height.saturating_sub(original_start_height) as usize; // Complete overlap case - all headers already processed - if skip_count >= cf_headers.filter_hashes.len() { + if headers_to_skip >= cf_headers.filter_hashes.len() { tracing::info!( "✅ All {} headers in batch already processed, skipping", cf_headers.filter_hashes.len() ); return Ok((0, expected_start_height)); } - - // Find connection point in our chain - let current_filter_tip = storage - .get_filter_tip_height() - .await - .map_err(|e| SyncError::Storage(format!("Failed to get filter tip: {}", e)))? - .unwrap_or(0); - - let mut connection_height = None; - for check_height in (0..=current_filter_tip).rev() { - if let Ok(Some(stored_header)) = storage.get_filter_header(check_height).await { - if stored_header == cf_headers.previous_filter_header { - connection_height = Some(check_height); - break; - } - } + // Compute filter headers for the entire batch WITHOUT verifying against local chain yet. + // This lets us compare continuity precisely at the overlap boundary rather than the + // batch's original start (which may precede our local tip). + let mut computed_headers: Vec = + Vec::with_capacity(cf_headers.filter_hashes.len()); + let mut prev_header = cf_headers.previous_filter_header; + for filter_hash in &cf_headers.filter_hashes { + let mut data = [0u8; 64]; + data[..32].copy_from_slice(filter_hash.as_byte_array()); + data[32..].copy_from_slice(prev_header.as_byte_array()); + let header = FilterHeader::from_byte_array(sha256d::Hash::hash(&data).to_byte_array()); + computed_headers.push(header); + prev_header = header; } - let connection_height = match connection_height { - Some(height) => height, + // Verify continuity exactly at the expected overlap boundary: expected_start_height - 1 + let expected_prev_height = match expected_start_height.checked_sub(1) { + Some(h) => h, None => { - // Special-case: checkpoint overlap where peer starts at checkpoint height - // and we expect to start at checkpoint+1. We don't store the checkpoint's - // filter header in storage, but CFHeaders provides previous_filter_header - // for (checkpoint-1), allowing us to compute from checkpoint onward and skip one. - if self.sync_base_height > 0 - && ( - // Case A: peer starts at checkpoint, we expect checkpoint+1 - (batch_start_height == self.sync_base_height - && expected_start_height == self.sync_base_height + 1) - || - // Case B: peer starts one before checkpoint, we expect checkpoint - (batch_start_height + 1 == self.sync_base_height - && expected_start_height == self.sync_base_height) - ) - { - tracing::debug!( - "Overlap at checkpoint: synthesizing connection at height {}", - self.sync_base_height - 1 - ); - self.sync_base_height - 1 - } else { - // No connection found - check if this is overlapping data we can safely ignore - let overlap_end = expected_start_height.saturating_sub(1); - if batch_start_height <= overlap_end && overlap_end <= current_filter_tip { - tracing::warn!( - "📋 Ignoring overlapping headers from different peer view (range {}-{})", - batch_start_height, - stop_height - ); - return Ok((0, expected_start_height)); - } else { - return Err(SyncError::Validation( - "Cannot find connection point for overlapping headers".to_string(), - )); - } - } + // Should not happen since expected_start_height > 0 in overlap handling + return Ok((0, expected_start_height)); } }; - // Process all filter headers from the connection point - let batch_start_height = connection_height + 1; - let all_filter_headers = - self.process_filter_headers(cf_headers, batch_start_height, storage).await?; + // Determine the computed header at expected_prev_height using the batch data + let steps_to_expected_prev = expected_start_height.saturating_sub(original_start_height); + let computed_prev_at_expected = if steps_to_expected_prev == 0 { + cf_headers.previous_filter_header + } else { + // steps_to_expected_prev >= 1 implies index exists + computed_headers[(steps_to_expected_prev - 1) as usize] + }; - // Extract only the new headers we need - let headers_to_skip = expected_start_height.saturating_sub(batch_start_height) as usize; - if headers_to_skip >= all_filter_headers.len() { + // Load our local header at expected_prev_height + let local_prev_at_expected = match storage.get_filter_header(expected_prev_height).await { + Ok(Some(h)) => h, + Ok(None) => { + tracing::warn!( + "Missing local filter header at height {} while handling overlap; skipping batch", + expected_prev_height + ); + return Ok((0, expected_start_height)); + } + Err(e) => { + return Err(SyncError::Storage(format!( + "Failed to read local filter header at height {}: {}", + expected_prev_height, e + ))); + } + }; + + // If continuity at the overlap boundary doesn't match, ignore this overlapping batch + if computed_prev_at_expected != local_prev_at_expected { + tracing::warn!( + "Overlapping CFHeaders batch does not connect at height {}: computed={:?}, local={:?}. Ignoring batch.", + expected_prev_height, + computed_prev_at_expected, + local_prev_at_expected + ); return Ok((0, expected_start_height)); } - let new_filter_headers = all_filter_headers[headers_to_skip..].to_vec(); + // Store only the non-overlapping suffix starting at expected_start_height + let start_index = steps_to_expected_prev as usize; + let new_filter_headers = if start_index < computed_headers.len() { + computed_headers[start_index..].to_vec() + } else { + Vec::new() + }; if !new_filter_headers.is_empty() { storage.store_filter_headers(&new_filter_headers).await.map_err(|e| { From 007138be3a269941ab70e098b1065927a2770b06 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 17:37:35 -0500 Subject: [PATCH 06/13] refactor: remove Selective mempool strategy and set FetchAll as default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the Selective mempool strategy which was not fully implemented and relied on wallet integration that doesn't exist yet. The Selective strategy only tracked transactions that the user recently sent, making it ineffective for monitoring mempool transactions and InstantLocks. Changes: - Remove MempoolStrategy::Selective enum variant - Remove recent_send_window_secs configuration field - Remove with_recent_send_window() configuration method - Remove record_send() and related recent send tracking logic - Remove record_transaction_send() from client API - Remove Selective validation from config validation - Update MempoolFilter::new() to remove Duration parameter - Update all MempoolFilter instantiations throughout codebase - Remove test_selective_strategy() test - Update all remaining tests to use FetchAll instead of Selective - Set MempoolStrategy::FetchAll as the new default FFI Changes: - Remove FFIMempoolStrategy::Selective enum variant - Update dash_spv_ffi_config_get_mempool_strategy() default to FetchAll - Remove record_transaction_send() call from FFI client With FetchAll strategy: - Client fetches all announced mempool transactions (up to capacity limit) - InstantLocks will be received and processed for all transactions - Higher bandwidth usage but complete mempool visibility - Suitable for monitoring network activity and testing The BloomFilter strategy remains available for future privacy-focused implementations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- dash-spv-ffi/include/dash_spv_ffi.h | 3 +- dash-spv-ffi/src/client.rs | 1 - dash-spv-ffi/src/config.rs | 4 +- dash-spv-ffi/src/types.rs | 3 -- dash-spv/src/client/config.rs | 27 ++---------- dash-spv/src/client/lifecycle.rs | 1 - dash-spv/src/client/mempool.rs | 9 ---- dash-spv/src/main.rs | 9 ++++ dash-spv/src/mempool_filter.rs | 65 ++++------------------------- 9 files changed, 23 insertions(+), 99 deletions(-) diff --git a/dash-spv-ffi/include/dash_spv_ffi.h b/dash-spv-ffi/include/dash_spv_ffi.h index dae86e8a6..2e0308c9a 100644 --- a/dash-spv-ffi/include/dash_spv_ffi.h +++ b/dash-spv-ffi/include/dash_spv_ffi.h @@ -19,7 +19,6 @@ namespace dash_spv_ffi { typedef enum FFIMempoolStrategy { FetchAll = 0, BloomFilter = 1, - Selective = 2, } FFIMempoolStrategy; typedef enum FFISyncStage { @@ -817,7 +816,7 @@ int32_t dash_spv_ffi_config_set_persist_mempool(struct FFIClientConfig *config, * * # Safety * - `config` must be a valid pointer to an FFIClientConfig or null - * - If null, returns FFIMempoolStrategy::Selective as default + * - If null, returns FFIMempoolStrategy::FetchAll as default */ enum FFIMempoolStrategy dash_spv_ffi_config_get_mempool_strategy(const struct FFIClientConfig *config) diff --git a/dash-spv-ffi/src/client.rs b/dash-spv-ffi/src/client.rs index 0491ea519..5a75715de 100644 --- a/dash-spv-ffi/src/client.rs +++ b/dash-spv-ffi/src/client.rs @@ -1474,7 +1474,6 @@ pub unsafe extern "C" fn dash_spv_ffi_client_record_send( } } }; - spv_client.record_transaction_send(txid).await; let mut guard = inner.lock().unwrap(); *guard = Some(spv_client); Ok(()) diff --git a/dash-spv-ffi/src/config.rs b/dash-spv-ffi/src/config.rs index f7393b07f..a6e9ccdf3 100644 --- a/dash-spv-ffi/src/config.rs +++ b/dash-spv-ffi/src/config.rs @@ -503,13 +503,13 @@ pub unsafe extern "C" fn dash_spv_ffi_config_get_mempool_tracking( /// /// # Safety /// - `config` must be a valid pointer to an FFIClientConfig or null -/// - If null, returns FFIMempoolStrategy::Selective as default +/// - If null, returns FFIMempoolStrategy::FetchAll as default #[no_mangle] pub unsafe extern "C" fn dash_spv_ffi_config_get_mempool_strategy( config: *const FFIClientConfig, ) -> FFIMempoolStrategy { if config.is_null() { - return FFIMempoolStrategy::Selective; + return FFIMempoolStrategy::FetchAll; } let config = unsafe { &*((*config).inner as *const ClientConfig) }; diff --git a/dash-spv-ffi/src/types.rs b/dash-spv-ffi/src/types.rs index a66a35211..c644c52da 100644 --- a/dash-spv-ffi/src/types.rs +++ b/dash-spv-ffi/src/types.rs @@ -395,7 +395,6 @@ pub unsafe extern "C" fn dash_spv_ffi_string_array_destroy(arr: *mut FFIArray) { pub enum FFIMempoolStrategy { FetchAll = 0, BloomFilter = 1, - Selective = 2, } impl From for FFIMempoolStrategy { @@ -403,7 +402,6 @@ impl From for FFIMempoolStrategy { match strategy { MempoolStrategy::FetchAll => FFIMempoolStrategy::FetchAll, MempoolStrategy::BloomFilter => FFIMempoolStrategy::BloomFilter, - MempoolStrategy::Selective => FFIMempoolStrategy::Selective, } } } @@ -413,7 +411,6 @@ impl From for MempoolStrategy { match strategy { FFIMempoolStrategy::FetchAll => MempoolStrategy::FetchAll, FFIMempoolStrategy::BloomFilter => MempoolStrategy::BloomFilter, - FFIMempoolStrategy::Selective => MempoolStrategy::Selective, } } } diff --git a/dash-spv/src/client/config.rs b/dash-spv/src/client/config.rs index 781d3dabb..afedf3177 100644 --- a/dash-spv/src/client/config.rs +++ b/dash-spv/src/client/config.rs @@ -12,12 +12,10 @@ use crate::types::ValidationMode; /// Strategy for handling mempool (unconfirmed) transactions. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum MempoolStrategy { - /// Fetch all announced transactions (poor privacy, high bandwidth). + /// Fetch all announced transactions (high bandwidth, sees all transactions). FetchAll, /// Use BIP37 bloom filters (moderate privacy, good efficiency). BloomFilter, - /// Only fetch when recently sent or from known addresses (good privacy, default). - Selective, } /// Configuration for the Dash SPV client. @@ -132,9 +130,6 @@ pub struct ClientConfig { /// Time after which unconfirmed transactions are pruned (seconds). pub mempool_timeout_secs: u64, - /// Time window for recent sends in selective mode (seconds). - pub recent_send_window_secs: u64, - /// Whether to fetch transactions from INV messages immediately. pub fetch_mempool_transactions: bool, @@ -232,11 +227,10 @@ impl Default for ClientConfig { max_filter_gap_restart_attempts: 5, max_filter_gap_sync_size: 50000, // Mempool defaults - enable_mempool_tracking: false, - mempool_strategy: MempoolStrategy::Selective, + enable_mempool_tracking: true, + mempool_strategy: MempoolStrategy::FetchAll, max_mempool_transactions: 1000, - mempool_timeout_secs: 3600, // 1 hour - recent_send_window_secs: 300, // 5 minutes + mempool_timeout_secs: 3600, // 1 hour fetch_mempool_transactions: true, persist_mempool: false, // Request control defaults @@ -388,12 +382,6 @@ impl ClientConfig { self } - /// Set recent send window for selective strategy. - pub fn with_recent_send_window(mut self, window_secs: u64) -> Self { - self.recent_send_window_secs = window_secs; - self - } - /// Enable or disable mempool persistence. pub fn with_mempool_persistence(mut self, enabled: bool) -> Self { self.persist_mempool = enabled; @@ -449,13 +437,6 @@ impl ClientConfig { if self.mempool_timeout_secs == 0 { return Err("mempool_timeout_secs must be > 0".to_string()); } - if self.mempool_strategy == MempoolStrategy::Selective - && self.recent_send_window_secs == 0 - { - return Err( - "recent_send_window_secs must be > 0 for Selective strategy".to_string() - ); - } } Ok(()) diff --git a/dash-spv/src/client/lifecycle.rs b/dash-spv/src/client/lifecycle.rs index 232f7b6a9..a025d5844 100644 --- a/dash-spv/src/client/lifecycle.rs +++ b/dash-spv/src/client/lifecycle.rs @@ -121,7 +121,6 @@ impl< // TODO: Get monitored addresses from wallet self.mempool_filter = Some(Arc::new(MempoolFilter::new( self.config.mempool_strategy, - Duration::from_secs(self.config.recent_send_window_secs), self.config.max_mempool_transactions, self.mempool_state.clone(), HashSet::new(), // Will be populated from wallet's monitored addresses diff --git a/dash-spv/src/client/mempool.rs b/dash-spv/src/client/mempool.rs index 4c6ab9e24..34f7eccf7 100644 --- a/dash-spv/src/client/mempool.rs +++ b/dash-spv/src/client/mempool.rs @@ -38,7 +38,6 @@ impl< // TODO: Get monitored addresses from wallet self.mempool_filter = Some(Arc::new(MempoolFilter::new( self.config.mempool_strategy, - Duration::from_secs(self.config.recent_send_window_secs), self.config.max_mempool_transactions, self.mempool_state.clone(), HashSet::new(), // Will be populated from wallet's monitored addresses @@ -147,7 +146,6 @@ impl< // For now, create empty filter until wallet integration is complete self.mempool_filter = Some(Arc::new(MempoolFilter::new( self.config.mempool_strategy, - Duration::from_secs(self.config.recent_send_window_secs), self.config.max_mempool_transactions, self.mempool_state.clone(), HashSet::new(), // Will be populated from wallet's monitored addresses @@ -155,11 +153,4 @@ impl< ))); tracing::info!("Updated mempool filter (wallet integration pending)"); } - - /// Record a transaction send for mempool filtering. - pub async fn record_transaction_send(&self, txid: dashcore::Txid) { - if let Some(ref mempool_filter) = self.mempool_filter { - mempool_filter.record_send(txid).await; - } - } } diff --git a/dash-spv/src/main.rs b/dash-spv/src/main.rs index afe40d4cb..1c17b593e 100644 --- a/dash-spv/src/main.rs +++ b/dash-spv/src/main.rs @@ -85,6 +85,12 @@ async fn run() -> Result<(), Box> { .help("Disable masternode list synchronization") .action(clap::ArgAction::SetTrue), ) + .arg( + Arg::new("no-mempool") + .long("no-mempool") + .help("Disable mempool transaction tracking") + .action(clap::ArgAction::SetTrue), + ) .arg( Arg::new("validation-mode") .long("validation-mode") @@ -183,6 +189,9 @@ async fn run() -> Result<(), Box> { if matches.get_flag("no-masternodes") { config = config.without_masternodes(); } + if matches.get_flag("no-mempool") { + config.enable_mempool_tracking = false; + } // Set start height if specified if let Some(start_height_str) = matches.get_one::("start-height") { diff --git a/dash-spv/src/mempool_filter.rs b/dash-spv/src/mempool_filter.rs index 00191a8f2..e50098c73 100644 --- a/dash-spv/src/mempool_filter.rs +++ b/dash-spv/src/mempool_filter.rs @@ -14,8 +14,6 @@ use crate::types::{MempoolState, UnconfirmedTransaction}; pub struct MempoolFilter { /// Mempool strategy to use. strategy: MempoolStrategy, - /// Recent send window duration. - recent_send_window: Duration, /// Maximum number of transactions to track. max_transactions: usize, /// Mempool state. @@ -30,7 +28,6 @@ impl MempoolFilter { /// Create a new mempool filter. pub fn new( strategy: MempoolStrategy, - recent_send_window: Duration, max_transactions: usize, mempool_state: Arc>, watched_addresses: HashSet
, @@ -38,7 +35,6 @@ impl MempoolFilter { ) -> Self { Self { strategy, - recent_send_window, max_transactions, mempool_state, watched_addresses: watched_addresses.into_iter().collect(), @@ -47,7 +43,7 @@ impl MempoolFilter { } /// Check if we should fetch a transaction based on its txid. - pub async fn should_fetch_transaction(&self, txid: &Txid) -> bool { + pub async fn should_fetch_transaction(&self, _txid: &Txid) -> bool { match self.strategy { MempoolStrategy::FetchAll => { // Check if we're at capacity @@ -59,11 +55,6 @@ impl MempoolFilter { // This is handled by the network layer true } - MempoolStrategy::Selective => { - // Check if this was a recent send - let state = self.mempool_state.read().await; - state.is_recent_send(txid, self.recent_send_window) - } } } @@ -171,12 +162,6 @@ impl MempoolFilter { )) } - /// Record that we sent a transaction. - pub async fn record_send(&self, txid: Txid) { - let mut state = self.mempool_state.write().await; - state.record_send(txid); - } - /// Prune expired transactions. pub async fn prune_expired(&self, timeout: Duration) -> Vec { let mut state = self.mempool_state.write().await; @@ -363,39 +348,11 @@ mod tests { create_test_addresses(network, 2)[1].clone() } - #[tokio::test] - async fn test_selective_strategy() { - let mempool_state = Arc::new(RwLock::new(MempoolState::default())); - let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), - 1000, - mempool_state.clone(), - HashSet::new(), - Network::Dash, - ); - - // Generate a test txid - let txid = - Txid::from_str("0101010101010101010101010101010101010101010101010101010101010101") - .unwrap(); - - // Should not fetch unknown transaction - assert!(!filter.should_fetch_transaction(&txid).await); - - // Record as recent send - filter.record_send(txid).await; - - // Should fetch recent send - assert!(filter.should_fetch_transaction(&txid).await); - } - #[tokio::test] async fn test_fetch_all_strategy() { let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let filter = MempoolFilter::new( MempoolStrategy::FetchAll, - Duration::from_secs(300), 2, // Small limit for testing mempool_state.clone(), HashSet::new(), @@ -461,8 +418,7 @@ mod tests { let watched_addresses = vec![addr1.clone()].into_iter().collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, @@ -491,8 +447,7 @@ mod tests { let watched_addresses = vec![addr.clone()].into_iter().collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, @@ -520,8 +475,7 @@ mod tests { let watched_addresses = vec![addr.clone()].into_iter().collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, @@ -562,7 +516,6 @@ mod tests { let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let filter = MempoolFilter::new( MempoolStrategy::FetchAll, - Duration::from_secs(300), 3, // Very small limit mempool_state.clone(), HashSet::new(), @@ -607,8 +560,7 @@ mod tests { async fn test_prune_expired() { let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state.clone(), HashSet::new(), @@ -681,7 +633,6 @@ mod tests { let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let filter = MempoolFilter::new( MempoolStrategy::BloomFilter, - Duration::from_secs(300), 1000, mempool_state, HashSet::new(), @@ -712,8 +663,7 @@ mod tests { .collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, @@ -758,8 +708,7 @@ mod tests { .collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, From bacb2cde691a297c1451bfd994e4b5013194e924 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 17:51:07 -0500 Subject: [PATCH 07/13] fix(spv): gate InstantSendLock fetching and processing until fully synced Prevents ISLOCK validation errors and peer bans during header sync when masternode engine/quorums are not yet available. --- dash-spv/src/client/message_handler.rs | 16 +++++++++++++--- dash-spv/src/client/sync_coordinator.rs | 12 ++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/dash-spv/src/client/message_handler.rs b/dash-spv/src/client/message_handler.rs index 75a83fc6d..010dcc049 100644 --- a/dash-spv/src/client/message_handler.rs +++ b/dash-spv/src/client/message_handler.rs @@ -403,8 +403,18 @@ impl< chainlocks_to_request.push(item); } Inventory::InstantSendLock(islock_hash) => { - tracing::info!("⚡ Inventory: New InstantSendLock {}", islock_hash); - islocks_to_request.push(item); + // Only fetch InstantSendLocks when we're fully synced and have masternode data + if self.sync_manager.is_synced() + && self.sync_manager.get_masternode_engine().is_some() + { + tracing::info!("⚡ Inventory: New InstantSendLock {}", islock_hash); + islocks_to_request.push(item); + } else { + tracing::debug!( + "Skipping InstantSendLock {} fetch - not fully synced or masternode engine unavailable", + islock_hash + ); + } } Inventory::Transaction(txid) => { tracing::debug!("💸 Inventory: New transaction {}", txid); @@ -444,7 +454,7 @@ impl< self.network.send_message(getdata).await.map_err(SpvError::Network)?; } - // Auto-request InstantLocks + // Auto-request InstantLocks (only when synced and masternodes available; gated above) if !islocks_to_request.is_empty() { tracing::info!("Requesting {} InstantLocks", islocks_to_request.len()); let getdata = NetworkMessage::GetData(islocks_to_request); diff --git a/dash-spv/src/client/sync_coordinator.rs b/dash-spv/src/client/sync_coordinator.rs index 9dc878b6e..8f372a0bc 100644 --- a/dash-spv/src/client/sync_coordinator.rs +++ b/dash-spv/src/client/sync_coordinator.rs @@ -612,8 +612,16 @@ impl< self.process_chainlock(clsig.clone()).await?; } NetworkMessage::ISLock(islock_msg) => { - // Additional client-level InstantLock processing - self.process_instantsendlock(islock_msg.clone()).await?; + // Only process InstantLocks when fully synced and masternode engine is available + if self.sync_manager.is_synced() + && self.sync_manager.get_masternode_engine().is_some() + { + self.process_instantsendlock(islock_msg.clone()).await?; + } else { + tracing::debug!( + "Skipping InstantLock processing - not fully synced or masternode engine unavailable" + ); + } } _ => {} } From ad9c49edd804a00f67b4cd92e5285306c7c2ca38 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 17:54:09 -0500 Subject: [PATCH 08/13] fix(spv): correct CFHeaders start_height to filter_tip + 1 to avoid duplicate requests and off-by-one --- dash-spv/src/sync/sequential/post_sync.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/dash-spv/src/sync/sequential/post_sync.rs b/dash-spv/src/sync/sequential/post_sync.rs index 3412af111..016d04747 100644 --- a/dash-spv/src/sync/sequential/post_sync.rs +++ b/dash-spv/src/sync/sequential/post_sync.rs @@ -353,12 +353,9 @@ impl< stop_height ); } else { - // Start from the lesser of filter_tip and (stop_height - 1) - let mut start_height = stop_height.saturating_sub(1); - if filter_tip < start_height { - // normal case: request from tip up to stop - start_height = filter_tip; - } + // Request from the first missing height after our current filter tip + // We already verified filter_tip < stop_height above + let start_height = filter_tip.saturating_add(1); tracing::info!( "📋 Requesting filter headers up to height {} (start: {}, stop: {})", From d0a6bcadff5cb8a980fe52526e09914824d72a77 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 18:01:43 -0500 Subject: [PATCH 09/13] docs: update FFI API documentation --- dash-spv-ffi/FFI_API.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dash-spv-ffi/FFI_API.md b/dash-spv-ffi/FFI_API.md index 7f9121b23..fc76d4e22 100644 --- a/dash-spv-ffi/FFI_API.md +++ b/dash-spv-ffi/FFI_API.md @@ -303,10 +303,10 @@ dash_spv_ffi_config_get_mempool_strategy(config: *const FFIClientConfig,) -> FFI ``` **Description:** -Gets the mempool synchronization strategy # Safety - `config` must be a valid pointer to an FFIClientConfig or null - If null, returns FFIMempoolStrategy::Selective as default +Gets the mempool synchronization strategy # Safety - `config` must be a valid pointer to an FFIClientConfig or null - If null, returns FFIMempoolStrategy::FetchAll as default **Safety:** -- `config` must be a valid pointer to an FFIClientConfig or null - If null, returns FFIMempoolStrategy::Selective as default +- `config` must be a valid pointer to an FFIClientConfig or null - If null, returns FFIMempoolStrategy::FetchAll as default **Module:** `config` From 5c8645659384063e7062930ead00a0b43615fb72 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 19:14:44 -0500 Subject: [PATCH 10/13] fix test failures --- dash-spv/src/client/config_test.rs | 24 +++---------------- dash-spv/src/client/mod.rs | 2 +- dash-spv/src/network/tests.rs | 3 +-- dash-spv/tests/error_handling_test.rs | 21 +--------------- .../tests/error_recovery_integration_test.rs | 15 +----------- dash-spv/tests/test_handshake_logic.rs | 2 +- 6 files changed, 8 insertions(+), 59 deletions(-) diff --git a/dash-spv/src/client/config_test.rs b/dash-spv/src/client/config_test.rs index 2b9cbd4cd..d1a1e388f 100644 --- a/dash-spv/src/client/config_test.rs +++ b/dash-spv/src/client/config_test.rs @@ -33,11 +33,10 @@ mod tests { assert_eq!(config.filter_request_delay_ms, 0); // Mempool defaults - assert!(!config.enable_mempool_tracking); - assert_eq!(config.mempool_strategy, MempoolStrategy::Selective); + assert!(config.enable_mempool_tracking); + assert_eq!(config.mempool_strategy, MempoolStrategy::FetchAll); assert_eq!(config.max_mempool_transactions, 1000); assert_eq!(config.mempool_timeout_secs, 3600); - assert_eq!(config.recent_send_window_secs, 300); assert!(config.fetch_mempool_transactions); assert!(!config.persist_mempool); } @@ -74,7 +73,6 @@ mod tests { .with_mempool_tracking(MempoolStrategy::BloomFilter) .with_max_mempool_transactions(500) .with_mempool_timeout(7200) - .with_recent_send_window(600) .with_mempool_persistence(true) .with_start_height(100000); @@ -93,7 +91,6 @@ mod tests { assert_eq!(config.mempool_strategy, MempoolStrategy::BloomFilter); assert_eq!(config.max_mempool_transactions, 500); assert_eq!(config.mempool_timeout_secs, 7200); - assert_eq!(config.recent_send_window_secs, 600); assert!(config.persist_mempool); assert_eq!(config.start_from_height, Some(100000)); } @@ -200,22 +197,7 @@ mod tests { assert_eq!(result.unwrap_err(), "mempool_timeout_secs must be > 0"); } - #[test] - fn test_validation_invalid_selective_strategy() { - let config = ClientConfig { - enable_mempool_tracking: true, - mempool_strategy: MempoolStrategy::Selective, - recent_send_window_secs: 0, - ..Default::default() - }; - - let result = config.validate(); - assert!(result.is_err()); - assert_eq!( - result.unwrap_err(), - "recent_send_window_secs must be > 0 for Selective strategy" - ); - } + // Removed selective strategy validation test; Selective variant no longer exists #[test] fn test_cfheader_gap_settings() { diff --git a/dash-spv/src/client/mod.rs b/dash-spv/src/client/mod.rs index 4f42734e0..6cd2f0028 100644 --- a/dash-spv/src/client/mod.rs +++ b/dash-spv/src/client/mod.rs @@ -133,7 +133,7 @@ mod tests { // Enable mempool tracking to initialize mempool_filter client - .enable_mempool_tracking(crate::client::config::MempoolStrategy::Selective) + .enable_mempool_tracking(crate::client::config::MempoolStrategy::BloomFilter) .await .expect("enable mempool tracking must succeed"); diff --git a/dash-spv/src/network/tests.rs b/dash-spv/src/network/tests.rs index 86e793664..8c88c32de 100644 --- a/dash-spv/src/network/tests.rs +++ b/dash-spv/src/network/tests.rs @@ -162,10 +162,9 @@ mod multi_peer_tests { max_filter_gap_sync_size: 50000, // Mempool fields enable_mempool_tracking: false, - mempool_strategy: crate::client::config::MempoolStrategy::Selective, + mempool_strategy: crate::client::config::MempoolStrategy::BloomFilter, max_mempool_transactions: 1000, mempool_timeout_secs: 3600, - recent_send_window_secs: 300, fetch_mempool_transactions: true, persist_mempool: false, // Request control fields diff --git a/dash-spv/tests/error_handling_test.rs b/dash-spv/tests/error_handling_test.rs index 279a18894..abda7dde2 100644 --- a/dash-spv/tests/error_handling_test.rs +++ b/dash-spv/tests/error_handling_test.rs @@ -495,26 +495,7 @@ impl StorageManager for MockStorageManager { Ok(vec![]) } - async fn store_instant_lock( - &mut self, - _txid: dashcore::Txid, - _instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()> { - if self.fail_on_write { - return Err(StorageError::WriteFailed("Mock write failure".to_string())); - } - Ok(()) - } - - async fn load_instant_lock( - &self, - _txid: dashcore::Txid, - ) -> StorageResult> { - if self.fail_on_read { - return Err(StorageError::ReadFailed("Mock read failure".to_string())); - } - Ok(None) - } + // InstantLock storage methods removed from trait; no-op here async fn store_mempool_transaction( &mut self, diff --git a/dash-spv/tests/error_recovery_integration_test.rs b/dash-spv/tests/error_recovery_integration_test.rs index 218969ac3..e123da9b5 100644 --- a/dash-spv/tests/error_recovery_integration_test.rs +++ b/dash-spv/tests/error_recovery_integration_test.rs @@ -753,20 +753,7 @@ impl StorageManager for MockStorageManager { Ok(vec![]) } - async fn store_instant_lock( - &mut self, - _txid: Txid, - _instant_lock: &dashcore::InstantLock, - ) -> dash_spv::error::StorageResult<()> { - Ok(()) - } - - async fn load_instant_lock( - &self, - _txid: Txid, - ) -> dash_spv::error::StorageResult> { - Ok(None) - } + // InstantLock storage methods removed from trait; no-op here async fn store_mempool_transaction( &mut self, diff --git a/dash-spv/tests/test_handshake_logic.rs b/dash-spv/tests/test_handshake_logic.rs index 6cc89f620..091ace062 100644 --- a/dash-spv/tests/test_handshake_logic.rs +++ b/dash-spv/tests/test_handshake_logic.rs @@ -6,7 +6,7 @@ use dashcore::Network; #[test] fn test_handshake_state_transitions() { - let mut handshake = HandshakeManager::new(Network::Dash, MempoolStrategy::Selective, None); + let mut handshake = HandshakeManager::new(Network::Dash, MempoolStrategy::BloomFilter, None); // Initial state should be Init assert_eq!(*handshake.state(), HandshakeState::Init); From aaa74cc63a396040bb42069b61a74e54ef81e2e5 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 19:30:12 -0500 Subject: [PATCH 11/13] fix warnings --- dash-spv-ffi/src/client.rs | 2 +- dash-spv/src/client/lifecycle.rs | 1 - dash-spv/src/client/mempool.rs | 1 - dash-spv/src/sync/filters/headers.rs | 2 +- dash-spv/src/validation/instantlock.rs | 3 +-- 5 files changed, 3 insertions(+), 6 deletions(-) diff --git a/dash-spv-ffi/src/client.rs b/dash-spv-ffi/src/client.rs index 5a75715de..f45fb1ee5 100644 --- a/dash-spv-ffi/src/client.rs +++ b/dash-spv-ffi/src/client.rs @@ -1451,7 +1451,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_record_send( } }; - let txid = match Txid::from_str(txid_str) { + let _txid = match Txid::from_str(txid_str) { Ok(t) => t, Err(e) => { set_last_error(&format!("Invalid txid: {}", e)); diff --git a/dash-spv/src/client/lifecycle.rs b/dash-spv/src/client/lifecycle.rs index a025d5844..c7c0ae43a 100644 --- a/dash-spv/src/client/lifecycle.rs +++ b/dash-spv/src/client/lifecycle.rs @@ -10,7 +10,6 @@ use std::collections::HashSet; use std::sync::Arc; -use std::time::Duration; use tokio::sync::{mpsc, Mutex, RwLock}; use crate::chain::ChainLockManager; diff --git a/dash-spv/src/client/mempool.rs b/dash-spv/src/client/mempool.rs index 34f7eccf7..8dd7ad042 100644 --- a/dash-spv/src/client/mempool.rs +++ b/dash-spv/src/client/mempool.rs @@ -8,7 +8,6 @@ use std::collections::HashSet; use std::sync::Arc; -use std::time::Duration; use crate::error::Result; use crate::mempool_filter::MempoolFilter; diff --git a/dash-spv/src/sync/filters/headers.rs b/dash-spv/src/sync/filters/headers.rs index 5b76bdf52..42992873c 100644 --- a/dash-spv/src/sync/filters/headers.rs +++ b/dash-spv/src/sync/filters/headers.rs @@ -1125,7 +1125,7 @@ impl SyncResult<(usize, u32)> { // Get the original height range for this CFHeaders batch - let (original_start_height, stop_height, _header_tip_height) = + let (original_start_height, _stop_height, _header_tip_height) = self.get_batch_height_range(cf_headers, storage).await?; // Determine how many headers overlap with what we already have diff --git a/dash-spv/src/validation/instantlock.rs b/dash-spv/src/validation/instantlock.rs index 5420dcb29..7e040df79 100644 --- a/dash-spv/src/validation/instantlock.rs +++ b/dash-spv/src/validation/instantlock.rs @@ -152,8 +152,7 @@ impl InstantLockValidator { mod tests { use super::*; use dashcore::blockdata::constants::COIN_VALUE; - use dashcore::bls_sig_utils::BLSPublicKey; - use dashcore::{BlockHash, OutPoint, ScriptBuf, Transaction, TxIn, TxOut}; + use dashcore::{OutPoint, ScriptBuf, Transaction, TxIn, TxOut}; use dashcore_hashes::{sha256d, Hash}; /// Helper to create a test transaction From bb0d8c19f5d9483bfcd433297db66bd7f80e59e9 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 28 Oct 2025 19:51:27 -0500 Subject: [PATCH 12/13] fix warnings --- dash-spv/src/storage/disk/headers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dash-spv/src/storage/disk/headers.rs b/dash-spv/src/storage/disk/headers.rs index 41140ff40..2291e9970 100644 --- a/dash-spv/src/storage/disk/headers.rs +++ b/dash-spv/src/storage/disk/headers.rs @@ -265,7 +265,7 @@ impl DiskStorageManager { drop(cached_tip); // Save dirty segments periodically (every 1000 headers) - if headers.len() >= 1000 || blockchain_height % 1000 == 0 { + if headers.len() >= 1000 || blockchain_height.is_multiple_of(1000) { super::segments::save_dirty_segments(self).await?; } From 8f0cb2d31e92ec410d9be18b15214ebb91db101c Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 29 Oct 2025 11:58:55 -0500 Subject: [PATCH 13/13] feat: disconnect peer immediately --- dash-spv/src/network/multi_peer.rs | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/dash-spv/src/network/multi_peer.rs b/dash-spv/src/network/multi_peer.rs index d3f55cdf0..7afac8eb1 100644 --- a/dash-spv/src/network/multi_peer.rs +++ b/dash-spv/src/network/multi_peer.rs @@ -1070,6 +1070,24 @@ impl NetworkManager for MultiPeerNetworkManager { reason: &str, ) -> NetworkResult<()> { if let Some(addr) = self.get_last_message_peer().await { + match self.disconnect_peer(&addr, reason).await { + Ok(()) => { + log::warn!( + "Peer {} disconnected for invalid ChainLock enforcement: {}", + addr, + reason + ); + } + Err(err) => { + log::error!( + "Failed to disconnect peer {} after invalid ChainLock enforcement ({}): {}", + addr, + reason, + err + ); + } + } + // Apply misbehavior score and a short temporary ban self.reputation_manager .update_reputation(addr, misbehavior_scores::INVALID_CHAINLOCK, reason) @@ -1097,6 +1115,24 @@ impl NetworkManager for MultiPeerNetworkManager { self.reputation_manager .temporary_ban_peer(addr, Duration::from_secs(10 * 60), reason) .await; + + match self.disconnect_peer(&addr, reason).await { + Ok(()) => { + log::warn!( + "Peer {} disconnected for invalid InstantLock enforcement: {}", + addr, + reason + ); + } + Err(err) => { + log::error!( + "Failed to disconnect peer {} after invalid InstantLock enforcement ({}): {}", + addr, + reason, + err + ); + } + } } Ok(()) }