fix: signer-scoped DirectV1 routing by osmaczko · Pull Request #162 · logos-messaging/libchat

osmaczko · 2026-07-02T20:02:56Z

Core is no longer account-aware: the client resolves an account address
to signer ids via the account directory, and the signer's verifying-key
hex serves as registry key, inbox subscription, and Welcome routing
target end to end. The MLS credential stays the full id().

GroupV2 reads the de-mls member id from the fetched key package and
maps it to the signer id the welcome is delivered to.
All account machinery (directory trait, bundle codec, resolution)
moves out of core into logos-account; the RegistrationService
supertrait and Core::account_directory() are gone, and the client
holds its own directory handle.
The account exposes functionality, never a signer: add_delegate_signer
does the lamport upsert and signs internally.
Every client acts for an account (ChatClientBuilder::new(account)).
DelegateSigner is a pure keypair; the client composes the wire
credential from the signer and the account, so the association is
client state. addr() is the account address.
resolve_device_ids fails fast (NotAnAccountKey / NoDeviceBundle /
Directory) instead of falling back to treating an unresolved address
as a signer id. LogosChatClient::open and chat-cli mint and publish a
dev account each launch.
EphemeralRegistry keys key packages by hex pubkey like HttpRegistry.

Supersedes #155 (routing_id).

jazzz · 2026-07-02T20:34:06Z

+// The test account holds its key locally, so it can act as its own signing
+// authority for device bundles.
+impl AccountAuthority for TestLogosAccount {
+    type Error = std::convert::Infallible;


In general this is the correct place for account related functionality 👍

jazzz · 2026-07-02T20:36:47Z

+    fn sign(&self, payload: &[u8]) -> Result<crypto::Ed25519Signature, Self::Error> {
+        Ok(IdentityProvider::sign(self, payload))
+    }


[!] Exposing access to an account signer is a bad design choice. Accounts should provide functionality (Add, Revoke, Rotate) and not provide dangerous surface areas.

jazzz · 2026-07-02T20:41:39Z

+        let retrieved = registry
+            .retrieve(signer.as_str())
+            .map_err(|e| ChatError::Generic(e.to_string()))?;
+        let Some(keypkg_bytes) = retrieved else {
+            return Err(ChatError::Protocol(format!(
+                "no keypackage for signer {signer}"
+            )));
+        };


This looks good.

Use the RegistryProvider to fetch KeyPackages based on the local identities.

jazzz · 2026-07-02T20:45:04Z

-        let sender_id = cx.mls_identity.id().as_str();
-        let reliable = cx.causal.on_send(&self.convo_id, sender_id, content);
+        // Causal-history sender hint: the signer routing id, the same
+        // identifier every other wire-level reference to this signer uses.
+        let sender_id = hex::encode(cx.mls_identity.public_key().as_ref());
+        let reliable = cx.causal.on_send(&self.convo_id, &sender_id, content);


[Sand] These changes don't look necessary. Avoid altering casual if not needed

jazzz · 2026-07-02T20:51:59Z

+                    if let Some(i) = self.pending_invites.iter().position(|(p, _)| p == joiner) {
+                        let (_, routing_id) = self.pending_invites.remove(i);


[!] I'd have to check this, hard to tell if this is the correct Identifier to be using.

This value should match the Id of the IdentityProvider which generated the keypackage

jazzz · 2026-07-02T20:53:24Z

-/// such as MLS.
+/// A PQ focused Conversation initializer.
+/// InboxV2 is signer-scoped: it receives invites under this installation's
+/// signer routing id, supporting PQ based conversation protocols such as MLS.


[Sand] Weary of introducing yet another concept 'routing id'. IdentId could be improved, however feels like we should wait until it stablizes.

jazzz · 2026-07-02T20:55:40Z

+/// Fetches the current (verified) device set, adds the device if absent, bumps
+/// the lamport, re-signs with the account key, and publishes. Safe to call
+/// repeatedly — an unchanged set is simply re-published, which also refreshes
+/// the server's retention clock. Account key custody stays outside libchat:
+/// the injected [`AccountAuthority`] is only ever asked to sign.


This is not the the correct place for this logic to exist

Core is no longer account-aware: the client resolves an account address to signer ids via the account directory, and the signer's verifying-key hex serves as registry key, inbox subscription, and Welcome routing target end to end. The MLS credential stays the full id(). - GroupV2 reads the de-mls member id from the fetched key package and maps it to the signer id the welcome is delivered to. - All account machinery (directory trait, bundle codec, resolution) moves out of core into logos-account; the RegistrationService supertrait and Core::account_directory() are gone, and the client holds its own directory handle. - The account exposes functionality, never a signer: add_delegate_signer does the lamport upsert and signs internally. - Every client acts for an account (ChatClientBuilder::new(account)). DelegateSigner is a pure keypair; the client composes the wire credential from the signer and the account, so the association is client state. addr() is the account address. - resolve_device_ids fails fast (NotAnAccountKey / NoDeviceBundle / Directory) instead of falling back to treating an unresolved address as a signer id. LogosChatClient::open and chat-cli mint and publish a dev account each launch. - EphemeralRegistry keys key packages by hex pubkey like HttpRegistry. Supersedes #155 (routing_id).

kaichaosun · 2026-07-03T14:39:28Z

 }

+impl Default for TestLogosAccount {
+    fn default() -> Self {


[nitpick] have a default value for account looks weird.

jazzz

As a fix to continue developement I think its great, and have no blockers.

This approach is generally in the right direction.

Removing AccountDirectory from core
Cleaning up Identifiers.
Introduction of a TestIdent

Things that need some more work in the future:

Updates to the builder pattern will be hard to maintain
Account crate needs to own all account operations
Strongly typed Identifiers with canonical Identifiers will cut down on confusion

jazzz · 2026-07-03T19:45:25Z

+    pub fn add_delegate_signer<D: AccountDirectory>(
+        &self,
+        directory: &mut D,


[Sand] Making this function generic over D is problematic. The issue is that this implementation still relies on the keypackage provider to perform account operations.

jazzz · 2026-07-03T19:52:35Z

-    impl AccountDirectory for NoopRegistration {
-        type Error = std::convert::Infallible;
-
-        fn publish(
-            &mut self,
-            _bundle: &SignedDeviceBundle,
-        ) -> Result<(), <Self as AccountDirectory>::Error> {
-            Ok(())
-        }
-
-        fn fetch(
-            &self,
-            _account: &Ed25519VerifyingKey,
-        ) -> Result<Option<DeviceSet>, <Self as AccountDirectory>::Error> {
+        fn retrieve(&self, _device_id: &str) -> Result<Option<Vec<u8>>, Self::Error> {


jazzz · 2026-07-03T20:01:00Z

+    pub fn new(account: impl Into<String>) -> Self {
        Self {
            ident: Unset,
+            account: account.into(),


[Pebble] The account address is insufficient. The end result requires that the Identity type is replaced with an account object.

jazzz · 2026-07-03T20:03:17Z

-    R: RegistrationService + Send + 'static,
+    R: RegistrationService + AccountDirectory + Clone + Send + 'static,


[Sand] These are categorically two different services, and should eventually not be collocated.

jazzz · 2026-07-03T20:09:16Z

            .lock()
            .unwrap()
-            .insert(identity.id().to_string(), key_bundle);
+            .insert(hex::encode(identity.public_key().as_ref()), key_bundle);


[Sand] Building non canoncial representations leads to bugs, and difficulty debugging. If a representation does not exist it should be added to the type.

The issue here is that IdentId is poorly designed and lacks clarity

osmaczko · 2026-07-03T21:17:54Z

@jazzz @kaichaosun thanks for the review. I’ll address your feedback in a follow-up. This is needed for logos-chat-module, hence the rush.

osmaczko mentioned this pull request Jul 2, 2026

fix: subscribe InboxV2 on routing_id, not the credential id #155

Closed

osmaczko force-pushed the fix/signer-scoped-directv1 branch 2 times, most recently from 20215aa to fdb4479 Compare July 2, 2026 20:23

jazzz reviewed Jul 2, 2026

View reviewed changes

osmaczko force-pushed the fix/signer-scoped-directv1 branch from 7031847 to 7a4c2ab Compare July 3, 2026 09:35

osmaczko changed the title ~~fix: signer-scoped invite routing; resolve accounts at the client layer~~ fix: signer-scoped DirectV1 routing Jul 3, 2026

osmaczko force-pushed the fix/signer-scoped-directv1 branch 4 times, most recently from 6e53ea6 to 00350a6 Compare July 3, 2026 10:52

osmaczko force-pushed the fix/signer-scoped-directv1 branch from 00350a6 to 518d6c1 Compare July 3, 2026 12:01

osmaczko mentioned this pull request Jul 3, 2026

feat: bump libchat to origin/main; migrate to DirectV1 logos-co/logos-chat-module#37

Open

osmaczko marked this pull request as ready for review July 3, 2026 13:11

osmaczko requested review from jazzz and kaichaosun July 3, 2026 13:11

kaichaosun approved these changes Jul 3, 2026

View reviewed changes

jazzz approved these changes Jul 3, 2026

View reviewed changes

osmaczko merged commit c09459c into main Jul 3, 2026
5 checks passed

osmaczko deleted the fix/signer-scoped-directv1 branch July 3, 2026 21:18

		if let Some(i) = self.pending_invites.iter().position(\|(p, _)\| p == joiner) {
		let (_, routing_id) = self.pending_invites.remove(i);

		R: RegistrationService + Send + 'static,
		R: RegistrationService + AccountDirectory + Clone + Send + 'static,

Uh oh!

Conversation

osmaczko commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jazzz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

osmaczko commented Jul 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

osmaczko commented Jul 2, 2026 •

edited

Loading