Page MenuHomePhabricator
Differential D12077

[lib] add `deviceID` prop to Tunnelbroker's `MessageProcessed`
ClosedPublic
Actions

Authored by kamil on May 16 2024, 9:12 AM.
Tags
None
Referenced Files
F3367936: D12077.diff
Mon, Nov 25, 5:21 PM
Unknown Object (File)
Oct 14 2024, 4:27 PM
Unknown Object (File)
Oct 14 2024, 4:27 PM
Unknown Object (File)
Oct 14 2024, 4:27 PM
Unknown Object (File)
Oct 14 2024, 4:26 PM
Unknown Object (File)
Oct 14 2024, 4:26 PM
Unknown Object (File)
Sep 4 2024, 1:43 AM
Unknown Object (File)
Sep 4 2024, 1:43 AM
View All 63 Files
Subscribers
ashoat

Details

Reviewers
tomek
bartek
Commits
rCOMMc5c181b5b06f: [lib] add `deviceID` prop to Tunnelbroker's `MessageProcessed`
Summary

This is needed to remove processed message form OutboundP2PMessage's table.

Depends on D12076

Test Plan

Test sending confirmation, use app for a while to make sure this not interfere with processing store ops (login/logout).

Diff Detail

Repository
rCOMM Comm
Branch
publish-broadcasting-2
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil created this revision.May 16 2024, 9:12 AM
kamil held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptMay 16 2024, 9:12 AM
Harbormaster completed remote builds in B28990: Diff 40294.May 16 2024, 9:29 AM
kamil published this revision for review.May 16 2024, 9:39 AM
bartek added inline comments.May 17 2024, 2:16 AM
lib/handlers/db-ops-handler.react.js
51 ↗(On Diff #40294)

Open question:
Does calling identityContext.getAuthMetadata() has any advantages over fetching deviceID from Olm using getContentSigningKey() from lib/utils/crypto-utils.js?

kamil added inline comments.May 17 2024, 3:33 AM
lib/handlers/db-ops-handler.react.js
51 ↗(On Diff #40294)

Overall yes, getAuthMetadata caches values and on native is subscribed to the emitter which means is reactive to changes of accessToken.

But looking at implementation, for only deviceID is doesn't really matter which method we'll use, this is more about readability. I am open to updating this to use getContentSigningKey(). (cc. @tomek)

bartek accepted this revision.May 20 2024, 1:52 AM
bartek added inline comments.
lib/handlers/db-ops-handler.react.js
51 ↗(On Diff #40294)

Yeah I'm talking specifically about deviceID.

This revision is now accepted and ready to land.May 20 2024, 1:52 AM
kamil added a child revision: D12111: [lib] implement removing OutboundP2P messages after confirmation.May 20 2024, 3:55 AM
kamil updated this revision to Diff 40582.May 23 2024, 7:33 AM

address review

kamil added inline comments.May 23 2024, 7:46 AM
lib/handlers/db-ops-handler.react.js
51 ↗(On Diff #40294)

I updated getContentSigningKey and created ENG-8192 to discuss some improvements.

Harbormaster completed remote builds in B29211: Diff 40582.May 23 2024, 7:49 AM
Closed by commit rCOMMc5c181b5b06f: [lib] add `deviceID` prop to Tunnelbroker's `MessageProcessed` (authored by kamil). · Explain WhyMay 23 2024, 7:53 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMMc5c181b5b06f: [lib] add `deviceID` prop to Tunnelbroker's `MessageProcessed`.

Revision Contents
Changeset List

PathSize
lib/
handlers/
32 lines
types/
tunnelbroker/
2 lines

Diff 40582

View Options

lib/handlers/db-ops-handler.react.js

Loading...
View Options

lib/types/tunnelbroker/peer-to-peer-message-types.js

Loading...