Page MenuHomePhabricator

[lib] Add hook to broadcast Olm-encrypted messages
ClosedPublic

Authored by bartek on Aug 6 2024, 1:02 AM.
Tags
None
Referenced Files
F3571141: D12995.id43172.diff
Sat, Dec 28, 7:47 AM
F3571132: D12995.id43176.diff
Sat, Dec 28, 7:47 AM
Unknown Object (File)
Mon, Dec 16, 10:36 PM
Unknown Object (File)
Mon, Dec 16, 10:36 PM
Unknown Object (File)
Mon, Dec 16, 10:36 PM
Unknown Object (File)
Nov 24 2024, 7:14 AM
Unknown Object (File)
Nov 21 2024, 8:57 PM
Unknown Object (File)
Nov 21 2024, 6:35 PM
Subscribers

Details

Summary

Address feedback from https://phab.comm.dev/D12944?id=43066#inline-75283
Extracted sending Olm messages (and optional session creation) to a hook.
It is used in a few places, which are updated in subsequent diffs

Test Plan

Flow. Logic tested in later diffs

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Aug 6 2024, 1:35 AM
kamil added inline comments.
lib/hooks/peer-list-hooks.js
147 ↗(On Diff #43165)

In most use cases we'll want to use PeerToPeerProvider and persist messages until confirmation from a recipient (code here),

This is different because we're about to log out so there is no need to persist, we also don't expect to receive the confirmation. I think it will be beneficial to reflect that in the name to avoid confusion, something like useBroadcastEphemeralOlmMessage or useBroadcastOlmMessageWithoutPersiting.

Alternatively, what do you think about making this hook as part of PeerToPeerProvider ? From this point every P2P communication should be implemented using usePeerToPeerCommunication hook.

169–210 ↗(On Diff #43165)

How about creating a promise for each recipient and then using Promise.all?

184 ↗(On Diff #43165)

we should filter errors here - but this is part of my work.

This revision is now accepted and ready to land.Aug 6 2024, 2:34 AM
lib/hooks/peer-list-hooks.js
162–166 ↗(On Diff #43165)

Looking at call sites we always have a getAuthMetadata call before, and I think we'll have the same in other use cases. Maybe we can pass AuthMetadata as a param here to avoid unnecessary calls?

Move to PeerToPeerProvider, pass auth metadata, use Promise.all