Paths

Table of Contentst

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

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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

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

Path

Size

lib/

handlers/

db-ops-handler.react.js

32 lines

types/

tunnelbroker/

peer-to-peer-message-types.js

2 lines

Diff 40584

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

Loading...

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

Loading...