chore: reduce duplication and unnecessary allocations

[jlucaso1]

Summary

-177 lines net across 7 files. No behavior changes.

DRY improvements

• signal_err() helper (signal_adapter.rs): Replaces 12 identical .map_err(|e| SignalProtocolError::InvalidState("...", e.to_string())) patterns with .map_err(signal_err("..."))
• flush_signal_cache_logged() (client.rs): Replaces 6 identical if-let-Err logging blocks across send.rs, message.rs, notification.rs, device_registry.rs, retry.rs
• Test helpers (device_registry.rs): setup_lid_pn() and setup_device_record() replace 17 duplicated fixture setup blocks

Allocation reductions

• .take() instead of .clone() on device snapshot fields (send.rs): Moves pn, lid, account out of the owned snapshot instead of cloning — eliminates 6 clone calls across 2 send paths
• Zero-copy participant JID comparison (message.rs): Hoists own_jid.to_string() before the loop, keeps Cow<str> from optional_string() as-is for direct &str comparison — eliminates 2 allocations per participant per message

Test plan

• cargo test --workspace --exclude e2e-tests — all tests pass
• Clippy clean, 0 warnings

Summary by CodeRabbit

• Refactor
  • Consolidated error logging and handling for signal cache operations across multiple handlers.
  • Optimized data handling by reducing unnecessary object duplication.
  • Unified error conversion in signal protocol implementations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d7110792-7795-45b6-a0d6-62e9579eb96d

📥 Commits

Reviewing files that changed from the base of the PR and between 06ca49a and cbf76b1.

📒 Files selected for processing (7)

• src/client.rs
• src/client/device_registry.rs
• src/handlers/notification.rs
• src/message.rs
• src/retry.rs
• src/send.rs
• src/store/signal_adapter.rs

---

📝 Walkthrough

Walkthrough

This pull request introduces a helper method flush_signal_cache_logged to centralize error logging for signal cache flush operations, refactors multiple call sites to use this helper instead of inline error handling, optimizes device snapshot field consumption via take() instead of clone(), introduces a signal protocol error mapping helper, and extracts test setup functions to reduce duplication.

Changes

Cohort / File(s)	Summary
Signal Cache Flush Centralization src/client.rs, src/client/device_registry.rs, src/handlers/notification.rs, src/message.rs, src/retry.rs, src/send.rs	Added new flush_signal_cache_logged helper method to centralize error logging; replaced inline flush_signal_cache() error handling across multiple call sites with calls to this helper, delegating logging responsibility.
Test Refactoring src/client/device_registry.rs	Extracted setup_lid_pn and setup_device_record async helper functions to reduce duplicated cache seeding logic in test module; removed repeated inline LidPnEntry and DeviceListRecord construction blocks.
Error Mapping Helper src/store/signal_adapter.rs	Introduced signal_err helper function to unify error-mapping behavior for SignalProtocolError::InvalidState across session, identity, pre-key, and sender-key store implementations, replacing repeated closure expressions.
Device Snapshot Optimization src/send.rs	Refactored device snapshot field handling in send_status_message and send_message_impl to use take() for field consumption instead of .clone() and .unwrap_or_else(), reducing allocations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• feat: signal cache #312 — Introduced the original Client::flush_signal_cache method and signal cache infrastructure that this PR extends with a logging wrapper.
• fix: invalidate sender key cache on device changes and handle identity notifications #489 — Introduced delete_sessions_for_devices with initial flush_signal_cache calls that this PR now updates to use the centralized logging helper.
• refactor: architecture audit — cache bounds, circuit breaker, correctness fixes #478 — Modified flush failure handling patterns in the same code paths (retry, message, send, notification) that this PR consolidates into a unified helper.

Suggested reviewers

• Ari4ka

Poem

🐰 A log wraps up the flush with care,
No error lost to thin air,
Fields take() when they're meant to go,
Tests setup quick—no more overhead show,
Signal errors map with grace,
Cleaner code in every place!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: reducing code duplication and unnecessary memory allocations across multiple files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/dry-and-reduce-allocations

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jlucaso1]

@claude review

[claude]

Claude finished @jlucaso1's task in 1m 4s —— View job

---

Code Review: chore/dry-and-reduce-allocations

• Read PR diff and changed files
• Review signal_adapter.rs — signal_err() helper
• Review client.rs — flush_signal_cache_logged()
• Review device_registry.rs — test helpers & DRY
• Review send.rs — .take() instead of .clone()
• Review message.rs — zero-copy participant JID comparison
• Review notification.rs and retry.rs changes
• Post final review

---

Clean PR — good DRY improvements and allocation reductions with no behavior changes. A few observations:

signal_adapter.rs — signal_err() helper ✅

Nice use of a curried closure to avoid repeating the map_err boilerplate. The &'static str constraint on context is correct since SignalProtocolError::InvalidState requires &'static str. Clean.

client.rs — flush_signal_cache_logged() ✅

Consolidates 6 identical if-let-Err logging blocks into one method. The log format changes slightly (e.g. "Failed to flush signal cache after message {id}" → "Failed to flush signal cache (message {id})") but this is purely cosmetic and the context string still identifies the call site.

send.rs — .take() instead of .clone() ✅

Correct: device_snapshot is not accessed after the .take() calls in either send_status_message or the group send path. Moving account_info extraction before own_jid/own_lid is fine since take() on different fields is independent. Good elimination of 6 unnecessary clones.

One subtlety worth noting: if someone later adds code that reads device_snapshot.pn after line 296, they'd get None unexpectedly. This is a standard footgun with .take(), but the mut binding makes it visible enough. No action needed.

message.rs — zero-copy JID comparison ✅

The type math checks out: optional_string() returns Cow<'_, str>, so to_jid is Cow<str>. Before this PR, .to_string() allocated a new String on every iteration. Now *to_jid == **ours compares str == str with zero allocations. Correct.

device_registry.rs — test helpers ✅

setup_lid_pn() and setup_device_record() are well-scoped test helpers that eliminate significant duplication. The device_ids: &[u32] parameter makes setup_device_record flexible enough for all existing call sites.

Minor note: the helper uses wacore::time::now_secs() for the timestamp, whereas a few of the original blocks used hardcoded values like 12345 or 12346. This means the timestamp value changed for those tests, but since none of the assertions check the timestamp field, this is a no-op in practice.

notification.rs and retry.rs ✅

Straightforward substitutions of the flush_signal_cache_logged helper. No issues.

---

Verdict: LGTM. No bugs, no behavioral changes, good reduction in duplication and allocations.

[github-actions]

Bencher Report

Branch	chore/dry-and-reduce-allocations
Testbed	ubuntu-latest

🚨 1 Alert

Benchmark	Measure Units	View	Benchmark Result (Result Δ%)	Upper Boundary (Limit %)
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions()	Instructions instructions x 1e3	📈 plot 🚷 threshold 🚨 alert (🔔)	47.03 x 1e3 (+7.61%) Baseline: 43.71 x 1e3	45.89 x 1e3 (102.49%)

Click to view all benchmark results

Benchmark	Instructions	Benchmark Result instructions (Result Δ%)	Upper Boundary instructions (Limit %)
binary_benchmark::attr_parser_group::bench_attr_parser attr_lookup:setup_attr_marshaled()	📈 view plot 🚷 view threshold	6,199.00 (-2.90%) Baseline: 6,383.97	6,703.17 (92.48%)
binary_benchmark::child_iteration_group::bench_get_children_by_tag	📈 view plot 🚷 view threshold	523,708.00 (-24.14%) Baseline: 690,330.71	724,847.25 (72.25%)
binary_benchmark::jid_optimization_group::bench_jid_to_owned_access jid_access:setup_jid_heavy_marshaled()	📈 view plot 🚷 view threshold	20,910.00 (-4.68%) Baseline: 21,937.05	23,033.90 (90.78%)
binary_benchmark::marshal_group::bench_marshal_allocating	📈 view plot 🚷 view threshold	98,703.00 (-11.31%) Baseline: 111,293.04	116,857.69 (84.46%)
binary_benchmark::marshal_group::bench_marshal_auto_allocating	📈 view plot 🚷 view threshold	98,731.00 (-8.09%) Baseline: 107,420.22	112,791.23 (87.53%)
binary_benchmark::marshal_group::bench_marshal_auto_huge_bytes_allocating	📈 view plot 🚷 view threshold	533,017.00 (-0.08%) Baseline: 533,427.83	560,099.22 (95.16%)
binary_benchmark::marshal_group::bench_marshal_auto_long_string	📈 view plot 🚷 view threshold	15,955.00 (-3.44%) Baseline: 16,523.40	17,349.57 (91.96%)
binary_benchmark::marshal_group::bench_marshal_auto_many_children_allocating	📈 view plot 🚷 view threshold	14,813,622.00 (-6.11%) Baseline: 15,777,985.55	16,566,884.83 (89.42%)
binary_benchmark::marshal_group::bench_marshal_exact_allocating	📈 view plot 🚷 view threshold	118,631.00 (-17.52%) Baseline: 143,833.58	151,025.26 (78.55%)
binary_benchmark::marshal_group::bench_marshal_exact_huge_bytes_allocating	📈 view plot 🚷 view threshold	534,447.00 (-0.08%) Baseline: 534,850.25	561,592.76 (95.17%)
binary_benchmark::marshal_group::bench_marshal_exact_long_string	📈 view plot 🚷 view threshold	18,004.00 (-3.07%) Baseline: 18,574.31	19,503.03 (92.31%)
binary_benchmark::marshal_group::bench_marshal_exact_many_children_allocating	📈 view plot 🚷 view threshold	28,200,726.00 (-18.24%) Baseline: 34,492,850.82	36,217,493.36 (77.86%)
binary_benchmark::marshal_group::bench_marshal_huge_bytes_allocating	📈 view plot 🚷 view threshold	533,456.00 (-0.08%) Baseline: 533,866.83	560,560.17 (95.16%)
binary_benchmark::marshal_group::bench_marshal_long_string	📈 view plot 🚷 view threshold	15,928.00 (-5.87%) Baseline: 16,921.88	17,767.97 (89.64%)
binary_benchmark::marshal_group::bench_marshal_many_children_allocating	📈 view plot 🚷 view threshold	14,815,122.00 (-6.11%) Baseline: 15,779,160.41	16,568,118.43 (89.42%)
binary_benchmark::marshal_group::bench_marshal_reusing_buffer	📈 view plot 🚷 view threshold	108,446.00 (-8.95%) Baseline: 119,105.81	125,061.10 (86.71%)
binary_benchmark::marshal_group::bench_marshal_reusing_buffer_vec_writer	📈 view plot 🚷 view threshold	98,803.00 (-8.08%) Baseline: 107,492.22	112,866.83 (87.54%)
binary_benchmark::roundtrip_group::bench_roundtrip large:setup_large_marshaled()	📈 view plot 🚷 view threshold	91,558.00 (-3.87%) Baseline: 95,241.95	100,004.05 (91.55%)
binary_benchmark::roundtrip_group::bench_roundtrip small:setup_small_marshaled()	📈 view plot 🚷 view threshold	7,431.00 (-2.10%) Baseline: 7,590.54	7,970.07 (93.24%)
binary_benchmark::roundtrip_group::bench_roundtrip_auto large:setup_large_marshaled()	📈 view plot 🚷 view threshold	91,589.00 (-0.87%) Baseline: 92,397.17	97,017.02 (94.41%)
binary_benchmark::roundtrip_group::bench_roundtrip_auto small:setup_small_marshaled()	📈 view plot 🚷 view threshold	7,454.00 (+1.00%) Baseline: 7,380.17	7,749.18 (96.19%)
binary_benchmark::roundtrip_group::bench_roundtrip_exact large:setup_large_marshaled()	📈 view plot 🚷 view threshold	107,134.00 (-0.96%) Baseline: 108,176.28	113,585.10 (94.32%)
binary_benchmark::roundtrip_group::bench_roundtrip_exact small:setup_small_marshaled()	📈 view plot 🚷 view threshold	8,966.00 (+0.83%) Baseline: 8,892.17	9,336.77 (96.03%)
binary_benchmark::unmarshal_group::bench_unmarshal large:setup_large_marshaled()	📈 view plot 🚷 view threshold	41,989.00 (-5.87%) Baseline: 44,609.05	46,839.51 (89.64%)
binary_benchmark::unmarshal_group::bench_unmarshal small:setup_small_marshaled()	📈 view plot 🚷 view threshold	2,716.00 (-1.69%) Baseline: 2,762.56	2,900.68 (93.63%)
binary_benchmark::unpack_group::bench_unpack_compressed	📈 view plot 🚷 view threshold	556,090.00 (+0.18%) Baseline: 555,112.96	582,868.61 (95.41%)
binary_benchmark::unpack_group::bench_unpack_uncompressed	📈 view plot 🚷 view threshold	773.00 (+0.09%) Baseline: 772.32	810.93 (95.32%)
libsignal_benchmark::conversation_group::bench_full_dm_conversation full:setup_conversation_data()	📈 view plot 🚷 view threshold	27,743,448.00 (+0.17%) Baseline: 27,697,016.90	29,081,867.75 (95.40%)
libsignal_benchmark::dm_group::bench_dm_decrypt_first_message decrypt_prekey:setup_dm_with_first_message()	📈 view plot 🚷 view threshold	5,544,698.00 (-0.04%) Baseline: 5,546,845.16	5,824,187.41 (95.20%)
libsignal_benchmark::dm_group::bench_dm_encrypt_first_message first_msg:setup_dm_session()	📈 view plot 🚷 view threshold	175,063.00 (-1.15%) Baseline: 177,105.62	185,960.90 (94.14%)
libsignal_benchmark::dm_group::bench_dm_encrypt_subsequent_message subsequent:setup_established_dm_session()	📈 view plot 🚷 view threshold	175,711.00 (-1.21%) Baseline: 177,866.09	186,759.39 (94.08%)
libsignal_benchmark::dm_group::bench_dm_session_establishment setup:setup_dm_users()	📈 view plot 🚷 view threshold	17,505,293.00 (+1.30%) Baseline: 17,280,527.63	18,144,554.02 (96.48%)
libsignal_benchmark::group_messaging_group::bench_group_create_distribution_message create:setup_group_sender()	📈 view plot 🚷 view threshold	298,495.00 (+0.50%) Baseline: 296,998.84	311,848.78 (95.72%)
libsignal_benchmark::group_messaging_group::bench_group_decrypt_message decrypt:setup_group_with_encrypted_message()	📈 view plot 🚷 view threshold	12,784,070.00 (+1.50%) Baseline: 12,595,499.01	13,225,273.96 (96.66%)
libsignal_benchmark::group_messaging_group::bench_group_encrypt_message encrypt:setup_group_with_distribution()	📈 view plot 🚷 view threshold	719,752.00 (+0.36%) Baseline: 717,191.54	753,051.12 (95.58%)
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions()	📈 view plot 🚷 view threshold 🚨 view alert (🔔)	47,034.00 (+7.61%) Baseline: 43,707.39	45,892.76 (102.49%)
libsignal_benchmark::session_optimization_group::bench_message_key_eviction eviction:setup_message_key_eviction()	📈 view plot 🚷 view threshold	15,561,917.00 (+0.00%) Baseline: 15,561,776.13	16,339,864.94 (95.24%)
libsignal_benchmark::session_optimization_group::bench_out_of_order_decryption out_of_order:setup_out_of_order_messages()	📈 view plot 🚷 view threshold	5,377,818.00 (-1.62%) Baseline: 5,466,348.38	5,739,665.80 (93.70%)
libsignal_benchmark::session_optimization_group::bench_promote_matching_session promote:setup_promote_matching_session()	📈 view plot 🚷 view threshold	311,551.00 (-58.32%) Baseline: 747,530.50	784,907.03 (39.69%)
libsignal_benchmark::signature_group::bench_key_generation keygen	📈 view plot 🚷 view threshold	2,830,506.00 (+0.16%) Baseline: 2,826,109.11	2,967,414.57 (95.39%)
libsignal_benchmark::signature_group::bench_signature_creation sign:setup_keypair_with_message()	📈 view plot 🚷 view threshold	3,452,844.00 (-0.46%) Baseline: 3,468,877.50	3,642,321.37 (94.80%)
libsignal_benchmark::signature_group::bench_signature_verification verify:setup_keypair_with_message()	📈 view plot 🚷 view threshold	126,473,698.00 (+0.88%) Baseline: 125,364,622.64	131,632,853.77 (96.08%)
reporting_token_benchmark::content_extraction_group::bench_content_extraction extended:setup_extended_message()	📈 view plot 🚷 view threshold	11,851.00 (+0.08%) Baseline: 11,841.77	12,433.85 (95.31%)
reporting_token_benchmark::content_extraction_group::bench_content_extraction simple:setup_simple_message()	📈 view plot 🚷 view threshold	3,879.00 (+0.86%) Baseline: 3,846.06	4,038.36 (96.05%)
reporting_token_benchmark::full_generation_group::bench_full_token_generation extended:setup_full_gen_extended()	📈 view plot 🚷 view threshold	86,863.00 (-0.90%) Baseline: 87,654.12	92,036.83 (94.38%)
reporting_token_benchmark::full_generation_group::bench_full_token_generation simple:setup_full_gen_simple()	📈 view plot 🚷 view threshold	78,912.00 (-0.96%) Baseline: 79,680.44	83,664.46 (94.32%)
reporting_token_benchmark::key_derivation_group::bench_key_derivation	📈 view plot 🚷 view threshold	50,362.00 (-0.94%) Baseline: 50,840.62	53,382.65 (94.34%)
reporting_token_benchmark::message_encoding_group::bench_message_encoding extended:setup_extended_message()	📈 view plot 🚷 view threshold	5,939.00 (+2.40%) Baseline: 5,799.75	6,089.73 (97.52%)
reporting_token_benchmark::message_encoding_group::bench_message_encoding simple:setup_simple_message()	📈 view plot 🚷 view threshold	2,214.00 (+3.26%) Baseline: 2,144.09	2,251.29 (98.34%)
reporting_token_benchmark::token_calculation_group::bench_token_calculation	📈 view plot 🚷 view threshold	21,920.00 (-0.00%) Baseline: 21,920.16	23,016.17 (95.24%)

🐰 View full continuous benchmarking report in Bencher