Skip to content

perf(size): -1.47 MiB (-13.3%) stripped binary — build-config levers + control-plane demonomorphization - #1055

Merged
jlucaso1 merged 6 commits into
mainfrom
claude/buffa-v0-9-0-upgrade-z91tgm
Jul 19, 2026
Merged

perf(size): -1.47 MiB (-13.3%) stripped binary — build-config levers + control-plane demonomorphization#1055
jlucaso1 merged 6 commits into
mainfrom
claude/buffa-v0-9-0-upgrade-z91tgm

Conversation

@jlucaso1

Copy link
Copy Markdown
Collaborator

Deep binary-size audit of the demo release binary (the binary-size CI metric), executed as a 6-lens parallel source audit plus symbol-level measurement of the fat-LTO output. Every lever below was measured in isolation locally with the exact CI methodology (strip --strip-all size + .text); every candidate with a real runtime, functionality, or diagnostics drawback was rejected — the list is at the end.

Result: 11,590,552 → 10,053,720 bytes stripped (−1,536,832 = −1.47 MiB, −13.3%); .text 9,470,326 → 8,069,110 (−14.8%).

step lever stripped Δ
1 -Zshare-generics=y (.cargo/config.toml, linux-x64) −335 KiB
2 lld + --icf=all (same scope) −73 KiB
3 [profile.release.package.libsqlite3-sys] opt-level="z" −856 KiB
4 LIBSQLITE3_FLAGS subsystem trim −227 KiB
5 code batch (codec sweep, generic collapses, Debug guards) −46 KiB

Build-config levers (.cargo/config.toml + one Cargo.toml override)

  • -Zshare-generics=y — a symbol-level dump showed 741 KiB of byte-duplicated function copies in the shipped binary (wa::Message::clone ×2 at 70 KiB each, Message drop glue ×4, whole async send state machines ×2): downstream crates re-instantiate cross-crate generics with distinct symbol hashes, and fat LTO keeps every copy. share-generics reuses upstream instantiations at the source. The Docker image build has run with this flag since perf(docker): enable -Zshare-generics in the image build #845; the toolchain is nightly-pinned so the -Z flag is stable-by-pin. Scoped to x86_64-unknown-linux-gnu so wasm and other hosts keep default behavior.
  • lld + ICF — tighter layout of the fat-LTO object plus identical-section folding. (Diagnosed why ICF alone can't fold the remaining duplicates: fat LTO optimizes sibling copies differently, breaking byte-identity across the duplicate graph — which is why share-generics, which prevents the duplication pre-LTO, is the bigger lever.)
  • SQLite at -Oz — the bundled sqlite3.c amalgamation was the one crate the 2026-06 per-package opt-level sweep missed: it still compiled at -O3. Single biggest lever (−856 KiB). Same I/O-bound tradeoff the sweep already accepted for diesel/r2d2/sqlite-storage; storage sits behind the async pool and the write-behind Signal cache, never on per-message crypto.
  • SQLite subsystem trimLIBSQLITE3_FLAGS drops FTS3, RTREE, STAT4, DBSTAT, SOUNDEX, JSON SQL, loadable extensions (also a dlopen surface), deprecated APIs and progress callbacks — verified unused across diesel/sqlite-storage/chat-store. FTS5 stays (chat-store search builds on it); shared cache stays (sqlite-storage's in-memory pool mode shares the DB via cache=shared — the omit attempt was caught by the roundtrip tests and reverted in c2baa36).

Consumers building whatsapp-rust from crates.io are unaffected: .cargo/config.toml and profile overrides apply to workspace builds only.

Code levers (all control-plane; hot paths untouched)

  • waproto::codec sweep finished — every remaining production call site of buffa's generic Message methods on waproto types outside waproto now routes through the #[inline(never)] pinned helpers (17 new helpers for the small roots). A clippy.toml disallowed-methods guard rejects new direct call sites, so the class can't regrow. libsignal's signal-wire protos deliberately keep direct calls: that crate is their sole instantiation site and the per-message hot path — pinning would buy nothing and cost a call.
  • Client::execute<S> type-erased — the request-id/direct-encode/send/timeout body was stamped per IqSpec (~55 live specs); now one non-generic body, thin per-spec wrapper.
  • Generic collapses — mex execute_request<V> split; appstate-sync's FDownload closure generic across eight wacore fns became a &BlobDownloadFn trait object (breaking for wacore consumers: parameter is now a dyn ref); upload_media_with_retry's 8 type parameters became boxed futures (each attempt wraps a whole HTTP transfer); the two group-IQ macros share one outlined build_iq.
  • Proto Debug guards — the single reachable {:?} of a waproto type now logs fields, and Event's derived Debug is a manual variant-name impl, so no future log line can silently resurrect the 627-impl generated proto Debug graph (a one-line change that would otherwise cost hundreds of KiB).

Evaluated and rejected (drawbacks)

  • Pruning proto clusters (Bot/AI ~257 KiB, template/buttons ~110 KiB, unused SyncActionValue fields ~70 KiB): verification showed the Bot cluster is live (msg-secret bot classification, history-sync bot prompts) and SyncActionValue is exposed whole through the public mutation surface — pruning would silently drop data consumers can read today.
  • -Cforce-unwind-tables=no (~350-480 KiB): kills RUST_BACKTRACE in release binaries — a field-diagnostics regression. Available as an opt-in for embedders; not defaulted.
  • -Zlocation-detail=none (~40-70 KiB): panics lose file:line.
  • -Zbuild-std (~300-600 KiB): CI/local build divergence and toolchain-bump fragility.
  • Replacing the ureq media stack (~150 KiB): real functionality surface (proxy env, redirects, connection reuse).
  • PortableCache value-erasure and the token-lookup PHF swap: plausible wins (~15-40 KiB, ~45 KiB) but the first touches concurrency-sensitive eviction code and the second sits on the per-frame encode path — both need dedicated benchmarking; left as documented follow-ups.

Verification

  • cargo fmt --check, cargo clippy --all --tests (including the new disallowed-methods guard): clean
  • cargo test --workspace --exclude e2e-tests: 3070 passed, 0 failed (the sqlite shared-cache regression the trim initially introduced was caught by these tests and fixed)
  • demo binary boots and initializes the trimmed SQLite backend normally
  • E2E + CodSpeed run on this PR: CodSpeed is the gate for the no-hot-path-regression claim; the appstate/upload/IQ paths touched are all control-plane

Generated by Claude Code

claude added 4 commits July 19, 2026 16:08
Three levers, each measured on the stripped release demo (the binary-size
CI metric), against the current main baseline of 11,590,552 bytes:

- [profile.release.package.libsqlite3-sys] opt-level="z": the bundled
  sqlite3.c amalgamation was the one crate the per-package opt-level
  sweep missed — it compiled at -O3. Same I/O-bound tradeoff already
  accepted for diesel/r2d2/sqlite-storage. Measured -856 KiB.
- -Zshare-generics=y (via .cargo/config.toml, linux-x64 scoped): stops
  per-crate re-instantiation of cross-crate generics that fat LTO keeps
  as distinct symbols; the Docker image build already ran with it.
  Measured -335 KiB.
- lld with --icf=all (linux-x64 scoped): tighter layout of the fat-LTO
  object plus identical-section folding. Measured -73 KiB.
- LIBSQLITE3_FLAGS trim: drops bundled-SQLite subsystems nothing in the
  workspace uses (FTS3, RTREE, STAT4, DBSTAT, SOUNDEX, JSON SQL,
  loadable extensions, shared cache, progress callbacks). FTS5 stays for
  chat-store's search feature.

Consumers building whatsapp-rust from crates.io are unaffected —
.cargo/config.toml and profile overrides only apply to workspace builds.

Co-Authored-By: Claude <noreply@anthropic.com>
execute<S> stamped the request-id generation, direct-encode branch,
send/wait glue and error mapping once per IqSpec (~55 live specs). The
generic shell now only encodes/builds/parses the spec; the shared body
lives in non-generic helpers, so each spec adds a thin wrapper instead
of a full copy. IQ dispatch is control-plane — no hot-path cost.

Co-Authored-By: Claude <noreply@anthropic.com>
…cs, guard proto Debug

Four coordinated reductions, all off the per-message hot path:

- waproto::codec sweep: every remaining production call site of buffa's
  generic Message methods on waproto types outside waproto now routes
  through the #[inline(never)] pinned helpers (17 new helpers added for
  the small roots: app-state key fingerprints, msmsg, event responses,
  reactions, shortcake, verified-name certs, noise cert chains, chat-store
  message codec). A clippy disallowed-methods guard now rejects new direct
  call sites; libsignal's signal-wire protos keep direct calls (sole
  instantiation site and per-message hot path — pinning buys nothing).
- mex: execute_request<V> splits into a thin per-V wrapper plus one
  non-generic execute body (was stamped per variables type).
- appstate sync: the FDownload closure generic across eight wacore fns is
  now a &BlobDownloadFn trait object — one instantiation of the large
  patch-processing bodies (breaking for wacore consumers: parameter is
  now a dyn ref).
- upload: the 8-type-parameter retry/failover driver takes boxed futures;
  media upload wraps whole HTTP transfers, so a BoxFuture per attempt is
  noise.
- group IQs: the two IQ macros share one outlined build_iq body instead
  of stamping it into 8 generated types.
- Debug guards: the one reachable {:?} of a waproto type (bot.rs device
  props override) logs fields instead, and Event's derived Debug is now a
  manual variant-name impl so no future log line can resurrect the
  627-impl generated proto Debug graph.

Co-Authored-By: Claude <noreply@anthropic.com>
sqlite-storage's in-memory mode shares one DB across pooled connections
via cache=shared URIs; SQLITE_OMIT_SHARED_CACHE made every reopen see a
fresh empty DB (caught by the save/load roundtrip tests).

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Performance
    • Improved release build size optimization and refined request/upload handling for leaner, more efficient processing.
  • Reliability
    • Strengthened protocol message encoding/decoding across messaging, sync, pairing, reactions, and secure storage workflows, while preserving existing behavior on malformed inputs and download/decode failures.
  • Diagnostics
    • Improved device override logging details and reduced event debug verbosity to limit excessive payload output.
  • Maintenance
    • Standardized serialization behavior across production, test, and benchmark codepaths to improve consistency and code-quality checks.

Walkthrough

This PR centralizes protobuf serialization through waproto::codec, updates protocol and storage call sites, refactors generic execution paths, adds scoped Clippy allowances, adjusts workspace build settings, and changes device-props logging and event debug formatting.

Changes

Codec and runtime changes

Layer / File(s) Summary
Workspace build and lint configuration
.cargo/*, .gitignore, Cargo.toml, clippy.toml, .github/workflows/main.yml
Adds target-specific Cargo settings, SQLite release optimization, stable-job flag handling, and rules directing protobuf codec calls through pinned wrappers.
Pinned codec wrapper API
waproto/src/lib.rs, waproto/build.rs
Adds public codec helpers for pre-key, certificate, pairing, reaction, event, secret, receipt, and app-state fingerprint messages.
Protocol and storage codec migration
src/*, wacore/*, storages/chat-store/*
Replaces direct buffa encode/decode calls with waproto::codec helpers across messaging, key storage, Noise, pairing, encryption, and chat persistence paths.
Generic execution and callback refactors
src/request.rs, src/features/mex.rs, src/upload.rs, wacore/src/appstate_sync.rs, wacore/src/sync_marker.rs
Separates generic preparation from non-generic execution and changes media and app-state callbacks to shared trait-object interfaces.
Protocol behavior cleanup
src/bot.rs, wacore/src/iq/groups.rs, wacore/src/types/events.rs
Logs device-props fields individually, centralizes group IQ construction, and limits Event debug output to its event kind.
Scoped Clippy allowances
src/**, wacore/**, storages/**, tests/**
Adds scoped allowances for intentional raw buffa usage in tests, benchmarks, generated-code paths, and local protobuf implementations.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: performance

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the PR’s main focus on binary-size reduction via build-config and control-plane demonomorphization.
Description check ✅ Passed The description is detailed and directly describes the same size-reduction and code-path changes in the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/buffa-v0-9-0-upgrade-z91tgm

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

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 19, 2026

Copy link
Copy Markdown

Greptile Summary

This PR reduces the stripped demo release binary by −1.47 MiB (−13.3%) through a combination of build-config levers and control-plane demonomorphization, with every lever measured in isolation and verified against the full test suite.

  • Build config (.cargo/config.toml, Cargo.toml): -Zshare-generics=y prevents per-crate re-instantiation of cross-crate generics (−335 KiB); rust-lld with --icf=all folds byte-identical sections (−73 KiB); libsqlite3-sys compiled at -Oz (−856 KiB); LIBSQLITE3_FLAGS drops unused SQLite subsystems while preserving FTS5 and shared-cache (−227 KiB). The stable CI gate correctly opts out of nightly flags via RUSTFLAGS: \"\".
  • Code changes: Client::execute<S> is split into a thin generic trampoline and a non-generic execute_prepared body; AppStateProcessor's FDownload type parameter is replaced with a &BlobDownloadFn<'_> trait-object reference (breaking: now requires Send + Sync); upload_media_with_retry delegates to a dyn-driven _dyn body; waproto::codec gains 17 #[inline(never)] pinned wrappers with a clippy.toml guard to prevent new direct call sites; Event::Debug is replaced with a manual variant-name-only impl.

Confidence Score: 5/5

Safe to merge — all hot paths are untouched, the SQLite shared-cache regression was caught and reverted before this PR landed, and the test suite (3070 tests) passes clean.

All changes are either mechanical codec-call redirections through the new pinned wrappers, or build-config additions scoped to a single target triple with an explicit stable-toolchain opt-out in CI. The execute_prepared refactor faithfully preserves the encoded and fallback IQ paths. The BlobDownloadFn breaking change is limited in scope and fully acknowledged. No control logic was removed or reordered.

No files require special attention — the most structurally significant changes (src/request.rs, src/upload.rs, wacore/src/appstate_sync.rs) were carefully reviewed and show no logic regressions.

Important Files Changed

Filename Overview
.cargo/config.toml New workspace build-config: -Zshare-generics=y, rust-lld with ICF, LIBSQLITE3_FLAGS subsystem trim; scoped to x86_64-linux-gnu, no system-lld dependency (uses bundled rust-lld via -Clinker-features=+lld)
.github/workflows/main.yml Adds RUSTFLAGS: empty string to the stable toolchain job so the nightly-only -Zshare-generics flag does not break the stable CI gate
Cargo.toml Adds [profile.release.package.libsqlite3-sys] opt-level=z so the C amalgamation compiles at -Oz via the cc crate OPT_LEVEL export
clippy.toml Adds 10 disallowed-methods entries for buffa::message::Message codec methods to enforce routing through waproto::codec pinned wrappers
src/request.rs Introduces PreparedIq enum and non-generic execute_prepared tail so the IQ send/wait body is monomorphized once instead of per-IqSpec
src/upload.rs Introduces upload_media_with_retry_dyn with boxed-future dyn callbacks, collapsing the large driver body to a single instantiation
wacore/src/appstate_sync.rs Replaces all FDownload type parameters with &BlobDownloadFn trait-object references; breaking change documented in PR description
waproto/src/lib.rs Adds 17 inline(never) pinned codec wrappers with disallowed_methods guard scoped to the codec module as the single sanctioned instantiation site
wacore/src/types/events.rs Replaces derived Debug on Event with a manual variant-name-only impl to avoid pulling in the entire proto Debug graph
wacore/src/iq/groups.rs Extracts build_participant_action_iq and build_property_toggle_iq free functions shared by macro-generated IqSpec impls
wacore/src/sync_marker.rs Adds MaybeSend trait alongside MaybeSendSync for constraining upload closures that cross task boundaries
wacore/libsignal/src/protocol/protocol.rs Adds disallowed_methods allow attributes to Signal protocol methods that are the sole instantiation site for their protos

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Client::execute S"] --> B["spec.encode_iq_direct"]
    B -- "Ok true" --> C["PreparedIq::Encoded buf"]
    B -- "Ok false" --> D["spec.build_iq -> PreparedIq::Query"]
    B -- "Err" --> E["return IqError::EncodeError"]
    C --> F["execute_prepared (non-generic)"]
    D --> F
    F -- "Encoded path" --> G["send_and_wait_iq"]
    F -- "Query path" --> H["send_iq"]
    G --> I["Arc OwnedNodeRef"]
    H --> I
    I --> J["spec.parse_response"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["Client::execute S"] --> B["spec.encode_iq_direct"]
    B -- "Ok true" --> C["PreparedIq::Encoded buf"]
    B -- "Ok false" --> D["spec.build_iq -> PreparedIq::Query"]
    B -- "Err" --> E["return IqError::EncodeError"]
    C --> F["execute_prepared (non-generic)"]
    D --> F
    F -- "Encoded path" --> G["send_and_wait_iq"]
    F -- "Query path" --> H["send_iq"]
    G --> I["Arc OwnedNodeRef"]
    H --> I
    I --> J["spec.parse_response"]
Loading

Reviews (2): Last reviewed commit: "fix(build): address PR review — bundled ..." | Re-trigger Greptile

Comment thread .cargo/config.toml

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2baa3680e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .cargo/config.toml
# byte-identical sections (measured -73 KiB on the stripped demo). Scoped to
# this target so wasm and non-Linux builds keep their default linkers.
rustflags = [
"-Zshare-generics=y",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep nightly rustflags out of the stable build

The test-stable job in .github/workflows/main.yml explicitly installs the stable toolchain and then runs several x86_64 Linux Cargo builds, so this target-level flag is applied there and makes every build fail before compilation. rustc --help -v describes -Z as “unstable compiler options,” and stable rustc rejects them; the repository's pinned nightly does not override the toolchain selected by dtolnay/rust-toolchain@stable. Apply this size flag only to the nightly release build or otherwise exclude the stable job.

Useful? React with 👍 / 👎.

Comment thread src/upload.rs

/// Boxed future for the dyn-driven retry loop below. `Send` keeps the upload
/// futures spawnable, as they were with the fully generic signature.
type BoxFut<'a, T> = std::pin::Pin<Box<dyn std::future::Future<Output = T> + Send + 'a>>;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve non-Send upload futures on wasm

When compiling the supported wasm32-unknown-unknown library target checked by .github/workflows/wasm.yml, HttpClient uses async_trait(?Send) and MaybeSendSync deliberately drops Send + Sync, so the futures returned by self.http_client.execute(...) in Client::upload are not guaranteed to be Send. Requiring Send in this boxed future and in all adapter bounds therefore makes the upload call sites fail to type-check on wasm; use target-conditional boxed-future and closure bounds like the existing IqSendFuture abstraction.

Useful? React with 👍 / 👎.

Comment thread .cargo/config.toml Outdated
# this target so wasm and non-Linux builds keep their default linkers.
rustflags = [
"-Zshare-generics=y",
"-Clink-arg=-fuse-ld=lld",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid requiring an undeclared lld installation

On an x86_64 Linux checkout where the normal system linker is installed but LLVM's ld.lld is not, every Cargo build now fails at link time because this target-wide flag unconditionally selects lld. GCC's --help=common describes -fuse-ld=lld as selecting the LLVM linker, but neither rustup nor Cargo installs that external executable, and the repository's build instructions and CI setup do not declare it as a prerequisite. Limit this linker choice to the measured release/size build or add an explicit, reliable lld setup instead of imposing it on ordinary cargo test and debug builds.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.cargo/config.toml:
- Around line 11-18: Extend the target-specific Rust flags configuration so
-Zshare-generics=y is also enabled for macOS and AArch64 targets, while keeping
the lld and ICF linker arguments restricted to the Linux configuration where
they are supported.

In `@clippy.toml`:
- Around line 11-16: Extend the clippy configuration beside the existing Message
method bans to also reject the generic buffer method decode and the
length-delimited methods encode_length_delimited and decode_length_delimited.
Add entries with reasons directing callers to the corresponding pinned
waproto::codec wrappers, preserving the existing bloat-prevention policy.

In `@src/bot.rs`:
- Around line 1313-1320: Replace the heap-allocating version String construction
in the override_.version formatting expression with a lightweight wrapper that
borrows the version components and implements custom Debug formatting. Use that
wrapper directly in the debug log so formatting remains equivalent while
avoiding allocation, preserving the existing zero defaults for missing primary,
secondary, and tertiary values.

In `@src/prekeys.rs`:
- Line 1065: The test fixtures currently bypass production codec wrappers
through direct buffa APIs and disallowed-method allowances. Migrate the
window_tests in src/prekeys.rs:1065-1065, the shortcake tests in
wacore/src/shortcake.rs:290-293, and the business tests in
wacore/src/stanza/business.rs:384-387 to use waproto::codec wrappers, then
remove the corresponding #[allow(clippy::disallowed_methods)] attributes.

In `@src/store/signal.rs`:
- Line 287: Replace the redundant byte-vector slice conversions by relying on
Rust deref coercion: update src/store/signal.rs lines 287-287 in the
pre_key_record_decode call to borrow bytes directly, and update
wacore/noise/src/handshake.rs lines 190-192 and 224-224 to pass
intermediate_details_bytes and leaf_details_bytes directly instead of calling
as_slice().
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e9b809bf-5ba1-4602-95c7-65ea6d43e274

📥 Commits

Reviewing files that changed from the base of the PR and between 2c463e5 and c2baa36.

📒 Files selected for processing (66)
  • .cargo/config.toml
  • .gitignore
  • Cargo.toml
  • clippy.toml
  • src/appstate_sync.rs
  • src/bot.rs
  • src/client/device_registry.rs
  • src/client/lid_pn.rs
  • src/features/message_edit.rs
  • src/features/mex.rs
  • src/features/rotate_key.rs
  • src/history_sync.rs
  • src/message/msg_secret.rs
  • src/message/special.rs
  • src/message/tests.rs
  • src/passkey/flow.rs
  • src/pdo.rs
  • src/prekeys.rs
  • src/reexports_test.rs
  • src/request.rs
  • src/send/mod.rs
  • src/store/signal.rs
  • src/upload.rs
  • storages/chat-store/src/materialize.rs
  • storages/chat-store/src/queries.rs
  • storages/chat-store/src/store.rs
  • storages/chat-store/tests/chat_store_test.rs
  • storages/sqlite-storage/src/wire.rs
  • tests/handshake_integration.rs
  • wacore/appstate/src/decode.rs
  • wacore/appstate/src/processor.rs
  • wacore/benches/history_sync_benchmark.rs
  • wacore/benches/message_utils_benchmark.rs
  • wacore/benches/reporting_token_benchmark.rs
  • wacore/benches/send_receive_benchmark.rs
  • wacore/libsignal/src/protocol/identity_key.rs
  • wacore/libsignal/src/protocol/protocol.rs
  • wacore/libsignal/src/protocol/sender_keys.rs
  • wacore/libsignal/src/protocol/state/session.rs
  • wacore/noise/src/handshake.rs
  • wacore/noise/src/test_util.rs
  • wacore/noise/tests/cert_chain_verify.rs
  • wacore/src/adv.rs
  • wacore/src/appstate_sync.rs
  • wacore/src/companion_reg.rs
  • wacore/src/event.rs
  • wacore/src/history_sync.rs
  • wacore/src/iq/groups.rs
  • wacore/src/iq/usync.rs
  • wacore/src/media_retry.rs
  • wacore/src/message_edit.rs
  • wacore/src/message_processing.rs
  • wacore/src/messages.rs
  • wacore/src/pair.rs
  • wacore/src/poll.rs
  • wacore/src/prekeys.rs
  • wacore/src/reaction.rs
  • wacore/src/shortcake.rs
  • wacore/src/stanza/business.rs
  • wacore/src/store/device.rs
  • wacore/src/types/events.rs
  • wacore/src/usync.rs
  • wacore/src/voip/mlow/smpl_tables_blob.rs
  • wacore/tests/appstate_external_mutations_test.rs
  • waproto/build.rs
  • waproto/src/lib.rs

Comment thread .cargo/config.toml
Comment on lines +11 to +18
# lld + ICF: lld lays out the fat-LTO object tighter than BFD ld and folds
# byte-identical sections (measured -73 KiB on the stripped demo). Scoped to
# this target so wasm and non-Linux builds keep their default linkers.
rustflags = [
"-Zshare-generics=y",
"-Clink-arg=-fuse-ld=lld",
"-Clink-arg=-Wl,--icf=all",
]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial

Consider enabling -Zshare-generics=y for other targets like macOS and AArch64.

Look, optimizing the Linux target with lld and share-generics to keep the binary tight is great. But half our engineering team runs Macs, and we run ARM servers. We need to move fast across all platforms. While lld and ICF might be Linux-specific in this flag combination, -Zshare-generics=y works on macOS and AArch64 too. Let's consider scoping the generics sharing to those targets as well so we don't leave performance and build speed on the table.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.cargo/config.toml around lines 11 - 18, Extend the target-specific Rust
flags configuration so -Zshare-generics=y is also enabled for macOS and AArch64
targets, while keeping the lld and ICF linker arguments restricted to the Linux
configuration where they are supported.

Comment thread clippy.toml
Comment thread src/bot.rs
Comment on lines +1313 to +1320
override_.version.as_ref().map(|v| {
format!(
"{}.{}.{}",
v.primary.unwrap_or(0),
v.secondary.unwrap_or(0),
v.tertiary.unwrap_or(0)
)
}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Avoid heap allocation for log formatting.

Look, we need WhatsApp to run as lean as possible if we're going to connect the world. Shrinking the binary is exactly what I expect from this team. However, you're allocating a String here just to format the version inside a debug log. It won't crash our infrastructure since it's on the startup path, but it sets a sloppy precedent for memory efficiency. Consider using a small lightweight wrapper struct with a custom Debug implementation to avoid this heap allocation entirely. Keep our hot paths zero-allocation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/bot.rs` around lines 1313 - 1320, Replace the heap-allocating version
String construction in the override_.version formatting expression with a
lightweight wrapper that borrows the version components and implements custom
Debug formatting. Use that wrapper directly in the debug log so formatting
remains equivalent while avoiding allocation, preserving the existing zero
defaults for missing primary, secondary, and tertiary values.

Comment thread src/prekeys.rs
Comment thread src/store/signal.rs Outdated
Ok(Some(bytes)) => {
// Try new format first (protobuf-encoded PreKeyRecordStructure)
if let Ok(record) = PreKeyRecordStructure::decode_from_slice(bytes.as_ref()) {
if let Ok(record) = waproto::codec::pre_key_record_decode(bytes.as_ref()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rely on Rust's deref coercion instead of manual slice conversions.

Look, we're trying to build a platform that connects billions of people, and that means we need a lean, consistent codebase. Across these files, you're manually converting byte vectors to slices using .as_ref() or .as_slice() when a simple borrow triggers Rust's deref coercion automatically. You even did it right on line 362 of signal.rs by using &bytes! Redundant method calls just clutter our logic and waste time. Clean this up so we can focus on shipping things that actually matter.

  • src/store/signal.rs#L287-L287: Replace bytes.as_ref() with &bytes.
  • wacore/noise/src/handshake.rs#L190-L192: Replace intermediate_details_bytes.as_slice() with intermediate_details_bytes.
  • wacore/noise/src/handshake.rs#L224-L224: Replace leaf_details_bytes.as_slice() with leaf_details_bytes.
📍 Affects 2 files
  • src/store/signal.rs#L287-L287 (this comment)
  • wacore/noise/src/handshake.rs#L190-L192
  • wacore/noise/src/handshake.rs#L224-L224
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/store/signal.rs` at line 287, Replace the redundant byte-vector slice
conversions by relying on Rust deref coercion: update src/store/signal.rs lines
287-287 in the pre_key_record_decode call to borrow bytes directly, and update
wacore/noise/src/handshake.rs lines 190-192 and 224-224 to pass
intermediate_details_bytes and leaf_details_bytes directly instead of calling
as_slice().

claude added 2 commits July 19, 2026 18:00
… Send bounds

- Use -Clinker-features=+lld (toolchain's rust-lld) instead of -fuse-ld=lld
  so contributors need no system lld; still unstable-gated, hence
  -Zunstable-options in the nightly-only rustflags block.
- CI test-stable job sets RUSTFLAGS="" to opt out of the nightly-only
  target rustflags (env overrides config).
- upload.rs: cfg-conditional boxed-future and dyn-callback types plus a new
  wacore MaybeSend marker keep the retry driver Send on native while
  preserving ?Send HttpClient futures on wasm32.
- Document variant-name-only Debug in the Event doc comment.
- clippy.toml: ban the remaining generic Message entry points (decode,
  decode/encode_length_delimited, encode_to_bytes); allow raw methods in
  record_helpers tests that verify the borrowed encoder against them.
- Drop redundant as_ref()/as_slice() where deref coercion applies.

Co-Authored-By: Claude <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@clippy.toml`:
- Around line 17-20: Add the corresponding waproto::codec pinned wrapper APIs
for Message::decode, decode_length_delimited, encode_length_delimited, and
encode_to_bytes before retaining these clippy.toml bans. Match the existing
wrapper patterns used by decode_from_slice, encode_to_vec, encode, and write_to,
then ensure the banned methods route through the new wrappers rather than
blocking valid call sites.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: de47f368-518f-41d5-af9b-5035c727414d

📥 Commits

Reviewing files that changed from the base of the PR and between c2baa36 and 58680d0.

📒 Files selected for processing (15)
  • .cargo/config.toml
  • .github/workflows/main.yml
  • clippy.toml
  • src/history_sync.rs
  • src/message/tests.rs
  • src/prekeys.rs
  • src/store/signal.rs
  • src/upload.rs
  • wacore/libsignal/src/protocol/protocol.rs
  • wacore/libsignal/src/store/record_helpers.rs
  • wacore/noise/src/handshake.rs
  • wacore/src/history_sync.rs
  • wacore/src/messages.rs
  • wacore/src/sync_marker.rs
  • wacore/src/types/events.rs

Comment thread clippy.toml
@github-actions

Copy link
Copy Markdown

📦 Binary size report

Metric main PR Δ
bin size (stripped) 11.07 MiB 9.61 MiB -1.46 MiB (-13.21%) 🎉
bin .text 9.04 MiB 7.71 MiB -1.34 MiB (-14.78%) 🎉
bin allocated (text+data+bss) 11.07 MiB 9.61 MiB -1.46 MiB (-13.21%) 🎉
llvm-lines wacore 513,954 472,749 -41,205 (-8.02%) 🎉
llvm-lines wacore copies 17,614 15,734 -1,880 (-10.67%) 🎉
llvm-lines whatsapp-rust lib 785,249 661,711 -123,538 (-15.73%) 🎉
llvm-lines whatsapp-rust lib copies 25,578 21,236 -4,342 (-16.98%) 🎉
deps crates (Cargo.lock) 471 471 0
.text per crate
Crate main PR Δ
.text whatsapp_rust 1.73 MiB 1.65 MiB -74.72 KiB (-4.23%) 🎉
.text wacore 509.94 KiB 598.65 KiB +88.71 KiB (+17.40%) ⚠️
.text wacore_binary 150.24 KiB 143.19 KiB -7.05 KiB (-4.69%) 🎉
.text wacore_libsignal 196.75 KiB 163.86 KiB -32.89 KiB (-16.72%) 🎉
.text wacore_appstate 47.23 KiB 22.36 KiB -24.87 KiB (-52.66%) 🎉
.text wacore_noise 20.42 KiB 22.46 KiB +2.03 KiB (+9.96%) ⚠️
.text waproto 1.83 MiB 1.74 MiB -90.11 KiB (-4.81%) 🎉
.text whatsapp_rust_sqlite_storage 516.15 KiB 510.52 KiB -5.63 KiB (-1.09%) 🎉
.text whatsapp_rust_tokio_transport 43.79 KiB 39.84 KiB -3.95 KiB (-9.03%) 🎉
.text whatsapp_rust_ureq_http_client 10.47 KiB 10.28 KiB -196 B (-1.83%) 🎉
.text std 1.02 MiB 945.35 KiB -97.20 KiB (-9.32%) 🎉
.text other deps 2.96 MiB 1.88 MiB -1.08 MiB (-36.47%) 🎉
Top movers (cargo-bloat attribution)
Crate main PR Δ
[Unknown] 1.51 MiB 627.07 KiB -922.57 KiB (-59.53%)
std 1.02 MiB 945.35 KiB -97.20 KiB (-9.32%)
waproto 1.83 MiB 1.74 MiB -90.11 KiB (-4.81%)
wacore 509.94 KiB 598.65 KiB +88.71 KiB (+17.40%)
libsqlite3_sys 100.27 KiB 23.40 KiB -76.87 KiB (-76.66%)
whatsapp_rust 1.73 MiB 1.65 MiB -74.72 KiB (-4.23%)
wacore_libsignal 196.75 KiB 163.86 KiB -32.89 KiB (-16.72%)
serde_core 28.12 KiB 112 B -28.01 KiB (-99.61%)
wacore_appstate 47.23 KiB 22.36 KiB -24.87 KiB (-52.66%)
serde_json 39.14 KiB 20.60 KiB -18.54 KiB (-47.37%)

Baseline: 2c463e5fe (latest main run) · Head: c891da195 · Graphs

@jlucaso1
jlucaso1 merged commit 67d18ab into main Jul 19, 2026
22 checks passed
@jlucaso1
jlucaso1 deleted the claude/buffa-v0-9-0-upgrade-z91tgm branch July 19, 2026 19:32
jlucaso1 pushed a commit that referenced this pull request Jul 20, 2026
…rink

Measured on identical trees with the pinned toolchain, default linker
both sides: the swap is -132 KiB stripped / +35 KiB .text vs main.
The SIZE_RUSTFLAGS (-fuse-ld=lld --icf=all) this PR added to the measure
step made the same binary ~300 KiB larger against the post-#1055 main,
so the workflow reverts to main's version — both sides now measure
flag-free, matching the baseline. A few remaining handshake-only crates
(x25519-dalek, rfc6979, pem-rfc7468, sha1_smol) join the opt-z list to
cover the residual .text delta.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018b9ZakKBufGJHS5pR29z6A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants