-
Notifications
You must be signed in to change notification settings - Fork 10
feat: update version to 0.40.0 and implement user agent configuration #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| [package] | ||
| name = "dash-network-ffi" | ||
| version.workspace = true | ||
| version = { workspace = true } | ||
| edition = "2021" | ||
| authors = ["Quantum Explorer <[email protected]>"] | ||
| license = "CC0-1.0" | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||||||
| [package] | ||||||||||||||
| name = "dash-spv-ffi" | ||||||||||||||
| version = "0.1.0" | ||||||||||||||
| version = { workspace = true } | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| edition = "2021" | ||||||||||||||
| authors = ["Dash Core Developers"] | ||||||||||||||
| license = "MIT" | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -185,7 +185,7 @@ mod tests { | |
| 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 | ||
| ); | ||
|
Comment on lines
185
to
189
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainAdd edge/negative tests for user_agent (invalid UTF-8, length cap, empty string behavior) To lock in the new semantics and prevent regressions:
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
Optionally, add an SPV handshake integration test to assert the Version message’s |
||
|
|
||
| assert_eq!( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,11 +40,16 @@ pub struct HandshakeManager { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| verack_received: bool, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| version_sent: bool, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mempool_strategy: MempoolStrategy, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_agent: Option<String>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| impl HandshakeManager { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Create a new handshake manager. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub fn new(network: Network, mempool_strategy: MempoolStrategy) -> Self { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub fn new( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| network: Network, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mempool_strategy: MempoolStrategy, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_agent: Option<String>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Self { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Self { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _network: network, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| state: HandshakeState::Init, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -55,6 +60,7 @@ impl HandshakeManager { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| verack_received: false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| version_sent: false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mempool_strategy, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_agent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -254,14 +260,29 @@ impl HandshakeManager { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .parse() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map_err(|_| NetworkError::AddressParse("Failed to parse local address".to_string()))?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+263
to
+276
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Ok(VersionMessage { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| version: self.our_version, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| services, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timestamp, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| receiver: dashcore::network::address::Address::new(&address, ServiceFlags::NETWORK), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sender: dashcore::network::address::Address::new(&local_addr, services), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nonce: rand::random(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_agent: "/rust-dash-spv:0.1.0/".to_string(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_agent: ua, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| start_height: 0, // SPV client starts at 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| relay: match self.mempool_strategy { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MempoolStrategy::FetchAll => true, // Want all transactions for FetchAll strategy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||||
| [package] | ||||||||||||
| name = "key-wallet-ffi" | ||||||||||||
| version = "0.39.6" | ||||||||||||
| version = { workspace = true } | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| authors = ["The Dash Core Developers"] | ||||||||||||
| edition = "2021" | ||||||||||||
| description = "FFI bindings for key-wallet library" | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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