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] refactor: peerstore provider (part 1) #804

Merged
merged 42 commits into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
34d42bf
refactor: move `persistencePeerstoreProvider` & implement `#GetUnstak…
bryanchriswhite Jun 5, 2023
142071d
feat: add `p2pPeerstoreProvider`
bryanchriswhite Jun 5, 2023
bcf3f65
refactor: only embed IntegratableModule in PeerstoreProvider
bryanchriswhite Jun 5, 2023
00730d4
refactor: remove unused `PeerstoreProvider#GetP2PConfig()` method
bryanchriswhite Jun 5, 2023
41d18ae
feat: add `PeerstoreProvider#GetUnstakedPeerstore()` interface method
bryanchriswhite Jun 5, 2023
507d8af
chore: implement `rpcPeerstoreProvider#GetUnstakedPeerstore()`
bryanchriswhite Jun 5, 2023
e74fea4
chore: add TECHDEBT comment
bryanchriswhite Jun 5, 2023
6128d84
chore: add `Factory` generic type
bryanchriswhite Jun 5, 2023
4bc605a
chore: update comments
bryanchriswhite Jun 5, 2023
094a95e
chore: update changelogs
bryanchriswhite Jun 5, 2023
2a5b300
chore: update changelogs
bryanchriswhite Jun 6, 2023
1d03f38
empty commit
bryanchriswhite Jun 6, 2023
c1318da
chore: replace empty interfaces with `any`
bryanchriswhite Jun 6, 2023
8d1f62a
chore: edit comment
bryanchriswhite Jun 6, 2023
08bcafd
chore: remove unused `GetP2PConfig()` method
bryanchriswhite Jun 7, 2023
5a54400
chore: add godoc comments
bryanchriswhite Jun 7, 2023
5000ad8
fix: retrieve p2p mdoule from bus on each call
bryanchriswhite Jun 7, 2023
c587afa
refactor: persistence peerstor provider
bryanchriswhite Jun 7, 2023
e70b99c
refactor: embed `p2pPStoreProviderFactory`
bryanchriswhite Jun 7, 2023
b3ce271
chore: oneline function signature
bryanchriswhite Jun 7, 2023
8a476a4
refactor: consolidate p2pPeerstoreProvider into persistencePeerstorPr…
bryanchriswhite Jun 7, 2023
650d432
refactor: rename persistence.go back to provider.go
bryanchriswhite Jun 7, 2023
59fffb5
refactor: `p2pPeerstoreProvider` to a single function'
bryanchriswhite Jun 7, 2023
a1e22c4
refactor: update peerstore provider method receivers
bryanchriswhite Jun 7, 2023
ccd9fb7
refactor: re-implement `GetUnstakedPeerstore`
bryanchriswhite Jun 7, 2023
6e1a5b8
chore: add TECHDEBT comments
bryanchriswhite Jun 8, 2023
60302a4
chore: add issue numbers to TECHDEBT comments
bryanchriswhite Jun 8, 2023
1ac2e00
chore: improve comment
bryanchriswhite Jun 8, 2023
3a8e138
refactor: rename `T` & `K` type params to `M` &`C`
bryanchriswhite Jun 8, 2023
0cbca43
chore: update changelogs
bryanchriswhite Jun 8, 2023
9cee9dd
fix: bugs
bryanchriswhite Jun 8, 2023
2489b92
Merge remote-tracking branch 'pokt/main' into refactor/peerstore-prov…
bryanchriswhite Jun 8, 2023
c98e8d1
chore: add TECHDEBT comments
bryanchriswhite Jun 12, 2023
84ce1bd
chore: combine `NewRPCPeerstoreProvider()` & `Create()`
bryanchriswhite Jun 12, 2023
341d982
refactor: rename `NewPersistencePeerstoreProvider()` to `Create()`
bryanchriswhite Jun 12, 2023
a9ffa56
chore: persistence peerstore provider includes all staked actors
bryanchriswhite Jun 12, 2023
0d92a09
chore: fix consensus test
bryanchriswhite Jun 12, 2023
024d88b
fix: p2p test
bryanchriswhite Jun 12, 2023
22ff0fd
chore: update changelogs
bryanchriswhite Jun 12, 2023
044f6f1
revert: `readCtx.GetAllStakedActors()`
bryanchriswhite Jun 13, 2023
b273e87
chore: update changelogs
bryanchriswhite Jun 13, 2023
2e4d60d
chore: fix nit
bryanchriswhite Jun 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions p2p/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [0.0.0.54] - 2023-06-05

- Replaced embedded modules.Module with simpler modules.IntegratableModule in PeerstoreProvider interface
- Removed unused PeerstoreProvider#GetP2PConfig() method
- Added PeerstoreProvider#GetUnstakedPeerstore() method
- Added p2pPeerstoreProvider implementation of PeerstoreProvider interface

