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

chore: Enabling to use a full node for lightpush via rest api without lightpush client configured #2626

Merged

Conversation

NagyZoltanPeter
Copy link
Contributor

Description

This is helper enhancment for testing (rate limit specifically for me) on lightpush... similar to query self store node via REST API.
With this change one can issue lightpush request via REST API on a service node having lightpush protocol installed.

In contrast it was needed to run a full node with lightpush service and a light client (configured with the service node peerId) to be usable from REST API.
With this change a lightpush service node (with relay of course) can serve such lightpush API request and can relay such messages.

This can help in various way.

  • easy develop on REST API with nwaku-compose (where a full node is set up)
  • easy test lightpush features via rest api.

Changes

  • /lightpush endpoint is enabled either lightpushclient or lightpush(protocol) mounted
  • lightpush protocol can handle such request by self hosted coming from REST API

How to test

  1. Run a full node or nwaku-compose with the right image
  2. issue REST /lightpush POST message
  3. check that relayed (can depend on various stuff)

Open question:

  • Currently relaying message via REST API will attach rln proof to the message. This is missing from lightpush, thus rest api user should be able to provide the proof.
  • I think this can be think as, I'm using a thin layer towards the network via REST API by running a light node with my rln credentials. Such case lightpush service can attach proof to the message as in relay (REST API case).

Copy link

github-actions bot commented Apr 25, 2024

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2626-rln-v1

Built from 9787e53

Copy link

github-actions bot commented Apr 25, 2024

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2626-rln-v2

Built from 9787e53

Copy link
Contributor

@SionoiS SionoiS left a comment

Choose a reason for hiding this comment

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

Approved to not block but this flow seams a bit convoluted.

Maybe in waku_node local vs remote light push should be 2 different procs?

waku/waku_api/rest/builder.nim Outdated Show resolved Hide resolved
@@ -35,6 +36,9 @@ const NoPeerNoDiscoError =
const NoPeerNoneFoundError =
RestApiResponse.serviceUnavailable("No suitable service peer & none discovered")

proc useSelfHostedLightPush(node: WakuNode): bool =
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe rephrase to local vs remote lightpush ?

Comment on lines +68 to +70
if useSelfHostedLightPush(node):
discard
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if useSelfHostedLightPush(node):
discard
else:
if not useSelfHostedLightPush(node):

waku/node/waku_node.nim Outdated Show resolved Hide resolved
Copy link
Contributor

@gabrielmer gabrielmer left a comment

Choose a reason for hiding this comment

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

Superr, thanks so much!

Added a couple questions/comments :)

waku/waku_lightpush/self_req_handler.nim Outdated Show resolved Hide resolved
waku/waku_lightpush/self_req_handler.nim Show resolved Hide resolved
waku/waku_lightpush/self_req_handler.nim Show resolved Hide resolved
@NagyZoltanPeter NagyZoltanPeter merged commit 2a4c0f1 into master Apr 26, 2024
14 of 15 checks passed
@NagyZoltanPeter NagyZoltanPeter deleted the chore-make-rest-lightpush-avail-on-service-node branch April 26, 2024 10:42
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