Page MenuHomePhabricator
Differential D10594

[native] initialize crypto account before any crypto-related operation
ClosedPublic
Actions

Authored by kamil on Jan 10 2024, 7:12 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 23, 4:24 AM
Unknown Object (File)
Mon, Dec 23, 4:24 AM
Unknown Object (File)
Mon, Dec 23, 4:24 AM
Unknown Object (File)
Mon, Dec 23, 4:23 AM
Unknown Object (File)
Fri, Dec 13, 1:41 PM
Unknown Object (File)
Nov 6 2024, 4:59 AM
Unknown Object (File)
Oct 18 2024, 1:24 AM
Unknown Object (File)
Oct 18 2024, 1:24 AM
View All 44 Files
Subscribers
ashoat
tomek

Details

Reviewers
marcin
michal
varun
Commits
rCOMM0e78e4efe821: [native] initialize crypto account before any crypto-related operation
Summary

The entire stack worked fine because of D10327, adding this to make sure the crypto thread is always present.

Created ENG-6419 to discuss alternatives.

Depends on D10526

Test Plan

Make sure app does not crash with recent changes

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil created this revision.Jan 10 2024, 7:12 AM
kamil held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptJan 10 2024, 7:12 AM
kamil published this revision for review.Jan 10 2024, 7:33 AM
kamil edited the summary of this revision. (Show Details)
Harbormaster completed remote builds in B25672: Diff 35495.Jan 10 2024, 7:37 AM
marcin added a comment.Jan 11 2024, 2:45 AM

I think this approach is inefficient. Here is my suggestion: https://linear.app/comm/issue/ENG-6419/initialize-crypto-account-on-native#comment-07e9238a

kamil added a comment.Jan 11 2024, 4:21 AM
In D10594#306739, @marcin wrote:

I think this approach is inefficient. Here is my suggestion: https://linear.app/comm/issue/ENG-6419/initialize-crypto-account-on-native#comment-07e9238a

Can we, for now, follow the existing convention to unblock my stack, discuss the best solution, and implement it in ENG-6419?

marcin accepted this revision.Jan 11 2024, 4:22 AM

Accepting since this diff works and it will unblock @kamil but it would be nice if someone implemented the right approach in February as a small project since it doesn't seem to be too much of a hassle.

This revision is now accepted and ready to land.Jan 11 2024, 4:22 AM
Closed by commit rCOMM0e78e4efe821: [native] initialize crypto account before any crypto-related operation (authored by kamil). · Explain WhyJan 11 2024, 5:28 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMM0e78e4efe821: [native] initialize crypto account before any crypto-related operation.

Revision Contents
Changeset List

PathSize
native/
handlers/
2 lines
profile/
1 line
utils/
2 lines

Diff 35525

View Options

native/handlers/peer-to-peer-message-handler.js

Loading...
View Options

native/profile/tunnelbroker-menu.react.js

Loading...
View Options

native/utils/crypto-utils.js

Loading...