Skip to content

Event cache: Move decryption retry in there #3872

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

Open
1 of 2 tasks
bnjbvr opened this issue Aug 21, 2024 · 1 comment
Open
1 of 2 tasks

Event cache: Move decryption retry in there #3872

bnjbvr opened this issue Aug 21, 2024 · 1 comment

Comments

@bnjbvr
Copy link
Member

bnjbvr commented Aug 21, 2024

Right now, it's the Timeline API that takes care of retrying decryption of an event. This has a few bad consequences:

  • retrying decryption, while it could be done in other contexts than the Timeline API, can only happen there
  • some observers (e.g. the UTD hook) want to know when a previously-UTD event could be decrypted; this happens in the Timeline code too, which is an abstraction leak
  • some other code basically retries decryption on its own: the latest event code in the base crate does that

I'm proposing that we move all of this over to the Event Cache. An event in a linked chunk / on disk would be replaced by its decrypted equivalent, if needs be, and we can implement a few enhancements mentioned in a few other issues too.

High-level rough plan

Names to be refined.

  • We have the UTD hook / manager, which helps notifying whenever there's a UTD.
    • It should be moved in a central location, could be in the Client for instance.
  • The timeline needs a way to call into the UTD hook, to report that a UTD event has been displayed.
  • I propose to have a new Redecryptor component which role is to react to
    1. new UTD events coming from sync
    2. new interesting events / notifications that some UTDs may be decryptable now
    • generic over a new trait RedecryptorCtx, which should help testing the component in isolation:
trait RedecryptorCtx {
  /// Returns the current list of UTD events known by the context.
  /// For instance, the event cache can load those from the database, deserializing only the event type.
  async fn current_utd_events(&self) -> Result<Vec<Event>, SomeErrorType>;

  /// Called back whenever UTD events have been decrypted by the `Redecryptor`.
  async fn on_resolved_utds(&self, events: Vec<Event>) -> Result<(), SomeErrorType>;
}
  • The event cache will implement the above trait.
    • on_resolved_utds will replace now decrypted events in the linked chunk and/or in memory.
      • also trigger an update for observers like the timeline (using RoomEvents::replace_event_at + Update::ReplaceItem for the store, as it's done here)
    • current_utd_events can either load each time from the DB and deserialize only the event type… or do something better
      • we could index the event type in the database, as a separate column in the events table
      • we could maintain a list of UTDs: load it from DB first, then keep it in sync (after we received new UTDs from sync, or we successfully decrypted some previous UTD)
      • this could be composed with the UTD hook, so that a resolved UTD also is taken into account by the UTD hook immediately. I think the above trait would lend itself to composition in a nice way, by having a UtdHookRedecryptorCtx wrap another RedecryptorCtx.
  • the Redecryptor would allow creating a task that listen to interesting events / notifications from other mechanisms (e.g. backup), and call the RedecryptorCtx as suited.
    • Ideally, there would be at most one single task to listen to all these things for all the rooms, using tokio::select! and global event handlers. We should not have a task per room, or multiple tasks, if we can.
    • The code to create the task should live alongside the Redecryptor, but the instantiated task can be owned by the EventCache or the Client themselves, for simplicity.

Flow

  • The event cache creates a RedecryptorCtx instance, then creates the Redecryptor listening task
  • Some events may come from sync: when they do, they're stored as such in the event cache, and propagated to the timeline.
    • When the timeline renders one such UTD, it calls back into the UTD hook.
  • The listening task observes interesting events / notifications.
    • When it considers it has new information, it calls into RedecryptorCtx::current_utd_events()
    • The event cache's implementation will load UTD events from the database (or from the up-to-date list)
    • If the Redecryptor managed to, well, re-decrypt, it then calls back RedecryptorCtx::on_resolved_utds with the decrypted events
      • The event cache's implementation stores those decrypted events in the in-memory chunk / database, and emits updates for observers
      • The UTD hook wrapper implementation of the trait takes that into consideration, and choose whether it should consider those late-decrypt or actual UTDs, etc.

Future improvements

Part of #3058.

@richvdh
Copy link
Member

richvdh commented Mar 20, 2025

A couple of notes for my own benefit:

Currently, the event cache can store either (a) a successfully-decrypted event, if the event could be decrypted immediately; or (b) an encrypted event, if the event could not be decrypted before it was added to the cache. In case (b), it is up to the Timeline (or whatever is reading from the event cache) to check for cached UTDs, decrypt them if possible, and register redecrypt listeners to handle late-arriving keys.

The consequence of the current implementation is that we have to redecrypt events whose key arrived late every time we pull them out of the event cache, as well as managing the redecrypt logic in multiple places.

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

No branches or pull requests

2 participants