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

feat(WAKU2-MESSAGE): added meta attribute to waku message #13

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

LNSD
Copy link
Contributor

@LNSD LNSD commented Feb 14, 2023

As part of the Message Unique ID initiative, a metadata optional application-specific field has been proposed.

  • This PR adds the field to the Waku message protobuf definition.

Note
This PR depends on the RFC PR: vacp2p/rfc#573

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

MSTM, but presumably depends on agreement on the RFC.

Copy link

@rymnc rymnc left a comment

Choose a reason for hiding this comment

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

LGTM
unrelated, but do we plan to include experimental feature fields in this repo? (i.e rln proof field on slot 21)
ref: https://github.com/waku-org/nwaku/blob/2f883f88e5264964a09f6bf19bef68a410745c33/waku/v2/protocol/waku_message/codec.nim#L60-L65

@LNSD
Copy link
Contributor Author

LNSD commented Feb 27, 2023

LGTM
unrelated, but do we plan to include experimental feature fields in this repo? (i.e rln proof field on slot 21)
ref: waku-org/nwaku@2f883f8/waku/v2/protocol/waku_message/codec.nim#L60-L65

That's a good question, @rymnc. Note this comment in the RFC PR: vacp2p/rfc#573 (comment)

In case we should need specific data to be associated with a message for relay layer DoS protections, I'd suggest introducing a new field for this.

This is the case for RLN. The RLN relay validator precise a proof Waku message attribute. Maybe that field could be generalized/standardised in a particular manner so it can be used for different purposes.

I would argue that the process would be something like this:

  1. Extract a generic field definition usable for other DoS mechanisms. A generic name (I am not entirely convinced with proof. It sounds too RLN-specific to me), a backwards compatible type (e.g. an optional variable length byte array), and a size limit (we should try to keep Waku message attributes shorter than the payload).
  2. Add it to the Waku message RFC: Add it to the attributes section with its description and finality and the protobuf section as feat: Added missing HistoryResponse Error codes, introduced by Rate Limiting #21. Similar to what I am doing with the meta attribute.
  3. In parallel to the RFC, open the "protobuf PR" (like this one).

I think that this work is worth an RFC issue to track it and have the discussion there.

@LNSD
Copy link
Contributor Author

LNSD commented Feb 28, 2023

Merging this PR, as the associated RFC has been approved an merged 😁

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.

3 participants