fix: use RFC 5737 TEST-NET-1 IP for timeout testing #308

xdustinface · 2025-12-26T08:57:13Z

The test_handshake_timeout test is currently flaky because it sometimes obviously returns a network unreachable error or so instead of timing out which then leads to way to short duration.

 thread 'test_handshake_timeout' (193272) panicked at dash-spv/tests/handshake_test.rs:61:5:
 Should respect timeout duration (elapsed: 1.010833ms)

Summary by CodeRabbit

Tests
- Updated handshake timeout test to use a standards-compliant test IP address and improved assertion messaging for better test clarity and reliability.

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

coderabbitai · 2025-12-26T08:57:26Z

📝 Walkthrough

Walkthrough

Updated test IP address in handshake timeout test from 10.255.255.1:9999 to 192.0.2.1:9999 (RFC 5737 TEST-NET-1) and adjusted assertion messaging for clarity.

Changes

Cohort / File(s)	Summary
Test Configuration Update `dash-spv/tests/handshake_test.rs`	Replaced non-routable test IP with RFC 5737 compliant test address; updated assertion message from "non-routable peer" to "unreachable peer"

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A hop, a skip through test IPs we go,
From 10.255 to 192.0.2—oh!
RFC standards now bright and clear,
This rabbit approves, have no fear! 🌙

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 describes the main change: replacing a non-routable IP with an RFC 5737 TEST-NET-1 IP address to fix flaky timeout testing behavior.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/flaky-handshake-timeout-test

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3884314 and cb2e0bf.

📒 Files selected for processing (1)

dash-spv/tests/handshake_test.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/tests/handshake_test.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/handshake_test.rs

**/*.{rs,toml}

📄 CodeRabbit inference engine (CLAUDE.md)

Never hardcode network parameters, addresses, or keys

Files:

dash-spv/tests/handshake_test.rs

**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality
Use secure random number generation for keys
Never log or expose private keys
Code must target Rust 1.89 minimum supported version (MSRV)
Format code using cargo fmt
Pass clippy linting without warnings

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

Files:

dash-spv/tests/handshake_test.rs

**/tests/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/tests/**/*.rs: Create integration tests for network operations
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate

**/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/handshake_test.rs

**/{dash-network,dash-spv,key-wallet}/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Use async/await for async operations in network and wallet modules

Files:

dash-spv/tests/handshake_test.rs

🧠 Learnings (12)

📓 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/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

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

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

📚 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/tests/handshake_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 : Implement configurable timeouts with recovery mechanisms for network operations

Applied to files:

dash-spv/tests/handshake_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/tests/handshake_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/handshake_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/handshake_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: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)

Applied to files:

dash-spv/tests/handshake_test.rs

📚 Learning: 2025-12-22T17:59:37.849Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/tests/**/*.rs : Test both mainnet and testnet configurations

Applied to files:

dash-spv/tests/handshake_test.rs

📚 Learning: 2025-12-22T17:59:37.849Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/tests/**/*.rs : Create integration tests for network operations

Applied to files:

dash-spv/tests/handshake_test.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:

dash-spv/tests/handshake_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/**/*.rs : Use async/await throughout the codebase, built on tokio runtime

Applied to files:

dash-spv/tests/handshake_test.rs

📚 Learning: 2025-06-26T15:51:03.126Z

Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:51:03.126Z
Learning: In Rust network code, using unwrap() on SystemTime::now().duration_since(UNIX_EPOCH) can panic if the system clock is before UNIX_EPOCH. Use unwrap_or with a safe fallback (e.g., Duration::from_secs(0)) to prevent panics in edge cases.

Applied to files:

dash-spv/tests/handshake_test.rs

🧬 Code graph analysis (1)

dash-spv/tests/handshake_test.rs (1)

dash-spv/src/network/peer.rs (1)

connect (81-125)

⏰ 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). (19)

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

🔇 Additional comments (2)

dash-spv/tests/handshake_test.rs (2)

53-56: Excellent fix using RFC 5737 TEST-NET-1 for reliable timeout testing.

The change from a private IP (10.255.255.1) to TEST-NET-1 (192.0.2.1) is the correct approach to eliminate flakiness. RFC 5737 documentation ranges are guaranteed to be non-routable, ensuring consistent timeout behavior rather than immediate "network unreachable" failures.

61-61: Improved assertion message clarity.

The updated message "Connection should fail for unreachable peer" more accurately describes the test scenario and is clearer than "non-routable peer".

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

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

fix: use RFC 5737 TEST-NET-1 IP for timeout testing

cb2e0bf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: use RFC 5737 TEST-NET-1 IP for timeout testing #308

fix: use RFC 5737 TEST-NET-1 IP for timeout testing #308

Uh oh!

xdustinface commented Dec 26, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 26, 2025 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: use RFC 5737 TEST-NET-1 IP for timeout testing #308

Are you sure you want to change the base?

fix: use RFC 5737 TEST-NET-1 IP for timeout testing #308

Uh oh!

Conversation

xdustinface commented Dec 26, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xdustinface commented Dec 26, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 26, 2025 •

edited

Loading