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

[P2P] Integrate LibP2P #347

Closed
1 of 6 tasks
jessicadaugherty opened this issue Nov 13, 2022 · 8 comments · Fixed by #534, #535, #540, #546 or #545
Closed
1 of 6 tasks

[P2P] Integrate LibP2P #347

jessicadaugherty opened this issue Nov 13, 2022 · 8 comments · Fixed by #534, #535, #540, #546 or #545
Assignees
Labels
core Core infrastructure - protocol related p2p P2P specific changes

Comments

@jessicadaugherty
Copy link
Contributor

jessicadaugherty commented Nov 13, 2022

Objective

Use the results of pokt-network/pocket-network-protocol#17 to integrate LibP2P into the p2p module.

Origin Document(s)

Goals

  • Integrate LibP2P into Pocket Core

Deliverables

  • MVP integration where LibP2P transport layer is used to send messages

Non-Goals / Deliverables

  • Integrating P2P deeper into transport layer
  • Implementing peer churn or discovery
  • Exhaustively enabling all the features we will need from LibP2P

General issue checklist

  • Update the appropriate CHANGELOG
  • Update the README
  • Update the source code tree explanation
  • Update any relevant global documentation & references
  • Document small issues / TODOs along the way

Creators: @jessicadaugherty @Olshansk

@Olshansk
Copy link
Member

Per our team discussion, I believe the scope of this can be summarized to, at a very high level, to bullet point 3 from the research SPIKE:

Screen Shot 2022-11-15 at 2 19 31 PM

@jessicadaugherty jessicadaugherty changed the title [P2P] Peer Discovery - LibP2P - Integrate with Pocket Core [P2P] LibP2P - Integrate with Pocket Core Dec 5, 2022
@jessicadaugherty
Copy link
Contributor Author

jessicadaugherty commented Dec 5, 2022

Define a Host interface in the pocket repo and embed libP2P's Host interface inside of it, implementing the underlying interface but not enabling it yet.

@Olshansk
Copy link
Member

Olshansk commented Feb 6, 2023

Synched up @bryanchriswhite on this offline and wanted to capture the potential action items for a next step.

Idea: LibP2P could be treated as another "connection type" similar to what we implemented in #268.

Next steps:

  1. Add LibP2P as a connection type (alongside TCP & noop)
  2. Create a new ticket titled "Consolidate LibP2P & Pocket Interface"
  3. "Hack" things together (in terms of data types) to replace the transport layer in P2P
  4. Add comments similar to CLEANUP(#XXX): Cleanup these interfaces wherever we kind of hacked things together

Other things discussed:

  1. Lack of connection pooling in the current version was not an intentional "thought-through" deicsion - we just got things working using the standard Golang libraries and I did not document it explicitly.

EDIT: @bryanchriswhite Let me know if I missed anything or if you have additional questions and/or clarifications.

@deblasis
Copy link
Contributor

deblasis commented Feb 7, 2023

@Olshansk:

In principle, I like the idea and I think it's a sensible step, in fact, it was what I initially thought was the right thing to do first when I started looking into this. I quickly developed some doubts and therefore I preferred a more targeted approach (eg: starting with primitives like IDs and moving up from there) because I didn't know how to "size" the effort, it sounded like a blanket, not a t-shirt in my head at the time 😄 I wanted to start small while learning along the way.

Maybe my pivot was due to the fact that I didn't know LibP2P deeply enough to know, top of my head, all the contact points, how they work, etc.

Some extra thoughts:

  1. If we want to use LibP2P side-by-side with our other implementations, we would have to abstract with interfaces all the common parts so that everything works regardless of the implementation that's selected at runtime. It sounds like a "big change" but maybe I am wrong.
  2. What is the difference between hacking things together in terms of data types and being deliberate and targeting them one by one (wherever possible) in a more controlled manner?
  3. If you confidently think you can hack things together in the fastest way possible with the approach described above, and the answer to the point 2) is "because... it's faster" please IGNORE all the above and go for it. I love being wrong when the outcomes are positive! 🚀

Re: connection pooling, I think it's necessary for the long run but probably overkill until we reach networks with hundreds of nodes. LibP2P however should provide features like this in an "easy way" I believe. That's basically all the networking stuff that we won't have to implement ourselves :)

@Olshansk
Copy link
Member

Olshansk commented Feb 7, 2023

What is the difference between hacking things together in terms of data types and being deliberate and targeting them one by one (wherever possible) in a more controlled manner?

@bryanchriswhite did research on all the data types but it's still a big undertaking.

I thought that if we just wire it together (with TODOs and a separate connection type) everywhere, we'd be closer to this approach and clean it up after:

Making-sense-of-MVP-

Re: connection pooling, I think it's necessary for the long...

We're on the same page. It seems like it's the default and Bryan mentioned we have to make an explicit effort to turn it off. If it's less effort to keep it (during initial integration), let's keep it.


@bryanchriswhite This is an integration where I haven't jumped in to get my hands dirty so I'm speaking out of my depth. However, if you think it'd be helpful for me to get a foundation PR up, lmk.

@jessicadaugherty jessicadaugherty changed the title [P2P] LibP2P - Integrate with Pocket Core [P2P] LibP2P - Make LibP2P a Connection Type Feb 7, 2023
@bryanchriswhite bryanchriswhite linked a pull request Feb 8, 2023 that will close this issue
16 tasks
@bryanchriswhite
Copy link
Contributor

bryanchriswhite commented Feb 8, 2023

@Olshansk sorry for the delay in responding, I've been heads-down in preparation for being able to participate meaningfully. 😅

In my view, it's looking like the path of least resistance is to implement a new modules.Module , types.Network, and types.Transport. I also figured that it would then be more straightforward to add a boolean config flag rather than coupling to the connection/transport type. Transport is also a degree of freedom that would be available in a libp2p-based module.

I will fill in the PR description with more details when I'm back at my desk this afternoon.

@bryanchriswhite
Copy link
Contributor

FWIW, libp2p is doing almost all of the work. To get to a stdnetwork analog (i.e. fully-connected network), it's just a matter of plugging things in to the right place and/or making them the right shape. To get RainTree support working, some additional massaging and/or interface redesign will be required.

bryanchriswhite added a commit that referenced this issue Mar 1, 2023
## Description

This is another of a series of PRs split out from
#500. Here we update the base
and P2P configs in preparation for the addition of the libp2p module.

## Issue

#347 

## Type of change

Please mark the relevant option(s):

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

## List of changes

- Replace `consensus_port` with `port` in P2P config.
- Update default P2P config `port` to from `8080` to `42069`.
- Add `use_libp2p` field to base config.
- Add `hostname` field to P2P config.
- Update state hash test after modifying genesis (updated port numbers).

## Testing

- [x] `make develop_test`
- [x]
[LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)
w/ all of the steps outlined in the `README`

## 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 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 added a commit that referenced this issue Mar 2, 2023
## Description

This is the first of a series of PRs split out from #500. Here we set up
the new libp2p module directory structure and add crypto and identity /
network helpers.

## Issue

#347 

## Type of change

Please mark the relevant option(s):

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

## List of changes

- prepare pocket repo new libp2p module
- add pocket / libp2p identity helpers
- add url <--> multiaddr conversion helpers for use with libp2p (see:
https://github.com/multiformats/go-multiaddr)
- add pokt --> libp2p crypto helpers

## Testing

- [x] `make develop_test`
- [x]
[LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)
w/ all of the steps outlined in the `README`

<!-- 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 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
- [x] 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: Alessandro De Blasis <[email protected]>
bryanchriswhite added a commit that referenced this issue Mar 3, 2023
## Description

This is another of a series of PRs split out from
#500. Here we add a new
implementation of `typesP2P.Network` in terms of libp2p abstractions,
utilizing the helpers which were added in #534.

## Issue

#347 

## Type of change

Please mark the relevant option(s):

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

## List of changes

- Added a new `typesP2P.Network` implementation to the `libp2p` module
directory
- Added embedded `modules.InitializableModule` to the P2P
`AddrBookProvider` interface so that it can be dependency injected as a
`modules.Module` via the bus registry.
- Added `PoktProtocolID` for use within the libp2p module or by public
API consumers

## Testing

- [x] `make develop_test`
- [x]
[LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)
w/ all of the steps outlined in the `README`

## 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
- [ ] 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
- [x] 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: Alessandro De Blasis <[email protected]>
bryanchriswhite added a commit that referenced this issue Mar 3, 2023
## Description

This is another of a series of PRs split out from
#500. Here we add a new
`modules.P2PModule` implementation which utilizes the `typesP2P.Network`
implementation which was added in #540. It will be utilized together
with the config changes introduced by #535 in forthcoming changes to the
node and debug CLI.

## Issue

#347 

## Type of change

Please mark the relevant option(s):

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

## List of changes

- Added a new `modules.P2PModule` implementation to the `libp2p` module
directory

## Testing

- [x] `make develop_test`
- [x]
[LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)
w/ all of the steps outlined in the `README`

## 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
- [ ] 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 added a commit that referenced this issue Mar 3, 2023
## Description

This is another (last, probably) in a series of PRs split out from
#500. Here we finally enable
support for use of the new libp2p module (and dependent packages) by
considering the config changes made in #535 in the node and debug CLI.

## Issue

#347 

## Type of change

Please mark the relevant option(s):

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

## List of changes

- Support libp2p module in node
- Support libp2p module in debug CLI

## Testing

- [x] `make develop_test`
- [x]
[LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)
w/ all of the steps outlined in the `README`

<!-- 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 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)
@Olshansk
Copy link
Member

Olshansk commented Mar 6, 2023

@bryanchriswhite Does autoscaling (with staking validators) using k8s LocalNet + LibP2P work? https://github.com/pokt-network/pocket/tree/main/build/localnet#scaling-actors

bryanchriswhite added a commit that referenced this issue Mar 18, 2023
## Description

Due to circumstances and timing, @Olshansk's final review on #540 came
in after merging. This PR implements improvements based on that review
feedback.

## Issue

Related to #347

## 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

- Correct libp2p module tests
- Improve libp2p code quality in response to post-merge review feedback

## Testing

- [ ] `make develop_test`
- [ ]
[LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)
w/ all of the steps outlined in the `README`

## 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 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)
dylanlott pushed a commit that referenced this issue Mar 24, 2023
## Description

Due to circumstances and timing, @Olshansk's final review on #540 came
in after merging. This PR implements improvements based on that review
feedback.

## Issue

Related to #347

## 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

- Correct libp2p module tests
- Improve libp2p code quality in response to post-merge review feedback

## Testing

- [ ] `make develop_test`
- [ ]
[LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)
w/ all of the steps outlined in the `README`

## 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 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment