removed filters field from ChainState #293
Conversation
WalkthroughThis PR removes fork detection, chain reorganization, filter header tracking, and synchronous storage abstraction from the Dash SPV library. Multiple modules and public APIs are deleted, including ForkDetector, ReorgManager, ChainStorage trait, and filter header persistence logic. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (13)
💤 Files with no reviewable changes (11)
🧰 Additional context used📓 Path-based instructions (3)**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
dash-spv/**/*.rs📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Files:
dash-spv/src/sync/**/*.rs📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Files:
🧠 Learnings (9)📓 Common learnings📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-06-26T16:01:37.609ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-01T08:00:50.618ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-01T08:01:18.174ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
🔇 Additional comments (2)
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 |
ZocoLini
force-pushed
the
remove-filters-from-chain-state
branch
from
2ba0b64 to
b492b93
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
dash-spv/src/sync/headers/manager.rs (2)
284-286: Remove misleading "temporarily disabled" comment about fork detection.The comment states fork detection is "temporarily disabled" for batch processing, but the
ForkDetectorinfrastructure was permanently removed in commit 4be4cef. Fork detection is now handled throughChainTipManager, which tracks multiple chain tips. Remove this comment or update it to clarify the architectural decision.
23-62: Fork detection mechanism exists but is acknowledged as incomplete for batch processing.The
ForkDetectorwas replaced withChainTipManagerandChainWork(line 10, 277-280). TheChainTipManagerimplements work-based fork selection viaupdate_active_tip()(selecting tips bychain_work), andadd_tip()is called after processing header batches.However, there are two critical limitations:
Only the last header per batch is tracked: Line 280 calls
add_tip()only with the final header in each batch, meaning intermediate headers and competing chains are not tracked during synchronous batch processing.Fork detection explicitly disabled for batch mode: The code comment at line 285 states: "Fork detection is temporarily disabled for batch processing. In a production implementation, we would need to handle fork detection at the batch level or in a separate phase."
The basic connectivity check at line 174 (
if first_header.prev_blockhash != tip_hash) validates headers connect to the current tip but does not perform fork selection by work. This means the system cannot detect when a longer/heavier chain arrives in a different batch and needs to reorganize.Integrate fork detection into the batch processing loop to track all header arrivals, not just the final one, and update the active chain tip based on cumulative work comparisons.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
dash-spv-ffi/src/types.rs(0 hunks)dash-spv-ffi/tests/unit/test_type_conversions.rs(0 hunks)dash-spv/src/chain/fork_detector.rs(0 hunks)dash-spv/src/chain/fork_detector_test.rs(0 hunks)dash-spv/src/chain/mod.rs(0 hunks)dash-spv/src/chain/reorg.rs(0 hunks)dash-spv/src/chain/reorg_test.rs(0 hunks)dash-spv/src/client/core.rs(0 hunks)dash-spv/src/storage/mod.rs(0 hunks)dash-spv/src/storage/state.rs(0 hunks)dash-spv/src/storage/sync_storage.rs(0 hunks)dash-spv/src/sync/headers/manager.rs(1 hunks)dash-spv/src/types.rs(1 hunks)
💤 Files with no reviewable changes (11)
- dash-spv/src/storage/state.rs
- dash-spv/src/client/core.rs
- dash-spv-ffi/src/types.rs
- dash-spv-ffi/tests/unit/test_type_conversions.rs
- dash-spv/src/chain/reorg_test.rs
- dash-spv/src/storage/mod.rs
- dash-spv/src/storage/sync_storage.rs
- dash-spv/src/chain/fork_detector_test.rs
- dash-spv/src/chain/reorg.rs
- dash-spv/src/chain/fork_detector.rs
- dash-spv/src/chain/mod.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code
**/*.rs: Each crate keeps sources insrc/; unit tests live alongside code with#[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code withrustfmt(seerustfmt.toml); runcargo fmt --allbefore commits
Lint Rust code withclippy; avoidunwrap()/expect()in library code; use error types (e.g.,thiserror)
Usesnake_casefor function and variable names in Rust
UseUpperCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async viatokiowhere applicable
Unit tests should be placed near code with descriptive names (e.g.,test_parse_address_mainnet)
Use#[ignore]for network-dependent or long-running tests; run with-- --ignoredflag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code
Files:
dash-spv/src/types.rsdash-spv/src/sync/headers/manager.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/types.rsdash-spv/src/sync/headers/manager.rs
dash-spv/src/sync/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Files:
dash-spv/src/sync/headers/manager.rs
🧠 Learnings (9)
📓 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: 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
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
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-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
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
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)
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Prepare support for future Dash upgrades including Schnorr/Taproot support, descriptor wallets, multi-signature account types, and Lightning Network payment channels
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
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/**/*.swift : Use actors for state management in SPVClient to ensure thread safety
📚 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/types.rsdash-spv/src/sync/headers/manager.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:
dash-spv/src/types.rsdash-spv/src/sync/headers/manager.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/types.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Implement three validation modes: ValidationMode::None (no validation), ValidationMode::Basic (structure and timestamp validation), and ValidationMode::Full (complete PoW and chain validation)
Applied to files:
dash-spv/src/types.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/types.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Applied to files:
dash-spv/src/types.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/sync/headers/manager.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/sync/headers/manager.rs
🔇 Additional comments (2)
dash-spv/src/sync/headers/manager.rs (1)
10-10: LGTM - Import updated correctly.The removal of
ForkDetectorfrom imports aligns with the broader removal of fork detection infrastructure in this PR.dash-spv/src/types.rs (1)
113-119: Verify the purpose of remaining filter-related fields.The PR removes the
filter_headersVec fromChainState, butSyncProgress.filter_header_height(line 119) andChainState.current_filter_tip(line 266) remain. Clarify whether:
- These fields are orphaned and should also be removed
- They serve an active purpose in the current sync architecture
- The removal is intentionally incomplete (e.g., deferred to a later phase)
Per the sequential sync phases (Headers → Masternode List → Filter Headers → Filters → Blocks), filter tracking appears to be part of the design, so removing the Vec while keeping height/tip fields suggests incomplete cleanup or a misalignment with the sync flow.
This was easy, the field was not being used xd
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.