Extend createThickRawThreadInfo so that it can create a thread with all the props.
Details
Details
Tested in combination with the rest of the stack by e.g. processing an operation that creates a new thread.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/shared/dm-ops/create-thread-spec.js | ||
|---|---|---|
| 47 ↗ | (On Diff #42876) | I'm not sure if it is safe to share this between clients because it can contain an encryptionKey. |
Comment Actions
What about +community?: ?string, field?
I'm not sure if it is safe to share this between clients because it can contain an encryptionKey.
This seems like something we need to figure out. But we should be able to use the same mechanism as we use for keyserver chats, right?
| lib/shared/dm-ops/create-thread-spec.js | ||
|---|---|---|
| 80 ↗ | (On Diff #42907) | This field is also optional (+parentThreadID?: ?string,). Should we add it in an if? |
| 103 ↗ | (On Diff #42907) | This field is also optional (+containingThreadID?: ?string,). Should we add it in an if? |
| lib/shared/dm-ops/create-thread-spec.js | ||
|---|---|---|
| 80 ↗ | (On Diff #42907) | The fields that are added in ifs are the ones for which types are optional non-nullable (+field?: T). When a field is optional and nullable, we don't have to move it to an if. |
| 47 ↗ | (On Diff #42876) | It seems like we could safely do that because the DM is E2E encrypted. @bartek or @ashoat could you confirm? To give more context, this field has ClientAvatar type, which has ClientEncryptedImageAvatar as its subtype in which we store an encryptionKey. This value would be sent as a part of an encrypted P2P message to other peers. |
Comment Actions
What about +community?: ?string, field?
I got a little confused here. But I think that communities are always thin because they live on keyservers. So for a thick thread, it is not possible to assign a community. Am I right?
I'm not sure if it is safe to share this between clients because it can contain an encryptionKey.
This seems like something we need to figure out. But we should be able to use the same mechanism as we use for keyserver chats, right?
Yeah, we could use the same mechanism. But if we're able to use ClientAvatar directly, the solution would be simpler.
| lib/shared/dm-ops/create-thread-spec.js | ||
|---|---|---|
| 47 ↗ | (On Diff #42876) | I think we can. The general idea is that blob-stored media (including avatars) are AES-encrypted, and peers allowed to see their content, receive the AES encryption key via DM which is E2E encrypted. |
Comment Actions
createThickRawThreadInfo returns MutableThickRawThreadInfo which includes a community field. I am also confused why that is, but it is types as +community?: ?string,, so I am assuming it can hold some value. If so, we should be able to set it
Comment Actions
Ok, let's add it for consistency, but it is still confusing. @ashoat do you know why MutableThickRawThreadInfo includes community?
Comment Actions
Ok, let's add it for consistency, but it is still confusing. @ashoat do you know why MutableThickRawThreadInfo includes community?
This was my mistake. Would it be easy to remove at this point? We might consider a follow-up task for this if not.
Comment Actions
This comment wasn't responded to. It looks like this diff was landing with support for the community column, which isn't ideal, but probably won't be an issue if nobody calls it with community set.
Still, at the very least I'd like to see a follow-up task created for removing community from ThickRawThreadInfo (and any related types).
Comment Actions
Ahhh, sorry for this! I started working on it and forgot to reply. I created a task to track https://linear.app/comm/issue/ENG-8937/remove-community-from-mutablethickrawthreadinfo. Overall it should be a quick change.