Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adr: added message uid document #18

Merged
merged 1 commit into from
Mar 28, 2023
Merged

adr: added message uid document #18

merged 1 commit into from
Mar 28, 2023

Conversation

LNSD
Copy link
Contributor

@LNSD LNSD commented Mar 28, 2023

Added Message Unique ID (v2) to the waku pm repository. Note that this is just a design document, non-normative.

This ADR is associated with issue #9

@LNSD LNSD self-assigned this Mar 28, 2023
Copy link
Collaborator

@alrevuelta alrevuelta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

as an opinion, I think its valuable to start such documents with few lines where its crystal clear what's being introduced, right at the begining. "this introduces message uid, calculated as follows, with this and this".

then ellaborate on rationale, tradeoffs, use cases, etc as you are doing.

just matter of ordering to allow 3rd parties to easily grasp the idea of it.

message_id: [u8; 32] = sha256(WakuMessageBuffer)
```

These structures provide a limited-time message deduplication capability o keep the memory footprint low. The *Message Cache* provides a short-term deduplication capability (~5 heartbeat periods). The **Seen Cache,** implemented as a bloom filter, provides a longer-term deduplication capability (~2 minutes).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"to keep"?

It is a computable 32 bytes fixed-length checksum based on the content of the Waku Message. It is defined as follows:

```rust
checksum: [u8; 32] = sha256(network_topic, WakuMessage.topic, WakuMessage.meta, WakuMessage.payload)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the v3, but this was my point in the previous call.

if we sign the checksum and include it in the new (64 bytes) meta field, then the checksum can't include meta. Unless ofc we sign something else.

@LNSD
Copy link
Contributor Author

LNSD commented Mar 28, 2023

lgtm.

as an opinion, I think its valuable to start such documents with few lines where its crystal clear what's being introduced, right at the begining. "this introduces message uid, calculated as follows, with this and this".

then ellaborate on rationale, tradeoffs, use cases, etc as you are doing.

just matter of ordering to allow 3rd parties to easily grasp the idea of it.

This document was posted two months ago in #9 before the DoS protection initiative was kicked off. It would have been great if you had provided some feedback by that time. I hope that reading now the document helps understand the ideas behind the meta field I proposed.

Please, wait for the next version of this document for further changes.

@LNSD LNSD merged commit c3ce6ec into master Mar 28, 2023
@LNSD LNSD deleted the adr-muid branch March 28, 2023 12:45
@fryorcraken fryorcraken mentioned this pull request Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants