Update Context to accept External Identity Provider.

[jazzz]

Problem

libchat::Core creates and owns an "Account" which is used as an Identity provider. This creates two issues.

1. Account refers to a high level object. An Account contains multiple associated ApplicationIdentities. Context should be unaware of Accounts and only concerned with an Application instance specific representation which statisfies IdentityProvider

2. Hard to update which IdentityProvider is used as its hardcoded into the system.

Solution

This PR addresses the two issues by allowing clients to pass in an IdentityProvider, and removes references to accounts.

• Moves Identity provider to a dedicated crate logos-traits
  • This crate can be used by other projects which want to expose common functionality across services and projects.
• Removes all references to "Account" within libchat::core.
  • Codifies that the chat core operates on Identities which are app installation specific.
• Updates Core to accept an external IdentityProvider instance.
• Creates Account instance in Client which is then passed to Core.

Deferred Work

• Update Accounts to have a Application instance specific representation. Current uses existing TestLogosAccount object.
• Simplify Integration tests.

Notes

• Reviewing by commits may be an easier approach from reviewers. The changes are broad reaching but repetitive

[kaichaosun]

How's the id difference with public_key? Is it like a one way conversion like ethereum public key to eth address? If it's so, we probably need to extract such conversion method out of identity provider to make calls layered correctly.

[jazzz]

At the trait level there is no enforced relation between the ID and the publicKey.

1 provides a longterm Identifier
1 provides a valid publicKey used for validation

Consumers cannot assume there is any correlation or conversion between the two

[kaichaosun]

If ID really needed in a decentralized system?
It's unclear to me how ID is created, transferred or maintained. If there is no clear definition of it, better remove it for now, and bring it back when necessary.

[jazzz]

If ID really needed in a decentralized system?
Yes. There needs to be some identifier used for representing an identity. E.g. Without it a recipient could not identify who the sender is.

[kaichaosun]

[nitpick]
This PR is not introducing ID, but the ID field has been bothering me for quite a long time, as in the tests we simply give it a name. In some tasks (account/keypackage store) I have to depend on the public key for such account identifying because we need signature validation capability on the ID.

If there is no ID definition yet (like ethereum address, did), we should not bring it into code, stick with display name and public key will be sufficient and less reasoning context.

[kaichaosun]

Logos identity could bring extra capabilities that libchat may not need, so let libchat depends on logos-trait directly brings too much uncertainty. Instead we can clearly design our requirement in libchat and have an adapter to bridge the logos-account to libchat identity provider once logos-account is built.

Logos-traits seems promising direction overall, but we need to make such concept out of libchat, and reach a consensus as it evolves among different teams. Such communication can be time consuming, a practical way is that logos has account module ready, then build the adapter for libchat's account requirements.

[jazzz]

If I understand correctly, I think you are making things more complicated than they need to be.

and reach a consensus as it evolves among different teams.

This crate is used exclusively in this project, for its own purposes. Any future considerations are out of scope for now.

let libchat depends on logos-trait directly brings too much uncertainty.

Long term I agree, and once there is more infrastructure it will be easier to separate. But today the goal is that Output from Logos Accounts can be used to satisfy chats definition of a IdentityProvider.

logos-traits is currently being used to provide trait definitions across separate concerns (Accounts + Chat). If the definition exists in the Core , then Accounts needs to import core. If the definition exists in Accounts then Core needs to import Accounts. Instead its placed in a neutral place.

I'm not sure what changes you are asking for, can you be more clear about the suggestion?

[kaichaosun]

[request change]
The crate is named logos-trait, the scope is implicitly extended to other modules of logos ecosystem, like storage, blockchain. So I assume this crate will add more structs/traits that not needed in libchat.

But since you clearly mentioned This crate is used exclusively in this project, we need to reduce the scope to be chat (not logos), also I feel trait is not a good way for crate name, but I don't have a better one.

[jazzz]

Changed name to "shared_traits" until it the coupling issues can be addresses later.

Unfortunately the pattern being used is referred to a "trait-crate" so excluding the term "trait" is difficult.

[kaichaosun]

👍 feel free to merge after the last request change.

[osmaczko]

[?] Seems to be wrong rebase artifact?