feat(c-bindings): rln relay #2544

richard-ramos · 2024-03-20T20:56:14Z

Description

Exposes the rln relay configuration to the bindings.

Issue

github-actions · 2024-03-20T21:00:51Z

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2544-rln-v2-false

Built from 9afbf17

richard-ramos · 2024-03-21T15:49:36Z

Oops! I forgot to append the proof to the RLN message. Will add that in a separate commit

EDIT: added in 964d7ab

Ivansete-status

It looks great indeed!

Nevertheless, I wonder if we could split the PR in two:

refactor node initialization
addition of rln config

cheers!

richard-ramos · 2024-03-22T03:32:12Z

refactor node initialization

Done in #2547

addition of rln config

I'll use this PR for this once I rebase it against #2547

richard-ramos · 2024-03-25T03:30:29Z

Done! PR rebased!

Ivansete-status

LGTM! Thanks!
I just added some nitpick comments ;P
Cheers

Ivansete-status · 2024-03-25T09:24:16Z

library/waku_thread/config.nim

+
+  if jsonNode.contains("clusterId"):
+    if jsonNode["clusterId"].kind != JsonNodeKind.JInt:
+      errorResp = "The clusterId config param should be a int"


Tiny typo :)

Suggested change

errorResp = "The clusterId config param should be a int"

errorResp = "The clusterId config param should be an int"

Ivansete-status · 2024-03-25T09:26:49Z

library/waku_thread/config.nim

@@ -149,6 +165,29 @@ proc parseTopics(jsonNode: JsonNode, conf: var WakuNodeConf) =
  else:
    conf.topics = @["/waku/2/default-waku/proto"]

+proc parseRLNRelay(jsonNode: JsonNode, conf: var WakuNodeConf, errorResp: var string): bool =
+  if not jsonNode.contains("rlnRelay"):


I wonder if we should use the same style as the wakunode has. Hyphen-separated terms.

Suggested change

if not jsonNode.contains("rlnRelay"):

if not jsonNode.contains("rln-relay"):

Ivansete-status · 2024-03-25T09:28:49Z

library/waku_thread/config.nim

+  conf.rlnRelayCredPath = jsonNode{"credentialPassword"}.getStr()
+  conf.rlnRelayEthClientAddress = EthRpcUrl.parseCmdArg(jsonNode{"ethClientAddress"}.getStr("http://localhost:8540"))
+  conf.rlnRelayEthContractAddress = jsonNode{"contractAddress"}.getStr()
+  conf.rlnRelayCredPassword = jsonNode{"credentialPassword"}.getStr()
+  conf.rlnRelayUserMessageLimit = uint64(jsonNode{"userMessageLimit"}.getInt(1))
+  conf.rlnEpochSizeSec = uint64(jsonNode{"epochSizeSec"}.getInt(1))
+  conf.rlnRelayCredIndex = some(uint(jsonNode{"membershipIndex"}.getInt()))
+  conf.rlnRelayDynamic = jsonNode{"dynamic"}.getBool()
+  conf.rlnRelayTreePath = jsonNode{"treePath"}.getStr()
+  conf.rlnRelayBandwidthThreshold = jsonNode{"bandwidthThreshold"}.getInt()


Analogue to my previous comment, I suggest using the same names as in wakunode app:

$ ./build/wakunode2 --help | grep rln --rln-relay-cred-path The path for peristing rln-relay credential. --rln-relay-eth-client-address HTTP address of an Ethereum testnet client e.g., http://localhost:8540/ --rln-relay-eth-contract-address Address of membership contract on an Ethereum testnet. --rln-relay-cred-password Password for encrypting RLN credentials. --rln-relay-eth-private-key Private key for broadcasting transactions. --rln-relay-user-message-limit Set a user message limit for the rln membership registration. Must be a positive --rln-relay-epoch-sec Epoch size in seconds used to rate limit RLN memberships. Default is 1 second. --rln-relay Enable spam protection through rln-relay: true|false [=false]. --rln-relay-membership-index the index of the onchain commitment to use. --rln-relay-dynamic Enable waku-rln-relay with on-chain dynamic group management: true|false --rln-relay-id-key Rln relay identity secret key as a Hex string. --rln-relay-id-commitment-key Rln relay identity commitment key as a Hex string. --rln-relay-tree-path Path to the RLN merkle tree sled db (https://github.com/spacejam/sled). --rln-relay-bandwidth-threshold Message rate in bytes/sec after which verification of proofs should happen [=0]. --rln-relay-tree-path Path to the RLN merkle tree sled db (https://github.com/spacejam/sled). warning: could not open directory 'postgres-data/': Permission denied

I can use the hyphens to separate the words but adding the rln prefix I'm not sure, because it currently looks like this:

{ "port": 123, "host": "0.0.0.0", "relay": true, "rlnRelay": { "enabled": "true", "ethClientAddress": "http://localhost:8545", "membershipIndex": 2, ... } }

And seems to me that adding the rln prefix would be redundant, as the values are already part of a rlnRelay section. A possibility could be to put everything a single level deep (i.e., move all the items from rlnRelay into the root json, but seems to me that it's not benefiting from the possibilities of json allowing more logical groups.

Would something like this work? (i.e. having the rln-relay prefix be instead the key that holds all the rln related configs):

{ "port": 123, "host": "0.0.0.0", "relay": true, "rln-relay": { "enabled": "true", "eth-client-address": "http://localhost:8545", "membership-index": 2, "bandwidth-threshold": 1, ... } }

Yes, I think this is the best balanced option (simplicity vs consistency)

Ivansete-status · 2024-03-25T09:33:00Z

library/waku_thread/inter_thread_communication/requests/node_lifecycle_request.nim

+  if conf.clusterId == 1:
+    let twnClusterConf = ClusterConf.TheWakuNetworkConf()
+    if len(conf.shards) != 0:
+      conf.pubsubTopics = conf.shards.mapIt(twnClusterConf.pubsubTopics[it.uint16])
+    else:
+      conf.pubsubTopics = twnClusterConf.pubsubTopics
+
+    # Override configuration
+    conf.maxMessageSize = twnClusterConf.maxMessageSize
+    conf.clusterId = twnClusterConf.clusterId
+    conf.rlnRelay = twnClusterConf.rlnRelay
+    conf.rlnRelayEthContractAddress = twnClusterConf.rlnRelayEthContractAddress
+    conf.rlnRelayDynamic = twnClusterConf.rlnRelayDynamic
+    conf.rlnRelayBandwidthThreshold = twnClusterConf.rlnRelayBandwidthThreshold
+    conf.discv5Discovery = twnClusterConf.discv5Discovery
+    conf.discv5BootstrapNodes =
+      conf.discv5BootstrapNodes & twnClusterConf.discv5BootstrapNodes
+    conf.rlnEpochSizeSec = twnClusterConf.rlnEpochSizeSec
+    conf.rlnRelayUserMessageLimit = twnClusterConf.rlnRelayUserMessageLimit


We have a similar structure in

nwaku/apps/wakunode2/wakunode2.nim

Lines 91 to 109 in 18a0535

if conf.clusterId == 1:

let twnClusterConf = ClusterConf.TheWakuNetworkConf()

if len(conf.shards) != 0:

conf.pubsubTopics = conf.shards.mapIt(twnClusterConf.pubsubTopics[it.uint16])

else:

conf.pubsubTopics = twnClusterConf.pubsubTopics

# Override configuration

conf.maxMessageSize = twnClusterConf.maxMessageSize

conf.clusterId = twnClusterConf.clusterId

conf.rlnRelay = twnClusterConf.rlnRelay

conf.rlnRelayEthContractAddress = twnClusterConf.rlnRelayEthContractAddress

conf.rlnRelayDynamic = twnClusterConf.rlnRelayDynamic

conf.rlnRelayBandwidthThreshold = twnClusterConf.rlnRelayBandwidthThreshold

conf.discv5Discovery = twnClusterConf.discv5Discovery

conf.discv5BootstrapNodes =

conf.discv5BootstrapNodes & twnClusterConf.discv5BootstrapNodes

conf.rlnEpochSizeSec = twnClusterConf.rlnEpochSizeSec

conf.rlnRelayUserMessageLimit = twnClusterConf.rlnRelayUserMessageLimit

I wonder if we could encapsulate that somehow. Maybe in a separate PR

SionoiS

I got a feeling of deja vu...

LGTM

richard-ramos · 2024-03-27T13:00:11Z

I got a feeling of deja vu...

LGTM

richard-ramos requested review from gabrielmer and Ivansete-status March 20, 2024 20:56

richard-ramos force-pushed the bindings/rln branch 2 times, most recently from 444a599 to 2a06e55 Compare March 21, 2024 15:25

Ivansete-status reviewed Mar 21, 2024

View reviewed changes

Ivansete-status assigned richard-ramos Mar 21, 2024

richard-ramos marked this pull request as draft March 22, 2024 03:31

richard-ramos changed the base branch from master to node-init March 22, 2024 03:32

richard-ramos force-pushed the node-init branch from 32297d9 to 0f30ea7 Compare March 22, 2024 20:27

richard-ramos force-pushed the bindings/rln branch from 964d7ab to 4939a5a Compare March 25, 2024 03:30

richard-ramos marked this pull request as ready for review March 25, 2024 03:36

Ivansete-status approved these changes Mar 25, 2024

View reviewed changes

Base automatically changed from node-init to master March 26, 2024 13:10

Ivansete-status requested a review from SionoiS March 27, 2024 10:42

SionoiS approved these changes Mar 27, 2024

View reviewed changes

richard-ramos force-pushed the bindings/rln branch from 4939a5a to ed35360 Compare March 27, 2024 13:31

feat(c-bindings): rln relay

a9757fe

richard-ramos force-pushed the bindings/rln branch from ed35360 to a9757fe Compare March 27, 2024 13:34

richard-ramos merged commit 2aa835e into master Mar 27, 2024
13 of 15 checks passed

richard-ramos deleted the bindings/rln branch March 27, 2024 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(c-bindings): rln relay #2544

feat(c-bindings): rln relay #2544

richard-ramos commented Mar 20, 2024 •

edited

github-actions bot commented Mar 20, 2024 •

edited

richard-ramos commented Mar 21, 2024 •

edited

Ivansete-status left a comment

richard-ramos commented Mar 22, 2024

richard-ramos commented Mar 25, 2024

Ivansete-status left a comment

Ivansete-status Mar 25, 2024

Ivansete-status Mar 25, 2024

Ivansete-status Mar 25, 2024

richard-ramos Mar 26, 2024 •

edited

Ivansete-status Mar 27, 2024

Ivansete-status Mar 25, 2024

SionoiS left a comment

richard-ramos commented Mar 27, 2024

	errorResp = "The clusterId config param should be a int"
	errorResp = "The clusterId config param should be an int"

	if not jsonNode.contains("rlnRelay"):
	if not jsonNode.contains("rln-relay"):

	if conf.clusterId == 1:
	let twnClusterConf = ClusterConf.TheWakuNetworkConf()
	if len(conf.shards) != 0:
	conf.pubsubTopics = conf.shards.mapIt(twnClusterConf.pubsubTopics[it.uint16])
	else:
	conf.pubsubTopics = twnClusterConf.pubsubTopics

	# Override configuration
	conf.maxMessageSize = twnClusterConf.maxMessageSize
	conf.clusterId = twnClusterConf.clusterId
	conf.rlnRelay = twnClusterConf.rlnRelay
	conf.rlnRelayEthContractAddress = twnClusterConf.rlnRelayEthContractAddress
	conf.rlnRelayDynamic = twnClusterConf.rlnRelayDynamic
	conf.rlnRelayBandwidthThreshold = twnClusterConf.rlnRelayBandwidthThreshold
	conf.discv5Discovery = twnClusterConf.discv5Discovery
	conf.discv5BootstrapNodes =
	conf.discv5BootstrapNodes & twnClusterConf.discv5BootstrapNodes
	conf.rlnEpochSizeSec = twnClusterConf.rlnEpochSizeSec
	conf.rlnRelayUserMessageLimit = twnClusterConf.rlnRelayUserMessageLimit

feat(c-bindings): rln relay #2544

feat(c-bindings): rln relay #2544

Conversation

richard-ramos commented Mar 20, 2024 • edited

Description

Issue

github-actions bot commented Mar 20, 2024 • edited

richard-ramos commented Mar 21, 2024 • edited

Ivansete-status left a comment

Choose a reason for hiding this comment

richard-ramos commented Mar 22, 2024

richard-ramos commented Mar 25, 2024

Ivansete-status left a comment

Choose a reason for hiding this comment

Ivansete-status Mar 25, 2024

Choose a reason for hiding this comment

Ivansete-status Mar 25, 2024

Choose a reason for hiding this comment

Ivansete-status Mar 25, 2024

Choose a reason for hiding this comment

richard-ramos Mar 26, 2024 • edited

Choose a reason for hiding this comment

Ivansete-status Mar 27, 2024

Choose a reason for hiding this comment

Ivansete-status Mar 25, 2024

Choose a reason for hiding this comment

SionoiS left a comment

Choose a reason for hiding this comment

richard-ramos commented Mar 27, 2024

richard-ramos commented Mar 20, 2024 •

edited

github-actions bot commented Mar 20, 2024 •

edited

richard-ramos commented Mar 21, 2024 •

edited

richard-ramos Mar 26, 2024 •

edited