waku-org / waku-proto

Waku v2 protocol wire formats repository
Apache License 2.0
3 stars 2 forks source link

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

Closed LNSD closed 1 year ago

LNSD commented 1 year ago

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

Note This PR depends on the RFC PR: https://github.com/vacp2p/rfc/pull/573

LNSD commented 1 year ago

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: https://github.com/vacp2p/rfc/pull/573#issuecomment-1446294482

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 #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 commented 1 year ago

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