## [0.0.0.53] - 2023-06-01

- Moved nonce field from RainTreeMessage to PocketEnvelope protobuf types
Expand Down
5 changes: 2 additions & 3 deletions p2p/providers/peerstore_provider/peerstore_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package peerstore_provider
import (
"github.com/pokt-network/pocket/logger"
typesP2P "github.com/pokt-network/pocket/p2p/types"
"github.com/pokt-network/pocket/runtime/configs"
coreTypes "github.com/pokt-network/pocket/shared/core/types"
cryptoPocket "github.com/pokt-network/pocket/shared/crypto"
"github.com/pokt-network/pocket/shared/modules"
Expand All @@ -16,10 +15,10 @@ const ModuleName = "peerstore_provider"

// PeerstoreProvider is an interface that provides Peerstore accessors
type PeerstoreProvider interface {
modules.Module
modules.IntegratableModule

GetStakedPeerstoreAtHeight(height uint64) (typesP2P.Peerstore, error)
GetP2PConfig() *configs.P2PConfig
GetUnstakedPeerstore() (typesP2P.Peerstore, error)
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
}

func ActorsToPeerstore(abp PeerstoreProvider, actors []*coreTypes.Actor) (pstore typesP2P.Peerstore, errs error) {
Expand Down
72 changes: 72 additions & 0 deletions p2p/providers/peerstore_provider/persistence/p2p.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package persistence

import (
"fmt"

"github.com/pokt-network/pocket/p2p/providers/peerstore_provider"
typesP2P "github.com/pokt-network/pocket/p2p/types"
"github.com/pokt-network/pocket/shared/modules"
"github.com/pokt-network/pocket/shared/modules/base_modules"
)

var (
_ peerstore_provider.PeerstoreProvider = &p2pPeerstoreProvider{}
_ p2pPStoreProviderFactory = &p2pPeerstoreProvider{}
)

// unstakedPeerstoreProvider is an interface which the p2p module supports in
// order to allow access to the unstaked-actor-router's peerstore
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
//
// TECHDEBT(#xxx): will become unnecessary after `modules.P2PModule#GetUnstakedPeerstore` is added.`
// CONSIDERATION: split `PeerstoreProvider` into `StakedPeerstoreProvider` and `UnstakedPeerstoreProvider`.
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
// (see: https://github.com/pokt-network/pocket/pull/804#issuecomment-1576531916)
type unstakedPeerstoreProvider interface {
GetUnstakedPeerstore() (typesP2P.Peerstore, error)
}

type p2pPStoreProviderFactory = modules.Factory[peerstore_provider.PeerstoreProvider]
Olshansk marked this conversation as resolved.
Show resolved Hide resolved

type p2pPeerstoreProvider struct {
base_modules.IntegratableModule
persistencePeerstoreProvider

p2pModule modules.P2PModule
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
}

func NewP2PPeerstoreProvider(
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
bus modules.Bus,
) (peerstore_provider.PeerstoreProvider, error) {
return new(p2pPeerstoreProvider).Create(bus)
}

func (*p2pPeerstoreProvider) Create(
bus modules.Bus,
) (peerstore_provider.PeerstoreProvider, error) {
if bus == nil {
return nil, fmt.Errorf("bus is required")
}

p2pModule := bus.GetP2PModule()
if p2pModule == nil {
return nil, fmt.Errorf("p2p module is not registered to bus and is required")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: consider passing the bus to the Errrof for debugging purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The bus doesn't implement Stringer, what do you expect/want the DX to look/feel like?

Copy link
Member

Choose a reason for hiding this comment

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

☝️ The bus is just a point to our "DI framework" and "pubsub" so curious what you had in mind for this @dylanlott?

}

p2pPSP := &p2pPeerstoreProvider{
IntegratableModule: *base_modules.NewIntegratableModule(bus),
p2pModule: p2pModule,
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
}

return p2pPSP, nil
}

func (*p2pPeerstoreProvider) GetModuleName() string {
return peerstore_provider.ModuleName
}

func (p2pPSP *p2pPeerstoreProvider) GetUnstakedPeerstore() (typesP2P.Peerstore, error) {
unstakedPSP, ok := p2pPSP.p2pModule.(unstakedPeerstoreProvider)
if !ok {
return nil, fmt.Errorf("p2p module does not implement unstakedPeerstoreProvider")
}
return unstakedPSP.GetUnstakedPeerstore()
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package persistence

import (
"fmt"

"github.com/pokt-network/pocket/p2p/providers/peerstore_provider"
typesP2P "github.com/pokt-network/pocket/p2p/types"
"github.com/pokt-network/pocket/runtime/configs"
Expand All @@ -15,6 +17,7 @@ type persistencePeerstoreProvider struct {
base_modules.InterruptableModule
}

// TECHDEBT: refactor
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
func NewPersistencePeerstoreProvider(bus modules.Bus, options ...func(*persistencePeerstoreProvider)) *persistencePeerstoreProvider {
pabp := &persistencePeerstoreProvider{
IntegratableModule: *base_modules.NewIntegratableModule(bus),
Expand All @@ -27,10 +30,12 @@ func NewPersistencePeerstoreProvider(bus modules.Bus, options ...func(*persisten
return pabp
}

// TECHDEBT: remove
func Create(bus modules.Bus, options ...modules.ModuleOption) (modules.Module, error) {
return new(persistencePeerstoreProvider).Create(bus, options...)
}

// TECHDEBT: refactor
func (*persistencePeerstoreProvider) Create(bus modules.Bus, options ...modules.ModuleOption) (modules.Module, error) {
return NewPersistencePeerstoreProvider(bus), nil
}
Expand All @@ -56,3 +61,7 @@ func (pabp *persistencePeerstoreProvider) GetStakedPeerstoreAtHeight(height uint
func (pabp *persistencePeerstoreProvider) GetP2PConfig() *configs.P2PConfig {
return pabp.GetBus().GetRuntimeMgr().GetConfig().P2P
}

func (pabp *persistencePeerstoreProvider) GetUnstakedPeerstore() (typesP2P.Peerstore, error) {
return nil, fmt.Errorf("persistence peerstore provider does not support unstaked peerstore")
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
}
4 changes: 4 additions & 0 deletions p2p/providers/peerstore_provider/rpc/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ func (rabp *rpcPeerstoreProvider) GetP2PConfig() *configs.P2PConfig {
return rabp.p2pCfg
}

func (rabp *rpcPeerstoreProvider) GetUnstakedPeerstore() (typesP2P.Peerstore, error) {
return nil, fmt.Errorf("unstaked peerstore not supported by rpc peerstore provider")
}

func (rabp *rpcPeerstoreProvider) initRPCClient() {
rpcClient, err := rpc.NewClientWithResponses(rabp.rpcURL)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions shared/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [0.0.0.60] - 2023-06-05

- Added `Factory` generic type
Olshansk marked this conversation as resolved.
Show resolved Hide resolved
- Improved godoc comments

## [0.0.0.59] - 2023-06-01

- Use ApplicationAddress in RelayMeta
Expand Down
18 changes: 12 additions & 6 deletions shared/modules/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,29 @@ package modules
// a variadic `ModuleOption` argument(s) and returns a `Module`and an error.
type ModuleFactoryWithOptions FactoryWithOptions[Module, ModuleOption]
Olshansk marked this conversation as resolved.
Show resolved Hide resolved

// FactoryWithConfig implements a `#Create()` factory method which takes a
// required "config" argument of type K and returns a value of type T and an error.
// Factory implements a `#Create()` factory method which takes a bus and returns
// a value of type T and an error.
type Factory[T interface{}] interface {
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
Create(bus Bus) (T, error)
}

// FactoryWithConfig implements a `#Create()` factory method which takes a bus and
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
// a required "config" argument of type K and returns a value of type T and an error.
// TECHDEBT: apply enforcement across applicable "sub-modules" (see: `p2p/raintree/router.go`: `raintTreeFactory`)
type FactoryWithConfig[T interface{}, K interface{}] interface {
Create(bus Bus, cfg K) (T, error)
}

// FactoryWithOptions implements a `#Create()` factory method which takes a
// variadic "optional" argument(s) of type O and returns a value of type T
// FactoryWithOptions implements a `#Create()` factory method which takes a bus
// and a variadic "optional" argument(s) of type O and returns a value of type T
// and an error.
// TECHDEBT: apply enforcement across applicable "sub-modules"
type FactoryWithOptions[T interface{}, O interface{}] interface {
Create(bus Bus, opts ...O) (T, error)
}

// FactoryWithConfigAndOptions implements a `#Create()` factory method which
// takes both a required "config" argument of type K and a variadic "optional"
// FactoryWithConfigAndOptions implements a `#Create()` factory method which takes
// a bus and both a required "config" argument of type K and a variadic "optional"
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
// argument(s) of type O and returns a value of type T and an error.
// TECHDEBT: apply enforcement across applicable "sub-modules"
type FactoryWithConfigAndOptions[T interface{}, K interface{}, O interface{}] interface {
Expand Down
2 changes: 2 additions & 0 deletions shared/modules/p2p_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ type P2PModule interface {

// Returns the public P2P address of this node
GetAddress() (cryptoPocket.Address, error)
// TECHDEBT(#xxx): uncomment after moving `typesP2P.Peerstore` interface to a shared package
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
// GetUnstakedPeerstore() (typesP2P.Peerstore, error)

// A network broadcast to all staked actors on the network using RainTree
Broadcast(msg *anypb.Any) error
Expand Down