Skip to content

Commit 88eacdf

Browse files
refactor: gate manager module behind feature flag (#584)
* refactor: gate `manager` module behind feature flag in key-wallet PR #503 merged key-wallet-manager into key-wallet but made `tokio` and `rayon` unconditional dependencies. This broke the lightweight dependency story — consumers who only need BIP32/BIP39/address generation were forced to pull in an async runtime. This PR: - Adds a `manager` feature flag (included in `default`) - Makes `tokio` and `rayon` optional, gated behind `manager` - Gates `pub mod manager` in lib.rs with `#[cfg(feature = "manager")]` - Gates manager-dependent test utilities (MockWallet, etc.) Existing consumers are unaffected since `manager` is in `default`. Consumers who want a lightweight key-wallet can now opt out: key-wallet = { default-features = false, features = ["std"] } Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: make rayon optional with parallel-filters feature Separates rayon from the manager feature. The `parallel-filters` feature enables parallel compact block filter checking via rayon. Without it, filter checking falls back to sequential iteration. This keeps rayon out of the dependency tree for consumers who need the wallet manager but not parallel filter matching. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: enable parallel-filters for dash-spv dash-spv uses compact block filter matching during sync and benefits from parallel iteration via rayon. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: enable manager feature for key-wallet-ffi key-wallet-ffi uses default-features = false and explicitly lists features. Since manager is no longer in the implicit defaults for this consumer, it needs to be listed explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: extract mock wallets into dedicated test_utils/mock_wallet module Instead of gating each item with #[cfg(feature = "manager")] in wallet.rs, move MockWallet and NonMatchingMockWallet into a separate mock_wallet.rs module that is conditionally compiled as a whole. wallet.rs now only contains ManagedWalletInfo::dummy and TestWalletContext, which have no manager dependency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use std::sync::Mutex for synchronously-accessed status_changes The status_changes field was using tokio::sync::Mutex but only accessed synchronously via try_lock().expect(), which panics on contention. Switched to std::sync::Mutex with lock().unwrap() which only panics on poison (not contention). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: update dash-spv tests for std::sync::Mutex on status_changes The MockWallet.status_changes field was changed from tokio::sync::Mutex to std::sync::Mutex, so callers need .lock().unwrap() instead of .lock().await. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * revert: keep status_changes as tokio::sync::Mutex Reverting the std::sync::Mutex change — dash-spv tests .await on the lock, so it must remain tokio::sync::Mutex. The try_lock() in process_instant_send_lock is fine since it's test-only code and the lock is never held across that call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 33f231d commit 88eacdf

8 files changed

Lines changed: 278 additions & 262 deletions

File tree

dash-spv/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ rust-version = "1.89"
1212
# Core Dash libraries
1313
dashcore = { path = "../dash", features = ["serde", "core-block-hash-use-x11", "message_verification", "bls", "quorum_validation"] }
1414
dashcore_hashes = { path = "../hashes" }
15-
key-wallet = { path = "../key-wallet" }
15+
key-wallet = { path = "../key-wallet", features = ["parallel-filters"] }
1616

1717
# BLS signatures
1818
blsful = { git = "https://github.com/dashpay/agora-blsful", rev = "0c34a7a488a0bd1c9a9a2196e793b303ad35c900" }

key-wallet-ffi/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ eddsa = ["dashcore/eddsa", "key-wallet/eddsa"]
2020
bls = ["dashcore/bls", "key-wallet/bls"]
2121

2222
[dependencies]
23-
key-wallet = { path = "../key-wallet", default-features = false, features = ["std"] }
23+
key-wallet = { path = "../key-wallet", default-features = false, features = ["std", "manager"] }
2424
dashcore = { path = "../dash" }
2525
secp256k1 = { version = "0.30.0", features = ["global-context"] }
2626
tokio = { version = "1.32", features = ["rt-multi-thread", "sync"] }
@@ -31,6 +31,6 @@ hex = "0.4"
3131
cbindgen = "0.29"
3232

3333
[dev-dependencies]
34-
key-wallet = { path = "../key-wallet", default-features = false, features = ["std", "test-utils"] }
34+
key-wallet = { path = "../key-wallet", default-features = false, features = ["std", "manager", "test-utils"] }
3535
tempfile = "3.0"
3636
hex = "0.4"

key-wallet/Cargo.toml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ readme = "README.md"
99
license = "CC0-1.0"
1010

1111
[features]
12-
default = ["std"]
12+
default = ["std", "manager"]
1313
std = ["secp256k1/std", "bip39/std", "getrandom", "rand"]
14+
manager = ["dep:tokio"]
15+
parallel-filters = ["manager", "dep:rayon"]
1416
serde = ["dep:serde", "dep:serde_json", "dashcore_hashes/serde", "secp256k1/serde", "dashcore/serde"]
1517
bincode = ["serde", "dep:bincode", "dep:bincode_derive", "dashcore_hashes/bincode", "dashcore/bincode"]
1618
bip38 = ["scrypt", "aes", "bs58", "rand"]
@@ -43,8 +45,8 @@ hex = { version = "0.4"}
4345
zeroize = { version = "1.8", features = ["derive"] }
4446
tracing = "0.1"
4547
async-trait = "0.1"
46-
tokio = { version = "1", features = ["macros", "rt", "sync"] }
47-
rayon = { version = "1.11" }
48+
tokio = { version = "1", features = ["macros", "rt", "sync"], optional = true }
49+
rayon = { version = "1.11", optional = true }
4850

4951
[dev-dependencies]
5052
dashcore = { path="../dash", features = ["test-utils"] }

key-wallet/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ pub mod dip9;
4343
pub mod error;
4444
pub mod gap_limit;
4545
pub mod managed_account;
46+
#[cfg(feature = "manager")]
4647
pub mod manager;
4748
pub mod mnemonic;
4849
pub mod psbt;

key-wallet/src/manager/matching.rs

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use alloc::vec::Vec;
22
use dashcore::bip158::BlockFilter;
33
use dashcore::prelude::CoreBlockHeight;
44
use dashcore::{Address, BlockHash};
5+
#[cfg(feature = "parallel-filters")]
56
use rayon::prelude::{IntoParallelIterator, ParallelIterator};
67
use std::collections::{BTreeSet, HashMap};
78

@@ -34,15 +35,22 @@ pub fn check_compact_filters_for_addresses(
3435
let script_pubkey_bytes: Vec<Vec<u8>> =
3536
addresses.iter().map(|address| address.script_pubkey().to_bytes()).collect();
3637

37-
input
38-
.into_par_iter()
39-
.filter_map(|(key, filter)| {
40-
filter
41-
.match_any(key.hash(), script_pubkey_bytes.iter().map(|v| v.as_slice()))
42-
.unwrap_or(false)
43-
.then_some(key.clone())
44-
})
45-
.collect()
38+
let match_filter = |(key, filter): (&FilterMatchKey, &BlockFilter)| {
39+
filter
40+
.match_any(key.hash(), script_pubkey_bytes.iter().map(|v| v.as_slice()))
41+
.unwrap_or(false)
42+
.then_some(key.clone())
43+
};
44+
45+
#[cfg(feature = "parallel-filters")]
46+
{
47+
input.into_par_iter().filter_map(match_filter).collect()
48+
}
49+
50+
#[cfg(not(feature = "parallel-filters"))]
51+
{
52+
input.iter().filter_map(match_filter).collect()
53+
}
4654
}
4755

4856
#[cfg(test)]
Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
use crate::manager::MempoolTransactionResult;
2+
use crate::manager::{BlockProcessingResult, WalletEvent, WalletInterface};
3+
use crate::transaction_checking::TransactionContext;
4+
use dashcore::address::NetworkUnchecked;
5+
use dashcore::prelude::CoreBlockHeight;
6+
use dashcore::{Address, Block, OutPoint, Transaction, Txid};
7+
use std::collections::BTreeMap;
8+
use std::str::FromStr;
9+
use std::sync::Arc;
10+
use tokio::sync::{broadcast, Mutex};
11+
12+
// Type alias for transaction effects map
13+
type TransactionEffectsMap = Arc<Mutex<BTreeMap<Txid, (i64, Vec<String>)>>>;
14+
15+
pub struct MockWallet {
16+
processed_blocks: Arc<Mutex<Vec<(dashcore::BlockHash, u32)>>>,
17+
processed_transactions: Arc<Mutex<Vec<dashcore::Txid>>>,
18+
// Map txid -> (net_amount, addresses)
19+
effects: TransactionEffectsMap,
20+
synced_height: CoreBlockHeight,
21+
event_sender: broadcast::Sender<WalletEvent>,
22+
/// When true, process_mempool_transaction returns is_relevant=true.
23+
mempool_relevant: bool,
24+
/// Addresses returned by monitored_addresses.
25+
addresses: Vec<Address>,
26+
/// Outpoints returned by watched_outpoints.
27+
outpoints: Vec<OutPoint>,
28+
/// New addresses returned by process_mempool_transaction.
29+
mempool_new_addresses: Vec<Address>,
30+
/// Recorded status change notifications for test assertions.
31+
status_changes: Arc<Mutex<Vec<(Txid, TransactionContext)>>>,
32+
/// Monitor revision counter for staleness detection.
33+
monitor_revision: u64,
34+
}
35+
36+
impl Default for MockWallet {
37+
fn default() -> Self {
38+
Self::new()
39+
}
40+
}
41+
42+
impl MockWallet {
43+
pub fn new() -> Self {
44+
let (event_sender, _) = broadcast::channel(16);
45+
Self {
46+
processed_blocks: Arc::new(Mutex::new(Vec::new())),
47+
processed_transactions: Arc::new(Mutex::new(Vec::new())),
48+
effects: Arc::new(Mutex::new(BTreeMap::new())),
49+
synced_height: 0,
50+
event_sender,
51+
mempool_relevant: false,
52+
addresses: Vec::new(),
53+
outpoints: Vec::new(),
54+
mempool_new_addresses: Vec::new(),
55+
status_changes: Arc::new(Mutex::new(Vec::new())),
56+
monitor_revision: 0,
57+
}
58+
}
59+
60+
/// Configure whether mempool transactions are reported as relevant.
61+
pub fn set_mempool_relevant(&mut self, relevant: bool) {
62+
self.mempool_relevant = relevant;
63+
}
64+
65+
/// Set the addresses returned by monitored_addresses.
66+
pub fn set_addresses(&mut self, addresses: Vec<Address>) {
67+
self.addresses = addresses;
68+
self.monitor_revision += 1;
69+
}
70+
71+
/// Set the outpoints returned by watched_outpoints.
72+
pub fn set_outpoints(&mut self, outpoints: Vec<OutPoint>) {
73+
self.outpoints = outpoints;
74+
self.monitor_revision += 1;
75+
}
76+
77+
/// Set new addresses returned by process_mempool_transaction.
78+
pub fn set_mempool_new_addresses(&mut self, addresses: Vec<Address>) {
79+
self.mempool_new_addresses = addresses;
80+
}
81+
82+
pub fn status_changes(&self) -> Arc<Mutex<Vec<(Txid, TransactionContext)>>> {
83+
self.status_changes.clone()
84+
}
85+
86+
pub async fn set_effect(&self, txid: dashcore::Txid, net: i64, addresses: Vec<String>) {
87+
let mut map = self.effects.lock().await;
88+
map.insert(txid, (net, addresses));
89+
}
90+
91+
pub fn processed_blocks(&self) -> Arc<Mutex<Vec<(dashcore::BlockHash, u32)>>> {
92+
self.processed_blocks.clone()
93+
}
94+
95+
pub fn processed_transactions(&self) -> Arc<Mutex<Vec<dashcore::Txid>>> {
96+
self.processed_transactions.clone()
97+
}
98+
}
99+
100+
#[async_trait::async_trait]
101+
impl WalletInterface for MockWallet {
102+
async fn process_block(&mut self, block: &Block, height: u32) -> BlockProcessingResult {
103+
let mut processed = self.processed_blocks.lock().await;
104+
processed.push((block.block_hash(), height));
105+
106+
BlockProcessingResult {
107+
new_txids: block.txdata.iter().map(|tx| tx.txid()).collect(),
108+
existing_txids: Vec::new(),
109+
new_addresses: Vec::new(),
110+
}
111+
}
112+
113+
async fn process_mempool_transaction(
114+
&mut self,
115+
tx: &Transaction,
116+
_is_instant_send: bool,
117+
) -> MempoolTransactionResult {
118+
let mut processed = self.processed_transactions.lock().await;
119+
processed.push(tx.txid());
120+
121+
if !self.mempool_relevant {
122+
return MempoolTransactionResult::default();
123+
}
124+
125+
let effects = self.effects.lock().await;
126+
let (net_amount, addresses) = if let Some((net, addr_strs)) = effects.get(&tx.txid()) {
127+
let addrs = addr_strs
128+
.iter()
129+
.filter_map(|s| {
130+
Address::<NetworkUnchecked>::from_str(s).ok().map(|a| a.assume_checked())
131+
})
132+
.collect();
133+
(*net, addrs)
134+
} else {
135+
(0, Vec::new())
136+
};
137+
138+
MempoolTransactionResult {
139+
is_relevant: true,
140+
net_amount,
141+
is_outgoing: net_amount < 0,
142+
addresses,
143+
new_addresses: self.mempool_new_addresses.clone(),
144+
}
145+
}
146+
147+
async fn describe(&self) -> String {
148+
"MockWallet (test implementation)".to_string()
149+
}
150+
151+
async fn transaction_effect(&self, tx: &Transaction) -> Option<(i64, Vec<String>)> {
152+
let map = self.effects.lock().await;
153+
map.get(&tx.txid()).cloned()
154+
}
155+
156+
fn monitored_addresses(&self) -> Vec<Address> {
157+
self.addresses.clone()
158+
}
159+
160+
fn watched_outpoints(&self) -> Vec<OutPoint> {
161+
self.outpoints.clone()
162+
}
163+
164+
fn synced_height(&self) -> CoreBlockHeight {
165+
self.synced_height
166+
}
167+
168+
fn update_synced_height(&mut self, height: CoreBlockHeight) {
169+
self.synced_height = height;
170+
}
171+
172+
fn monitor_revision(&self) -> u64 {
173+
self.monitor_revision
174+
}
175+
176+
fn subscribe_events(&self) -> broadcast::Receiver<WalletEvent> {
177+
self.event_sender.subscribe()
178+
}
179+
180+
fn process_instant_send_lock(&mut self, txid: Txid) {
181+
let mut changes =
182+
self.status_changes.try_lock().expect("status_changes lock contention in test helper");
183+
changes.push((txid, TransactionContext::InstantSend));
184+
}
185+
}
186+
187+
/// Mock wallet that returns false for filter checks
188+
pub struct NonMatchingMockWallet {
189+
synced_height: CoreBlockHeight,
190+
event_sender: broadcast::Sender<WalletEvent>,
191+
}
192+
193+
impl Default for NonMatchingMockWallet {
194+
fn default() -> Self {
195+
Self::new()
196+
}
197+
}
198+
199+
impl NonMatchingMockWallet {
200+
pub fn new() -> Self {
201+
let (event_sender, _) = broadcast::channel(16);
202+
Self {
203+
synced_height: 0,
204+
event_sender,
205+
}
206+
}
207+
}
208+
209+
#[async_trait::async_trait]
210+
impl WalletInterface for NonMatchingMockWallet {
211+
async fn process_block(&mut self, _block: &Block, _height: u32) -> BlockProcessingResult {
212+
BlockProcessingResult::default()
213+
}
214+
215+
async fn process_mempool_transaction(
216+
&mut self,
217+
_tx: &Transaction,
218+
_is_instant_send: bool,
219+
) -> MempoolTransactionResult {
220+
MempoolTransactionResult::default()
221+
}
222+
223+
fn monitored_addresses(&self) -> Vec<Address> {
224+
Vec::new()
225+
}
226+
227+
fn watched_outpoints(&self) -> Vec<OutPoint> {
228+
Vec::new()
229+
}
230+
231+
fn synced_height(&self) -> CoreBlockHeight {
232+
self.synced_height
233+
}
234+
235+
fn update_synced_height(&mut self, height: CoreBlockHeight) {
236+
self.synced_height = height;
237+
}
238+
239+
fn subscribe_events(&self) -> broadcast::Receiver<WalletEvent> {
240+
self.event_sender.subscribe()
241+
}
242+
243+
async fn describe(&self) -> String {
244+
"NonMatchingWallet (test implementation)".to_string()
245+
}
246+
}

key-wallet/src/test_utils/mod.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
mod account;
2+
#[cfg(feature = "manager")]
3+
mod mock_wallet;
24
mod utxo;
35
mod wallet;
46

5-
pub use wallet::MockWallet;
6-
pub use wallet::NonMatchingMockWallet;
7+
#[cfg(feature = "manager")]
8+
pub use mock_wallet::MockWallet;
9+
#[cfg(feature = "manager")]
10+
pub use mock_wallet::NonMatchingMockWallet;
711
pub use wallet::TestWalletContext;

0 commit comments

Comments
 (0)