chore: instalock validation cleanup

[ZocoLini]

Summary by CodeRabbit

• Refactor
  • Simplified the InstantLock validator's public surface: internal validation helpers were privatized and removed, leaving a single public validate entry point and enabling default construction for the validator.
• Tests
  • Updated tests to reflect the streamlined public API and removed helper-focused tests while keeping full validation coverage.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR reduces the public API surface of InstantLockValidator: adds derive(Default), removes the explicit Default impl, converts validate_structure and validate_signature to private, and removes public helpers is_still_valid and conflicts_with. The public validate entry point remains.

Changes

Cohort / File(s)	Summary
InstantLock validator API dash-spv/src/validation/instantlock.rs	Added #[derive(Default)] to InstantLockValidator and removed the explicit Default impl. Made validate_structure and validate_signature private (pub fn → fn). Removed public helper methods is_still_valid and conflicts_with and updated tests to reflect these removals.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐇 I nibble at the public trail,

Tuck helpers in a hidden vale.
Default stitched, but simpler made,
One validation gate displayed.
A tiny hop — the API trimmed, hooray!

Pre-merge checks and finishing touches

✅ 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 accurately summarizes the main change: refactoring InstantLockValidator to reduce public API surface by making internal methods private and simplifying the struct.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a4dd55 and 0c9a85b.

📒 Files selected for processing (1)

• dash-spv/src/validation/instantlock.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/validation/instantlock.rs

dash-spv/src/validation/**/*.rs

📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)

Implement three validation modes: ValidationMode::None (no validation), ValidationMode::Basic (structure and timestamp validation), and ValidationMode::Full (complete PoW and chain validation)

Files:

• dash-spv/src/validation/instantlock.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/validation/instantlock.rs

🧠 Learnings (3)

📓 Common learnings

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)

📚 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/validation/instantlock.rs

📚 Learning: 2025-02-27T05:44:42.338Z

Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 56
File: dash/src/sml/masternode_list_engine/message_request_verification.rs:98-99
Timestamp: 2025-02-27T05:44:42.338Z
Learning: In the Dash codebase, quorum selection for InstantLock verification uses a bit-shifting operation with (64 - n - 1) to extract n bits starting from the second-highest bit of a 64-bit selection hash. The exact reasoning for the extra "-1" isn't documented, but it matches the original C++ implementation for DIP-24 quorum selection.

Applied to files:

• dash-spv/src/validation/instantlock.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: fuzz (hashes_sha512_256)
• GitHub Check: fuzz (hashes_sha1)
• GitHub Check: fuzz (hashes_cbor)
• GitHub Check: fuzz (hashes_sha512)
• GitHub Check: fuzz (hashes_ripemd160)
• GitHub Check: fuzz (dash_deserialize_script)
• GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
• GitHub Check: fuzz (dash_deser_net_msg)
• GitHub Check: fuzz (dash_deserialize_block)
• GitHub Check: fuzz (hashes_json)
• GitHub Check: fuzz (hashes_sha256)
• GitHub Check: fuzz (dash_deserialize_amount)
• GitHub Check: fuzz (dash_deserialize_witness)
• GitHub Check: fuzz (dash_outpoint_string)
• GitHub Check: fuzz (dash_deserialize_address)
• GitHub Check: RPC Tests (stable, true)
• GitHub Check: Core Components Tests
• GitHub Check: SPV Components Tests
• GitHub Check: Pre-commit (macos-latest)
• GitHub Check: Pre-commit (ubuntu-latest)

🔇 Additional comments (4)

dash-spv/src/validation/instantlock.rs (4)

9-9: LGTM! Clean use of derive(Default).

Adding derive(Default) for the empty struct is idiomatic. The coexistence of new() (line 17) and Default::default() is a common Rust pattern and causes no issues.

---

9-116: Well-designed API cleanup that improves security posture.

The changes successfully reduce the public API surface while maintaining functionality:

• derive(Default) provides clean default construction
• Making validation helpers private enforces the security-critical requirement that callers must use full validation (structure + signature)
• The public validate entry point (line 30) remains the single point of entry for complete validation

This aligns well with the security warnings in the code (lines 27-29, 51-52) that emphasize never accepting InstantLocks without full signature verification.

---

53-53: Making validate_structure private is a safe breaking API change with no external dependents.

