fix: reject empty hostname in peer address in FFI #318
Conversation
📝 WalkthroughWalkthroughdash_spv_ffi_config_add_peer now accepts IP or hostname with or without port, applies the network default port when omitted, rejects empty or colon-leading host/port cases, constructs an address-with-port string when needed, and resolves hostnames via DNS before returning errors on resolution failures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)dash-spv-ffi/src/**/*.rs📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
Files:
**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (11)📓 Common learnings📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
🧬 Code graph analysis (1)dash-spv-ffi/src/config.rs (1)
⏰ 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)
🔇 Additional comments (3)
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-ffi/src/config.rs (1)
174-188: Validation logic correctly addresses the Windows DNS bug.The enhanced validation using
split_once(':')properly rejects empty hostnames (e.g.,:9999), missing ports, and missing colons before attempting DNS resolution. The specific error messages are clear and actionable.Minor suggestion: The comment on line 174 says "Attempt DNS resolution" but the code block performs validation. Consider updating to "Validate hostname:port format before DNS resolution" for clarity.
🔎 Suggested comment clarification
- // 3) Attempt DNS resolution for hostnames with explicit port + // 3) Validate hostname:port format before DNS resolution match addr_str.split_once(':') {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dash-spv-ffi/src/config.rsdash-spv-ffi/tests/unit/test_configuration.rs
🧰 Additional context used
📓 Path-based instructions (6)
dash-spv-ffi/tests/unit/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
Add corresponding unit tests in
tests/unit/for each new FFI function
Files:
dash-spv-ffi/tests/unit/test_configuration.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode network parameters, addresses, or keys
Files:
dash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/src/config.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 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
Files:
dash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/src/config.rs
**/*-ffi/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*-ffi/**/*.rs: Exercise careful handling at FFI boundaries for memory safety
Be careful with FFI memory management
Files:
dash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/src/config.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-ffi/tests/unit/test_configuration.rs
dash-spv-ffi/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/src/**/*.rs: Use#[no_mangle] extern "C"attribute when implementing new FFI functions in Rust
All FFI types must have corresponding_destroy()functions for explicit memory management
Rust strings must be returned as*const c_charwith caller responsibility to free usingdash_string_free
Input strings in FFI functions are*const c_char(borrowed, not freed by C caller)
Add cbindgen annotations for complex types in FFI functions
Use thread-local storage for error propagation viadash_spv_ffi_get_last_error()function
Files:
dash-spv-ffi/src/config.rs
🧠 Learnings (18)
📓 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/tests/**/*.rs : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
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`
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
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: When debugging FFI issues, check `dash_spv_ffi_get_last_error()` for error details
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: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.{c,h} : Add corresponding C tests in `tests/c_tests/` for each new FFI function
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: 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
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`)
📚 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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.rsdash-spv-ffi/src/config.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.rsdash-spv-ffi/src/config.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-ffi/tests/unit/test_configuration.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 : Add cbindgen annotations for complex types in FFI functions
Applied to files:
dash-spv-ffi/tests/unit/test_configuration.rsdash-spv-ffi/src/config.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/c_tests/**/*.{c,h} : Add corresponding C tests in `tests/c_tests/` for each new FFI function
Applied to files:
dash-spv-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.rsdash-spv-ffi/src/config.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-ffi/tests/unit/test_configuration.rsdash-spv-ffi/src/config.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-ffi/src/config.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-ffi/src/config.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 : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)
Applied to files:
dash-spv-ffi/src/config.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-ffi/src/config.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-ffi/src/config.rs
🧬 Code graph analysis (2)
dash-spv-ffi/tests/unit/test_configuration.rs (1)
dash-spv-ffi/src/config.rs (1)
dash_spv_ffi_config_add_peer(137-207)
dash-spv-ffi/src/config.rs (1)
dash-spv-ffi/src/error.rs (1)
set_last_error(26-31)
⏰ 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_ripemd160)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Key Wallet Components Tests
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: SPV Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (2)
dash-spv-ffi/tests/unit/test_configuration.rs (2)
56-74: Excellent test coverage for the enhanced validation.The updated test cases comprehensively cover the new validation paths:
:9999validates the empty hostname checkhostname-without-portvalidates the missing colon checklocalhost:abcvalidates DNS resolution error handling for invalid portsThe improved assertion message with the specific address that failed enhances debugging clarity.
89-89: Good addition of hostname:port test case.Adding
localhost:9999ensures the validation logic correctly accepts valid hostname:port combinations and successfully resolves them via DNS.
xdustinface
force-pushed
the
fix/windows-dns
branch
from
6bc762e to
be8abf1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
dash-spv-ffi/tests/unit/test_configuration.rs (1)
87-89: LGTM! Valid address coverage expanded appropriately.The additions correctly test:
- IP-only forms that receive default ports based on network configuration
- Hostname with explicit port that triggers DNS resolution
Optional: Consider verifying default port behavior
While the test confirms that bare IP addresses are accepted, it doesn't verify that they actually receive the correct default port (19999 for testnet). You could enhance the test by introspecting the config's peer list to confirm the port assignment, though this may require additional FFI getter functions.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dash-spv-ffi/src/config.rsdash-spv-ffi/tests/unit/test_configuration.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- dash-spv-ffi/src/config.rs
🧰 Additional context used
📓 Path-based instructions (5)
dash-spv-ffi/tests/unit/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
Add corresponding unit tests in
tests/unit/for each new FFI function
Files:
dash-spv-ffi/tests/unit/test_configuration.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode network parameters, addresses, or keys
Files:
dash-spv-ffi/tests/unit/test_configuration.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 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
Files:
dash-spv-ffi/tests/unit/test_configuration.rs
**/*-ffi/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*-ffi/**/*.rs: Exercise careful handling at FFI boundaries for memory safety
Be careful with FFI memory management
Files:
dash-spv-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.rs
🧠 Learnings (11)
📓 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-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: When debugging FFI issues, check `dash_spv_ffi_get_last_error()` for error details
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: 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
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
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
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`
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/c_tests/**/*.{c,h} : Add corresponding C tests in `tests/c_tests/` for each new FFI function
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
📚 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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.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 : Add cbindgen annotations for complex types in FFI functions
Applied to files:
dash-spv-ffi/tests/unit/test_configuration.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-ffi/tests/unit/test_configuration.rs
🧬 Code graph analysis (1)
dash-spv-ffi/tests/unit/test_configuration.rs (1)
dash-spv-ffi/src/config.rs (1)
dash_spv_ffi_config_add_peer(137-207)
⏰ 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: Key Wallet Components Tests
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: Pre-commit (ubuntu-latest)
- GitHub Check: Pre-commit (macos-latest)
🔇 Additional comments (2)
dash-spv-ffi/tests/unit/test_configuration.rs (2)
57-64: LGTM! Test cases align with enhanced validation logic.The updated invalid address test cases correctly reflect the stricter host:port validation from
config.rs. Key improvements:
- Line 61 (
:9999) specifically tests the Windows DNS bug fix mentioned in the PR description- "hostname-without-port" validates that hostnames now require explicit ports
- Cases cover empty hostname, invalid ports, and malformed addresses
69-74: LGTM! Enhanced error reporting for test failures.The assertion now includes the specific address that failed validation, making test failures more actionable and easier to debug.
ZocoLini
left a comment
ZocoLini
left a comment
There was a problem hiding this comment.
A couple of things I would like to ask:
-
Why
hostname-without-portis invalid?? If we have default ports and we are able to work with Ips without port, we should also be able to work with hostnames without ports. -
Why don't we just append the port if missing to the string and then rely on
to_socket_addrs. It already tries to use parse:: and, if that fails, it triesrsplit_once(':')on the string
Fix a bug in Windows discovered in CI overhaul PR #253 where `:9999` was being accepted as a valid peer address on Windows due to DNS resolution differences.
xdustinface
force-pushed
the
fix/windows-dns
branch
from
be8abf1 to
c669d1a
Compare
Fix a bug in Windows discovered in CI overhaul PR #253 where
:9999was being accepted as a valid peer address on Windows due to DNS resolution differences.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.