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.

Sign up for GitHub

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

Jump to bottom

[P2P] Peer discovery debug CLI sub-commands #730

Open
10 tasks
bryanchriswhite opened this issue May 4, 2023 · 2 comments · Fixed by #891 or #895 · May be fixed by #801 or #892
Open
10 tasks

[P2P] Peer discovery debug CLI sub-commands #730

bryanchriswhite opened this issue May 4, 2023 · 2 comments · Fixed by #891 or #895 · May be fixed by #801 or #892
Assignees
@bryanchriswhite
Labels
p2p P2P specific changes tooling tooling to support development, testing et al
Milestone
M1: Pocket PoS (P...

Comments

@bryanchriswhite
Copy link
Contributor

bryanchriswhite commented May 4, 2023 •
edited
Loading

Objective

Extend the debug CLI to expose sub-commands for driving peer discovery in the P2P module (analogous to how it currently supports driving consensus). This will facilitate both manual and automated testing efforts.

Origin Document

Based on a discussion between @dylanlott and myself about end-to-end test planning for peer discovery. It has been made clear in discussions past (probably in documentation somewhere as well) that we want to prioritize supporting robust manual testing/investigation of these dynamic systems (before they get a chance to run away from us).

Goals

  • Expose controls to peer discovery to developers
  • Support step-by-step progression through peer discovery
  • Support debugging & reproducing peer discovery scenarios (incl. edge cases)

Deliverable

  • Extend the debug CLI to expose the sub-commands subcommands outlined below (peer CLI UX)
  • (Bonus): draft a design for reproducible peer discovery failure reports or something
    • Could be generic (e.g. trace based)

CLI UX - peer & p2p subcommands

The following commands should send/broadcast debug messages into the network, resulting in nodes to print to their respective logs, similar to the debug subcommand's behavior.

peer discovery status # background router only
peer discovery start  # background router only
peer discovery stop   # background router only
peer list [--all | --staked | --unstaked]
peer forget (--all | <forgetter peer ID>)
peer connections
peer interrogate <peer ID>
peer disconnect <peer ID>

p2pCmd persisten flags:

  • --trace

NOTE: p2p commands MAY interact with a remote/standalone node rather than utilize the CLI's own P2P module

p2p send [--trace] <peer ID> <text message>
p2p broadcast [--trace] <text message>

Non-goals / Non-deliverables

  • Refactoring/simplifying the Router interface
  • Additional, unrelated libp2p integration work

General issue deliverables

  • Update the appropriate CHANGELOG(s)
  • Update any relevant local/global README(s)
  • Update relevant source code tree explanations
  • Add or update any relevant or supporting mermaid diagrams

Testing Methodology

  • New CLI subcommands behave as expected
  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md
  • k8s LocalNet: verify a k8s LocalNet is still functioning correctly by following the instructions here

Creator: @bryanchriswhite
@bryanchriswhite bryanchriswhite converted this from a draft issue May 4, 2023
@bryanchriswhite bryanchriswhite self-assigned this May 4, 2023
@bryanchriswhite bryanchriswhite changed the title [P2P] Peer discovery debug CLI sub-commands [WIP] [P2P] Peer discovery debug CLI sub-commands May 4, 2023
@bryanchriswhite bryanchriswhite moved this to Rescope in V1 Dashboard May 4, 2023
@bryanchriswhite bryanchriswhite removed the status in V1 Dashboard May 4, 2023
@bryanchriswhite bryanchriswhite added the p2p P2P specific changes label May 4, 2023
@bryanchriswhite bryanchriswhite moved this to Up Next in V1 Dashboard May 4, 2023
@bryanchriswhite bryanchriswhite changed the title [WIP] [P2P] Peer discovery debug CLI sub-commands [P2P] Peer discovery debug CLI sub-commands May 4, 2023
@bryanchriswhite bryanchriswhite added the tooling tooling to support development, testing et al label May 4, 2023
@bryanchriswhite bryanchriswhite moved this from Up Next to Rescope in V1 Dashboard May 31, 2023
@bryanchriswhite bryanchriswhite moved this from Rescope to In Progress in V1 Dashboard Jun 1, 2023
@bryanchriswhite bryanchriswhite linked a pull request Jun 2, 2023 that will close this issue
Open
20 tasks
@bryanchriswhite bryanchriswhite linked a pull request Jun 6, 2023 that will close this issue
[P2P] [Tooling] Peer discovery peer connections subcommand #801
Open
20 tasks
@bryanchriswhite bryanchriswhite mentioned this issue Jun 6, 2023
Merged
20 tasks
bryanchriswhite added a commit that referenced this issue Jun 13, 2023
@bryanchriswhite
[Client] [Tooling] refactor: CLI (#806)
12f9f49 
## Description

<!-- reviewpad:summarize:start -->
### Summary generated by Reviewpad on 13 Jun 23 12:53 UTC
This pull request includes various updates across the codebase,
including changes to CLI flags, import statements, and default values.
It also involves refactoring code to use shared helper functions,
removing unused code, and improving error handling. Some variables have
been renamed to be more descriptive. The specific changes include
modifications to files such as `validator.go`, `cluster-manager.yaml`,
`cli-client.yaml`, `utils.go`, and `runtime_defaults.go`. For example,
the `RPC_HOST` variable has been replaced with `POCKET_REMOTE_CLI_URL`.
A new `helper` package has been added, and a new `flags` package
contains several CLI flags. Code has also been updated to use
`flags.RemoteCLIURL` instead of `remoteCLIURL`. Some constants have been
renamed to include `Hostname`, and there are updates to endpoint
hostnames for some validators.
<!-- reviewpad:summarize:end -->

## Issue

Dependants:
- #730

## Type of change

Please mark the relevant option(s):

- [ ] New feature, functionality or library
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Major breaking change
- [ ] Documentation
- [ ] Other <!-- add details here if it a different type of change -->

## List of changes

- Exported `rootCmd` to support cross-package usage (i.e. subcommands w/
own pkgs)
- Refactored common CLI code
  - Moved & refactored `PersistentPreRunE()` helper
  - Moved & exported `busCLICtxKey`
  - Exported `GetValueFromCLIContext()` and `SetValueInCLIContext()`
  - Moved `setupAndStartP2PModule`
  - Moved `setupCurrentHeightProvider`
  - Moved `setupPeerstoreProvider`
- Refactored CLI flags to own package for cross-package use
- Replaced RPC_HOST with POCKET_REMOTE_CLI_URL where appropriate
- Added `remote-cli-url` flag to cluster manager & refactor
- Added support for overriding of `remote-cli-url` flag with env var
- Append "Hostname" to validator endpoint hostname constants
- Promoted string literal to `RandomValidatorEndpointK8SHostname`
constant
- Improved e2e tests error messaging
- Simplify e2e debug command calls
- Improve error handling in client CLI


## Testing

- [ ] `make develop_test`; if any code changes were made
- [ ] `make test_e2e` on [k8s
LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md);
if any code changes were made
- [ ] `e2e-devnet-test` passes tests on
[DevNet](https://pocketnetwork.notion.site/How-to-DevNet-ff1598f27efe44c09f34e2aa0051f0dd);
if any code was changed
- [x] [Docker Compose
LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md);
if any major functionality was changed or introduced
- [x] [k8s
LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md);
if any infrastructure or configuration changes were made


## Required Checklist

- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added, or updated, [`godoc` format
comments](https://go.dev/blog/godoc) on touched members (see:
[tip.golang.org/doc/comment](https://tip.golang.org/doc/comment))
- [x] I have tested my changes using the available tooling
- [x] I have updated the corresponding CHANGELOG

### If Applicable Checklist

- [ ] I have updated the corresponding README(s); local and/or global
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added, or updated,
[mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding
README(s)
- [ ] I have added, or updated, documentation and
[mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*`
if I updated `shared/*`README(s)
@bryanchriswhite bryanchriswhite linked a pull request Jul 7, 2023 that will close this issue
[CLI] Consistent config/flag parsing & common helpers #891
Merged
20 tasks
@bryanchriswhite bryanchriswhite linked a pull request Jul 7, 2023 that will close this issue
Open
20 tasks
@bryanchriswhite bryanchriswhite linked a pull request Jul 7, 2023 that will close this issue
[P2P] [Tooling] Peer discovery peer list subcommand #892
Open
20 tasks
@bryanchriswhite bryanchriswhite mentioned this issue Jul 10, 2023
Merged
20 tasks
@bryanchriswhite bryanchriswhite linked a pull request Jul 11, 2023 that will close this issue
[P2P] Refactor P2P submodules #895
Merged
20 tasks
@bryanchriswhite bryanchriswhite mentioned this issue Jul 11, 2023
Merged
20 tasks
bryanchriswhite added a commit that referenced this issue Jul 13, 2023
@bryanchriswhite @Olshansk
[P2P] Refactor P2P submodules (#895)
41c3d32 
## @Reviewer
This PR may be more digestible / reviewable on a commit-by-commit basis.
Commits are organized logically and any given line is only modified in a
single commit, with few exceptions*.

*(In the interest of preserving the git-time-continuum
:police_officer::rotating_light:, this applies in batches of commits
between comments or reviews *by humans*, only once "in review")

---

## Description

Refactor P2P module dependencies as submodules, ultimately to support
usage P2P module usage in the CLI.

## Issue

Related:

- #730 

Dependant(s):

- #891 
- #801 
- #892 

## Type of change

Please mark the relevant option(s):

- [ ] New feature, functionality or library
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Major breaking change
- [ ] Documentation
- [ ] Other <!-- add details here if it a different type of change -->

## List of changes

- decoupled P2P module dependencies from Consensus module (disambiguates
modules registry names)
  - added "current_height_provider" module registry slot
  - promoted `CurrentHeightProvider` to a submodule interface type
  - added `consensusCurrentHeightProvider` implementation
- promoted `Router` interface to a submodule interface type
  - added "staked_actor_router" modules registry slot
  - added "unstaked_actor_router" modules registry slot
  - converted `backgroundRouter` implementation to submodule
  - converted `rainTreeRouter` implementation to submodule
  - simplified router base config

## Testing

- [x] `make develop_test`; if any code changes were made
- [x] `make test_e2e` on [k8s
LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md);
if any code changes were made
- [x] `e2e-devnet-test` passes tests on
[DevNet](https://pocketnetwork.notion.site/How-to-DevNet-ff1598f27efe44c09f34e2aa0051f0dd);
if any code was changed
- [ ] [Docker Compose
LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md);
if any major functionality was changed or introduced
- [x] [k8s
LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md);
if any infrastructure or configuration changes were made

<!-- REMOVE this comment block after following the instructions
 If you added additional tests or infrastructure, describe it here.
 Bonus points for images and videos or gifs.
-->

## Required Checklist

- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added, or updated, [`godoc` format
comments](https://go.dev/blog/godoc) on touched members (see:
[tip.golang.org/doc/comment](https://tip.golang.org/doc/comment))
- [x] I have tested my changes using the available tooling
- [ ] I have updated the corresponding CHANGELOG

### If Applicable Checklist

- [ ] I have updated the corresponding README(s); local and/or global
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added, or updated,
[mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding
README(s)
- [ ] I have added, or updated, documentation and
[mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*`
if I updated `shared/*`README(s)

---------

Co-authored-by: Daniel Olshansky <[email protected]>
@github-project-automation github-project-automation bot moved this from In Progress to Done in V1 Dashboard Jul 13, 2023
@Olshansk
Copy link
Member

@bryanchriswhite We haven't merged in the P2P subcommand in yet, so do you think this was closed prematurely?

@Olshansk
Copy link
Member

Per my comment above, reopening until it's merged or told otherwise.

@Olshansk Olshansk reopened this Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bryanchriswhite bryanchriswhite

Labels
p2p P2P specific changes tooling tooling to support development, testing et al
Projects
V1 Dashboard
Status: Done
Milestone
M1: Pocket PoS (Proof of Stake)
2 participants
@bryanchriswhite @Olshansk