feat: expose filters #189
feat: expose filters #189
Conversation
WalkthroughThe changes extend the Dash SPV client FFI and core library with comprehensive filter-matching capabilities. They replace boolean filter-match responses with vectors of matched wallet IDs, introduce ChainState persistence for filter matches, and add FFI APIs for loading and querying filters within configurable height ranges with 10,000-block limits. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (4)dash-spv-ffi/src/**/*.rs📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*-ffi/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (24)📓 Common learnings📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-06-26T16:01:37.609ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-22T17:59:37.833ZApplied to files:
📚 Learning: 2025-06-26T15:54:02.509ZApplied to files:
📚 Learning: 2025-06-26T16:02:42.390ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-08-21T05:01:58.949ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-06-26T15:47:37.438ZApplied to files:
🧬 Code graph analysis (1)dash-spv-ffi/src/client.rs (3)
⏰ 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)
🔇 Additional comments (4)
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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
dash-spv/src/storage/mod.rs (1)
130-133: Filter height/query APIs match backing implementationsThe new
get_stored_filter_heightandload_filtersmethods, including the 10,000-block range cap and “skip missing filters” semantics, line up with both disk and memory implementations and give a clear abstraction for batch filter access.As an optional refinement, you might briefly note that the
Range<u32>end is exclusive (e.g., “heights in[range.start, range.end)”) in the docs to make the boundary behavior explicit for callers.Also applies to: 152-159
dash-spv/tests/storage_test.rs (1)
319-352: Prefer matching onStorageError::InvalidInputover error string textThe limit checks in
test_load_filters_range_limitcurrently assert onto_string()contents. That works, but it’s a bit brittle if the error message wording changes.You can make the test more robust by matching the concrete error variant and only optionally asserting on the message, e.g.:
-use dash_spv::storage::{MemoryStorageManager, StorageManager}; +use dash_spv::storage::{MemoryStorageManager, StorageManager}; +use dash_spv::error::StorageError; - let result = storage.load_filters(0..10_001).await; - assert!(result.is_err(), "Should reject range exceeding 10,000 blocks"); - let error_msg = result.unwrap_err().to_string(); - assert!( - error_msg.contains("exceeds maximum") && error_msg.contains("10000"), - "Error message should mention the limit" - ); + let result = storage.load_filters(0..10_001).await; + match result { + Err(StorageError::InvalidInput(msg)) => { + assert!( + msg.contains("exceeds maximum") && msg.contains("10000"), + "Error message should mention the limit" + ); + } + other => panic!("Expected StorageError::InvalidInput, got {:?}", other), + }This keeps the semantic check while tying the assertion to the intended error variant rather than its string formatting.
dash-spv/src/sync/sequential/message_handlers.rs (1)
607-612: Filter persistence and match recording logic looks correct; minor clone can be avoidedPersisting the CFilter after header verification and then recording
matched_wallet_idsintoChainStategives you durable filter data plus per-height wallet match tracking, which fits the newfilter_matchesmodel and the surrounding sync/FFI changes. Error mapping toSyncError::Storageis consistent with existing patterns.One small refinement: you currently clone
matched_wallet_idsjust to pass ownership intorecord_filter_matches:chain_state.record_filter_matches(height, matched_wallet_ids.clone());Since you only use the vector afterward for its length in logs, you could avoid the clone by capturing the count first and then moving the vector:
let match_count = matched_wallet_ids.len(); // ... chain_state.record_filter_matches(height, matched_wallet_ids); // use match_count for logging belowThis keeps behavior identical while avoiding an extra allocation/copy on each matching filter.
Also applies to: 613-621, 625-655, 656-661
key-wallet-manager/src/wallet_interface.rs (1)
35-40: Trait docs and return type look good; consider a WalletId alias for clarityThe new documentation and
Vec<[u8; 32]>return type clearly define the semantics (empty = no download, non-empty = download those wallet IDs) and align with the rest of the PR.Given that this
Vec<[u8; 32]>shape now appears in several places (here, chain state, filter matching), consider introducing a dedicated alias or newtype (e.g.type WalletId = [u8; 32];) and usingVec<WalletId>in the trait and downstream code to improve readability and reduce potential for mismatched array lengths.Also applies to: 41-47
dash-spv/src/sync/sequential/phase_execution.rs (1)
153-186: Consider clampingstart_heightwithsync_base_heightto avoid redundant filter downloadsThe resume logic from
stored_filter_heightis sensible, but there are a couple of edge cases worth tightening:
- If
stored_filter_height < self.header_sync.get_sync_base_height(), you’ll resume below the sync base and re-download filters you’ll never use. Clamping tomax(stored_height + 1, sync_base_height.max(1))would avoid this.- If
stored_filter_height > filter_header_tip, the current code will log that all filters are downloaded and skip the phase, but silently accepts an inconsistent state (stored past tip). Emitting a warning (or falling back to a safer strategy) could make diagnosing storage corruption easier.Refactoring the
start_heightcalculation to incorporatesync_base_heightin theSome(stored_height)branch would address the first point without changing the main happy-path behavior.Also applies to: 188-200
dash-spv/src/storage/disk/filters.rs (1)
263-289: Range-bounded bulk filter loading is correct; consider de-duplicatingMAX_RANGEThe implementation:
- Uses
saturating_subto avoid underflow,- Enforces a 10,000-block range cap, and
- Skips missing filters while returning
(height, filter)pairs.For maintainability, you might optionally extract
MAX_RANGEto a shared constant used by both disk and memory implementations to prevent divergence over time.dash-spv/src/types.rs (1)
428-448: Consider adding deduplication logic to prevent duplicate wallet IDs.Both
record_filter_matchandrecord_filter_matchescan insert duplicate wallet IDs at the same height if called multiple times. This could lead to memory bloat and incorrect counting.If deduplication is needed, consider using a
BTreeSetor checking for existence before inserting:pub fn record_filter_match(&mut self, height: u32, wallet_id: [u8; 32]) { - self.filter_matches.entry(height).or_insert_with(Vec::new).push(wallet_id); + let vec = self.filter_matches.entry(height).or_insert_with(Vec::new); + if !vec.contains(&wallet_id) { + vec.push(wallet_id); + } }Alternatively, change the type to
BTreeMap<u32, BTreeSet<[u8; 32]>>if order doesn't matter:-pub filter_matches: BTreeMap<u32, Vec<[u8; 32]>>, +pub filter_matches: BTreeMap<u32, BTreeSet<[u8; 32]>>,dash-spv-ffi/src/types.rs (1)
609-628: Memory management is correct, but note the logic duplication.The destroy function properly frees nested allocations: first each filter's data (lines 614-622), then the filters array itself (line 625). However, the filter data deallocation logic is duplicated from
dash_spv_ffi_compact_filter_destroy(lines 595-597).This follows the existing pattern in the codebase (similar to
FFIUnconfirmedTransaction), so it's consistent, but if the FFICompactFilter structure changes, both functions must be updated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
dash-spv-ffi/include/dash_spv_ffi.h(5 hunks)dash-spv-ffi/src/client.rs(2 hunks)dash-spv-ffi/src/types.rs(1 hunks)dash-spv-ffi/tests/unit/test_client_lifecycle.rs(1 hunks)dash-spv-ffi/tests/unit/test_type_conversions.rs(1 hunks)dash-spv/src/client/block_processor.rs(2 hunks)dash-spv/src/client/block_processor_test.rs(2 hunks)dash-spv/src/error.rs(2 hunks)dash-spv/src/storage/disk/filters.rs(3 hunks)dash-spv/src/storage/disk/state.rs(3 hunks)dash-spv/src/storage/memory.rs(2 hunks)dash-spv/src/storage/mod.rs(2 hunks)dash-spv/src/storage/sync_state.rs(4 hunks)dash-spv/src/sync/filters/matching.rs(1 hunks)dash-spv/src/sync/sequential/message_handlers.rs(2 hunks)dash-spv/src/sync/sequential/phase_execution.rs(1 hunks)dash-spv/src/types.rs(4 hunks)dash-spv/tests/storage_test.rs(1 hunks)key-wallet-manager/src/wallet_interface.rs(1 hunks)key-wallet-manager/src/wallet_manager/process_block.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/unit/test_type_conversions.rsdash-spv/src/storage/mod.rsdash-spv/src/sync/sequential/message_handlers.rsdash-spv-ffi/tests/unit/test_client_lifecycle.rsdash-spv/src/sync/sequential/phase_execution.rsdash-spv/src/storage/disk/filters.rsdash-spv/src/types.rsdash-spv/src/sync/filters/matching.rsdash-spv/src/storage/memory.rsdash-spv/src/client/block_processor.rsdash-spv-ffi/src/types.rsdash-spv-ffi/include/dash_spv_ffi.hdash-spv/src/client/block_processor_test.rsdash-spv-ffi/src/client.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/sync_state.rs
📚 Learning: 2025-06-26T15:54:02.509Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:54:02.509Z
Learning: The `StorageManager` trait in `dash-spv/src/storage/mod.rs` uses `&mut self` methods but is also `Send + Sync`, and implementations often use interior mutability for concurrency. This can be confusing, so explicit documentation should clarify thread-safety expectations and the rationale for the API design.
Applied to files:
dash-spv/src/error.rsdash-spv/src/storage/mod.rsdash-spv/tests/storage_test.rsdash-spv/src/storage/memory.rsdash-spv-ffi/src/types.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/sync_state.rs
📚 Learning: 2025-06-15T15:31:44.136Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 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:
key-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-06-26T15:47:37.438Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/src/types.rsdash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-06-26T15:46:56.854Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/include/dash_spv_ffi.h
📚 Learning: 2025-08-21T05:01:58.949Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list/scores_for_quorum.rs:50-73
Timestamp: 2025-02-25T06:13:52.858Z
Learning: ScoreHash is a cryptographic hash in the rust-dashcore library, and therefore does not need collision handling when used as a key in collections like BTreeMap due to the extremely low probability of collisions.
Applied to files:
dash-spv/src/storage/disk/state.rsdash-spv/src/storage/sync_state.rs
🧬 Code graph analysis (12)
dash-spv/src/storage/mod.rs (3)
dash-spv/src/storage/disk/filters.rs (2)
get_stored_filter_height(191-243)load_filters(269-289)dash-spv/src/storage/disk/state.rs (2)
get_stored_filter_height(550-552)load_filters(578-580)dash-spv/src/storage/memory.rs (2)
get_stored_filter_height(261-268)load_filters(297-315)
dash-spv/src/sync/sequential/message_handlers.rs (3)
dash-spv/src/client/core.rs (2)
storage(171-173)chain_state(206-209)dash-spv/src/storage/disk/manager.rs (1)
new(79-129)dash-spv/src/sync/sequential/lifecycle.rs (1)
new(31-62)
dash-spv-ffi/tests/unit/test_client_lifecycle.rs (2)
dash-spv-ffi/src/client.rs (4)
dash_spv_ffi_client_new(140-210)dash_spv_ffi_client_get_transaction_count(1790-1817)dash_spv_ffi_client_destroy(1417-1455)dash_spv_ffi_client_get_blocks_with_transactions_count(1835-1873)dash-spv-ffi/src/config.rs (1)
dash_spv_ffi_config_destroy(345-354)
key-wallet-manager/src/wallet_manager/process_block.rs (2)
key-wallet-manager/src/wallet_manager/mod.rs (2)
new(70-76)new(107-114)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
wallet_id(30-30)wallet_id(126-128)
dash-spv/src/sync/sequential/phase_execution.rs (1)
dash-spv/src/sync/headers_with_reorg.rs (1)
get_sync_base_height(1119-1121)
dash-spv/tests/storage_test.rs (1)
dash-spv/src/storage/memory.rs (1)
new(30-42)
dash-spv/src/storage/disk/filters.rs (4)
dash-spv/src/storage/disk/state.rs (2)
get_stored_filter_height(550-552)load_filters(578-580)dash-spv/src/storage/memory.rs (3)
get_stored_filter_height(261-268)load_filters(297-315)new(30-42)dash-spv/src/storage/mod.rs (2)
get_stored_filter_height(132-132)load_filters(158-158)dash-spv/src/storage/disk/manager.rs (1)
new(79-129)
dash-spv/src/storage/memory.rs (3)
dash-spv/src/storage/disk/filters.rs (2)
get_stored_filter_height(191-243)load_filters(269-289)dash-spv/src/storage/disk/state.rs (2)
get_stored_filter_height(550-552)load_filters(578-580)dash-spv/src/storage/mod.rs (2)
get_stored_filter_height(132-132)load_filters(158-158)
key-wallet-manager/src/wallet_interface.rs (2)
dash-spv/src/client/block_processor_test.rs (2)
check_compact_filter(66-74)check_compact_filter(289-297)key-wallet-manager/src/wallet_manager/process_block.rs (1)
check_compact_filter(87-123)
dash-spv-ffi/include/dash_spv_ffi.h (2)
dash-spv-ffi/src/client.rs (4)
dash_spv_ffi_client_load_filters(1310-1385)dash_spv_ffi_client_get_filter_matched_heights(1697-1772)dash_spv_ffi_client_get_transaction_count(1790-1817)dash_spv_ffi_client_get_blocks_with_transactions_count(1835-1873)dash-spv-ffi/src/types.rs (4)
dash_spv_ffi_compact_filter_destroy(592-599)dash_spv_ffi_compact_filters_destroy(609-628)dash_spv_ffi_filter_match_entry_destroy(663-674)dash_spv_ffi_filter_matches_destroy(684-701)
dash-spv-ffi/src/client.rs (2)
dash-spv-ffi/src/error.rs (1)
set_last_error(26-31)dash-spv/src/client/core.rs (1)
wallet(161-163)
dash-spv/src/storage/disk/state.rs (3)
dash-spv/src/storage/memory.rs (2)
get_stored_filter_height(261-268)load_filters(297-315)dash-spv/src/storage/disk/filters.rs (2)
get_stored_filter_height(191-243)load_filters(269-289)dash-spv/src/storage/mod.rs (2)
get_stored_filter_height(132-132)load_filters(158-158)
⏰ 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: update-docs-comment
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: Strict Warnings and Clippy Checks
🔇 Additional comments (26)
dash-spv-ffi/tests/unit/test_type_conversions.rs (1)
165-176: Initialize newfilter_matchesfield consistentlySetting
filter_matchesto an emptyBTreeMapkeeps the test’sChainStateconstruction aligned with the updated struct and avoids uninitialized field issues. Looks correct and consistent with the new persistence model.dash-spv/src/error.rs (1)
114-116:StorageError::InvalidInputvariant is correctly integratedThe new
InvalidInput(String)variant and its explicit arm in the manualCloneimplementation are wired consistently with the existing string-based variants, so downstream storage code can distinguish invalid input cleanly without changing existing behavior.Also applies to: 118-130
dash-spv-ffi/tests/unit/test_client_lifecycle.rs (1)
245-300: Nice coverage for zero-count and null-client FFI behaviorThese tests validate the expected 0 counts for fresh wallets and ensure the new FFI query functions behave safely when passed a null client pointer, aligning with the
null_check!behavior inclient.rs. The lifecycle (create → query → destroy) mirrors existing patterns, so this fits cleanly into the test suite.dash-spv/tests/storage_test.rs (1)
288-317: Filter range behavior is well covered
test_load_filters_rangedoes a good job exercising the intended semantics ofload_filters: skipping missing heights, confirming end-exclusive ranges (101..103 only returns 101 even though 103 exists), and returning an empty vector when no filters are present in the range. This aligns with the MemoryStorageManager implementation.dash-spv/src/client/block_processor.rs (1)
180-190: Compact filter handling correctly adapted to per-wallet matchesUsing
matched_wallet_idsand derivinghas_matchespreserves the existing boolean contract for callers while enriching internal logging with the number of matching wallets. The control flow and locking remain consistent with prior behavior.Also applies to: 195-205
dash-spv/src/sync/filters/matching.rs (1)
54-60: Filter–wallet integration cleanly exposes per-wallet matches
check_filter_for_matchesnow correctly wraps the wallet’scheck_compact_filter, returning theVec<[u8; 32]>of wallet IDs and logging only when matches exist. This aligns with the updatedWalletInterfaceand the broader move toward per-wallet filter match tracking.Also applies to: 61-69
dash-spv/src/storage/disk/filters.rs (2)
8-8: ImportingStorageErrorfor range validation is appropriateThe added import is necessary for the new
load_filtersrange validation and is consistent with existing error handling in this module.
185-243: Directory-based stored filter height detection looks robustThe scan correctly:
- Ignores non-files and non-
.datentries.- Restricts to purely numeric filenames and caps heights at 2,000,000 with a warning.
- Returns
Nonewhen no valid filter files are present.This aligns with the documented intent to reflect actually persisted filters rather than headers.
key-wallet-manager/src/wallet_manager/process_block.rs (1)
87-123: Per-wallet compact filter matching and caching behavior looks correctThe new implementation:
- Evaluates each wallet independently using its
monitored_addresses(network),- Builds per-wallet script pubkeys and calls
filter.match_any(..).unwrap_or(false),- Collects wallet IDs with hits, and
- Caches
any_matchas a boolean infilter_matches, preserving the prior “any wallet matched” semantics.This cleanly exposes matched wallet IDs while keeping the existing cache contract.
dash-spv/src/storage/memory.rs (2)
261-268: In-memory stored filter height computation is simple and correctUsing
self.filters.keys().max().copied()to derive the highest stored filter height (orNonewhen empty) matches the intended semantics and is consistent with how filters are stored in this manager.
297-315: In-memory bulk filter loading correctly mirrors disk behaviorThe range-bounded
load_filters:
- Enforces the same 10,000-block maximum range as the disk implementation,
- Handles inverted ranges safely via
saturating_sub,- Iterates the range and returns
(height, filter.clone())only for existing entries.This keeps memory and disk storage backends behaviorally aligned.
dash-spv/src/client/block_processor_test.rs (2)
66-74: MockWallet’scheck_compact_filteradaptation matches new APIReturning a non-empty
Vec<[u8; 32]>with a fixed test wallet ID for all filters is a straightforward way to signal “matched” in tests under the new signature.
289-297: NonMatchingWallet correctly models the “no match” caseReturning an empty
Vec<[u8; 32]>cleanly represents a non-matching filter for tests and aligns with the updatedWalletInterfacecontract.dash-spv/src/storage/disk/state.rs (3)
5-5:Rangeimport is appropriate for new trait method signatureThe added
use std::ops::Range;matches the newload_filtersmethod signature and keeps the implementation concise.
550-552: Trait forwarding forget_stored_filter_heightis consistentDelegating
StorageManager::get_stored_filter_heighttoDiskStorageManager::get_stored_filter_heightmatches the existing pattern used throughout this impl and correctly exposes the disk-level helper.
578-580:load_filtersdelegation cleanly exposes disk-backed bulk loadingForwarding
load_filtersthrough theStorageManagerimpl ensures callers can use the new disk bulk filter loading API via the trait without duplicating logic.dash-spv/src/types.rs (3)
285-287: Good choice of data structure.Using
BTreeMap<u32, Vec<[u8; 32]>>enables efficient range queries via therange()method, which is exactly whatget_filter_matched_heightsleverages. The 32-byte fixed arrays for wallet IDs align with FFI best practices.Based on learnings
461-480: LGTM! Good range validation and error handling.The 10,000 block limit prevents abuse, and the error messages are clear. The use of
saturating_subprevents underflow, andBTreeMap::range()provides efficient extraction.Note: Line 477 clones each Vec, which is necessary for thread safety across the FFI boundary but could be expensive for ranges with many matches. Given the 10K block limit, this is acceptable.
1163-1245: Excellent test coverage!The tests comprehensively validate:
- Single and bulk filter match recording
- Range queries with hits and misses
- Boundary conditions (exactly 10,000 blocks passes, 10,001 fails)
- Empty and partially overlapping ranges
dash-spv-ffi/src/types.rs (2)
554-599: LGTM! Memory management is correct.The
FFICompactFilterstruct and its destroy function properly handle ownership transfer. The use ofVec::from_raw_parts(data, data_len, data_len)is correct because the Vec is forgotten immediately after creation (seedash-spv-ffi/src/client.rs:1356-1359), ensuring len equals capacity.
630-701: LGTM! FFIFilterMatches memory management mirrors FFICompactFilters pattern.The destroy functions correctly handle the two-level allocation:
- Array of entries (line 698)
- Each entry's wallet_ids array (lines 688-696)
The logic properly reconstructs and drops
Vec<[u8; 32]>with matching len/capacity, which is safe given how these are created indash-spv-ffi/src/client.rs:1742-1757.dash-spv-ffi/src/client.rs (3)
1310-1385: LGTM! Well-designed for concurrent access.Lines 1335-1344: The function gets a storage reference without taking the client mutex, allowing other operations to proceed concurrently. This is excellent for responsiveness during sync.
The memory transfer pattern (lines 1354-1370) correctly uses
std::mem::forgetto transfer ownership to the FFI caller, who must calldash_spv_ffi_compact_filters_destroy.
1697-1772: LGTM! Consistent pattern with proper ownership transfer.The function follows the same concurrent-access pattern as
load_filters, obtaining chain state without blocking other operations (lines 1722-1731). The BTreeMap-to-FFI conversion correctly transfers ownership viastd::mem::forget(lines 1742-1757).
1790-1873: LGTM! Both count functions are well-implemented.
get_transaction_count(lines 1790-1817): Simple and correct, returns the persisted transaction history length.
get_blocks_with_transactions_count(lines 1835-1873): Properly counts unique block heights using a HashSet (lines 1854-1859), filtering for confirmed transactions only. The warning logs (lines 1810, 1864-1866) help with debugging.dash-spv-ffi/include/dash_spv_ffi.h (2)
112-749: LGTM! C declarations correctly match Rust implementations.The auto-generated header properly declares all new FFI types and functions:
- Struct layouts match the Rust definitions
- Function signatures correctly translate Rust types to C types
- Memory management requirements are clearly documented (e.g., lines 116-118, 558-560, 687-688)
- Safety requirements are explicit (e.g., lines 576-577, 704-705)
1156-1198: LGTM! Destroy function declarations are comprehensive and well-documented.All four destroy functions are properly declared with clear safety documentation emphasizing:
- Pointer validity requirements
- One-time-only calling constraint
- Risk of use-after-free
This matches the Rust implementations and follows FFI best practices.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
key-wallet-ffi/include/key_wallet_ffi.h (1)
896-900: Documentation inconsistency: wrong enum type referenced.The doc comment at line 899 says
"Returns FFINetwork::NoNetworks"but the function returnsFFINetworks, and the correct value isNO_NETWORKS. Same issue appears at lines 953 and 1009 forbls_account_get_networkandeddsa_account_get_network.Since this file is auto-generated by cbindgen, the fix should be in the Rust source doc comments.
🧹 Nitpick comments (2)
dash-spv-ffi/include/dash_spv_ffi.h (2)
713-731: Doc wording for transaction count is slightly misleadingThe function name and summary (“Get the total count of transactions across all wallets”) are clear, but the sentence:
“Use this to show how many blocks contained relevant transactions for the user's wallets.”
describes what the blocks‑with‑transactions metric does, not transaction count. I’d suggest tightening the doc to avoid confusion with
dash_spv_ffi_client_get_blocks_with_transactions_count.Example rewording:
- * not the ephemeral sync statistics. Use this to show how many - * blocks contained relevant transactions for the user's wallets. + * not the ephemeral sync statistics. Use this to show how many + * wallet-related transactions have been seen across all wallets.
1156-1199: Destroy helpers for new types match existing FFI patternsThe four destroy functions (
dash_spv_ffi_compact_filter_destroy,dash_spv_ffi_compact_filters_destroy,dash_spv_ffi_filter_match_entry_destroy,dash_spv_ffi_filter_matches_destroy) match the crate’s pattern of providing explicit free functions for heap-owned FFI structs. The docs make the single‑ownership requirement clear (“must not be used after this function” / “only be called once per allocation”). Just ensure the high-level docs/examples clearly indicate which destroy function to call for objects returned by each API (e.g., arrays from*_client_load_filters/*_get_filter_matched_heightsshould only go through the array-level destroy to avoid accidental double frees).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dash-spv-ffi/include/dash_spv_ffi.h(7 hunks)key-wallet-ffi/include/key_wallet_ffi.h(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-21T05:01:58.949Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.
Applied to files:
key-wallet-ffi/include/key_wallet_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-06-26T15:47:37.438Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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:
key-wallet-ffi/include/key_wallet_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-06-26T15:46:56.854Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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:
key-wallet-ffi/include/key_wallet_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/include/dash_spv_ffi.h
⏰ 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). (8)
- GitHub Check: update-docs-comment
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: Key Wallet Components Tests
🔇 Additional comments (12)
dash-spv-ffi/include/dash_spv_ffi.h (7)
6-6: cbindgen version bump is fineUpdating the generated-header marker to
cbindgen:0.29.2is consistent with a regenerated FFI surface; no issues here.
32-35: Explicit mempool strategy discriminants preserve ABIReintroducing
FFIMempoolStrategyhere with explicit numeric values (FetchAll = 0,BloomFilter = 1) keeps the C ABI stable for existing callers and resolves any forward-declaration ordering issues with the config APIs.
112-153: Compact filter structs and ownership semantics look correctThe
FFICompactFilter/FFICompactFilterslayout (height +uint8_t *data+uintptr_t data_len, plus array-of-filters) is FFI-safe and matches the existingFFIArray/usizepattern. The docs clearly direct callers to free results fromdash_spv_ffi_client_load_filtersviadash_spv_ffi_compact_filters_destroy, anddash_spv_ffi_compact_filter_destroyis available for single-filter allocations. I don’t see ABI or ownership issues here.
216-252: Filter match structs follow the established 32‑byte ID pattern
FFIFilterMatchEntryandFFIFilterMatchesmirror the compact-filter pattern and useuint8_t (*wallet_ids)[32]for fixed-size 32‑byte identifiers, which aligns with the existing practice of passing binary IDs as fixed arrays instead of strings. The docs also make it clear thatdash_spv_ffi_filter_matches_destroyis responsible for freeing both the entries array and each entry’swallet_ids. Based on learnings, this is consistent with how txids/block hashes are handled.
555-583: Range-based filter loading API is well-specified; confirm inclusive/exclusive expectationsThe
dash_spv_ffi_client_load_filtersAPI is clearly documented:[start_height, end_height)semantics, 10 000‑block max range, null on error with details viadash_spv_ffi_get_last_error, and ownership transferred to the caller viadash_spv_ffi_compact_filters_destroy. Functionally and ABI‑wise this looks sound; just ensure the Rust implementation enforces the same inclusive/exclusive bounds and range limit so C callers don’t hit off‑by‑one surprises.
683-711: Filter match query API pairs cleanly with the new match structs
dash_spv_ffi_client_get_filter_matched_heightsmirrorsclient_load_filtersin range semantics, error handling, and ownership rules, and returns the newFFIFilterMatcheswrapper. This is a coherent extension of the FFI surface; as with filters, just confirm the Rust side enforces the documented[start_height, end_height)range and 10 000‑block limit.
732-749: Blocks-with-transactions count API is consistent and useful
dash_spv_ffi_client_get_blocks_with_transactions_countrounds out the metrics nicely: it returns a persistent count of unique block heights with relevant transactions and documents the “0 if client not initialized or wallet not available” behavior. Signature and semantics are consistent withdash_spv_ffi_client_get_transaction_count.key-wallet-ffi/include/key_wallet_ffi.h (5)
27-35: LGTM: Well-designed single-network enum.The
FFINetworkenum correctly uses sequential values for single network selection, complementing the bit-flagFFINetworksenum for multi-network scenarios.
97-107: LGTM: Correct bit flag implementation.The
FFINetworksenum properly uses powers of 2 for individual flags, andALL_NETWORKS = 15correctly represents the bitwise OR of all network flags (1|2|4|8).
128-144: LGTM: Clear address pool type enumeration.The enum appropriately uses sequential values for mutually exclusive pool types, with good documentation for each variant.
184-208: LGTM: Account creation options properly defined.The enum provides clear, mutually exclusive options for wallet account creation with helpful documentation for each variant.
814-818: API consistency between FFINetwork and FFINetworks is correctly implemented.The Rust implementation confirms the review comment's claims:
wallet_create_from_mnemonic_with_optionsacceptsFFINetworks(bit flags) for multi-network wallet configurationwallet_get_accountacceptsFFINetwork(single value) for network-specific operations- Proper
From/TryFromtrait implementations exist inkey-wallet-ffi/src/types.rsfor bidirectional conversion betweenNetwork,FFINetwork, andFFINetworksThe API design correctly distinguishes between wallet-level multi-network configuration and operation-level single-network targeting.
There was a problem hiding this comment.
Changes found:
- WalletInterface::check_compact_filter now returns a vector of 32-byte wallet IDs that matched the filter. This method is used by FilterSyncManager<S, N>::check_filter_for_matches that returns the vector of wallet IDs for later being stored in ChainState.filter_matches field. The function dash_spv_ffi_client_get_filter_matched_heights calls ChainState::get_filter_matched_heights to extract data for the previous mentioned field.
- the function dash_spv_ffi_client_load_filters makes use of the new StorageManager::load_filters method. All the structs that implement the trait were updated to implement it.
- The new method StorageManager::get_stored_filter_height doesn't look like it has any relation with the filters exposition. Is more an optimization that allows downloading from the last stored filter during SyncPhase::DownloadingFilters
- The FilterSyncState::filter_matches doesn't look like it is being used
- Missing
get_stored_filter_height,load_filtersimplementation in MockStorageManager (The file where this issue exists is no longer in the v0.41-dev branch)
It took my sometime but here is what I found. Almost everything has something to do with the filters exposition. Made some notes on stuff that doesn't make sense to me and the only change I found that has nothing to do with what this PR tries to add is the method StorageManager::get_stored_filter_height used to avoid downloading all the filters in each sync.
@QuantumExplorer if this looks good to you I will push the changes suggested
…ck SyncPhase::DownloadingFilters phase execution logic
…_matches optimized
…andle_cfilter_message
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
dash-spv/src/storage/disk/state.rs(2 hunks)dash-spv/src/storage/memory.rs(1 hunks)dash-spv/src/storage/mod.rs(1 hunks)dash-spv/src/storage/sync_state.rs(0 hunks)dash-spv/src/sync/sequential/message_handlers.rs(2 hunks)dash-spv/src/sync/sequential/phase_execution.rs(1 hunks)dash-spv/src/types.rs(4 hunks)
💤 Files with no reviewable changes (1)
- dash-spv/src/storage/sync_state.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- dash-spv/src/sync/sequential/phase_execution.rs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code
**/*.rs: Each crate keeps sources insrc/; unit tests live alongside code with#[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code withrustfmt(seerustfmt.toml); runcargo fmt --allbefore commits
Lint Rust code withclippy; avoidunwrap()/expect()in library code; use error types (e.g.,thiserror)
Usesnake_casefor function and variable names in Rust
UseUpperCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async viatokiowhere applicable
Unit tests should be placed near code with descriptive names (e.g.,test_parse_address_mainnet)
Use#[ignore]for network-dependent or long-running tests; run with-- --ignoredflag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code
Files:
dash-spv/src/storage/mod.rsdash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/memory.rs
dash-spv/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/src/**/*.rs: Use trait-based abstractions for swappable implementations (e.g.,NetworkManager,StorageManager)
Use async/await throughout the codebase with tokio runtime
Use domain-specific error types (NetworkError,StorageError,SyncError,ValidationError,SpvError) for error handling
Use trait objects (Arc<dyn StorageManager>,Arc<dyn NetworkManager>) for runtime polymorphism
Use Tokio channels for inter-component message passing
Add comprehensive unit tests in-module for individual components
Files:
dash-spv/src/storage/mod.rsdash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/memory.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/src/storage/**/*.rs: ImplementStorageManagertrait with segmented storage: headers in 10,000-header segments with index files, filters in separate storage, and state persistence in dedicated paths
Use segmented storage with headers in 10,000-header chunks stored with index files for efficient retrieval
Files:
dash-spv/src/storage/mod.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/memory.rs
dash-spv/src/sync/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/src/sync/**/*.rs: Use sequential phase-based synchronization withSequentialSyncManagerfor organized sync phases
Use state machines withSyncStateenum to drive synchronization flow and state transitions
Files:
dash-spv/src/sync/sequential/message_handlers.rs
dash-spv/src/types.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Define common data structures in
types.rsincludingSyncProgress,ValidationMode,WatchItem, etc.
Files:
dash-spv/src/types.rs
🧠 Learnings (21)
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Implement `StorageManager` trait with segmented storage: headers in 10,000-header segments with index files, filters in separate storage, and state persistence in dedicated paths
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/memory.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Use segmented storage with headers in 10,000-header chunks stored with index files for efficient retrieval
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/memory.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Use trait objects (`Arc<dyn StorageManager>`, `Arc<dyn NetworkManager>`) for runtime polymorphism
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/types.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/memory.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Use trait-based abstractions for swappable implementations (e.g., `NetworkManager`, `StorageManager`)
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/types.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/memory.rs
📚 Learning: 2025-06-26T15:54:02.509Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:54:02.509Z
Learning: The `StorageManager` trait in `dash-spv/src/storage/mod.rs` uses `&mut self` methods but is also `Send + Sync`, and implementations often use interior mutability for concurrency. This can be confusing, so explicit documentation should clarify thread-safety expectations and the rationale for the API design.
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/memory.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Use async/await throughout the codebase with tokio runtime
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/storage/disk/state.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Use sequential phase-based synchronization with `SequentialSyncManager` for organized sync phases
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/sync/sequential/message_handlers.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Implement wallet functionality with UTXO tracking, balance calculation with confirmation states, and transaction processing
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rsdash-spv/src/storage/disk/state.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/sync/sequential/message_handlers.rsdash-spv/src/types.rsdash-spv/src/storage/memory.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Use state machines with `SyncState` enum to drive synchronization flow and state transitions
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rsdash-spv/src/storage/disk/state.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : In SPV sync integration, use `wallet_info.check_transaction()` with appropriate `TransactionContext` and update wallet state atomically only when `is_relevant` flag is true
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rs
📚 Learning: 2025-12-01T08:01:00.652Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Implement staged gap limit management using `GapLimitStage` structure with tracking of last_used_index and used_indices HashSet to enable efficient address discovery without loading entire address chains into memory
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rsdash-spv/src/storage/disk/state.rsdash-spv/src/storage/memory.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates in ManagedAccount: when processing transactions, update address usage state, UTXO set, transaction history, and balance calculations together to maintain consistency
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Add comprehensive unit tests in-module for individual components
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/tests/**/*.rs : Add integration tests in the `tests/` directory for comprehensive test suites
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/types.rs : Define common data structures in `types.rs` including `SyncProgress`, `ValidationMode`, `WatchItem`, etc.
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account}/**/*.rs : Use BTreeMap for ordered data structures (accounts, transactions) and HashMap for quick lookups (address to index mapping) to optimize data structure performance
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement configurable timeouts with recovery mechanisms for network operations
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Use domain-specific error types (`NetworkError`, `StorageError`, `SyncError`, `ValidationError`, `SpvError`) for error handling
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list/scores_for_quorum.rs:50-73
Timestamp: 2025-02-25T06:13:52.858Z
Learning: ScoreHash is a cryptographic hash in the rust-dashcore library, and therefore does not need collision handling when used as a key in collections like BTreeMap due to the extremely low probability of collisions.
Applied to files:
dash-spv/src/storage/disk/state.rs
🧬 Code graph analysis (3)
dash-spv/src/sync/sequential/message_handlers.rs (1)
dash-spv/src/storage/disk/manager.rs (1)
new(79-129)
dash-spv/src/storage/disk/state.rs (3)
dash-spv/src/storage/memory.rs (1)
load_filters(288-306)dash-spv/src/storage/mod.rs (1)
load_filters(154-154)dash-spv/src/storage/disk/filters.rs (1)
load_filters(269-289)
dash-spv/src/storage/memory.rs (2)
dash-spv/src/storage/mod.rs (1)
load_filters(154-154)dash-spv/src/storage/disk/filters.rs (1)
load_filters(269-289)
⏰ 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). (14)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: Key Wallet Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: Pre-commit (ubuntu-latest)
- GitHub Check: Pre-commit (macos-latest)
🔇 Additional comments (8)
dash-spv/src/types.rs (4)
18-18: LGTM! Well-chosen data structure for ordered filter matches.Using
BTreeMap<u32, Vec<[u8; 32]>>forfilter_matchesis appropriate as it provides efficient range-based queries (used inget_filter_matched_heights) and maintains height ordering naturally.Also applies to: 284-287
428-443: LGTM! Efficient batch recording with proper merge handling.The method correctly handles both new entries and merging with existing wallet IDs. The early return for empty
wallet_idsavoids unnecessary BTreeMap operations.
445-475: LGTM! Range query with appropriate size limit.The 10,000 block limit is consistent with the segmented storage architecture. The use of
BTreeMap::rangeprovides efficient O(log n + m) lookup where m is the number of entries in the range.
1154-1223: LGTM! Comprehensive test coverage for the new filter match API.The tests cover critical scenarios including bulk recording, range queries, the 10,000-block limit enforcement, and edge cases (empty ranges, partial overlaps). Well done!
dash-spv/src/sync/sequential/message_handlers.rs (1)
607-625: LGTM! Correct ordering of filter persistence and match detection.Storing the filter to disk (lines 607-611) before checking for matches ensures the filter data is persisted regardless of match results. The transition from boolean to
Vec<[u8; 32]>for wallet IDs enables fine-grained tracking.dash-spv/src/storage/mod.rs (1)
148-154: LGTM! Well-documented trait method with appropriate constraints.The
load_filtersmethod signature and documentation are clear. The 10,000 block limit is consistent with the segmented storage architecture documented in the coding guidelines.dash-spv/src/storage/disk/state.rs (1)
5-5: LGTM! Straightforward trait method delegation.The
Rangeimport and trait method implementation follow the established pattern in this file of delegating to inherent implementation methods.Also applies to: 574-576
dash-spv/src/storage/memory.rs (1)
288-306: LGTM! Consistent implementation with proper range validation.The implementation correctly:
- Enforces the 10,000 block limit using
saturating_sub- Returns
InvalidInputerror for oversized ranges- Skips missing filters as documented
- Matches the behavior of the disk storage implementation
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dash-spv-ffi/FFI_API.md (1)
1393-1412: Add new FFI types to the Type Definitions section.The Type Definitions section should include the new filter-related types introduced in this PR to maintain complete documentation coverage.
Add the following types to the "Core Types" subsection after the existing types:
- `FFIEventCallbacks` - Event callback structure - `CoreSDKHandle` - Platform SDK integration handle +- `FFICompactFilter` - Compact filter with height and data +- `FFICompactFilters` - Array of compact filters with count +- `FFIFilterMatchEntry` - Filter match entry with height and wallet IDs +- `FFIFilterMatches` - Array of filter match entries with count
🧹 Nitpick comments (4)
dash-spv-ffi/FFI_API.md (2)
1414-1423: Update Memory Management section to document new filter types.The Memory Management section should reflect the new types and their lifecycle, particularly the range-query limits mentioned in the function documentation.
Add a new subsection after the "Important Rules" to document filter-specific lifecycle requirements:
5. **Shared Ownership**: `dash_spv_ffi_client_get_wallet_manager()` returns `FFIWalletManager*` that must be released with `dash_spv_ffi_wallet_manager_free()` + +### Filter API Usage + +1. **Range Limits**: Filter queries (`dash_spv_ffi_client_load_filters` and `dash_spv_ffi_client_get_filter_matched_heights`) are limited to 10,000-block ranges +2. **Filter Cleanup**: `FFICompactFilters` must be freed with `dash_spv_ffi_compact_filters_destroy()` +3. **Match Cleanup**: `FFIFilterMatches` must be freed with `dash_spv_ffi_filter_matches_destroy()`
1424-1474: Add example usage for new filter and transaction APIs.The Usage Examples section should demonstrate the new filter loading and query capabilities introduced in this PR.
Add a new example section after the Event Callbacks example:
dash_spv_ffi_client_set_event_callbacks(client, callbacks);Then append:
+ +### Loading and Querying Filters + +```c +// Load compact filters for a range +FFICompactFilters* filters = dash_spv_ffi_client_load_filters(client, 100, 1100); +if (filters != NULL) { + for (size_t i = 0; i < filters->count; i++) { + printf("Filter at height %u, data length: %zu\n", + filters->filters[i].height, + filters->filters[i].data_len); + } + dash_spv_ffi_compact_filters_destroy(filters); +} + +// Get filter-matched heights with wallet IDs +FFIFilterMatches* matches = dash_spv_ffi_client_get_filter_matched_heights(client, 100, 1100); +if (matches != NULL) { + for (size_t i = 0; i < matches->count; i++) { + printf("Height %u matched %zu wallet(s)\n", + matches->entries[i].height, + matches->entries[i].wallet_ids_count); + } + dash_spv_ffi_filter_matches_destroy(matches); +} + +// Get transaction statistics +usize tx_count = dash_spv_ffi_client_get_transaction_count(client); +usize block_count = dash_spv_ffi_client_get_blocks_with_transactions_count(client); +printf("Transactions: %zu across %zu blocks\n", tx_count, block_count); +```key-wallet-manager/src/wallet_manager/mod.rs (1)
91-94: UseWalletIdalias infilter_matches(and consider caching policy).You already define
pub type WalletId = [u8; 32];above; here you hard‑code[u8; 32]instead of usingWalletId. For consistency and easier refactors, it would be clearer to declare this as:filter_matches: BTreeMap<Network, BTreeMap<BlockHash, Vec<WalletId>>>,Also, you currently cache an entry even when the matched list is empty. If filters are processed for large ranges of heights, you may want to consider skipping insertion for the empty case to bound the cache size over long runs (the miss path just recomputes once). Not urgent, but worth thinking about given this is per‑block state.
key-wallet-manager/src/wallet_manager/process_block.rs (1)
87-132: Align return/cached types withWalletIdalias.
check_compact_filterreturnsVec<[u8; 32]>and pushes*wallet_idvalues, whileWalletManageralready definesWalletId = [u8; 32]. For clarity and to keep types in sync, it would be preferable to use the alias consistently here, e.g.:async fn check_compact_filter( &mut self, filter: &BlockFilter, block_hash: &BlockHash, network: Network, ) -> Vec<WalletId> { // ... let mut matched_wallet_ids: Vec<WalletId> = Vec::new(); // ... }and update the
WalletInterfacetrait / callers accordingly if feasible. This keeps the concept of “wallet id” explicit throughout the API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dash-spv-ffi/FFI_API.md(9 hunks)key-wallet-manager/src/wallet_manager/mod.rs(1 hunks)key-wallet-manager/src/wallet_manager/process_block.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code
**/*.rs: Each crate keeps sources insrc/; unit tests live alongside code with#[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code withrustfmt(seerustfmt.toml); runcargo fmt --allbefore commits
Lint Rust code withclippy; avoidunwrap()/expect()in library code; use error types (e.g.,thiserror)
Usesnake_casefor function and variable names in Rust
UseUpperCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async viatokiowhere applicable
Unit tests should be placed near code with descriptive names (e.g.,test_parse_address_mainnet)
Use#[ignore]for network-dependent or long-running tests; run with-- --ignoredflag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code
Files:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_manager/process_block.rs
🧠 Learnings (35)
📓 Common learnings
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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.
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Implement wallet functionality with UTXO tracking, balance calculation with confirmation states, and transaction processing
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Implement `StorageManager` trait with segmented storage: headers in 10,000-header segments with index files, filters in separate storage, and state persistence in dedicated paths
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account}/**/*.rs : Use BTreeMap for ordered data structures (accounts, transactions) and HashMap for quick lookups (address to index mapping) to optimize data structure performance
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account}/**/*.rs : Clearly separate watch-only wallets from full wallets using the wallet type system; never attempt signing operations on watch-only accounts and validate external signatures match expected pubkeys
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{bip32,derivation}/**/*.rs : Cache intermediate derivation results and use hardened derivation only when required; batch derive child keys when possible to optimize derivation performance
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Implement staged gap limit management using `GapLimitStage` structure with tracking of last_used_index and used_indices HashSet to enable efficient address discovery without loading entire address chains into memory
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{account,managed_account}/**/*.rs : Separate immutable and mutable concerns: Use `Account` for immutable identity information (keys, derivation paths) and `ManagedAccount` for mutable state (address pools, metadata, balances)
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{bip32,derivation,account}/**/*.rs : Use `ExtendedPubKey` and `ExtendedPrivKey` from the BIP32 implementation for key derivation; avoid direct private key exposure or serialization in logs
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates in ManagedAccount: when processing transactions, update address usage state, UTXO set, transaction history, and balance calculations together to maintain consistency
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Support multiple `KeySource` variants (Private, Public, NoKeySource) in address pool initialization to enable both full wallets and watch-only wallets with the same interface
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{account,managed_account}/**/*.rs : Use strongly typed enum-based system for `AccountType` with specific variants (Standard, Identity, Masternode, etc.) to provide compile-time safety for different account purposes
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Implement `StorageManager` trait with segmented storage: headers in 10,000-header segments with index files, filters in separate storage, and state persistence in dedicated paths
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rs
📚 Learning: 2025-06-15T15:31:44.136Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 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:
key-wallet-manager/src/wallet_manager/mod.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account,address_pool}/**/*.rs : Always validate network consistency: use the account's network type when creating addresses and assert that operations use the expected network to prevent mainnet/testnet mixing
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : In SPV sync integration, use `wallet_info.check_transaction()` with appropriate `TransactionContext` and update wallet state atomically only when `is_relevant` flag is true
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : Use `TransactionRouter::classify_transaction()` to determine transaction type and `get_relevant_account_types()` to avoid checking all accounts for every transaction; route CoinJoin and other specialized transactions only to relevant account types
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Implement wallet functionality with UTXO tracking, balance calculation with confirmation states, and transaction processing
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/*.rs : Use `?` operator for error propagation in Rust code to maintain clean error handling patterns
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-01T08:00:50.618Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.rs : Implement new FFI functions in Rust with `#[no_mangle] extern "C"` annotation in dash-spv-ffi
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use `#[no_mangle] extern "C"` attribute when implementing new FFI functions in Rust
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types in FFI functions
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.{c,h} : Add corresponding C tests in `tests/c_tests/` for each new FFI function
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : All FFI types must have corresponding `_destroy()` functions for explicit memory management
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/FFI_API.md
📚 Learning: 2025-12-01T08:00:50.618Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: When making changes to Rust FFI in dash-spv-ffi, rebuild with `cargo build --release` and run Swift tests to verify integration
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:01:00.652Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Use DashSDK's public methods, not direct SPVClient access, when accessing SPV functionality in Swift code
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-06-26T15:47:37.438Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/FFI_API.md
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: When debugging FFI issues, check `dash_spv_ffi_get_last_error()` for error details
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Rust strings must be returned as `*const c_char` with caller responsibility to free using `dash_string_free`
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Use domain-specific error types (`NetworkError`, `StorageError`, `SyncError`, `ValidationError`, `SpvError`) for error handling
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Add comprehensive unit tests in-module for individual components
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/tests/**/*.rs : Add integration tests in the `tests/` directory for comprehensive test suites
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv-ffi/FFI_API.md
🧬 Code graph analysis (1)
key-wallet-manager/src/wallet_manager/process_block.rs (1)
key-wallet-manager/src/wallet_manager/mod.rs (2)
new(70-76)new(107-114)
🪛 LanguageTool
dash-spv-ffi/FFI_API.md
[grammar] ~1117-~1117: Use a hyphen to join words.
Context: ...Matches ``` Description: Get filter matched heights with wallet IDs in a giv...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (3)
dash-spv-ffi/FFI_API.md (3)
99-100: LGTM!The new transaction count functions (
dash_spv_ffi_client_get_transaction_countanddash_spv_ffi_client_get_blocks_with_transactions_count) are well-documented with clear parameter descriptions and safety annotations. The distinction between persisted metrics and ephemeral sync statistics is appropriately explained.
1113-1124: LGTM!The filter loading and matched-heights query functions are comprehensively documented with:
- Clear range-limit constraints (10,000 blocks max)
- Explicit caller ownership and cleanup requirements
- Proper safety annotations and error handling guidance
- Return value semantics
Also applies to: 1193-1204
1238-1310: LGTM!All four destroy functions for the new filter types follow the established memory-management pattern consistently. Safety documentation is clear and comprehensive for each destruction operation.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
dash-spv/src/sync/sequential/message_handlers.rs (1)
641-656: ChainState per-match I/O concern remains unaddressed.Loading and saving ChainState for every filter match (lines 644-656) could cause performance issues during bulk filter sync. Additionally,
unwrap_or_else(|| ChainState::new())on line 648 masks storage errors by silently creating a fresh state.Consider:
- Batching filter match recording and persisting periodically (e.g., every N matches or at phase completion)
- Propagating the storage error rather than silently creating a new ChainState
- let mut chain_state = storage - .load_chain_state() - .await - .map_err(|e| SyncError::Storage(format!("Failed to load chain state: {}", e)))? - .unwrap_or_else(|| crate::types::ChainState::new()); + let mut chain_state = storage + .load_chain_state() + .await + .map_err(|e| SyncError::Storage(format!("Failed to load chain state: {}", e)))? + .ok_or_else(|| SyncError::Storage("Chain state not found".to_string()))?;dash-spv-ffi/FFI_API.md (1)
1116-1117: Hyphenate “filter-matched heights” in the description.The phrase “Get filter matched heights…” should be hyphenated as “filter-matched heights” to read as a compound adjective in the description.
-Get filter matched heights with wallet IDs in a given range. +Get filter-matched heights with wallet IDs in a given range.
🧹 Nitpick comments (3)
dash-spv/src/types.rs (1)
428-443: Consider deduplicating wallet IDs on record.If a filter at the same height is processed multiple times (e.g., due to retries), duplicates will accumulate since
extendappends without checking for existing entries. If this is a concern for memory or query accuracy:pub fn record_filter_matches(&mut self, height: u32, wallet_ids: Vec<[u8; 32]>) { if wallet_ids.is_empty() { return; } if let Some(ids) = self.filter_matches.get_mut(&height) { - ids.extend(wallet_ids); + for id in wallet_ids { + if !ids.contains(&id) { + ids.push(id); + } + } } else { self.filter_matches.insert(height, wallet_ids); } }Alternatively, use a
HashSetinternally if uniqueness is required.dash-spv-ffi/src/client.rs (2)
1669-1767: Avoid holding the client mutex acrosschain_state().awaitinget_filter_matched_heights.
dash_spv_ffi_client_get_filter_matched_heightscurrently keeps aMutexGuardoninnerwhile awaitingclient.chain_state().await. This is generally discouraged in async Rust because it:
- Holds a blocking
std::sync::Mutexacross an.await, potentially stalling any other FFI call that needs the same mutex while the future is pending.- Diverges from the established “take, await, then put back” pattern used elsewhere in this file.
Functionality and FFI ownership transfer for
FFIFilterMatchesandFFIFilterMatchEntryotherwise look correct.A more consistent pattern would be:
pub unsafe extern "C" fn dash_spv_ffi_client_get_filter_matched_heights( client: *mut FFIDashSpvClient, start_height: u32, end_height: u32, ) -> *mut crate::types::FFIFilterMatches { @@ - let result = client.runtime.block_on(async { - // Get chain state without taking the client - let chain_state = { - let guard = inner.lock().unwrap(); - match guard.as_ref() { - Some(client) => client.chain_state().await, - None => { - set_last_error("Client not initialized"); - return None; - } - } - }; + let result = client.runtime.block_on(async { + // Take the client out of the mutex while we await. + let mut spv_client = { + let mut guard = inner.lock().unwrap(); + match guard.take() { + Some(c) => c, + None => { + set_last_error("Client not initialized"); + return None; + } + } + }; + + let chain_state = spv_client.chain_state().await; + + // Put the client back before doing further work. + let mut guard = inner.lock().unwrap(); + *guard = Some(spv_client); @@ - // Get filter matches in range - works even during sync - let matches_result = chain_state.get_filter_matched_heights(start_height..end_height); + // Get filter matches in range. + let matches_result = chain_state.get_filter_matched_heights(start_height..end_height);This keeps the blocking mutex out of the async wait while preserving existing error reporting and FFI conversion semantics. Please double-check against how
DashSpvClientis used during long‑running sync to ensure this doesn’t conflict with your intended concurrency model.
1814-1867: Refine async + mutex usage inget_blocks_with_transactions_count.Functionally this looks good: it derives a set of unique confirmed block heights from the wallet’s transaction history and returns the count, matching the documentation.
As with
get_transaction_count, though, it keeps astd::sync::MutexGuardoninneracrosswallet.read().await, which can serialize any other caller needing the same mutex while that async lock is pending.You can follow the same pattern of cloning the wallet handle before awaiting:
pub unsafe extern "C" fn dash_spv_ffi_client_get_blocks_with_transactions_count( client: *mut FFIDashSpvClient, ) -> usize { @@ - let result = client.runtime.block_on(async { - // Get wallet without taking the client - let guard = inner.lock().unwrap(); - match guard.as_ref() { - Some(spv_client) => { - // Access wallet and get unique block heights - let wallet = spv_client.wallet(); - let wallet_guard = wallet.read().await; - let tx_history = wallet_guard.transaction_history(); - - // Count unique block heights (confirmed transactions only) - let mut unique_heights = std::collections::HashSet::new(); - for tx in tx_history { - if let Some(height) = tx.height { - unique_heights.insert(height); - } - } - - unique_heights.len() - } - None => { - tracing::warn!( - "Client not initialized when querying blocks with transactions count" - ); - 0 - } - } - }); + let result = client.runtime.block_on(async { + // Clone wallet handle without holding the mutex across .await + let wallet = { + let guard = inner.lock().unwrap(); + match guard.as_ref() { + Some(spv_client) => spv_client.wallet().clone(), + None => { + tracing::warn!( + "Client not initialized when querying blocks with transactions count" + ); + return 0; + } + } + }; + + let wallet_guard = wallet.read().await; + let tx_history = wallet_guard.transaction_history(); + + // Count unique block heights (confirmed transactions only) + let mut unique_heights = std::collections::HashSet::new(); + for tx in tx_history { + if let Some(height) = tx.height { + unique_heights.insert(height); + } + } + + unique_heights.len() + });This keeps the FFI interface the same while making the async locking behaviour friendlier to other callers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
dash-spv-ffi/FFI_API.md(9 hunks)dash-spv-ffi/src/client.rs(2 hunks)dash-spv/src/error.rs(2 hunks)dash-spv/src/storage/disk/state.rs(2 hunks)dash-spv/src/sync/sequential/message_handlers.rs(2 hunks)dash-spv/src/sync/sequential/phase_execution.rs(1 hunks)dash-spv/src/types.rs(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- dash-spv/src/sync/sequential/phase_execution.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- dash-spv/src/error.rs
- dash-spv/src/storage/disk/state.rs
🧰 Additional context used
📓 Path-based instructions (6)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code
**/*.rs: Each crate keeps sources insrc/; unit tests live alongside code with#[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code withrustfmt(seerustfmt.toml); runcargo fmt --allbefore commits
Lint Rust code withclippy; avoidunwrap()/expect()in library code; use error types (e.g.,thiserror)
Usesnake_casefor function and variable names in Rust
UseUpperCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async viatokiowhere applicable
Unit tests should be placed near code with descriptive names (e.g.,test_parse_address_mainnet)
Use#[ignore]for network-dependent or long-running tests; run with-- --ignoredflag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code
Files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv-ffi/src/client.rsdash-spv/src/types.rs
dash-spv/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/src/**/*.rs: Use trait-based abstractions for swappable implementations (e.g.,NetworkManager,StorageManager)
Use async/await throughout the codebase with tokio runtime
Use domain-specific error types (NetworkError,StorageError,SyncError,ValidationError,SpvError) for error handling
Use trait objects (Arc<dyn StorageManager>,Arc<dyn NetworkManager>) for runtime polymorphism
Use Tokio channels for inter-component message passing
Add comprehensive unit tests in-module for individual components
Files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rs
dash-spv/src/sync/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/src/sync/**/*.rs: Use sequential phase-based synchronization withSequentialSyncManagerfor organized sync phases
Use state machines withSyncStateenum to drive synchronization flow and state transitions
Files:
dash-spv/src/sync/sequential/message_handlers.rs
**/*-ffi/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Careful handling of memory safety at FFI boundaries in Rust code
Files:
dash-spv-ffi/src/client.rs
dash-spv-ffi/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/src/**/*.rs: Use#[no_mangle] extern "C"attribute when implementing new FFI functions in Rust
All FFI types must have corresponding_destroy()functions for explicit memory management
Rust strings must be returned as*const c_charwith caller responsibility to free usingdash_string_free
Input strings in FFI functions are*const c_char(borrowed, not freed by C caller)
Add cbindgen annotations for complex types in FFI functions
Use thread-local storage for error propagation viadash_spv_ffi_get_last_error()function
Files:
dash-spv-ffi/src/client.rs
dash-spv/src/types.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Define common data structures in
types.rsincludingSyncProgress,ValidationMode,WatchItem, etc.
Files:
dash-spv/src/types.rs
🧠 Learnings (32)
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Implement `StorageManager` trait with segmented storage: headers in 10,000-header segments with index files, filters in separate storage, and state persistence in dedicated paths
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv-ffi/src/client.rsdash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Implement wallet functionality with UTXO tracking, balance calculation with confirmation states, and transaction processing
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv-ffi/src/client.rsdash-spv/src/types.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/sync/sequential/message_handlers.rsdash-spv-ffi/src/client.rsdash-spv/src/types.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : In SPV sync integration, use `wallet_info.check_transaction()` with appropriate `TransactionContext` and update wallet state atomically only when `is_relevant` flag is true
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Use state machines with `SyncState` enum to drive synchronization flow and state transitions
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rs
📚 Learning: 2025-12-01T08:01:00.652Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Use sequential phase-based synchronization with `SequentialSyncManager` for organized sync phases
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates in ManagedAccount: when processing transactions, update address usage state, UTXO set, transaction history, and balance calculations together to maintain consistency
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Implement staged gap limit management using `GapLimitStage` structure with tracking of last_used_index and used_indices HashSet to enable efficient address discovery without loading entire address chains into memory
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rsdash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Use segmented storage with headers in 10,000-header chunks stored with index files for efficient retrieval
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rs
📚 Learning: 2025-06-26T16:02:42.390Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:02:42.390Z
Learning: Passing exclusive mutable references to network and storage managers into the sync manager in async Rust can lead to borrow checker issues or runtime contention if concurrent access is needed elsewhere. It's advisable to document this architectural tradeoff and consider refactoring to interior mutability or message passing for shared access as the codebase evolves.
Applied to files:
dash-spv/src/sync/sequential/message_handlers.rs
📚 Learning: 2025-12-01T08:00:50.618Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.rs : Implement new FFI functions in Rust with `#[no_mangle] extern "C"` annotation in dash-spv-ffi
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types in FFI functions
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use `#[no_mangle] extern "C"` attribute when implementing new FFI functions in Rust
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Add comprehensive unit tests in-module for individual components
Applied to files:
dash-spv-ffi/src/client.rsdash-spv/src/types.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-06-26T15:47:37.438Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/src/client.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/tests/**/*.rs : Add integration tests in the `tests/` directory for comprehensive test suites
Applied to files:
dash-spv/src/types.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/types.rs : Define common data structures in `types.rs` including `SyncProgress`, `ValidationMode`, `WatchItem`, etc.
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Use trait-based abstractions for swappable implementations (e.g., `NetworkManager`, `StorageManager`)
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account}/**/*.rs : Use BTreeMap for ordered data structures (accounts, transactions) and HashMap for quick lookups (address to index mapping) to optimize data structure performance
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Use trait objects (`Arc<dyn StorageManager>`, `Arc<dyn NetworkManager>`) for runtime polymorphism
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement configurable timeouts with recovery mechanisms for network operations
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Use domain-specific error types (`NetworkError`, `StorageError`, `SyncError`, `ValidationError`, `SpvError`) for error handling
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T08:01:00.652Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Use DashSDK's public methods, not direct SPVClient access, when accessing SPV functionality in Swift code
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : All FFI types must have corresponding `_destroy()` functions for explicit memory management
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.{c,h} : Add corresponding C tests in `tests/c_tests/` for each new FFI function
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: When debugging FFI issues, check `dash_spv_ffi_get_last_error()` for error details
Applied to files:
dash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Rust strings must be returned as `*const c_char` with caller responsibility to free using `dash_string_free`
Applied to files:
dash-spv-ffi/FFI_API.md
🧬 Code graph analysis (2)
dash-spv/src/sync/sequential/message_handlers.rs (2)
dash-spv/src/storage/disk/state.rs (2)
stats(400-431)stats(601-603)dash-spv/src/storage/disk/manager.rs (1)
new(79-129)
dash-spv-ffi/src/client.rs (3)
dash-spv-ffi/src/error.rs (1)
set_last_error(26-31)dash-spv/src/client/core.rs (2)
chain_state(206-209)wallet(161-163)dash-spv/src/client/status_display.rs (1)
chain_state(146-149)
🪛 LanguageTool
dash-spv-ffi/FFI_API.md
[grammar] ~1117-~1117: Use a hyphen to join words.
Context: ...Matches ``` Description: Get filter matched heights with wallet IDs in a giv...
(QB_NEW_EN_HYPHEN)
⏰ 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). (5)
- GitHub Check: Key Wallet Components Tests
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (8)
dash-spv/src/sync/sequential/message_handlers.rs (3)
614-618: Filter persistence before evaluation is the right approach.Storing the filter to disk before evaluating matches ensures data is persisted even if processing fails later. This aligns with the segmented storage pattern for filters.
620-632: API change to return matched wallet IDs is well-integrated.The shift from boolean to
Vec<[u8; 32]>enables tracking which specific wallets matched, supporting richer diagnostics and the new ChainState persistence. The length check on line 634 correctly replaces the previous boolean condition.
658-669: Improved logging provides useful diagnostic context.Including the matched wallet count and height in logs aids debugging filter sync issues. The log levels (debug for ChainState recording, info for block request) are appropriate.
dash-spv/src/types.rs (3)
284-288: Filter matches field uses appropriate data structure.
BTreeMap<u32, Vec<[u8; 32]>>provides ordered iteration by height and efficient range queries, which aligns well with theget_filter_matched_heightsmethod. The documentation clearly explains the purpose.Consider adding
filter_matchesto theDebugimpl (lines 543-555) for completeness, though omitting it is reasonable if the map could be large.
456-475: Range query implementation is efficient and well-bounded.The 10,000 block limit aligns with the storage segmentation pattern (headers in 10,000-header segments per coding guidelines). Using
BTreeMap::range()provides efficient O(log n + k) iteration. Thesaturating_subprevents underflow ifrange.start > range.end.
1153-1223: Unit tests provide good coverage of the new filter match API.Tests cover bulk recording, range queries, range limit enforcement, and edge cases (empty/partial ranges). The test names follow the descriptive naming convention per coding guidelines.
Note: The past review comment "method only tested, never used" is now resolved—
record_filter_matchesis called inmessage_handlers.rsline 651.dash-spv-ffi/FFI_API.md (1)
7-7: Docs for new filter and count APIs look consistent with the Rust FFI surface.The updated function counts, table rows, and detailed sections for the new filter-loading, filter-match, and transaction/block-count APIs correctly mirror the
extern "C"signatures, range limits, and destroy semantics fromdash-spv-ffi/src/client.rsanddash-spv-ffi/src/types.rs. Nothing to change here.Also applies to: 94-101, 143-165, 817-847, 1110-1123, 1190-1203, 1238-1267, 1280-1309
dash-spv-ffi/src/client.rs (1)
1281-1379: Filter-loading FFI looks sound and matches ownership expectations.The
dash_spv_ffi_client_load_filtersimplementation correctly:
- Validates the height range with a hard limit of 10,000 blocks.
- Avoids holding the client mutex across
awaitby cloning the storage handle first.- Transfers ownership of filter byte buffers and the
FFICompactFilterarray to FFI viaas_mut_ptr+mem::forget, relying on the corresponding destroy helpers.No changes needed here.
If you want an extra safety check, you can grep for the definitions of
FFICompactFilter,FFICompactFilters, anddash_spv_ffi_compact_filters_destroyto confirm the destroy path reconstructs and frees both the outer array and each filter’sdatabuffer symmetrically.
|
Need to fix the following clippy errors: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
dash-spv/src/storage/disk/filters.rs (1)
220-228: Consider parallel loading for better performance.The sequential file I/O works correctly but could be slow for large ranges (up to 10,000 reads). Consider using
futures::stream::iterwithbuffer_unorderedortokio::spawnto parallelize filter loading.Example optimization:
use futures::stream::{self, StreamExt}; let tasks = range.map(|height| async move { self.load_filter(height).await.map(|opt| opt.map(|filter| (height, filter))) }); let results: Vec<_> = stream::iter(tasks) .buffer_unordered(50) // Load up to 50 filters concurrently .collect() .await; let mut result = Vec::new(); for res in results { if let Some((height, filter)) = res? { result.push((height, filter)); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dash-spv/src/storage/disk/filters.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code
**/*.rs: Each crate keeps sources insrc/; unit tests live alongside code with#[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code withrustfmt(seerustfmt.toml); runcargo fmt --allbefore commits
Lint Rust code withclippy; avoidunwrap()/expect()in library code; use error types (e.g.,thiserror)
Usesnake_casefor function and variable names in Rust
UseUpperCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async viatokiowhere applicable
Unit tests should be placed near code with descriptive names (e.g.,test_parse_address_mainnet)
Use#[ignore]for network-dependent or long-running tests; run with-- --ignoredflag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code
Files:
dash-spv/src/storage/disk/filters.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use trait-based abstractions for core components likeNetworkManagerandStorageManagerto enable swappable implementations
Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles withcargo check --all-features
Files:
dash-spv/src/storage/disk/filters.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Storage module should provide abstraction via
StorageManagertrait with concrete implementations forMemoryStorageManagerandDiskStorageManager
Files:
dash-spv/src/storage/disk/filters.rs
🧠 Learnings (5)
📓 Common learnings
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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.
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Implement staged gap limit management using `GapLimitStage` structure with tracking of last_used_index and used_indices HashSet to enable efficient address discovery without loading entire address chains into memory
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Pre-generate addresses in batches (typically 20-100) and store in address pools; only derive on-demand when pool is exhausted to optimize performance and memory usage
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account}/**/*.rs : Use BTreeMap for ordered data structures (accounts, transactions) and HashMap for quick lookups (address to index mapping) to optimize data structure performance
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{bip32,derivation}/**/*.rs : Cache intermediate derivation results and use hardened derivation only when required; batch derive child keys when possible to optimize derivation performance
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/*ffi*.rs : For FFI bindings, expose safe C interfaces with appropriate memory management and use #[no_mangle] for public FFI functions
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Storage should use segmented storage with headers stored in 10,000-header segments with index files, separate storage for filter headers and compact block filters
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
Applied to files:
dash-spv/src/storage/disk/filters.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations
Applied to files:
dash-spv/src/storage/disk/filters.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/error.rs : Maintain domain-specific error types in `error.rs` with variants for `NetworkError`, `StorageError`, `SyncError`, `ValidationError`, and `SpvError` (top-level wrapper)
Applied to files:
dash-spv/src/storage/disk/filters.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Storage should use segmented storage with headers stored in 10,000-header segments with index files, separate storage for filter headers and compact block filters
Applied to files:
dash-spv/src/storage/disk/filters.rs
🧬 Code graph analysis (1)
dash-spv/src/storage/disk/filters.rs (3)
dash-spv/src/storage/mod.rs (1)
load_filters(154-154)dash-spv/src/storage/memory.rs (2)
load_filters(288-306)new(30-42)dash-spv/src/storage/disk/state.rs (1)
load_filters(581-583)
🔇 Additional comments (3)
dash-spv/src/storage/disk/filters.rs (3)
8-8: LGTM: Import addition supports new error handling.The
StorageErrorimport is correctly added to support the range validation error in the newload_filtersmethod.
203-209: LGTM: Clear documentation and consistent signature.The method signature correctly implements the
StorageManagertrait, and the documentation clearly explains the behavior including the range limit and missing filter handling.
210-218: LGTM: Robust range validation with clear error messaging.The range validation logic is correct and consistent with the
MemoryStorageManagerimplementation. The use ofsaturating_subprevents underflow, and the error message clearly communicates the constraint.
github-actions
bot
added
the
merge-conflict
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
github-actions
bot
removed
the
merge-conflict
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
key-wallet-manager/src/wallet_manager/process_block.rs (1)
116-118: Handlematch_anyerrors explicitly instead of silently treating them as "no match".
filter.match_any(...).unwrap_or(false)converts any error (e.g., IO errors, corrupted filter data) into a "no match" result. The subsequent cache insert permanently stores this incorrect empty result for the(network, block_hash)pair, causing the wallet manager to miss relevant blocks without indication of failure.Consider:
- Skip caching on error, allowing retry
- Add logging/tracing when errors occur
- Return
Result<Vec<[u8; 32]>, Error>to let callers handle errorsAs per coding guidelines, proper error propagation is expected.
dash-spv-ffi/FFI_API.md (2)
1105-1107: Optional: hyphenate “filter-matched heights” in the description.For grammatical correctness, consider hyphenating the compound adjective:
-Get filter matched heights with wallet IDs in a given range. +Get filter-matched heights with wallet IDs in a given range.This keeps the meaning unchanged while fixing the wording; regenerate the docs from the updated Rust comments when convenient.
822-835: Fix description: this returns a transaction count, not “blocks with transactions”.The description for
dash_spv_ffi_client_get_transaction_countstill says “Use this to show how many blocks contained relevant transactions”, but this API returns the total number of transactions across all wallets. Please update the sentence to avoid confusion, e.g.:-This returns the persisted transaction count from the wallet, not the ephemeral sync statistics. Use this to show how many blocks contained relevant transactions for the user's wallets. +This returns the persisted transaction count from the wallet, not the ephemeral sync statistics. Use this to show how many transactions have been recorded for the user's wallets.dash-spv/src/sync/message_handlers.rs (1)
607-663: Filter-match persistence via ChainState looks correct; consider batching if it becomes hot.The new logic to:
- compute
matched_wallet_ids,- bump
filters_matched,- record matches with
chain_state.record_filter_matches(height, matched_wallet_ids), and- persist via
store_chain_stateis functionally sound and ties into
ChainState::filter_matchesas intended. If you later see many matches in quick succession, you might want to cacheChainStatein memory during filter sync and flush periodically instead of load+store per match, but this is an optimization and not a blocker.dash-spv-ffi/src/client.rs (2)
1666-1741: Fixawaitwhile holdinginnermutex indash_spv_ffi_client_get_filter_matched_heights.Inside the async block you call
client.chain_state().awaitwhile holding astd::sync::MutexGuardfrominner.lock().unwrap(). This is exactly what Clippy’sawait_holding_lockwarns about and can cause unnecessary blocking or deadlock ifchain_state()ever tries to acquire the same mutex or other blocking resources.You can follow the existing pattern used elsewhere in this file: temporarily move the
DashSpvClientout of the mutex, perform the async call, then put it back, so no std mutex is held across.await:Proposed refactor for `dash_spv_ffi_client_get_filter_matched_heights`
- let result = client.runtime.block_on(async { - // Get chain state without taking the client - let chain_state = { - let guard = inner.lock().unwrap(); - match guard.as_ref() { - Some(client) => client.chain_state().await, - None => { - set_last_error("Client not initialized"); - return None; - } - } - }; - - // Get filter matches in range - works even during sync - let matches_result = chain_state.get_filter_matched_heights(start_height..end_height); + let result = client.runtime.block_on(async { + // Take client out of the mutex so no std::sync::MutexGuard is held across .await + let mut spv_client = { + let mut guard = inner.lock().unwrap(); + match guard.take() { + Some(client) => client, + None => { + set_last_error("Client not initialized"); + return None; + } + } + }; + + // Query chain state without holding the mutex + let chain_state = spv_client.chain_state().await; + + // Put client back + { + let mut guard = inner.lock().unwrap(); + *guard = Some(spv_client); + } + + // Get filter matches in range - works even during sync + let matches_result = chain_state.get_filter_matched_heights(start_height..end_height);The rest of the function (conversion into
FFIFilterMatchesand error handling) can remain unchanged.--- `1759-1786`: **Avoid holding `inner` mutex across `wallet.read().await` in both count helpers and fix doc text.** Both `dash_spv_ffi_client_get_transaction_count` and `dash_spv_ffi_client_get_blocks_with_transactions_count` currently: - Lock `inner` with `inner.lock().unwrap()`, - Call `spv_client.wallet()`, - Then `await` on `wallet.read()` while the std mutex guard is still held. This triggers Clippy’s `await_holding_lock` and can block other users of the client mutex while the async read is pending. You can fix this by cloning the `Arc<RwLock<_>>` wallet handle out of `spv_client` while holding the mutex, then dropping the guard before you `await`: <details> <summary>Proposed refactor for both count functions</summary> ```diff #[no_mangle] pub unsafe extern "C" fn dash_spv_ffi_client_get_transaction_count( client: *mut FFIDashSpvClient, ) -> usize { null_check!(client, 0); let client = &(*client); let inner = client.inner.clone(); let result = client.runtime.block_on(async { - // Get wallet without taking the client - let guard = inner.lock().unwrap(); - match guard.as_ref() { - Some(spv_client) => { - // Access wallet and get transaction count - let wallet = spv_client.wallet(); - let wallet_guard = wallet.read().await; - let tx_history = wallet_guard.transaction_history(); - tx_history.len() - } - None => { - tracing::warn!("Client not initialized when querying transaction count"); - 0 - } - } + // Clone wallet handle without holding the client mutex across .await + let wallet = { + let guard = inner.lock().unwrap(); + match guard.as_ref() { + Some(spv_client) => spv_client.wallet().clone(), + None => { + tracing::warn!("Client not initialized when querying transaction count"); + return 0; + } + } + }; + + let wallet_guard = wallet.read().await; + let tx_history = wallet_guard.transaction_history(); + tx_history.len() }); result } @@ pub unsafe extern "C" fn dash_spv_ffi_client_get_blocks_with_transactions_count( client: *mut FFIDashSpvClient, ) -> usize { null_check!(client, 0); let client = &(*client); let inner = client.inner.clone(); let result = client.runtime.block_on(async { - // Get wallet without taking the client - let guard = inner.lock().unwrap(); - match guard.as_ref() { - Some(spv_client) => { - // Access wallet and get unique block heights - let wallet = spv_client.wallet(); - let wallet_guard = wallet.read().await; - let tx_history = wallet_guard.transaction_history(); - - // Count unique block heights (confirmed transactions only) - let mut unique_heights = std::collections::HashSet::new(); - for tx in tx_history { - if let Some(height) = tx.height { - unique_heights.insert(height); - } - } - - unique_heights.len() - } - None => { - tracing::warn!( - "Client not initialized when querying blocks with transactions count" - ); - 0 - } - } + // Clone wallet handle without holding the client mutex across .await + let wallet = { + let guard = inner.lock().unwrap(); + match guard.as_ref() { + Some(spv_client) => spv_client.wallet().clone(), + None => { + tracing::warn!( + "Client not initialized when querying blocks with transactions count" + ); + return 0; + } + } + }; + + let wallet_guard = wallet.read().await; + let tx_history = wallet_guard.transaction_history(); + + // Count unique block heights (confirmed transactions only) + let mut unique_heights = std::collections::HashSet::new(); + for tx in tx_history { + if let Some(height) = tx.height { + unique_heights.insert(height); + } + } + + unique_heights.len() }); result }Also, the Rust doc comment for
dash_spv_ffi_client_get_transaction_count(mirrored into the generated FFI_API) still says:“Use this to show how many blocks contained relevant transactions…”
but this function returns a transaction count, not a block count. Please update that sentence accordingly (as suggested in the FFI_API.md comment) so the docs and behavior align.
Also applies to: 1804-1841
🧹 Nitpick comments (1)
dash-spv/src/types.rs (1)
18-18: ChainState filter-match tracking is well-structured and covered by tests.
filter_matches: BTreeMap<u32, Vec<[u8; 32]>>plusrecord_filter_matchesandget_filter_matched_heightsimplement the per-height wallet-ID tracking cleanly, with a clear 10,000-block range limit.- The new tests exercise bulk recording, typical range queries, the range limit boundary, and empty/partial overlaps, which is good coverage.
If you ever need stricter semantics, you might consider:
- deduplicating wallet IDs per height inside
record_filter_matches, or- introducing a
type WalletId = [u8; 32];alias to make signatures more self-documenting,but the current design is perfectly acceptable as-is.
Also applies to: 281-285, 429-476, 1159-1221
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
dash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/error.rsdash-spv/src/sync/filters/matching.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/sync/phase_execution.rsdash-spv/src/types.rsdash-spv/tests/storage_test.rskey-wallet-manager/src/wallet_interface.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_manager/process_block.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- dash-spv/src/error.rs
- dash-spv/tests/storage_test.rs
- key-wallet-manager/src/wallet_manager/mod.rs
🧰 Additional context used
📓 Path-based instructions (10)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use async/await throughout the codebase, built on tokio runtime
Use Arc for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
Use Tokio channels for inter-component message passing between async tasks
Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Files:
dash-spv/src/sync/filters/matching.rsdash-spv/src/sync/phase_execution.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/types.rs
dash-spv/src/sync/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Files:
dash-spv/src/sync/filters/matching.rsdash-spv/src/sync/phase_execution.rsdash-spv/src/sync/message_handlers.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode network parameters, addresses, or keys
Files:
dash-spv/src/sync/filters/matching.rsdash-spv/src/sync/phase_execution.rskey-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/types.rskey-wallet-manager/src/wallet_interface.rsdash-spv-ffi/src/client.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality
Use secure random number generation for keys
Never log or expose private keys
Code must target Rust 1.89 minimum supported version (MSRV)
Format code using cargo fmt
Pass clippy linting without warnings
**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with#[cfg(test)]annotation; integration tests use thetests/directory
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtbefore commits; ensurecargo fmt --allis run
Runcargo clippy --workspace --all-targets -- -D warningsfor linting; avoid warnings in CI
Preferasync/awaitviatokiofor asynchronous operations
Files:
dash-spv/src/sync/filters/matching.rsdash-spv/src/sync/phase_execution.rskey-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/types.rskey-wallet-manager/src/wallet_interface.rsdash-spv-ffi/src/client.rs
**/{dash-network,dash-spv,key-wallet}/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use async/await for async operations in network and wallet modules
Files:
dash-spv/src/sync/filters/matching.rsdash-spv/src/sync/phase_execution.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/types.rs
dash-spv/src/client/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization
Files:
dash-spv/src/client/block_processor_test.rs
dash-spv-ffi/tests/unit/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
Add corresponding unit tests in
tests/unit/for each new FFI function
Files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
**/*-ffi/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*-ffi/**/*.rs: Exercise careful handling at FFI boundaries for memory safety
Be careful with FFI memory management
Files:
dash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv-ffi/src/client.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/tests/**/*.rs: Create integration tests for network operations
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate
**/tests/**/*.rs: Use descriptive test names (e.g.,test_parse_address_mainnet)
Mark network-dependent or long-running tests with#[ignore]and run with-- --ignored
Files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
dash-spv-ffi/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/src/**/*.rs: Use#[no_mangle] extern "C"attribute when implementing new FFI functions in Rust
All FFI types must have corresponding_destroy()functions for explicit memory management
Rust strings must be returned as*const c_charwith caller responsibility to free usingdash_string_free
Input strings in FFI functions are*const c_char(borrowed, not freed by C caller)
Add cbindgen annotations for complex types in FFI functions
Use thread-local storage for error propagation viadash_spv_ffi_get_last_error()function
Files:
dash-spv-ffi/src/client.rs
🧠 Learnings (40)
📓 Common learnings
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Applied to files:
dash-spv/src/sync/filters/matching.rsdash-spv/src/sync/phase_execution.rskey-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/sync/message_handlers.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/types.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Applied to files:
dash-spv/src/sync/filters/matching.rskey-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rskey-wallet-manager/src/wallet_interface.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/sync/filters/matching.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/types.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : Implement transaction classification and routing through `TransactionRouter` to avoid checking all accounts for every transaction
Applied to files:
dash-spv/src/sync/filters/matching.rskey-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/client/block_processor_test.rskey-wallet-manager/src/wallet_interface.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Implement three validation modes: ValidationMode::None (no validation), ValidationMode::Basic (structure and timestamp validation), and ValidationMode::Full (complete PoW and chain validation)
Applied to files:
dash-spv/src/sync/filters/matching.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/types.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data
Applied to files:
dash-spv/src/sync/filters/matching.rskey-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/types.rskey-wallet-manager/src/wallet_interface.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-22T17:59:37.833Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.833Z
Learning: Applies to **/{dash-network,dash-spv,key-wallet}/**/*.rs : Use async/await for async operations in network and wallet modules
Applied to files:
dash-spv/src/sync/filters/matching.rskey-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/types.rsdash-spv-ffi/FFI_API.mdkey-wallet-manager/src/wallet_interface.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Support multiple `KeySource` variants (Private, Public, NoKeySource) to enable both full wallets and watch-only wallets with the same interface
Applied to files:
dash-spv/src/sync/filters/matching.rskey-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/client/block_processor_test.rskey-wallet-manager/src/wallet_interface.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Applied to files:
dash-spv/src/sync/phase_execution.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/types.rsdash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/sync/message_handlers.rskey-wallet-manager/src/wallet_interface.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Separate immutable structures (`Account`, `Wallet`) containing only identity information from mutable wrappers (`ManagedAccount`, `ManagedWalletInfo`) with state management
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/types.rskey-wallet-manager/src/wallet_interface.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Use staged gap limit management with `GapLimitStage` tracking `last_used_index` and `used_indices` to enable efficient address discovery without loading entire chains into memory
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/types.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Generate addresses in batches using gap limit and staged generation instead of unbounded address generation to prevent memory and performance issues
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use the `?` operator for error propagation, provide context in error messages, never panic in library code, and return `Result<T>` for all fallible operations
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-06-15T15:31:44.136Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 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:
key-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-06-26T16:02:42.390Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:02:42.390Z
Learning: Passing exclusive mutable references to network and storage managers into the sync manager in async Rust can lead to borrow checker issues or runtime contention if concurrent access is needed elsewhere. It's advisable to document this architectural tradeoff and consider refactoring to interior mutability or message passing for shared access as the codebase evolves.
Applied to files:
key-wallet-manager/src/wallet_manager/process_block.rsdash-spv/src/sync/message_handlers.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Applied to files:
dash-spv/src/client/block_processor_test.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/types.rsdash-spv-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
dash-spv/src/client/block_processor_test.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/types.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Applied to files:
dash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use property-based testing for complex invariants such as gap limit constraints
Applied to files:
dash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types in FFI functions
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/error.rs : Use domain-specific error types (NetworkError, StorageError, SyncError, ValidationError, SpvError) rather than generic error handling
Applied to files:
dash-spv/src/types.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase, built on tokio runtime
Applied to files:
dash-spv/src/types.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/client/**/*.rs : Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization
Applied to files:
dash-spv/src/types.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement configurable timeouts with recovery mechanisms for network operations
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use Tokio channels for inter-component message passing between async tasks
Applied to files:
dash-spv/src/types.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use `#[no_mangle] extern "C"` attribute when implementing new FFI functions in Rust
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.{c,h} : Add corresponding C tests in `tests/c_tests/` for each new FFI function
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : All FFI types must have corresponding `_destroy()` functions for explicit memory management
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-06-26T15:47:37.438Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: When debugging FFI issues, check `dash_spv_ffi_get_last_error()` for error details
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Rust strings must be returned as `*const c_char` with caller responsibility to free using `dash_string_free`
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-06-26T15:46:56.854Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/include/dash_spv_ffi.h
📚 Learning: 2025-08-21T05:01:58.949Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/client.rs
📚 Learning: 2025-06-26T15:54:02.509Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:54:02.509Z
Learning: The `StorageManager` trait in `dash-spv/src/storage/mod.rs` uses `&mut self` methods but is also `Send + Sync`, and implementations often use interior mutability for concurrency. This can be confusing, so explicit documentation should clarify thread-safety expectations and the rationale for the API design.
Applied to files:
dash-spv-ffi/src/client.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use Arc<dyn TraitName> for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
Applied to files:
dash-spv-ffi/src/client.rs
🧬 Code graph analysis (3)
key-wallet-manager/src/wallet_manager/process_block.rs (3)
key-wallet/src/wallet/managed_wallet_info/mod.rs (2)
network(116-118)new(56-67)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
network(32-32)network(127-129)key-wallet-manager/src/wallet_manager/mod.rs (2)
new(72-78)new(109-116)
dash-spv/src/client/block_processor_test.rs (4)
dash-spv/tests/chainlock_validation_test.rs (1)
new(32-37)dash-spv/tests/edge_case_filter_sync_test.rs (1)
new(51-55)dash-spv/tests/block_download_test.rs (1)
new(39-45)dash-spv/tests/filter_header_verification_test.rs (1)
new(46-50)
key-wallet-manager/src/wallet_interface.rs (2)
key-wallet-manager/src/wallet_manager/process_block.rs (1)
check_compact_filter(87-132)dash-spv/src/client/block_processor_test.rs (2)
check_compact_filter(65-73)check_compact_filter(290-298)
🪛 LanguageTool
dash-spv-ffi/FFI_API.md
[grammar] ~1106-~1106: Use a hyphen to join words.
Context: ...Matches ``` Description: Get filter matched heights with wallet IDs in a giv...
(QB_NEW_EN_HYPHEN)
⏰ 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 (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
🔇 Additional comments (15)
key-wallet-manager/src/wallet_manager/process_block.rs (3)
87-92: LGTM: Function signature correctly updated for per-wallet tracking.The return type change from
booltoVec<[u8; 32]>enables tracking which specific wallets match the compact filter, aligning with the broader per-wallet matching architecture described in the PR.
94-98: LGTM: Cache lookup logic correctly handles new return type.The early return with cloned vector is appropriate and avoids redundant filter evaluations.
125-132: Cache insertion logic is correct.The nested
BTreeMapupdate correctly stores the matched wallet IDs. Note that fixing the error handling in lines 116-118 will ensure only valid results are cached here.dash-spv/src/sync/phase_execution.rs (1)
150-150: LGTM! Formatting improvement.The blank line improves readability by adding visual separation between the variable declarations.
Note: The PR objectives mention Clippy warnings about
await_holding_lockindash-spv-ffi/src/client.rs(lines 1718, 1795, 1840). Those warnings are in a different file not included in this review.dash-spv-ffi/FFI_API.md (1)
7-7: New FFI API surface and counts look consistent.The updated totals, section counts, and new FFI entries for filter loading, filter matches, and the *_destroy helpers are coherent with the Rust implementations and ownership semantics. No issues from the docs side.
Also applies to: 93-101, 142-170, 152-164, 160-168
dash-spv/src/sync/filters/matching.rs (1)
25-49: check_filter_for_matches correctly adopts per-wallet ID semantics.Returning
SyncResult<Vec<[u8; 32]>>and delegating towallet.check_compact_filterwith a simple “non-empty => log and propagate” flow matches the new wallet interface and keeps the logic straightforward.dash-spv-ffi/tests/unit/test_type_conversions.rs (1)
163-178: ChainState test updated correctly for newfilter_matchesfield.Initializing
filter_matcheswithBTreeMap::new()keepstest_chain_state_none_valuesin sync with the struct definition and FFI conversion without changing the test’s intent.key-wallet-manager/src/wallet_interface.rs (1)
35-47: WalletInterface::check_compact_filter trait update is clear and consistent.Switching to
Vec<[u8; 32]>with documented semantics (empty = no match, non-empty = matched wallet IDs) is well-specified and aligns with the new per-wallet filter matching pipeline.dash-spv/src/client/block_processor_test.rs (1)
65-73: Test wallets correctly adapted to Vec-basedcheck_compact_filter.Both
MockWallet(always returns one wallet ID) andNonMatchingWallet(always returns an empty vector) now match the updated trait while preserving the intended “match / no-match” behavior in the tests.Also applies to: 290-297
dash-spv-ffi/src/client.rs (1)
1254-1353: FFI filter loading implementation looks correct and lock-safe.
dash_spv_ffi_client_load_filters:
- Enforces the 10,000-block range limit up front.
- Clones the storage handle while holding
inner.lock()and drops the guard before any.await.- Converts the loaded filters into
FFICompactFilterswith ownership correctly transferred to FFI.No concurrency or ownership issues stand out here.
dash-spv-ffi/include/dash_spv_ffi.h (5)
112-152: LGTM - Well-documented memory management.The
FFICompactFilterandFFICompactFiltersstructures follow proper FFI patterns with clear documentation about heap allocation and the required cleanup functions. The ownership transfer semantics are explicit.
216-251: LGTM - Proper use of fixed-size arrays for wallet IDs.The structures correctly use fixed 32-byte arrays for wallet IDs, consistent with the established pattern for transaction IDs and block hashes in the FFI layer. Memory management is well-documented.
Based on learnings, fixed-size byte arrays are preferred over C strings for hash-like identifiers.
705-741: LGTM - Simple counter APIs with sensible defaults.The transaction and block count functions return primitive types (no memory management needed) and sensibly return 0 when the client is not initialized or wallet is unavailable. Documentation clearly indicates these are persistent metrics from wallet storage, not ephemeral sync statistics.
1148-1190: LGTM - Complete memory management lifecycle.All new heap-allocated types have corresponding destroyer functions with proper safety documentation. This satisfies the FFI requirement that all FFI types must have
_destroy()functions for explicit memory management.Based on learnings, proper cleanup functions are required for all FFI types.
547-575: API design looks solid with proper FFI safety patterns.The new filter APIs follow correct FFI patterns with clear documentation about ownership transfer, safety requirements, error handling via thread-local storage, and range limits (10,000 blocks).
The implementation properly handles async/await by scoping MutexGuard locks and dropping them immediately before awaiting storage operations, avoiding any lock-across-await violations.
|
Just for the record, closing this for now because i don't really think we need it and it goes a bit in a different direction that i had in mind in regard to filter matching. Also im just stubborn and talked @QuantumExplorer out of it even though he wanted it. But i might be wrong here and we will end up re-opening this later if we run into trouble on iOS and potentially need more debug information there. |
|
There is always the option of exposing this kind of tools with a Rust feature named |
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.