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

Optimistically store message in libxmtp DB on message send #529

Closed
richardhuaaa opened this issue Feb 21, 2024 · 2 comments
Closed

Optimistically store message in libxmtp DB on message send #529

richardhuaaa opened this issue Feb 21, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@richardhuaaa
Copy link
Contributor

richardhuaaa commented Feb 21, 2024

On message send, libxmtp does a sophisticated publishing flow that may take multiple network round trips to resolve. Integrators should be able to optimistically render that message in their UI before the send has been confirmed to be published. To enable that, we can write the message to our database as soon as the send is requested, with a "state" field of UNPUBLISHED, and update it after publishing to PUBLISHED. That way, integrators can just query the messages, and decide whether or not they want to render the UNPUBLISHED messages.

The message ID currently depends on a server timestamp, and therefore can only be determined once the message is published. In order to optimistically write the message to the database, we need to modify how the message ID is determined, and use a client timestamp instead. The client timestamp is not included on the message payload anywhere, so others won't be able to determine the message ID until we add it onto the payload.

  1. Write an envelope proto for the encrypted MLS payload that wraps encoded content. Add a field to the envelope called ‘sender_idempotency_key’ or similar.
  2. (Breaking change) Run dev/gen_protos.sh in libxmtp and implement the proto (serialization on message send and deserialization on message receive), including setting the new field on message send to the sender's client timestamp. Once landed, notify everyone that it is a breaking change.
  3. Update how message_id is generated to use the new field - previously used the group_id, hash of the decrypted message contents, and the server timestamp - now we can replace the server timestamp with (sender_account_address, sender_client_timestamp). Add “state” field to group_messages table (default it to PUBLISHED in the SQL migration, but make it a required field in the code going forward). Insert an optimistic row on message send with state UNPUBLISHED, and update it to PUBLISHED after commit publishing flow.

Other thoughts:

  • The send() function won't return for integrators until the full publish flow is complete. In future, we could consider having it return early and do the rest in a background thread. Not sure if this matters or not/how multi-threaded integrators are already.
  • The sent message won't be returned via message streams until it is published and then read back from the server. Unsure if this matters or not.
  • Integrators can't subscribe to changes on message state. I assume this is too sophisticated for now, and they could also determine that the state has transitioned to PUBLISHED once they read it back from the stream.
@neekolas
Copy link
Contributor

I have a PR up for the envelope (not sure about the naming, open to suggestions).

I also added some ideas of where we should use this, and where we should store the two fields separately. Feel free to weigh in here on usage. Think we want to get that clear so that we don't have callers expecting one thing and getting another.

@insipx
Copy link
Contributor

insipx commented Aug 28, 2024

This should be fixed, rel. #881 #544

@insipx insipx closed this as completed Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants