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(rln-relay): pass in the path to the tree db #1782

Merged
merged 4 commits into from
Jun 8, 2023
Merged

Conversation

rymnc
Copy link
Contributor

@rymnc rymnc commented Jun 7, 2023

Description

This pr allows the user/operator to pass in the path to the persisted merkle tree, this will allow users to only sync
from the block that they need to, instead of syncing from the start every single time.

Changes

  • Converted RlnConfig into a struct, and initializes it in createRlnInstanceLocal, which is the only proc that
    uses it
  • Added a new config to the wakunode2 config, --rln-relay-tree-path that accepts a string

Note:

  • A follow up PR will be created to actually make sure that the onchain sync starts from the last processed block, split
    the prs to reduce cognitive load.

Issue

addresses a part of #1772

@rymnc rymnc marked this pull request as ready for review June 7, 2023 12:12
Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! It looks great!
I just added a few questions

@@ -201,6 +201,11 @@ type
defaultValue: ""
name: "rln-relay-cred-password" }: string

rlnRelayTreePath* {.
desc: "Path to the RLN merkle tree db",
Copy link
Collaborator

Choose a reason for hiding this comment

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

What type of db is expected? Maybe it is worth specifying it a bit more :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! good idea, addressed in 69dc716

waku/v2/waku_rln_relay/rln/wrappers.nim Outdated Show resolved Hide resolved
mode: "high_throughput",
compression: false,
flush_interval: 12_000,
path: if tree_path != "": tree_path else: DefaultRlnTreePath
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can directly pass the tree_path variable as it already has an appropriate default value.

Suggested change
path: if tree_path != "": tree_path else: DefaultRlnTreePath
path: tree_path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would have to disagree there, since the default value from the wakunode config is "", and could potentially be passed into this proc

## wrapper around the generic JObject constructor.
## We don't need to have a separate proc for the tree_config field
let tree_config = %{ "cache_capacity": %c.tree_config.cache_capacity,
"mode": %c.tree_config.mode,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe very basic question but what does % do by default in Nim? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If std/json is imported, % converts the type of the variable to a JsonNode

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah true, I forgot it. Thanks! We do the same in

$( %* jsonMessage )

waku/v2/waku_rln_relay/rln/wrappers.nim Outdated Show resolved Hide resolved
waku/v2/waku_rln_relay/rln/wrappers.nim Outdated Show resolved Hide resolved
rymnc and others added 2 commits June 8, 2023 16:02
Co-authored-by: Ivan Folgueira Bande <128452529+Ivansete-status@users.noreply.github.com>
@rymnc rymnc self-assigned this Jun 8, 2023
@@ -53,14 +53,14 @@ proc membershipKeyGen*(ctxPtr: ptr RLN): RlnRelayResult[IdentityCredential] =

return ok(identityCredential)

type RlnTreeConfig* = ref object of RootObj
type RlnTreeConfig = ref object of RootObj
cache_capacity*: int
Copy link
Collaborator

Choose a reason for hiding this comment

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

@rymnc - Sorry to be picky but could these be "hidden" as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, addressed in 358bba1!

@rymnc rymnc merged commit dba8424 into master Jun 8, 2023
@rymnc rymnc deleted the config-for-tree-path branch June 8, 2023 11:52
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.

4 participants