fix: store checkpoints at checkpoint height not 0

[xdustinface]

Fix checkpoint sync after #292. Before that PR the checkpoint header was stored via ChainState::store_chain_state like below making use of sync_base_height:

// First store all headers
// For checkpoint sync, we need to store headers starting from the checkpoint height
self.store_headers_at_height(&state.headers, state.sync_base_height).await?;

After that PR its stored at height 0 via:

storage.store_headers(&[checkpoint_header]).await?;

So this PR fixes it by using store_headers_at_height again with the actual checkpoint height.

Summary by CodeRabbit

• Refactor
  • Enhanced checkpoint header storage system with height-aware tracking to improve blockchain synchronization reliability.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

The change updates the block header storage API in the lifecycle initialization to include height information. The method store_headers() is replaced with store_headers_at_height(), adding a height parameter to the call in initialize_genesis_block when using a checkpoint. No control flow or error handling is altered.

Changes

Cohort / File(s)	Summary
StorageManager API Enhancement dash-spv/src/client/lifecycle.rs	Updated checkpoint initialization to use height-aware header storage. Method call changed from store_headers(&[BlockHeader]) to store_headers_at_height(&[BlockHeader], height). Public API signature updated to require explicit height parameter.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A height-aware hop through storage's door,
Headers now know which block they're for!
One parameter added, one call refined—
Genesis blocks, so neatly aligned. ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: store checkpoints at checkpoint height not 0' directly and accurately describes the main change: restoring checkpoint headers to be stored at their correct height instead of height 0.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

dash-spv/src/client/lifecycle.rs (1)

388-388: Consider using store_headers_at_height for consistency.

For API consistency with the checkpoint path (line 327-329), consider using store_headers_at_height(&genesis_headers, 0) to make the height explicit. This would make both initialization paths use the same API pattern.

♻️ Proposed change for consistency

-            storage.store_headers(&genesis_headers).await.map_err(SpvError::Storage)?;
+            storage.store_headers_at_height(&genesis_headers, 0).await.map_err(SpvError::Storage)?;

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 189a66d and a068f4c.

📒 Files selected for processing (1)

• dash-spv/src/client/lifecycle.rs

🧰 Additional context used

📓 Path-based instructions (3)

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

• dash-spv/src/client/lifecycle.rs

🧠 Learnings (4)

📚 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/lifecycle.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/client/lifecycle.rs

📚 Learning: 2025-06-26T15:54:02.509Z

Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:54:02.509Z
Learning: The `StorageManager` trait in `dash-spv/src/storage/mod.rs` uses `&mut self` methods but is also `Send + Sync`, and implementations often use interior mutability for concurrency. This can be confusing, so explicit documentation should clarify thread-safety expectations and the rationale for the API design.

Applied to files:

• dash-spv/src/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 : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function

Applied to files:

• dash-spv/src/client/lifecycle.rs

⏰ 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: Ubuntu / wallet
• GitHub Check: macOS / tools
• GitHub Check: macOS / spv
• GitHub Check: fuzz (hashes_cbor)
• GitHub Check: fuzz (hashes_sha256)
• GitHub Check: fuzz (hashes_json)
• GitHub Check: fuzz (dash_deser_net_msg)
• GitHub Check: fuzz (hashes_sha512_256)
• GitHub Check: fuzz (hashes_ripemd160)
• GitHub Check: fuzz (dash_deserialize_block)
• GitHub Check: fuzz (dash_deserialize_witness)
• GitHub Check: fuzz (dash_deserialize_amount)
• GitHub Check: fuzz (dash_outpoint_string)
• GitHub Check: fuzz (dash_deserialize_address)
• GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
• GitHub Check: Pre-commit (ubuntu-latest)
• GitHub Check: Pre-commit (windows-latest)
• GitHub Check: Pre-commit (macos-latest)
• GitHub Check: Thread Sanitizer
• GitHub Check: Address Sanitizer

🔇 Additional comments (1)

dash-spv/src/client/lifecycle.rs (1)

327-329: Fix correctly stores checkpoint header at checkpoint height instead of height 0.

The change from store_headers to store_headers_at_height with explicit checkpoint.height parameter fixes the bug where checkpoint headers were stored at height 0. This ensures consistency with chain state initialization and sync manager updates, both of which use the same checkpoint height. The segmented storage implementation correctly handles arbitrary heights, including storing a single header at a checkpoint height.