chore: drop redundant block processing #349
Conversation
📝 WalkthroughWalkthroughThe PR removes the entire block processing subsystem from the Dash SPV client, including the BlockProcessor worker, BlockProcessingTask enum, and all associated block processing orchestration logic that handled block, transaction, and compact filter task queuing across multiple components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
dash-spv/src/client/message_handler_test.rs (1)
1-8: Tests disabled pending architecture rewrite.The entire test module is commented out. While the import on line 8 correctly removes
BlockProcessingTask, consider tracking the re-enablement of these tests with a TODO issue to ensure they don't remain disabled indefinitely after the planned sync rewrite.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
TEST_SUMMARY.mddash-spv/ARCHITECTURE.mddash-spv/src/client/block_processor.rsdash-spv/src/client/block_processor_test.rsdash-spv/src/client/core.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/message_handler.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/mod.rsdash-spv/src/client/sync_coordinator.rsdash-spv/tests/transaction_calculation_test.rs
💤 Files with no reviewable changes (6)
- dash-spv/src/client/mod.rs
- TEST_SUMMARY.md
- dash-spv/src/client/message_handler.rs
- dash-spv/src/client/block_processor_test.rs
- dash-spv/ARCHITECTURE.md
- dash-spv/src/client/block_processor.rs
🧰 Additional context used
📓 Path-based instructions (6)
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/sync_coordinator.rsdash-spv/tests/transaction_calculation_test.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/core.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/sync_coordinator.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/core.rs
**/*.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:
dash-spv/src/client/sync_coordinator.rsdash-spv/tests/transaction_calculation_test.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/core.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
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
Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Files:
dash-spv/tests/transaction_calculation_test.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:
dash-spv/tests/transaction_calculation_test.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:
dash-spv/tests/transaction_calculation_test.rsdash-spv/src/client/message_handler_test.rs
🧠 Learnings (24)
📓 Common learnings
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Applied to files:
dash-spv/src/client/sync_coordinator.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/core.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/sync_coordinator.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/core.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/sync_coordinator.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/core.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Applied to files:
dash-spv/src/client/sync_coordinator.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/core.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/sync_coordinator.rsdash-spv/src/client/core.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/sync_coordinator.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/client/**/*.rs : Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization
Applied to files:
dash-spv/src/client/sync_coordinator.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/core.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: Follow a layered, trait-based architecture with clear separation of concerns across modules: client/, network/, storage/, sync/, validation/, wallet/, with trait-based abstractions for swappable implementations
Applied to files:
dash-spv/src/client/sync_coordinator.rsdash-spv/src/client/core.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/sync_coordinator.rsdash-spv/src/client/core.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Implement three validation modes: ValidationMode::None (no validation), ValidationMode::Basic (structure and timestamp validation), and ValidationMode::Full (complete PoW and chain validation)
Applied to files:
dash-spv/tests/transaction_calculation_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:
dash-spv/tests/transaction_calculation_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:
dash-spv/tests/transaction_calculation_test.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/client/core.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:
dash-spv/tests/transaction_calculation_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:
dash-spv/src/client/message_handler_test.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*integration*.rs : Write integration tests for network operations
Applied to files:
dash-spv/src/client/message_handler_test.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*test*.rs : Test both mainnet and testnet configurations
Applied to files:
dash-spv/src/client/message_handler_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:
dash-spv/src/client/message_handler_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 : Use DNS-first peer discovery with automatic DNS seeds (dnsseed.dash.org, testnet-seed.dashdot.io) when no explicit peers are configured; implement immediate startup with 10-second delay only for subsequent peer searches
Applied to files:
dash-spv/src/client/lifecycle.rsdash-spv/src/client/core.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 : Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Applied to files:
dash-spv/src/client/lifecycle.rsdash-spv/src/client/core.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv/src/client/lifecycle.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Rust strings must be returned as `*const c_char` with caller responsibility to free using `dash_string_free`
Applied to files:
dash-spv/src/client/core.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Applied to files:
dash-spv/src/client/core.rs
📚 Learning: 2025-11-27T10:30:54.015Z
Learnt from: xdustinface
Repo: dashpay/rust-dashcore PR: 214
File: dash-spv/examples/filter_sync.rs:48-51
Timestamp: 2025-11-27T10:30:54.015Z
Learning: The DashSpvClient::run method in dash-spv internally handles Ctrl-C shutdown by spawning a task that listens for tokio::signal::ctrl_c() and cancels the provided CancellationToken. Examples and callers should create a CancellationToken without explicitly cancelling it, as the cancellation is managed internally by the run method.
Applied to files:
dash-spv/src/client/core.rs
🔇 Additional comments (5)
dash-spv/tests/transaction_calculation_test.rs (1)
35-35: LGTM!Comment correctly updated to remove the now-deleted
block_processor.rsreference, consistent with the PR's removal of the block processing subsystem.dash-spv/src/client/core.rs (1)
27-27: LGTM!Import correctly updated to remove
BlockProcessingTaskwhich is no longer exposed from the module. The struct definition below properly excludes the removedblock_processor_txfield.dash-spv/src/client/lifecycle.rs (1)
26-26: LGTM!Import correctly updated to remove
BlockProcessor. The constructor below properly excludes creating theblock_processor_txchannel and spawning theBlockProcessorworker, aligning with the PR's intent to rely on inline block processing during sync phases.dash-spv/src/client/sync_coordinator.rs (2)
14-14: LGTM!Import correctly updated to remove
BlockProcessingTask. The change aligns with the removal of the background block processor channel throughout the codebase.
601-609: LGTM!
MessageHandler::new()constructor call correctly updated to exclude the removedblock_processor_txparameter. The handler retains all other necessary dependencies for network message processing.
ZocoLini
left a comment
ZocoLini
left a comment
There was a problem hiding this comment.
feel free to merge this
Blocks are already processed in the downloading blocks sync phase. And even though it anyway doesn't work properly in the current sync implementation this block processor adds a another layer of redundant block processing. Which to be fair might be a bid of a band aid if we request blocks while we are still downloading filters but thats imo not how its intended to work and im coming up with a sync rewrite PR soon which will leave this behind anyway.
Summary by CodeRabbit
Release Notes
Removed Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.