Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Message History feature doc and more group chat code samples #775

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

jhaaaa
Copy link
Collaborator

@jhaaaa jhaaaa commented Jul 8, 2024

Copy link

vercel bot commented Jul 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
junk-range-possible ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 10:40pm

Copy link

cloudflare-workers-and-pages bot commented Jul 8, 2024

Deploying xmtp-org with  Cloudflare Pages  Cloudflare Pages

Latest commit: ae42d8f
Status: ✅  Deploy successful!
Preview URL: https://38297b6a.website-9re.pages.dev
Branch Preview URL: https://message-hist.website-9re.pages.dev

View logs

docs/build/message-history.md Outdated Show resolved Hide resolved
docs/build/message-history.md Outdated Show resolved Hide resolved
@jhaaaa jhaaaa requested a review from nplasterer July 9, 2024 01:34
@jhaaaa jhaaaa changed the title Add Message History feature doc Add Message History feature doc and more group chat code samples Jul 9, 2024

To enable your users to avoid losing access to their local databases, allow them to store their local cache in iCloud or Google Cloud, for example. This option will enable message persistence within a single app ecosystem.
If you choose to not enable message history, you can help users avoid losing access to their local databases by allowing them to store their local cache in iCloud or Google Cloud, for example. This option will enable message persistence within a single app ecosystem.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we still advice at the very least adding a url on create so other apps may benefit from message history.


If you choose to provide neither message history nor local cache storage, be sure to clearly communicate the concept of losing access to group chat messages to your users. For example, you might consider using this language:

> If you log out of <app name> and log into a different app on this device, or delete <app name> from this device, you will lose access to all group chat messages you sent using this installation of <app name> on this device.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry though you will still have access to your group chat conversations. Just non of the previous messages in them.


## Set message history sync URL

When your app creates a new client, specify the location where the client should sync message history.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether you want to use the other message history features or not we recommend atleast setting this url. This allows other apps to pull in message history in the future. The easiest way would be to use our history sync url.

enableV3: true,
appVersion: 'YourApp/1.0.0',
// Add the history sync URL option
historySyncUrl: 'SYNC_URL'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should set our message history sync url here? Or should the sdk by default set it to the ephemera one? @tuddman did we decide if the SDK is going to set a message history url by default if they don't provide one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the idea of setting it to the Ephemera message history server URL by default... This way devs who aren't implementing message history won't have to do anything to ensure that they support the UX of other apps.

If anything, in the docs we can just disclose that this URL is set by default and if you don't want to participate - you can remove it.

However - is there any harm in having an app's message history collected on this server without their explicit permission?

Copy link

@tuddman tuddman Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's really two options to set here:

  1. should message-history-sync be enabled (by default)?
  2. and if so (enabled), what URL should the app use? And if not set, what is our default?

We decided that option-1 - should be the case in the SDK. Message history sync ability- should be enabled by default.

Ephemera's History server URL is http://history.dev.xmtp.network/
We can spin up an additional server which will be accessible at http://history.production.xmtp.network/ but the latter is not live yet.

We can set option 2 - if not otherwise provided - to that URL.

However - is there any harm in having an app's message history collected on this server without their explicit permission?

An app's message history is NOT collected on any server without their explicit permission. Even if it were, its all encrypted so it's safe in that respect, but it's not stored without their explicit permission, so a non-issue. The nuance is that new-device explicitly requests a history-sync, and it is the responding device with the same inbox-id that responds automatically. But as the inbox owner is in all cases presumptively the same person, it's really the case that they are providing explicit permission.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this, @tuddman and @nplasterer! I have just a few comments and questions, please...

  • Is message history sync ability enabled by default if the historySyncUrl needs to be set manually by the dev? The functionality is there - but it isn't enabled until the dev sets this value, is that correct? If we were to set the historySyncUrl value to http://history.dev.xmtp.network/ without dev intervention, I think this would truly be enabled by default?

  • Regarding Ephemera's history server URL http://history.dev.xmtp.network/ - I wonder if we should adjust the URL to make it clear that it is a centralized server run by Ephemera?

    • Maybe history.dev.ephemera-xmtp.network?
  • Regarding the future URL http://history.production.xmtp.network/ - I don't think it needs to include "production" and can be just http://history.ephemera-xmtp.network/, for example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense for it to have the name ephemera in it! @tuddman can you make that change?

