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(rln-relay): group manager integration #1496

Merged
merged 2 commits into from
Feb 28, 2023
Merged

Conversation

rymnc
Copy link
Contributor

@rymnc rymnc commented Jan 17, 2023

This PR integrates the group manager for rln introduced in #1465. This involved the following changes

  • separate credential generation from mounting process
  • credentials can be generated out of band, and used as credential files
  • decoupled waku_relay and waku_rln_relay
  • The rln_relay now generates a pubsub topic validator which is used in the mountRlnRelay proc on wakuNode
  • Removed test_waku_rln_relay_onchain.nim since all the existing functionality has been tested in test_rln_group_manager_onchain.nim

@rymnc rymnc added this to the Release 0.15.0 milestone Jan 17, 2023
@rymnc rymnc added the track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications label Jan 17, 2023
@rymnc rymnc self-assigned this Jan 17, 2023
@status-im-auto
Copy link
Collaborator

status-im-auto commented Jan 17, 2023

Jenkins Builds

Click to see older builds (6)
Commit #️⃣ Finished (UTC) Duration Platform Result
58e409a #1 2023-01-17 23:05:57 ~18 min macos 📄log
✔️ 316c7af #2 2023-01-22 23:07:26 ~20 min macos 📦bin
✔️ d237e76 #3 2023-01-23 23:12:05 ~24 min macos 📦bin
✔️ 1e3c12e #1 2023-01-24 10:03:55 ~15 min linux 📦bin
1e3c12e #2 2023-01-24 15:47:07 ~11 min linux 📄log
✔️ 1e3c12e #3 2023-01-24 16:03:40 ~13 min linux 📦bin
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 7a1ac1f #4 2023-01-24 23:02:55 ~15 min macos 📦bin
✔️ 29a541d #5 2023-01-25 23:06:15 ~18 min macos 📦bin

@rymnc
Copy link
Contributor Author

rymnc commented Jan 18, 2023

Currently blocked by #1497

@rymnc rymnc added track:rlnp2p and removed track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications labels Jan 19, 2023
@rymnc
Copy link
Contributor Author

rymnc commented Jan 24, 2023

CI seems to be timing out on experimental/ubuntu

@@ -154,7 +154,7 @@ proc stopGanache(runGanache: Process) {.used.} =

# We wait the daemon to exit
try:
let returnCodeExit = runGanache.waitForExit()
let returnCodeExit = runGanache.waitForExit(15)
Copy link
Contributor Author

@rymnc rymnc Jan 24, 2023

Choose a reason for hiding this comment

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

This is most likely the source of the ci timeouts - https://nim-lang.org/docs/osproc.html#waitForExit%2CProcess%2Cint

The ganache instance is used more heavily in these tests, with more contract deployments and interactions, hence may have filled the output buffers. No reason for this to error out on ubuntu specifically though, so I may be wrong

@@ -0,0 +1,470 @@
when (NimMajor, NimMinor) < (1, 4):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: renamed utils.nim to rln_relay.nim

There is more refactoring to be done in terms of message validation, will create a tracking issue for it.

@rymnc rymnc changed the title feat(rln-relay): init group manager integration feat(rln-relay): group manager integration Jan 25, 2023
@rymnc rymnc marked this pull request as ready for review January 26, 2023 03:08
@rymnc rymnc requested review from s1fr0 and LNSD January 26, 2023 03:08
@rymnc rymnc force-pushed the integrate-group-manager branch 2 times, most recently from e5b8fb6 to 6cc4bba Compare February 15, 2023 10:23
@rymnc
Copy link
Contributor Author

rymnc commented Feb 15, 2023

Moving back to draft to integrate waku_keystore

@rymnc rymnc marked this pull request as draft February 15, 2023 10:24
@rymnc rymnc force-pushed the integrate-group-manager branch 10 times, most recently from fb05489 to 0040816 Compare February 20, 2023 09:58
Comment on lines 357 to 414
proc mount(conf: WakuRlnConfig,
registrationHandler: Option[RegistrationHandler] = none(RegistrationHandler)
): Future[WakuRlnRelay] {.async.} =
var groupManager: GroupManager
var credentials: MembershipCredentials
var persistCredentials = false
# create an RLN instance
let rlnInstanceRes = createRLNInstance()
if rlnInstanceRes.isErr():
raise newException(CatchableError, "RLN instance creation failed")
let rlnInstance = rlnInstanceRes.get()
if not conf.rlnRelayDynamic:
# static setup
let parsedGroupKeysRes = StaticGroupKeys.toIdentityCredentials()
if parsedGroupKeysRes.isErr():
raise newException(ValueError, "Static group keys are not valid")
groupManager = StaticGroupManager(groupSize: StaticGroupSize,
groupKeys: parsedGroupKeysRes.get(),
membershipIndex: conf.rlnRelayMembershipIndex,
rlnInstance: rlnInstance)
# we don't persist credentials in static mode since they exist in ./constants.nim
# TODO: registration handler
else:
# dynamic setup
let ethPrivateKey = if conf.rlnRelayEthAccountPrivateKey == "": none(string) else: some(conf.rlnRelayEthAccountPrivateKey)
let rlnRelayCredPath = if conf.rlnRelayCredPath == "": none(string) else: some(conf.rlnRelayCredPath)
let rlnRelayCredentialsPassword = if conf.rlnRelayCredentialsPassword == "": none(string) else: some(conf.rlnRelayCredentialsPassword)
groupManager = OnchainGroupManager(ethClientUrl: conf.rlnRelayEthClientAddress,
ethContractAddress: $conf.rlnRelayEthContractAddress,
ethPrivateKey: ethPrivateKey,
rlnInstance: rlnInstance,
registrationHandler: registrationHandler,
keystorePath: rlnRelayCredPath,
keystorePassword: rlnRelayCredentialsPassword,
saveKeystore: true)

# Initialize the groupManager
await groupManager.init()
# Start the group sync
await groupManager.startGroupSync()

return WakuRLNRelay(pubsubTopic: conf.rlnRelayPubsubTopic,
contentTopic: conf.rlnRelayContentTopic,
groupManager: groupManager)


proc new*(T: type WakuRlnRelay,
conf: WakuRlnConfig,
registrationHandler: Option[RegistrationHandler] = none(RegistrationHandler)
): Future[RlnRelayResult[WakuRlnRelay]] {.async.} =
## Mounts the rln-relay protocol on the node.
## The rln-relay protocol can be mounted in two modes: on-chain and off-chain.
## Returns an error if the rln-relay protocol could not be mounted.
debug "rln-relay input validation passed"
try:
waku_rln_relay_mounting_duration_seconds.nanosecondTime:
let rlnRelay = await mount(conf,
registrationHandler)
return ok(rlnRelay)
except CatchableError as e:
return err(e.msg)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the only lines changed from utils.nim, for any comments re: other code, I'll create a issue for it and address it in a follow up pr.

@rymnc rymnc marked this pull request as ready for review February 22, 2023 03:26
@rymnc rymnc requested review from jm-clius and alrevuelta and removed request for s1fr0 February 22, 2023 03:27
@LNSD LNSD modified the milestones: Release 0.15.0, Release 0.16.0 Feb 22, 2023
@LNSD
Copy link
Contributor

LNSD commented Feb 22, 2023

@rymnc I have a couple questions/queries:

@rymnc
Copy link
Contributor Author

rymnc commented Feb 22, 2023

  • Nope, the two PRs are independent

  • Sure, no problem. Haven't made any functional changes to how the protocol works, just integrates the group manager and removes old code

@jm-clius
Copy link
Contributor

Afaik this also closes #1357 ? (The only remaining CI issue related to macos timeouts during RLN tests on experimental runs).

@rymnc
Copy link
Contributor Author

rymnc commented Feb 27, 2023

Afaik this also closes #1357 ? (The only remaining CI issue related to macos timeouts during RLN tests on experimental runs).

Yes

Copy link
Contributor

@LNSD LNSD left a comment

Choose a reason for hiding this comment

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

In general LGTM ✅

Comment on lines 44 to 46
proc createMembershipList*(n: int): RlnRelayResult[(
seq[(string, string, string, string)], string
)] =
Copy link
Contributor

Choose a reason for hiding this comment

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

When you have these types (string, string, string, string) my recommendation is that you define a Type name/alias. For example:

type MembershipList = (string, string, string, string)

Copy link
Contributor Author

@rymnc rymnc Feb 28, 2023

Choose a reason for hiding this comment

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

Addressed in 375aaf2 261b1e1, thanks for the review!

Comment on lines 53 to 56
let rlnInstance = createRLNInstance()
if rlnInstance.isErr():
return err("could not create rln instance: " & rlnInstance.error())
let rln = rlnInstance.get()
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend passing this instance as a parameter (as it is created from the very beginning of the function). And create the instance one level up and check there the initialzation error.

Copy link
Contributor Author

@rymnc rymnc Feb 28, 2023

Choose a reason for hiding this comment

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

Addressed in 375aaf2 261b1e1

fix(rln-relay): integrate group manager. todo spam and reg handlers

fix(rln-relay): decouple waku-relay and waku-rln-relay

fix(rln-relay): compiles now

fix(chat2): compilation

fix(rln-relay): wip segfault

fix(rln-relay): segfault

fix(chat2|wakunode2): use optional field

fix(rln-relay): wakunode test

fix(rln-relay): uncomment fields in proto decode

fix(rln-relay): used pragma on tests

fix(rln-relay): include cred processing

fix(rln-relay): add reg callback

fix(rln-relay): args to mount

fix(rln-relay): add timeout to waitForExit

fix(rln-relay): use osproc term instead of posix kill

fix(rln-relay): use poParentStream to prevent deadlock

fix(rln-relay): remove poParentStream, remove ganache log output
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.

None yet

4 participants