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
F3338749: D12995.id43172.diff
Thu, Nov 21, 6:35 PM
F3337636: D12995.id43165.diff
Thu, Nov 21, 4:56 PM
F3332714: D12995.id43172.diff
Thu, Nov 21, 1:15 AM
F3332699: D12995.id43176.diff
Thu, Nov 21, 1:14 AM
Unknown Object (File)
Sat, Nov 9, 4:39 PM
Unknown Object (File)
Wed, Nov 6, 10:34 PM
Unknown Object (File)
Sat, Nov 2, 4:06 AM
Unknown Object (File)
Sat, Nov 2, 4:06 AM
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
No Lint Coverage
Unit
No Test Coverage

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

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

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

184

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

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