Replies: 2 comments 8 replies
|
I think this is a good write up of the problem, and potential paths.
eg. ConvoStorage and ClientStorage are treated as separate problem/solutions. If generic client wants to enforce that the storage provider implements both traits, that is a implementation detail. But perhaps I've missing something that requires these be strictly coupled.
This works well for a singular protocol version. We may want to consider using (proto, instance, key, value) => (GroupV2, e4a2..12b3, "enabled", "true"). This ensures that data is scoped to the protocol that generated it so maintenance, and sandboxing becomes easier. |
Quote from the post:
Namespace is
Not sure I am following this part, but if you are concerned some protocols might reach others' data, we can come up with a bound scope of some sort, example: let convo: Box<dyn Convo<S>> = match record.kind {
ConversationKind::GroupV1 => Box::new(GroupV1Convo::load(
cx, self.storage.scope(Namespace::GroupV1), record.local_convo_id)?),
ConversationKind::GroupV2 => Box::new(GroupV2Convo::load(
cx, self.storage.scope(Namespace::GroupV2), record.local_convo_id)?),
ConversationKind::Unknown(kind) => return Err(ChatError::UnsupportedConvoType(kind)),
};The above gives protocol isolation but not instance isolation, and I would keep it there. The type shapes its own keys. |
Ah that is much clearer. [Sand] I'd turn that "ns mls..." block into a markdown table, to cut down on confusion.
Ignore for now. |
Option A
Option B
I don't have strong opinions here and could be convinced with a compelling argument either way. But heres my rationale for B:
|
|
Agreed, I see more pros with option B. One catch: a conversation doesn't always know its id when it starts writing. On a welcome the id is the MLS group id, and that's inside the welcome we're decrypting. Works out fine though, decrypt and persist are separate steps: let staged = StagedWelcome::build_from_welcome(&kp_only, &cfg, welcome)?.build()?; // reads key packages, nothing group-scoped
let convo_id = hex::encode(staged.group_context().group_id().as_slice());
let mls = MlsAdapter::new(tx.scope(GroupV1, &convo_id), cx.key_packages());
let group = staged.into_group(&mls)?; // first group-scoped write |
Welcomes are encrypted with the HPKE key. Its not tied to any particular group or conversation at the moment of injestion. We can restructure the code to make that available via InboxV2, parse group then pass to the convo to be persisted. This does bring up an interesting critical section, we'll need to plan for. That convo initialization could fail, post group creation. Since openmls ha updated state - we'll need a recovery path. To re-initialize the convo or scrub the group. |
Uh oh!
There was an error while loading. Please reload this page.
I'm starting on #112 (expose and implement MLS storage). Before coding I want to settle how storage works across core, client, and app, and record it as an ADR in
docs/adr/. Three candidate shapes below; I want input before writing the decision down.Goals:
In the diagrams: amber = touched when a hypothetical next type ("V3") ships, red = broken by it, teal = the injected boundary, green = untouched.
A: one typed store, grown per domain
ChatStorestays a union of typed sub-traits and every backend implements each domain natively. Shipping V3: a new sub-trait, tables plus a migration, an implementation per backend, and a breaking change for every store implemented outside the repo.Strongest typing and real SQL for everything. But the boundary breaks on every new type, goals 2 and 3 fail for custom stores, and OpenMLS's 57-method
StorageProvidermakes external backends effectively unimplementable (a SQLite implementation of that surface alone is ~925 lines).B: a namespaced kv substrate; types own their schemas above it
The injected contract shrinks to six verbs over
(namespace, key, value)bytes: get, put, delete, scan_prefix, delete_prefix, and an atomic write scope, all taking&selfbecause OpenMLS requires it. The stock backend becomes one table:CREATE TABLE kv (ns TEXT, key BLOB, value BLOB, PRIMARY KEY (ns, key)). Everything typed lives above the seam inside libchat, written once: theStorageProvideradapter, thePeerScoreStorageadapter, the client's state layer, each type's own namespace. Shipping V3: code in libchat only; zero DDL, zero trait change, zero backend change.What MLS looks like over it
OpenMLS never exposes group internals as a query. The only way to get a group's details is
MlsGroup::load(storage, group_id), which fans out to twelve typed getters on the provider, so the adapter's whole job is composing a key per entity:Group ids are opaque bytes, so
group_keyhex-encodes them and the separators stay unambiguous. Reading a group back, and dropping one:That is also how OpenMLS's own reference store works: a flat map keyed by
label || serde_json(key) || version. Two deliberate departures: the group id leads, so one group is one prefix instead of fourteen group-scoped labels to visit on removal, and lists come fromscan_prefixrather than a separately maintained array of refs.Goals 2 and 3 met fully and backends become trivial (a SQLite table, a HashMap), so storage stays genuinely pluggable. The cost: nothing is SQL-visible anymore; listing is scan-and-decode, inspection sees blobs, schema discipline moves from DDL to serialization conventions, and removal is a delete-by-prefix sweep instead of a cascade.
C: hybrid, a typed ClientStore plus the substrate
Same substrate as B for everything conversation-type-owned. Client-level state goes below the seam instead:
ChatStorekeeps a typedClientStorefor the stack's own slow-moving domains (the conversation list the client hydrates at open, identity, whatever the client grows next). One trait for simplicity, possibly composed of smaller stores behind it. Shipping V3: same as B, code in libchat only, because a new kind is a row value, not DDL.Why this is worth a second idiom: client state is the part a human or a tool actually looks at. As rows with columns,
sqlite3and any DB browser answer "which conversations does this install have, of what kind, since when" with libchat out of the loop, which is what support and bug reports need; ordering and filtering come from the engine instead of a decode-and-sort in Rust; and constraints (primary key on the convo id, NOT NULL on the kind, foreign keys once the client grows a second table) are enforced by the database rather than by convention inside encoders. Identity secrets stay typed columns a backend can treat deliberately, rather than one more opaque blob.The cost is that a
ClientStorechange is a breaking migration for backends, and that is the bet worth arguing about: conversation types keep arriving, potentially every few weeks, while the client's own model (a list of conversations, one identity) is close to complete and should stabilise. All the churn sits on the substrate side, where it costs nobody a migration; the typed side changes rarely, in coordinated steps. If the bet proves wrong, folding client state into namespaces converges C back to B, and nothing in C blocks that.Scorecard
ClientStoretablesidentity+conversations, add the kv tableWhere I lean
C. It delivers B's property where it matters (the boundary a new conversation type would move), keeps the queryable state queryable, and is the smallest change from today's store.
All reactions