feat: spv ffi sync improvements related to bip158 #147
Conversation
|
Warning Rate limit exceeded@PastaPastaPasta has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a CLI binary and clap dependency; extends FFI config with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CLI as External CLI
participant FFI as FFIDashSpvClient
participant RT as Tokio Runtime
participant RX as event_rx
participant Cbs as Registered Callbacks
CLI->>FFI: create config (may set worker_threads) & start client
FFI->>RT: build/start runtime (worker_threads configurable)
RT->>FFI: spawn SPV client -> produce event_rx
FFI->>RX: capture event_rx into client
loop client running
CLI->>FFI: dash_spv_ffi_client_drain_events()
alt events available
FFI->>RX: try_recv() repeatedly
RX-->>FFI: SpvEvent*
FFI->>Cbs: invoke callbacks for each event
else no events
FFI-->>CLI: return success (no-op)
end
end
sequenceDiagram
autonumber
participant Sync as SequentialSync
participant Wallet as WalletInterface
participant Cfg as ClientConfig
participant FS as FilterSync
Sync->>Wallet: earliest_required_height(network)
Wallet-->>Sync: Option<wallet_hint>
Sync->>Cfg: config_start_height()
Cfg-->>Sync: Option<cfg_height>
Sync->>Sync: compute start_height = max(default_start, wallet_hint, cfg_height) clamped to tip
alt actual_count > 0
Sync->>FS: sync_filters_coordinated(range)
else empty range
Sync-->>Sync: return Ok([]) (no-op)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Introduced logging for wallet transactions, capturing net balance changes and transaction context (mempool or block). - Implemented UTXO ingestion for outputs that pay to monitored addresses, ensuring accurate tracking of spendable accounts. - Removed UTXOs that are spent by transaction inputs, improving wallet state management. - Added periodic logging of detected transactions and wallet balances to enhance monitoring capabilities.
thread options; support --no-masternodes, --network, --peer, --start-height, --log-level
…ss before sort; keep match_all vacuous
…dash_spv_ffi_client_drain_events; reduce FFI logging verbosity
…in check_compact_filter
… config; expose earliest_required_height on WalletInterface
…add regression test
PastaPastaPasta
force-pushed
the
feat/spv-ffi-sync-improvements-bip158
branch
from
6ebaa76 to
cb1a32a
Compare
|
Replace alloc::format!("mempool") with "mempool".to_string() to fix
clippy::useless_format warning.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
dash-spv/src/main.rs (1)
423-427: Surface wallet history lookup failures
wallet_transaction_historyonly errors in exceptional situations (e.g., wallet missing). Silently collapsing that into0will hide real problems in the newaffecting_walletmetric. Capturing the error in the log keeps operators informed while preserving the fallback behaviour.- let wallet_affecting = mgr - .wallet_transaction_history(&wallet_id_for_logger) - .map(|v| v.len()) - .unwrap_or(0); + let wallet_affecting = match mgr.wallet_transaction_history(&wallet_id_for_logger) { + Ok(history) => history.len(), + Err(e) => { + tracing::warn!(error = %e, "Failed to fetch wallet transaction history"); + 0 + } + };key-wallet/src/transaction_checking/wallet_checker.rs (3)
301-312: Avoid format! for constant mempool contextUse a cheaper construction for the constant mempool string to avoid the overhead of
format!.- TransactionContext::Mempool => alloc::format!("mempool"), + TransactionContext::Mempool => alloc::string::String::from("mempool"),
592-605: Strengthen test: assert UTXO presence after fundingAfter asserting the funding result, also assert that a UTXO was actually added before performing the spend. This ensures the subsequent “spent UTXO should be removed” check is meaningful.
let funding_result = managed_wallet.check_transaction(&funding_tx, network, funding_context, Some(&wallet)); assert!(funding_result.is_relevant, "Funding transaction must be relevant"); assert_eq!(funding_result.total_received, funding_value); + + // Ensure the funding UTXO was added + { + let acct = managed_wallet + .accounts + .get(&network) + .expect("Should have managed accounts") + .standard_bip44_accounts + .get(&0) + .expect("Should have managed BIP44 account"); + assert_eq!(acct.utxos.len(), 1, "Funding UTXO should be present"); + }
606-627: Improve readability: name the fee and reuse itNaming the fee clarifies expectations and de-duplicates literals. Also self-documents the test’s intent.
- let spend_tx = Transaction { + let fee: u64 = 1_000; + let spend_tx = Transaction { version: 2, lock_time: 0, input: vec![TxIn { previous_output: OutPoint { txid: funding_tx.txid(), vout: 0, }, script_sig: ScriptBuf::new(), sequence: 0xffffffff, witness: dashcore::Witness::new(), }], output: vec![TxOut { - value: funding_value - 1_000, // leave a small fee + value: funding_value - fee, // leave a small fee script_pubkey: external_address.script_pubkey(), }], special_transaction_payload: None, };key-wallet/src/transaction_checking/account_checker.rs (1)
458-479: Deduplicate involved addresses & drop redundant clones
- In the outputs loop,
get_address_inforeturns an ownedAddressInfo; replace.push(address_info.clone())with.push(address_info)to match the inputs handling.- Filter or dedupe entries in
involved_receive/change/other_addresses(e.g. by.addressor index) to avoid duplicate addresses.dash-spv/src/client/filter_sync.rs (1)
76-94: Start-height selection logic is reasonable; consider a tiny clarity tweak.Current behavior prefers the earlier of wallet hint and default window, then applies config as a lower bound and caps to tip. If you want to make intent explicit, a small readability refactor helps, but behavior is fine.
Example:
- let wallet_hint = self.sync_manager.wallet_birth_height_hint().await; - let mut start_height = wallet_hint.unwrap_or(default_start).min(default_start); + let wallet_hint = self.sync_manager.wallet_birth_height_hint().await; + // Use the earlier of the wallet hint (if any) and the default window start. + let mut start_height = std::cmp::min(default_start, wallet_hint.unwrap_or(default_start));dash/src/bip158.rs (1)
300-306: Empty-query semantics for match_any are now correct; consider an early return for empty filters.LGTM on returning false for an empty query. You can also avoid work when the filter itself is empty by returning early before building/sorting the mapped vector.
let n_elements: VarInt = Decodable::consensus_decode(&mut decoder).unwrap_or(VarInt(0)); let reader = &mut decoder; // map hashes to [0, n_elements << grp] let nm = n_elements.0 * self.m; + if n_elements.0 == 0 { + // No elements in the filter -> no matches possible. + return Ok(false); + } let mut mapped = query.map(|e| map_to_range(self.filter.hash(e.borrow()), nm)).collect::<Vec<_>>(); // For an empty query set, "any" should be false (no items to match). if mapped.is_empty() { return Ok(false); } // sort mapped.sort_unstable(); - if n_elements.0 == 0 { - return Ok(false); - }dash-spv-ffi/src/client.rs (1)
393-521: Narrow the event-callback lock scope to avoid contention/reentrancy.Currently, event_callbacks is locked for the entire drain loop. Lock per event to reduce contention and avoid potential deadlocks if callbacks trigger set_event_callbacks from another thread.
- let callbacks = self.event_callbacks.lock().unwrap(); loop { match rx.try_recv() { - Ok(event) => match event { + Ok(event) => { + let callbacks = self.event_callbacks.lock().unwrap(); + match event { dash_spv::types::SpvEvent::BalanceUpdate { confirmed, unconfirmed, .. } => { callbacks.call_balance_update(confirmed, unconfirmed); } dash_spv::types::SpvEvent::FilterHeadersProgress { filter_header_height, header_height, percentage, } => { callbacks.call_filter_headers_progress( filter_header_height, header_height, percentage, ); } dash_spv::types::SpvEvent::TransactionDetected { ref txid, confirmed, ref addresses, amount, block_height, .. } => { if let Ok(txid_parsed) = txid.parse::<dashcore::Txid>() { callbacks.call_transaction( &txid_parsed, confirmed, amount, addresses, block_height, ); let wallet_id_hex = "unknown"; let account_index = 0; let block_height = block_height.unwrap_or(0); let is_ours = amount != 0; callbacks.call_wallet_transaction( wallet_id_hex, account_index, &txid_parsed, confirmed, amount, addresses, block_height, is_ours, ); } } dash_spv::types::SpvEvent::BlockProcessed { height, ref hash, .. } => { if let Ok(hash_parsed) = hash.parse::<dashcore::BlockHash>() { callbacks.call_block(height, &hash_parsed); } } dash_spv::types::SpvEvent::SyncProgress { .. } => {} dash_spv::types::SpvEvent::ChainLockReceived { .. } => {} dash_spv::types::SpvEvent::MempoolTransactionAdded { ref txid, amount, ref addresses, is_instant_send, .. } => { callbacks.call_mempool_transaction_added( txid, amount, addresses, is_instant_send, ); } dash_spv::types::SpvEvent::MempoolTransactionConfirmed { ref txid, block_height, ref block_hash, } => { callbacks.call_mempool_transaction_confirmed( txid, block_height, block_hash, ); } dash_spv::types::SpvEvent::MempoolTransactionRemoved { ref txid, ref reason } => { let ffi_reason: crate::types::FFIMempoolRemovalReason = reason.clone().into(); let reason_code = ffi_reason as u8; callbacks.call_mempool_transaction_removed(txid, reason_code); } dash_spv::types::SpvEvent::CompactFilterMatched { hash } => { if let Ok(block_hash_parsed) = hash.parse::<dashcore::BlockHash>() { callbacks.call_compact_filter_matched( &block_hash_parsed, &[], "unknown", ); } } - }, + } + } Err(TryRecvError::Empty) => break, Err(TryRecvError::Disconnected) => { *rx_guard = None; break; } } }dash-spv-ffi/Cargo.toml (1)
33-33: Scope clap to the binary target to keep the library lean.Optional: move clap behind a crate feature used only by the [[bin]] or split the CLI into a small sibling crate to avoid pulling clap for library consumers.
dash-spv-ffi/src/config.rs (1)
366-379: Add basic input validation for worker_threads.Consider capping to a reasonable upper bound (and documenting that 0 => auto) to avoid accidental huge thread counts from FFI callers.
pub unsafe extern "C" fn dash_spv_ffi_config_set_worker_threads( config: *mut FFIClientConfig, threads: u32, ) -> i32 { null_check!(config); let cfg = &mut *config; - cfg.worker_threads = threads; + // 0 => auto; otherwise clamp to a sane range (e.g., 1..=1024) + cfg.worker_threads = if threads == 0 { 0 } else { threads.min(1024) }; FFIErrorCode::Success as i32 }dash-spv-ffi/src/bin/ffi_cli.rs (4)
12-17: Remove unused ValueEnum or use it in the CLI.NetworkOpt is defined but not used. Prefer using it as the value parser for --network for type safety.
-#[derive(Copy, Clone, Debug, ValueEnum)] -enum NetworkOpt { - Mainnet, - Testnet, - Regtest, -} +#[derive(Copy, Clone, Debug, ValueEnum)] +enum NetworkOpt { + Mainnet, + Testnet, + Regtest, +}And update argument parsing and mapping:
- let matches = Command::new("dash-spv-ffi") + let matches = Command::new("dash-spv-ffi") .about("Run SPV sync via FFI") .arg( Arg::new("network") .long("network") .short('n') - .value_parser(clap::builder::PossibleValuesParser::new([ - "mainnet", "testnet", "regtest", - ])) - .default_value("mainnet"), + .value_parser(clap::value_parser!(NetworkOpt)) + .default_value("mainnet"), )- let network = match matches.get_one::<String>("network").map(|s| s.as_str()) { - Some("mainnet") => FFINetwork::Dash, - Some("testnet") => FFINetwork::Testnet, - Some("regtest") => FFINetwork::Regtest, - _ => FFINetwork::Dash, - }; + let network = match *matches.get_one::<NetworkOpt>("network").unwrap() { + NetworkOpt::Mainnet => FFINetwork::Dash, + NetworkOpt::Testnet => FFINetwork::Testnet, + NetworkOpt::Regtest => FFINetwork::Regtest, + };
56-59: Prefer a single logging initializer to avoid duplicate/conflicting log output.You already call dash_spv_ffi_init_logging; consider removing env_logger::init() here.
-fn main() { - env_logger::init(); +fn main() {
140-151: Avoid panicking on invalid peer strings with interior NUL bytes.CString::new(..).unwrap() can panic. Handle errors and surface them to the user.
- let c = CString::new(p.as_str()).unwrap(); - let rc = dash_spv_ffi_config_add_peer(cfg, c.as_ptr()); + match CString::new(p.as_str()) { + Ok(c) => { + let rc = dash_spv_ffi_config_add_peer(cfg, c.as_ptr()); + if rc != FFIErrorCode::Success as i32 { + eprintln!("Invalid peer {}: {}", p, ffi_string_to_rust(dash_spv_ffi_get_last_error())); + } + } + Err(_) => { + eprintln!("Invalid peer (contains NUL): {}", p); + } + } - if rc != FFIErrorCode::Success as i32 { - eprintln!( - "Invalid peer {}: {}", - p, - ffi_string_to_rust(dash_spv_ffi_get_last_error()) - ); - }
198-214: Optional: graceful shutdown on Ctrl+C.Add signal handling to break the loop, call cancel/stop, and exit cleanly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
dash-spv-ffi/Cargo.toml(2 hunks)dash-spv-ffi/FFI_API.md(6 hunks)dash-spv-ffi/include/dash_spv_ffi.h(3 hunks)dash-spv-ffi/src/bin/ffi_cli.rs(1 hunks)dash-spv-ffi/src/client.rs(7 hunks)dash-spv-ffi/src/config.rs(5 hunks)dash-spv/src/client/filter_sync.rs(1 hunks)dash-spv/src/main.rs(2 hunks)dash-spv/src/sync/sequential/mod.rs(2 hunks)dash/src/bip158.rs(2 hunks)key-wallet-manager/src/wallet_interface.rs(1 hunks)key-wallet-manager/src/wallet_manager/process_block.rs(1 hunks)key-wallet/src/transaction_checking/account_checker.rs(2 hunks)key-wallet/src/transaction_checking/wallet_checker.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
key-wallet-manager/src/wallet_manager/process_block.rskey-wallet/src/transaction_checking/account_checker.rsdash-spv-ffi/src/config.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv/src/sync/sequential/mod.rsdash-spv-ffi/src/client.rskey-wallet/src/transaction_checking/wallet_checker.rsdash-spv/src/client/filter_sync.rsdash-spv/src/main.rskey-wallet-manager/src/wallet_interface.rsdash/src/bip158.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
key-wallet-manager/src/wallet_manager/process_block.rskey-wallet/src/transaction_checking/account_checker.rsdash-spv-ffi/src/config.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv/src/sync/sequential/mod.rsdash-spv-ffi/src/client.rskey-wallet/src/transaction_checking/wallet_checker.rsdash-spv/src/client/filter_sync.rsdash-spv/src/main.rskey-wallet-manager/src/wallet_interface.rsdash/src/bip158.rs
{key-wallet,swift-dash-core-sdk}/**/*.{rs,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Never log or expose private keys
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use secure random number generation for key material
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/src/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/src/**/*.rs: Never serialize, print, or log private keys; avoid exposing secret key material in any form
Prefer using public keys or key fingerprints for identification in logs and messages instead of private keys
Always validate network consistency; never mix mainnet and testnet operations
Use the custom Error type; propagate errors with the ? operator and provide contextual messages
Never panic in library code; return Result<T, Error> for all fallible operations
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/src/{managed_account,transaction_checking}/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/src/transaction_checking/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Use transaction type routing to check only relevant accounts
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
dash-spv-ffi/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/src/**/*.rs: FFI functions must be declared with #[no_mangle] extern "C"
Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Accept input strings from C as *const c_char (borrowed; do not free inputs)
Functions returning pointers transfer ownership to the caller; functions taking pointers only borrow
Add cbindgen annotations for complex types so headers are generated correctly
Files:
dash-spv-ffi/src/config.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/src/client.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Be careful with memory management at FFI boundaries (C/Swift interop)
Files:
dash-spv-ffi/src/config.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
*-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
FFI bindings live in *-ffi crates
Files:
dash-spv-ffi/src/config.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/Cargo.tomldash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/sync/sequential/mod.rsdash-spv/src/client/filter_sync.rsdash-spv/src/main.rs
**/Cargo.toml
📄 CodeRabbit inference engine (AGENTS.md)
MSRV is 1.89; set and respect rust-version = "1.89" in Cargo.toml
Files:
dash-spv-ffi/Cargo.toml
dash-spv-ffi/include/dash_spv_ffi.h
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
The public C header is generated; ensure it reflects FFI changes after builds
Files:
dash-spv-ffi/include/dash_spv_ffi.h
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{managed_account,transaction_checking}/**/*.rs : Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation
Applied to files:
key-wallet/src/transaction_checking/account_checker.rsdash-spv/src/sync/sequential/mod.rskey-wallet/src/transaction_checking/wallet_checker.rsdash-spv/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/transaction_checking/**/*.rs : Use transaction type routing to check only relevant accounts
Applied to files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rsdash-spv/src/main.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/Cargo.tomldash-spv/src/main.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/Cargo.tomldash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/Cargo.tomldash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : FFI functions must be declared with #[no_mangle] extern "C"
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-16T04:15:29.335Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h : After updating dash-spv-ffi, synchronize the generated header into the Swift SDK using ./sync-headers.sh so Sources/DashSPVFFI/include/dash_spv_ffi.h stays up to date
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types so headers are generated correctly
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Accept input strings from C as *const c_char (borrowed; do not free inputs)
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/callbacks.rs : Use C function pointers for async progress/completion callbacks; callbacks may be invoked from any thread
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift} : Be careful with memory management at FFI boundaries (C/Swift interop)
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Applied to files:
dash-spv/src/sync/sequential/mod.rsdash-spv/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/**/*.rs : Always validate network consistency; never mix mainnet and testnet operations
Applied to files:
dash-spv/src/sync/sequential/mod.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to key-wallet/**/*.rs : Use secure random number generation for key material
Applied to files:
dash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Include integration tests that cover full wallet workflows
Applied to files:
dash-spv-ffi/Cargo.tomlkey-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/address_tests.rs : Place address generation and validation tests in tests/address_tests.rs
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-09-01T14:44:53.683Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Pre-PR checks: cargo fmt, cargo clippy, cargo test (workspace); update docs/CHANGELOG for user-facing changes
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Applied to files:
dash-spv/src/main.rs
🧬 Code graph analysis (8)
key-wallet-manager/src/wallet_manager/process_block.rs (2)
key-wallet-manager/src/wallet_interface.rs (1)
earliest_required_height(49-51)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
birth_height(45-45)birth_height(142-144)
key-wallet/src/transaction_checking/account_checker.rs (1)
key-wallet/src/managed_account/address_pool.rs (1)
address_info(814-816)
dash-spv-ffi/src/bin/ffi_cli.rs (5)
dash-spv-ffi/src/types.rs (1)
from_ptr(27-32)dash-spv-ffi/src/utils.rs (1)
dash_spv_ffi_init_logging(11-31)dash-spv-ffi/src/config.rs (6)
dash_spv_ffi_config_new(34-41)dash_spv_ffi_config_set_worker_threads(371-379)dash_spv_ffi_config_set_start_from_height(527-536)dash_spv_ffi_config_set_masternode_sync_enabled(283-292)dash_spv_ffi_config_add_peer(137-196)dash_spv_ffi_config_destroy(345-354)dash-spv-ffi/src/error.rs (1)
dash_spv_ffi_get_last_error(40-45)dash-spv-ffi/src/client.rs (9)
dash_spv_ffi_client_new(140-210)dash_spv_ffi_client_set_event_callbacks(1418-1440)dash_spv_ffi_client_start(625-664)dash_spv_ffi_client_sync_to_tip_with_progress(906-1095)dash_spv_ffi_client_drain_events(529-534)dash_spv_ffi_client_get_sync_progress(1129-1162)dash_spv_ffi_sync_progress_destroy(1492-1496)dash_spv_ffi_client_stop(671-682)dash_spv_ffi_client_destroy(1447-1485)
dash-spv-ffi/include/dash_spv_ffi.h (2)
dash-spv-ffi/src/client.rs (1)
dash_spv_ffi_client_drain_events(529-534)dash-spv-ffi/src/config.rs (1)
dash_spv_ffi_config_set_worker_threads(371-379)
dash-spv-ffi/src/client.rs (2)
dash-spv/src/client/mod.rs (1)
new(198-265)dash-spv/src/client/message_handler.rs (1)
new(36-60)
key-wallet/src/transaction_checking/wallet_checker.rs (5)
dash/src/address.rs (5)
network(944-946)p2pkh(545-547)p2pkh(1042-1044)script_pubkey(518-524)script_pubkey(1145-1147)key-wallet/src/wallet/initialization.rs (1)
new_random(86-105)key-wallet/src/wallet/managed_wallet_info/mod.rs (2)
from_wallet_with_name(102-106)new(54-64)dash/src/blockdata/transaction/mod.rs (1)
txid(199-214)key-wallet/src/account/account_collection.rs (1)
new(52-68)
dash-spv/src/main.rs (1)
key-wallet-manager/src/wallet_manager/mod.rs (2)
get_network_state(1051-1053)wallet_transaction_history(956-964)
key-wallet-manager/src/wallet_interface.rs (1)
key-wallet-manager/src/wallet_manager/process_block.rs (1)
earliest_required_height(121-136)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: Core Components Tests
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: SPV Components Tests
🔇 Additional comments (25)
key-wallet/src/transaction_checking/wallet_checker.rs (1)
639-642: total_sent includes fees total_sent sums UTXO input values (fees included); the existing assertion is correct.key-wallet/src/transaction_checking/account_checker.rs (2)
394-394: LGTM: explicit sent initializationGood addition; pairs well with saturating add below to avoid overflow.
485-487: Broader relevance criterion via sent > 0 — add a testIncluding sent > 0 is the right call to catch pure-spend transactions. Please add a unit test that spends a tracked UTXO with no outputs to our addresses and asserts:
- is_relevant = true,
- received = 0, sent > 0,
- affected account match is produced (even if involved address lists are empty).
key-wallet-manager/src/wallet_manager/process_block.rs (1)
104-113: Good defensive fix for empty watch set.Preventing false positives when no scripts are watched is correct; caching the miss is also good.
key-wallet-manager/src/wallet_interface.rs (1)
43-51: Trait extension with safe default is fine.Adding
earliest_required_heightwith a defaultNoneavoids breaking implementors and fits the intended hint semantics.dash-spv/src/sync/sequential/mod.rs (1)
161-165: Config start-height passthrough looks good.Returning
config.start_from_heightas-is is appropriate.dash-spv/src/client/filter_sync.rs (3)
72-74: Range size clamping is correct.Ensuring at least 1 block avoids zero-length requests and off-by-one issues.
101-104: Helpful observability.Logging the wallet hint aids diagnostics without being noisy.
107-111: Guarding empty ranges is correct.Only initiating sync when
actual_count > 0avoids no-op calls.dash/src/bip158.rs (1)
350-353: match_all keeps correct vacuous truth behavior; good call moving sort/dedup after emptiness check.No concerns.
dash-spv-ffi/src/client.rs (6)
130-132: Storing the event receiver for pull-based draining is appropriate.This aligns with the new non-blocking drain API and avoids spawning a background thread.
147-153: Configurable Tokio worker threads: good integration.Builder usage is correct; 0 => auto is a sensible default.
649-656: Taking the event receiver after start() is correct.Ensures a single consumer for events before exposing drain.
524-534: drain_events FFI entrypoint looks good.Non-blocking behavior with graceful no-op when no receiver is present.
1426-1439: Debug logs for callback registration are fine; keep at debug level.No issues.
528-533: Add unit tests for the new drain API.Please add a minimal test that:
- Starts a client (or a mocked one), ensures drain returns Success and does not block when no events.
- Verifies that draining after pushing a known event invokes the expected callback once.
Would you like me to draft a test template under dash-spv-ffi/tests/unit?
dash-spv-ffi/include/dash_spv_ffi.h (3)
62-66: FFIClientConfig now includes worker_threads; ensure repr and field order stay in sync.Matches the Rust struct layout; confirm cbindgen output is committed after changes.
285-292: New non-blocking event drain API declared.Header matches implementation.
714-721: Confirm header sync in Swift SDK
Bothdash_spv_ffi_client_drain_eventsanddash_spv_ffi_config_set_worker_threadsare declared inswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h; header is synced.dash-spv-ffi/Cargo.toml (1)
44-47: New CLI target registration looks good.Name and path are consistent; builds alongside the library.
dash-spv-ffi/src/config.rs (3)
25-31: FFIClientConfig gains worker_threads; repr(C) maintained.This keeps the C layout stable. Defaulting to 0 (auto) is sensible.
33-41: Constructors initialize worker_threads to 0 (auto).LGTM.
Also applies to: 44-51, 53-61
366-379: Add unit tests for new setter.Please add a small test that sets 0, 1, and a large value to ensure the stored value follows the intended policy.
dash-spv-ffi/FFI_API.md (2)
7-7: API surface size updated; docs reflect new functions.Docs align with code changes.
37-38: New config setter and drain API are documented.Looks consistent and complete.
Also applies to: 66-66, 123-129, 622-637, 926-941
Replace manual modulo checks with the cleaner is_multiple_of() method: - hashes/src/hex.rs: Check for odd-length hex strings - hashes/src/sha256.rs: Check block size alignment in from_midstate 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Replace manual modulo checks with is_multiple_of() method: - TaprootMerkleBranch::decode: Check node size alignment - ControlBlock::decode: Check control block size alignment 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hashes/src/hex.rs (1)
89-95: Confirmusize::is_multiple_offits our MSRV.
usize::is_multiple_ofonly became stable recently (Rust 1.84). If our published MSRV is still tracking an older toolchain, this change will break the build. Please keep the original% 2check or update the documented MSRV/toolchain accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
dash-spv-ffi/Cargo.toml(2 hunks)dash-spv-ffi/FFI_API.md(6 hunks)dash-spv-ffi/include/dash_spv_ffi.h(3 hunks)dash-spv-ffi/src/bin/ffi_cli.rs(1 hunks)dash-spv-ffi/src/client.rs(7 hunks)dash-spv-ffi/src/config.rs(5 hunks)dash-spv/src/client/filter_sync.rs(1 hunks)dash-spv/src/main.rs(2 hunks)dash-spv/src/sync/sequential/mod.rs(2 hunks)dash/src/bip158.rs(2 hunks)hashes/src/hex.rs(1 hunks)hashes/src/sha256.rs(1 hunks)key-wallet-manager/src/wallet_interface.rs(1 hunks)key-wallet-manager/src/wallet_manager/process_block.rs(1 hunks)key-wallet/src/transaction_checking/account_checker.rs(2 hunks)key-wallet/src/transaction_checking/wallet_checker.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
*-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
FFI bindings live in *-ffi crates
Files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/src/config.rs
**/Cargo.toml
📄 CodeRabbit inference engine (AGENTS.md)
MSRV is 1.89; set and respect rust-version = "1.89" in Cargo.toml
Files:
dash-spv-ffi/Cargo.toml
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
hashes/src/hex.rskey-wallet-manager/src/wallet_manager/process_block.rshashes/src/sha256.rskey-wallet-manager/src/wallet_interface.rsdash/src/bip158.rskey-wallet/src/transaction_checking/account_checker.rsdash-spv/src/main.rsdash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv/src/client/filter_sync.rsdash-spv-ffi/src/config.rskey-wallet/src/transaction_checking/wallet_checker.rsdash-spv/src/sync/sequential/mod.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
hashes/src/hex.rskey-wallet-manager/src/wallet_manager/process_block.rshashes/src/sha256.rskey-wallet-manager/src/wallet_interface.rsdash/src/bip158.rskey-wallet/src/transaction_checking/account_checker.rsdash-spv/src/main.rsdash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv/src/client/filter_sync.rsdash-spv-ffi/src/config.rskey-wallet/src/transaction_checking/wallet_checker.rsdash-spv/src/sync/sequential/mod.rs
{key-wallet,swift-dash-core-sdk}/**/*.{rs,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Never log or expose private keys
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use secure random number generation for key material
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/src/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/src/**/*.rs: Never serialize, print, or log private keys; avoid exposing secret key material in any form
Prefer using public keys or key fingerprints for identification in logs and messages instead of private keys
Always validate network consistency; never mix mainnet and testnet operations
Use the custom Error type; propagate errors with the ? operator and provide contextual messages
Never panic in library code; return Result<T, Error> for all fallible operations
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/src/{managed_account,transaction_checking}/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/src/transaction_checking/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Use transaction type routing to check only relevant accounts
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
dash-spv-ffi/include/dash_spv_ffi.h
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
The public C header is generated; ensure it reflects FFI changes after builds
Files:
dash-spv-ffi/include/dash_spv_ffi.h
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Be careful with memory management at FFI boundaries (C/Swift interop)
Files:
dash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/src/config.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/main.rsdash-spv/src/client/filter_sync.rsdash-spv/src/sync/sequential/mod.rs
dash-spv-ffi/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/src/**/*.rs: FFI functions must be declared with #[no_mangle] extern "C"
Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Accept input strings from C as *const c_char (borrowed; do not free inputs)
Functions returning pointers transfer ownership to the caller; functions taking pointers only borrow
Add cbindgen annotations for complex types so headers are generated correctly
Files:
dash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/src/config.rs
🧠 Learnings (34)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{managed_account,transaction_checking}/**/*.rs : Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types so headers are generated correctly
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : FFI functions must be declared with #[no_mangle] extern "C"
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/FFI_API.mddash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:15:29.335Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h : After updating dash-spv-ffi, synchronize the generated header into the Swift SDK using ./sync-headers.sh so Sources/DashSPVFFI/include/dash_spv_ffi.h stays up to date
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to key-wallet/**/*.rs : Use secure random number generation for key material
Applied to files:
dash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Include integration tests that cover full wallet workflows
Applied to files:
dash-spv-ffi/Cargo.tomlkey-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Accept input strings from C as *const c_char (borrowed; do not free inputs)
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{managed_account,transaction_checking}/**/*.rs : Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rskey-wallet/src/transaction_checking/account_checker.rsdash-spv/src/main.rskey-wallet/src/transaction_checking/wallet_checker.rsdash-spv/src/sync/sequential/mod.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift} : Be careful with memory management at FFI boundaries (C/Swift interop)
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/callbacks.rs : Use C function pointers for async progress/completion callbacks; callbacks may be invoked from any thread
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/transaction_checking/**/*.rs : Use transaction type routing to check only relevant accounts
Applied to files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/address_tests.rs : Place address generation and validation tests in tests/address_tests.rs
Applied to files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Applied to files:
dash-spv/src/main.rsdash-spv/src/sync/sequential/mod.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : Add a corresponding C test for each new FFI function
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Applied to files:
dash-spv-ffi/src/client.rskey-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : In C code, free all strings returned by the FFI using dash_string_free
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided
Applied to files:
dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/psbt.rs : Place PSBT serialization/deserialization tests in tests/psbt.rs
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/derivation_tests.rs : Place path derivation tests in tests/derivation_tests.rs
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Use property-based tests for complex invariants where suitable
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/bip32_tests.rs : Organize unit tests so key-derivation correctness lives in tests/bip32_tests.rs
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/mnemonic_tests.rs : Place mnemonic generation and validation tests in tests/mnemonic_tests.rs
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rs
📚 Learning: 2025-06-15T15:31:44.136Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#74
File: key-wallet/src/address.rs:30-40
Timestamp: 2025-06-15T15:31:44.136Z
Learning: In the key-wallet crate, QuantumExplorer prefers keeping wildcard arms that default unknown Network variants to testnet prefixes rather than using exhaustive matches or panics. This fallback behavior is intentional.
Applied to files:
dash-spv/src/sync/sequential/mod.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/**/*.rs : Always validate network consistency; never mix mainnet and testnet operations
Applied to files:
dash-spv/src/sync/sequential/mod.rs
📚 Learning: 2025-08-16T04:15:57.225Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:57.225Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : When accessing SPV functionality, use DashSDK public APIs instead of direct client access; add public wrapper methods to DashSDK if needed
Applied to files:
dash-spv/src/sync/sequential/mod.rs
🧬 Code graph analysis (8)
key-wallet-manager/src/wallet_manager/process_block.rs (2)
key-wallet-manager/src/wallet_interface.rs (1)
earliest_required_height(49-51)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
birth_height(45-45)birth_height(142-144)
key-wallet-manager/src/wallet_interface.rs (1)
key-wallet-manager/src/wallet_manager/process_block.rs (1)
earliest_required_height(121-136)
key-wallet/src/transaction_checking/account_checker.rs (1)
key-wallet/src/managed_account/address_pool.rs (1)
address_info(814-816)
dash-spv-ffi/include/dash_spv_ffi.h (2)
dash-spv-ffi/src/client.rs (1)
dash_spv_ffi_client_drain_events(529-534)dash-spv-ffi/src/config.rs (1)
dash_spv_ffi_config_set_worker_threads(371-379)
dash-spv/src/main.rs (3)
key-wallet-manager/src/wallet_manager/mod.rs (2)
get_network_state(1051-1053)wallet_transaction_history(956-964)key-wallet/src/managed_account/managed_account_trait.rs (1)
transactions(42-42)key-wallet/src/managed_account/mod.rs (1)
transactions(811-813)
dash-spv-ffi/src/client.rs (2)
dash-spv/src/client/mod.rs (1)
new(198-265)dash-spv/src/client/message_handler.rs (1)
new(36-60)
dash-spv-ffi/src/bin/ffi_cli.rs (5)
dash-spv-ffi/src/types.rs (1)
from_ptr(27-32)dash-spv-ffi/src/utils.rs (1)
dash_spv_ffi_init_logging(11-31)dash-spv-ffi/src/config.rs (6)
dash_spv_ffi_config_new(34-41)dash_spv_ffi_config_set_worker_threads(371-379)dash_spv_ffi_config_set_start_from_height(527-536)dash_spv_ffi_config_set_masternode_sync_enabled(283-292)dash_spv_ffi_config_add_peer(137-196)dash_spv_ffi_config_destroy(345-354)dash-spv-ffi/src/error.rs (1)
dash_spv_ffi_get_last_error(40-45)dash-spv-ffi/src/client.rs (9)
dash_spv_ffi_client_new(140-210)dash_spv_ffi_client_set_event_callbacks(1418-1440)dash_spv_ffi_client_start(625-664)dash_spv_ffi_client_sync_to_tip_with_progress(906-1095)dash_spv_ffi_client_drain_events(529-534)dash_spv_ffi_client_get_sync_progress(1129-1162)dash_spv_ffi_sync_progress_destroy(1492-1496)dash_spv_ffi_client_stop(671-682)dash_spv_ffi_client_destroy(1447-1485)
key-wallet/src/transaction_checking/wallet_checker.rs (7)
key-wallet/src/managed_account/mod.rs (3)
network(787-789)new(63-73)utxos(819-821)dash/src/address.rs (5)
network(944-946)p2pkh(545-547)p2pkh(1042-1044)script_pubkey(518-524)script_pubkey(1145-1147)key-wallet/src/wallet/initialization.rs (1)
new_random(86-105)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (4)
from_wallet_with_name(27-27)from_wallet_with_name(118-120)utxos(63-63)utxos(175-186)key-wallet/src/wallet/managed_wallet_info/mod.rs (2)
from_wallet_with_name(102-106)new(54-64)dash/src/blockdata/transaction/mod.rs (1)
txid(199-214)key-wallet/src/account/account_collection.rs (1)
new(52-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: Core Components Tests
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: SPV Components Tests
🔇 Additional comments (11)
dash/src/bip158.rs (2)
300-305: Solid empty-query handling inmatch_any.Short-circuiting to
Ok(false)before sorting keeps the semantics straight: an empty query set should not report a match, and we now avoid the unnecessary allocation churn as well. Nice tighten-up.
350-353: Efficient early exit formatch_all.Bailing out with
Ok(true)before the sort/dedup preserves the expected behavior for empty queries and saves the redundant work. Looks good.key-wallet-manager/src/wallet_manager/process_block.rs (1)
124-135: Don't treat unknown birth heights as genesisDefaulting
birth_heightto 0 forces a full rescan to genesis whenever the wallet lacks metadata, undoing the optimisation this hint is supposed to provide. Only compute the minimum across wallets that actually report a height and returnNonewhen none do.- if info.accounts(network).is_some() { - let birth_height = info.birth_height().unwrap_or(0); - earliest = Some(match earliest { - Some(current) => current.min(birth_height), - None => birth_height, - }); - } + if info.accounts(network).is_some() { + if let Some(birth_height) = info.birth_height() { + earliest = Some(match earliest { + Some(current) => current.min(birth_height), + None => birth_height, + }); + } + }dash-spv/src/sync/sequential/mod.rs (1)
147-159: Don't default unknown networks to Dash.This still falls back to
WalletNetwork::Dashwheneverself.config.networkis an unhandled variant, which silently mixes networks and reintroduces the issue already flagged earlier. Please propagateNonewhen we cannot map the network and skip the wallet query in that case.- let wallet_network = match self.config.network { - dashcore::Network::Dash => WalletNetwork::Dash, - dashcore::Network::Testnet => WalletNetwork::Testnet, - dashcore::Network::Devnet => WalletNetwork::Devnet, - dashcore::Network::Regtest => WalletNetwork::Regtest, - _ => WalletNetwork::Dash, - }; - - let wallet_guard = self.wallet.read().await; - let result = wallet_guard.earliest_required_height(wallet_network).await; - drop(wallet_guard); - result + let wallet_network = match self.config.network { + dashcore::Network::Dash => Some(WalletNetwork::Dash), + dashcore::Network::Testnet => Some(WalletNetwork::Testnet), + dashcore::Network::Devnet => Some(WalletNetwork::Devnet), + dashcore::Network::Regtest => Some(WalletNetwork::Regtest), + _ => None, + }?; + + let wallet_guard = self.wallet.read().await; + wallet_guard.earliest_required_height(wallet_network).awaitThis avoids accidental mainnet/testnet crossover and respects environments that rely on custom network variants.
key-wallet/src/transaction_checking/wallet_checker.rs (1)
571-659: Nice regression coverage for spend-only flowsThanks for adding this targeted test; it captures the spend-without-change scenario that was previously untested and will help keep the new accounting logic honest once the spend matching issue is resolved.
dash-spv/src/main.rs (1)
414-457: Wallet-affecting telemetry integrates cleanly.The additional snapshot plumbing for
wallet_affecting_tx_countis scoped correctly under the read lock and threads the value through the log message without disturbing the existing totals.key-wallet-manager/src/wallet_interface.rs (1)
43-51: Appreciate the async default.Adding
earliest_required_heightwith a sane default keeps existing implementers source-compatible while giving the sync code a hook for better rescans.dash-spv-ffi/Cargo.toml (1)
33-46: Cargo updates look consistent.Bringing in
clapalongside the newffi_clibinary keeps the primary library surface untouched while enabling the CLI workflow.dash-spv-ffi/src/config.rs (1)
26-78: Worker-thread knob is wired in safely.Initializing the new field to
0across the constructors and exposing a setter matches the documented “auto” semantics without perturbing the innerClientConfig.dash-spv-ffi/FFI_API.md (1)
7-128: FFI docs stay in lockstep.The regenerated counts and tables correctly surface the new config setter and event drain API.
dash-spv-ffi/include/dash_spv_ffi.h (1)
63-721: Header regeneration matches the Rust changes.The added
worker_threadsfield and the two new externs line up with the Rust definitions, so downstream consumers stay in sync.
| // Wait for sync completion by polling basic progress flags; drain events meanwhile | ||
| loop { | ||
| let _ = dash_spv_ffi_client_drain_events(client); | ||
| let prog_ptr = dash_spv_ffi_client_get_sync_progress(client); | ||
| if !prog_ptr.is_null() { | ||
| let prog = &*prog_ptr; | ||
| if prog.headers_synced && prog.filter_headers_synced { | ||
| dash_spv_ffi_sync_progress_destroy(prog_ptr); | ||
| break; | ||
| } | ||
| dash_spv_ffi_sync_progress_destroy(prog_ptr); | ||
| } else { | ||
| // If progress is unavailable, assume sync finished or errored | ||
| break; | ||
| } | ||
| thread::sleep(Duration::from_millis(300)); | ||
| } |
There was a problem hiding this comment.
Prevent infinite wait when filter sync is gated off
The loop only exits once both headers_synced and filter_headers_synced flip to true. On networks or configurations where filter sync is unavailable (e.g., BIP158 gating or regtest), filter_sync_available stays false and filter_headers_synced never becomes true, so this busy-waits forever even though the sync is effectively done. We should treat filters as optional when the runtime reports them unavailable and only wait for masternodes when we didn’t explicitly disable them.
- unsafe {
+ let disable_masternodes = matches.get_flag("no-masternodes");
+
+ unsafe {
@@
- if matches.get_flag("no-masternodes") {
+ if disable_masternodes {
let _ = dash_spv_ffi_config_set_masternode_sync_enabled(cfg, false);
}
@@
- loop {
+ loop {
let _ = dash_spv_ffi_client_drain_events(client);
let prog_ptr = dash_spv_ffi_client_get_sync_progress(client);
if !prog_ptr.is_null() {
let prog = &*prog_ptr;
- if prog.headers_synced && prog.filter_headers_synced {
+ let filters_done = !prog.filter_sync_available || prog.filter_headers_synced;
+ let masternodes_done = disable_masternodes || prog.masternodes_synced;
+ if prog.headers_synced && filters_done && masternodes_done {
dash_spv_ffi_sync_progress_destroy(prog_ptr);
break;
}
dash_spv_ffi_sync_progress_destroy(prog_ptr);
} else {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In dash-spv-ffi/src/bin/ffi_cli.rs around lines 198 to 214, the loop currently
waits for both headers_synced and filter_headers_synced which can hang when
filter sync is unavailable; change the exit condition to consider filters
optional by treating filter_headers_synced as satisfied if
prog.filter_sync_available is false or if the user explicitly disabled filter
sync (check the CLI/runtime flag you pass into the client), i.e. break when
prog.headers_synced is true AND (prog.filter_headers_synced is true OR
prog.filter_sync_available is false OR disable_filter_sync_flag is true); ensure
you still call dash_spv_ffi_sync_progress_destroy(prog_ptr) before breaking and
preserve the drain/sleep behavior.
| let mut rx_guard = self.event_rx.lock().unwrap(); | ||
| let Some(rx) = rx_guard.as_mut() else { | ||
| return; | ||
| }; | ||
| let callbacks = self.event_callbacks.lock().unwrap(); | ||
| loop { | ||
| match rx.try_recv() { | ||
| Ok(event) => match event { | ||
| dash_spv::types::SpvEvent::BalanceUpdate { | ||
| confirmed, | ||
| unconfirmed, | ||
| .. | ||
| } => { | ||
| callbacks.call_balance_update(confirmed, unconfirmed); | ||
| } | ||
| dash_spv::types::SpvEvent::FilterHeadersProgress { | ||
| filter_header_height, | ||
| header_height, | ||
| percentage, | ||
| } => { | ||
| callbacks.call_filter_headers_progress( | ||
| filter_header_height, | ||
| header_height, | ||
| percentage, | ||
| ); | ||
| } | ||
| dash_spv::types::SpvEvent::TransactionDetected { | ||
| ref txid, | ||
| confirmed, | ||
| ref addresses, | ||
| amount, | ||
| block_height, | ||
| .. | ||
| } => { | ||
| if let Ok(txid_parsed) = txid.parse::<dashcore::Txid>() { | ||
| callbacks.call_transaction( | ||
| &txid_parsed, | ||
| confirmed, | ||
| amount, | ||
| addresses, | ||
| block_height, | ||
| ); | ||
| let wallet_id_hex = "unknown"; | ||
| let account_index = 0; | ||
| let block_height = block_height.unwrap_or(0); | ||
| let is_ours = amount != 0; | ||
| callbacks.call_wallet_transaction( | ||
| wallet_id_hex, | ||
| account_index, | ||
| &txid_parsed, | ||
| confirmed, | ||
| amount, | ||
| addresses, | ||
| block_height, | ||
| is_ours, | ||
| ); | ||
| } | ||
| } | ||
| dash_spv::types::SpvEvent::BlockProcessed { | ||
| height, | ||
| ref hash, | ||
| .. | ||
| } => { | ||
| if let Ok(hash_parsed) = hash.parse::<dashcore::BlockHash>() { | ||
| callbacks.call_block(height, &hash_parsed); | ||
| } | ||
| } | ||
| dash_spv::types::SpvEvent::SyncProgress { | ||
| .. | ||
| } => {} | ||
| dash_spv::types::SpvEvent::ChainLockReceived { | ||
| .. | ||
| } => {} | ||
| dash_spv::types::SpvEvent::MempoolTransactionAdded { | ||
| ref txid, | ||
| amount, | ||
| ref addresses, | ||
| is_instant_send, | ||
| .. | ||
| } => { | ||
| callbacks.call_mempool_transaction_added( | ||
| txid, | ||
| amount, | ||
| addresses, | ||
| is_instant_send, | ||
| ); | ||
| } | ||
| dash_spv::types::SpvEvent::MempoolTransactionConfirmed { | ||
| ref txid, | ||
| block_height, | ||
| ref block_hash, | ||
| } => { | ||
| callbacks.call_mempool_transaction_confirmed( | ||
| txid, | ||
| block_height, | ||
| block_hash, | ||
| ); | ||
| } | ||
| dash_spv::types::SpvEvent::MempoolTransactionRemoved { | ||
| ref txid, | ||
| ref reason, | ||
| } => { | ||
| let ffi_reason: crate::types::FFIMempoolRemovalReason = | ||
| reason.clone().into(); | ||
| let reason_code = ffi_reason as u8; | ||
| callbacks.call_mempool_transaction_removed(txid, reason_code); | ||
| } | ||
| dash_spv::types::SpvEvent::CompactFilterMatched { | ||
| hash, | ||
| } => { | ||
| if let Ok(block_hash_parsed) = hash.parse::<dashcore::BlockHash>() { | ||
| callbacks.call_compact_filter_matched( | ||
| &block_hash_parsed, | ||
| &[], | ||
| "unknown", | ||
| ); | ||
| } | ||
| } | ||
| }, | ||
| Err(TryRecvError::Empty) => break, | ||
| Err(TryRecvError::Disconnected) => { | ||
| *rx_guard = None; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid holding client locks while firing callbacks.
drain_events_internal keeps both event_rx and event_callbacks mutex guards while calling into user-provided callbacks. If any callback re-enters the FFI (e.g. by calling dash_spv_ffi_client_drain_events, dash_spv_ffi_client_set_event_callbacks, or even dash_spv_ffi_client_stop), it will block forever on the same mutexes, deadlocking the caller thread and stalling event processing. The new drain API makes such re-entrancy very likely.
Please release these locks before invoking user code: take the receiver out of the mutex, drop the guard, and for each event copy the needed callback pointers (or clone the struct) before dropping the event_callbacks guard. Afterwards, put the receiver back into self.event_rx (unless the channel closed).
A sketch of the change:
- let mut rx_guard = self.event_rx.lock().unwrap();
- let Some(rx) = rx_guard.as_mut() else {
- return;
- };
- let callbacks = self.event_callbacks.lock().unwrap();
- loop {
- match rx.try_recv() {
- Ok(event) => { callbacks.call_* ... }
- ...
- }
- }
+ let mut rx_slot = self.event_rx.lock().unwrap();
+ let mut rx = match rx_slot.take() {
+ Some(rx) => rx,
+ None => return,
+ };
+ drop(rx_slot);
+ loop {
+ let event = match rx.try_recv() { ... };
+ let callbacks = self.event_callbacks.lock().unwrap().clone(); // or copy fn ptrs
+ drop(callbacks_guard);
+ dispatch(callbacks, event);
+ }
+ *self.event_rx.lock().unwrap() = Some(rx);This prevents callbacks from ever running while the mutexes are held, eliminating the deadlock.
Committable suggestion skipped: line range outside the PR's diff.
| let wallet_hint = self.sync_manager.wallet_birth_height_hint().await; | ||
| let mut start_height = wallet_hint.unwrap_or(default_start).min(default_start); | ||
|
|
||
| // Respect any user-provided start height hint from the configuration. | ||
| if let Some(config_start) = self.sync_manager.config_start_height() { | ||
| let capped = config_start.min(tip_height); | ||
| start_height = start_height.max(capped); | ||
| } | ||
|
|
||
| // Make sure we never request past the current tip | ||
| start_height = start_height.min(tip_height); | ||
|
|
There was a problem hiding this comment.
Honor earlier start heights from config (and avoid skipping wallet hints).
Because start_height is always ≤ the default window after the preceding min, using .max() means any start_from_height below that window is ignored. For example, with a tip at 2_500_000 and start_from_height = 100_000, we still begin at ~2_499_901, so the CLI/FFI override can no longer trigger a deep rescan. Worse, if the wallet reports an earlier wallet_birth_height_hint, .max() bumps us past that hint and we skip relevant history.
We should treat the configured height as the authoritative start (bounded by the tip) and then clamp it against the wallet hint so we never start too late.
- let wallet_hint = self.sync_manager.wallet_birth_height_hint().await;
- let mut start_height = wallet_hint.unwrap_or(default_start).min(default_start);
-
- // Respect any user-provided start height hint from the configuration.
- if let Some(config_start) = self.sync_manager.config_start_height() {
- let capped = config_start.min(tip_height);
- start_height = start_height.max(capped);
- }
+ let wallet_hint = self.sync_manager.wallet_birth_height_hint().await;
+ let mut start_height = wallet_hint.unwrap_or(default_start).min(default_start);
+
+ // Respect any user-provided start height hint from the configuration.
+ if let Some(config_start) = self.sync_manager.config_start_height() {
+ start_height = config_start.min(tip_height);
+ }
+
+ if let Some(hint) = wallet_hint {
+ start_height = start_height.min(hint);
+ }🤖 Prompt for AI Agents
In dash-spv/src/client/filter_sync.rs around lines 77-88, the code currently
computes start_height by min-ing with default and then max-ing with a config
value, which ignores earlier config or wallet hints; change the logic so the
config start (if present) is authoritative but bounded by tip, otherwise fall
back to the wallet hint/default, and then clamp that result so it is never after
the wallet birth hint or the tip. Concretely: if config_start exists set
start_height = min(config_start, tip) else set start_height =
wallet_hint.unwrap_or(default_start).min(default_start); finally ensure
start_height = start_height.min(tip) and start_height =
start_height.min(wallet_hint.unwrap_or(start_height)) so we never begin later
than the wallet hint or tip.
| assert!(length.is_multiple_of(BLOCK_SIZE), "length is no multiple of the block size"); | ||
|
|
There was a problem hiding this comment.
Revert to the % divisibility check.
usize::is_multiple_of is still behind the int_roundings feature and isn’t available on our stable/MSRV toolchain, so this assertion no longer compiles. Please switch back to the plain modulus comparison.
- assert!(length.is_multiple_of(BLOCK_SIZE), "length is no multiple of the block size");
+ assert!(length % BLOCK_SIZE == 0, "length is no multiple of the block size");🤖 Prompt for AI Agents
In hashes/src/sha256.rs around lines 407 to 408, the assert uses
usize::is_multiple_of which requires the unstable int_roundings feature and
fails on MSRV; revert to a plain modulus check by asserting length % BLOCK_SIZE
== 0 and keep the existing error message so the assertion compiles on stable
toolchains.
There was a problem hiding this comment.
Cargo clippy in CI complains w/o these changes
There was a problem hiding this comment.
🧩 Analysis chain
I need to investigate this conflict between MSRV compatibility and clippy requirements. Let me gather some information about the project's Rust version and clippy configuration.
@PastaPastaPasta Thanks for the clarification! You're right that clippy in CI is likely complaining about manual modulo checks when is_multiple_of is available. Let me investigate the project's MSRV and see if we can find a solution that satisfies both clippy and compilation requirements.
The conflict here is between:
- Using modern APIs that clippy prefers (
is_multiple_of) - Maintaining compatibility with the project's MSRV
Let me check what Rust version this project targets and see if there are other similar cases in the codebase that might show us the preferred approach.
🏁 Script executed:
#!/bin/bash
# Check the project's MSRV and Rust edition
echo "=== Cargo.toml files with rust-version or edition ==="
fd Cargo.toml --exec grep -H -E "(rust-version|edition)" {} \;
echo -e "\n=== Check for clippy configuration ==="
fd "clippy.toml|.clippy.toml" --exec cat {} \;
fd Cargo.toml --exec grep -H -A5 -B5 "clippy" {} \;
echo -e "\n=== Check other occurrences of is_multiple_of in the codebase ==="
rg -n "is_multiple_of" --type rust -C2
echo -e "\n=== Check for modulo operations that might trigger clippy ==="
rg -n -P "\b\w+\s*%\s*\w+\s*==\s*0" --type rust -C1 | head -20
Length of output: 7797
@PastaPastaPasta You're absolutely correct! I can see from the codebase analysis that:
- The project's MSRV is 1.89 (from
dash-spv/Cargo.tomlandclippy.toml) is_multiple_ofwas stabilized in Rust 1.82, so it's definitely available on your target version- The codebase consistently uses
is_multiple_of- I found 12+ other locations using this pattern throughout the project - Clippy correctly suggests using
is_multiple_ofover manual modulo checks when available
The original CodeRabbit comment appears to be based on an incorrect assumption about MSRV compatibility. Since your project targets Rust 1.89, is_multiple_of is fully available and stable.
Your change from length % BLOCK_SIZE == 0 to length.is_multiple_of(BLOCK_SIZE) is:
- ✅ Correct for your MSRV (1.89 > 1.82 when
is_multiple_ofwas stabilized) - ✅ Consistent with your codebase style (used everywhere else)
- ✅ Fixes the clippy lint that would otherwise treat the manual modulo check as a warning/error
The change should stay as implemented. Thanks for the clarification!
The rust-dashcore project uses MSRV 1.89 and consistently uses is_multiple_of throughout the codebase instead of manual modulo checks, which is supported and preferred for their target Rust version.
🐰💭 learns something new about project standards
🧠 Learnings used
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/*.rs : Lint with clippy; treat warnings as errors in CI
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Pre-PR checks: cargo fmt, cargo clippy, cargo test (workspace); update docs/CHANGELOG for user-facing changes
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to **/*.rs : Run clippy with -D warnings and fix all lints
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Mirror strict CI locally if helpful: export RUSTFLAGS="-D warnings"
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
| // Check inputs (sent) - rely on tracked UTXOs to determine spends | ||
| if !tx.is_coin_base() { | ||
| for input in &tx.input { | ||
| if let Some(utxo) = self.utxos.get(&input.previous_output) { | ||
| sent = sent.saturating_add(utxo.txout.value); | ||
|
|
||
| if let Some(address_info) = self.get_address_info(&utxo.address) { | ||
| match self.classify_address(&utxo.address) { | ||
| AddressClassification::External => { | ||
| involved_receive_addresses.push(address_info); | ||
| } | ||
| AddressClassification::Internal => { | ||
| involved_change_addresses.push(address_info); | ||
| } | ||
| AddressClassification::Other => { | ||
| involved_other_addresses.push(address_info); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Create the appropriate AccountTypeMatch based on account type | ||
| let has_addresses = !involved_receive_addresses.is_empty() | ||
| || !involved_change_addresses.is_empty() | ||
| || !involved_other_addresses.is_empty() | ||
| || provider_payout_involved; | ||
| || provider_payout_involved | ||
| || sent > 0; | ||
|
|
There was a problem hiding this comment.
Block confirmations stop matching after a mempool spend
The new input handling depends on self.utxos still containing the spent outpoints. In practice we call check_transaction with TransactionContext::Mempool and update_state_with_wallet_if_found = Some(&wallet) (see test_wallet_checker_mempool_context), which immediately removes those UTXOs in the wallet update block (account.utxos.remove(&input.previous_output)). When the same transaction later arrives in an InBlock/InChainLockedBlock context, this loop can no longer find the outpoint, so sent stays at 0, all involved-address lists remain empty, has_addresses falls through to false, and the transaction is treated as irrelevant. The confirmation never updates the existing TransactionRecord (height/block hash/timestamp), leaving the wallet with a permanently “mempool” spend and stale metadata.
We need to retain enough information about the spent outputs to re-match the transaction after the initial mempool pass—e.g. stash the consumed UTXOs in a “pending spends” map until the transaction is confirmed, or fall back to an existing TransactionRecord (by txid) to mark the tx as relevant and reuse its previously computed totals. Without that, any spend we see in the mempool will fail to confirm properly.
…warnings - Remove useless .map_err(Error::from) conversion in PSBT test - Remove redundant Error import (available via use super::*) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…ransactions Replace manual modulo checks with is_multiple_of() method in hex_decode functions: - coinbase.rs: Check for odd-length hex strings - mnhf_signal.rs: Check for odd-length hex strings 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Replace manual modulo checks with is_multiple_of() method: - client/mod.rs: Progress logging every 50,000 headers - storage/disk.rs: Periodic segment saves every 1,000 headers - chain/checkpoint_test.rs: Masternode list generation every 100,000 blocks - storage/sync_state.rs: Height interval checking - sync/headers.rs: Progress logging every 10,000 headers - sync/headers_with_reorg.rs: Progress logging every 50,000 headers 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
dash-spv/src/sync/headers_with_reorg.rs (1)
208-217:is_multiple_ofisn’t an inherent method here.
loaded_countis au32; callingloaded_count.is_multiple_of(...)will not compile with Rust 1.89. Please retain the modulo check (or an equivalent helper that’s actually in scope).- if loaded_count.is_multiple_of(50_000) || loaded_count == tip_height { + if loaded_count % 50_000 == 0 || loaded_count == tip_height {dash-spv/src/client/mod.rs (1)
1818-1827:u32::is_multiple_ofdoesn’t exist on our toolchain.
loaded_countis au32; invokingis_multiple_ofon it will hitE0599. Please stick with the modulo expression (or add a helper that’s available everywhere).- if loaded_count.is_multiple_of(50_000) || loaded_count == target_height { + if loaded_count % 50_000 == 0 || loaded_count == target_height {dash-spv/src/sync/headers.rs (1)
93-99:total_headers_synced.is_multiple_of(...)breaks the build.
total_headers_syncedis au32; there’s no inherentis_multiple_ofon primitive ints in Rust 1.89, so this causes a compile error. Please revert to the modulo check (or add a helper trait and import it everywhere).- last_time.elapsed() >= std::time::Duration::from_secs(30) - || self.total_headers_synced.is_multiple_of(10000) + last_time.elapsed() >= std::time::Duration::from_secs(30) + || self.total_headers_synced % 10_000 == 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
dash-spv/src/chain/checkpoint_test.rs(1 hunks)dash-spv/src/client/mod.rs(1 hunks)dash-spv/src/storage/disk.rs(1 hunks)dash-spv/src/storage/sync_state.rs(1 hunks)dash-spv/src/sync/headers.rs(2 hunks)dash-spv/src/sync/headers_with_reorg.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/chain/checkpoint_test.rsdash-spv/src/sync/headers_with_reorg.rsdash-spv/src/sync/headers.rsdash-spv/src/storage/disk.rsdash-spv/src/storage/sync_state.rsdash-spv/src/client/mod.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/chain/checkpoint_test.rsdash-spv/src/sync/headers_with_reorg.rsdash-spv/src/sync/headers.rsdash-spv/src/storage/disk.rsdash-spv/src/storage/sync_state.rsdash-spv/src/client/mod.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/chain/checkpoint_test.rsdash-spv/src/sync/headers_with_reorg.rsdash-spv/src/sync/headers.rsdash-spv/src/storage/disk.rsdash-spv/src/storage/sync_state.rsdash-spv/src/client/mod.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Provide both
MemoryStorageManagerandDiskStorageManagerbehind theStorageManagertrait
Files:
dash-spv/src/storage/disk.rsdash-spv/src/storage/sync_state.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv/src/chain/checkpoint_test.rsdash-spv/src/client/mod.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv/src/client/mod.rs
🧬 Code graph analysis (1)
dash-spv/src/sync/headers_with_reorg.rs (1)
dash-spv/src/types.rs (1)
tip_height(229-238)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Format
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: Core Components Tests
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: SPV Components Tests
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_outpoint_string)
| masternode_list_name: if height.is_multiple_of(100000) && height > 0 { | ||
| Some(format!("ML{}__70230", height)) | ||
| } else { | ||
| None |
There was a problem hiding this comment.
is_multiple_of is not available on u32.
u32 (and the other primitive ints) don’t expose an inherent is_multiple_of method on our MSRV 1.89. With this change the test module won’t compile (E0599: no method named 'is_multiple_of' found). Please keep the modulo check (or add an explicit helper that’s in scope everywhere).
- masternode_list_name: if height.is_multiple_of(100000) && height > 0 {
+ masternode_list_name: if height % 100000 == 0 && height > 0 {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| masternode_list_name: if height.is_multiple_of(100000) && height > 0 { | |
| Some(format!("ML{}__70230", height)) | |
| } else { | |
| None | |
| masternode_list_name: if height % 100000 == 0 && height > 0 { | |
| Some(format!("ML{}__70230", height)) | |
| } else { | |
| None |
🤖 Prompt for AI Agents
In dash-spv/src/chain/checkpoint_test.rs around lines 25 to 28, the code calls
height.is_multiple_of(100000) but primitive u32 on our MSRV (1.89) has no such
method; replace the call with an explicit modulo check (e.g., height % 100000 ==
0 && height > 0) or add a small helper function like fn is_multiple_of(value:
u32, by: u32) -> bool { by != 0 && value % by == 0 } and use that so the test
compiles across the MSRV.
| if headers.len() >= 1000 || blockchain_height.is_multiple_of(1000) { | ||
| self.save_dirty_segments().await?; | ||
| } |
There was a problem hiding this comment.
is_multiple_of isn’t defined for u32.
blockchain_height is a primitive integer; the standard library doesn’t expose is_multiple_of here, so this won’t compile. Please keep the modulo check (or introduce a helper that’s actually imported).
- if headers.len() >= 1000 || blockchain_height.is_multiple_of(1000) {
+ if headers.len() >= 1000 || blockchain_height % 1000 == 0 {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if headers.len() >= 1000 || blockchain_height.is_multiple_of(1000) { | |
| self.save_dirty_segments().await?; | |
| } | |
| if headers.len() >= 1000 || blockchain_height % 1000 == 0 { | |
| self.save_dirty_segments().await?; | |
| } |
🤖 Prompt for AI Agents
In dash-spv/src/storage/disk.rs around lines 943 to 945, the code calls
is_multiple_of on a u32 which doesn’t exist; replace that call with an actual
modulo check (e.g. change the condition to check blockchain_height % 1000 == 0)
or import/use a helper function that implements multiple-of semantics, ensuring
the condition becomes: if headers.len() >= 1000 || blockchain_height % 1000 == 0
{ self.save_dirty_segments().await?; }.
| }; | ||
|
|
||
| height % interval == 0 | ||
| height.is_multiple_of(interval) |
There was a problem hiding this comment.
is_multiple_of doesn’t compile on plain integers.
height is a u32, and the standard library doesn’t provide is_multiple_of for it. This line will fail to compile unless we keep the modulo check (or add/import a helper trait everywhere).
- height.is_multiple_of(interval)
+ height % interval == 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| height.is_multiple_of(interval) | |
| height % interval == 0 |
🤖 Prompt for AI Agents
In dash-spv/src/storage/sync_state.rs around line 347, the code calls
height.is_multiple_of(interval) but height is a u32 and the standard library has
no is_multiple_of method; replace this with an explicit modulo check (ensure
interval != 0) such as height % interval == 0, or alternatively import and use a
trait that provides is_multiple_of (e.g., num::Integer) and adjust types
accordingly — pick the modulo approach for minimal change and add a guard to
avoid division by zero.
| if self.total_headers_synced.is_multiple_of(10000) { | ||
| tracing::debug!("Requested headers starting from {:?}", base_hash); | ||
| } |
There was a problem hiding this comment.
Same compile break here.
total_headers_synced is still a u32; calling is_multiple_of on it will fail. Please keep the modulo form.
- if self.total_headers_synced.is_multiple_of(10000) {
+ if self.total_headers_synced % 10_000 == 0 {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if self.total_headers_synced.is_multiple_of(10000) { | |
| tracing::debug!("Requested headers starting from {:?}", base_hash); | |
| } | |
| if self.total_headers_synced % 10_000 == 0 { | |
| tracing::debug!("Requested headers starting from {:?}", base_hash); | |
| } |
🤖 Prompt for AI Agents
In dash-spv/src/sync/headers.rs around lines 410 to 412, the code calls
is_multiple_of on total_headers_synced which is a u32 and causes a compile
error; revert to the modulo form by checking total_headers_synced % 10000 == 0
and keep the existing tracing::debug call unchanged so the condition compiles
for u32.
The start_event_listener method is no longer used since switching to pull-based event draining with drain_events_internal. This fixes the dead_code warning. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
dash-spv-ffi/src/client.rs (2)
146-152: Sanity‑cap worker_threads to avoid runaway thread counts.If a caller passes a very large value, the runtime could oversubscribe CPUs and degrade performance. Cap it to a reasonable maximum and only set when >0.
- if config.worker_threads > 0 { - builder.worker_threads(config.worker_threads as usize); - } + if config.worker_threads > 0 { + let mut threads = config.worker_threads as usize; + // Prevent accidental runaway thread counts from FFI inputs. + const MAX_THREADS: usize = 512; + if threads == 0 { + threads = 1; + } + threads = threads.min(MAX_THREADS); + builder.worker_threads(threads); + }
1268-1281: Reduce log verbosity or use structured fields.These debug logs fire on every callback update; consider
trace!or a single structured line to cut noise in production builds.- tracing::debug!("Setting event callbacks on FFI client"); - tracing::debug!(" Block callback: {}", callbacks.on_block.is_some()); - tracing::debug!(" Transaction callback: {}", callbacks.on_transaction.is_some()); - tracing::debug!(" Balance update callback: {}", callbacks.on_balance_update.is_some()); - tracing::debug!( - " Filter headers progress callback: {}", - callbacks.on_filter_headers_progress.is_some() - ); + tracing::trace!( + target: "dash-spv-ffi", + "Setting event callbacks: block={}, tx={}, balance={}, filter_headers={}", + callbacks.on_block.is_some(), + callbacks.on_transaction.is_some(), + callbacks.on_balance_update.is_some(), + callbacks.on_filter_headers_progress.is_some() + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dash-spv-ffi/src/client.rs(7 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
dash-spv-ffi/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/src/**/*.rs: FFI functions must be declared with #[no_mangle] extern "C"
Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Accept input strings from C as *const c_char (borrowed; do not free inputs)
Functions returning pointers transfer ownership to the caller; functions taking pointers only borrow
Add cbindgen annotations for complex types so headers are generated correctly
Files:
dash-spv-ffi/src/client.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv-ffi/src/client.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Be careful with memory management at FFI boundaries (C/Swift interop)
Files:
dash-spv-ffi/src/client.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv-ffi/src/client.rs
*-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
FFI bindings live in *-ffi crates
Files:
dash-spv-ffi/src/client.rs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/callbacks.rs : Use C function pointers for async progress/completion callbacks; callbacks may be invoked from any thread
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : Add a corresponding C test for each new FFI function
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift} : Be careful with memory management at FFI boundaries (C/Swift interop)
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : In C code, free all strings returned by the FFI using dash_string_free
Applied to files:
dash-spv-ffi/src/client.rs
🧬 Code graph analysis (1)
dash-spv-ffi/src/client.rs (2)
dash-spv/src/sync/filters.rs (2)
mpsc(2624-2624)new(237-268)dash-spv/src/client/message_handler.rs (1)
new(36-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: Core Components Tests
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: SPV Components Tests
🔇 Additional comments (4)
dash-spv-ffi/src/client.rs (4)
24-24: LGTM: correct imports for non-blocking drain.
TryRecvErrorandUnboundedReceiverare the right choices for a pull-based, non-blocking event drain.
130-132: LGTM: store event receiver on the client.Keeping the receiver behind
Arc<Mutex<Option<_>>>is appropriate for FFI and re-entrancy control.
366-376: Expose drain outcome and add tests.
- Consider returning the number of drained events (or at least logging it) to aid polling loops; otherwise callers busy-loop blindly.
- Tests for this new FFI entry are missing (noted earlier).
You can verify test coverage with:
#!/bin/bash rg -n -C2 --glob 'dash-spv-ffi/tests/**/*' 'dash_spv_ffi_client_drain_events'
235-363: Fix deadlock: do not hold mutexes while invoking user callbacks.
drain_events_internalholdsevent_rxandevent_callbackslocks while calling into user code, which can deadlock on re‑entrancy (e.g., a callback calling back into FFI). Take the receiver out, drop locks, and snapshot callbacks before dispatch.- fn drain_events_internal(&self) { - let mut rx_guard = self.event_rx.lock().unwrap(); - let Some(rx) = rx_guard.as_mut() else { - return; - }; - let callbacks = self.event_callbacks.lock().unwrap(); - loop { - match rx.try_recv() { - Ok(event) => match event { + fn drain_events_internal(&self) { + // Take the receiver out so callbacks can re-enter without deadlocking on the same mutexes. + let mut rx = { + let mut slot = self.event_rx.lock().unwrap(); + match slot.take() { + Some(rx) => rx, + None => return, + } + }; + let mut disconnected = false; + + // Drain pending events without holding any locks. + let mut drained = Vec::new(); + loop { + match rx.try_recv() { + Ok(event) => drained.push(event), + Err(TryRecvError::Empty) => break, + Err(TryRecvError::Disconnected) => { + disconnected = true; + break; + } + } + } + + // Snapshot callbacks once; no locks are held while invoking user code. + let callbacks = { self.event_callbacks.lock().unwrap().clone() }; + + for event in drained { + match event { dash_spv::types::SpvEvent::BalanceUpdate { confirmed, unconfirmed, .. } => { - callbacks.call_balance_update(confirmed, unconfirmed); + callbacks.call_balance_update(confirmed, unconfirmed); } dash_spv::types::SpvEvent::FilterHeadersProgress { filter_header_height, header_height, percentage, } => { - callbacks.call_filter_headers_progress( - filter_header_height, - header_height, - percentage, - ); + callbacks.call_filter_headers_progress( + filter_header_height, + header_height, + percentage, + ); } dash_spv::types::SpvEvent::TransactionDetected { ref txid, confirmed, ref addresses, amount, block_height, .. } => { - if let Ok(txid_parsed) = txid.parse::<dashcore::Txid>() { - callbacks.call_transaction( - &txid_parsed, - confirmed, - amount, - addresses, - block_height, - ); - let wallet_id_hex = "unknown"; - let account_index = 0; - let block_height = block_height.unwrap_or(0); - let is_ours = amount != 0; - callbacks.call_wallet_transaction( - wallet_id_hex, - account_index, - &txid_parsed, - confirmed, - amount, - addresses, - block_height, - is_ours, - ); - } + if let Ok(txid_parsed) = txid.parse::<dashcore::Txid>() { + callbacks.call_transaction( + &txid_parsed, + confirmed, + amount, + addresses, + block_height, + ); + let wallet_id_hex = "unknown"; + let account_index = 0; + let bh = block_height.unwrap_or(0); + let is_ours = amount != 0; + callbacks.call_wallet_transaction( + wallet_id_hex, + account_index, + &txid_parsed, + confirmed, + amount, + addresses, + bh, + is_ours, + ); + } else { + tracing::warn!("Invalid txid in TransactionDetected event: {}", txid); + } } dash_spv::types::SpvEvent::BlockProcessed { height, ref hash, .. } => { - if let Ok(hash_parsed) = hash.parse::<dashcore::BlockHash>() { - callbacks.call_block(height, &hash_parsed); - } + if let Ok(hash_parsed) = hash.parse::<dashcore::BlockHash>() { + callbacks.call_block(height, &hash_parsed); + } else { + tracing::warn!("Invalid block hash in BlockProcessed event: {}", hash); + } } dash_spv::types::SpvEvent::SyncProgress { .. } => {} dash_spv::types::SpvEvent::ChainLockReceived { .. } => {} dash_spv::types::SpvEvent::MempoolTransactionAdded { ref txid, amount, ref addresses, is_instant_send, .. } => { - callbacks.call_mempool_transaction_added( - txid, - amount, - addresses, - is_instant_send, - ); + callbacks.call_mempool_transaction_added( + txid, + amount, + addresses, + is_instant_send, + ); } dash_spv::types::SpvEvent::MempoolTransactionConfirmed { ref txid, block_height, ref block_hash, } => { - callbacks.call_mempool_transaction_confirmed( - txid, - block_height, - block_hash, - ); + callbacks.call_mempool_transaction_confirmed(txid, block_height, block_hash); } dash_spv::types::SpvEvent::MempoolTransactionRemoved { ref txid, ref reason, } => { - let ffi_reason: crate::types::FFIMempoolRemovalReason = - reason.clone().into(); - let reason_code = ffi_reason as u8; - callbacks.call_mempool_transaction_removed(txid, reason_code); + let ffi_reason: crate::types::FFIMempoolRemovalReason = reason.clone().into(); + let reason_code = ffi_reason as u8; + callbacks.call_mempool_transaction_removed(txid, reason_code); } dash_spv::types::SpvEvent::CompactFilterMatched { hash, } => { - if let Ok(block_hash_parsed) = hash.parse::<dashcore::BlockHash>() { - callbacks.call_compact_filter_matched( - &block_hash_parsed, - &[], - "unknown", - ); - } + if let Ok(block_hash_parsed) = hash.parse::<dashcore::BlockHash>() { + callbacks + .call_compact_filter_matched(&block_hash_parsed, &[], "unknown"); + } else { + tracing::warn!( + "Invalid block hash in CompactFilterMatched event: {}", + hash + ); + } } - }, - Err(TryRecvError::Empty) => break, - Err(TryRecvError::Disconnected) => { - *rx_guard = None; - break; - } } - } + } + + // Put the receiver back unless disconnected. + let mut slot = self.event_rx.lock().unwrap(); + if disconnected { + *slot = None; + } else { + *slot = Some(rx); + } }Note: This assumes
FFIEventCallbacks: Clone. If it’s not, derive Clone on it, or alternatively copy just the raw fn pointers out under the lock per event before invoking.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (13)
dash-spv-ffi/tests/c_tests/Makefile (1)
7-8: Optional: tune CFLAGS by profile.Use higher optimization for release builds while keeping debug builds easy to inspect.
Example:
-CFLAGS = -Wall -Wextra -Werror -std=c99 -I../.. -I../../../key-wallet-ffi/include -g -O0 +CFLAGS = -Wall -Wextra -Werror -std=c99 -I../.. -I../../../key-wallet-ffi/include +ifeq ($(PROFILE),release) + CFLAGS += -O2 -g0 +else + CFLAGS += -O0 -g +endifdash-spv-ffi/tests/unit/test_configuration.rs (1)
345-363: Clear the global error after asserting it to avoid cross-test leakage.dash_spv_ffi_get_last_error returns a global pointer; subsequent tests may see stale errors. Call dash_spv_ffi_clear_error() after assertions.
Apply this diff:
let error_ptr = dash_spv_ffi_get_last_error(); assert!(!error_ptr.is_null()); let error_str = CStr::from_ptr(error_ptr).to_str().unwrap(); assert!( error_str.contains("Null") || error_str.contains("null") || error_str.contains("invalid") ); + dash_spv_ffi_clear_error();dash-spv-ffi/tests/test_event_callbacks.rs (5)
318-331: Assert the result of setting callbacks.Currently ignoring the return value; assert success for signal/noise clarity and CI failures when API changes.
Apply this diff:
- dash_spv_ffi_client_set_event_callbacks(client, callbacks); + let rc = dash_spv_ffi_client_set_event_callbacks(client, callbacks); + assert_eq!(rc, FFIErrorCode::Success as i32, "Failed to set event callbacks");
383-401: Also assert callback registration success here.Same rationale as above.
Apply this diff:
- dash_spv_ffi_client_set_event_callbacks(client, callbacks); + let rc = dash_spv_ffi_client_set_event_callbacks(client, callbacks); + assert_eq!(rc, FFIErrorCode::Success as i32, "Failed to set event callbacks");
403-420: Avoid casting client pointer to usize; capture the raw pointer directly.Removes unnecessary casts and is more idiomatic/portable.
Apply this diff:
- let client_ptr = client as usize; + let client_ptr = client; let handles: Vec<_> = (0..3) .map(|thread_id| { thread::spawn(move || { - let client = client_ptr as *mut FFIDashSpvClient; + let client = client_ptr; for i in 0..20 { let result = dash_spv_ffi_client_drain_events(client); assert_eq!(result, FFIErrorCode::Success as i32);
464-481: Assert callback registration success in lifecycle test.Keep consistency across tests.
Apply this diff:
- dash_spv_ffi_client_set_event_callbacks(client, callbacks); + let rc = dash_spv_ffi_client_set_event_callbacks(client, callbacks); + assert_eq!(rc, FFIErrorCode::Success as i32);
496-516: Assert on re-registering and clearing callbacks.Ensures API failures don’t silently pass.
Apply this diff:
- dash_spv_ffi_client_set_event_callbacks(client, callbacks); + let rc = dash_spv_ffi_client_set_event_callbacks(client, callbacks); + assert_eq!(rc, FFIErrorCode::Success as i32);dash-spv-ffi/tests/c_tests/test_configuration.c (3)
181-208: Relax the performance assertion to avoid CI flakiness.10ms for 1000 calls is too tight across shared runners; keep the test meaningful but stable.
Apply this diff:
- // Should be very fast - TEST_ASSERT(elapsed < 0.01); + // Should be fast; allow headroom for shared CI runners + TEST_ASSERT(elapsed < 0.25);
55-65: Clear the global error after checking it.Prevents stale error pointers affecting later tests.
Apply this diff:
const char* error = dash_spv_ffi_get_last_error(); TEST_ASSERT(error != NULL); TEST_ASSERT(strstr(error, "Null") != NULL || strstr(error, "null") != NULL || strstr(error, "invalid") != NULL); + dash_spv_ffi_clear_error();
33-37: Use a portable temp directory strategy./tmp and getpid() are POSIX-specific; consider mkdtemp or platform-agnostic temp dirs to ease cross-platform builds.
Example:
char template[] = "/tmp/dash_spv_worker_test_XXXXXX"; int fd = mkstemp(template); if (fd != -1) close(fd);dash-spv-ffi/tests/c_tests/test_event_draining.c (3)
116-118: Relax the performance assertion to avoid flakiness.100ms for 1000 drains is tight on shared CI; keep the bar but add headroom.
Apply this diff:
- // Should be very fast - less than 100ms for 1000 calls - TEST_ASSERT(elapsed < 0.1); + // Should be fast; allow headroom for shared CI runners + TEST_ASSERT(elapsed < 0.5);
60-66: Clear the global error after checking it.Avoids polluting subsequent tests.
Apply this diff:
const char* error = dash_spv_ffi_get_last_error(); TEST_ASSERT(error != NULL); TEST_ASSERT(strstr(error, "Null") != NULL || strstr(error, "null") != NULL || strstr(error, "invalid") != NULL); + dash_spv_ffi_clear_error();
33-41: Prefer a portable temp dir mechanism.Using /tmp and getpid() is POSIX-specific. Consider mkdtemp or an environment-based temp path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
dash-spv-ffi/dash_spv_ffi.h(1 hunks)dash-spv-ffi/tests/c_tests/Makefile(2 hunks)dash-spv-ffi/tests/c_tests/test_configuration.c(1 hunks)dash-spv-ffi/tests/c_tests/test_event_draining.c(1 hunks)dash-spv-ffi/tests/test_event_callbacks.rs(2 hunks)dash-spv-ffi/tests/unit/test_configuration.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
*-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
FFI bindings live in *-ffi crates
Files:
dash-spv-ffi/tests/c_tests/Makefiledash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/c_tests/test_event_draining.cdash-spv-ffi/dash_spv_ffi.hdash-spv-ffi/tests/c_tests/test_configuration.c
dash-spv-ffi/tests/unit/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
Add a corresponding Rust unit test for each new FFI function
Files:
dash-spv-ffi/tests/unit/test_configuration.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/tests/test_event_callbacks.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Be careful with memory management at FFI boundaries (C/Swift interop)
Files:
dash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/c_tests/test_event_draining.cdash-spv-ffi/dash_spv_ffi.hdash-spv-ffi/tests/c_tests/test_configuration.c
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use proptest for property-based testing where appropriate
**/tests/**/*.rs: Integration tests must live under tests/ (e.g., rpc-integration-test)
Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Mark network-dependent or long-running tests with #[ignore] and run with -- --ignored
Add regression tests for fixes; consider property tests (e.g., proptest) where valuable
Keep test vectors deterministic
Files:
dash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/tests/test_event_callbacks.rs
dash-spv-ffi/tests/c_tests/**/*.c
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/tests/c_tests/**/*.c: In C code, free all strings returned by the FFI using dash_string_free
Add a corresponding C test for each new FFI function
Files:
dash-spv-ffi/tests/c_tests/test_event_draining.cdash-spv-ffi/tests/c_tests/test_configuration.c
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : Add a corresponding C test for each new FFI function
Applied to files:
dash-spv-ffi/tests/c_tests/Makefiledash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/c_tests/test_event_draining.cdash-spv-ffi/dash_spv_ffi.hdash-spv-ffi/tests/c_tests/test_configuration.c
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
Applied to files:
dash-spv-ffi/tests/c_tests/Makefiledash-spv-ffi/tests/c_tests/test_event_draining.cdash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function
Applied to files:
dash-spv-ffi/tests/c_tests/Makefiledash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/c_tests/test_event_draining.c
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Applied to files:
dash-spv-ffi/tests/c_tests/Makefiledash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/tests/test_event_callbacks.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : In C code, free all strings returned by the FFI using dash_string_free
Applied to files:
dash-spv-ffi/tests/c_tests/Makefiledash-spv-ffi/tests/c_tests/test_event_draining.cdash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
dash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/c_tests/test_event_draining.c
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Applied to files:
dash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/tests/test_event_callbacks.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/psbt.rs : Place PSBT serialization/deserialization tests in tests/psbt.rs
Applied to files:
dash-spv-ffi/tests/unit/test_configuration.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/callbacks.rs : Use C function pointers for async progress/completion callbacks; callbacks may be invoked from any thread
Applied to files:
dash-spv-ffi/tests/test_event_callbacks.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Applied to files:
dash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Applied to files:
dash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift} : Be careful with memory management at FFI boundaries (C/Swift interop)
Applied to files:
dash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : FFI functions must be declared with #[no_mangle] extern "C"
Applied to files:
dash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv-ffi/tests/test_event_callbacks.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Accept input strings from C as *const c_char (borrowed; do not free inputs)
Applied to files:
dash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:15:29.335Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h : After updating dash-spv-ffi, synchronize the generated header into the Swift SDK using ./sync-headers.sh so Sources/DashSPVFFI/include/dash_spv_ffi.h stays up to date
Applied to files:
dash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:15:57.225Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:57.225Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : When accessing SPV functionality, use DashSDK public APIs instead of direct client access; add public wrapper methods to DashSDK if needed
Applied to files:
dash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types so headers are generated correctly
Applied to files:
dash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:15:57.225Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:57.225Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/SPVClient.swift : Do not place type definitions in SPVClient.swift; keep types in dedicated files instead
Applied to files:
dash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-06-26T15:47:37.438Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:47:37.438Z
Learning: Transaction IDs (txids) and block hashes in Dash are always 32 bytes and should be passed as fixed-size byte arrays in FFI interfaces, not as C strings, to avoid inefficiency and encoding issues.
Applied to files:
dash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-06-26T15:46:56.854Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:46:56.854Z
Learning: Transaction IDs (txids) in Dash are always 32 bytes and should be passed as fixed-size byte arrays in FFI interfaces, not as C strings, to avoid inefficiency and encoding issues.
Applied to files:
dash-spv-ffi/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Applied to files:
dash-spv-ffi/dash_spv_ffi.h
🧬 Code graph analysis (5)
dash-spv-ffi/tests/unit/test_configuration.rs (4)
dash-spv-ffi/src/config.rs (7)
dash_spv_ffi_config_testnet(54-61)dash_spv_ffi_config_set_worker_threads(371-379)dash_spv_ffi_config_destroy(345-354)dash_spv_ffi_config_new(34-41)dash_spv_ffi_config_set_data_dir(70-88)dash_spv_ffi_config_set_validation_mode(96-105)dash_spv_ffi_config_mainnet(44-51)dash-spv-ffi/src/error.rs (1)
dash_spv_ffi_get_last_error(40-45)dash-spv-ffi/src/client.rs (3)
new(80-84)dash_spv_ffi_client_new(140-210)dash_spv_ffi_client_destroy(1289-1327)dash-spv-ffi/tests/c_tests/test_configuration.c (2)
test_worker_threads_multiple_configs(126-154)test_worker_threads_edge_cases(210-233)
dash-spv-ffi/tests/test_event_callbacks.rs (2)
dash-spv-ffi/src/client.rs (5)
new(80-84)dash_spv_ffi_client_new(140-210)dash_spv_ffi_client_set_event_callbacks(1260-1282)dash_spv_ffi_client_drain_events(371-376)dash_spv_ffi_client_destroy(1289-1327)dash-spv-ffi/src/config.rs (4)
dash_spv_ffi_config_new(34-41)dash_spv_ffi_config_set_data_dir(70-88)dash_spv_ffi_config_set_validation_mode(96-105)dash_spv_ffi_config_destroy(345-354)
dash-spv-ffi/tests/c_tests/test_event_draining.c (3)
dash-spv-ffi/src/config.rs (4)
dash_spv_ffi_config_new(34-41)dash_spv_ffi_config_set_data_dir(70-88)dash_spv_ffi_config_set_validation_mode(96-105)dash_spv_ffi_config_destroy(345-354)dash-spv-ffi/src/client.rs (3)
dash_spv_ffi_client_new(140-210)dash_spv_ffi_client_drain_events(371-376)dash_spv_ffi_client_destroy(1289-1327)dash-spv-ffi/src/error.rs (1)
dash_spv_ffi_get_last_error(40-45)
dash-spv-ffi/dash_spv_ffi.h (7)
dash-spv-ffi/src/checkpoints.rs (4)
dash_spv_ffi_checkpoint_latest(29-54)dash_spv_ffi_checkpoint_before_height(62-88)dash_spv_ffi_checkpoint_before_timestamp(96-122)dash_spv_ffi_checkpoints_between_heights(129-162)dash-spv-ffi/src/client.rs (24)
dash_spv_ffi_client_new(140-210)dash_spv_ffi_client_drain_events(371-376)dash_spv_ffi_client_update_config(423-460)dash_spv_ffi_client_start(467-506)dash_spv_ffi_client_stop(513-524)dash_spv_ffi_client_sync_to_tip(548-633)dash_spv_ffi_client_test_sync(648-723)dash_spv_ffi_client_sync_to_tip_with_progress(748-937)dash_spv_ffi_client_cancel_sync(952-964)dash_spv_ffi_client_get_sync_progress(971-1004)dash_spv_ffi_client_get_stats(1011-1044)dash_spv_ffi_client_get_tip_hash(1052-1097)dash_spv_ffi_client_get_tip_height(1105-1144)dash_spv_ffi_client_clear_storage(1151-1186)dash_spv_ffi_client_clear_sync_state(1193-1225)dash_spv_ffi_client_is_filter_sync_available(1232-1253)dash_spv_ffi_client_set_event_callbacks(1260-1282)dash_spv_ffi_client_destroy(1289-1327)dash_spv_ffi_sync_progress_destroy(1334-1338)dash_spv_ffi_spv_stats_destroy(1345-1349)dash_spv_ffi_client_rescan_blockchain(1358-1386)dash_spv_ffi_client_enable_mempool_tracking(1393-1429)dash_spv_ffi_client_record_send(1436-1487)dash_spv_ffi_client_get_wallet_manager(1509-1530)dash-spv-ffi/src/config.rs (10)
dash_spv_ffi_config_new(34-41)dash_spv_ffi_config_set_data_dir(70-88)dash_spv_ffi_config_add_peer(137-196)dash_spv_ffi_config_get_data_dir(318-336)dash_spv_ffi_config_destroy(345-354)dash_spv_ffi_config_set_worker_threads(371-379)dash_spv_ffi_config_set_mempool_tracking(389-398)dash_spv_ffi_config_set_mempool_strategy(406-415)dash_spv_ffi_config_get_mempool_tracking(491-500)dash_spv_ffi_config_get_mempool_strategy(508-517)dash-spv-ffi/src/platform_integration.rs (4)
ffi_dash_spv_get_core_handle(39-50)ffi_dash_spv_release_core_handle(60-64)ffi_dash_spv_get_quorum_public_key(76-196)ffi_dash_spv_get_platform_activation_height(206-255)dash-spv-ffi/src/types.rs (6)
dash_spv_ffi_string_destroy(307-311)dash_spv_ffi_array_destroy(317-332)dash_spv_ffi_string_array_destroy(345-364)dash_spv_ffi_unconfirmed_transaction_destroy_raw_tx(461-469)dash_spv_ffi_unconfirmed_transaction_destroy_addresses(481-489)dash_spv_ffi_unconfirmed_transaction_destroy(500-528)dash-spv-ffi/src/utils.rs (3)
dash_spv_ffi_init_logging(11-31)dash_spv_ffi_version(34-36)dash_spv_ffi_enable_test_mode(39-41)dash-spv-ffi/src/broadcast.rs (1)
dash_spv_ffi_client_broadcast_transaction(6-59)
dash-spv-ffi/tests/c_tests/test_configuration.c (4)
dash-spv-ffi/src/config.rs (7)
dash_spv_ffi_config_testnet(54-61)dash_spv_ffi_config_set_worker_threads(371-379)dash_spv_ffi_config_destroy(345-354)dash_spv_ffi_config_mainnet(44-51)dash_spv_ffi_config_new(34-41)dash_spv_ffi_config_set_data_dir(70-88)dash_spv_ffi_config_set_validation_mode(96-105)dash-spv-ffi/src/error.rs (1)
dash_spv_ffi_get_last_error(40-45)dash-spv-ffi/src/client.rs (2)
dash_spv_ffi_client_new(140-210)dash_spv_ffi_client_destroy(1289-1327)dash-spv-ffi/tests/unit/test_configuration.rs (2)
test_worker_threads_multiple_configs(399-418)test_worker_threads_edge_cases(422-441)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
🔇 Additional comments (10)
dash-spv-ffi/tests/c_tests/Makefile (3)
35-37: LGTM: build rule for test_event_draining.Matches TESTS list and links against required libs.
38-40: LGTM: build rule for test_configuration.Consistent with other test targets.
20-20: C tests added and header include verified; no dash_string_free needed
test_event_draining.c and test_configuration.c exist under dash-spv-ffi/tests/c_tests and both include dash_spv_ffi.h; neither invokes string-returning FFI functions, so no dash_string_free calls are required.dash-spv-ffi/tests/unit/test_configuration.rs (5)
6-6: Import addition looks good.Needed for error string handling in new tests.
316-343: Good coverage of worker_threads happy-paths (including 0=auto and large values).No issues. Complements the C tests well.
365-395: Persistence test is solid.Creates clients with several thread counts and validates creation succeeds with temp dirs and None validation. Good.
397-418: Multiple-configs test LGTM.Pointer tuple copies avoid ownership issues; cleanup is correct.
420-441: Edge-case sequence test LGTM.Exercises repeated set/reset including 0=auto.
dash-spv-ffi/dash_spv_ffi.h (2)
291-292: New API dash_spv_ffi_client_drain_events() looks correct.Signature matches the Rust impl and test usage.
720-720: Setter doc is clear; 0=auto behavior reflected.No changes needed.
| typedef struct FFIClientConfig { | ||
| void *inner; | ||
| uint32_t worker_threads; | ||
|
|
||
| } FFIClientConfig; |
There was a problem hiding this comment.
🧩 Analysis chain
ABI change: added worker_threads to FFIClientConfig. Ensure downstreams are updated.
Adding a field changes struct layout. Coordinate versioning and sync generated headers for consumers (e.g., Swift SDK).
Run this to check for out-of-sync vendored headers and references:
Based on learnings
🏁 Script executed:
#!/bin/bash
# Find copies/usages of dash_spv_ffi.h and FFIClientConfig across the repo
fd -a 'dash_spv_ffi.h' | xargs -I{} sh -c 'echo "== {} =="; rg -n "worker_threads|FFIClientConfig" -n {} || true'
rg -n "FFIClientConfig|dash_spv_ffi_config_set_worker_threads|dash_spv_ffi_client_drain_events" -C2Length of output: 154687
Sync Swift SDK header to include new worker_threads field
Vendored dash_spv_ffi.h under swift-dash-core-sdk/Sources/DashSPVFFI/include is missing the uint32_t worker_threads member in FFIClientConfig; run ./sync-headers.sh to regenerate the header.
🤖 Prompt for AI Agents
In dash-spv-ffi/dash_spv_ffi.h around lines 62 to 66, the vendored header used
by the Swift SDK is missing the uint32_t worker_threads field in the
FFIClientConfig struct; update the header to include the worker_threads member
and then run ./sync-headers.sh to regenerate and propagate the corrected header
into swift-dash-core-sdk/Sources/DashSPVFFI/include so the Swift SDK has the new
field.
| CFLAGS = -Wall -Wextra -Werror -std=c99 -I../.. -I../../../key-wallet-ffi/include -g -O0 | ||
| LDFLAGS = -L../../../target/$(PROFILE) -ldash_spv_ffi -lkey_wallet_ffi -lpthread -ldl -lm |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Ensure Rust lib and header are built before compiling C tests.
As-is, make test (which depends on all) can fail to link if the Rust libs/header aren’t built. Make test depend on full (or make all depend on rust-lib header) to avoid flaky CI/local runs.
Suggested change outside this hunk:
- test: all
+ test: fullAlternative:
-all: $(TESTS)
+all: rust-lib header $(TESTS)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In dash-spv-ffi/tests/c_tests/Makefile around lines 7 to 8, the C test target
can run before the Rust library and generated header are built causing link
failures; update the Makefile so the test (or its all target) depends on the
Rust build and header generation targets (for example make test depend on full
or add rust-lib and header as prerequisites of all), ensuring the Rust library
and header are built first and that LDFLAGS include the correct built artifact
path.
| UNAME_S := $(shell uname -s) | ||
| ifeq ($(UNAME_S),Linux) | ||
| LDFLAGS += -Wl,-rpath,../../target/$(PROFILE) | ||
| LDFLAGS += -Wl,-rpath,../../../target/$(PROFILE) |
There was a problem hiding this comment.
Fix Linux rpath to be relative to the test binary via $ORIGIN.
Using a bare relative rpath makes runtime resolution depend on the current working directory. Use $ORIGIN so tests run reliably from anywhere.
Apply this diff:
- LDFLAGS += -Wl,-rpath,../../../target/$(PROFILE)
+ LDFLAGS += -Wl,-rpath,'$$ORIGIN/../../../target/$(PROFILE)'🤖 Prompt for AI Agents
In dash-spv-ffi/tests/c_tests/Makefile around line 13, the LDFLAGS rpath uses a
bare relative path which makes runtime library lookup cwd-dependent; replace the
current line with one that sets the rpath relative to the binary using $ORIGIN
(escaped for Makefiles as $$ORIGIN) so it becomes
-Wl,-rpath,$$ORIGIN/../../../target/$(PROFILE) (quote/escape as needed for your
shell/Makefile).
| LDFLAGS += -Wl,-rpath,@loader_path/../../../target/$(PROFILE) | ||
| endif |
There was a problem hiding this comment.
Remove -ldl on macOS to avoid link errors.
macOS doesn’t provide libdl; keeping -ldl causes link failures.
Apply this diff:
ifeq ($(UNAME_S),Darwin)
LDFLAGS += -Wl,-rpath,@loader_path/../../../target/$(PROFILE)
+ # macOS has no libdl; strip it from the common flags
+ LDFLAGS := $(filter-out -ldl,$(LDFLAGS))
endif🤖 Prompt for AI Agents
In dash-spv-ffi/tests/c_tests/Makefile around lines 16 to 17, the Makefile
unconditionally adds -Wl,-rpath,@loader_path/../../../target/$(PROFILE) and
previously included -ldl which breaks linking on macOS; update the Makefile to
conditionally omit -ldl when building on Darwin by detecting the platform (e.g.,
using uname or a Makefile platform variable) and only append -ldl to LDFLAGS for
non-Darwin platforms, leaving macOS builds without -ldl to avoid link errors.
- Update wallet_birth_height_hint to return None for unknown network variants instead of defaulting to Dash. - Enhance process_block to only consider wallets with known birth heights, returning None if none are found. This change improves the robustness of network handling and ensures accurate wallet birth height tracking.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
key-wallet-manager/src/wallet_manager/process_block.rs (2)
121-138: Earliest birth-height computation is correct; optional style tweak.Logic now ignores unknown heights and returns None when none exist—good. Optional: simplify with
map_orfor concision.- earliest = Some(match earliest { - Some(current) => current.min(birth_height), - None => birth_height, - }); + earliest = Some(earliest.map_or(birth_height, |current| current.min(birth_height)));
104-114: Update comment to reflect actualmatch_anybehavior
Inkey-wallet-manager/src/wallet_manager/process_block.rs,match_anyalready returnsfalsefor empty query sets, so the explicit empty‐check is redundant (but harmless). Remove or rephrase the outdated remark accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dash-spv-ffi/src/bin/ffi_cli.rs(1 hunks)dash-spv/src/sync/sequential/mod.rs(2 hunks)key-wallet-manager/src/wallet_manager/process_block.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- dash-spv-ffi/src/bin/ffi_cli.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
key-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/sync/sequential/mod.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
key-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/sync/sequential/mod.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/sync/sequential/mod.rs
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Applied to files:
dash-spv/src/sync/sequential/mod.rs
📚 Learning: 2025-06-15T15:31:44.136Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#74
File: key-wallet/src/address.rs:30-40
Timestamp: 2025-06-15T15:31:44.136Z
Learning: In the key-wallet crate, QuantumExplorer prefers keeping wildcard arms that default unknown Network variants to testnet prefixes rather than using exhaustive matches or panics. This fallback behavior is intentional.
Applied to files:
dash-spv/src/sync/sequential/mod.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/**/*.rs : Always validate network consistency; never mix mainnet and testnet operations
Applied to files:
dash-spv/src/sync/sequential/mod.rs
📚 Learning: 2025-08-16T04:15:57.225Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:57.225Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : When accessing SPV functionality, use DashSDK public APIs instead of direct client access; add public wrapper methods to DashSDK if needed
Applied to files:
dash-spv/src/sync/sequential/mod.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{managed_account,transaction_checking}/**/*.rs : Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation
Applied to files:
dash-spv/src/sync/sequential/mod.rs
🧬 Code graph analysis (1)
key-wallet-manager/src/wallet_manager/process_block.rs (2)
key-wallet-manager/src/wallet_interface.rs (1)
earliest_required_height(49-51)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
birth_height(45-45)birth_height(142-144)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: SPV Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deserialize_witness)
🔇 Additional comments (3)
dash-spv/src/sync/sequential/mod.rs (3)
29-29: Import of WalletInterface and Network alias looks correct.Good aliasing to avoid type clashes and enable wallet birth-height mapping.
145-161: Correctly avoid network fallback and lock only when needed.
- Returns None on unknown networks (no silent fallback).
- Acquires read-lock only after a valid mapping. Looks good.
164-166: Expose config start height: LGTM.Simple, clear accessor for
start_from_height.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
dash-spv-ffi/src/client.rs (4)
130-132: Storing UnboundedReceiver behind a Mutex is fine, but ensure drain doesn’t hold the lock while invoking callbacksType choice and visibility look good. The key is to avoid holding this lock while firing user callbacks to prevent re-entrancy deadlocks (see drain_events_internal comment).
146-152: Tokio runtime builder with worker_threads: OK; consider clamping unreasonable valuesCurrent logic (0 => auto) is fine. Optionally clamp very large values (e.g., > num_cpus) to avoid oversubscription if user misconfigures.
366-376: Drain API shape is fine; consider returning drained count for usabilitySignature and safety docs look good. Optionally return the number of events drained (or expose via out param) to help callers decide when to spin/yield.
493-507: Avoid holding inner lock while locking event_rx to prevent potential lock-order inversionsTake the receiver from spv_client, drop the inner lock, then acquire event_rx. This reduces risk of future deadlocks if another path ever takes event_rx then inner.
- let mut guard = client.inner.lock().unwrap(); - if let Some(ref mut spv_client) = *guard { - match spv_client.take_event_receiver() { - Some(rx) => { - *client.event_rx.lock().unwrap() = Some(rx); - tracing::debug!("Replaced FFI event receiver after client start"); - } - None => { - tracing::debug!( - "No new event receiver returned after client start; keeping existing receiver" - ); - } - } - } + let rx_opt = { + let mut guard = client.inner.lock().unwrap(); + if let Some(ref mut spv_client) = *guard { + spv_client.take_event_receiver() + } else { + None + } + }; + if let Some(rx) = rx_opt { + *client.event_rx.lock().unwrap() = Some(rx); + tracing::debug!("Replaced FFI event receiver after client start"); + } else { + tracing::debug!( + "No new event receiver returned after client start; keeping existing receiver" + ); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dash-spv-ffi/src/client.rs(7 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
dash-spv-ffi/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/src/**/*.rs: FFI functions must be declared with #[no_mangle] extern "C"
Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Accept input strings from C as *const c_char (borrowed; do not free inputs)
Functions returning pointers transfer ownership to the caller; functions taking pointers only borrow
Add cbindgen annotations for complex types so headers are generated correctly
Files:
dash-spv-ffi/src/client.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv-ffi/src/client.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Be careful with memory management at FFI boundaries (C/Swift interop)
Files:
dash-spv-ffi/src/client.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv-ffi/src/client.rs
*-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
FFI bindings live in *-ffi crates
Files:
dash-spv-ffi/src/client.rs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/callbacks.rs : Use C function pointers for async progress/completion callbacks; callbacks may be invoked from any thread
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : Add a corresponding C test for each new FFI function
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift} : Be careful with memory management at FFI boundaries (C/Swift interop)
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : In C code, free all strings returned by the FFI using dash_string_free
Applied to files:
dash-spv-ffi/src/client.rs
🧬 Code graph analysis (1)
dash-spv-ffi/src/client.rs (2)
dash-spv/src/client/mod.rs (1)
new(198-265)dash-spv/src/client/message_handler.rs (1)
new(36-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: Core Components Tests
🔇 Additional comments (4)
dash-spv-ffi/src/client.rs (4)
24-24: LGTM: non-blocking mpsc import is appropriateImporting TryRecvError and UnboundedReceiver matches the new pull-based draining design.
201-201: LGTM: event receiver initialized to NoneSane default; will be populated on start.
1277-1285: LGTM: helpful debug logs around callback assignmentGood visibility without leaking sensitive data; keeps background noise low by using debug level.
Also applies to: 1289-1289
235-363: Extract receiver and snapshot callbacks outside locks to avoid deadlocks
- Refactor
drain_events_internal(dash-spv-ffi/src/client.rs) to take the receiver out of theevent_rxmutex (e.g..lock().unwrap().take()), loop onrx.try_recv(), snapshotevent_callbacksby cloning and dropping the lock before invoking user callbacks, then reinsert the receiver at the end.- Add
#[derive(Clone)]toFFIEventCallbacksin dash-spv-ffi/src/callbacks.rs so it can be cloned safely.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Style
Documentation
Tests