[P2P] refactor: unicast router

bryanchriswhite commented 1 year ago

Description

Factor out "unicast routing" concerns which will be common to both RainTree and background routers. To implement the Router interface (which I believe is applied appropriately), each must be able to both send and receive messages directly to/from individual peers. In libp2p this is done via streams.

Before

classDiagram
    class RainTreeMessage {
        <<protobuf>>
        +Level uint32
        +Data []byte
    }

    class PocketEnvelope {
        <<protobuf>>
        +Content *anypb.Any
        +Nonce uint64
    }

    RainTreeMessage --* PocketEnvelope : serialized as `Data`

    %% class p2pModule {
    %%     -handlePocketEnvelope([]byte) error
    %% }

    %% class P2PModule {
    %%     <<interface>>
    %%     GetAddress() (Address, error)
    %%     HandleEvent(*anypb.Any) error
    %%     Send([]byte, address Address) error
    %%     Broadcast([]byte) error
    %%     BroadcastStaked([]byte) error
    %% }
    %% p2pModule --|> P2PModule

    class RainTreeRouter {
        -handler RouterHandler
        +Broadcast([]byte) error
        +Send([]byte, address Address) error
        -handleStream(stream libp2pNetwork.Stream)
        -readStream(stream libp2pNetwork.Stream)
        -handleRainTreeMsg([]byte) ([]byte, error)
    }

    %% p2pModule --* RainTreeRouter
    %% RainTreeRouter --> p2pModule : `handler` == `handlePocketEnvelope`
    RainTreeRouter --o RainTreeMessage

    %% p2pModule --o PocketEnvelope
    %% p2pModule --* NonceDeduper

    class Router {
        <<interface>>
        +Send([]byte, address Address) error
        +Broadcast([]byte) error
    }
    RainTreeRouter --|> Router

After

classDiagram
    class RainTreeMessage {
        <<protobuf>>
        +Level uint32
        +Data []byte
    }

    class PocketEnvelope {
        <<protobuf>>
        +Content *anypb.Any
        +Nonce uint64
    }

    RainTreeMessage --* PocketEnvelope : serialized as `Data`

    %% class p2pModule {
    %%     -handlePocketEnvelope([]byte) error
    %% }

    %% class P2PModule {
    %%     <<interface>>
    %%     GetAddress() (Address, error)
    %%     HandleEvent(*anypb.Any) error
    %%     Send([]byte, address Address) error
    %%     Broadcast([]byte) error
    %%     BroadcastStaked([]byte) error
    %% }
    %% p2pModule --|> P2PModule

    class RainTreeRouter {
        UnicastRouter
        -handler MessageHandler
        +Broadcast([]byte) error
        -handleRainTreeMsg([]byte) error
    }

    class UnicastRouter {
        -messageHandler MessageHandler
        -peerHandler PeerHandler
        +Send([]byte, address Address) error
        -handleStream(stream libp2pNetwork.Stream)
        -readStream(stream libp2pNetwork.Stream)
    }
    RainTreeRouter --* UnicastRouter : (embedded)
    %% UnicastRouter --> RainTreeRouter : via `messageHandler`

    %% p2pModule --* RainTreeRouter
    %% RainTreeRouter --> p2pModule : `handler` == `handlePocketEnvelope`
    RainTreeRouter --o RainTreeMessage

    %% p2pModule --o PocketEnvelope
    %% p2pModule --* NonceDeduper

    class Router {
        <<interface>>
        +Send([]byte, address Address) error
        +Broadcast([]byte) error
    }
    RainTreeRouter --|> Router

See #505 "Integration / Architecture" class diagrams for more context.

Summary generated by Reviewpad on 29 Jun 23 06:52 UTC

This pull request includes several changes related to improving testability, reducing technical debt, and implementing unicast functionality.

The overall changes aim to improve the code structure, make it more maintainable and testable, and add support for handling unicast messages in a peer-to-peer network.

Here is a summary of the changes per file:

In the file setup.go, changes were made to register providers and routers to the module registry and improve testability.
In the file router.go, changes were made to improve peer discovery, introduce encapsulated approaches, and share interfaces among modules. Additionally, type and function names were updated.
The file logging.go underwent a rename and changes were made to the package name, logging functionality, and function names.
In the file network.go, changes were made to imports, type names, function names, and the implementation of the handleRainTreeMsg function to support the unicast functionality.
The file testutil.go was added as a new file to provide testing utilities for the unicast functionality.
In the file p2p/raintree/testutil.go, changes were made to imports and a function to call the HandleStream method from the UnicastRouter struct.

These changes collectively improve the testability, code structure, and unicast functionality in the project.

Issue

505

Dependants

732

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

List of changes

Refactored stream handling logic (i.e. unicast routing) out of rainTreeRouter and into new UnicastRouter type

Testing

[x] make develop_test; if any code changes were made
[x] make test_e2e on k8s LocalNet; if any code changes were made
[x] e2e-devnet-test passes tests on DevNet; if any code was changed
[ ] Docker Compose LocalNet; if any major functionality was changed or introduced
[x] k8s LocalNet; if any infrastructure or configuration changes were made

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 on touched members (see: 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
[x] I have added tests that prove my fix is effective or that my feature works
[ ] I have added, or updated, mermaid.js diagrams in the corresponding README(s)
[ ] I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 32.43% and project coverage change: +0.25 :tada:

Comparison is base (9d5dffe) 31.52% compared to head (fe24824) 31.78%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #844 +/- ## ========================================== + Coverage 31.52% 31.78% +0.25% ========================================== Files 107 106 -1 Lines 9034 8996 -38 ========================================== + Hits 2848 2859 +11 + Misses 5846 5795 -51 - Partials 340 342 +2 ``` | [Impacted Files](https://app.codecov.io/gh/pokt-network/pocket/pull/844?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pokt-network) | Coverage Δ | | |---|---|---| | [p2p/raintree/testutil.go](https://app.codecov.io/gh/pokt-network/pocket/pull/844?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pokt-network#diff-cDJwL3JhaW50cmVlL3Rlc3R1dGlsLmdv) | `0.00% <0.00%> (ø)` | | | [p2p/raintree/router.go](https://app.codecov.io/gh/pokt-network/pocket/pull/844?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pokt-network#diff-cDJwL3JhaW50cmVlL3JvdXRlci5nbw==) | `29.53% <40.00%> (+8.71%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

gitguardian[bot] commented 1 year ago

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request

| GitGuardian id | Secret | Commit | Filename | | | -------------- | ------------------------- | ---------------- | --------------- | -------------------- | | [5841025](https://dashboard.gitguardian.com/incidents/5841025?occurrence=99443742) | Generic High Entropy Secret | 4b19d8f561ff6bb628748bae16713c3eb8ed1424 | charts/pocket/templates/configmap-genesis.yaml | [View secret](https://github.com/pokt-network/pocket/commit/4b19d8f561ff6bb628748bae16713c3eb8ed1424#diff-33d68fb410c6b83957f59a2c6fcdc96d62a90d236e6a2a65bf24b205baa46f5dR1799) |

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code. 2. Replace and store your secret safely. [Learn here](https://blog.gitguardian.com/secrets-api-management?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment) the best practices. 3. Revoke and [rotate this secret](https://docs.gitguardian.com/secrets-detection/detectors/generics/generic_high_entropy_secret#revoke-the-secret?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment). 4. If possible, [rewrite git history](https://blog.gitguardian.com/rewriting-git-history-cheatsheet?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment). Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data. To avoid such incidents in the future consider - following these [best practices](https://blog.gitguardian.com/secrets-api-management/?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment) for managing and storing secrets including API keys and other credentials - install [secret detection on pre-commit](https://docs.gitguardian.com/ggshield-docs/integrations/git-hooks/pre-commit?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment) to catch secret before it leaves your machine and ease remediation.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!}

pokt-network / pocket