feat: update version to 0.40.0 and implement user agent configuration #133
Conversation
|
Warning Rate limit exceeded@QuantumExplorer has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughImplements configurable user agent propagation from configuration (including Swift SDK via FFI) into dash-spv handshake logic. Normalizes and caps user agent in VersionMessage. Widely switches crates to workspace-managed versions and bumps workspace to 0.40.0. Expands key-wallet-ffi public C header with extensive derivation and wallet-management APIs. Updates docs and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant SwiftSDK as Swift SDK
participant FFI as dash_spv_ffi (C)
participant RustCfg as dash-spv-ffi::config
participant SPV as dash-spv::ClientConfig
App->>SwiftSDK: set userAgent
SwiftSDK->>FFI: dash_spv_ffi_config_set_user_agent(config, userAgent)
FFI->>RustCfg: config_set_user_agent(config*, const char*)
RustCfg-->>FFI: Result (Success / InvalidArgument)
FFI-->>SwiftSDK: status
SwiftSDK-->>App: propagate error if any
note over RustCfg,SPV: Stores user_agent in ClientConfig (Option<String>)
sequenceDiagram
autonumber
participant NetMgr as Tcp/MultiPeer NetworkManager
participant HS as HandshakeManager
participant Peer as Remote Peer
NetMgr->>HS: new(network, mempool_strategy, user_agent)
HS->>HS: compute UA (configured or default "/rust-dash-spv:<ver>/")
HS->>HS: normalize (ensure leading/trailing '/')\ntruncate to 256 chars
HS->>Peer: send VersionMessage{ user_agent: UA, ... }
Peer-->>HS: VerAck / response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
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/network/handshake.rs (1)
69-69: Incorrect import of Duration from tokio::time.tokio::time doesn’t export Duration; this will fail/clippy. Use std::time::Duration (already in scope) and only import timeout.
Apply:
- use tokio::time::{timeout, Duration}; + use tokio::time::timeout;dash-spv/src/network/multi_peer.rs (1)
1252-1260: Infinite recursion: trait impl get_last_message_peer_id calls itself.This awaits itself forever. Disambiguate to call the inherent method.
Apply:
- async fn get_last_message_peer_id(&self) -> crate::types::PeerId { - // Call the instance method to avoid code duplication - self.get_last_message_peer_id().await - } + async fn get_last_message_peer_id(&self) -> crate::types::PeerId { + // Call inherent method explicitly to avoid recursion + MultiPeerNetworkManager::get_last_message_peer_id(self).await + }
🧹 Nitpick comments (11)
Cargo.toml (1)
5-6: Addrust-version = "1.89"to[workspace.package]in Cargo.toml
Ensures the MSRV propagates to all member crates (currently missing).[workspace.package] version = "0.40.0" +rust-version = "1.89"key-wallet-ffi/include/key_wallet_ffi.h (1)
1773-1778: Consider returning an empty array instead of NotFound for empty ranges.Ergonomics: an empty result for a valid range reduces special-casing in SDKs.
Proposed Rust tweak (key-wallet-ffi/src/address_pool.rs):
- if infos.is_empty() { - FFIError::set_error( - error, - FFIErrorCode::NotFound, - "No addresses found in the specified range".to_string(), - ); - return std::ptr::null_mut(); - } + if infos.is_empty() { + *count_out = 0; + FFIError::set_success(error); + // Return a valid empty array for callers to free consistently + let empty: Box<[*mut FFIAddressInfo]> = Vec::new().into_boxed_slice(); + return Box::into_raw(empty) as *mut *mut FFIAddressInfo; + }dash-spv-ffi/include/dash_spv_ffi.h (1)
521-529: Clarify UTF-8 requirement and handshake normalization/capping in the docstring.Callers need to know the string must be valid UTF-8 and that normalization/truncation occurs at handshake time. Recommend expanding the comment.
-/** - * Sets the user agent string to advertise in the P2P handshake +/** + * Sets the user agent string to advertise in the P2P handshake. + * + * Notes: + * - The input must be valid UTF-8; non‑UTF‑8 will return InvalidArgument and set the last error. + * - The library normalizes this value during handshake (ensures leading/trailing '/' and + * truncates to a protocol‑safe maximum, currently 256 bytes).Also, please confirm the generated header is up to date and synced to the Swift SDK:
- Rebuild the FFI crate to regenerate headers.
- Run the sync script to update swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h.
dash-spv/src/client/config.rs (2)
72-75: Document handshake-side normalization and length limit.Minor doc tweak to set expectations for integrators.
- /// Optional user agent string to advertise in the P2P version message. - /// If not set, a sensible default is used (includes crate version). + /// Optional user agent string to advertise in the P2P version message. + /// If not set, a sensible default is used (includes crate version). + /// Normalization (leading/trailing slash) and truncation to 256 bytes occur during handshake.
314-319: Trim input and avoid storing empty or whitespace-only values; optionally pre-validate length.Pre-sanitizing here prevents surprising empty/oversized values from propagating.
- pub fn with_user_agent(mut self, agent: impl Into<String>) -> Self { - self.user_agent = Some(agent.into()); + pub fn with_user_agent(mut self, agent: impl Into<String>) -> Self { + let ua = agent.into().trim().to_string(); + self.user_agent = if ua.is_empty() { None } else { Some(ua) }; self }Optionally, add a light check in validate():
@@ pub fn validate(&self) -> Result<(), String> { @@ - Ok(()) + if let Some(ua) = &self.user_agent { + if ua.is_empty() { + return Err("user_agent must not be empty when set".into()); + } + if ua.len() > 256 { + return Err("user_agent must be <= 256 bytes".into()); + } + } + Ok(()) }Finally, if FFI layout relies on this type, confirm no external C/Swift callers construct it directly; all construction should go through FFI helpers to avoid ABI/representation pitfalls with Option.
dash-spv/tests/test_handshake_logic.rs (1)
7-17: Add a UA propagation test.Please add a test that sets a custom UA and asserts it’s used (and normalized/capped) in the Version message built by the handshake.
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (1)
390-399: Document normalization/length-capping and verify header sync.Consider noting that the UA is normalized and length-capped during handshake, and re-sync this header from cbindgen if it was edited manually.
Apply this doc tweak:
-/** - * Sets the user agent string to advertise in the P2P handshake +/** + * Sets the user agent string to advertise in the P2P handshake. + * Note: The SPV client normalizes and length-caps the UA during handshake.swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClientConfiguration.swift (1)
87-92: Sanitize and cap UA before passing over FFI.Trim whitespace and cap length to avoid surprises; handshake also caps, but pre-validating here improves UX.
Apply:
- // Configure user agent advertised during P2P handshake - result = FFIBridge.withCString(userAgent) { agent in + // Configure user agent advertised during P2P handshake + let sanitizedUA = userAgent.trimmingCharacters(in: .whitespacesAndNewlines) + let uaToSend = String(sanitizedUA.prefix(256)) + result = FFIBridge.withCString(uaToSend) { agent in dash_spv_ffi_config_set_user_agent(config, agent) } try FFIBridge.checkError(result)dash-spv-ffi/src/config.rs (1)
149-176: Validate non-empty UA at the FFI boundary.Reject empty/whitespace-only strings to prevent ambiguous advertisement like “/” or falling back unintentionally.
Apply:
pub unsafe extern "C" fn dash_spv_ffi_config_set_user_agent( config: *mut FFIClientConfig, user_agent: *const c_char, ) -> i32 { null_check!(config); null_check!(user_agent); // Validate the user_agent string match CStr::from_ptr(user_agent).to_str() { - Ok(agent_str) => { - // Store as-is; normalization/length capping is applied at handshake build time - let cfg = &mut (*config).inner; - cfg.user_agent = Some(agent_str.to_string()); - FFIErrorCode::Success as i32 - } + Ok(agent_str) => { + let trimmed = agent_str.trim(); + if trimmed.is_empty() { + set_last_error("User agent cannot be empty"); + return FFIErrorCode::InvalidArgument as i32; + } + // Store as-is; normalization/length capping is applied at handshake build time + let cfg = &mut (*config).inner; + cfg.user_agent = Some(trimmed.to_string()); + FFIErrorCode::Success as i32 + } Err(e) => { set_last_error(&format!("Invalid UTF-8 in user agent: {}", e)); FFIErrorCode::InvalidArgument as i32 } } }dash-spv-ffi/FFI_API.md (1)
558-561: Document normalization and limits for user_agent.To set expectations for FFI callers, please note in the Description/Safety:
- UA is normalized to start/end with “/”.
- Non-ASCII/control chars are stripped (or rejected) and length is capped at 256 bytes at handshake time; longer inputs are truncated.
- NUL bytes are not allowed.
Can you confirm dash-spv-ffi/include/dash_spv_ffi.h was regenerated and carries the same wording? (Recall: the public C header is generated and must reflect FFI changes.)
dash-spv/src/network/multi_peer.rs (1)
208-222: Forward UA into handshake — LGTM; consider minor readability tweak.Passing cloned Option is fine. Optional: inline HandshakeManager::new on one line to reduce wrapping.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
Cargo.toml(1 hunks)dash-network-ffi/Cargo.toml(1 hunks)dash-network/README.md(1 hunks)dash-spv-ffi/Cargo.toml(1 hunks)dash-spv-ffi/FFI_API.md(2 hunks)dash-spv-ffi/include/dash_spv_ffi.h(1 hunks)dash-spv-ffi/src/config.rs(2 hunks)dash-spv/Cargo.toml(1 hunks)dash-spv/src/client/config.rs(3 hunks)dash-spv/src/network/handshake.rs(3 hunks)dash-spv/src/network/mod.rs(1 hunks)dash-spv/src/network/multi_peer.rs(5 hunks)dash-spv/src/network/tests.rs(1 hunks)dash-spv/tests/test_handshake_logic.rs(1 hunks)key-wallet-ffi/Cargo.toml(1 hunks)key-wallet-ffi/include/key_wallet_ffi.h(1 hunks)key-wallet-manager/Cargo.toml(1 hunks)key-wallet/Cargo.toml(1 hunks)swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h(1 hunks)swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClientConfiguration.swift(1 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
**/Cargo.toml
📄 CodeRabbit inference engine (AGENTS.md)
MSRV is 1.89; set and respect rust-version = "1.89" in Cargo.toml
Files:
Cargo.tomldash-spv/Cargo.tomldash-network-ffi/Cargo.tomlkey-wallet-manager/Cargo.tomlkey-wallet/Cargo.tomldash-spv-ffi/Cargo.tomlkey-wallet-ffi/Cargo.toml
dash-spv/**/Cargo.toml
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Set MSRV to 1.89 using the
rust-versionfield and keep the code compatible with Rust 1.89
Files:
dash-spv/Cargo.toml
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
📄 CodeRabbit inference engine (swift-dash-core-sdk/CLAUDE.md)
After updating dash-spv-ffi, synchronize the generated header into the Swift SDK using ./sync-headers.sh so Sources/DashSPVFFI/include/dash_spv_ffi.h stays up to date
Files:
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Be careful with memory management at FFI boundaries (C/Swift interop)
Files:
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/src/config.rsswift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClientConfiguration.swiftdash-spv-ffi/include/dash_spv_ffi.hkey-wallet-ffi/include/key_wallet_ffi.h
swift-dash-core-sdk/**
📄 CodeRabbit inference engine (AGENTS.md)
Mobile SDK code resides in swift-dash-core-sdk/
Files:
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hswift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClientConfiguration.swift
*-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
FFI bindings live in *-ffi crates
Files:
dash-network-ffi/Cargo.tomldash-spv-ffi/src/config.rsdash-spv-ffi/Cargo.tomldash-spv-ffi/FFI_API.mdkey-wallet-ffi/Cargo.tomldash-spv-ffi/include/dash_spv_ffi.hkey-wallet-ffi/include/key_wallet_ffi.h
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/network/handshake.rsdash-spv/tests/test_handshake_logic.rsdash-spv/src/network/multi_peer.rsdash-spv/src/network/mod.rsdash-spv/src/network/tests.rsdash-spv/src/client/config.rs
dash-spv/src/network/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided
Files:
dash-spv/src/network/handshake.rsdash-spv/src/network/multi_peer.rsdash-spv/src/network/mod.rsdash-spv/src/network/tests.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/network/handshake.rsdash-spv/tests/test_handshake_logic.rsdash-spv-ffi/src/config.rsdash-spv/src/network/multi_peer.rsdash-spv/src/network/mod.rsdash-spv/src/network/tests.rsdash-spv/src/client/config.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/network/handshake.rsdash-spv-ffi/src/config.rsdash-spv/src/network/multi_peer.rsdash-spv/src/network/mod.rsdash-spv/src/network/tests.rsdash-spv/src/client/config.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Place integration tests under the
tests/directory
Files:
dash-spv/tests/test_handshake_logic.rs
{dash-spv,rpc-integration-test}/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Provide integration tests for network operations
Files:
dash-spv/tests/test_handshake_logic.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use proptest for property-based testing where appropriate
**/tests/**/*.rs: Integration tests must live under tests/ (e.g., rpc-integration-test)
Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Mark network-dependent or long-running tests with #[ignore] and run with -- --ignored
Add regression tests for fixes; consider property tests (e.g., proptest) where valuable
Keep test vectors deterministic
Files:
dash-spv/tests/test_handshake_logic.rs
dash-spv-ffi/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/src/**/*.rs: FFI functions must be declared with #[no_mangle] extern "C"
Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Accept input strings from C as *const c_char (borrowed; do not free inputs)
Functions returning pointers transfer ownership to the caller; functions taking pointers only borrow
Add cbindgen annotations for complex types so headers are generated correctly
Files:
dash-spv-ffi/src/config.rs
key-wallet/**/Cargo.toml
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Set MSRV via rust-version = "1.89" in Cargo.toml
Files:
key-wallet/Cargo.toml
swift-dash-core-sdk/Sources/**/*.swift
📄 CodeRabbit inference engine (swift-dash-core-sdk/CLAUDE.md)
swift-dash-core-sdk/Sources/**/*.swift: Use actors for state management to maintain thread safety in SDK components
Use @available checks for features that require newer OS versions (e.g., SwiftData on iOS 17+)
Files:
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClientConfiguration.swift
{key-wallet,swift-dash-core-sdk}/**/*.{rs,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Never log or expose private keys
Files:
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClientConfiguration.swift
dash-spv-ffi/include/dash_spv_ffi.h
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
The public C header is generated; ensure it reflects FFI changes after builds
Files:
dash-spv-ffi/include/dash_spv_ffi.h
🧠 Learnings (16)
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/Cargo.toml : Set MSRV to 1.89 using the `rust-version` field and keep the code compatible with Rust 1.89
Applied to files:
dash-spv/Cargo.tomldash-network-ffi/Cargo.tomldash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv/Cargo.tomldash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : Enforce Rust formatting via `cargo fmt --check` on all Rust source files
Applied to files:
dash-spv/Cargo.tomldash-spv-ffi/Cargo.toml
📚 Learning: 2025-09-01T14:44:53.683Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Pre-PR checks: cargo fmt, cargo clippy, cargo test (workspace); update docs/CHANGELOG for user-facing changes
Applied to files:
dash-spv/Cargo.tomldash-network-ffi/Cargo.tomlkey-wallet-manager/Cargo.tomlkey-wallet/Cargo.tomldash-spv-ffi/Cargo.tomlkey-wallet-ffi/Cargo.toml
📚 Learning: 2025-08-21T04:45:50.436Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#108
File: key-wallet/src/gap_limit.rs:291-295
Timestamp: 2025-08-21T04:45:50.436Z
Learning: The rust-dashcore project uses Rust 1.89 as evidenced by rust-version = "1.89" in dash-spv/Cargo.toml. Modern Rust features like Option::is_none_or (stabilized in 1.82) can be used safely.
Applied to files:
dash-spv/Cargo.tomldash-network/README.md
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
Applied to files:
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-network-ffi/Cargo.tomldash-spv-ffi/Cargo.tomldash-spv-ffi/FFI_API.mddash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:15:29.335Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h : After updating dash-spv-ffi, synchronize the generated header into the Swift SDK using ./sync-headers.sh so Sources/DashSPVFFI/include/dash_spv_ffi.h stays up to date
Applied to files:
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/Cargo.tomldash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
dash-network/README.mddash-spv/src/network/tests.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/**/Cargo.toml : Set MSRV via rust-version = "1.89" in Cargo.toml
Applied to files:
key-wallet-manager/Cargo.tomlkey-wallet/Cargo.tomlkey-wallet-ffi/Cargo.toml
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Applied to files:
key-wallet/Cargo.toml
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-16T04:15:57.225Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:57.225Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : When accessing SPV functionality, use DashSDK public APIs instead of direct client access; add public wrapper methods to DashSDK if needed
Applied to files:
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClientConfiguration.swift
📚 Learning: 2025-08-16T04:15:57.225Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:57.225Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/SPVClient.swift : Do not place type definitions in SPVClient.swift; keep types in dedicated files instead
Applied to files:
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClientConfiguration.swift
📚 Learning: 2025-08-21T05:01:58.949Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.
Applied to files:
key-wallet-ffi/include/key_wallet_ffi.h
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/address_pool/**/*.rs : Pre-generate addresses in bounded batches (typically 20–100) and store them in pools
Applied to files:
key-wallet-ffi/include/key_wallet_ffi.h
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/address_pool/**/*.rs : Do not generate unbounded addresses; enforce gap limit and staged address generation in address pools
Applied to files:
key-wallet-ffi/include/key_wallet_ffi.h
🧬 Code graph analysis (6)
dash-spv/src/network/handshake.rs (2)
dash-spv/src/network/mod.rs (1)
new(165-180)dash-spv/src/network/multi_peer.rs (1)
new(75-121)
dash-spv/tests/test_handshake_logic.rs (2)
dash-spv/src/network/handshake.rs (1)
new(48-65)dash-spv/src/network/mod.rs (1)
new(165-180)
dash-spv/src/network/multi_peer.rs (3)
dash-spv/src/network/handshake.rs (1)
new(48-65)dash-spv/src/network/mod.rs (1)
new(165-180)dash-spv/src/network/connection.rs (1)
new(51-76)
dash-spv/src/network/mod.rs (2)
dash-spv/src/network/handshake.rs (1)
new(48-65)dash-spv/src/network/multi_peer.rs (1)
new(75-121)
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClientConfiguration.swift (2)
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/FFIBridge.swift (2)
withCString(131-133)checkError(60-65)dash-spv-ffi/src/config.rs (1)
dash_spv_ffi_config_set_user_agent(156-176)
key-wallet-ffi/include/key_wallet_ffi.h (2)
key-wallet-ffi/src/address.rs (4)
address_free(20-26)address_array_free(37-50)address_validate(59-114)address_get_type(129-189)key-wallet-ffi/src/address_pool.rs (9)
address_pool_free(217-221)managed_wallet_get_address_pool_info(248-356)managed_wallet_set_gap_limit(368-463)managed_wallet_generate_addresses_to_index(477-630)managed_wallet_mark_address_used(643-799)address_pool_get_address_at_index(812-841)address_pool_get_addresses_in_range(858-907)address_info_free(916-943)address_info_array_free(953-960)
⏰ 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). (6)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
🔇 Additional comments (14)
dash-network/README.md (1)
22-22: Update looks good; ensure release timing.The version bump in the usage snippet matches the workspace 0.40.0. If this merges before publishing 0.40.0 to crates.io, consider holding or adding a note to avoid broken copy-paste.
dash-spv/Cargo.toml (1)
3-3: LGTM on workspace-managed version.MSRV already set to 1.89 (Line 9). No further changes needed.
key-wallet-ffi/include/key_wallet_ffi.h (2)
1751-1754: API contract/ownership looks good.Returning a heap-allocated FFIAddressInfo and pairing with address_info_free is consistent. No issues.
1604-1617: Free helpers are consistent with alloc sites.address_free/address_array_free pair correctly with the documented producers. No issues.
dash-spv/src/network/tests.rs (1)
181-182: LGTM: test config includes the new user_agent field.Keeps the struct literal in sync with ClientConfig.
dash-spv/src/client/config.rs (1)
199-199: Default of None is appropriate.Defers to library default at handshake; no concerns.
dash-spv/tests/test_handshake_logic.rs (1)
9-9: Constructor update looks correct.Passing
Nonefor the newuser_agentparameter matches the updated API.dash-spv/src/network/mod.rs (1)
171-175: Good: user agent is now propagated to HandshakeManager.This wires
config.user_agent.clone()into the handshake as intended.dash-spv-ffi/FFI_API.md (1)
62-62: UA config doc change looks correct.The table now reflects that the UA is advertised during handshake. No API signature change here.
dash-spv/src/network/handshake.rs (2)
43-44: Add user_agent to HandshakeManager state — LGTM.Clear ownership and propagation path for UA.
48-52: Constructor signature change applied; all 3-arg call sites updatedVerified every
HandshakeManager::newuse now supplies(network, mempool_strategy, user_agent), including tests and bothmod.rsandmulti_peer.rsinstantiations.dash-spv/src/network/multi_peer.rs (3)
69-71: Carry user_agent in MultiPeerNetworkManager — LGTM.Field addition and doc are clear.
119-120: Populate UA from config — LGTM.Keeps config-driven behavior consistent across single/multi-peer managers.
979-980: Clone implementation updated for UA — LGTM.Ensures UA is preserved across cloned managers.
| [package] | ||
| name = "dash-network-ffi" | ||
| version.workspace = true | ||
| version = { workspace = true } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Switch to workspace versioning is correct; add MSRV.
Per project guidelines, declare MSRV explicitly.
[package]
name = "dash-network-ffi"
version = { workspace = true }
+rust-version = "1.89"
edition = "2021"🤖 Prompt for AI Agents
In dash-network-ffi/Cargo.toml at line 3, the package is using workspace
versioning but does not declare the MSRV; add a rust-version entry in the
package table (rust-version = "<PROJECT_MSRV>") set to the project’s documented
minimum supported Rust version so the crate explicitly declares the MSRV and
stays consistent with workspace/CI.
| [package] | ||
| name = "dash-spv-ffi" | ||
| version = "0.1.0" | ||
| version = { workspace = true } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align with workspace versioning; also set MSRV.
Declare rust-version here to satisfy the repo’s MSRV policy.
[package]
name = "dash-spv-ffi"
version = { workspace = true }
+rust-version = "1.89"
edition = "2021"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| version = { workspace = true } | |
| [package] | |
| name = "dash-spv-ffi" | |
| version = { workspace = true } | |
| rust-version = "1.89" | |
| edition = "2021" |
🤖 Prompt for AI Agents
In dash-spv-ffi/Cargo.toml around line 3, the package lacks a rust-version
declaration required by the repo MSRV policy; add a rust-version entry (e.g.
rust-version = "<MSRV>") alongside the existing version = { workspace = true }
so the crate explicitly declares the minimum supported Rust version used by the
workspace. Replace <MSRV> with the repository's agreed MSRV (for example "1.70"
or the value defined in the repo policy).
| // Determine user agent: prefer configured value, else default to crate/version. | ||
| let default_agent = format!("/rust-dash-spv:{}/", env!("CARGO_PKG_VERSION")); | ||
| let mut ua = self.user_agent.clone().unwrap_or(default_agent); | ||
| // Normalize: ensure it starts and ends with '/'; trim if excessively long. | ||
| if !ua.starts_with('/') { | ||
| ua.insert(0, '/'); | ||
| } | ||
| if !ua.ends_with('/') { | ||
| ua.push('/'); | ||
| } | ||
| // Keep within a reasonable bound (match peer validation bound of 256) | ||
| if ua.len() > 256 { | ||
| ua.truncate(256); | ||
| } |
There was a problem hiding this comment.
Possible panic on String::truncate(256) with non-ASCII UA; also allow-only printable ASCII.
truncate expects a char boundary; a multi-byte UA from config/FFI can panic. Also, per BIP14 practice, restrict to printable ASCII and cap length deterministically.
Apply:
- let default_agent = format!("/rust-dash-spv:{}/", env!("CARGO_PKG_VERSION"));
- let mut ua = self.user_agent.clone().unwrap_or(default_agent);
- // Normalize: ensure it starts and ends with '/'; trim if excessively long.
- if !ua.starts_with('/') {
- ua.insert(0, '/');
- }
- if !ua.ends_with('/') {
- ua.push('/');
- }
- // Keep within a reasonable bound (match peer validation bound of 256)
- if ua.len() > 256 {
- ua.truncate(256);
- }
+ let default_agent = format!("/rust-dash-spv:{}/", env!("CARGO_PKG_VERSION"));
+ let raw = self.user_agent.as_deref().unwrap_or(&default_agent);
+ // Strip non-printable / non-ASCII and inner slashes; rebuild as /.../
+ let mut core = raw
+ .trim()
+ .trim_matches('/')
+ .chars()
+ .filter(|c| matches!(*c, ' '..='~')) // printable ASCII only
+ .collect::<String>();
+ // Rewrap with leading/trailing slash
+ let mut ua = String::with_capacity(core.len() + 2);
+ ua.push('/');
+ ua.push_str(&core);
+ ua.push('/');
+ // Cap to 256 bytes; ASCII ensures byte==char, safe truncate
+ if ua.len() > 256 {
+ // Preserve closing '/' by limiting core length
+ let keep = 255; // leave space for trailing '/'
+ ua.truncate(keep);
+ if !ua.ends_with('/') {
+ ua.pop(); // ensure final char slot
+ ua.push('/');
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Determine user agent: prefer configured value, else default to crate/version. | |
| let default_agent = format!("/rust-dash-spv:{}/", env!("CARGO_PKG_VERSION")); | |
| let mut ua = self.user_agent.clone().unwrap_or(default_agent); | |
| // Normalize: ensure it starts and ends with '/'; trim if excessively long. | |
| if !ua.starts_with('/') { | |
| ua.insert(0, '/'); | |
| } | |
| if !ua.ends_with('/') { | |
| ua.push('/'); | |
| } | |
| // Keep within a reasonable bound (match peer validation bound of 256) | |
| if ua.len() > 256 { | |
| ua.truncate(256); | |
| } | |
| // Determine user agent: prefer configured value, else default to crate/version. | |
| let default_agent = format!("/rust-dash-spv:{}/", env!("CARGO_PKG_VERSION")); | |
| let raw = self.user_agent.as_deref().unwrap_or(&default_agent); | |
| // Strip non-printable / non-ASCII and inner slashes; rebuild as /.../ | |
| let mut core = raw | |
| .trim() | |
| .trim_matches('/') | |
| .chars() | |
| .filter(|c| matches!(*c, ' '..='~')) // printable ASCII only | |
| .collect::<String>(); | |
| // Rewrap with leading/trailing slash | |
| let mut ua = String::with_capacity(core.len() + 2); | |
| ua.push('/'); | |
| ua.push_str(&core); | |
| ua.push('/'); | |
| // Cap to 256 bytes; ASCII ensures byte==char, safe truncate | |
| if ua.len() > 256 { | |
| // Preserve closing '/' by limiting core length | |
| let keep = 255; // leave space for trailing '/' | |
| ua.truncate(keep); | |
| if !ua.ends_with('/') { | |
| ua.pop(); // drop last char if it's not '/', then push '/' | |
| ua.push('/'); | |
| } | |
| } |
| [package] | ||
| name = "key-wallet-ffi" | ||
| version = "0.39.6" | ||
| version = { workspace = true } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Set MSRV in manifest (rust-version = "1.89").
Per project guidelines, explicitly declare MSRV in this crate.
[package]
name = "key-wallet-ffi"
-version = { workspace = true }
+version = { workspace = true }
+rust-version = "1.89"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| version = { workspace = true } | |
| [package] | |
| name = "key-wallet-ffi" | |
| version = { workspace = true } | |
| rust-version = "1.89" |
🤖 Prompt for AI Agents
In key-wallet-ffi/Cargo.toml around lines 3-3, the crate manifest does not
declare the MSRV; add a rust-version = "1.89" entry to the package manifest
(place it alongside the existing version = { workspace = true } line under
[package]) so the crate explicitly declares the minimum supported Rust version.
| bool managed_wallet_generate_addresses_to_index(FFIManagedWallet *managed_wallet, | ||
| const FFIWallet *wallet, | ||
| FFINetwork network, | ||
| FFIAccountType account_type, | ||
| unsigned int account_index, | ||
| FFIAddressPoolType pool_type, | ||
| unsigned int target_index, | ||
| FFIError *error) | ||
| ; |
There was a problem hiding this comment.
Off-by-one and “no addresses yet” bug in generate-to-index.
If highest_generated is None, the Rust impl treats it as 0 and won’t generate index 0; also the inclusive semantics (“up to and including”) are not guaranteed. Fix needed in the Rust function.
Proposed Rust patch (key-wallet-ffi/src/address_pool.rs):
- {
- let current = external_addresses.highest_generated.unwrap_or(0);
- if target_index > current {
- let needed = target_index - current;
- external_addresses.generate_addresses(needed, &key_source, true)
- } else {
- Ok(Vec::new())
- }
- }
+ {
+ let needed = match external_addresses.highest_generated {
+ Some(cur) if target_index > cur => target_index - cur,
+ None => target_index.saturating_add(1),
+ _ => 0,
+ };
+ if needed > 0 {
+ external_addresses.generate_addresses(needed, &key_source, true)
+ } else {
+ Ok(Vec::new())
+ }
+ }Apply the same pattern to the Internal and Single pool branches.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bool managed_wallet_generate_addresses_to_index(FFIManagedWallet *managed_wallet, | |
| const FFIWallet *wallet, | |
| FFINetwork network, | |
| FFIAccountType account_type, | |
| unsigned int account_index, | |
| FFIAddressPoolType pool_type, | |
| unsigned int target_index, | |
| FFIError *error) | |
| ; | |
| { | |
| let needed = match external_addresses.highest_generated { | |
| Some(cur) if target_index > cur => target_index - cur, | |
| None => target_index.saturating_add(1), | |
| _ => 0, | |
| }; | |
| if needed > 0 { | |
| external_addresses.generate_addresses(needed, &key_source, true) | |
| } else { | |
| Ok(Vec::new()) | |
| } | |
| } |
🤖 Prompt for AI Agents
In key-wallet-ffi/include/key_wallet_ffi.h around lines 1709 to 1717: the Rust
implementation that backs managed_wallet_generate_addresses_to_index has an
off-by-one/no-addresses-generated bug because when highest_generated is None it
is treated as 0 and index 0 is skipped, and the target-index semantics aren’t
guaranteed to be inclusive; update key-wallet-ffi/src/address_pool.rs so each
pool branch calculates start_index = highest_generated.map(|g| g +
1).unwrap_or(0) and then generates addresses in an inclusive range
start_index..=target_index (not exclusive), and apply this exact pattern to the
Internal and Single pool branches as well to ensure index 0 is produced when no
addresses exist and the target is inclusive.
| bool managed_wallet_mark_address_used(FFIManagedWallet *managed_wallet, | ||
| FFINetwork network, | ||
| const char *address, | ||
| FFIError *error) | ||
| ; |
There was a problem hiding this comment.
Address network is not validated; may mark wrong-network addresses as used.
The Rust impl parses an unchecked address and calls assume_checked() without verifying the provided FFINetwork. Validate network before marking.
Proposed Rust patch (key-wallet-ffi/src/address_pool.rs):
- // Assume the address uses the same network we're working with
- let address = unchecked_addr.assume_checked();
+ // Validate address matches requested network
+ let network_dash: dashcore::Network = network_rust.into();
+ let address = match unchecked_addr.require_network(network_dash) {
+ Ok(checked) => checked,
+ Err(_) => {
+ FFIError::set_error(
+ error,
+ FFIErrorCode::InvalidAddress,
+ "Address not valid for provided network".to_string(),
+ );
+ return false;
+ }
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bool managed_wallet_mark_address_used(FFIManagedWallet *managed_wallet, | |
| FFINetwork network, | |
| const char *address, | |
| FFIError *error) | |
| ; | |
| // Validate address matches requested network | |
| let network_dash: dashcore::Network = network_rust.into(); | |
| let address = match unchecked_addr.require_network(network_dash) { | |
| Ok(checked) => checked, | |
| Err(_) => { | |
| FFIError::set_error( | |
| error, | |
| FFIErrorCode::InvalidAddress, | |
| "Address not valid for provided network".to_string(), | |
| ); | |
| return false; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In key-wallet-ffi/include/key_wallet_ffi.h around lines 1732-1736, the
managed_wallet_mark_address_used path does not validate that the provided
FFINetwork matches the parsed address network; update the Rust implementation so
it parses the address, verifies the parsed address's network equals the provided
FFINetwork, and only then calls assume_checked() and marks it used. If the
networks differ (or parsing fails) populate FFIError with a clear error and
return false; do not call assume_checked on an unchecked/mismatched address.
Ensure the function returns false on error and avoids marking the wrong-network
address as used.
| [package] | ||
| name = "key-wallet-manager" | ||
| version = "0.1.0" | ||
| version = { workspace = true } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Set MSRV in manifest (rust-version = "1.89").
Matches the requirement for key-wallet/* crates.
[package]
name = "key-wallet-manager"
-version = { workspace = true }
+version = { workspace = true }
+rust-version = "1.89"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| version = { workspace = true } | |
| [package] | |
| name = "key-wallet-manager" | |
| version = { workspace = true } | |
| rust-version = "1.89" |
🤖 Prompt for AI Agents
In key-wallet-manager/Cargo.toml around line 3 the crate lacks an explicit MSRV;
add rust-version = "1.89" to the manifest. Edit the Cargo.toml to include
rust-version = "1.89" (alongside or near the existing version = { workspace =
true }) so the crate declares the minimum supported Rust version consistent with
key-wallet/*.
| [package] | ||
| name = "key-wallet" | ||
| version = "0.40.0-dev" | ||
| version = { workspace = true } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Set MSRV in manifest (rust-version = "1.89").
Keep test/build matrices aligned with the workspace MSRV.
[package]
name = "key-wallet"
-version = { workspace = true }
+version = { workspace = true }
+rust-version = "1.89"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| version = { workspace = true } | |
| [package] | |
| name = "key-wallet" | |
| version = { workspace = true } | |
| rust-version = "1.89" |
🤖 Prompt for AI Agents
In key-wallet/Cargo.toml around line 3, the package manifest lacks an explicit
minimum supported Rust version; add rust-version = "1.89" to the manifest
(alongside the existing version = { workspace = true }) so the crate declares
MSRV 1.89 and keeps test/build matrices aligned with the workspace.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dash-spv-ffi/tests/unit/test_configuration.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
dash-spv-ffi/tests/unit/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
Add a corresponding Rust unit test for each new FFI function
Files:
dash-spv-ffi/tests/unit/test_configuration.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv-ffi/tests/unit/test_configuration.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Be careful with memory management at FFI boundaries (C/Swift interop)
Files:
dash-spv-ffi/tests/unit/test_configuration.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use proptest for property-based testing where appropriate
**/tests/**/*.rs: Integration tests must live under tests/ (e.g., rpc-integration-test)
Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Mark network-dependent or long-running tests with #[ignore] and run with -- --ignored
Add regression tests for fixes; consider property tests (e.g., proptest) where valuable
Keep test vectors deterministic
Files:
dash-spv-ffi/tests/unit/test_configuration.rs
*-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
FFI bindings live in *-ffi crates
Files:
dash-spv-ffi/tests/unit/test_configuration.rs
🧠 Learnings (1)
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function
Applied to files:
dash-spv-ffi/tests/unit/test_configuration.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). (11)
- GitHub Check: Integration Tests (stable, 22.1.3)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: Clippy (Non-strict)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_block)
🔇 Additional comments (1)
dash-spv-ffi/tests/unit/test_configuration.rs (1)
185-189: Expecting Success for set_user_agent — LGTMThis aligns with the updated config behavior returning Success on valid UTF-8 input.
| let user_agent = CString::new("TestAgent/1.0").unwrap(); | ||
| assert_eq!( | ||
| dash_spv_ffi_config_set_user_agent(config, user_agent.as_ptr()), | ||
| FFIErrorCode::ConfigError as i32 | ||
| FFIErrorCode::Success as i32 | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add edge/negative tests for user_agent (invalid UTF-8, length cap, empty string behavior)
To lock in the new semantics and prevent regressions:
- Add a unit test that passes invalid UTF-8 bytes (no interior NUL) and asserts InvalidArgument.
- Add a test with a very long user agent (e.g., >1024 bytes) to ensure the setter succeeds and does not crash; cover normalization/capping in an SPV handshake-level test.
- Clarify and test the expected behavior for empty user agent (accept and default vs. reject).
Example unit test snippet to add in this file:
#[test]
#[serial]
fn test_user_agent_invalid_utf8_and_long() {
unsafe {
let config = dash_spv_ffi_config_testnet();
// Invalid UTF-8 sequence: 0xF0 0x28 0x8C 0x28
let invalid = std::ffi::CString::new(vec![0xF0, 0x28, 0x8C, 0x28]).unwrap();
let rc = dash_spv_ffi_config_set_user_agent(config, invalid.as_ptr());
assert_eq!(rc, FFIErrorCode::InvalidArgument as i32);
// Very long but valid ASCII user agent
let long = "A".repeat(10_000);
let long_c = std::ffi::CString::new(long).unwrap();
let rc2 = dash_spv_ffi_config_set_user_agent(config, long_c.as_ptr());
assert_eq!(rc2, FFIErrorCode::Success as i32);
dash_spv_ffi_config_destroy(config);
}
}For normalization/capping, add an integration test in the SPV layer that inspects the Version message user agent after handshake and asserts it’s normalized and within the cap.
Run this quick scan to confirm presence of related FFI and handshake hooks:
🏁 Script executed:
#!/bin/bash
# Locate set/get user agent FFI functions and handshake usage
rg -nP 'dash_spv_ffi_config_(set|get)_user_agent'
rg -nP --type=c 'dash_spv_ffi_config_(set|get)_user_agent' -g '*.{h,c}'
rg -nP --type=rust -C2 '\bVersion(Message)?\b.*user[_ ]?agent|user_agent' Length of output: 22219
Add edge-case unit tests for user_agent setter
In dash-spv-ffi/tests/unit/test_configuration.rs, add tests for:
- Invalid UTF-8 (e.g. bytes
[0xF0, 0x28, 0x8C, 0x28]) returnsFFIErrorCode::InvalidArgument. - Empty string returns
FFIErrorCode::Successand stores"". - Very long ASCII string (>10 000 bytes) returns
FFIErrorCode::Successwithout crashing.
Optionally, add an SPV handshake integration test to assert the Version message’s user_agent is normalized (starts/ends with /) and capped to MAX_USER_AGENT_LENGTH.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores