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

docs: Migrate oasis-node setup section, more commands #5281

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

matevz
Copy link
Member

@matevz matevz commented Jun 7, 2023

Part of oasisprotocol/docs#399. This PR migrates a number of oasis-node commands previously described inside the general/manage-tokens/advanced/oasis-cli-tools section to this repository where they should probably be in the first place.

@matevz matevz force-pushed the matevz/docs/oasis-node-cli branch from 558d15e to 3d9dde1 Compare June 7, 2023 09:20
@matevz matevz force-pushed the matevz/docs/oasis-node-cli branch from 3d9dde1 to 914f7c4 Compare June 7, 2023 09:32
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

Is there a reason to keep this even? It seems like this is describing commands which will be deprecated/removed anyway due to the CLI and we should not be encouraging their use, instead we should just point people at the CLI.

@matevz
Copy link
Member Author

matevz commented Jun 7, 2023

There are two sections referenced from the other parts of docs as I noticed:

  • Setup section (mostly describing $TX_FLAGS and $ADDR used in node operators docs) is not in the CLI
  • gen_cast_vote section which is currently missing in the CLI

We could remove other token management sections I guess, but these still exist in the current oasis-core master and 22.x branches right so why not document them?

@kostko
Copy link
Member

kostko commented Jun 7, 2023

Setup section (mostly describing $TX_FLAGS and $ADDR used in node operators docs) is not in the CLI

The node operator docs should be migrated to use the CLI as well because all of this is clunky as hell, but ok let's leave them for now.

gen_cast_vote section which is currently missing in the CLI

This should be easy to add to the CLI, but ok let's leave it for now.

We could remove other token management sections I guess, but these still exist in the current oasis-core master and 22.x branches right so why not document them?

Yes we should remove them. The reason is that we should be directing people to use the CLI instead and not having these in docs allows us to remove them from master at any point. Also the CLI already supports 22.x so we should push people to use it now not later.

Copy link
Contributor

@peternose peternose left a comment

Choose a reason for hiding this comment

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

I guess that doc review wasn't needed, but I wanted to go through the examples and test some of them locally. Commands and examples are great, very precise and informative.

I left some comments which can be overlooked, just nitpicking :)


- `oasis-node stake info`: Shows general staking information.
- `oasis-node stake list`: Lists all accounts with positive balance.
- `oasis-node stake account info`: Shows detailed information for an account.
Copy link
Contributor

Choose a reason for hiding this comment

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

level=error ts=2023-06-09T11:32:52.580181299Z caller=account.go:143 module=cmd/stake msg="failed to parse account address" err="address: decoding from bech32 failed: decoding bech32 failed: invalid bech32 string length 0"

Some commands fail by default as not all parameters are set/correct. This could confuse users. We could add required flags (too much work, too long) or just mention somewhere that commands might need extra flags to work and direct user to help.

located (e.g. `/serverdir/node/`) before executing the command, or
- pass the `-a $ADDR` flag where `ADDR` represents the path to the internal
Oasis node UNIX socket prefixed with `unix:`
(e.g.`unix:/serverdir/node/internal.sock`).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(e.g.`unix:/serverdir/node/internal.sock`).
(e.g. `unix:/serverdir/node/internal.sock`).

need access to the [network's current genesis file] and your signer's private
key:

- `oasis-node stake account gen_transfer`
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to be consistent with the previous list, the descriptions should be added.

- `--signer.backend file`: Specifies use of the file signer.

[Oasis Core Ledger]: https://github.com/oasisprotocol/oasis-core-ledger/blob/master/docs/usage/transactions.md
[Running a Node on the Network]: https://github.com/oasisprotocol/docs/blob/main/docs/node/run-your-node/validator-node.mdx#creating-your-entity
Copy link
Contributor

Choose a reason for hiding this comment

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

404

Copy link
Member Author

Choose a reason for hiding this comment

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

This hasn't been merged yet: oasisprotocol/docs#399


- `--genesis.file`: Path to the genesis file, e.g. `/localhostdir/genesis.json`.

For convenience, set the `GENESIS_FILE` environment value to its value, e.g.:
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not obvious (until you get to the TX_FLAGS) why would one set this env value. This paragraph also splits enumeration into 2 parts, so one option would be to move it to the section Storing Base and Signer flags in an Environment Variable.

11450.384816640 / 10,185,014,125,910 which is 1.124238481664054 * 10^-9 token
per share.

For 357 billion shares, the amount of tokens that will be reclaimed is thu
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For 357 billion shares, the amount of tokens that will be reclaimed is thu
For 357 billion shares, the amount of tokens that will be reclaimed is thus

command:

```bash
oasis-node governance list_proposals --address unix:/path/to/node/internal.sock
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we split this into multiple subsections, e.g. list_proposals, proposal_votes, ...?


:::info

At this time only entities which have active validator nodes scheduled in the
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a command/script to check that?

- `oasis1qr6swa6gsp2ukfjcdmka8wrkrwz294t7ev39nrw6`is our staking account
address.

We're not allowed to change the commission bounds too close in near future, so
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also explain with few words why.

advance (`rate_bound_lead`) and that we can change a commission rate on every
epoch (`rate_change_interval`).

The `max_rate_steps` and `max_bound_steps` determine the maximum number of
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't explain anything.

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.

3 participants