feat(54/WAKU2-X3DH-SESSIONS): include topic usage #587
Conversation
|
cc: @felicio |
content/docs/rfcs/54/README.md
Outdated
| @@ -29,13 +29,73 @@ A node identifies a peer by their `installation-id` which MAY be interpreted as | |||
| ## Discovery of pre-key bundles | |||
|
|
|||
| The node's pre-key bundle SHOULD be broadcasted on a content topic derived from the node's public key. | |||
| <!--- TODO: We need to move other specs from status to vac research, including STATUS-WAKU-USAGE --> | |||
| The derivation is further described in [10/WAKU-USAGE](https://specs.status.im/spec/10#contact-code-topic) | |||
| Peers MUST derive a topic from their public key to broadcast their pre-key bundles on, so that the first message may be PFS-encrypted. | |||
There was a problem hiding this comment.
This sentence and the one before overlap.
Suggestion: replace both with:
The node's pre-key bundle MUST be broadcast on a content topic derived from the node's public key,
, so that the first message may be PFS-encrypted.
There was a problem hiding this comment.
Also: What is the first message in this context?
There was a problem hiding this comment.
Addressed in c03a83b
The first message is the pre-key bundle they send
There was a problem hiding this comment.
In this case, the phrasing suggests that the PFS of this message is dependent on the fact that the topic name is derived from the key. How are these connected?
There was a problem hiding this comment.
Is any data appended the message that also contains the pre-key bundle?
Would this match what is done in the impl:
Nodes MUST derive a topic from their public key to broadcast their pre-key bundles on.
This allows peers to infer the topic on which they can receive this pre-key bundle.
Data appended to the pre-key bundle message, as well as following messages, can be PFS-encrypted.
There was a problem hiding this comment.
cc: @felicio for confirmation, afaik, this is not done in the impl
|
|
||
| var hash []byte = keccak256(partitionTopic) | ||
| var topicLen int = 4 | ||
|
|
There was a problem hiding this comment.
Why limiting topicLen like this and not simply using keccak256(partitionTopic) as the topic name.
It only saves 4 bytes, and topic names are typically descriptive, i.e. 4 more bytes should not matter much.
Also what is the resulting topic name?
Is it simply the content of var topic [4]byte
To be more in line with https://rfc.vac.dev/spec/23/ , the topic could be named:
/status/<version>/1to1/<keccak256(partitionTopic)>
cc @felicio
There was a problem hiding this comment.
Unsure, this is how its implemented afaik
There was a problem hiding this comment.
Let's wait for Status feedback :).
There was a problem hiding this comment.
Got back from a vacation, will check this PR today.
There was a problem hiding this comment.
While I don't have the historical context for the length, I believe it was inherited from Whisper where even there I'd say it was an arbitrary constant that was considered collision resistant enough.
About the resulting topic name, see #567 (comment).
And as far as being in line goes, I agree. The way I see it now we're "overloading" the bridging spec and should revisit the need for bridging or send to both format. Actually following Waku v2 spec for content topics would help with monitoring too I believe, see https://discord.com/channels/864066763682218004/1075738141681528863/1075747858067759174.
There was a problem hiding this comment.
Yes, it answers why this was chosen :).
Thanks @felicio for poiting out the bridge.
Bridge copies v2 contentTopics into v1 topics (which have a 4bytes limit).
Imo, we should update the bridge spec to hash contentTopic to a 4byte topic instead of simply copying it,
because v2 topics do not have the size limit. The current version of the bridging spec does not take the size limit into consideraiton.
If we do this, we can also move to Waku v2 contentTopic names, as suggested above.
Also, bridging to v1 would introduce scaling problems and we should not do that for the MVP... Imo, it should be limited to specific cases.
Adding @jm-clius to the discussion regarding bridge.
There was a problem hiding this comment.
Indeed. Bridging from/to v1 is not considered for this MVP (and likely wouldn't be required in future either).
There was a problem hiding this comment.
Thanks for clarifying this :).
There was a problem hiding this comment.
@felicio we could integrate hierarchical topic naming into the RFC now in this PR, or wait until post-MVP and integrate topic naming as the current version of this PR reflects.
What would Status app prefer?
There was a problem hiding this comment.
Post-MVP, please. But would appreciate a follow-up issue for tracking.
|
|
||
| After the peers have performed the initial key exchange, they MUST derive a topic from their shared secret to send messages on. | ||
| To obtain this value, take the first four bytes of the keccak256 hash of the shared secret encoded in hexadecimal format. | ||
|
|
There was a problem hiding this comment.
(see above: why only 4 bytes; does not save much in this context.)
|
Thanks :). I added a few more comments. |
After confirming with the implementation, this PR includes the topic usage and negotiation for x3dh based sessions used by the status desktop app.