From 097602e8a6a3f10d11cd9f1ce85194bef6c27ca8 Mon Sep 17 00:00:00 2001 From: b5 Date: Sun, 17 May 2026 18:16:37 -0400 Subject: [PATCH 1/7] feat: endpoint groups and attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add cloud-side metadata for client endpoints beyond the existing name: a single `group` and an arbitrary key-value `attributes` map. Both are settable at build time via `ClientBuilder::group` / `attributes` and updatable post-construction via `Client::set_group` / `set_attributes`, mirroring the name API. Attributes use full-replace semantics on each call. Adds matching `SetGroup` / `SetAttributes` RPC messages to the protocol, plus an `endpoint_meta` example that exercises both paths. Group names follow the same 2–128 byte UTF-8 rules as endpoint names. Attribute keys share those rules; values may be empty and are capped at 128 bytes; the map is capped at 128 entries. --- examples/endpoint_meta.rs | 56 ++++++ src/client.rs | 348 +++++++++++++++++++++++++++++++++++++- src/protocol.rs | 20 +++ 3 files changed, 421 insertions(+), 3 deletions(-) create mode 100644 examples/endpoint_meta.rs diff --git a/examples/endpoint_meta.rs b/examples/endpoint_meta.rs new file mode 100644 index 00000000..f9cdef43 --- /dev/null +++ b/examples/endpoint_meta.rs @@ -0,0 +1,56 @@ +//! Endpoint metadata example. +//! +//! Demonstrates how to associate metadata with an endpoint via the iroh-services +//! Client: a human-readable `name`, a single `group`, and arbitrary key-value +//! `attributes`. Each can be set at build time via the [`ClientBuilder`], and +//! updated later through the `Client::set_*` methods. +//! +//! Run with: `IROH_SERVICES_API_SECRET=... cargo run --example endpoint_meta` +use iroh::{Endpoint, endpoint::presets}; +use iroh_services::Client; + +#[tokio::main] +async fn main() -> anyhow::Result<()> { + tracing_subscriber::fmt::init(); + + let endpoint = Endpoint::bind(presets::N0).await?; + + // Derive a unique name from the endpoint id so repeated runs don't collide + // in dashboards. In a real app this is typically a user id, machine name, + // or other stable identifier from your application. + let id = endpoint.id().to_string(); + let name = format!("endpoint-meta-example-{}", &id[..8]); + + // Set name, group, and attributes at build time. The client sends these + // immediately after authenticating with iroh-services. Validation errors + // (e.g. name too long) surface here; transport errors during startup are + // logged at `warn` level rather than failing the build. + let client = Client::builder(&endpoint) + .api_secret_from_env()? + .name(name)? + .group("examples")? + .attributes([ + ("env", "dev"), + ("region", "us-west"), + ("role", "endpoint-meta-example"), + ])? + .build() + .await?; + + client.ping().await?; + println!("endpoint registered with initial metadata"); + + // Each metadata field can also be updated after construction. These calls + // return explicit errors, unlike the builder which logs and continues. + client.set_name("endpoint-meta-example-renamed").await?; + client.set_group("staging").await?; + + // set_attributes fully replaces the prior set on each call. Pass an empty + // iterator to clear all attributes. + client + .set_attributes([("env", "staging"), ("region", "eu-central")]) + .await?; + + println!("metadata updated"); + Ok(()) +} diff --git a/src/client.rs b/src/client.rs index 5931d209..fa08b702 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,4 +1,5 @@ use std::{ + collections::BTreeMap, str::FromStr, sync::{Arc, RwLock}, }; @@ -20,7 +21,7 @@ use crate::{ net_diagnostics::{DiagnosticsReport, checks::run_diagnostics}, protocol::{ ALPN, Auth, IrohServicesClient, NameEndpoint, Ping, Pong, PutMetrics, - PutNetworkDiagnostics, RemoteError, + PutNetworkDiagnostics, RemoteError, SetAttributes, SetGroup, }, }; @@ -64,6 +65,8 @@ pub struct ClientBuilder { cap: Option>, endpoint: Endpoint, name: Option, + group: Option, + attributes: Option>, metrics_interval: Option, remote: Option, registry: Registry, @@ -79,6 +82,8 @@ impl ClientBuilder { cap_expiry: DEFAULT_CAP_EXPIRY, endpoint: endpoint.clone(), name: None, + group: None, + attributes: None, metrics_interval: Some(Duration::from_secs(60)), remote: None, registry, @@ -128,6 +133,47 @@ impl ClientBuilder { Ok(self) } + /// Attach the endpoint to a single named group when the client first + /// authenticates. Group names follow the same rules as endpoint names + /// (2–128 bytes UTF-8). Errors during startup propagate as warn-level + /// logs; for explicit error handling use [`Client::set_group`]. + pub fn group(mut self, group: impl Into) -> Result { + let group = group.into(); + validate_name(&group).map_err(BuildError::InvalidGroup)?; + self.group = Some(group); + Ok(self) + } + + /// Attach arbitrary key-value attributes to the endpoint when the client + /// first authenticates. Accepts any iterable of `(key, value)` pairs: + /// + /// ```no_run + /// # use iroh::{Endpoint, endpoint::presets}; + /// # use iroh_services::Client; + /// # async fn example(endpoint: &Endpoint) -> anyhow::Result<()> { + /// let _ = Client::builder(endpoint).attributes([("env", "prod"), ("region", "us-west")])?; + /// # Ok(()) } + /// ``` + /// + /// Keys follow the same length rules as endpoint names (2–128 bytes); + /// values may be empty and are capped at 128 bytes; the map is limited + /// to 128 entries. Errors during startup propagate as warn-level logs; + /// for explicit error handling use [`Client::set_attributes`]. + pub fn attributes(mut self, attrs: I) -> Result + where + I: IntoIterator, + K: Into, + V: Into, + { + let collected: BTreeMap = attrs + .into_iter() + .map(|(k, v)| (k.into(), v.into())) + .collect(); + validate_attributes(&collected).map_err(BuildError::InvalidAttributes)?; + self.attributes = Some(collected); + Ok(self) + } + /// Check IROH_SERVICES_API_SECRET environment variable for a valid API secret pub fn api_secret_from_env(self) -> Result { let ticket = ApiSecret::from_env_var(API_SECRET_ENV_VAR_NAME)?; @@ -216,10 +262,19 @@ impl ClientBuilder { capabilities, client: irpc_client, name: self.name.clone(), + group: self.group.clone(), + attributes: self.attributes.clone().unwrap_or_default(), session_id: Uuid::new_v4(), authorized: false, } - .run(self.name, self.registry, self.metrics_interval, rx), + .run( + self.name, + self.group, + self.attributes, + self.registry, + self.metrics_interval, + rx, + ), )); Ok(Client { @@ -246,6 +301,10 @@ pub enum BuildError { Connect(ConnectError), #[error("Invalid endpoint name: {0}")] InvalidName(#[from] ValidateNameError), + #[error("Invalid endpoint group: {0}")] + InvalidGroup(ValidateNameError), + #[error("Invalid endpoint attributes: {0}")] + InvalidAttributes(#[from] ValidateAttributesError), } impl From for BuildError { @@ -288,10 +347,45 @@ fn validate_name(name: &str) -> Result<(), ValidateNameError> { } } +/// Maximum length in bytes for an attribute value. Values may be empty. +pub const CLIENT_ATTRIBUTE_VALUE_MAX_LENGTH: usize = 128; +/// Maximum number of entries allowed in the attributes map. +pub const CLIENT_ATTRIBUTES_MAX_COUNT: usize = 128; + +/// Error returned when an attributes map fails validation. +#[derive(Debug, thiserror::Error)] +pub enum ValidateAttributesError { + #[error("Too many attributes (must be no more than {CLIENT_ATTRIBUTES_MAX_COUNT}).")] + TooManyEntries, + #[error("Invalid attribute key: {0}")] + InvalidKey(#[from] ValidateNameError), + #[error( + "Attribute value too long (must be no more than {CLIENT_ATTRIBUTE_VALUE_MAX_LENGTH} bytes)." + )] + ValueTooLong, +} + +fn validate_attributes(attrs: &BTreeMap) -> Result<(), ValidateAttributesError> { + if attrs.len() > CLIENT_ATTRIBUTES_MAX_COUNT { + return Err(ValidateAttributesError::TooManyEntries); + } + for (k, v) in attrs { + validate_name(k)?; + if v.len() > CLIENT_ATTRIBUTE_VALUE_MAX_LENGTH { + return Err(ValidateAttributesError::ValueTooLong); + } + } + Ok(()) +} + #[derive(thiserror::Error, Debug)] pub enum Error { #[error("Invalid endpoint name: {0}")] InvalidName(#[from] ValidateNameError), + #[error("Invalid endpoint group: {0}")] + InvalidGroup(ValidateNameError), + #[error("Invalid endpoint attributes: {0}")] + InvalidAttributes(#[from] ValidateAttributesError), #[error("Remote error: {0}")] Remote(#[from] RemoteError), #[error("Connection error: {0}")] @@ -325,6 +419,45 @@ impl Client { set_name_inner(self.message_channel.clone(), name.into()).await } + /// Attach the active endpoint to a single named group cloud-side. + /// + /// Group names follow the same rules as endpoint names: any UTF-8 string, + /// minimum 2 bytes, maximum 128 bytes. **group uniqueness is not enforced.** + pub async fn set_group(&self, group: impl Into) -> Result<(), Error> { + set_group_inner(self.message_channel.clone(), group.into()).await + } + + /// Replace the arbitrary key-value attributes on the active endpoint cloud-side. + /// + /// Accepts any iterable of `(key, value)` pairs (arrays of tuples, `Vec`s, + /// `HashMap`s, `BTreeMap`s, etc.), so most calls fit on a single line: + /// + /// ```no_run + /// # use iroh_services::Client; + /// # async fn example(client: Client) -> anyhow::Result<()> { + /// client + /// .set_attributes([("env", "prod"), ("region", "us-west")]) + /// .await?; + /// # Ok(()) } + /// ``` + /// + /// Keys follow the same rules as endpoint names (2–128 bytes). Values may + /// be empty and are limited to 128 bytes. The map is limited to 128 + /// entries. Each call fully replaces the prior set; passing an empty + /// iterator clears all attributes. + pub async fn set_attributes(&self, attrs: I) -> Result<(), Error> + where + I: IntoIterator, + K: Into, + V: Into, + { + let collected: BTreeMap = attrs + .into_iter() + .map(|(k, v)| (k.into(), v.into())) + .collect(); + set_attributes_inner(self.message_channel.clone(), collected).await + } + /// Pings the remote node. pub async fn ping(&self) -> Result { let (tx, rx) = oneshot::channel(); @@ -429,12 +562,22 @@ enum ClientActorMessage { name: String, done: oneshot::Sender>, }, + SetGroup { + group: String, + done: oneshot::Sender>, + }, + SetAttributes { + attributes: BTreeMap, + done: oneshot::Sender>, + }, } struct ClientActor { capabilities: Rcan, client: IrohServicesClient, name: Option, + group: Option, + attributes: BTreeMap, session_id: Uuid, authorized: bool, } @@ -443,6 +586,8 @@ impl ClientActor { async fn run( mut self, initial_name: Option, + initial_group: Option, + initial_attributes: Option>, registry: Registry, interval: Option, mut inbox: tokio::sync::mpsc::Receiver, @@ -458,6 +603,18 @@ impl ClientActor { warn!(err = %err, "failed setting endpoint name on startup"); } + if let Some(group) = initial_group + && let Err(err) = self.send_set_group(group).await + { + warn!(err = %err, "failed setting endpoint group on startup"); + } + + if let Some(attributes) = initial_attributes + && let Err(err) = self.send_set_attributes(attributes).await + { + warn!(err = %err, "failed setting endpoint attributes on startup"); + } + loop { trace!("client actor tick"); tokio::select! { @@ -496,6 +653,18 @@ impl ClientActor { warn!("failed to name endpoint: {:#?}", err); } } + ClientActorMessage::SetGroup{ group, done } => { + let res = self.send_set_group(group).await; + if let Err(err) = done.send(res) { + warn!("failed to set group: {:#?}", err); + } + } + ClientActorMessage::SetAttributes{ attributes, done } => { + let res = self.send_set_attributes(attributes).await; + if let Err(err) = done.send(res) { + warn!("failed to set attributes: {:#?}", err); + } + } ClientActorMessage::PutNetworkDiagnostics{ report, done } => { let res = self.put_network_diagnostics(*report).await; if let Err(err) = done.send(res) { @@ -563,6 +732,39 @@ impl ClientActor { Ok(()) } + async fn send_set_group(&mut self, group: String) -> Result<(), RemoteError> { + trace!("client sending set group request"); + self.auth().await?; + + self.client + .rpc(SetGroup { + group: group.clone(), + }) + .await + .inspect_err(|e| debug!("set group error: {e}")) + .map_err(|_| RemoteError::InternalServerError)??; + self.group = Some(group); + Ok(()) + } + + async fn send_set_attributes( + &mut self, + attributes: BTreeMap, + ) -> Result<(), RemoteError> { + trace!("client sending set attributes request"); + self.auth().await?; + + self.client + .rpc(SetAttributes { + attributes: attributes.clone(), + }) + .await + .inspect_err(|e| debug!("set attributes error: {e}")) + .map_err(|_| RemoteError::InternalServerError)??; + self.attributes = attributes; + Ok(()) + } + async fn send_metrics(&mut self, encoder: &mut Encoder) -> Result<(), RemoteError> { trace!("client actor send metrics"); self.auth().await?; @@ -628,8 +830,45 @@ async fn set_name_inner( .map_err(Error::Remote) } +async fn set_group_inner( + message_channel: tokio::sync::mpsc::Sender, + group: String, +) -> Result<(), Error> { + validate_name(&group).map_err(Error::InvalidGroup)?; + debug!(group_len = group.len(), "calling set group"); + let (tx, rx) = oneshot::channel(); + message_channel + .send(ClientActorMessage::SetGroup { group, done: tx }) + .await + .map_err(|_| Error::Other(anyhow!("sending set group request")))?; + rx.await + .map_err(|e| Error::Other(anyhow!("response on internal channel: {:?}", e)))? + .map_err(Error::Remote) +} + +async fn set_attributes_inner( + message_channel: tokio::sync::mpsc::Sender, + attributes: BTreeMap, +) -> Result<(), Error> { + validate_attributes(&attributes)?; + debug!(attr_count = attributes.len(), "calling set attributes"); + let (tx, rx) = oneshot::channel(); + message_channel + .send(ClientActorMessage::SetAttributes { + attributes, + done: tx, + }) + .await + .map_err(|_| Error::Other(anyhow!("sending set attributes request")))?; + rx.await + .map_err(|e| Error::Other(anyhow!("response on internal channel: {:?}", e)))? + .map_err(Error::Remote) +} + #[cfg(test)] mod tests { + use std::collections::HashMap; + use iroh::{Endpoint, EndpointAddr, SecretKey, endpoint::presets}; use rand::{RngExt, SeedableRng}; use temp_env_vars::temp_env_vars; @@ -638,7 +877,10 @@ mod tests { Client, api_secret::ApiSecret, caps::{Cap, Caps}, - client::{API_SECRET_ENV_VAR_NAME, BuildError, ValidateNameError}, + client::{ + API_SECRET_ENV_VAR_NAME, BuildError, CLIENT_ATTRIBUTES_MAX_COUNT, + ValidateAttributesError, ValidateNameError, + }, }; #[tokio::test] @@ -725,4 +967,104 @@ mod tests { Some(BuildError::InvalidName(ValidateNameError::TooLong)) )); } + + #[tokio::test] + async fn test_group() { + let mut rng = rand::rngs::ChaCha8Rng::seed_from_u64(0); + let shared_secret = SecretKey::from_bytes(&rng.random()); + let fake_endpoint_id = SecretKey::from_bytes(&rng.random()).public(); + let api_secret = ApiSecret::new(shared_secret.clone(), fake_endpoint_id); + + let endpoint = Endpoint::builder(presets::Minimal).bind().await.unwrap(); + + let builder = Client::builder(&endpoint) + .group("staging") + .unwrap() + .api_secret(api_secret) + .unwrap(); + + assert_eq!(builder.group, Some("staging".to_string())); + + let Err(err) = Client::builder(&endpoint).group("a") else { + panic!("group should fail for strings under 2 bytes"); + }; + assert!(matches!( + err.downcast_ref::(), + Some(BuildError::InvalidGroup(ValidateNameError::TooShort)) + )); + + let too_long_group = "πŸ‘‹".repeat(129); + let Err(err) = Client::builder(&endpoint).group(&too_long_group) else { + panic!("group should fail for strings over 128 bytes"); + }; + assert!(matches!( + err.downcast_ref::(), + Some(BuildError::InvalidGroup(ValidateNameError::TooLong)) + )); + } + + #[tokio::test] + async fn test_attributes() { + let endpoint = Endpoint::builder(presets::Minimal).bind().await.unwrap(); + + // empty iterator is accepted (clears attributes server-side) + let builder = Client::builder(&endpoint) + .attributes(std::iter::empty::<(String, String)>()) + .unwrap(); + assert_eq!(builder.attributes.as_ref().map(|m| m.len()), Some(0)); + + // array literal of `&str` tuples β€” the one-liner ergonomics + let builder = Client::builder(&endpoint) + .attributes([("env", "prod"), ("region", "us-west")]) + .unwrap(); + let attrs = builder.attributes.as_ref().expect("attributes set"); + assert_eq!(attrs.get("env").map(String::as_str), Some("prod")); + assert_eq!(attrs.get("region").map(String::as_str), Some("us-west")); + + // HashMap also works + let mut map: HashMap = HashMap::new(); + map.insert("k1".into(), "v1".into()); + map.insert("k2".into(), "".into()); // empty value is allowed + let builder = Client::builder(&endpoint).attributes(map).unwrap(); + let attrs = builder.attributes.as_ref().expect("attributes set"); + assert_eq!(attrs.get("k2").map(String::as_str), Some("")); + + // value over 128 bytes errors + let too_long_value = "x".repeat(129); + let Err(err) = Client::builder(&endpoint).attributes([("ok", too_long_value.as_str())]) + else { + panic!("attributes should fail for value over 128 bytes"); + }; + assert!(matches!( + err.downcast_ref::(), + Some(BuildError::InvalidAttributes( + ValidateAttributesError::ValueTooLong + )) + )); + + // key under 2 bytes errors + let Err(err) = Client::builder(&endpoint).attributes([("a", "v")]) else { + panic!("attributes should fail for key under 2 bytes"); + }; + assert!(matches!( + err.downcast_ref::(), + Some(BuildError::InvalidAttributes( + ValidateAttributesError::InvalidKey(ValidateNameError::TooShort) + )) + )); + + // more than 128 entries errors + let big: Vec<(String, String)> = (0..(CLIENT_ATTRIBUTES_MAX_COUNT + 1)) + .map(|i| (format!("key_{i:04}"), format!("val_{i}"))) + .collect(); + let Err(err) = Client::builder(&endpoint).attributes(big) else { + panic!("attributes should fail for more than 128 entries"); + }; + assert!(matches!( + err.downcast_ref::(), + Some(BuildError::InvalidAttributes( + ValidateAttributesError::TooManyEntries + )) + )); + } } diff --git a/src/protocol.rs b/src/protocol.rs index 40cbbf0e..cc9c1a93 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeMap; + use anyhow::Result; use irpc::{channel::oneshot, rpc_requests}; use rcan::Rcan; @@ -30,6 +32,12 @@ pub enum IrohServicesProtocol { #[rpc(tx=oneshot::Sender>)] NameEndpoint(NameEndpoint), + + #[rpc(tx=oneshot::Sender>)] + SetGroup(SetGroup), + + #[rpc(tx=oneshot::Sender>)] + SetAttributes(SetAttributes), } /// Dedicated protocol for cloud-to-endpoint net diagnostics connections. @@ -104,3 +112,15 @@ pub struct GrantCap { pub struct NameEndpoint { pub name: String, } + +/// Attach the client endpoint to a single named group cloud-side. +#[derive(Debug, Serialize, Deserialize)] +pub struct SetGroup { + pub group: String, +} + +/// Replace the arbitrary key-value attributes on the client endpoint cloud-side. +#[derive(Debug, Serialize, Deserialize)] +pub struct SetAttributes { + pub attributes: BTreeMap, +} From b3dfe9198ab668ffdbc90d192b7422d7ef1754c6 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 29 Jun 2026 12:50:18 +0200 Subject: [PATCH 2/7] feat: add RemoteError::InvalidInput --- src/protocol.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/protocol.rs b/src/protocol.rs index cc9c1a93..a3ab97b9 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -59,6 +59,8 @@ pub enum RemoteError { MissingCapability(Caps), #[error("Unauthorized: {}", _0)] AuthError(String), + #[error("Invalid input: {}", _0)] + InvalidInput(String), #[error("Internal server error")] InternalServerError, } From 8581e5a36a5b1b4c738e8accba583ee3d4922b78 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 29 Jun 2026 13:37:52 +0200 Subject: [PATCH 3/7] feat: add Client::set_attribute for single-value updates Add set_attribute(key, value) which merges a single entry into the endpoint's attributes (rather than replacing the whole map) and sends the full SetAttributes. Attributes remain a BTreeMap. --- examples/endpoint_meta.rs | 27 +++-- src/client.rs | 209 +++++++++++++++++++++++++++++++++++++- src/protocol.rs | 74 +++++++++++++- 3 files changed, 295 insertions(+), 15 deletions(-) diff --git a/examples/endpoint_meta.rs b/examples/endpoint_meta.rs index f9cdef43..a8295a1b 100644 --- a/examples/endpoint_meta.rs +++ b/examples/endpoint_meta.rs @@ -6,6 +6,8 @@ //! updated later through the `Client::set_*` methods. //! //! Run with: `IROH_SERVICES_API_SECRET=... cargo run --example endpoint_meta` +use std::time::Duration; + use iroh::{Endpoint, endpoint::presets}; use iroh_services::Client; @@ -15,7 +17,7 @@ async fn main() -> anyhow::Result<()> { let endpoint = Endpoint::bind(presets::N0).await?; - // Derive a unique name from the endpoint id so repeated runs don't collide + //> Derive a unique name from the endpoint id so repeated runs don't collide // in dashboards. In a real app this is typically a user id, machine name, // or other stable identifier from your application. let id = endpoint.id().to_string(); @@ -25,32 +27,35 @@ async fn main() -> anyhow::Result<()> { // immediately after authenticating with iroh-services. Validation errors // (e.g. name too long) surface here; transport errors during startup are // logged at `warn` level rather than failing the build. + let mut attrs = vec![]; + for i in 0..25 { + attrs.push((format!("my-thing: {i}"), i.to_string())); + } let client = Client::builder(&endpoint) .api_secret_from_env()? .name(name)? - .group("examples")? - .attributes([ - ("env", "dev"), - ("region", "us-west"), - ("role", "endpoint-meta-example"), - ])? + .group("staging")? + .attributes(attrs)? .build() .await?; client.ping().await?; println!("endpoint registered with initial metadata"); + tokio::time::sleep(Duration::from_millis(500)).await; + println!("updating endpoint metadata..."); + // Each metadata field can also be updated after construction. These calls // return explicit errors, unlike the builder which logs and continues. client.set_name("endpoint-meta-example-renamed").await?; - client.set_group("staging").await?; + client.set_group("production").await?; // set_attributes fully replaces the prior set on each call. Pass an empty // iterator to clear all attributes. - client - .set_attributes([("env", "staging"), ("region", "eu-central")]) - .await?; + client.set_attribute("version", "41.0.3").await?; println!("metadata updated"); + endpoint.close().await; + Ok(()) } diff --git a/src/client.rs b/src/client.rs index fa08b702..84da69f5 100644 --- a/src/client.rs +++ b/src/client.rs @@ -458,6 +458,22 @@ impl Client { set_attributes_inner(self.message_channel.clone(), collected).await } + /// Set (or replace) a single attribute, merging it into the endpoint's + /// existing attributes rather than replacing the whole set. Insertion + /// order is preserved β€” re-setting an existing key keeps its position. + /// Convenience over [`set_attributes`](Self::set_attributes) when you only + /// need to change one value. + /// + /// The key follows endpoint-name rules (2–128 bytes) and the value is + /// limited to 128 bytes. + pub async fn set_attribute( + &self, + key: impl Into, + value: impl Into, + ) -> Result<(), Error> { + set_attribute_inner(self.message_channel.clone(), key.into(), value.into()).await + } + /// Pings the remote node. pub async fn ping(&self) -> Result { let (tx, rx) = oneshot::channel(); @@ -570,6 +586,11 @@ enum ClientActorMessage { attributes: BTreeMap, done: oneshot::Sender>, }, + SetAttribute { + key: String, + value: String, + done: oneshot::Sender>, + }, } struct ClientActor { @@ -665,6 +686,16 @@ impl ClientActor { warn!("failed to set attributes: {:#?}", err); } } + ClientActorMessage::SetAttribute{ key, value, done } => { + // Merge into the current set, preserving insertion + // order (re-setting an existing key keeps its slot). + let mut merged = self.attributes.clone(); + merged.insert(key, value); + let res = self.send_set_attributes(merged).await; + if let Err(err) = done.send(res) { + warn!("failed to set attribute: {:#?}", err); + } + } ClientActorMessage::PutNetworkDiagnostics{ report, done } => { let res = self.put_network_diagnostics(*report).await; if let Err(err) = done.send(res) { @@ -865,6 +896,31 @@ async fn set_attributes_inner( .map_err(Error::Remote) } +async fn set_attribute_inner( + message_channel: tokio::sync::mpsc::Sender, + key: String, + value: String, +) -> Result<(), Error> { + // Validate the single entry the same way the full map is validated (key is + // name-shaped, value within the size limit). The merged-total count is + // enforced server-side. + let mut one = BTreeMap::new(); + one.insert(key.clone(), value.clone()); + validate_attributes(&one)?; + let (tx, rx) = oneshot::channel(); + message_channel + .send(ClientActorMessage::SetAttribute { + key, + value, + done: tx, + }) + .await + .map_err(|_| Error::Other(anyhow!("sending set attribute request")))?; + rx.await + .map_err(|e| Error::Other(anyhow!("response on internal channel: {:?}", e)))? + .map_err(Error::Remote) +} + #[cfg(test)] mod tests { use std::collections::HashMap; @@ -878,8 +934,9 @@ mod tests { api_secret::ApiSecret, caps::{Cap, Caps}, client::{ - API_SECRET_ENV_VAR_NAME, BuildError, CLIENT_ATTRIBUTES_MAX_COUNT, - ValidateAttributesError, ValidateNameError, + API_SECRET_ENV_VAR_NAME, BuildError, CLIENT_ATTRIBUTE_VALUE_MAX_LENGTH, + CLIENT_ATTRIBUTES_MAX_COUNT, CLIENT_NAME_MAX_LENGTH, Error, ValidateAttributesError, + ValidateNameError, }, }; @@ -1067,4 +1124,152 @@ mod tests { )) )); } + + /// Build a client with no reachable server, mirroring `test_no_metrics_interval`. + /// The runtime setters validate input locally before any network call, so + /// validation errors surface without a live server. + async fn build_serverless_client(seed: u64) -> Client { + let mut rng = rand::rngs::ChaCha8Rng::seed_from_u64(seed); + let shared_secret = SecretKey::from_bytes(&rng.random()); + let fake_endpoint_id = SecretKey::from_bytes(&rng.random()).public(); + let api_secret = ApiSecret::new(shared_secret, fake_endpoint_id); + + let endpoint = Endpoint::builder(presets::Minimal).bind().await.unwrap(); + + Client::builder(&endpoint) + .disable_metrics_interval() + .api_secret(api_secret) + .unwrap() + .build() + .await + .unwrap() + } + + /// Covers the runtime `Client::set_group` path the builder tests miss: + /// validation runs locally and returns `Error::InvalidGroup` without a server. + #[tokio::test] + async fn test_set_group_runtime_validation() { + let client = build_serverless_client(2).await; + + let err = client + .set_group("a") + .await + .expect_err("too-short group should fail validation"); + assert!(matches!( + err, + Error::InvalidGroup(ValidateNameError::TooShort) + )); + + let too_long = "x".repeat(CLIENT_NAME_MAX_LENGTH + 1); + let err = client + .set_group(too_long) + .await + .expect_err("too-long group should fail validation"); + assert!(matches!( + err, + Error::InvalidGroup(ValidateNameError::TooLong) + )); + } + + /// Covers the runtime `Client::set_attributes` path the builder tests miss: + /// validation runs locally and returns `Error::InvalidAttributes` without a server. + #[tokio::test] + async fn test_set_attributes_runtime_validation() { + let client = build_serverless_client(3).await; + + // key under 2 bytes + let err = client + .set_attributes([("a", "v")]) + .await + .expect_err("too-short attribute key should fail validation"); + assert!(matches!( + err, + Error::InvalidAttributes(ValidateAttributesError::InvalidKey( + ValidateNameError::TooShort + )) + )); + + // value over the max length + let too_long_value = "x".repeat(CLIENT_ATTRIBUTE_VALUE_MAX_LENGTH + 1); + let err = client + .set_attributes([("ok", too_long_value.as_str())]) + .await + .expect_err("too-long attribute value should fail validation"); + assert!(matches!( + err, + Error::InvalidAttributes(ValidateAttributesError::ValueTooLong) + )); + + // more entries than allowed + let big: Vec<(String, String)> = (0..(CLIENT_ATTRIBUTES_MAX_COUNT + 1)) + .map(|i| (format!("key_{i:04}"), format!("val_{i}"))) + .collect(); + let err = client + .set_attributes(big) + .await + .expect_err("too many attributes should fail validation"); + assert!(matches!( + err, + Error::InvalidAttributes(ValidateAttributesError::TooManyEntries) + )); + } + + #[tokio::test] + async fn test_set_attribute_runtime_validation() { + let client = build_serverless_client(7).await; + + // A bad single key is rejected before any network call. + let err = client + .set_attribute("a", "v") + .await + .expect_err("too-short attribute key should fail validation"); + assert!(matches!( + err, + Error::InvalidAttributes(ValidateAttributesError::InvalidKey( + ValidateNameError::TooShort + )) + )); + + // A valid single attribute passes validation, then reaches the remote + // layer (no server) and surfaces a remote error β€” proving set_attribute + // is wired through the actor/RPC path. + let err = client + .set_attribute("firmware", "2.1.0") + .await + .expect_err("no server: remote call must fail after validation passes"); + assert!(matches!(err, Error::Remote(_)), "got {err:?}"); + } + + /// Boundary "accepted" case for the runtime setter. Without a live server we + /// cannot assert success; instead we assert the input passes local validation + /// and the call proceeds to the (failing) remote layer, surfacing + /// `Error::Remote` rather than an `Error::InvalidAttributes` validation error. + #[tokio::test] + async fn test_set_attributes_runtime_boundary_accepted() { + let client = build_serverless_client(4).await; + + // value of exactly the max length is accepted by validation + let max_value = "x".repeat(CLIENT_ATTRIBUTE_VALUE_MAX_LENGTH); + let err = client + .set_attributes([("ok".to_string(), max_value)]) + .await + .expect_err("no server: remote call must fail after validation passes"); + assert!( + matches!(err, Error::Remote(_)), + "expected a remote error (validation accepted), got {err:?}" + ); + + // exactly CLIENT_ATTRIBUTES_MAX_COUNT entries is accepted by validation + let max_entries: Vec<(String, String)> = (0..CLIENT_ATTRIBUTES_MAX_COUNT) + .map(|i| (format!("key_{i:04}"), format!("val_{i}"))) + .collect(); + let err = client + .set_attributes(max_entries) + .await + .expect_err("no server: remote call must fail after validation passes"); + assert!( + matches!(err, Error::Remote(_)), + "expected a remote error (validation accepted), got {err:?}" + ); + } } diff --git a/src/protocol.rs b/src/protocol.rs index a3ab97b9..33763c6e 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -16,6 +16,7 @@ pub type IrohServicesClient = irpc::Client; #[rpc_requests(message = ServicesMessage)] #[derive(Debug, Serialize, Deserialize)] #[allow(clippy::large_enum_variant)] +#[non_exhaustive] pub enum IrohServicesProtocol { #[rpc(tx=oneshot::Sender<()>)] Auth(Auth), @@ -54,15 +55,22 @@ pub enum ClientHostProtocol { pub type RemoteResult = Result; #[derive(Clone, Serialize, Deserialize, thiserror::Error, Debug)] +#[non_exhaustive] pub enum RemoteError { + // The first three variants and their order are the v1 wire contract: postcard + // encodes enum variants by index, so a v1 client only decodes these and at + // their original positions. New variants MUST be appended after them, and the + // server must only send new variants in response to new (v2+) requests. #[error("Missing capability: {}", _0.to_strings().join(", "))] MissingCapability(Caps), #[error("Unauthorized: {}", _0)] AuthError(String), - #[error("Invalid input: {}", _0)] - InvalidInput(String), #[error("Internal server error")] InternalServerError, + #[error("Invalid input: {}", _0)] + InvalidInput(String), + #[error("Rate limit exceeded")] + RateLimited, } /// Authentication on first request @@ -126,3 +134,65 @@ pub struct SetGroup { pub struct SetAttributes { pub attributes: BTreeMap, } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use super::{RemoteError, SetAttributes, SetGroup}; + use crate::client::CLIENT_ATTRIBUTE_VALUE_MAX_LENGTH; + + #[test] + fn test_remote_error_wire_compat() { + // postcard encodes enum variants by their index. v1 clients only know + // the first three RemoteError variants, so these indices are a frozen + // wire contract; new variants must be appended after them. + let idx = |e: &RemoteError| postcard::to_stdvec(e).expect("encode")[0]; + assert_eq!(idx(&RemoteError::AuthError(String::new())), 1); + assert_eq!(idx(&RemoteError::InternalServerError), 2); + // v2+ variants, appended after the v1 set. + assert_eq!(idx(&RemoteError::InvalidInput(String::new())), 3); + assert_eq!(idx(&RemoteError::RateLimited), 4); + } + + // The wire format used by irpc (and elsewhere in this crate, see + // `api_secret.rs`) is postcard. These round-trips pin the on-the-wire + // contract these messages share with the server. + + #[test] + fn test_set_group_serde_roundtrip() { + // a normal group, plus a unicode group for good measure + for group in ["staging", "my-group πŸ‘‹"] { + let msg = SetGroup { + group: group.to_string(), + }; + let bytes = postcard::to_stdvec(&msg).expect("postcard serialize"); + let decoded: SetGroup = postcard::from_bytes(&bytes).expect("postcard deserialize"); + assert_eq!(decoded.group, msg.group); + } + } + + #[test] + fn test_set_attributes_serde_roundtrip() { + // empty map: the documented "clear" case + let empty = SetAttributes { + attributes: BTreeMap::new(), + }; + let bytes = postcard::to_stdvec(&empty).expect("postcard serialize"); + let decoded: SetAttributes = postcard::from_bytes(&bytes).expect("postcard deserialize"); + assert!(decoded.attributes.is_empty()); + assert_eq!(decoded.attributes, empty.attributes); + + // unicode key/value plus a value at exactly the documented max length + let mut attributes = BTreeMap::new(); + attributes.insert("rΓ©gion 🌍".to_string(), "us-wΓ«st πŸš€".to_string()); + let max_value = "x".repeat(CLIENT_ATTRIBUTE_VALUE_MAX_LENGTH); + assert_eq!(max_value.len(), CLIENT_ATTRIBUTE_VALUE_MAX_LENGTH); + attributes.insert("max".to_string(), max_value); + + let msg = SetAttributes { attributes }; + let bytes = postcard::to_stdvec(&msg).expect("postcard serialize"); + let decoded: SetAttributes = postcard::from_bytes(&bytes).expect("postcard deserialize"); + assert_eq!(decoded.attributes, msg.attributes); + } +} From bfe111b7d32e969b3948f079ab2427900395d5d8 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 29 Jun 2026 17:31:25 +0200 Subject: [PATCH 4/7] chore: update anyhow to clear RUSTSEC-2026-0190 --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7f31915c..1bd49f22 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -63,9 +63,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.102" +version = "1.0.103" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +checksum = "2a4385e2e34eb35d6b3efe798b9eb88096925d87726c0798709bf56d9ed84af3" [[package]] name = "arc-swap" From dbf502b9d3e934f040be9b09e5eedd3ca6626878 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Wed, 1 Jul 2026 12:31:04 +0200 Subject: [PATCH 5/7] fix(client): address PR review feedback - set_attribute validates the *merged* set in the actor: a valid single entry can still push a full set over the max-entry limit, so the single-entry check was insufficient. Surfaces InvalidAttributes(TooManyEntries) locally. - Read initial name/group/attributes from the actor's own state instead of passing them to run() again; add ReadGroup + Client::group() to mirror name. - Docs: single-sentence style, state the 2-128 byte rules directly, drop the misleading insertion-order / group-uniqueness / same-as-names claims, and clarify that builder validation errors return immediately while startup-send failures are logged (use set_* for explicit handling). - Log the group (not just its length) in the set-group debug line. --- src/client.rs | 173 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 114 insertions(+), 59 deletions(-) diff --git a/src/client.rs b/src/client.rs index 84da69f5..508e80e6 100644 --- a/src/client.rs +++ b/src/client.rs @@ -112,20 +112,17 @@ impl ClientBuilder { self } - /// Set an optional human-readable name for the endpoint the client is - /// constructed with, making metrics from this endpoint easier to identify. - /// This is often used for associating with other services in your app, - /// like a database user id, machine name, permanent username, etc. + /// Set an optional human-readable name for the endpoint, making its metrics + /// easier to identify. /// - /// When this builder method is called, the provided name is sent after the - /// client initially authenticates the endpoint server-side. - /// Errors will not interrupt client construction, instead producing a - /// warn-level log. For explicit error handling, use [`Client::set_name`]. + /// Often a database user id, machine name, or other stable identifier from + /// your application. A name must be 2–128 bytes of UTF-8; uniqueness is not + /// enforced, so different endpoints may share a name. /// - /// names can be any UTF-8 string, with a min length of 2 bytes, and - /// maximum length of 128 bytes. **name uniqueness is not enforced - /// server-side**, which means using the same name for different endpoints - /// will not produce an error + /// Validation errors are returned here. The name is sent to the server after + /// the client authenticates; a failure to send it at that point is logged at + /// warn level rather than returned β€” use [`Client::set_name`] to set it later + /// with explicit error handling. pub fn name(mut self, name: impl Into) -> Result { let name = name.into(); validate_name(&name).map_err(BuildError::InvalidName)?; @@ -134,9 +131,13 @@ impl ClientBuilder { } /// Attach the endpoint to a single named group when the client first - /// authenticates. Group names follow the same rules as endpoint names - /// (2–128 bytes UTF-8). Errors during startup propagate as warn-level - /// logs; for explicit error handling use [`Client::set_group`]. + /// authenticates. + /// + /// A group name must be 2–128 bytes of UTF-8. Validation errors are returned + /// here. The group is sent to the server after the client authenticates; a + /// failure to send it at that point is logged at warn level rather than + /// returned β€” use [`Client::set_group`] to set it later with explicit error + /// handling. pub fn group(mut self, group: impl Into) -> Result { let group = group.into(); validate_name(&group).map_err(BuildError::InvalidGroup)?; @@ -155,10 +156,12 @@ impl ClientBuilder { /// # Ok(()) } /// ``` /// - /// Keys follow the same length rules as endpoint names (2–128 bytes); - /// values may be empty and are capped at 128 bytes; the map is limited - /// to 128 entries. Errors during startup propagate as warn-level logs; - /// for explicit error handling use [`Client::set_attributes`]. + /// Each key must be 2–128 bytes of UTF-8; values may be empty and are capped + /// at 128 bytes; at most 128 entries are allowed. Validation errors are + /// returned here. The attributes are sent to the server after the client + /// authenticates; a failure to send them at that point is logged at warn + /// level rather than returned β€” use [`Client::set_attributes`] to set them + /// later with explicit error handling. pub fn attributes(mut self, attrs: I) -> Result where I: IntoIterator, @@ -267,14 +270,7 @@ impl ClientBuilder { session_id: Uuid::new_v4(), authorized: false, } - .run( - self.name, - self.group, - self.attributes, - self.registry, - self.metrics_interval, - rx, - ), + .run(self.registry, self.metrics_interval, rx), )); Ok(Client { @@ -411,6 +407,18 @@ impl Client { .map_err(|e| Error::Other(anyhow!("response on internal channel: {:?}", e))) } + /// Read the current endpoint group from the local client. + pub async fn group(&self) -> Result, Error> { + let (tx, rx) = oneshot::channel(); + self.message_channel + .send(ClientActorMessage::ReadGroup { done: tx }) + .await + .map_err(|_| Error::Other(anyhow!("sending group read request")))?; + + rx.await + .map_err(|e| Error::Other(anyhow!("response on internal channel: {:?}", e))) + } + /// Name the active endpoint cloud-side. /// /// names can be any UTF-8 string, with a min length of 2 bytes, and @@ -421,8 +429,7 @@ impl Client { /// Attach the active endpoint to a single named group cloud-side. /// - /// Group names follow the same rules as endpoint names: any UTF-8 string, - /// minimum 2 bytes, maximum 128 bytes. **group uniqueness is not enforced.** + /// A group name must be 2–128 bytes of UTF-8. pub async fn set_group(&self, group: impl Into) -> Result<(), Error> { set_group_inner(self.message_channel.clone(), group.into()).await } @@ -441,10 +448,9 @@ impl Client { /// # Ok(()) } /// ``` /// - /// Keys follow the same rules as endpoint names (2–128 bytes). Values may - /// be empty and are limited to 128 bytes. The map is limited to 128 - /// entries. Each call fully replaces the prior set; passing an empty - /// iterator clears all attributes. + /// Each key must be 2–128 bytes of UTF-8; values may be empty and are limited + /// to 128 bytes; at most 128 entries are allowed. Each call fully replaces + /// the prior set; passing an empty iterator clears all attributes. pub async fn set_attributes(&self, attrs: I) -> Result<(), Error> where I: IntoIterator, @@ -458,14 +464,12 @@ impl Client { set_attributes_inner(self.message_channel.clone(), collected).await } - /// Set (or replace) a single attribute, merging it into the endpoint's - /// existing attributes rather than replacing the whole set. Insertion - /// order is preserved β€” re-setting an existing key keeps its position. - /// Convenience over [`set_attributes`](Self::set_attributes) when you only - /// need to change one value. + /// Set or replace a single attribute, merging it into the endpoint's existing + /// attributes rather than replacing the whole set. /// - /// The key follows endpoint-name rules (2–128 bytes) and the value is - /// limited to 128 bytes. + /// A convenience over [`set_attributes`](Self::set_attributes) when you only + /// need to change one value. The key must be 2–128 bytes of UTF-8 and the + /// value is limited to 128 bytes; the merged set must stay within 128 entries. pub async fn set_attribute( &self, key: impl Into, @@ -574,6 +578,9 @@ enum ClientActorMessage { ReadName { done: oneshot::Sender>, }, + ReadGroup { + done: oneshot::Sender>, + }, NameEndpoint { name: String, done: oneshot::Sender>, @@ -589,7 +596,10 @@ enum ClientActorMessage { SetAttribute { key: String, value: String, - done: oneshot::Sender>, + // Carries the full client `Error` (not just `RemoteError`) because the + // merged-set validation happens in the actor, where the current set is + // known, and can fail with a local `InvalidAttributes` error. + done: oneshot::Sender>, }, } @@ -606,9 +616,6 @@ struct ClientActor { impl ClientActor { async fn run( mut self, - initial_name: Option, - initial_group: Option, - initial_attributes: Option>, registry: Registry, interval: Option, mut inbox: tokio::sync::mpsc::Receiver, @@ -618,20 +625,22 @@ impl ClientActor { let mut metrics_timer = interval.map(|interval| n0_future::time::interval(interval)); trace!("starting client actor"); - if let Some(name) = initial_name + // Send the initial metadata (set via the builder) once the actor starts. + // These live on `self`; a send failure here is logged, not fatal. + if let Some(name) = self.name.clone() && let Err(err) = self.send_name_endpoint(name).await { warn!(err = %err, "failed setting endpoint name on startup"); } - if let Some(group) = initial_group + if let Some(group) = self.group.clone() && let Err(err) = self.send_set_group(group).await { warn!(err = %err, "failed setting endpoint group on startup"); } - if let Some(attributes) = initial_attributes - && let Err(err) = self.send_set_attributes(attributes).await + if !self.attributes.is_empty() + && let Err(err) = self.send_set_attributes(self.attributes.clone()).await { warn!(err = %err, "failed setting endpoint attributes on startup"); } @@ -668,6 +677,11 @@ impl ClientActor { warn!("sending name value: {:#?}", err); } } + ClientActorMessage::ReadGroup{ done } => { + if let Err(err) = done.send(self.group.clone()) { + warn!("sending group value: {:#?}", err); + } + } ClientActorMessage::NameEndpoint{ name, done } => { let res = self.send_name_endpoint(name).await; if let Err(err) = done.send(res) { @@ -687,11 +701,18 @@ impl ClientActor { } } ClientActorMessage::SetAttribute{ key, value, done } => { - // Merge into the current set, preserving insertion - // order (re-setting an existing key keeps its slot). + // Merge into the current set and validate the union: + // adding one valid entry to a valid set can still + // exceed the max entry count, so the single entry + // being valid is not enough. let mut merged = self.attributes.clone(); merged.insert(key, value); - let res = self.send_set_attributes(merged).await; + let res = match validate_attributes(&merged) { + Ok(()) => { + self.send_set_attributes(merged).await.map_err(Error::Remote) + } + Err(err) => Err(Error::from(err)), + }; if let Err(err) = done.send(res) { warn!("failed to set attribute: {:#?}", err); } @@ -866,7 +887,7 @@ async fn set_group_inner( group: String, ) -> Result<(), Error> { validate_name(&group).map_err(Error::InvalidGroup)?; - debug!(group_len = group.len(), "calling set group"); + debug!(%group, "calling set group"); let (tx, rx) = oneshot::channel(); message_channel .send(ClientActorMessage::SetGroup { group, done: tx }) @@ -901,12 +922,9 @@ async fn set_attribute_inner( key: String, value: String, ) -> Result<(), Error> { - // Validate the single entry the same way the full map is validated (key is - // name-shaped, value within the size limit). The merged-total count is - // enforced server-side. - let mut one = BTreeMap::new(); - one.insert(key.clone(), value.clone()); - validate_attributes(&one)?; + // Validation happens in the actor against the merged set (current attributes + // plus this entry), since only there is the current set known β€” merging can + // exceed the entry-count limit even when this single entry is valid. let (tx, rx) = oneshot::channel(); message_channel .send(ClientActorMessage::SetAttribute { @@ -918,7 +936,6 @@ async fn set_attribute_inner( .map_err(|_| Error::Other(anyhow!("sending set attribute request")))?; rx.await .map_err(|e| Error::Other(anyhow!("response on internal channel: {:?}", e)))? - .map_err(Error::Remote) } #[cfg(test)] @@ -1240,6 +1257,44 @@ mod tests { assert!(matches!(err, Error::Remote(_)), "got {err:?}"); } + #[tokio::test] + async fn test_set_attribute_merge_over_limit_rejected() { + // A client already holding the maximum number of attributes. + let full: Vec<(String, String)> = (0..CLIENT_ATTRIBUTES_MAX_COUNT) + .map(|i| (format!("key_{i:04}"), "v".to_string())) + .collect(); + + let mut rng = rand::rngs::ChaCha8Rng::seed_from_u64(9); + let shared_secret = SecretKey::from_bytes(&rng.random()); + let fake_endpoint_id = SecretKey::from_bytes(&rng.random()).public(); + let api_secret = ApiSecret::new(shared_secret, fake_endpoint_id); + let endpoint = Endpoint::builder(presets::Minimal).bind().await.unwrap(); + let client = Client::builder(&endpoint) + .disable_metrics_interval() + .attributes(full) + .unwrap() + .api_secret(api_secret) + .unwrap() + .build() + .await + .unwrap(); + + // Merging one more (individually valid) entry pushes the set over the + // limit. The single-entry check would miss this; the merged-set check in + // the actor catches it locally, before any network call. + let err = client + .set_attribute("one-too-many", "v") + .await + .expect_err("merging past the attribute limit must fail"); + assert!( + matches!( + err, + Error::InvalidAttributes(ValidateAttributesError::TooManyEntries) + ), + "expected TooManyEntries, got {err:?}" + ); + } + /// Boundary "accepted" case for the runtime setter. Without a live server we /// cannot assert success; instead we assert the input passes local validation /// and the call proceeds to the (failing) remote layer, surfacing From a973cf306cfce12ae55de71c7be4da70bea25425 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Wed, 1 Jul 2026 12:34:10 +0200 Subject: [PATCH 6/7] docs(protocol): document the append-only ALPN versioning policy --- src/protocol.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/protocol.rs b/src/protocol.rs index 33763c6e..5d4c97bf 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -9,10 +9,33 @@ use uuid::Uuid; use crate::{caps::Caps, net_diagnostics::DiagnosticsReport}; /// The main ALPN for connecting from the client to the cloud node. +/// +/// # Versioning policy +/// +/// The wire protocol evolves **append-only** and does not bump the version in +/// this ALPN for additive changes. postcard encodes enum variants by index, so +/// as long as new [`IrohServicesProtocol`] request variants and new +/// [`RemoteError`] variants are only ever *appended* (never inserted, reordered, +/// or removed), every previously-defined message stays wire-compatible. That +/// gives the following compatibility guarantees: +/// +/// - **Old client β†’ new server: fully compatible.** The server decodes every +/// request an older client can send and only ever replies with error variants +/// that client already knows. +/// - **New client β†’ old server:** the pre-existing requests (auth, metrics, …) +/// still work; a *new* request the old server doesn't know fails as a +/// per-request error rather than corrupting the connection or other traffic. +/// +/// Because the cloud node is deployed at or ahead of the clients that talk to it, +/// the second case is transient and confined to the new calls. A *breaking* +/// change β€” reordering or removing variants, or changing a message's shape β€” +/// would instead require a new ALPN. pub const ALPN: &[u8] = b"/iroh/n0des/1"; pub type IrohServicesClient = irpc::Client; +/// New request variants MUST be appended, never inserted or reordered β€” see the +/// versioning policy on [`ALPN`]. #[rpc_requests(message = ServicesMessage)] #[derive(Debug, Serialize, Deserialize)] #[allow(clippy::large_enum_variant)] From 3a54c3e23ac033da3f3445164bcd88a55ec43be3 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Wed, 1 Jul 2026 12:41:52 +0200 Subject: [PATCH 7/7] docs: plain-text comments (no em/en dashes); name limits are bytes, not characters Reword the versioning doc and cleanups without em dashes, en dashes, ellipses, or arrows. Fix the ValidateNameError messages to say bytes, matching the actual byte-length (name.len()) check and the doc wording. --- src/client.rs | 28 ++++++++++++++-------------- src/protocol.rs | 34 ++++++++++++++++------------------ 2 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/client.rs b/src/client.rs index 508e80e6..effcb2c4 100644 --- a/src/client.rs +++ b/src/client.rs @@ -116,12 +116,12 @@ impl ClientBuilder { /// easier to identify. /// /// Often a database user id, machine name, or other stable identifier from - /// your application. A name must be 2–128 bytes of UTF-8; uniqueness is not + /// your application. A name must be 2 to 128 bytes of UTF-8; uniqueness is not /// enforced, so different endpoints may share a name. /// /// Validation errors are returned here. The name is sent to the server after /// the client authenticates; a failure to send it at that point is logged at - /// warn level rather than returned β€” use [`Client::set_name`] to set it later + /// warn level rather than returned; use [`Client::set_name`] to set it later /// with explicit error handling. pub fn name(mut self, name: impl Into) -> Result { let name = name.into(); @@ -133,10 +133,10 @@ impl ClientBuilder { /// Attach the endpoint to a single named group when the client first /// authenticates. /// - /// A group name must be 2–128 bytes of UTF-8. Validation errors are returned + /// A group name must be 2 to 128 bytes of UTF-8. Validation errors are returned /// here. The group is sent to the server after the client authenticates; a /// failure to send it at that point is logged at warn level rather than - /// returned β€” use [`Client::set_group`] to set it later with explicit error + /// returned; use [`Client::set_group`] to set it later with explicit error /// handling. pub fn group(mut self, group: impl Into) -> Result { let group = group.into(); @@ -156,11 +156,11 @@ impl ClientBuilder { /// # Ok(()) } /// ``` /// - /// Each key must be 2–128 bytes of UTF-8; values may be empty and are capped + /// Each key must be 2 to 128 bytes of UTF-8; values may be empty and are capped /// at 128 bytes; at most 128 entries are allowed. Validation errors are /// returned here. The attributes are sent to the server after the client /// authenticates; a failure to send them at that point is logged at warn - /// level rather than returned β€” use [`Client::set_attributes`] to set them + /// level rather than returned; use [`Client::set_attributes`] to set them /// later with explicit error handling. pub fn attributes(mut self, attrs: I) -> Result where @@ -327,9 +327,9 @@ pub const CLIENT_NAME_MAX_LENGTH: usize = 128; /// Error returned when an endpoint name fails validation. #[derive(Debug, thiserror::Error)] pub enum ValidateNameError { - #[error("Name is too long (must be no more than {CLIENT_NAME_MAX_LENGTH} characters).")] + #[error("Name is too long (must be no more than {CLIENT_NAME_MAX_LENGTH} bytes).")] TooLong, - #[error("Name is too short (must be at least {CLIENT_NAME_MIN_LENGTH} characters).")] + #[error("Name is too short (must be at least {CLIENT_NAME_MIN_LENGTH} bytes).")] TooShort, } @@ -429,7 +429,7 @@ impl Client { /// Attach the active endpoint to a single named group cloud-side. /// - /// A group name must be 2–128 bytes of UTF-8. + /// A group name must be 2 to 128 bytes of UTF-8. pub async fn set_group(&self, group: impl Into) -> Result<(), Error> { set_group_inner(self.message_channel.clone(), group.into()).await } @@ -448,7 +448,7 @@ impl Client { /// # Ok(()) } /// ``` /// - /// Each key must be 2–128 bytes of UTF-8; values may be empty and are limited + /// Each key must be 2 to 128 bytes of UTF-8; values may be empty and are limited /// to 128 bytes; at most 128 entries are allowed. Each call fully replaces /// the prior set; passing an empty iterator clears all attributes. pub async fn set_attributes(&self, attrs: I) -> Result<(), Error> @@ -468,7 +468,7 @@ impl Client { /// attributes rather than replacing the whole set. /// /// A convenience over [`set_attributes`](Self::set_attributes) when you only - /// need to change one value. The key must be 2–128 bytes of UTF-8 and the + /// need to change one value. The key must be 2 to 128 bytes of UTF-8 and the /// value is limited to 128 bytes; the merged set must stay within 128 entries. pub async fn set_attribute( &self, @@ -923,7 +923,7 @@ async fn set_attribute_inner( value: String, ) -> Result<(), Error> { // Validation happens in the actor against the merged set (current attributes - // plus this entry), since only there is the current set known β€” merging can + // plus this entry), since only there is the current set known. Merging can // exceed the entry-count limit even when this single entry is valid. let (tx, rx) = oneshot::channel(); message_channel @@ -1087,7 +1087,7 @@ mod tests { .unwrap(); assert_eq!(builder.attributes.as_ref().map(|m| m.len()), Some(0)); - // array literal of `&str` tuples β€” the one-liner ergonomics + // array literal of `&str` tuples, for the one-liner ergonomics let builder = Client::builder(&endpoint) .attributes([("env", "prod"), ("region", "us-west")]) .unwrap(); @@ -1248,7 +1248,7 @@ mod tests { )); // A valid single attribute passes validation, then reaches the remote - // layer (no server) and surfaces a remote error β€” proving set_attribute + // layer (no server) and surfaces a remote error, proving set_attribute // is wired through the actor/RPC path. let err = client .set_attribute("firmware", "2.1.0") diff --git a/src/protocol.rs b/src/protocol.rs index 5d4c97bf..f2bcf844 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -10,31 +10,29 @@ use crate::{caps::Caps, net_diagnostics::DiagnosticsReport}; /// The main ALPN for connecting from the client to the cloud node. /// -/// # Versioning policy +/// # Versioning /// -/// The wire protocol evolves **append-only** and does not bump the version in -/// this ALPN for additive changes. postcard encodes enum variants by index, so -/// as long as new [`IrohServicesProtocol`] request variants and new -/// [`RemoteError`] variants are only ever *appended* (never inserted, reordered, -/// or removed), every previously-defined message stays wire-compatible. That -/// gives the following compatibility guarantees: +/// The wire protocol is append-only and does not bump this ALPN for additive +/// changes. postcard encodes enum variants by index, so as long as new +/// [`IrohServicesProtocol`] and [`RemoteError`] variants are only appended +/// (never inserted, reordered, or removed), older messages stay wire-compatible: /// -/// - **Old client β†’ new server: fully compatible.** The server decodes every -/// request an older client can send and only ever replies with error variants -/// that client already knows. -/// - **New client β†’ old server:** the pre-existing requests (auth, metrics, …) -/// still work; a *new* request the old server doesn't know fails as a -/// per-request error rather than corrupting the connection or other traffic. +/// - An older client always works against a newer server: the server decodes +/// every request the client can send, and only replies with error variants the +/// client already knows. +/// - A newer client against an older server keeps working for the pre-existing +/// requests (auth, metrics, and so on); a request the old server does not know +/// fails as a per-request error rather than breaking the connection. /// -/// Because the cloud node is deployed at or ahead of the clients that talk to it, -/// the second case is transient and confined to the new calls. A *breaking* -/// change β€” reordering or removing variants, or changing a message's shape β€” -/// would instead require a new ALPN. +/// The cloud node is deployed at or ahead of the clients that talk to it, so the +/// second case is transient and limited to the new calls. A breaking change +/// (reordering or removing variants, or changing a message's shape) requires a +/// new ALPN. pub const ALPN: &[u8] = b"/iroh/n0des/1"; pub type IrohServicesClient = irpc::Client; -/// New request variants MUST be appended, never inserted or reordered β€” see the +/// New request variants MUST be appended, never inserted or reordered. See the /// versioning policy on [`ALPN`]. #[rpc_requests(message = ServicesMessage)] #[derive(Debug, Serialize, Deserialize)]