gno

gno copied to clipboard

Description

This PR introduces the secret management command suite as a the gnoland subcommand -- gnoland secrets.

It is part of a series of PRs I plan to do on improving the chain initialization flow, with subsequent PRs focusing on the config file and its manipulation.

Secrets being managed:

• Validator private key (consensus)
• Node p2p key (networking)
• Validator last signed state (consensus optimization)

Available commands:

• secrets init - Initializes the Gno node secrets locally, including the validator key, validator state and node key
• secrets verify - Verifies the Gno node secrets locally, including the validator key, validator state and node key
• secrets show - Shows the Gno node secrets locally, including the validator key, validator state and node key

---
title: secrets command suite
---
flowchart LR
    subgraph init
        A[init] --> B[all]
        A[init] --> C[single]

        B --> B1("--data-dir")
        C --> C1("--validator-key-path")
        C --> C2("--validator-state-path")
        C --> C3("--node-key-path")
    end
    subgraph verify
        D[verify] --> E[all]
        D[verify] --> F[single]

        E --> E1("--data-dir")
        F --> F1("--validator-key-path")
        F --> F2("--validator-state-path")
        F --> F3("--node-key-path")
    end
    subgraph show
        G[show] --> H[all]
        G[show] --> J[single]


        H --> H1("--data-dir")
        J --> J1("--validator-key-path")
        J --> J2("--validator-state-path")
        J --> J3("--node-key-path")
    end

Jan 26 '24 16:01 zivkovicmilos

Codecov Report

Attention: Patch coverage is 79.32817% with 80 lines in your changes are missing coverage. Please review.

Project coverage is 45.04%. Comparing base (6760265) to head (cf5abb6).

@@            Coverage Diff             @@
##           master    #1593      +/-   ##
==========================================
- Coverage   47.54%   45.04%   -2.51%     
==========================================
  Files         388      464      +76     
  Lines       61242    67975    +6733     
==========================================
+ Hits        29117    30617    +1500     
- Misses      29686    34785    +5099     
- Partials     2439     2573     +134     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Jan 26 '24 16:01 codecov[bot]

Looks nice, could you add some usage examples to better understand how these commands will be used by the users, and how they will interact with 3rd party commands? Thanks.

Jan 29 '24 08:01 ajnavarro

Can we rename secrets for the binary? How about using gnoland config ... as a subcommand instead?

Also, we can change secrets init to gnoland init, and then have gnoland config for verification and gnoland config --show for displaying.

@albttx, what are your thoughts on this API proposal?

Jan 29 '24 12:01 moul

Just a reminder on my vision for node management in the future:

1. For local development, we should strive for extreme simplicity. Currently, the all-in-one gnoland start command (including initialization) was great for this purpose. However, since we introduced gnodev, I believe we should concentrate our efforts on improving the developer experience with gnodev.
2. "Junior node operators" (apologies for the term "junior," but it will become clear in point 3): these are individuals who want to join an existing network. They would likely download the genesis and configuration from an official repository and follow tutorials. Right now, I think our primary focus for the gnoland binary should be on this group.
3. "Advanced node operators" and "network architects" are individuals who are more independent and capable of debugging, among other things. While they still require some tools and documentation, we can consider this an advanced section. Our main emphasis should be on point 2, which should be our primary promotion.

I am mainly referring to "gno.land" and not TM2 for building other blockchains, where we can adopt a different DX strategy.

If we proceed in this direction, I suggest removing features related to secrets and keeping only the essential steps in the gnoland binary to set up a production node that connects to an existing network. Essentially, it would be something like: gnoland start --with-parameters or possibly gnoland init --params followed by gnoland start. Additionally, we can have gnoland status/doctor/info to verify everything and provide valuable information for support.

As for the new binary, I believe it would be more suitable for advanced users at the moment. We can relocate it to contribs/gnolandtools <secrets> and include various new helpers that could be useful for advanced scenarios.

This approach would give us a binary focused on being a production node (gnoland) and avoid unnecessary updates just because of a new initialization flag. Instead, we want to update the node only if there is a clear advantage for a running node to be updated.

Jan 29 '24 13:01 moul

Hello, I'm not really sure to understand where is the need of this command.

As a node runner on gnoland and cosmos-sdk based chain, i don't get where i could need this.

I believe what we need is:

• gnoland init to generate the priv_validator_key.json + node_key.json + genesis.json
• gnoland genesis verify to verify the genesis file, it could also check what you set.

When i need new keys on a cosmos-sdk based chain i do (which is quite rare...)

gaiad init --moniker osef --home /tmp/osef
cp /tmp/osef/config/{node_key.json,priv_validator_key.json} .

I made a PR on a proposal for the gnoland cli here: https://github.com/gnolang/gno/pull/1102

Jan 29 '24 13:01 albttx

Hello, I'm not really sure to understand where is the need of this command.

As a node runner on gnoland and cosmos-sdk based chain, i don't get where i could need this.

I believe what we need is:

• gnoland init to generate the priv_validator_key.json + node_key.json + genesis.json
• gnoland genesis verify to verify the genesis file, it could also check what you set.

When i need new keys on a cosmos-sdk based chain i do (which is quite rare...)

gaiad init --moniker osef --home /tmp/osef
cp /tmp/osef/config/{node_key.json,priv_validator_key.json} .

I made a PR on a proposal for the gnoland cli here: #1102

@albttx

To start a node and connect to a new or existing network, users essentially need 4 things:

• the network genesis file (genesis.json), that outlines the state for block 0 (and the initial validator set). This file is the same for all network participants, and is shared via whatever medium
• the validator private key (used for consensus), even if the node is not a validator (they can become one at any time)
• the validator's last sign state (this is a consensus optimization, but in the current implementation required)
• the configuration file for the node (in the current implementation required, will fix this in another PR). This file outlines info like bootnodes (for multinode connectivity), path to the genesis.json and so on. Essentially holds info all modules will node to make the node run correctly

gnoland start does this entire generation process for you, without letting you specify anything apart from the output directory on where it should dump this data.

