Always-async client with coexisting SQLite + Postgres backends (handoff prototype) - #4055
Draft
tylerhawkes wants to merge 15 commits into
Draft
Always-async client with coexisting SQLite + Postgres backends (handoff prototype)#4055tylerhawkes wants to merge 15 commits into
tylerhawkes wants to merge 15 commits into
Conversation
…ver maybe_async openmls fork Full sqlx/Postgres async storage track for herald-rs: - xmtp_db: async storage models (group, group_message, group_intent, refresh_state, identity), generalized XmtpDb/XmtpMlsStorageProvider, PgKeyStore, real sqlx PgDb Store/StoreOrIgnore/Fetch impls; sync (diesel/SQLite) track unchanged and green. - openmls_pg_storage: new sqlx-Postgres StorageProvider crate for the MLS crypto store. - xmtp_mls: maybe_async(AFIT) threading — transaction/savepoint -> impl Future + MaybeSend, generate_commit_with_rollback -> async closure, Store/Fetch AFIT, UFCS fetch pins. Async lib green; sync lib + sync --tests green (181 tests pass). - test infra: openmls test-utils sync-leak fix, memory_storage V_TEST maybe_async, MockSqlKeyStore async signatures, xmtp_db_pg_tests query suite.
…ed SAVEPOINT, integration coverage Builds the full async xmtp_mls client on the sqlx/Postgres storage track and covers it end-to-end. - PgMlsDb: the async XmtpDb store over a PgDb pool. - CBOR (CborCodec/ciborium) for OpenMLS serialization on Postgres, with the openmls/0-8-1-storage-format fix for the enum-by-name vs enum-by-tag asymmetry that broke CBOR round-trips. SQLite stays bincode. - Async KV: XmtpMlsStorageProvider read/read_list/delete/write are genuinely async over sqlx (no block_in_place); the sync track uses ready futures. - PgDb::savepoint: real Postgres SAVEPOINT nesting, the async analog of the sync track's nested diesel transaction. MLS welcome/commit processing nests an atomic sub-unit inside transaction() (xmtp_welcome's transaction().savepoint()) and that sub-unit must roll back independently; PgKeyStore::savepoint now maps to it instead of re-entering PgDb::transaction (which rejects nesting). - SQLite-tx guard: drive_to_completion panics on Poll::Pending so a suspending await inside a sync-track transaction fails loudly instead of deadlocking. - Parity guard: sync-side assert_db_query twin of pg.rs's, so a Query* method cannot be added to one backend and forgotten on the other. - migrations_pg: openmls_pg_storage's typed tables + the generic openmls_key_value. - xmtp_db_pg_tests: Postgres openmls-table coverage (KV + all 8 typed tables). - xmtp_mls_pg_tests (new, excluded crate): full async client over a live XMTP node + Postgres. Bidirectional two-client messaging and a three-member group with two concurrent welcomes. This coverage caught the SAVEPOINT bug above, which the herald smoke (bare registration only) could not.
…e-KV
Retires libxmtp's generic `XmtpMlsStorageProvider::{read,read_list,write,delete}`
(a stringly-typed `(label, key) -> bytes` API inherited from the SQLite
"everything is a KV" model) in favor of typed, named operations for the closed
set of values libxmtp actually stores:
set/get commit_log_signer_key(group_id)
set/get/delete key_package_reference(public_key)
set/get/delete key_package_wrapper_key(hash_ref)
The interface is track-agnostic; the storage LAYOUT is each backend's business:
- SQLite (SqlKeyStore): unchanged on disk — the impl keeps the openmls_key_value
KV, encoding the label + key internally, so bytes are byte-identical to before.
- Postgres (PgKeyStore): hits purpose-built tables (kp_references,
kp_wrapper_private_keys, commit_log_signer_keys) directly. Callers pass the RAW
group id / public key / hash ref, so the commit-log path no longer does the
bincode(group_id)->decode round-trip the generic label forced.
Because the operation set is closed and compile-time known, an unhandled value is
now a compile error, not a runtime fallback that could strand data — so the
generic openmls_key_value table, route_label dispatch and its panic guard are all
gone on Postgres.
Callers converted: identity (store_key_package_references + test helpers),
decrypted_welcome (find_key_package_hash_ref/find_private_key), commit_log_key
(CommitLogKeyStore), key_package_maintenance (delete_key_package).
Sequencing note (per plan): the typed interface + SQLite impl is a pure sync-track
change that can land FIRST as its own PR (async isn't in CI); the Postgres impl
(PgKeyStore + the typed tables) follows separately.
Verified: async lib + sync lib + xmtp_mls sync tests compile; xmtp_mls_pg_tests
(live node + Postgres) all green — registration, welcome processing, bidirectional
+ 3-member messaging via the typed accessors, commit-log key in its typed table
keyed by raw group id, no backup KV table. (Pre-existing: xmtp_db's own lib tests
don't compile on sync yet — the async Store/Fetch->AFIT migration deferred them;
untouched here.)
Kept as a separate change on top of the async-storage work for easy rollback.
…etch became async The async storage prototype made Store/StoreOrIgnore/Fetch/transaction return futures (AFIT); production code was threaded with .await but xmtp_db's own encrypted_store/* tests were deferred, leaving 322 compile errors on the sync lib-test target. This threads the awaits: - ~290 direct-terminal sites (model.store(conn).unwrap(), .store(conn)?, ...) awaited. - 5 sync test-helper fns (create_test_group / create_test_message / insert_group / generate_message_with_reference) made async, with .await inserted at their call sites via paren-matching so a `let x = helper(..)` binding awaits the CALL, not a later use of x (the binding-vs-usage distinction a naive await-at-error-site pass gets wrong). - 15 typed conn.fetch(&k) sites: method resolution cannot pick the Fetch<Model> impl through the .await.unwrap() chain, so rewritten UFCS as crate::Fetch::<Model>::fetch(recv, &k).await, with &recv where the connection is owned rather than a borrowed closure param. - 1 E0716: a store() future outliving its temporary receiver, inlined. xmtp_db test target compiles; nextest: 219 passed, 8 skipped. All changes are in #[cfg(test)] code that compiles only on the sync --tests build, so the sync/async libs and other crates are untouched. NOTE: `cargo check --workspace --tests` still fails ONLY on openmls_pg_storage (56 E0053) — a separate, pre-existing feature-unification leak (async-only crate built under is_sync), not touched here.
…utor at the sync FFI getters)
…e) config for the all-async client spike
…bench/test-utils build under both shapes - crates/xmtp_db_shape_bench: excluded criterion crate benching SqlKeyStore's openmls StorageProvider (write/read_signature_key_pair) over the same diesel backend, blocking vs ready-future (is_sync-off) shape; now_or_never drives the ready future so the async arm carries no executor overhead. Result: write path no change (p=0.69), cheapest read +~1.2% shape-attributable (~30-40ns/call). - xmtp_db: gate the hand-written SqlKeyStore mock + its automock to the blocking SHAPE (they call the delegate synchronously); matches the documented intent. - xmtp_mls: dev-deps xmtp_db/xmtp_archive -> default-features=false so building xmtp_mls's own benches doesn't unify openmls back to the blocking shape; +2 bench-support .await after Store/Fetch became permanent-AFIT.
…om the ready-future shape (stable generate_commit_with_rollback)
…-suite maybe_await) Makes libxmtp test infrastructure compile under the async (ready-future SQLite) shape while keeping the blocking track green. - xmtp_db MockSqlKeyStore: maybe_async(AFIT) + async fn + .await on the 55 in-memory delegates (mirrors the real impl); un-gated to both shapes; traits.rs automock + mock module gates restored to the sync (diesel-backend) track. - xmtp_api_d14n: blocking marker feature + cfg-split the test-util KeyPackage build (now_or_never on async), forwarded by xmtp_mls blocking. - test suite: 12 shape-dependent openmls calls in test bodies to maybe_await!; openmls-framework tests (openmls_mock, welcome_sync mod tests, update_group_membership::applies_group_membership_intent) gated to blocking until openmls multi-client test framework is threaded for async. Both test harnesses compile: async on STABLE + blocking; full --workspace --tests green.
…y async openmls is left exactly as-is (its maybe_async machinery intact) but is now depended on unconditionally in its ASYNC shape: nothing enables maybe-async/is_sync or openmls/sync any more. Ready-future SQLite (diesel storage futures resolved with one poll) is the only shape; sqlx/Postgres is the other backend. `sync`/`async` remain as BACKEND selectors (rename to sqlite/sqlx is a follow-up). - Manifests: removed the `blocking` feature from xmtp_db, xmtp_mls_common, xmtp_archive, xmtp_mls, xmtp_api_d14n (marker), bindings/node, bindings/wasm; default is now ["sync"]. xmtp_db_shape_bench is async-only. - maybe_await! is now an unconditional `.await` passthrough (inlining the ~53 call sites is a cosmetic follow-up). generate_commit_with_rollback diesel def gated on `sync`; the sqlx def + nightly gate stay `all(async, not sync)`. - The now_or_never fake-executor arms (group_save, d14n envelope_builder, mls_validation_service test helper) are unconditional; ~31 xmtp_db test bodies got unconditional `.await` (blocking is gone, so no maybe_await needed). - Re-homed the 3 openmls-test-framework tests onto the async openmls MlsGroup API (openmls_mock rewritten self-contained; welcome_sync mod + applies_group_membership_intent). 14 welcome_sync + 1 membership test PASS. Verified STABLE (no RUSTC_BOOTSTRAP): full --workspace --tests, wasm binding; sqlx/herald still builds under bootstrap. openmls fork unchanged.
…ro (blocking is gone)
… one build Retiring the blocking feature removed the maybe-async is_sync global switch that used to collapse the async track onto sync whenever both features were on. Without it, --all-features (sync + async) tried to compile BOTH storage backends and collided: duplicate DefaultStore/EncryptedMessageStore definitions and impls, and dangling PgDb: Query* bounds whose impls were (correctly) gated not(feature=sync) while their consumers were not. just lint-rust runs clippy --all-features, so CI could not go green. Make sync dominant: every async-EXCLUSIVE backend gate now carries not(feature=sync), matching the handful that already did (e.g. PgMlsDb). This includes the gate the xmtp_macro::PgModel derive emits (crates/xmtp_macro), which was the source of 20 crate::pg references surviving under --all-features. So sync wins whenever it is on -- --all-features and the default both build the SQLite track; --no-default-features --features async still builds the Postgres track. The two backends are now mutually exclusive in one binary, which is fine: no shipping target needs both (mobile/node/wasm = SQLite, herald = Postgres). Left the one any(feature=sync, feature=async) either-backend gate untouched. Also add a missing .await in a d14n migration test that only --all-features compiles.
The two backends are selected by cargo features. Name them for what they ARE — `sqlite` (diesel/SQLite, the mobile/node/wasm shipping backend) and `sqlx` (sqlx/Postgres, the herald/server backend) — instead of `sync`/`async`, which described the OLD blocking-vs-async client shape that no longer exists (the client is unconditionally async on both backends). Pure rename of the ~639 cfg gates and the feature declarations/forwards; no behaviour change.
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
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.
Always-async libxmtp client with coexisting SQLite + Postgres backends (handoff)
What this is
Reworks the client to be unconditionally async with two storage backends selected by
cargo feature, coexisting in one codebase:
sqlite(default, dominant) — diesel / encrypted SQLite. What mobile / node / wasm ship.Every
sqlximpl is gatednot(feature = "sqlite"), so--all-featuresstill compiles theSQLite backend and keeps its on-disk format.
sqlx— sqlx / Postgres, for server-side deployments (herald). Async all the way down.The old blocking/
maybe_await!split is gone — the client is async everywhere; SQLite runs itssynchronous work behind an immediately-ready future.
Structure (15 commits, oldest → newest)
prototype(xmtp_db,xmtp_mls,openmls_pg_storage): async storage track over the maybe_async openmls fork (= the oldtyler/asyncdbbase)xmtp_db: typed storage accessors replacing the generic byte-KV.awaitthrough the storage-layer tests after Store/Fetch became asyncopenmls_pg_storagefrom the sync workspace build.awaitthreading in apps + bindings (fake-executor at the sync FFI getters)xmtp_db: stand up is_sync-off + diesel (ready-future SQLite) configxmtp_db_shape_bench)xmtp_mls: drop nightlyasync_fn_traits/unboxed_closuresfrom the ready-future shapemaybe_await)xmtp_mls: inlinemaybe_await!to.awaitand delete the macrosync→sqlite,async→sqlxDependencies (why CI won't be green here)
This builds on the maybe_async openmls fork, which is itself a stack of open PRs on
xmtp/openmls:mainup to upstream openmls v0.9.0 (validated, MERGEABLE).#1923) + sqlx Postgres CI,plus the Send/MaybeSend-for-wasm, CBOR-derive, and test-framework-gating follow-ons.
main(a ~14-file conflict merge across the core MLS group logic — deferred, not done here).
Point the openmls dep at the fork's maybe_async branch to build the
sqlxshape.Build / feature model
sqliteandsqlxselect the backend;sqliteis the default and dominant one.sqlxpath uses a few nightly features (async_fn_traits,unboxed_closures), scoped toxmtp_db/xmtp_mlsvia.cargo/config.tomlRUSTC_BOOTSTRAPso the rest builds on stable.accepts either the storage tag or the variant name so both share the enums with no migration.
(which would change the SQLite on-disk format).
Known state / next steps for whoever picks this up
xmtp_db(sqlx) compiles and has live-Postgres + 2-client round-trip coverage(
xmtp_mls_pg_tests, an excluded crate needing a live node + Postgres).xmtp_mlsasync integration is in progress; the sqlx shape depends on the openmls fork worklanding / being reconciled with v0.9.0.
xmtp_db_pg_tests,xmtp_mls_pg_tests,xmtp_db_shape_bench) carry their ownlockfiles that stale when the workspace-hack changes.
Note
Make all DB query traits async and add coexisting Postgres/sqlx backend
Query*trait,Store/Fetch/StoreOrIgnoretrait, andStorageProvidermethod from synchronous returns to async futures (impl Future + MaybeSend), so all callers acrossxmtp_mls, bindings, and tools must.awaitdatabase operations#[cfg(feature = "sqlite")]and adds a new#[derive(PgModel)]macro (pg_model.rs) that generatessqlx::FromRowimpls and table/column metadata for the Postgres trackPgDbhandle (pg.rs) with pooled/transaction-aware connection management, plus newpg_implmodules in every encrypted-store file providing async sqlx-based implementations of all query traitsopenmls_pg_storagecrate with a fullStorageProviderimpl for OpenMLS entities backed by sqlxdrive_to_completion(transactions.rs), which panics if the closure's future is not immediately readynow_or_never().expect(...)and will panic if the underlying future is not immediately ready; all other methods became async and return Promises/futuresdrive_to_completionintransactions.rswill panic on any suspending await inside a SQLite transaction body;now_or_never().expect(...)calls in wasm/mobile bindings (conversation.rs, mls.rs) will panic if a storage future is not ready on first poll; thepg_implmodule'sSELECTquery ind14n_migration_cutover.rscontains a literal\ +sequence in the SQL string that reviewers should verifyMacroscope summarized 4c5144f.