feat: Implement bidirectional ProtocolNode types for prekey responses - #250
Merged
Conversation
- Added SignedPreKeyNode and OneTimePreKeyNode structs for handling signed and one-time prekeys. - Introduced PreKeyBundleUserNode to represent complete prekey bundle responses. - Implemented ProtocolNode trait for these types to facilitate XML serialization and deserialization. - Added helper functions for byte manipulation related to prekey IDs. - Enhanced tests for new prekey structures and their round-trip serialization. feat: Extend A/B property handling in props module - Introduced SamplingProp struct for A/B sampling properties. - Updated AbPropConfig enum to include both AbProp and SamplingProp. - Modified PropsResponse to handle a vector of AbPropConfig instead of just AbProp. - Enhanced parsing logic in PropsSpec to utilize the new ProtocolNode implementations. - Added tests for new A/B property structures and their serialization. feat: Add message stanza types with ProtocolNode derive macro - Created MessageStanza struct for typed message stanzas with JID-aware attributes. - Implemented ProtocolNode derive macro for MessageStanza to facilitate XML serialization. - Updated stanza module to include message handling. fix: Update lib.rs to use self as wacore
📝 WalkthroughWalkthroughAdds JID-aware ProtocolNode derive behavior, new ProtocolNode types for prekeys and media connections, a type-safe MessageStanza, and switches prekey retry handling to node-based representations across the codebase. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bencher Report![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/retry.rs (1)
569-575: Usedevice_snapshot.signed_pre_key_idinstead of hardcoding1.The signed prekey ID is already stored in
device_snapshot(inherited from the Device struct). Hardcoding it to1will cause a mismatch with the actual signed prekey ID that was generated and stored for this device, leading the recipient to cache or reference the wrong key. Pull the value from the snapshot like you do for other device attributes:let skey_id = device_snapshot.signed_pre_key_id;
🤖 Fix all issues with AI agents
In `@src/retry.rs`:
- Around line 424-431: The code silently drops malformed <key> entries by
converting Results to Options with .ok(); change the logic so that if
keys_node.get_optional_child("key") returns Some(key_node) then call
OneTimePreKeyNode::try_from_node(key_node) and propagate its Err, and likewise
propagate errors from PublicKey::from_djb_public_key_bytes instead of using
.ok()/ .ok()?; update the surrounding function signature to return Result so
prekey_data becomes a Result<Option<(u64, PublicKey)>, YourErrorType>, and
return Err when try_from_node or from_djb_public_key_bytes fail (refer to
symbols keys_node, OneTimePreKeyNode::try_from_node,
PublicKey::from_djb_public_key_bytes, and prekey_data).
In `@wacore/src/iq/prekeys.rs`:
- Around line 346-381: In try_from_node ensure the parsed signed-prekey's value
and signature are validated for expected lengths before returning: after
extracting public_bytes from value_node and signature from signature node (in
try_from_node) add explicit checks (e.g., public_bytes.len() == 32 and
signature.len() == 64 or whatever the protocol requires) and return an anyhow!
error if they don't match; apply the same pattern to the one-time prekey parsing
code paths (the counterpart functions handling <prekey> nodes) so malformed
nodes fail fast and downstream code can safely assume 32/64-byte sizes.
- Around line 597-623: The optional-node parsing silently swallows malformed
`<key>` and `<device-identity>` nodes and the `identity_key` isn't validated for
length; change parsing so that if node.get_optional_child("key") or
node.get_optional_child("device-identity") returns Some(n) you attempt strict
parsing and propagate any error (i.e. call OneTimePreKeyNode::try_from_node(n)
and return Err on failure rather than .ok()), and for `device-identity` if
present require NodeContent::Bytes else return an error instead of None; also
validate `identity_key` length equals 32 bytes after extracting it (return an
error via anyhow! if not 32). Ensure you update the same block that uses
required_child/node.get_optional_child, identity_key,
SignedPreKeyNode::try_from_node, OneTimePreKeyNode::try_from_node and the
device-identity match to follow this strict behavior.
🧹 Nitpick comments (4)
wacore/src/iq/mediaconn.rs (1)
67-89: Remove unnecessary.clone()onhostname.The
hostnameparameter is already an ownedStringand is only used once, so the clone is redundant.♻️ Suggested fix
pub fn primary( hostname: String, fallback_hostname: String, ip4: String, ip6: String, download_categories: Vec<String>, download_buckets: Vec<String>, ) -> Self { Self { - hostname: hostname.clone(), + hostname, host_type: "primary".to_string(), fallback_hostname: Some(fallback_hostname), ip4: Some(ip4.clone()), ip6: Some(ip6.clone()), fallback_ip4: Some(ip4), fallback_ip6: Some(ip6), upload: true, download: true, download_categories, download_buckets, } }wacore/derive/src/lib.rs (2)
215-222: Consider making the catch-all arm a compile-time error.The
_ => quote! {}arm silently produces no output, which could lead to confusing compilation errors downstream if this branch is ever reached due to a logic bug. Converting it to apanic!orunreachable!would provide clearer diagnostics during macro expansion.♻️ Suggested fix
(AttrType::String, false, Some(default)) => { quote! { `#field_ident`: `#default.to_string`() } } - _ => quote! {}, // This shouldn't happen if all_have_defaults is true + _ => unreachable!("all_have_defaults check should prevent this branch"), }
187-194: Clarify behavior: optional string with default never returnsNone.When both
optionalanddefaultare specified, the generated code always producesSome(...), neverNone. If this is intentional (meaning "optional in XML but always has a value in Rust"), consider documenting this in the macro's doc comment. Otherwise, users might expectNoneto be possible.wacore/src/iq/prekeys.rs (1)
289-300: Guard against out-of-range prekey IDs before truncation.
truncate_to_3bytessilently drops the high byte; if a caller ever passes> 0x00FF_FFFF, the serialized node won’t match the in-memory id. Consider a lightweight guard to catch misuse early.🔧 Suggested guard
fn truncate_to_3bytes(id: u32) -> Vec<u8> { + debug_assert!( + id <= 0x00FF_FFFF, + "prekey id exceeds 3-byte range: {id}" + ); id.to_be_bytes()[1..].to_vec() }
jlucaso1
force-pushed
the
feat-improve-iq-stanza
branch
from
January 25, 2026 23:24
af3c352 to
2a52187
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/client.rs`:
- Around line 727-729: The current check using is_connected() (which uses
try_lock) can falsely return false under transient contention and drop ACKs;
change this to use an async-aware lock check instead of try_lock — either make
is_connected async and await the lock or directly acquire the async lock (e.g.,
await the same connection/state mutex/read-write lock used elsewhere) before
checking connectivity and expected_disconnect, so the ACK path isn't skipped due
to brief contention; update the call site where is_connected() is used and
ensure expected_disconnect.load(Ordering::Relaxed) is still evaluated safely
after acquiring the async lock (references: is_connected(),
expected_disconnect).
🧹 Nitpick comments (2)
wacore/derive/src/lib.rs (1)
368-409: Add explicit validation foroptional/defaultattribute combos.Right now
#[attr(optional)]on a non-Option<T>field ordefaulton ajidattribute will fail with generated-code type errors. Consider catching these inextract_attr_infoand returning a targetedsyn::Errorto make misuse clearer.♻️ Possible validation guard
match attr_name { Some(name) => { let attr_type = if is_jid { AttrType::Jid } else { AttrType::String }; // Determine if optional: either explicit marker or Option<T> type let optional = explicit_optional || is_optional; + if explicit_optional && !is_optional { + return Err(syn::Error::new_spanned( + &field.ty, + "`optional` requires an Option<T> field type", + )); + } + if is_jid && default.is_some() { + return Err(syn::Error::new_spanned( + attr, + "`default` is only supported for String attributes", + )); + } + return Ok(Some(AttrFieldInfo { field_ident, attr_name: name, attr_type, optional, default, }));wacore/src/iq/mediaconn.rs (1)
162-197: Userequired_attrfor required attributes.
hostnameandauthare mandatory; using the helper keeps error messages consistent and aligns with parsing patterns elsewhere inwacore/src/iq. As per coding guidelines, prefer the shared node helpers for required fields.🔧 Suggested adjustment
+use crate::iq::node::required_attr;- let hostname = attrs - .optional_string("hostname") - .ok_or_else(|| anyhow!("missing hostname attribute"))? - .to_string(); + let hostname = required_attr(node, "hostname")?.to_string();- let auth = attrs - .optional_string("auth") - .ok_or_else(|| anyhow!("missing auth attribute"))? - .to_string(); + let auth = required_attr(node, "auth")?.to_string();Also applies to: 266-275
Comment on lines
+727
to
+729
| if !self.is_connected() || self.expected_disconnect.load(Ordering::Relaxed) { | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
Avoid dropping ACKs on transient lock contention.
Line 727 uses is_connected(), which relies on try_lock; a briefly held lock will return false and skip the ACK entirely. Prefer an async lock check here so ACKs aren’t silently dropped under load.
🛠️ Suggested fix
- if !self.is_connected() || self.expected_disconnect.load(Ordering::Relaxed) {
- return Ok(());
- }
+ if self.expected_disconnect.load(Ordering::Relaxed) {
+ return Ok(());
+ }
+ if self.noise_socket.lock().await.is_none() {
+ return Ok(());
+ }📝 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.
Suggested change
| if !self.is_connected() || self.expected_disconnect.load(Ordering::Relaxed) { | |
| return Ok(()); | |
| } | |
| if self.expected_disconnect.load(Ordering::Relaxed) { | |
| return Ok(()); | |
| } | |
| if self.noise_socket.lock().await.is_none() { | |
| return Ok(()); | |
| } |
🤖 Prompt for AI Agents
In `@src/client.rs` around lines 727 - 729, The current check using is_connected()
(which uses try_lock) can falsely return false under transient contention and
drop ACKs; change this to use an async-aware lock check instead of try_lock —
either make is_connected async and await the lock or directly acquire the async
lock (e.g., await the same connection/state mutex/read-write lock used
elsewhere) before checking connectivity and expected_disconnect, so the ACK path
isn't skipped due to brief contention; update the call site where is_connected()
is used and ensure expected_disconnect.load(Ordering::Relaxed) is still
evaluated safely after acquiring the async lock (references: is_connected(),
expected_disconnect).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@wacore/src/iq/prekeys.rs`:
- Around line 289-293: The helper truncate_to_3bytes currently uses
debug_assert!, which is stripped in release builds and allows silent truncation
of out-of-range prekey ids; change the guard to a hard runtime check (e.g.,
replace debug_assert! with assert! or an explicit if that panics with a clear
message) to ensure id > 0x00FF_FFFF is always rejected, and keep the existing
id.to_be_bytes()[1..].to_vec() logic unchanged; reference function:
truncate_to_3bytes and the prekey id check.
- Around line 591-623: The code accepts signed prekeys without validating the
<type> child; update the parsing to require and validate the <type> element
(e.g., via required_child(node, "type")?) and ensure its content matches the
expected curve string before calling SignedPreKeyNode::try_from_node; if the
<type> child is missing or the value is not the supported curve, return an error
(use the same anyhow! pattern). You can either add this check in the current
parsing function (before calling SignedPreKeyNode::try_from_node) or add the
validation inside SignedPreKeyNode::try_from_node itself so mismatched/absent
types are rejected consistently.
In `@wacore/src/iq/props.rs`:
- Around line 164-189: The try_from_node implementation for AbPropConfig
currently moves the `experiment` Result when matching, then reuses it for error
reporting; change the flow to a nested match so you pattern-match `experiment`
and, on Err(e1), attempt `sampling` and pattern-match it too, returning
Ok(Self::Experiment(prop)) or Ok(Self::Sampling(prop)) on success and
constructing the final anyhow::Error using the two captured errors (e1 and e2)
on double failure; reference AbProp::try_from_node, SamplingProp::try_from_node
and the try_from_node function for AbPropConfig to locate where to replace the
if-let checks with a match-based approach that avoids moving values before you
need their errors.
🧹 Nitpick comments (1)
wacore/derive/src/lib.rs (1)
330-340: Validateoptional/defaultcombos early to avoid opaque macro failures.
#[attr(..., optional)]on non‑Option<T>(ordefaultonjidattrs) will generate confusing compile errors or hit theunreachable!branch later. Consider rejecting these during parsing with a clear diagnostic.🧩 Suggested validation in
extract_attr_infoattr.parse_nested_meta(|meta| { if meta.path.is_ident("name") { let value: syn::LitStr = meta.value()?.parse()?; attr_name = Some(value.value()); } else if meta.path.is_ident("default") { let value: syn::LitStr = meta.value()?.parse()?; default = Some(value.value()); } else if meta.path.is_ident("jid") { is_jid = true; } else if meta.path.is_ident("optional") { explicit_optional = true; } Ok(()) })?; + if explicit_optional && !is_optional { + return Err(syn::Error::new_spanned( + &field.ty, + "`optional` requires an Option<T> field", + )); + } + if is_jid && default.is_some() { + return Err(syn::Error::new_spanned( + attr, + "`default` is only supported for String attributes", + )); + }Also applies to: 368-409, 424-432
Comment on lines
+289
to
+293
| /// Helper function to truncate u32 to 3-byte big-endian representation. | ||
| fn truncate_to_3bytes(id: u32) -> Vec<u8> { | ||
| debug_assert!(id <= 0x00FF_FFFF, "prekey id exceeds 3-byte range: {id}"); | ||
| id.to_be_bytes()[1..].to_vec() | ||
| } |
There was a problem hiding this comment.
Avoid silent truncation of prekey IDs in release builds.
debug_assert! is stripped in release, so out‑of‑range IDs will truncate silently, corrupting key IDs. Prefer a hard check (or validation in constructors) that always runs.
🔧 Safer guard
fn truncate_to_3bytes(id: u32) -> Vec<u8> {
- debug_assert!(id <= 0x00FF_FFFF, "prekey id exceeds 3-byte range: {id}");
+ assert!(id <= 0x00FF_FFFF, "prekey id exceeds 3-byte range: {id}");
id.to_be_bytes()[1..].to_vec()
}🤖 Prompt for AI Agents
In `@wacore/src/iq/prekeys.rs` around lines 289 - 293, The helper
truncate_to_3bytes currently uses debug_assert!, which is stripped in release
builds and allows silent truncation of out-of-range prekey ids; change the guard
to a hard runtime check (e.g., replace debug_assert! with assert! or an explicit
if that panics with a clear message) to ensure id > 0x00FF_FFFF is always
rejected, and keep the existing id.to_be_bytes()[1..].to_vec() logic unchanged;
reference function: truncate_to_3bytes and the prekey id check.
Comment on lines
+591
to
+623
| // Parse registration ID (4 bytes big-endian) | ||
| let reg_node = required_child(node, "registration")?; | ||
| let reg_bytes = reg_node | ||
| .content | ||
| .as_ref() | ||
| .and_then(|c| match c { | ||
| NodeContent::Bytes(b) => Some(b), | ||
| _ => None, | ||
| }) | ||
| .ok_or_else(|| anyhow!("missing bytes in <registration>"))?; | ||
| if reg_bytes.len() != 4 { | ||
| return Err(anyhow!("registration ID must be 4 bytes")); | ||
| } | ||
| let registration_id = | ||
| u32::from_be_bytes([reg_bytes[0], reg_bytes[1], reg_bytes[2], reg_bytes[3]]); | ||
|
|
||
| // Parse identity key (32 bytes) | ||
| let identity_node = required_child(node, "identity")?; | ||
| let identity_key = identity_node | ||
| .content | ||
| .as_ref() | ||
| .and_then(|c| match c { | ||
| NodeContent::Bytes(b) => Some(b.clone()), | ||
| _ => None, | ||
| }) | ||
| .ok_or_else(|| anyhow!("missing bytes in <identity>"))?; | ||
| if identity_key.len() != 32 { | ||
| return Err(anyhow!("identity key must be 32 bytes")); | ||
| } | ||
|
|
||
| // Parse signed prekey | ||
| let skey_node = required_child(node, "skey")?; | ||
| let signed_pre_key = SignedPreKeyNode::try_from_node(skey_node)?; |
There was a problem hiding this comment.
Validate the <type> child to enforce the expected curve.
try_from_node ignores <type> entirely; a mismatched or missing type should error to avoid accepting unsupported key formats.
🧩 Proposed validation
// Parse registration ID (4 bytes big-endian)
let reg_node = required_child(node, "registration")?;
let reg_bytes = reg_node
.content
.as_ref()
.and_then(|c| match c {
NodeContent::Bytes(b) => Some(b),
_ => None,
})
.ok_or_else(|| anyhow!("missing bytes in <registration>"))?;
if reg_bytes.len() != 4 {
return Err(anyhow!("registration ID must be 4 bytes"));
}
let registration_id =
u32::from_be_bytes([reg_bytes[0], reg_bytes[1], reg_bytes[2], reg_bytes[3]]);
+ // Parse type (expect Curve25519 = 0x05)
+ let type_node = required_child(node, "type")?;
+ let type_bytes = type_node
+ .content
+ .as_ref()
+ .and_then(|c| match c {
+ NodeContent::Bytes(b) => Some(b),
+ _ => None,
+ })
+ .ok_or_else(|| anyhow!("missing bytes in <type>"))?;
+ if type_bytes != [5] {
+ return Err(anyhow!("unexpected key type in <type>"));
+ }🤖 Prompt for AI Agents
In `@wacore/src/iq/prekeys.rs` around lines 591 - 623, The code accepts signed
prekeys without validating the <type> child; update the parsing to require and
validate the <type> element (e.g., via required_child(node, "type")?) and ensure
its content matches the expected curve string before calling
SignedPreKeyNode::try_from_node; if the <type> child is missing or the value is
not the supported curve, return an error (use the same anyhow! pattern). You can
either add this check in the current parsing function (before calling
SignedPreKeyNode::try_from_node) or add the validation inside
SignedPreKeyNode::try_from_node itself so mismatched/absent types are rejected
consistently.
Comment on lines
+164
to
+189
| fn try_from_node(node: &Node) -> Result<Self, anyhow::Error> { | ||
| if node.tag != "prop" { | ||
| return Err(anyhow::anyhow!("expected <prop>, got <{}>", node.tag)); | ||
| } | ||
|
|
||
| let experiment = AbProp::try_from_node(node); | ||
| if let Ok(prop) = experiment { | ||
| return Ok(Self::Experiment(prop)); | ||
| } | ||
|
|
||
| let sampling = SamplingProp::try_from_node(node); | ||
| if let Ok(prop) = sampling { | ||
| return Ok(Self::Sampling(prop)); | ||
| } | ||
|
|
||
| let experiment_err = experiment | ||
| .err() | ||
| .unwrap_or_else(|| anyhow::anyhow!("unknown error")); | ||
| let sampling_err = sampling | ||
| .err() | ||
| .unwrap_or_else(|| anyhow::anyhow!("unknown error")); | ||
| Err(anyhow::anyhow!( | ||
| "prop did not match experiment or sampling config: experiment_err={}; sampling_err={}", | ||
| experiment_err, | ||
| sampling_err | ||
| )) |
There was a problem hiding this comment.
Fix move-after-use in AbPropConfig::try_from_node.
if let Ok(prop) = experiment moves experiment, then it’s reused for error reporting—this won’t compile. Use a match to avoid moving the result twice.
✅ Safe match-based fix
- let experiment = AbProp::try_from_node(node);
- if let Ok(prop) = experiment {
- return Ok(Self::Experiment(prop));
- }
-
- let sampling = SamplingProp::try_from_node(node);
- if let Ok(prop) = sampling {
- return Ok(Self::Sampling(prop));
- }
-
- let experiment_err = experiment
- .err()
- .unwrap_or_else(|| anyhow::anyhow!("unknown error"));
- let sampling_err = sampling
- .err()
- .unwrap_or_else(|| anyhow::anyhow!("unknown error"));
- Err(anyhow::anyhow!(
- "prop did not match experiment or sampling config: experiment_err={}; sampling_err={}",
- experiment_err,
- sampling_err
- ))
+ let experiment = AbProp::try_from_node(node);
+ let sampling = SamplingProp::try_from_node(node);
+ match (experiment, sampling) {
+ (Ok(prop), _) => Ok(Self::Experiment(prop)),
+ (Err(_), Ok(prop)) => Ok(Self::Sampling(prop)),
+ (Err(experiment_err), Err(sampling_err)) => Err(anyhow::anyhow!(
+ "prop did not match experiment or sampling config: experiment_err={}; sampling_err={}",
+ experiment_err,
+ sampling_err
+ )),
+ }📝 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.
Suggested change
| fn try_from_node(node: &Node) -> Result<Self, anyhow::Error> { | |
| if node.tag != "prop" { | |
| return Err(anyhow::anyhow!("expected <prop>, got <{}>", node.tag)); | |
| } | |
| let experiment = AbProp::try_from_node(node); | |
| if let Ok(prop) = experiment { | |
| return Ok(Self::Experiment(prop)); | |
| } | |
| let sampling = SamplingProp::try_from_node(node); | |
| if let Ok(prop) = sampling { | |
| return Ok(Self::Sampling(prop)); | |
| } | |
| let experiment_err = experiment | |
| .err() | |
| .unwrap_or_else(|| anyhow::anyhow!("unknown error")); | |
| let sampling_err = sampling | |
| .err() | |
| .unwrap_or_else(|| anyhow::anyhow!("unknown error")); | |
| Err(anyhow::anyhow!( | |
| "prop did not match experiment or sampling config: experiment_err={}; sampling_err={}", | |
| experiment_err, | |
| sampling_err | |
| )) | |
| fn try_from_node(node: &Node) -> Result<Self, anyhow::Error> { | |
| if node.tag != "prop" { | |
| return Err(anyhow::anyhow!("expected <prop>, got <{}>", node.tag)); | |
| } | |
| let experiment = AbProp::try_from_node(node); | |
| let sampling = SamplingProp::try_from_node(node); | |
| match (experiment, sampling) { | |
| (Ok(prop), _) => Ok(Self::Experiment(prop)), | |
| (Err(_), Ok(prop)) => Ok(Self::Sampling(prop)), | |
| (Err(experiment_err), Err(sampling_err)) => Err(anyhow::anyhow!( | |
| "prop did not match experiment or sampling config: experiment_err={}; sampling_err={}", | |
| experiment_err, | |
| sampling_err | |
| )), | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@wacore/src/iq/props.rs` around lines 164 - 189, The try_from_node
implementation for AbPropConfig currently moves the `experiment` Result when
matching, then reuses it for error reporting; change the flow to a nested match
so you pattern-match `experiment` and, on Err(e1), attempt `sampling` and
pattern-match it too, returning Ok(Self::Experiment(prop)) or
Ok(Self::Sampling(prop)) on success and constructing the final anyhow::Error
using the two captured errors (e1 and e2) on double failure; reference
AbProp::try_from_node, SamplingProp::try_from_node and the try_from_node
function for AbPropConfig to locate where to replace the if-let checks with a
match-based approach that avoids moving values before you need their errors.
This was referenced Mar 11, 2026
This was referenced Mar 26, 2026
jlucaso1
added a commit
that referenced
this pull request
Jun 29, 2026
…lt perf Temporarily git-pins buffa/buffa-build/buffa-descriptor to main (rev 64efc3b) so CI/CodSpeed measures the encode/decode performance of #250's inline-by-default sub-message representation (anthropics/buffa#257). Non-publishable while git-pinned; revert to a crates.io version once buffa releases #250. Inline-by-default changes the generated sender_chain field from Box to inline, so DecryptSnapshot now stores Option<Chain> instead of MessageField<Chain> to stay agnostic to buffa's sub-message representation.
jlucaso1
added a commit
that referenced
this pull request
Jun 29, 2026
…lt perf Temporarily git-pins buffa/buffa-build/buffa-descriptor to main (rev 64efc3b) so CI/CodSpeed measures the encode/decode performance of #250's inline-by-default sub-message representation (anthropics/buffa#257). Non-publishable while git-pinned; revert to a crates.io version once buffa releases #250. Inline-by-default changes the generated sender_chain field from Box to inline, so DecryptSnapshot now stores Option<Chain> instead of MessageField<Chain> to stay agnostic to buffa's sub-message representation.
jlucaso1
added a commit
that referenced
this pull request
Jun 29, 2026
…lt perf Temporarily git-pins buffa/buffa-build/buffa-descriptor to main (rev f1c8088) so CI/CodSpeed measures the encode/decode performance of #250's inline-by-default sub-message representation and #259's inlined Tag::new (anthropics/buffa#257). Non-publishable while git-pinned; revert to a crates.io version once buffa releases these. Inline-by-default changes the generated sender_chain field from Box to inline, so DecryptSnapshot now stores Option<Chain> instead of MessageField<Chain> to stay agnostic to buffa's sub-message representation.
jlucaso1
added a commit
that referenced
this pull request
Jun 29, 2026
…osion buffa main defaults singular message fields to an inline representation (#250). For WhatsApp's deep, many-optional-field messages that makes size_of explode recursively — WebMessageInfo and HistorySyncMsg reach ~15 KB each, since every message variant becomes its own inline slot — so decode and Vec growth turn into large struct memcpys. The history-sync stream-drain bench spent ~90% of its time in memcpy moving 15 KB HistorySyncMsg structs into a Vec and reallocating it. Pin buffa-build's blanket box_type(PointerRepr::Box), restoring the pre-inline boxed layout (WebMessageInfo 888 B, HistorySyncMsg 24 B) while staying on main for #259's inlined Tag::new encode win. The DecryptSnapshot and history-sync snapshot fields stay representation-agnostic, so this composes with either layout.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.