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(lightpush): validate messages before relaying #2595

Closed
3 tasks done
jm-clius opened this issue Apr 16, 2024 · 3 comments
Closed
3 tasks done

feat(lightpush): validate messages before relaying #2595

jm-clius opened this issue Apr 16, 2024 · 3 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@jm-clius
Copy link
Contributor

jm-clius commented Apr 16, 2024

Problem

Lightpush service nodes will currently attempt to publish/relay messages received from clients without checking the message validity first.
Messages may violate violation rules for a variety of different reasons, including exceeding the maximum size, having an invalid RLN proof, etc. See the network spec for more.
This prevents the lightpush service node from returning an appropriate error message to the client.
At least two reported bugs will be fixed with message validation checking in the lightpush service node:

Validation checking in the Relay REST API can be used as inspiration:

(await node.wakuRelay.validateMessage(pubsubTopic, message)).isOkOr:
return RestApiResponse.badRequest("Failed to publish: " & error)

More resources: lightpush spec

Acceptance criteria

  • lightpush service node checks message validity before publishing
  • lightpush service node returns useful error to clients on validation failure
  • unit tests for most common message validation failure scenarios
@fryorcraken
Copy link
Collaborator

Considering "Provision RLN for light push clients PoC" milestone, this does not seem needed at this point in time.

@jm-clius
Copy link
Contributor Author

Considering "Provision RLN for light push clients PoC" milestone, this does not seem needed at this point in time.

This will be the first step for a RLN proofs service (validating the message - with provisioned proof - before publishing). In fact, I think this issue can be closed with the merging of #2695. @shash256 please confirm and close if addressed.

@shash256
Copy link
Contributor

Yes, I am closing this issue now as we have validation in light push already merged.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Waku May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Archived in project
Development

No branches or pull requests

3 participants