feat: Capture new addresses from maintain_gap_limit
#287
Conversation
📝 WalkthroughWalkthroughBlock and transaction processing returns were replaced with structured results: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
|
Reopen to trigger CI. |
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. |
xdustinface
force-pushed
the
feat/capture-new-addresses
branch
from
5ac6510 to
c108a63
Compare
github-actions
bot
removed
the
merge-conflict
![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 (2)
key-wallet-manager/src/wallet_manager/mod.rs (1)
542-542: Consider deduplication if multiple wallets could generate identical addresses.The code aggregates addresses from all wallets without deduplication. While proper key derivation should prevent address collisions between wallets, consider using a
BTreeSetor deduplicating before returning if duplicate addresses could cause issues in downstream rescanning logic.💡 Optional deduplication approach
If deduplication is desired:
// At the end of the function, before returning result.new_addresses.sort(); result.new_addresses.dedup();Or collect into a set during aggregation:
// Change CheckTransactionsResult.new_addresses to BTreeSet<Address> // Then use insert instead of extend result.new_addresses.extend(check_result.new_addresses);key-wallet/src/transaction_checking/wallet_checker.rs (1)
396-900: Consider adding test coverage for thenew_addressesfield.The test suite doesn't verify that
new_addressesis correctly populated when gap limit maintenance generates new addresses. Consider adding tests to ensure:
new_addressescontains addresses generated during gap limit maintenancenew_addressesis empty when no new addresses are generated- Addresses are captured from both external and internal address pools
Note: If this is covered by PR #286, you can disregard this comment.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
dash-spv/src/client/block_processor.rs(3 hunks)dash-spv/src/client/block_processor_test.rs(3 hunks)dash-spv/src/sync/message_handlers.rs(1 hunks)key-wallet-ffi/src/wallet_manager.rs(1 hunks)key-wallet-manager/Cargo.toml(1 hunks)key-wallet-manager/src/lib.rs(1 hunks)key-wallet-manager/src/wallet_interface.rs(1 hunks)key-wallet-manager/src/wallet_manager/mod.rs(4 hunks)key-wallet-manager/src/wallet_manager/process_block.rs(3 hunks)key-wallet-manager/tests/spv_integration_tests.rs(2 hunks)key-wallet/src/transaction_checking/account_checker.rs(2 hunks)key-wallet/src/transaction_checking/wallet_checker.rs(2 hunks)test-utils/src/builders.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- key-wallet-manager/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (4)
- dash-spv/src/sync/message_handlers.rs
- key-wallet-ffi/src/wallet_manager.rs
- key-wallet-manager/src/wallet_interface.rs
- key-wallet-manager/src/wallet_manager/process_block.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:
key-wallet-manager/src/lib.rskey-wallet/src/transaction_checking/account_checker.rstest-utils/src/builders.rskey-wallet-manager/tests/spv_integration_tests.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/mod.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/client/block_processor.rs
key-wallet/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/**/*.rs: Separate immutable structures (Account,Wallet) containing only identity information from mutable wrappers (ManagedAccount,ManagedWalletInfo) with state management
Never serialize or log private keys in production; use public keys or key fingerprints for identification instead
Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
UseBTreeMapfor ordered data (accounts, transactions) andHashMapfor lookups (address mappings); apply memory management strategies for old transaction data
Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Use the?operator for error propagation, provide context in error messages, never panic in library code, and returnResult<T>for all fallible operations
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/**/transaction_checking/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Implement transaction classification and routing through
TransactionRouterto avoid checking all accounts for every transaction
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/tests/**/*.rs: Write integration tests for network operations in Rust
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate in RustIntegration tests use
tests/directory (e.g.,rpc-integration-test)
Files:
key-wallet-manager/tests/spv_integration_tests.rs
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/client/block_processor_test.rsdash-spv/src/client/block_processor.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.rsdash-spv/src/client/block_processor.rs
🧠 Learnings (33)
📓 Common learnings
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
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
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
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
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
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-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/lib.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/mod.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:
key-wallet-manager/src/lib.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/mod.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/lib.rskey-wallet-manager/src/wallet_manager/mod.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:
key-wallet-manager/src/lib.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/mod.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/lib.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet-manager/tests/spv_integration_tests.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/mod.rsdash-spv/src/client/block_processor.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/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/mod.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:
key-wallet/src/transaction_checking/account_checker.rskey-wallet-manager/tests/spv_integration_tests.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/mod.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/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.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 : Pre-generate addresses in batches (typically 20-100) and store them in pools; only derive on-demand when the pool is exhausted
Applied to files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.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/**/account/**/*.rs : Use enum-based type system for `AccountType` with specific variants (Standard, IdentityAuthentication, IdentityEncryption, MasternodeOperator, etc.) to provide compile-time safety and clear semantics
Applied to files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet-manager/src/wallet_manager/mod.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:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/mod.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:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.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:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/client/block_processor.rs
📚 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: Cover critical parsing, networking, SPV, and wallet flows with tests; add regression tests for fixes; consider property tests with `proptest`
Applied to files:
key-wallet-manager/tests/spv_integration_tests.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/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.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/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Applied to files:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/client/block_processor.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:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.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 : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Applied to files:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.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:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.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:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.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:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/client/block_processor.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/tests/spv_integration_tests.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/mod.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: 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/src/client/block_processor_test.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/src/client/block_processor_test.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/src/client/block_processor_test.rsdash-spv/src/client/block_processor.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/client/block_processor_test.rs
📚 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/src/client/block_processor_test.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/client/block_processor_test.rsdash-spv/src/client/block_processor.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/client/block_processor_test.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/src/client/block_processor_test.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/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/src/client/block_processor.rs
🧬 Code graph analysis (2)
key-wallet-manager/tests/spv_integration_tests.rs (3)
test-utils/src/builders.rs (3)
create_transaction_to_address(164-169)new(36-38)new(111-113)dash-spv/src/types.rs (1)
txid(1008-1010)key-wallet-manager/src/wallet_manager/mod.rs (2)
new(80-86)new(117-124)
dash-spv/src/client/block_processor.rs (3)
dash-spv/src/client/core.rs (1)
wallet(158-160)dash-spv-ffi/src/client.rs (1)
txid(273-273)dash-spv/src/types.rs (1)
txid(1008-1010)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: SPV Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: Pre-commit (ubuntu-latest)
- GitHub Check: Pre-commit (macos-latest)
🔇 Additional comments (15)
key-wallet-manager/src/lib.rs (1)
43-43: LGTM: Public API addition aligns with structured result pattern.The re-export of
BlockProcessingResultappropriately exposes the new structured return type to crate consumers, replacing the previous rawVec<Txid>pattern with a more extensible structure.test-utils/src/builders.rs (1)
164-169: LGTM: Clean test utility for address-targeted transactions.The helper function provides a straightforward way to create test transactions directed to specific addresses, which is useful for testing address monitoring and transaction relevance detection.
dash-spv/src/client/block_processor.rs (2)
229-245: LGTM: Refactored to use structured BlockProcessingResult.The update correctly accesses
result.relevant_txidsfrom the new structured return type, improving code clarity and type safety.
229-279: Verify: new_addresses field is captured but unused.The
BlockProcessingResult.new_addressesfield is returned fromwallet.process_blockbut never accessed in this file. Per the PR description, this is intended "to enable a later PR to rescan newly generated addresses during synchronization."Please confirm this is the intended design and that the follow-up PR is planned soon to utilize this data. If the delay is significant, consider documenting this with a TODO comment to avoid confusion.
key-wallet-manager/tests/spv_integration_tests.rs (3)
3-10: LGTM: Updated imports support enhanced test scenarios.The expanded imports enable construction of targeted test transactions and validation of block processing results, aligning with the new BlockProcessingResult structure.
106-136: LGTM: Comprehensive test validates BlockProcessingResult fields.The test effectively validates:
relevant_txidscorrectly filters transactions affecting monitored addresses (tx1 and tx2) while ignoring external addresses (tx3)new_addressesaccurately reports newly generated addresses from gap limit maintenance- Address cache consistency after block processing
This provides solid coverage for the new structured result type.
138-157: LGTM: Test validates empty result for unrelated transactions.The negative test case correctly verifies that transactions to external addresses produce an empty
BlockProcessingResult, ensuring the filtering logic works as expected.dash-spv/src/client/block_processor_test.rs (3)
11-11: LGTM: Import added for new result type.
49-57: LGTM: Mock updated to return BlockProcessingResult.The mock wallet correctly returns the structured result with
relevant_txidspopulated from block transactions and an emptynew_addressesvector, which is appropriate for test scenarios.
280-281: LGTM: Non-matching wallet returns default result.Using
BlockProcessingResult::default()appropriately indicates no matches, which should produce empty vectors for both fields.key-wallet-manager/src/wallet_manager/mod.rs (2)
52-59: LGTM! Well-structured result type.The new
CheckTransactionsResultstruct clearly captures both affected wallets and newly generated addresses. TheDefaultderive enables clean initialization.
520-543: Verifycheck_transactionreturn type and update all call sites.The implementation correctly aggregates
new_addressesfrom each wallet'scheck_transactionresult. Verification confirms:
- The
check_transactionmethod returnsTransactionCheckResultwithis_relevant: boolandnew_addresses: Vec<Address>fields- All call sites of
check_transaction_in_all_walletshandle the newCheckTransactionsResultreturn type correctly, accessingaffected_walletsandnew_addressesas expectedkey-wallet/src/transaction_checking/wallet_checker.rs (1)
78-78: LGTM!The mutation is necessary to accumulate new addresses generated during gap limit maintenance.
key-wallet/src/transaction_checking/account_checker.rs (2)
41-42: LGTM!The new
new_addressesfield is properly documented and typed correctly for capturing addresses generated during gap limit maintenance.
289-289: LGTM!The field is properly initialized with an empty vector, which will be populated during gap limit maintenance when state updates are performed.
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. |
xdustinface
force-pushed
the
feat/capture-new-addresses
branch
from
d59fe60 to
46fc77a
Compare
github-actions
bot
removed
the
merge-conflict
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
key-wallet-manager/tests/spv_integration_tests.rs (1)
104-116: Index safety: Test assumes at least 2 pre-generated addresses.The test indexes
addresses[0]andaddresses[1](lines 114-115) but only asserts!addresses.is_empty()(line 105). WhileWalletAccountCreationOptions::Defaultlikely pre-generates 20+ addresses (based on typical gap limits), the test is fragile without explicit verification.Optional: More robust assertion
let addresses = manager.monitored_addresses(); - assert!(!addresses.is_empty()); + assert!(addresses.len() >= 2, "Need at least 2 pre-generated addresses for test"); let external = dashcore::Address::p2pkh(
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
dash-spv/src/client/block_processor.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rskey-wallet-ffi/src/wallet_manager.rskey-wallet-manager/Cargo.tomlkey-wallet-manager/src/lib.rskey-wallet-manager/src/wallet_interface.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_manager/process_block.rskey-wallet-manager/tests/spv_integration_tests.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rstest-utils/src/builders.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- key-wallet-ffi/src/wallet_manager.rs
- key-wallet-manager/src/lib.rs
- dash-spv/src/sync/message_handlers.rs
- test-utils/src/builders.rs
- key-wallet-manager/Cargo.toml
- key-wallet/src/transaction_checking/account_checker.rs
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode network parameters, addresses, or keys
Files:
key-wallet-manager/src/wallet_interface.rskey-wallet-manager/tests/spv_integration_tests.rskey-wallet-manager/src/wallet_manager/mod.rsdash-spv/src/client/block_processor.rsdash-spv/src/client/block_processor_test.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/process_block.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:
key-wallet-manager/src/wallet_interface.rskey-wallet-manager/tests/spv_integration_tests.rskey-wallet-manager/src/wallet_manager/mod.rsdash-spv/src/client/block_processor.rsdash-spv/src/client/block_processor_test.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-manager/src/wallet_manager/process_block.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:
key-wallet-manager/tests/spv_integration_tests.rs
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/client/block_processor.rsdash-spv/src/client/block_processor_test.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.rsdash-spv/src/client/block_processor_test.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/client/block_processor.rsdash-spv/src/client/block_processor_test.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/**/*.rs: Separate immutable structures (Account,Wallet) containing only identity information from mutable wrappers (ManagedAccount,ManagedWalletInfo) with state management
Never serialize or log private keys in production; use public keys or key fingerprints for identification instead
Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
UseBTreeMapfor ordered data (accounts, transactions) andHashMapfor lookups (address mappings); apply memory management strategies for old transaction data
Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Use the?operator for error propagation, provide context in error messages, never panic in library code, and returnResult<T>for all fallible operations
Files:
key-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/**/transaction_checking/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Implement transaction classification and routing through
TransactionRouterto avoid checking all accounts for every transaction
Files:
key-wallet/src/transaction_checking/wallet_checker.rs
🧠 Learnings (31)
📓 Common learnings
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
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
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
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
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
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-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_interface.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-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/**/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_interface.rskey-wallet-manager/tests/spv_integration_tests.rskey-wallet-manager/src/wallet_manager/mod.rsdash-spv/src/client/block_processor.rsdash-spv/src/client/block_processor_test.rskey-wallet/src/transaction_checking/wallet_checker.rskey-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 `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data
Applied to files:
key-wallet-manager/src/wallet_interface.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-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/**/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:
key-wallet-manager/src/wallet_interface.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/wallet_checker.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/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Applied to files:
key-wallet-manager/src/wallet_interface.rskey-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor.rsdash-spv/src/client/block_processor_test.rskey-wallet-manager/src/wallet_manager/process_block.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/{dash-network,dash-spv,key-wallet}/**/*.rs : Use async/await for async operations in network and wallet modules
Applied to files:
key-wallet-manager/src/wallet_interface.rskey-wallet-manager/tests/spv_integration_tests.rskey-wallet-manager/src/wallet_manager/mod.rsdash-spv/src/client/block_processor.rsdash-spv/src/client/block_processor_test.rskey-wallet-manager/src/wallet_manager/process_block.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:
key-wallet-manager/src/wallet_interface.rskey-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor.rsdash-spv/src/client/block_processor_test.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:
key-wallet-manager/src/wallet_interface.rsdash-spv/src/client/block_processor_test.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:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.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 : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Applied to files:
key-wallet-manager/tests/spv_integration_tests.rsdash-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/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`
Applied to files:
key-wallet-manager/tests/spv_integration_tests.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:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)
Applied to files:
key-wallet-manager/tests/spv_integration_tests.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:
key-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor_test.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:
key-wallet-manager/tests/spv_integration_tests.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/tests/spv_integration_tests.rskey-wallet/src/transaction_checking/wallet_checker.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:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-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 : Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Applied to files:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/wallet_checker.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/mod.rskey-wallet/src/transaction_checking/wallet_checker.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/**/account/**/*.rs : Use enum-based type system for `AccountType` with specific variants (Standard, IdentityAuthentication, IdentityEncryption, MasternodeOperator, etc.) to provide compile-time safety and clear semantics
Applied to files:
key-wallet-manager/src/wallet_manager/mod.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/src/client/block_processor.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/client/block_processor.rsdash-spv/src/client/block_processor_test.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/client/block_processor_test.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/src/client/block_processor_test.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/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/**/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/src/transaction_checking/wallet_checker.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/src/transaction_checking/wallet_checker.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 : Pre-generate addresses in batches (typically 20-100) and store them in pools; only derive on-demand when the pool is exhausted
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.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/src/transaction_checking/wallet_checker.rs
🧬 Code graph analysis (3)
key-wallet-manager/tests/spv_integration_tests.rs (2)
test-utils/src/builders.rs (3)
create_transaction_to_address(164-169)new(36-38)new(111-113)dash-spv/tests/block_download_test.rs (1)
create_test_block(139-153)
dash-spv/src/client/block_processor_test.rs (2)
key-wallet-manager/src/wallet_interface.rs (1)
process_block(27-31)key-wallet-manager/src/wallet_manager/process_block.rs (1)
process_block(16-45)
key-wallet/src/transaction_checking/wallet_checker.rs (1)
key-wallet/src/transaction_checking/account_checker.rs (1)
account_index(180-212)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Key Wallet Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (9)
key-wallet-manager/src/wallet_manager/mod.rs (3)
52-59: LGTM! Well-structured result type.The
CheckTransactionsResultstruct clearly separates the affected wallets from newly generated addresses, making the return value self-documenting. TheDefaultderive enables clean initialization.
462-469: All call sites ofcheck_transaction_in_all_walletshave been properly updated to handle theCheckTransactionsResultreturn type. The breaking change fromVec<WalletId>has been fully implemented:
key-wallet-ffi/src/wallet_manager.rs:771accessesresult.affected_walletskey-wallet-manager/src/wallet_manager/process_block.rs:33accesses bothcheck_result.affected_walletsandcheck_result.new_addresseskey-wallet-manager/src/wallet_manager/process_block.rs:51awaits the result (intentionally unused for mempool transactions)
482-491: No action required — return type is correct.The
). The code correctly accesses these fields at lines 486 and 491. Collecting new addresses outside the relevance check is correct behavior for gap limit maintenance.WalletTransactionChecker::check_transactionmethod returnsTransactionCheckResult, which includes both required fields:is_relevant(bool) andnew_addresses(Veckey-wallet/src/transaction_checking/wallet_checker.rs (1)
78-78: LGTM: Clean integration of new address capture with proper error handling.The mutation of
resultand the error-handling pattern formaintain_gap_limitare well-implemented:
- Successfully generated addresses are accumulated in
result.new_addresses- Errors are logged with full context (account index, pool type, error message) per the previous review feedback
- Processing continues if one pool fails, allowing other pools to succeed
This aligns with the PR's goal of capturing new addresses from gap-limit maintenance for downstream rescanning.
Also applies to: 284-320
key-wallet-manager/src/wallet_interface.rs (1)
12-31: LGTM: Well-designed API evolution.The introduction of
BlockProcessingResultfollows good API design patterns:
- Structured result type allows future expansion without breaking changes
- Clear documentation of both fields
- Appropriate derives (Debug, Default, Clone) for a result type
- Public fields are correct for this pattern
key-wallet-manager/tests/spv_integration_tests.rs (1)
121-133: LGTM: Comprehensive validation of new address generation.The test properly verifies:
- Relevant transaction detection (2 out of 3 transactions)
- New address generation count
- New addresses appear in the monitored set
- Monitored set growth matches new address count
key-wallet-manager/src/wallet_manager/process_block.rs (1)
16-45: LGTM: Correct accumulation of block processing results.The implementation properly accumulates both fields:
relevant_txidsare collected only whenaffected_walletsis non-empty (line 35-36)new_addressesare accumulated for every transaction (line 39), which is correct since gap-limit maintenance can occur during any transaction's processing, regardless of relevanceThe refactoring cleanly integrates the new structured result without changing the core processing logic.
dash-spv/src/client/block_processor_test.rs (1)
44-53: LGTM: Test mocks properly updated for new interface.Both mock implementations correctly return
BlockProcessingResult:
MockWalletreturns all transaction IDs as relevant with emptynew_addresses(reasonable test simplification)NonMatchingWalletusesBlockProcessingResult::default()for empty resultsAlso applies to: 255-261
dash-spv/src/client/block_processor.rs (1)
225-275: LGTM: Consistent usage of BlockProcessingResult throughout.All references properly use
result.relevant_txids:
- Checking if empty (line 226)
- Logging count (line 229)
- Iterating for event emission (line 241)
- Reporting in BlockProcessed event (line 274)
The
new_addressesfield is not used here, which aligns with the PR description stating it's for a subsequent PR to implement rescanning.
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. |
xdustinface
force-pushed
the
feat/capture-new-addresses
branch
from
46fc77a to
2b96afc
Compare
github-actions
bot
removed
the
merge-conflict
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
key-wallet/src/transaction_checking/wallet_checker.rs (1)
312-313: Minor: Logging format differs between pool types.For Standard accounts,
pool_typeuses string literals ("external","internal"), while generic pools use debug format (?pool.pool_type). This is functionally fine, but consider using a consistent format for log aggregation purposes:pool_type = ?pool.pool_type,or
pool_type = %pool.pool_type,This is a minor observation and not blocking.
dash-spv/src/client/block_processor.rs (1)
268-273: Note:result.new_addressesis not used yet.The
new_addressesfield fromBlockProcessingResultis populated but not consumed in this block processor. Per the PR description, this is intentional as a subsequent PR will use these addresses for rescanning during synchronization.Consider adding a brief comment or TODO to document this future integration point:
+ // TODO: Use result.new_addresses for rescanning in subsequent PR // Emit BlockProcessed event with actual relevant transaction countkey-wallet-manager/tests/spv_integration_tests.rs (1)
106-109: This pattern is used consistently throughout the codebase as an intentional test placeholder.The
[0x02; 33]public key pattern is intentionally used as a dummy/placeholder key in multiple test files in the codebase (wallet_checker.rs,routing.rs,provider.rs, andspv_integration_tests.rs). The tests work correctly with this pattern—the key serves its purpose by creating an address that does not match any monitored wallet addresses, which is what the test verifies (line 124:assert!(!result.relevant_txids.contains(&tx3.txid()))).While using a mathematically valid test public key (e.g., the generator point G) would be a code quality improvement, the current pattern is functional and idiomatic within this codebase. No changes are required.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
dash-spv/src/client/block_processor.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/sync/message_handlers.rskey-wallet-ffi/src/wallet_manager.rskey-wallet-manager/Cargo.tomlkey-wallet-manager/src/lib.rskey-wallet-manager/src/wallet_interface.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_manager/process_block.rskey-wallet-manager/tests/spv_integration_tests.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rstest-utils/src/builders.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- key-wallet-ffi/src/wallet_manager.rs
- dash-spv/src/client/block_processor_test.rs
- key-wallet-manager/Cargo.toml
- dash-spv/src/sync/message_handlers.rs
🧰 Additional context used
📓 Path-based instructions (8)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.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
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types (thiserror) and propagate errors appropriately in Rust
Use tokio runtime for async operations in Rust
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format code using cargo fmt
Run clippy with all features and all targets, treating warnings as errors
Never log or expose private keys in any code
Always validate inputs from untrusted sources in Rust
Use secure random number generation for keys
Files:
key-wallet-manager/src/lib.rskey-wallet-manager/src/wallet_manager/mod.rstest-utils/src/builders.rskey-wallet-manager/src/wallet_interface.rskey-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet-manager/src/wallet_manager/process_block.rskey-wallet/src/transaction_checking/wallet_checker.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/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:
key-wallet-manager/tests/spv_integration_tests.rs
**/*integration*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Write integration tests for network operations
Files:
key-wallet-manager/tests/spv_integration_tests.rs
**/*test*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*test*.rs: Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate
Files:
key-wallet-manager/tests/spv_integration_tests.rs
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/client/block_processor.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.rs
key-wallet/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/**/*.rs: Separate immutable structures (Account,Wallet) containing only identity information from mutable wrappers (ManagedAccount,ManagedWalletInfo) with state management
Never serialize or log private keys in production; use public keys or key fingerprints for identification instead
Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
UseBTreeMapfor ordered data (accounts, transactions) andHashMapfor lookups (address mappings); apply memory management strategies for old transaction data
Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Use the?operator for error propagation, provide context in error messages, never panic in library code, and returnResult<T>for all fallible operations
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/**/transaction_checking/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Implement transaction classification and routing through
TransactionRouterto avoid checking all accounts for every transaction
Files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rs
🧠 Learnings (27)
📓 Common learnings
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
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
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
📚 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:
key-wallet-manager/src/lib.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_interface.rskey-wallet/src/transaction_checking/wallet_checker.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/lib.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_interface.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet-manager/src/wallet_manager/process_block.rskey-wallet/src/transaction_checking/wallet_checker.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/lib.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/wallet_checker.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:
key-wallet-manager/src/lib.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_interface.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet-manager/src/wallet_manager/process_block.rskey-wallet/src/transaction_checking/wallet_checker.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/lib.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/src/wallet_interface.rskey-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet-manager/src/wallet_manager/process_block.rskey-wallet/src/transaction_checking/wallet_checker.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:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet-manager/src/wallet_manager/process_block.rskey-wallet/src/transaction_checking/wallet_checker.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:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/wallet_checker.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/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Applied to files:
key-wallet-manager/src/wallet_interface.rskey-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor.rskey-wallet-manager/src/wallet_manager/process_block.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:
key-wallet-manager/src/wallet_interface.rskey-wallet-manager/tests/spv_integration_tests.rsdash-spv/src/client/block_processor.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:
key-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/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:
key-wallet-manager/tests/spv_integration_tests.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`
Applied to files:
key-wallet-manager/tests/spv_integration_tests.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:
key-wallet-manager/tests/spv_integration_tests.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 : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Applied to files:
key-wallet-manager/tests/spv_integration_tests.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/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
key-wallet-manager/tests/spv_integration_tests.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)
Applied to files:
key-wallet-manager/tests/spv_integration_tests.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:
key-wallet-manager/tests/spv_integration_tests.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:
key-wallet-manager/tests/spv_integration_tests.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/tests/spv_integration_tests.rskey-wallet/src/transaction_checking/wallet_checker.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/src/client/block_processor.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/client/block_processor.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/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.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/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.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 : Pre-generate addresses in batches (typically 20-100) and store them in pools; only derive on-demand when the pool is exhausted
Applied to files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.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/**/account/**/*.rs : Use enum-based type system for `AccountType` with specific variants (Standard, IdentityAuthentication, IdentityEncryption, MasternodeOperator, etc.) to provide compile-time safety and clear semantics
Applied to files:
key-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.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/src/transaction_checking/wallet_checker.rs
🧬 Code graph analysis (5)
key-wallet-manager/src/wallet_manager/mod.rs (2)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
wallet_id(35-35)wallet_id(134-136)key-wallet/src/wallet/mod.rs (1)
wallet_id(116-116)
key-wallet-manager/tests/spv_integration_tests.rs (2)
key-wallet/src/transaction_checking/wallet_checker.rs (1)
create_transaction_to_address(433-452)test-utils/src/builders.rs (3)
create_transaction_to_address(164-169)new(36-38)new(111-113)
dash-spv/src/client/block_processor.rs (3)
dash-spv/src/client/core.rs (1)
wallet(153-155)dash-spv/src/types.rs (1)
txid(985-987)dash/src/blockdata/transaction/mod.rs (1)
txid(199-214)
key-wallet-manager/src/wallet_manager/process_block.rs (1)
key-wallet-manager/src/wallet_interface.rs (1)
process_block(27-31)
key-wallet/src/transaction_checking/wallet_checker.rs (1)
key-wallet/src/transaction_checking/account_checker.rs (1)
account_index(180-212)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: Pre-commit (ubuntu-latest)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: Pre-commit (macos-latest)
🔇 Additional comments (14)
key-wallet-manager/src/wallet_manager/mod.rs (2)
52-59: LGTM! Well-structured result type.The
CheckTransactionsResultstruct provides a clear composite result that aligns with the broaderBlockProcessingResultpattern introduced in this PR. The use ofDefaultsimplifies initialization.
462-496: LGTM! Properly aggregates results from all wallets.The function correctly accumulates both
affected_walletsandnew_addressesfrom each wallet's transaction check. The logic properly extendsresult.new_addresseswith per-wallet results that are populated by thecheck_transactionmethod inwallet_checker.rswhen it callsmaintain_gap_limit()on address pools during state updates.key-wallet-manager/src/lib.rs (1)
43-43: LGTM! Proper public API exposure.The re-export of
BlockProcessingResultmakes it conveniently accessible to external consumers through the crate root, consistent with other public types.test-utils/src/builders.rs (1)
164-169: LGTM! Useful test helper.The
create_transaction_to_addresshelper provides a convenient way to construct test transactions directed to specific addresses. The implementation is straightforward and appropriate for testing scenarios.key-wallet-manager/src/wallet_interface.rs (2)
12-19: LGTM! Well-documented result structure.The
BlockProcessingResultstruct provides a clear composite result for block processing. The documentation appropriately describes both fields.
24-31: Breaking API change has been properly migrated across all call sites.The return type change from
Vec<Txid>toBlockProcessingResultis confirmed. All implementations and call sites have been correctly updated to useresult.relevant_txidsinstead of accessing a vector directly. The new structure also appropriately includesnew_addressesfor gap limit maintenance information.key-wallet/src/transaction_checking/account_checker.rs (1)
41-42: Field is properly populated viamaintain_gap_limitcalls.The
new_addressesfield inTransactionCheckResultis initialized asVec::new()at line 289 inaccount_checker.rs, and it is populated inwallet_checker.rswheremaintain_gap_limitis called on address pools. The results are extended into the field at lines 285, 296, and 309 respectively:Ok(new_addrs) => result.new_addresses.extend(new_addrs)This occurs for both standard BIP44 accounts (external and internal address pools) and other account types. The implementation is complete.
Likely an incorrect or invalid review comment.
key-wallet-manager/tests/spv_integration_tests.rs (2)
104-134: Well-structured test for BlockProcessingResult verification.The test properly validates:
relevant_txidscontains only wallet-relevant transactions (tx1, tx2)new_addressescount matches the actual increase in monitored addresses- New addresses are correctly included in the updated monitored set
The approach of capturing
addresses_beforeand comparing withaddresses_afterensures the gap-limit maintenance is working correctly.
136-155: Good negative test case for external-only transactions.This test correctly validates that when transactions only target external addresses (not monitored by the wallet), the
BlockProcessingResultreturns emptyrelevant_txidsandnew_addresses.key-wallet/src/transaction_checking/wallet_checker.rs (2)
78-78: Good: Result is now mutable to accumulate new addresses.Making
resultmutable enables the accumulation ofnew_addressesfrom gap limit maintenance across all address pools.
284-319: Proper error handling with structured logging for gap limit maintenance.The implementation correctly:
- Logs errors with account index, pool type, and error details for debugging
- Continues processing after errors (per earlier discussion)
- Accumulates successful new addresses into
result.new_addressesThe structured logging with
tracing::error!and named fields (account_index,pool_type,error) aligns well with observability best practices.key-wallet-manager/src/wallet_manager/process_block.rs (2)
16-44: Clean refactor to return BlockProcessingResult.The implementation correctly:
- Initializes
BlockProcessingResult::default()for accumulation- Pushes to
result.relevant_txidswhencheck_result.affected_walletsis non-empty- Extends
result.new_addresseswith newly generated addresses from each transaction check- Returns the complete result
This aligns with the PR objective to capture new addresses from gap-limit maintenance.
33-39: [No action needed; the code is correct.]The struct
CheckTransactionsResultinkey-wallet-manager/src/wallet_manager/mod.rs(lines 54-56) correctly defines the field aspub affected_wallets: Vec<WalletId>, which matches the usage inprocess_block.rsline 35. The review comment conflated this withTransactionCheckResultfromaccount_checker.rs, which is a different struct in a different module.Likely an incorrect or invalid review comment.
dash-spv/src/client/block_processor.rs (1)
223-264: Correctly adapted to BlockProcessingResult.The block processor properly:
- Receives
BlockProcessingResultfromwallet.process_block()- Uses
result.relevant_txids.is_empty()for the relevance check- Iterates over
&result.relevant_txidsfor transaction detection events- Passes
result.relevant_txids.len()to theBlockProcessedeventThe integration maintains the existing logging and event emission patterns while adapting to the new return type.
Capture the new addresses generated by the gap limit maintaining and pass them through to the
process_blockcaller in aBlockProcessingResult. Will be used in a later PR to rescan new addresses during sync.Tests currently still fail, requires:
maintain_gap_limittarget calculation off by one #286Summary by CodeRabbit
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.