[P2P] Refactor P2P interfaces

[bryanchriswhite]

@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.

---

Description

Redifines P2P module packages in terms of new interfaces:

• Peer / PeerList
• Peerstore
• PeerManager / PeersView

This has the effect of decoupling the Network implementation (e.g. rainTreeNetwork) from the peer representation and storage/lookup. This will be necessary for use with the libp2p based P2P module as it uses different implementations for these concerns.

Organization

This PR adds a temporary shared/p2p package to facilitate the migration to the new libp2p based P2P module implementation. This package will be re-consolidated into the remaining P2P module as part of #554.

├── p2p
│   └── * addrbook_delta_test.go -> peer_test.go
└── shared
    └── p2p
        ├── + peer.go
        ├── + peer_manager.go
        └── + peerstore.go

peer.go

• Peer interface
  • abstracted from typesP2P.NetworkPeer and its usage
• PeerList concrete type ([]Peer)
  • implements a #Delta() method
    • ported from GetAddrBookDelta

peerstore.go

• Peerstore interface plus a simple map-based implementation, PeerAddrMap

peer_manager.go

• PeerManager and PeerView interfaces
  • abstracted from raintree.peersManager and its usage
• SortedPeerManager implementation factored out of the same, raintree.peersManager
  • decouples sorting and "view construction" (networkView) behavior from any conception of raintree specific network features (e.g. "levels")

Nomenclature changes

AddrBook --> Peerstore: The word "address" is becoming ambiguous as libp2p nomenclature uses it to refer to the network address (e.g. IP / hostname). As it's currently used, this object holds more than just the both identity and network address so regardless of interpretation, I would argue that AddrBook doesn't accurately convey its responsibility. Peerstore also better aligns with other libp2p nomenclature and should result it in more consistent and coherent integration and refactoring going forward.

Potential next steps

1. Decouple peer connection management from identity/address storage and lookup.
   • Add a ConnManager interface to be responsible for this
     • Move Peer#GetStream() to it
     • Move PeerManager#GetSelfAddr() to it
     • Refactor accordingly

Issue

Fixes #552

Type of change

Please mark the relevant option(s):

• New feature, functionality or library
• Bug fix
• Code health or cleanup
• Major breaking change
• Documentation
• Other

List of changes

• Add Peer and Peerstore interfaces
• Refactor AddrBook to PeerList type
• Refactor getAddrBookDelta to be a member of PeerList
• Move typesP2P.AddrBookMap to sharedP2P.PeerAddrMap and refactor to implement the new Peerstore interface
• Factor SortedPeerManager out of raintree.peersManager and add peerManager interface
• Refactor raintree.peersManager to use SortedPeerManager and implement PeerManager interface
• Refactor stdnetwork.Network implementation to use P2P interfaces
• Refactor AddrBookProvider to use new P2P interfaces
• Refactor typesP2P.Network to use new P2P interfaces
• Refactor typesP2P.Transport to embed io.ReadWriteCloser
• Rename NetworkPeer#Dialer to NetworkPeer#Transportfor readability and consistency
• Refactor typesP2P.NetworkPeer to implement the new Peer interface
• Refactor debug CLI to use new P2P interfaces

Testing

• make develop_test
• LocalNet w/ all of the steps outlined in the README

Required Checklist

• 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 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)

[Olshansk]

@bryanchriswhite I didn't do a super deep dive, but mostly looks good.

Overall, the PR got pretty big with lots of renaming and things moving around, so it is getting a little hard to maintain overall context. No actionable feedback but just FYI that I don't think my review on this one is as best as it could be.

[Olshansk]

Nice

[Olshansk]

@bryanchriswhite I left a couple replies regarding that we could/should potentially add here. Use your best judgment to do what you deem is appropriate. Approving since it is not a blocker, since you will likely revisit a lot of it in the followup PR and because I'm getting a bit of "PR fatigue" from this one :)

[bryanchriswhite]

Thanks for bearing with me @Olshansk 🙏 I think we're almost done with the necessarily large PRs (after #576 😅).

FWIW, after reflecting on my most recent experience, I've noticed one heuristic I seemed to be using when thinking about if a PR like this can be split seems to boil down the atomicity of commits with respect to the test suite. I.e., can the commits be organized such that there is a commit at which the tests are passing prior to the conclusion of the main change? If the answer is yes, then you have a decent fist estimation for how/where to make a split.

[Olshansk]

can the commits be organized such that there is a commit at which the tests are passing prior to the conclusion of the main change? If the answer is yes, then you have a decent fist estimation for how/where to make a split.