waku-org / pm

Project management, admin, misc
3 stars 1 forks source link

Enhance light push protocol #93

Open fryorcraken opened 11 months ago

fryorcraken commented 11 months ago

Summary

The Light Push protocol as [currently defined](https://rfc.vac.dev/spec/19/) is very bare. After usage and implementations, the need of some features was raised: - Returning errors _a la_ [filter](https://github.com/vacp2p/rfc/blob/master/content/docs/rfcs/12/README.md) with error code and description to clarify reason of failure: - service node does not have enough relay peer in this mesh for this pubsub topic - service node not subscrived to this pub sub topic - message pushed is invalid/malformed - timestamp out of range - (future) RLN proof is invalid (or could be same as above) - (future) this content topic is not served - Returning information in case of success such as: - number of peers to whom the message was forwarded. Considering the scope of change, backward compatibility should be possible and preferred # Acceptance Criteria

Tasks

chaitanyaprem commented 4 months ago

Adding the additional error case which was identified during https://github.com/waku-org/nwaku/pull/2601 is

fryorcraken commented 3 months ago

What is the Status on that? is it done part of Req-Res DoS Protection? @Ivansete-status

Ivansete-status commented 3 months ago

What is the Status on that? is it done part of Req-Res DoS Protection? @Ivansete-status

Morning @fryorcraken! Thanks for the heads-up! I think we have something done but I'm not sure about the actual implementation

@NagyZoltanPeter - Kindly share some insight about the implementation we did, if any. On the other hand, we need to create/link nwaku issues as a tasks of this [Epic] Enhance light push protocol, similarly as we are doing in the following epics:

Summarizing, if we haven't had the chance to look at it yet we need to create the necessary nwaku tasks, and link them to this epic, so that we can prioritize them from the nwaku PM sessions :)

Cheers

NagyZoltanPeter commented 3 months ago

What is the Status on that? is it done part of Req-Res DoS Protection? @Ivansete-status

Morning @fryorcraken! Thanks for the heads-up! I think we have something done but I'm not sure about the actual implementation

@NagyZoltanPeter - Kindly share some insight about the implementation we did, if any. On the other hand, we need to create/link nwaku issues as a tasks of this [Epic] Enhance light push protocol, similarly as we are doing in the following epics:

Summarizing, if we haven't had the chance to look at it yet we need to create the necessary nwaku tasks, and link them to this epic, so that we can prioritize them from the nwaku PM sessions :)

Cheers

@fryorcraken, @Ivansete-status: Honestly I don't remembe why this one is in-progress as we have not implemented these enhancement on error codes for lightpush (currently we are returning a text description of the error or "Ok"). Also to be enhanced the relay's publish does not really propagate errors, only the peers count that succeeded to relay to. In the last release @shash256 implemented message size validation. If that ok I will create the issues on nwaku repo and link them here.

Also these are not in scope for DOS protection.

fryorcraken commented 3 months ago

This is part of #186 cc @chair28980