This PR introduces commands that let you generate and manage your secrets, outside of the lazy load gnoland start context, because you need to be able to reuse existing keys (if you're migrating them) or simply generate new ones easily. We don't have any ability in gno to generate or manage these secrets, outside of this PR. You shouldn't need to start the node in order to obtain the validator / p2p key (that you need regardless, in order to construct a valid config.toml in a multinode environment). I wholeheartedly believe that having a dedicated command suite for these simple operations is better than manually trying to copy and figure out where the files are on disk.

With @moul's suggestion, your chain connection process would look like this:

1. generate genesis.json, or obtain it from somewhere else (genesis generate ... / genesis ...) <---- We have this after #1252
2. generate the node secrets, or use existing ones (gnoland init) <---- We have this in this PR
3. generate the configuration file, if any (gnoland config ...) <---- This is still missing, I will open a PR on it
4. start the node with the configuration file, secrets and genesis (gnoland start ...) <---- This is already present in the codebase

Please note that the configuration file you provide will contain the list of initial bootnodes that you'll connect to upon startup, and your node will start syncing / communicating with other peers in the network

We've already solved genesis manipulation with #1252 (and child PRs), meaning you can use those subcommands to add validators, change the initial state and so on.

Jan 29 '24 13:01 zivkovicmilos

Can we rename secrets for the binary? How about using gnoland config ... as a subcommand instead?

Also, we can change secrets init to gnoland init, and then have gnoland config for verification and gnoland config --show for displaying.

@albttx, what are your thoughts on this API proposal?

@moul

I plan to integrate a config command suite in my next PR, under gnoland config, so having this functionality labelled as such will just cause confusion (this suite is working with node secrets, not the config itself).

What do you think about swapping secrets -> keys? As in, gnoland keys init ..., and so on.

I am fully in favor of integrating this into the gnoland binary, either through gnoland <secrets> or directly with gnoland.

What option seems better?

1. gnoland <secrets> all ..., gnoland <secrets> verify ... gnoland <secrets> show ...
2. gnoland init ..., gnoland verify ..., gnoland show ...

I'm leaning much more towards the first option, because there is a clear separation of functionality.

Keep in mind we will have a config management suite as well, so it would need to follow this pattern as well (gnoland config or directly with gnoland)

cc @gfanton @ajnavarro

Jan 29 '24 14:01 zivkovicmilos

After discussing with @albttx internally, we've converged on a possibly best solution:

The gnoland binary should:

• manage the config generation / management (gnoland config ...) (will open up a PR on it)
• manage the genesis generation / management (gnoland genesis ...) (currently this is an external binary)
• manage the secrets generation / management (gnoland <secrets> ...) (currently this is an external binary, in this PR)

But we also want to introduce a new command: gnoland init, which will be a wombo combo of gnoland config init + gnoland <secrets> init. This way, the flow for any user to join a Gno chain can be:

• gnoland init - initializes the default config and secrets into a data directory
• gnoland config p2p --permanent-peers ... (if people want to change default peers). At this point, power users can set up whatever they want in terms of config changes / key changes and so on. This step is not mandatory
• gnoland start - starts the node, using the provided genesis.json, config.toml (and initialized secrets)

It's essentially a 2 step process to get up and running as a node (if we exclude the step where people change their default peers...).

I like this approach because:

• it's simple, anybody can follow it and not mess up
• it gives power users the tools to tinker and create their own gno networks if they want

Looking for thoughts @moul @ajnavarro @gfanton

Jan 29 '24 18:01 zivkovicmilos

I've made this command suite as a subcommand of gnoland: 7d24dd4

Jan 29 '24 19:01 zivkovicmilos

I'm not really fan of gnoland secret, i really believe secrets generation must be under gnoland init only.

Here is a quick recap of my position on gnoland secret:

• I believe it could lead to people making mistakes, gnoland init is enough for generating secrets.
• If the idea behind gnoland secret is to have multiple secrets providers in the future (eg: Hashicorp vault). I think it's a mistake. tm2 provide in his configuration priv_validator_laddr for setting up remote signer (see tmkms or hocrux on tendermint/cometbft) which should be use for setup a secret signer

# TCP or UNIX socket address for CometBFT to listen on for
# connections from an external PrivValidator process
priv_validator_laddr = ""

• gnoland config p2p --permanent-peers ... is great 🎉

What do you think about about changing this into a flag ? something like

$ gnoland init --force-new-secrets
are you sure ? [y,N]

Jan 30 '24 09:01 albttx

From an external perspective (since I've never had to manage a validator), I think having a single command to initialize everything is great. However, I don't see an issue with also having subcommands like secrets or genesis for more granular manipulation of configurations. As long as init is a subcommand of the root command itself, it should be clear that it's the main entrypoint for initializing everything. Additionally, I believe that gnoland start should suggest running gnoland init if it hasn't been done yet.

Jan 30 '24 22:01 gfanton

@gfanton

I don't see an issue with also having subcommands like secrets or genesis

Using gnolang init will create a genesis.json by default

Except for creating new networks, we don't need gnoland genesis ... again.

gnoland genesis is a mendatory need, where gnoland secrets isn't.

Once the init done, there is couple possibilies:

The network exist, we want to join

1. gnoland init
2. wget the genesis.json
3. set the config p2p (seeds / persistent_peers)
4. (in the case we want to move a validator, we set our priv_validator_key.json)
5. gnoland start

We create a new networks

1. gnoland init
2. gnoland genesis add-account & gnoland genesis add txs (example api)
3. gnolang genesis gentx ... (need exec by all validators)
4. Once all gentx gathered, gnoland genesis collect-gentx to craft the genesis.json
5. Share the genesis.json to all nodes
6. set the config p2p (seeds / persistent_peers)
7. gnoland start

Additionally, I believe that gnoland start should suggest running gnoland init if it hasn't been done yet.

in cosmos-sdk based chain, you have this error

Error: error during handshake: error on replay: validator set is nil in genesis and still empty after InitChain

Jan 31 '24 12:01 albttx

I'm not really fan of gnoland secret, i really believe secrets generation must be under gnoland init only.

Here is a quick recap of my position on gnoland secret:

• I believe it could lead to people making mistakes, gnoland init is enough for generating secrets.

It is required in the moment when you want to start a new (custom) Gno network. How would you get the Node ID otherwise? How would you get the validator address otherwise?

Node p2p keys are saved as:

{
  "priv_key": {
    "@type": "/tm.PrivKeyEd25519",
    "value": "uWQCD4AT+0Vu2cbS1yzZ3PJNBrSypNbNJPQHim8cpydHpWLK+LwqcPRTPFDUUchw+KTNhJrisz0uE4UYd6M3IA=="
  }
}

Validator keys are saved as:

{
  "address": "g1spdkfhmx3m7gjlyv03v9qjser6rkk3u7vhtgxc",
  "pub_key": {
    "@type": "/tm.PubKeyEd25519",
    "value": "+htraA3OqnpX9VX6nsKHWDCnNYxrtntRPt5pAcmxc7Y="
  },
  "priv_key": {
    "@type": "/tm.PrivKeyEd25519",
    "value": "b1TER+lym/QaPr9ZE7Xv4S2lkpXNACsBHxEwocLF/rj6G2toDc6qelf1VfqewodYMKc1jGu2e1E+3mkBybFztg=="
  }
}

• There is no way to get the node ID (derived address) apart from literally deriving it manually
• There is no way to get the validator address apart from doing cat priv_validator_key.json | grep "address"; not great UX

The latter is not even going into the problematic that you need the bech32 representation of the node's public key to be able to add in a validator into the genesis.json, which is not present in this Amino JSON representation: https://github.com/gnolang/gno/blob/12b4b458e1b13d491a5797aa11b2002242f012bd/gno.land/cmd/genesis/validator_add.go#L50-L55

This CLI suite gives you this information easily and does the crunching for you, no code acrobatics needed.

The secrets CLI suite is meant to be an optional toolchain for power users - and custom Gno network operators (anyone who wants to run their own closed gno testnet). As I've mentioned, the ultimate goal is to have:

• gnoland init
• gnoland start

with giving the user the freedom to tinker and experiment with their secrets (not to mention we need this kind of functionality for our testing pipelines)

If the idea behind gnoland secret is to have multiple secrets providers in the future (eg: Hashicorp vault). I think it's a mistake. tm2 provide in his configuration priv_validator_laddr for setting up remote signer (see tmkms or hocrux on tendermint/cometbft) which should be use for setup a secret signer

No need to have Vault / GCP / AWS secrets storing at the moment, as you've mentioned the remote signing process is good for now

gnoland config p2p --permanent-peers ... is great 🎉

I've added this in #1605 in the meantime

What do you think about about changing this into a flag ? something like $ gnoland init --force-new-secrets are you sure ? [y,N]

I actually love this idea -- I've envisionedgnoland init to simply be a combination of 2 other commands we'd already have: gnoland genesis ... and gnoland secrets (with default values), so having this safety feature makes complete sense.

These are the lego blocks we need to:

• accommodate "Junior node operators" and "Advanced node operators" as @moul outlined
• make the chain init flow as easy as possible (2 commands)

These CLI suites, accompanied with great docs will resolve all problems and potential blockers we might have

Jan 31 '24 16:01 zivkovicmilos

To give you a 10k feet overview of the user flows after these CLI PRs:

1️⃣ A user wants to connect to the existing gno mainnet (testnet, whatever):

• gnoland init
• gnoland start

That's it -> you don't really need to configure anything else, because our genesis.json will be publicly known and initial bootnodes hardcoded by default in the config p2p section

2️⃣ A user wants to set up their own Gno network (testnet):

• gnoland init
• gnoland genesis ... -> if they want to edit the genesis json (to add validators)
• gnoland config p2p --persistent-peers ... -> so they can set their bootnodes
• gnoland start

For the user to get these persistent-peers Node IDs, those peers would need to make their IDs public, and they can fetch them locally with: gnoland secrets show (all, or a single secret, in this case the node key)

3️⃣ A user wants to connect to a custom Gno network (testnet):

• gnoland init
• gnoland config p2p --persistent-peers ... -> so they can connect to the custom network's bootnodes
• gnoland start

This user would reuse the genesis.json that was generated at some point for the custom gno network, and have info on the custom Gno network's persistent peers

Jan 31 '24 16:01 zivkovicmilos

This flow seems good to me. Even if secrets might currently have just a single use, it seems useful enough to be implemented, It doesn't cause any harm, and in my opinion, its purpose is quite clear. I don't see any strong reasons to hold back on merging this.

Jan 31 '24 17:01 gfanton

Oh ok, i didn't get the point of utility for gnoland secrets show address.

Why do you think to stick with cosmos-sdk api and put this under gnoland tendermint ... ?

Here is gaiad tendermint --help

gaiad tendermint -h                                                                                                                                                                                                 
Tendermint subcommands

Usage:
  gaiad tendermint [command]

Available Commands:
  reset-state      Remove all the data and WAL
  show-address     Shows this node's tendermint validator consensus address
  show-node-id     Show this node's ID
  show-validator   Show this node's tendermint validator info
  unsafe-reset-all (unsafe) Remove all the data and WAL, reset this node's validator to genesis state
  version          Print tendermint libraries' version

Feb 01 '24 10:02 albttx

@albttx I can understand that you are used to cosmos-sdk API, but since the command only contains items related specifically to secrets, I think it's preferable to keep secrets as the name (at least for now):

• secrets for secret file
• genesis for genesis file
• config for configuration file

This seems clear enough to me.

Feb 15 '24 16:02 gfanton

quick note, let's finalize the design of these "config-manipulation" commands in #1605, and make it consistent with here.

In the meantime, let's review/discuss here about having the secrets command itself rather than its CLI design (which will mimick gnoland config anyway)

Feb 15 '24 17:02 thehowl

Here's what I would think with respect to the CLI design, trying to follow what we agreed/merged for gnoland config

gnoland secrets <init|verify|get> [--data-dir <path>] [<key>]

• get instead of show for consistency with config
• Instead of having two different commands (single and all), make the value being accessed a simple single optional argument to the command.
• I don't think it makes sense to be able to specify the individual paths to the secret files if the gno.land node just assumes they will be placed within the data directory. So, instead of having the three flags, let's just have the data-dir flag instead.
• The keys can be named what we want, though I think it makes sense to keep them consistent with what we have for gnoland config. So ValidatorKey, ValidatorState, NodeKey.

Also, I would be happy if the documentation could answer what "verify" actually means (i.e. when is it useful?), and what each of the keys are. What do you think?

Mar 12 '24 11:03 thehowl

@thehowl

I've updated this PR after your suggestions, and our internal discussions:

• generate secrets into a common --data-dir (validator key, node key, validator state). This directory is actually used for all node data (with subdirs of course, for DBs etc)
• have the secrets command auto read the directory (subfolder) for these secrets (predefined file names)

The good part is we were able to reuse the entire testing suite from the previous iteration 😎

5367c44

Mar 27 '24 15:03 zivkovicmilos

@thehowl

I'm not entirely sure what the best way to display available secrets keys (constants) is in the CLI itself. Do you think it's a good idea to show it in the help?

Mar 27 '24 15:03 zivkovicmilos

I'm not entirely sure what the best way to display available secrets keys (constants) is in the CLI itself. Do you think it's a good idea to show it in the help?

Yes, let's put them in help, or have a error message like "invalid secret, valid ones: [1 2 3]", like we do for when you put a wrong key in gnoland config

Mar 27 '24 16:03 thehowl

[ea258b3](/gnolang/gno/pull/1593/commits/ea258b35e13b956cf5fbd5ca87332a42c99ec9ed)

They were in the error message before, but I've added them to the long help now, so it's even more clear (before the user runs the command):

ea258b3

We will also have them on the docs somewhere when we add a secrets page

Mar 28 '24 02:03 zivkovicmilos

Looking good! My only complaint would be that the return value of the secrets get command on a specific <key> doesn't produce an exploitable format. I would personally expect that when I use secrets get NodeKey, it produces something that I can pipe with any other command, like mycmd --with-nodekey "$(gnoland secrets getNodeKey)". But maybe that doesn't make sense for other <keys>. In any case, this isn't mandatory for me in this PR. Feel free to merge it.

I'll look into having a standard format that can be piped (ie. just the raw key value, address value...) in the future 🙏

Apr 02 '24 11:04 zivkovicmilos