Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Workspace build configuration. Applies to builds of THIS repo only —
# crates.io consumers are unaffected (cargo does not inherit this file
# across package boundaries).

[target.x86_64-unknown-linux-gnu]
# -Zshare-generics=y: downstream crates reuse upstream monomorphic
# instantiations instead of stamping per-crate copies with distinct symbol
# hashes that fat LTO cannot merge (measured -335 KiB on the stripped demo;
# the Docker image build already ran with this flag). Nightly-only, and the
# toolchain is pinned by rust-toolchain.toml.
# 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",

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 👍 / 👎.

"-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 👍 / 👎.

"-Clink-arg=-Wl,--icf=all",
]
Comment on lines +11 to +23

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.


[env]
# Trim bundled-SQLite subsystems the workspace never touches. FTS5 stays:
# chat-store's `search` feature builds its index on it. Shared cache stays:
# sqlite-storage's in-memory mode shares one DB across the pool via
Comment thread
greptile-apps[bot] marked this conversation as resolved.
# `cache=shared` URIs. JSON and extension loading are unused by
# diesel/sqlite-storage/chat-store (checked: no json_* SQL, no
# load_extension callers); dropping loadable extensions also removes a
# dlopen surface. STAT4/DBSTAT/RTREE/FTS3/SOUNDEX are upstream-default-off
# features libsqlite3-sys turns on; none are exercised here.
LIBSQLITE3_FLAGS = "-USQLITE_ENABLE_FTS3 -USQLITE_ENABLE_FTS3_PARENTHESIS -USQLITE_ENABLE_RTREE -USQLITE_ENABLE_STAT4 -USQLITE_ENABLE_DBSTAT_VTAB -USQLITE_SOUNDEX -DSQLITE_OMIT_JSON -DSQLITE_OMIT_LOAD_EXTENSION -DSQLITE_OMIT_DEPRECATED -DSQLITE_OMIT_PROGRESS_CALLBACK"
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
/*.xml
/*.txt
/whatsapp_store
/.cargo
/.cargo/*
!/.cargo/config.toml
/.vscode
/*.db*
.env
Expand Down
5 changes: 5 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,11 @@ opt-level = "s"
opt-level = "s"
[profile.release.package.serde_json]
opt-level = "s"
# The bundled SQLite amalgamation (sqlite3.c) — cargo exports OPT_LEVEL to the
# build script and the cc crate maps it to -Oz. The C core was the one crate
# the per-package sweep above missed; same I/O-bound tradeoff as diesel.
[profile.release.package.libsqlite3-sys]
opt-level = "z"

# Benchmark profile: release-grade codegen, but thin LTO keeps bench builds
# fast (release uses fat LTO) and debug symbols stay for local profiling.
Expand Down
11 changes: 11 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@ disallowed-methods = [
{ path = "chrono::Local::now", reason = "use wacore::time::now_utc() (Local depends on SystemTime which panics on WASM)" },
{ path = "std::time::SystemTime::now", reason = "use wacore::time::now_millis() / now_utc()" },
{ path = "std::time::Instant::now", reason = "use wacore::time::Instant::now()" },
# buffa's Message codec methods are generic over the buffer type, so every
# calling crate stamps its own copy of the full encode/decode tree (LTO
# cannot merge them). Route waproto types through the pinned wrappers in
# waproto::codec — add one there if missing. Crate-local protos (their sole
# instantiation site) and tests carry a scoped allow instead.
{ path = "buffa::message::Message::encode_to_vec", reason = "use waproto::codec::*_to_vec (pinned single instantiation; direct calls duplicate the encode tree per crate)" },
{ path = "buffa::message::Message::encode", reason = "use waproto::codec::*_encode_into (pinned single instantiation; direct calls duplicate the encode tree per crate)" },
{ path = "buffa::message::Message::write_to", reason = "use waproto::codec::*_write_to (pinned single instantiation; direct calls duplicate the encode tree per crate)" },
{ path = "buffa::message::Message::decode_from_slice", reason = "use waproto::codec::*_decode (pinned single instantiation; direct calls duplicate the decode tree per crate)" },
{ path = "buffa::message::Message::merge_from_slice", reason = "use waproto::codec::*_merge (pinned single instantiation; direct calls duplicate the decode tree per crate)" },
{ path = "buffa::message::Message::merge_to_limit", reason = "use a waproto::codec pinned wrapper (direct calls duplicate the decode tree per crate)" },
Comment thread
coderabbitai[bot] marked this conversation as resolved.
]

# 64-bit atomic *types*: 32-bit targets (Xtensa/ESP32) have no native AtomicU64/
Expand Down
1 change: 1 addition & 0 deletions src/appstate_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pub use wacore::appstate::Mutation;
pub use wacore::appstate_sync::{AppStateProcessor, AppStateSyncDriver, AppStateSyncError};

#[cfg(test)]
#[allow(clippy::disallowed_methods)]
mod tests {
use super::*;
use async_lock::Mutex;
Expand Down
21 changes: 20 additions & 1 deletion src/bot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1305,7 +1305,26 @@ impl BotBuilder<Provided, Provided, Provided, Provided> {
if let Some(override_) = self.device_props_override
&& !override_.is_empty()
{
info!("Applying device props override: {:?}", override_);
// Field-by-field to avoid Debug-formatting waproto types (keeps their
// generated Debug impls out of the binary).
info!(
"Applying device props override: os={:?} version={:?} platform_type={:?} history_sync_config={}",
override_.os.as_deref(),
override_.version.as_ref().map(|v| {
format!(
"{}.{}.{}",
v.primary.unwrap_or(0),
v.secondary.unwrap_or(0),
v.tertiary.unwrap_or(0)
)
}),
Comment on lines +1313 to +1320

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.

override_.platform_type.map(|p| p as i32),
if override_.history_sync_config.is_some() {
"overridden"
} else {
"default"
},
);
persistence_manager
.process_command(DeviceCommand::SetDeviceProps(override_))
.await;
Expand Down
1 change: 1 addition & 0 deletions src/client/device_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,7 @@ impl Client {
}

#[cfg(test)]
#[allow(clippy::disallowed_methods)]
mod tests {
use super::*;
use crate::lid_pn_cache::LearningSource;
Expand Down
1 change: 1 addition & 0 deletions src/client/lid_pn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,7 @@ impl Client {
}

#[cfg(test)]
#[allow(clippy::disallowed_methods)]
mod tests {
use super::*;
use crate::lid_pn_cache::LearningSource;
Expand Down
1 change: 1 addition & 0 deletions src/features/message_edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ pub fn decrypt_secret_encrypted_with_fallback(
}

#[cfg(test)]
#[allow(clippy::disallowed_methods)]
mod tests {
use super::*;
use wacore::message_edit::encrypt_message_edit;
Expand Down
6 changes: 6 additions & 0 deletions src/features/mex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,20 @@ impl<'a> Mex<'a> {
self.execute_request(request).await
}

#[inline]
async fn execute_request<V: Serialize>(
&self,
request: MexRequest<V>,
) -> Result<MexResponse, MexError> {
// Serialize the variables here so a caller-side serialization error
// surfaces as MexError::Json instead of a malformed empty request.
let spec = MexQuerySpec::new(request.doc, &request.variables)?;
self.execute_spec(spec).await
}

// Non-generic so the execute/error-handling body instantiates once, not
// per variables type.
async fn execute_spec(&self, spec: MexQuerySpec) -> Result<MexResponse, MexError> {
let response = self.client.execute(spec).await?;

// Check for fatal errors (the IqSpec already checks, but we want to return our error type)
Expand Down
14 changes: 9 additions & 5 deletions src/features/rotate_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@

use crate::client::Client;
use crate::request::IqError;
use buffa::Message;
use wacore::iq::prekeys::RotateSignedPreKeySpec;
use wacore::libsignal::protocol::{KeyPair, PrivateKey, PublicKey};
use wacore::libsignal::store::record_helpers::new_signed_pre_key_record;
use wacore::store::commands::DeviceCommand;
use waproto::whatsapp::SignedPreKeyRecordStructure;

/// Rotation cadence. This is the one value NOT grounded in the WA Web bundle
/// (there it is a persisted background job with a server-tuned schedule), so
Expand Down Expand Up @@ -110,7 +108,7 @@ impl Client {
.map_err(|e| anyhow::anyhow!("failed to load staged signed pre-key: {e}"))?
{
Some(bytes) => {
let s = SignedPreKeyRecordStructure::decode_from_slice(&bytes)
let s = waproto::codec::signed_pre_key_record_decode(&bytes)
.map_err(|e| anyhow::anyhow!("staged signed pre-key decode: {e}"))?;
let public = PublicKey::from_djb_public_key_bytes(
s.public_key
Expand Down Expand Up @@ -143,7 +141,10 @@ impl Client {
let record =
new_signed_pre_key_record(new_id, &kp, signature, wacore::time::now_utc());
backend
.store_signed_prekey(new_id, &record.encode_to_vec())
.store_signed_prekey(
new_id,
&waproto::codec::signed_pre_key_record_to_vec(&record),
)
.await
.map_err(|e| anyhow::anyhow!("failed to stage new signed pre-key: {e}"))?;
(kp, signature)
Expand All @@ -161,7 +162,10 @@ impl Client {
wacore::time::now_utc(),
);
backend
.store_signed_prekey(old_id, &old_record.encode_to_vec())
.store_signed_prekey(
old_id,
&waproto::codec::signed_pre_key_record_to_vec(&old_record),
)
.await
.map_err(|e| anyhow::anyhow!("failed to retain old signed pre-key: {e}"))?;

Expand Down
1 change: 1 addition & 0 deletions src/history_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ fn push_unique_sender(senders: &mut Vec<Jid>, sender: Jid) {
}

#[cfg(test)]
#[allow(clippy::disallowed_methods)]
mod tests {
use super::*;
use buffa::Message as ProtoMessage;
Expand Down
4 changes: 1 addition & 3 deletions src/message/msg_secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,12 +464,10 @@ impl Client {
info: &Arc<MessageInfo>,
payload: EncPayload,
) {
use buffa::Message as _;
use wa::MessageSecretMessage;
use wacore::bot_message::{BotMessageContext, decrypt_bot_message};
use wacore::protocol::nack::NackReason;

let ms_msg = match MessageSecretMessage::decode_from_slice(&payload.ciphertext) {
let ms_msg = match waproto::codec::message_secret_message_decode(&payload.ciphertext) {
Ok(m) => m,
Err(e) => {
log::warn!(
Expand Down
9 changes: 4 additions & 5 deletions src/message/special.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Special message types: newsletter, app-state key share, sender-key distribution.

use super::*;
use buffa::Message as _;

const APP_STATE_KEY_SHARE_SEND_ATTEMPTS: u8 = 3;
const APP_STATE_KEY_SHARE_SEND_RETRY: std::time::Duration = std::time::Duration::from_secs(1);
Expand Down Expand Up @@ -90,7 +89,9 @@ impl Client {
Some(KeyComponents {
key_id,
data,
fingerprint_bytes: fingerprint.encode_to_vec(),
fingerprint_bytes: waproto::codec::app_state_sync_key_fingerprint_to_vec(
fingerprint,
),
timestamp: key_data.timestamp.unwrap_or_default(),
})
}
Expand Down Expand Up @@ -182,9 +183,7 @@ impl Client {
continue;
};
let key_data = if let Some(stored) = key_store.get_sync_key(key_id).await? {
match wa::message::AppStateSyncKeyFingerprint::decode_from_slice(
&stored.fingerprint,
) {
match waproto::codec::app_state_sync_key_fingerprint_decode(&stored.fingerprint) {
Ok(fingerprint) => {
buffa::MessageField::some(wa::message::AppStateSyncKeyData {
key_data: Some(stored.key_data),
Expand Down
3 changes: 3 additions & 0 deletions src/message/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//! Tests for the message receive/decrypt pipeline.

// Tests/benches exercise the raw buffa API.
#![allow(clippy::disallowed_methods)]

use super::*;
use crate::store::SqliteStore;
use crate::store::persistence_manager::PersistenceManager;
Expand Down
1 change: 1 addition & 0 deletions src/passkey/flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,7 @@ pub(crate) async fn handle_passkey_continuation(client: &Arc<Client>, node: Arc<
}

#[cfg(test)]
#[allow(clippy::disallowed_methods)]
mod tests {
use super::*;
use crate::test_utils::{TestEventCollector, create_test_client, node_to_owned_ref};
Expand Down
1 change: 1 addition & 0 deletions src/pdo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ impl Client {
}

#[cfg(test)]
#[allow(clippy::disallowed_methods)]
mod tests {
use super::self_peer_target;
use wacore::store::Device;
Expand Down
11 changes: 6 additions & 5 deletions src/prekeys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,8 @@ impl Client {
};
let key_pair = KeyPair::generate(&mut rand::make_rng::<rand::rngs::StdRng>());
let record = new_pre_key_record(id, &key_pair);
use buffa::Message;
backend
.store_prekey(id, &record.encode_to_vec(), false)
.store_prekey(id, &waproto::codec::pre_key_record_to_vec(&record), false)
.await?;
self.persistence_manager
.process_command(DeviceCommand::SetPreKeyWatermarks {
Expand Down Expand Up @@ -516,8 +515,6 @@ impl Client {
// the async executor responsive. Records are encoded into one contiguous
// buffer with zero-copy Bytes slices instead of an alloc per record.
let (encoded_batch, generated) = wacore::runtime::blocking(&*self.runtime, move || {
use buffa::Message;

// Seed one CSPRNG and advance it per key, rather than reseeding from
// entropy on every iteration.
let mut rng = rand::make_rng::<rand::rngs::StdRng>();
Expand All @@ -536,7 +533,10 @@ impl Client {
let pre_key_id = gen_start + i as u32;
let key_pair = KeyPair::generate(&mut rng);
let start = buf.len();
new_pre_key_record(pre_key_id, &key_pair).encode(&mut buf);
waproto::codec::pre_key_record_encode_into(
&new_pre_key_record(pre_key_id, &key_pair),
&mut buf,
);
offsets.push((pre_key_id, start..buf.len()));
pubkeys.push((pre_key_id, key_pair.public_key));
}
Expand Down Expand Up @@ -1062,6 +1062,7 @@ mod tests {
}

#[cfg(test)]
#[allow(clippy::disallowed_methods)]
Comment thread
coderabbitai[bot] marked this conversation as resolved.
mod window_tests {
use wacore::libsignal::protocol::PublicKey;

Expand Down
2 changes: 2 additions & 0 deletions src/reexports_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
//! implement the async traits using only this crate's re-exports (no direct
//! buffa/bytes/anyhow/async-trait/chrono dependency of its own).
#![cfg(test)]
// Tests/benches exercise the raw buffa API.
#![allow(clippy::disallowed_methods)]

use crate as whatsapp_rust;
use whatsapp_rust::waproto::whatsapp as wa;
Expand Down
Loading