fix: update wallet heights when processing blocks

[xdustinface]

We currently don't update the wallet heights when processing blocks. This PR fixes it and adds a test to ensure the updates happen.

Based on:

• feat: track synced_height per ManagedWalletInfo #305

Summary by CodeRabbit

• Refactor

  • Improved internal height update mechanism by using a dedicated method call.

• Tests

  • Added integration test to verify wallet height synchronization after block processing at multiple heights.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request refactors the wallet manager's internal height update mechanism to use a dedicated method call instead of direct field assignment, and adds integration tests to verify the height-tracking functionality works correctly during block processing.

Changes

Cohort / File(s)	Summary
Height Update Refactoring key-wallet-manager/src/wallet_manager/process_block.rs	Replaces direct field assignment self.current_height = height with method call self.update_height(height) for managing internal height state.
Integration Tests key-wallet-manager/tests/spv_integration_tests.rs	Adds WalletInfoInterface import and introduces assert_wallet_heights helper function to verify manager and wallet heights. Adds new test test_height_updated_after_block_processing that validates height updates when processing blocks at sequential heights (1000, 2000, 3000).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A method call replaces the assignment line,
Heights now update in a way more divine,
Tests hop through blocks at 1000, 2000, 3000's chime,
Verifying each wallet keeps perfect time! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: updating wallet heights during block processing, which is directly reflected in the code modifications.

✨ Finishing touches

• 📝 Generate docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

key-wallet-manager/tests/spv_integration_tests.rs (1)

165-183: Consider adding edge case tests for height updates.

The test correctly verifies the happy path for height updates. However, consider adding tests for:

• Processing blocks out of order (e.g., height 1000, then 500) - does it handle or reject?
• Processing the same height twice - is it idempotent?
• Multiple wallets - ensures all wallets are updated, not just one
• Rollback scenarios if supported

These edge cases would strengthen confidence in the height-tracking mechanism.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd12793 and 7f62404.

📒 Files selected for processing (2)

• key-wallet-manager/src/wallet_manager/process_block.rs
• key-wallet-manager/tests/spv_integration_tests.rs

🧰 Additional context used

📓 Path-based instructions (4)

**/*.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 the tests/ directory
Use snake_case for function and variable names
Use UpperCamelCase for types and traits
Use SCREAMING_SNAKE_CASE for constants
Format code with rustfmt before commits; ensure cargo fmt --all is run
Run cargo clippy --workspace --all-targets -- -D warnings for linting; avoid warnings in CI
Prefer async/await via tokio for 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/wallet_manager/process_block.rs
• key-wallet-manager/tests/spv_integration_tests.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

🧠 Learnings (11)

📓 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-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: 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`

📚 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.rs
• 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 : Separate immutable structures (`Account`, `Wallet`) containing only identity information from mutable wrappers (`ManagedAccount`, `ManagedWalletInfo`) with state management

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-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/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-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.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.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-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/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 : 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

🧬 Code graph analysis (1)

key-wallet-manager/tests/spv_integration_tests.rs (3)

key-wallet-manager/src/wallet_manager/mod.rs (2)

• current_height (934-936)
• new (70-77)

key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)

• synced_height (65-65)
• synced_height (162-164)

key-wallet/src/wallet/managed_wallet_info/mod.rs (1)

• new (55-66)

🔇 Additional comments (3)

key-wallet-manager/tests/spv_integration_tests.rs (2)

13-13: LGTM!

The import is correctly added to support the new test functionality.

---

153-163: LGTM!

The helper function correctly validates that both the manager's height and all individual wallet synced heights are updated consistently.

key-wallet-manager/src/wallet_manager/process_block.rs (1)

40-40: Encapsulation improved with method call for height updates.

The update_height method correctly updates both self.current_height and the synced_height for all managed wallets by iterating through wallet_infos and calling update_synced_height on each one. Heights are synchronized atomically as part of the single method call.