The method change from pub fn to private fn improves security by enforcing the use of the full validate method which includes signature verification. The search confirms no external code depends on InstantLockValidator::validate_structure outside of the instantlock module. This change correctly aligns with the security concern noted in the code comments and forces callers to perform comprehensive validation.

---

93-93: Making validate_signature private is safe design—no external code depends on this method.

The verification confirms that validate_signature has no external callers outside of instantlock.rs. The public API surface is properly narrowed, and external code using InstantLockValidator only calls the public validate() method in dash-spv/src/client/chainlock.rs.

---

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

dash-spv/src/validation/instantlock.rs (1)

48-53: Update stale documentation.

Line 52 states "This method is public only for testing purposes," but the method is now private (fn instead of pub fn). Update the documentation to reflect the current visibility.

🔎 Suggested documentation fix

-    /// **WARNING**: This is insufficient for accepting network messages.
-    /// For production use, always call `validate()` with a masternode engine.
-    /// This method is public only for testing purposes.
+    /// **WARNING**: This is insufficient for accepting network messages.
+    /// For production use, always call `validate()` with a masternode engine.
     fn validate_structure(&self, instant_lock: &InstantLock) -> ValidationResult<()> {

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42c5264 and 9a4dd55.

📒 Files selected for processing (1)

• dash-spv/src/validation/instantlock.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/validation/instantlock.rs

dash-spv/src/validation/**/*.rs

📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)

Implement three validation modes: ValidationMode::None (no validation), ValidationMode::Basic (structure and timestamp validation), and ValidationMode::Full (complete PoW and chain validation)

Files:

• dash-spv/src/validation/instantlock.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/validation/instantlock.rs

🧠 Learnings (4)

📓 Common learnings

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)

📚 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/validation/instantlock.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:

• dash-spv/src/validation/instantlock.rs

📚 Learning: 2025-02-27T05:44:42.338Z

Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 56
File: dash/src/sml/masternode_list_engine/message_request_verification.rs:98-99
Timestamp: 2025-02-27T05:44:42.338Z
Learning: In the Dash codebase, quorum selection for InstantLock verification uses a bit-shifting operation with (64 - n - 1) to extract n bits starting from the second-highest bit of a 64-bit selection hash. The exact reasoning for the extra "-1" isn't documented, but it matches the original C++ implementation for DIP-24 quorum selection.

Applied to files:

• dash-spv/src/validation/instantlock.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). (18)

• GitHub Check: Core Components Tests
• GitHub Check: fuzz (dash_deserialize_address)
• GitHub Check: SPV Components Tests
• GitHub Check: RPC Tests (stable, true)
• GitHub Check: fuzz (hashes_sha512)
• GitHub Check: fuzz (hashes_sha1)
• GitHub Check: fuzz (dash_deserialize_script)
• GitHub Check: fuzz (dash_deserialize_block)
• GitHub Check: fuzz (hashes_sha512_256)
• GitHub Check: fuzz (hashes_json)
• GitHub Check: fuzz (dash_deserialize_amount)
• GitHub Check: fuzz (hashes_ripemd160)
• GitHub Check: fuzz (hashes_cbor)
• GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
• GitHub Check: fuzz (dash_deserialize_witness)
• GitHub Check: fuzz (dash_deser_net_msg)
• GitHub Check: fuzz (dash_outpoint_string)
• GitHub Check: Pre-commit (macos-latest)

🔇 Additional comments (4)

dash-spv/src/validation/instantlock.rs (4)

8-18: LGTM: Clean stateless validator design.

The public new() constructor provides a clear instantiation point while the removal of the Default implementation (per PR summary) tightens the API surface. The stateless design with quorum manager passed as a parameter to methods avoids circular dependencies.

---

20-46: LGTM: Well-documented public validation entry point.

The validate() method remains the sole public interface for InstantLock validation, with clear security warnings about the requirement for BLS signature verification. The implementation properly chains structural and signature validation.

---

88-116: LGTM: Proper BLS signature verification with DIP 24 compliance.

The method correctly delegates to masternode_engine.verify_is_lock() for cyclehash-based quorum selection per DIP 24, with appropriate error handling and debug logging.

---

119-297: LGTM: Comprehensive test coverage for structural validation.

The test suite adequately covers structural validation edge cases (empty inputs, zero signatures, null txids) and request ID computation. Tests can access the now-private validate_structure method because they reside in the same module under #[cfg(test)].