The ability to get the messages into another installation is enabled by default if we include a default url. But requesting message history is not enabled by default if that makes sense. The app you are using still needs to make a request for a message history but with this default url being set it atleast gives us a better chance of an app having a message history to sync from.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it, @nplasterer - absolutely understood.

Providing a default message history server URL helps ensure that App A can provide message history for other apps to use without any manual steps required for the dev of App A.

Comment on lines 46 to 49
ClientOptions.Api(XMTPEnvironment.PRODUCTION, true),
enableV3 = true,
appContext = context
historySyncUrl = "SYNC_URL"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR but we recently started requiring the dbEncryption key is passed. So maybe we should change all of these client options to include a dbEncryption key to make that clear.

@jhaaaa jhaaaa requested a review from rygine July 11, 2024 01:46
<TabItem value="swift" label="Swift" attributes={{className: "swift_tab"}}>

```swift
try await client.requestMessageHistorySync()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
try await client.requestMessageHistorySync()
try await client.requestHistorySync()

<TabItem value="kotlin" label="Kotlin" attributes={{className: "kotlin_tab"}}>

```kotlin
client.requestMessageHistorySync()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
client.requestMessageHistorySync()
client.requestHistorySync()

Copy link
Contributor

@nplasterer nplasterer Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think this is named correctly. I wanted it to be more clear what we are syncing especially with potential conversation and personal preference syncs coming in the future. Open to revisiting this wording if you think it should just be history sync.

https://github.com/xmtp/xmtp-android/blob/main/library/src/main/java/org/xmtp/android/library/Client.kt#L637-L639

<TabItem value="rn" label="React Native" attributes={{className: "rn_tab"}}>

```jsx
await client.requestMessageHistorySync()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
await client.requestMessageHistorySync()
await client.requestHistorySync()

docs/build/message-history.md Outdated Show resolved Hide resolved
Calling `sync()` for a group or groups gets any updates since the last sync and adds them to the local database. Be sure to periodically synchronize each group chat to ensure your app has the latest group chat details, including the most recent messages, member list, and group chat details, for example.
Calling `sync()` for a group or groups gets any updates since the last sync and adds them to the local database.

Sync group chats anytime a user accesses a conversation view. This sync is especially important if a user backgrounds (switches away from) your app, then reopens the app and accesses a conversation view.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we say "conversation view" - just to be sure - Is this the view when a user opens a group chat and can see the messages in the group chat?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly.


Sync group chats anytime a user accesses a conversation view. This sync is especially important if a user backgrounds (switches away from) your app, then reopens the app and accesses a conversation view.

When the user reopens the app, the app might be using an old epoch (snapshot) of data. This is even more likely to be true if there were many group membership changes during the time the app was backgrounded. A group chat sync bumps the app to the latest epoch, helping to ensure that your user always sees the current group chat, including the most recent messages, member list, and group chat details, for example.

Updates are also retrieved and added to the local database when streaming and when the user takes an action.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unclear on what this sentence above means. Is this saying that streaming and user actions automatically trigger "sync group chat"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an automatic sync but streaming does receive messages and add them to local users database. My understanding is if a commit message is streamed (adding a user to the group) it should update the epoch in the stream.


Sync group chats anytime a user accesses a conversation view. This sync is especially important if a user backgrounds (switches away from) your app, then reopens the app and accesses a conversation view.

When the user reopens the app, the app might be using an old epoch (snapshot) of data. This is even more likely to be true if there were many group membership changes during the time the app was backgrounded. A group chat sync bumps the app to the latest epoch, helping to ensure that your user always sees the current group chat, including the most recent messages, member list, and group chat details, for example.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could make it clearer what an old epoch is. An old epoch is an old decryption/encryption key for the group. Every time a member is added or removed the decryption/encryption keys get rotated so that old members cannot decrypt messages going forward and no members can not decrypt old messages from the group before they joined etc... If an app doesn't sync often the epochs for a user can get so far behind that they can no longer decrypt messages (read new messages) or encrypt messages (send messages that can be decrypted by others in the group). This is why syncing often is important.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants