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_db_inspector): include in wakunode2 binary #2292

Merged
merged 2 commits into from
Mar 6, 2024

Conversation

rymnc
Copy link
Contributor

@rymnc rymnc commented Dec 14, 2023

Description

This PR includes the rln-db-inspector tool within the wakunode2 binary for ease of use.

Changes

  • Removes rln_db_inspector external_config.nim, and nim.cfg
  • procifies the rln_db_inspector for use with wakunode2
  • adds inspectRlnDb command to wakunode2

How to test

  1. First, we compile the binary

    make -j16 wakunode2

    This command will fetch the rln static library and link it automatically.

  2. Define the arguments you wish to use

    export RLN_TREE_DB_PATH="xxx"
  3. Run the db inspector

    ./build/wakunode2 inspectRlnDb \
    --rln-relay-tree-path:$RLN_TREE_DB_PATH 

    What this does is -
    a. loads the tree db from the path provided
    b. Logs out the metadata, including, number of leaves set, past 5 merkle roots, last synced block number

@rymnc rymnc added the track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications label Dec 14, 2023
@rymnc rymnc self-assigned this Dec 14, 2023
Copy link

This PR may contain changes to configuration options of one of the apps.

If you are introducing a breaking change (i.e. the set of options in latest release would no longer be applicable) make sure the original option is preserved with a deprecation note for 2 following releases before it is actually removed.

Please also make sure the label release-notes is added to make sure any changes to the user interface are properly announced in changelog and release notes.

Copy link

github-actions bot commented Dec 14, 2023

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2292

Built from b026693

@rymnc rymnc force-pushed the rln-db-inspector-in-wakunode branch from 8726fbd to a5c85b2 Compare December 14, 2023 09:42
Copy link
Contributor

@alrevuelta alrevuelta left a comment

Choose a reason for hiding this comment

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

minor comment but lgtm

@@ -30,9 +30,12 @@ type ProtectedTopic* = object
topic*: string
key*: secp256k1.SkPublicKey

proc noop() = discard
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn 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.

still a wip, need to figure out how we can have a command for wakunode2 which doesn't accept additional args, see my message here - https://discord.com/channels/1110799176264056863/1151319265694199829/1184765875329433681

@Ivansete-status
Copy link
Collaborator

While I understand this approach is followed by famous apps/projects such as Geth, Docker, Git, etc., I think for now it is better to have the tools separated.

It sounds a bit weird to me having to use it with something like: ./build/wakunode2 inspectRlnDb --rln-relay-tree-path:$RLN_TREE_DB_PATH

imo, I think it would be better to have the following subcommand set:

./build/waku node ...
./build/waku rln ...
./build/waku canary ... ***
./build/waku monitor ... ***

*** Notice that I've added a line for to the current tools that we are packaging (wakucanary and networkmonitor.)

My suggestion is to postpone that rln-app unification for now and leave it for a future global unification where we will have the above commands. If not possible to postpone, I suggest following that direction so that we achieve that command-calling-convention in the future.

wdyt @alrevuelta @rymnc @jm-clius @NagyZoltanPeter @gabrielmer @SionoiS?

Also, very important to align this approach with go-waku colleagues (cc @richard-ramos @chaitanyaprem .)

@alrevuelta
Copy link
Contributor

@Ivansete-status Tools such as monitor seems very specific.

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.

I approve as I don't want to be a blocker

@rymnc rymnc marked this pull request as ready for review March 6, 2024 13:17
@rymnc rymnc merged commit a9d0e48 into master Mar 6, 2024
9 of 10 checks passed
@rymnc rymnc deleted the rln-db-inspector-in-wakunode branch March 6, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants