From fe24824e458717bd4ca844cd94be3be34611b239 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 19 Jun 2023 13:41:15 +0200 Subject: [PATCH 01/59] refactor: unicast router --- internal/testutil/proxy.go | 3 + p2p/config/config.go | 44 ++++++++++- p2p/raintree/logging.go | 31 -------- p2p/raintree/router.go | 147 ++++++++++++------------------------- p2p/raintree/testutil.go | 22 +++++- p2p/types/router.go | 2 +- p2p/unicast/logging.go | 44 +++++++++++ p2p/unicast/router.go | 139 +++++++++++++++++++++++++++++++++++ p2p/unicast/testutil.go | 10 +++ 9 files changed, 306 insertions(+), 136 deletions(-) create mode 100644 internal/testutil/proxy.go delete mode 100644 p2p/raintree/logging.go create mode 100644 p2p/unicast/logging.go create mode 100644 p2p/unicast/router.go create mode 100644 p2p/unicast/testutil.go diff --git a/internal/testutil/proxy.go b/internal/testutil/proxy.go new file mode 100644 index 000000000..a586a6c05 --- /dev/null +++ b/internal/testutil/proxy.go @@ -0,0 +1,3 @@ +package testutil + +type ProxyFactory[T any] func(target T) (proxy T) diff --git a/p2p/config/config.go b/p2p/config/config.go index e64fe8519..d98f4b969 100644 --- a/p2p/config/config.go +++ b/p2p/config/config.go @@ -4,9 +4,19 @@ import ( "fmt" "github.com/libp2p/go-libp2p/core/host" + "github.com/libp2p/go-libp2p/core/protocol" + "go.uber.org/multierr" + "github.com/pokt-network/pocket/p2p/providers" + typesP2P "github.com/pokt-network/pocket/p2p/types" "github.com/pokt-network/pocket/shared/crypto" - "go.uber.org/multierr" + "github.com/pokt-network/pocket/shared/modules" +) + +var ( + _ typesP2P.RouterConfig = &baseConfig{} + _ typesP2P.RouterConfig = &UnicastRouterConfig{} + _ typesP2P.RouterConfig = &RainTreeConfig{} ) // baseConfig implements `RouterConfig` using the given libp2p host and current @@ -22,6 +32,14 @@ type baseConfig struct { PeerstoreProvider providers.PeerstoreProvider } +type UnicastRouterConfig struct { + Logger *modules.Logger + Host host.Host + ProtocolID protocol.ID + MessageHandler typesP2P.MessageHandler + PeerHandler func(peer typesP2P.Peer) error +} + // BackgroundConfig implements `RouterConfig` for use with `BackgroundRouter`. type BackgroundConfig struct { Host host.Host @@ -57,6 +75,30 @@ func (cfg *baseConfig) IsValid() (err error) { if cfg.PeerstoreProvider == nil { err = multierr.Append(err, fmt.Errorf("peerstore provider not configured")) } + return nil +} + +// IsValid implements the respective member of the `RouterConfig` interface. +func (cfg *UnicastRouterConfig) IsValid() (err error) { + if cfg.Logger == nil { + err = multierr.Append(err, fmt.Errorf("logger not configured")) + } + + if cfg.Host == nil { + err = multierr.Append(err, fmt.Errorf("host not configured")) + } + + if cfg.ProtocolID == "" { + err = multierr.Append(err, fmt.Errorf("protocol id not configured")) + } + + if cfg.MessageHandler == nil { + err = multierr.Append(err, fmt.Errorf("message handler not configured")) + } + + if cfg.PeerHandler == nil { + err = multierr.Append(err, fmt.Errorf("peer handler not configured")) + } return err } diff --git a/p2p/raintree/logging.go b/p2p/raintree/logging.go deleted file mode 100644 index bbe3e6c3b..000000000 --- a/p2p/raintree/logging.go +++ /dev/null @@ -1,31 +0,0 @@ -package raintree - -import ( - libp2pNetwork "github.com/libp2p/go-libp2p/core/network" - - "github.com/pokt-network/pocket/logger" - "github.com/pokt-network/pocket/p2p/utils" -) - -// logStream logs the incoming stream and its scope stats -func (rtr *rainTreeRouter) logStream(stream libp2pNetwork.Stream) { - rtr.logStreamScopeStats(stream) - - remotePeer, err := utils.PeerFromLibp2pStream(stream) - if err != nil { - rtr.logger.Debug().Err(err).Msg("getting remote remotePeer") - } else { - utils.LogIncomingMsg(rtr.logger, rtr.getHostname(), remotePeer) - } -} - -// logStreamScopeStats logs the incoming stream's scope stats -// (see: https://pkg.go.dev/github.com/libp2p/go-libp2p@v0.27.0/core/network#StreamScope) -func (rtr *rainTreeRouter) logStreamScopeStats(stream libp2pNetwork.Stream) { - if err := utils.LogScopeStatFactory( - &logger.Global.Logger, - "stream scope (read-side)", - )(stream.Scope()); err != nil { - rtr.logger.Debug().Err(err).Msg("logging stream scope stats") - } -} diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index 707f0a0a9..a68fd293d 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -2,11 +2,8 @@ package raintree import ( "fmt" - "io" - "time" - libp2pHost "github.com/libp2p/go-libp2p/core/host" - libp2pNetwork "github.com/libp2p/go-libp2p/core/network" + "github.com/pokt-network/pocket/p2p/unicast" "google.golang.org/protobuf/proto" "github.com/pokt-network/pocket/logger" @@ -22,15 +19,9 @@ import ( "github.com/pokt-network/pocket/shared/messaging" "github.com/pokt-network/pocket/shared/modules" "github.com/pokt-network/pocket/shared/modules/base_modules" - telemetry "github.com/pokt-network/pocket/telemetry" + "github.com/pokt-network/pocket/telemetry" ) -// TECHDEBT(#629): configure timeouts. Consider security exposure vs. real-world conditions. -// TECHDEBT(#629): parameterize and expose via config. -// readStreamTimeout is the duration to wait for a read operation on a -// stream to complete, after which the stream is closed ("timed out"). -const readStreamTimeout = time.Second * 10 - var ( _ typesP2P.Router = &rainTreeRouter{} _ modules.IntegratableModule = &rainTreeRouter{} @@ -41,10 +32,11 @@ type rainTreeFactory = modules.FactoryWithConfig[typesP2P.Router, *config.RainTr type rainTreeRouter struct { base_modules.IntegratableModule + unicast.UnicastRouter logger *modules.Logger // handler is the function to call when a message is received. - handler typesP2P.RouterHandler + handler typesP2P.MessageHandler // host represents a libp2p libp2pNetwork node, it encapsulates a libp2p peerstore // & connection manager. `libp2p.New` configures and starts listening // according to options. @@ -84,7 +76,6 @@ func (*rainTreeRouter) Create(bus modules.Bus, cfg *config.RainTreeConfig) (type return nil, err } - rtr.host.SetStreamHandler(protocol.PoktProtocolID, rtr.handleStream) return typesP2P.Router(rtr), nil } @@ -191,9 +182,11 @@ func (rtr *rainTreeRouter) sendInternal(data []byte, address cryptoPocket.Addres return nil } -// handleRainTreeMsg handles a RainTree message, continuing broadcast propagation -// if applicable. Returns the serialized `PocketEnvelope` data contained within. -func (rtr *rainTreeRouter) handleRainTreeMsg(data []byte) ([]byte, error) { +// handleRainTreeMsg deserializes a RainTree message to extract the `PocketEnvelope` +// bytes contained within, continues broadcast propagation, if applicable, and +// passes them off to the application by calling the configured `rtr.handler`. +// Intended to be called in a go routine. +func (rtr *rainTreeRouter) handleRainTreeMsg(rainTreeMsgBz []byte) error { blockHeightInt := rtr.GetBus().GetConsensusModule().CurrentHeight() blockHeight := fmt.Sprintf("%d", blockHeightInt) @@ -207,25 +200,36 @@ func (rtr *rainTreeRouter) handleRainTreeMsg(data []byte) ([]byte, error) { ) var rainTreeMsg typesP2P.RainTreeMessage - if err := proto.Unmarshal(data, &rainTreeMsg); err != nil { - return nil, err + if err := proto.Unmarshal(rainTreeMsgBz, &rainTreeMsg); err != nil { + return err } + // TECHDEBT(#763): refactor as "pre-propagation validation" networkMessage := messaging.PocketEnvelope{} if err := proto.Unmarshal(rainTreeMsg.Data, &networkMessage); err != nil { rtr.logger.Error().Err(err).Msg("Error decoding network message") - return nil, err + return err } + // -- // Continue RainTree propagation if rainTreeMsg.Level > 0 { if err := rtr.broadcastAtLevel(rainTreeMsg.Data, rainTreeMsg.Level-1); err != nil { - return nil, err + return err } } - // Return the data back to the caller so it can be handled by the app specific bus - return rainTreeMsg.Data, nil + // There was no error, but we don't need to forward this to the app-specific bus. + // For example, the message has already been handled by the application. + if rainTreeMsg.Data == nil { + return nil + } + + // call configured handler to forward to app-specific bus + if err := rtr.handler(rainTreeMsg.Data); err != nil { + rtr.logger.Error().Err(err).Msg("handling raintree message") + } + return nil } // GetPeerstore implements the respective member of `typesP2P.Router`. @@ -270,94 +274,41 @@ func (rtr *rainTreeRouter) Size() int { return rtr.peersManager.GetPeerstore().Size() } -// handleStream ensures the peerstore contains the remote peer and then reads -// the incoming stream in a new go routine. -func (rtr *rainTreeRouter) handleStream(stream libp2pNetwork.Stream) { - rtr.logger.Debug().Msg("handling incoming stream") - peer, err := utils.PeerFromLibp2pStream(stream) - if err != nil { - rtr.logger.Error().Err(err). - Str("address", peer.GetAddress().String()). - Msg("parsing remote peer identity") - - if err = stream.Reset(); err != nil { - rtr.logger.Error().Err(err).Msg("resetting stream") - } - return - } - - if err := rtr.AddPeer(peer); err != nil { - rtr.logger.Error().Err(err). - Str("address", peer.GetAddress().String()). - Msg("adding remote peer to router") - } - - go rtr.readStream(stream) +// shouldSendToTarget returns false if target is self. +func shouldSendToTarget(target target) bool { + return !target.isSelf } -// readStream reads the incoming stream, extracts the serialized `PocketEnvelope` -// data from the incoming `RainTreeMessage`, and passes it to the application by -// calling the configured `rtr.handler`. Intended to be called in a go routine. -func (rtr *rainTreeRouter) readStream(stream libp2pNetwork.Stream) { - // Time out if no data is sent to free resources. - // NB: tests using libp2p's `mocknet` rely on this not returning an error. - if err := stream.SetReadDeadline(newReadStreamDeadline()); err != nil { - // `SetReadDeadline` not supported by `mocknet` streams. - rtr.logger.Error().Err(err).Msg("setting stream read deadline") +func (rtr *rainTreeRouter) setupUnicastRouter() error { + unicastRouterCfg := config.UnicastRouterConfig{ + Logger: rtr.logger, + Host: rtr.host, + ProtocolID: protocol.PoktProtocolID, + MessageHandler: rtr.handleRainTreeMsg, + PeerHandler: rtr.AddPeer, } - // log incoming stream - rtr.logStream(stream) - - // read stream - rainTreeMsgBz, err := io.ReadAll(stream) + unicastRouter, err := unicast.Create(rtr.GetBus(), &unicastRouterCfg) if err != nil { - rtr.logger.Error().Err(err).Msg("reading from stream") - if err := stream.Reset(); err != nil { - rtr.logger.Error().Err(err).Msg("resetting stream (read-side)") - } - return - } - - // done reading; reset to signal this to remote peer - // NB: failing to reset the stream can easily max out the number of available - // network connections on the receiver's side. - if err := stream.Reset(); err != nil { - rtr.logger.Error().Err(err).Msg("resetting stream (read-side)") - } - - // extract `PocketEnvelope` from `RainTreeMessage` (& continue propagation) - poktEnvelopeBz, err := rtr.handleRainTreeMsg(rainTreeMsgBz) - if err != nil { - rtr.logger.Error().Err(err).Msg("handling raintree message") - return - } - - // There was no error, but we don't need to forward this to the app-specific bus. - // For example, the message has already been handled by the application. - if poktEnvelopeBz == nil { - return - } - - // call configured handler to forward to app-specific bus - if err := rtr.handler(poktEnvelopeBz); err != nil { - rtr.logger.Error().Err(err).Msg("handling pocket envelope") + return fmt.Errorf("setting up unicast router: %w", err) } -} -// shouldSendToTarget returns false if target is self. -func shouldSendToTarget(target target) bool { - return !target.isSelf + rtr.UnicastRouter = *unicastRouter + return nil } func (rtr *rainTreeRouter) setupDependencies() error { + if err := rtr.setupUnicastRouter(); err != nil { + return err + } + pstore, err := rtr.pstoreProvider.GetStakedPeerstoreAtHeight(rtr.currentHeightProvider.CurrentHeight()) if err != nil { - return err + return fmt.Errorf("getting staked peerstore: %w", err) } if err := rtr.setupPeerManager(pstore); err != nil { - return err + return fmt.Errorf("setting up peer manager: %w", err) } if err := utils.PopulateLibp2pHost(rtr.host, pstore); err != nil { @@ -374,9 +325,3 @@ func (rtr *rainTreeRouter) setupPeerManager(pstore typesP2P.Peerstore) (err erro func (rtr *rainTreeRouter) getHostname() string { return rtr.GetBus().GetRuntimeMgr().GetConfig().P2P.Hostname } - -// newReadStreamDeadline returns a future deadline -// based on the read stream timeout duration. -func newReadStreamDeadline() time.Time { - return time.Now().Add(readStreamTimeout) -} diff --git a/p2p/raintree/testutil.go b/p2p/raintree/testutil.go index ebcb464b5..f12ef0a58 100644 --- a/p2p/raintree/testutil.go +++ b/p2p/raintree/testutil.go @@ -2,12 +2,30 @@ package raintree -import libp2pNetwork "github.com/libp2p/go-libp2p/core/network" +import ( + libp2pNetwork "github.com/libp2p/go-libp2p/core/network" + "github.com/regen-network/gocuke" + + "github.com/pokt-network/pocket/internal/testutil" + typesP2P "github.com/pokt-network/pocket/p2p/types" +) // RainTreeRouter exports `rainTreeRouter` for testing purposes. type RainTreeRouter = rainTreeRouter +type routerHandlerProxyFactory = testutil.ProxyFactory[typesP2P.MessageHandler] + // HandleStream exports `rainTreeRouter#handleStream` for testing purposes. func (rtr *rainTreeRouter) HandleStream(stream libp2pNetwork.Stream) { - rtr.handleStream(stream) + rtr.UnicastRouter.HandleStream(stream) +} +func (rtr *rainTreeRouter) HandlerProxy( + t gocuke.TestingT, + handlerProxyFactory routerHandlerProxyFactory, +) { + t.Helper() + + // pass original handler to proxy factory & replace it with the proxy + origHandler := rtr.handler + rtr.handler = handlerProxyFactory(origHandler) } diff --git a/p2p/types/router.go b/p2p/types/router.go index f9c7f2d74..21a2b8010 100644 --- a/p2p/types/router.go +++ b/p2p/types/router.go @@ -22,7 +22,7 @@ type Router interface { RemovePeer(peer Peer) error } -type RouterHandler func(data []byte) error +type MessageHandler func(data []byte) error // RouterConfig is used to configure `Router` implementations and to test a // given configuration's validity. diff --git a/p2p/unicast/logging.go b/p2p/unicast/logging.go new file mode 100644 index 000000000..7e2539c63 --- /dev/null +++ b/p2p/unicast/logging.go @@ -0,0 +1,44 @@ +package unicast + +import ( + libp2pNetwork "github.com/libp2p/go-libp2p/core/network" + + "github.com/pokt-network/pocket/logger" + "github.com/pokt-network/pocket/p2p/utils" +) + +// TECHDEBT(#830): it would be nice to have at least one more degree of freedom with which +// to limit logging in areas where it is known to be excessive / high frequency. +// Especially applicable to debug log lines which only contribute in edge cases, +// unusual circumstances, or regressions (e.g. hitting OS resource limits because +// of too many concurrent streams). +// +// This could ultimately be actuated from the CLI via flags, configs, and/or env +// vars. Initially, weo could consider coupling to a `--verbose` persistent flag. +// + +// logStream logs the incoming stream and its scope stats +func (rtr *UnicastRouter) logStream(stream libp2pNetwork.Stream) { + rtr.logStreamScopeStats(stream) + + remotePeer, err := utils.PeerFromLibp2pStream(stream) + if err != nil { + rtr.logger.Debug().Err(err).Msg("getting remote remotePeer") + } else { + utils.LogIncomingMsg(rtr.logger, rtr.getHostname(), remotePeer) + } +} + +// logStreamScopeStats logs the incoming stream's scope stats +// (see: https://pkg.go.dev/github.com/libp2p/go-libp2p@v0.27.0/core/network#StreamScope) +func (rtr *UnicastRouter) logStreamScopeStats(stream libp2pNetwork.Stream) { + if err := utils.LogScopeStatFactory( + &logger.Global.Logger, + "stream scope (read-side)", + )(stream.Scope()); err != nil { + rtr.logger.Debug().Err(err).Msg("logging stream scope stats") + } +} +func (rtr *UnicastRouter) getHostname() string { + return rtr.GetBus().GetRuntimeMgr().GetConfig().P2P.Hostname +} diff --git a/p2p/unicast/router.go b/p2p/unicast/router.go new file mode 100644 index 000000000..fcb8112d8 --- /dev/null +++ b/p2p/unicast/router.go @@ -0,0 +1,139 @@ +package unicast + +import ( + "io" + "time" + + libp2pHost "github.com/libp2p/go-libp2p/core/host" + libp2pNetwork "github.com/libp2p/go-libp2p/core/network" + + "github.com/pokt-network/pocket/p2p/config" + typesP2P "github.com/pokt-network/pocket/p2p/types" + "github.com/pokt-network/pocket/p2p/utils" + "github.com/pokt-network/pocket/shared/modules" + "github.com/pokt-network/pocket/shared/modules/base_modules" +) + +// TECHDEBT(#629): configure timeouts. Consider security exposure vs. real-world conditions. +// TECHDEBT(#629): parameterize and expose via config. +// readStreamTimeout is the duration to wait for a read operation on a +// stream to complete, after which the stream is closed ("timed out"). +const readStreamTimeout = time.Second * 10 + +var _ unicastRouterFactory = &UnicastRouter{} + +// TODO_THIS_COMMIT: consider defining/(re)using `RouterFactory` type +type unicastRouterFactory = modules.FactoryWithConfig[*UnicastRouter, *config.UnicastRouterConfig] + +type UnicastRouter struct { + base_modules.IntegratableModule + + logger *modules.Logger + // messageHandler is the function to call when a message is received. + // host represents a libp2p network node, it encapsulates a libp2p peerstore + // & connection manager. `libp2p.New` configures and starts listening + // according to options. + // (see: https://pkg.go.dev/github.com/libp2p/go-libp2p#section-readme) + host libp2pHost.Host + messageHandler typesP2P.MessageHandler + // TODO_THIS_COMMIT: consider defining alongside `MessageHandler` type + // OR removing `MessageHandler` type + peerHandler func(peer typesP2P.Peer) error +} + +func Create(bus modules.Bus, cfg *config.UnicastRouterConfig) (*UnicastRouter, error) { + return new(UnicastRouter).Create(bus, cfg) +} + +func (*UnicastRouter) Create(bus modules.Bus, cfg *config.UnicastRouterConfig) (*UnicastRouter, error) { + if err := cfg.IsValid(); err != nil { + return nil, err + } + + rtr := &UnicastRouter{ + logger: cfg.Logger, + host: cfg.Host, + messageHandler: cfg.MessageHandler, + peerHandler: cfg.PeerHandler, + } + rtr.SetBus(bus) + + // Don't handle incoming streams in client debug mode. + if !rtr.isClientDebugMode() { + rtr.host.SetStreamHandler(cfg.ProtocolID, rtr.handleStream) + } + + return rtr, nil +} + +// handleStream ensures the peerstore contains the remote peer and then reads +// the incoming stream in a new go routine. +func (rtr *UnicastRouter) handleStream(stream libp2pNetwork.Stream) { + rtr.logger.Debug().Msg("handling incoming stream") + peer, err := utils.PeerFromLibp2pStream(stream) + if err != nil { + rtr.logger.Error().Err(err). + Str("address", peer.GetAddress().String()). + Msg("parsing remote peer identity") + + if err = stream.Reset(); err != nil { + rtr.logger.Error().Err(err).Msg("resetting stream") + } + return + } + + if err := rtr.peerHandler(peer); err != nil { + rtr.logger.Error().Err(err). + Str("address", peer.GetAddress().String()). + Msg("adding remote peer to router") + } + + go rtr.readStream(stream) +} + +// readStream reads the message bytes out of the incoming stream and passes it to +// the configured `rtr.messageHandler`. Intended to be called in a go routine. +func (rtr *UnicastRouter) readStream(stream libp2pNetwork.Stream) { + // Time out if no data is sent to free resources. + // NB: tests using libp2p's `mocknet` rely on this not returning an error. + if err := stream.SetReadDeadline(newReadStreamDeadline()); err != nil { + // `SetReadDeadline` not supported by `mocknet` streams. + rtr.logger.Error().Err(err).Msg("setting stream read deadline") + } + + // log incoming stream + rtr.logStream(stream) + + // read stream + messageBz, err := io.ReadAll(stream) + if err != nil { + rtr.logger.Error().Err(err).Msg("reading from stream") + if err := stream.Reset(); err != nil { + rtr.logger.Error().Err(err).Msg("resetting stream (read-side)") + } + return + } + + // done reading; reset to signal this to remote peer + // NB: failing to reset the stream can easily max out the number of available + // network connections on the receiver's side. + if err := stream.Reset(); err != nil { + rtr.logger.Error().Err(err).Msg("resetting stream (read-side)") + } + + if err := rtr.messageHandler(messageBz); err != nil { + rtr.logger.Error().Err(err).Msg("handling message") + return + } +} + +// isClientDebugMode returns the value of `ClientDebugMode` in the base config +func (rtr *UnicastRouter) isClientDebugMode() bool { + return rtr.GetBus().GetRuntimeMgr().GetConfig().ClientDebugMode +} + +// newReadStreamDeadline returns a future deadline +// based on the read stream timeout duration. +func newReadStreamDeadline() time.Time { + return time.Now().Add(readStreamTimeout) +} diff --git a/p2p/unicast/testutil.go b/p2p/unicast/testutil.go new file mode 100644 index 000000000..8a3f9caf3 --- /dev/null +++ b/p2p/unicast/testutil.go @@ -0,0 +1,10 @@ +//go:build test + +package unicast + +import libp2pNetwork "github.com/libp2p/go-libp2p/core/network" + +// HandleStream exports `unicastRouter#handleStream` for testing purposes. +func (rtr *UnicastRouter) HandleStream(stream libp2pNetwork.Stream) { + rtr.handleStream(stream) +} From 1277859b624ac503a5193826b5bdb111d5a65235 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 17:38:06 +0200 Subject: [PATCH 02/59] chore: cleanup TODOs --- p2p/unicast/router.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/p2p/unicast/router.go b/p2p/unicast/router.go index fcb8112d8..bec52b36b 100644 --- a/p2p/unicast/router.go +++ b/p2p/unicast/router.go @@ -22,7 +22,6 @@ const readStreamTimeout = time.Second * 10 var _ unicastRouterFactory = &UnicastRouter{} -// TODO_THIS_COMMIT: consider defining/(re)using `RouterFactory` type type unicastRouterFactory = modules.FactoryWithConfig[*UnicastRouter, *config.UnicastRouterConfig] type UnicastRouter struct { @@ -36,8 +35,6 @@ type UnicastRouter struct { // (see: https://pkg.go.dev/github.com/libp2p/go-libp2p#section-readme) host libp2pHost.Host messageHandler typesP2P.MessageHandler - // TODO_THIS_COMMIT: consider defining alongside `MessageHandler` type - // OR removing `MessageHandler` type peerHandler func(peer typesP2P.Peer) error } From 871af48daf7692290bd76af6f3457d1bb197a660 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Fri, 16 Jun 2023 09:49:19 +0200 Subject: [PATCH 03/59] chore: add background message --- Makefile | 2 +- p2p/types/proto/background.proto | 8 ++++++++ p2p/{raintree => }/types/proto/raintree.proto | 0 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 p2p/types/proto/background.proto rename p2p/{raintree => }/types/proto/raintree.proto (100%) diff --git a/Makefile b/Makefile index fff096aef..6a0947014 100644 --- a/Makefile +++ b/Makefile @@ -297,7 +297,7 @@ protogen_local: go_protoc-go-inject-tag ## Generate go structures for all of the $(PROTOC_SHARED) -I=./consensus/types/proto --go_out=./consensus/types ./consensus/types/proto/*.proto # P2P - $(PROTOC_SHARED) -I=./p2p/raintree/types/proto --go_out=./p2p/types ./p2p/raintree/types/proto/*.proto + $(PROTOC_SHARED) -I=./p2p/types/proto --go_out=./p2p/types ./p2p/types/proto/*.proto # echo "View generated proto files by running: make protogen_show" diff --git a/p2p/types/proto/background.proto b/p2p/types/proto/background.proto new file mode 100644 index 000000000..d5046ff71 --- /dev/null +++ b/p2p/types/proto/background.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; +package background; + +option go_package = "github.com/pokt-network/pocket/p2p/types"; + +message BackgroundMessage { + bytes data = 1; +} diff --git a/p2p/raintree/types/proto/raintree.proto b/p2p/types/proto/raintree.proto similarity index 100% rename from p2p/raintree/types/proto/raintree.proto rename to p2p/types/proto/raintree.proto From 013c433c0e0cda79d16c87ce7975ce1e1ce53563 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 12:50:04 +0200 Subject: [PATCH 04/59] chore: add `Router#Close()` --- p2p/raintree/router.go | 4 ++++ p2p/types/router.go | 1 + 2 files changed, 5 insertions(+) diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index a68fd293d..5062d052c 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -79,6 +79,10 @@ func (*rainTreeRouter) Create(bus modules.Bus, cfg *config.RainTreeConfig) (type return typesP2P.Router(rtr), nil } +func (rtr *rainTreeRouter) Close() error { + return nil +} + // NetworkBroadcast implements the respective member of `typesP2P.Router`. func (rtr *rainTreeRouter) Broadcast(data []byte) error { return rtr.broadcastAtLevel(data, rtr.peersManager.GetMaxNumLevels()) diff --git a/p2p/types/router.go b/p2p/types/router.go index 21a2b8010..42ea194f1 100644 --- a/p2p/types/router.go +++ b/p2p/types/router.go @@ -14,6 +14,7 @@ type Router interface { Broadcast(data []byte) error Send(data []byte, address cryptoPocket.Address) error + Close() error // Address book helpers // TECHDEBT: simplify - remove `GetPeerstore` From 4f998eeae3a679d72552bbe088905f7c6e9d2358 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 12:50:35 +0200 Subject: [PATCH 05/59] chore: separate raintree & bg protocol IDs --- p2p/protocol/protocol.go | 14 ++++++++++---- p2p/raintree/router.go | 5 ++--- p2p/utils/host.go | 9 +++++---- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/p2p/protocol/protocol.go b/p2p/protocol/protocol.go index 81737e9a8..df06603b8 100644 --- a/p2p/protocol/protocol.go +++ b/p2p/protocol/protocol.go @@ -3,10 +3,16 @@ package protocol import "github.com/libp2p/go-libp2p/core/protocol" const ( - // PoktProtocolID is the libp2p protocol ID used when opening a new stream - // to a remote peer and setting the stream handler for the local peer. - // Libp2p APIs use this to distinguish which multiplexed protocols/streams to consider. - PoktProtocolID = protocol.ID("pokt/v1.0.0") + // RaintreeProtocolID is the libp2p protocol ID used in the Raintree router + // when opening a new stream to a remote peer and setting the stream handler + // for the local peer. Libp2p APIs use this to distinguish which multiplexed + // protocols/streams to consider. + RaintreeProtocolID = protocol.ID("pokt/raintree/v1.0.0") + // BackgroundProtocolID is the libp2p protocol ID used in the Background router + // when opening a new stream to a remote peer and setting the stream handler + // for the local peer. Libp2p APIs use this to distinguish which multiplexed + // protocols/streams to consider. + BackgroundProtocolID = protocol.ID("pokt/background/v1.0.0") // BackgroundTopicStr is a "default" pubsub topic string used when // subscribing and broadcasting. BackgroundTopicStr = "pokt/background" diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index 5062d052c..cf426798d 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -163,8 +163,7 @@ func (rtr *rainTreeRouter) sendInternal(data []byte, address cryptoPocket.Addres hostname := rtr.getHostname() utils.LogOutgoingMsg(rtr.logger, hostname, peer) - if err := utils.Libp2pSendToPeer(rtr.host, data, peer); err != nil { - rtr.logger.Debug().Err(err).Msg("from libp2pSendInternal") + if err := utils.Libp2pSendToPeer(rtr.host, protocol.RaintreeProtocolID, data, peer); err != nil { return err } @@ -287,7 +286,7 @@ func (rtr *rainTreeRouter) setupUnicastRouter() error { unicastRouterCfg := config.UnicastRouterConfig{ Logger: rtr.logger, Host: rtr.host, - ProtocolID: protocol.PoktProtocolID, + ProtocolID: protocol.RaintreeProtocolID, MessageHandler: rtr.handleRainTreeMsg, PeerHandler: rtr.AddPeer, } diff --git a/p2p/utils/host.go b/p2p/utils/host.go index e9c6e130b..3597856b7 100644 --- a/p2p/utils/host.go +++ b/p2p/utils/host.go @@ -6,10 +6,11 @@ import ( "time" libp2pHost "github.com/libp2p/go-libp2p/core/host" + libp2pProtocol "github.com/libp2p/go-libp2p/core/protocol" + "go.uber.org/multierr" + "github.com/pokt-network/pocket/logger" - "github.com/pokt-network/pocket/p2p/protocol" typesP2P "github.com/pokt-network/pocket/p2p/types" - "go.uber.org/multierr" ) const ( @@ -76,7 +77,7 @@ func RemovePeerFromLibp2pHost(host libp2pHost.Host, peer typesP2P.Peer) error { } // Libp2pSendToPeer sends data to the given pocket peer from the given libp2p host. -func Libp2pSendToPeer(host libp2pHost.Host, data []byte, peer typesP2P.Peer) error { +func Libp2pSendToPeer(host libp2pHost.Host, protocolID libp2pProtocol.ID, data []byte, peer typesP2P.Peer) error { // TECHDEBT(#595): add ctx to interface methods and propagate down. ctx := context.TODO() @@ -94,7 +95,7 @@ func Libp2pSendToPeer(host libp2pHost.Host, data []byte, peer typesP2P.Peer) err logger.Global.Debug().Err(err).Msg("logging resource scope stats") } - stream, err := host.NewStream(ctx, peerInfo.ID, protocol.PoktProtocolID) + stream, err := host.NewStream(ctx, peerInfo.ID, protocolID) if err != nil { return fmt.Errorf("opening stream: %w", err) } From 1c9c18ce2f77200fbcec047b093dd4b8766f38f5 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 12:51:35 +0200 Subject: [PATCH 06/59] chore: generate `PocketEnvelope` nonce in `PackMessage()` --- shared/messaging/envelope.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/shared/messaging/envelope.go b/shared/messaging/envelope.go index 4150fbfba..f65304007 100644 --- a/shared/messaging/envelope.go +++ b/shared/messaging/envelope.go @@ -4,6 +4,8 @@ import ( "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/known/anypb" + + cryptoPocket "github.com/pokt-network/pocket/shared/crypto" ) // PackMessage returns a *PocketEnvelope after having packed the message supplied as an argument @@ -12,7 +14,10 @@ func PackMessage(message proto.Message) (*PocketEnvelope, error) { if err != nil { return nil, err } - return &PocketEnvelope{Content: anyMsg}, nil + return &PocketEnvelope{ + Content: anyMsg, + Nonce: cryptoPocket.GetNonce(), + }, nil } // UnpackMessage extracts the message inside the PocketEnvelope decorating it with typing information From f9a0c10d48754ab8cead066e2e92201d5b04f563 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 12:52:24 +0200 Subject: [PATCH 07/59] refactor: add `Handler` to router config validation --- p2p/config/config.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/p2p/config/config.go b/p2p/config/config.go index d98f4b969..60361f662 100644 --- a/p2p/config/config.go +++ b/p2p/config/config.go @@ -16,6 +16,7 @@ import ( var ( _ typesP2P.RouterConfig = &baseConfig{} _ typesP2P.RouterConfig = &UnicastRouterConfig{} + _ typesP2P.RouterConfig = &BackgroundConfig{} _ typesP2P.RouterConfig = &RainTreeConfig{} ) @@ -30,6 +31,7 @@ type baseConfig struct { Addr crypto.Address CurrentHeightProvider providers.CurrentHeightProvider PeerstoreProvider providers.PeerstoreProvider + Handler func(data []byte) error } type UnicastRouterConfig struct { @@ -75,7 +77,11 @@ func (cfg *baseConfig) IsValid() (err error) { if cfg.PeerstoreProvider == nil { err = multierr.Append(err, fmt.Errorf("peerstore provider not configured")) } - return nil + + if cfg.Handler == nil { + err = multierr.Append(err, fmt.Errorf("handler not configured")) + } + return err } // IsValid implements the respective member of the `RouterConfig` interface. @@ -103,23 +109,25 @@ func (cfg *UnicastRouterConfig) IsValid() (err error) { } // IsValid implements the respective member of the `RouterConfig` interface. -func (cfg *BackgroundConfig) IsValid() (err error) { +func (cfg *BackgroundConfig) IsValid() error { baseCfg := baseConfig{ Host: cfg.Host, Addr: cfg.Addr, CurrentHeightProvider: cfg.CurrentHeightProvider, PeerstoreProvider: cfg.PeerstoreProvider, + Handler: cfg.Handler, } - return multierr.Append(err, baseCfg.IsValid()) + return baseCfg.IsValid() } // IsValid implements the respective member of the `RouterConfig` interface. -func (cfg *RainTreeConfig) IsValid() (err error) { +func (cfg *RainTreeConfig) IsValid() error { baseCfg := baseConfig{ Host: cfg.Host, Addr: cfg.Addr, CurrentHeightProvider: cfg.CurrentHeightProvider, PeerstoreProvider: cfg.PeerstoreProvider, + Handler: cfg.Handler, } - return multierr.Append(err, baseCfg.IsValid()) + return baseCfg.IsValid() } From 0bdffbd210a5e49992d0359933dd4f6c7badb60c Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 12:54:09 +0200 Subject: [PATCH 08/59] refactor: raintree router --- p2p/raintree/router.go | 21 +++++++++++++-------- p2p/types/errors.go | 9 ++++++++- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index cf426798d..f86520446 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -55,7 +55,7 @@ func NewRainTreeRouter(bus modules.Bus, cfg *config.RainTreeConfig) (typesP2P.Ro } func (*rainTreeRouter) Create(bus modules.Bus, cfg *config.RainTreeConfig) (typesP2P.Router, error) { - routerLogger := logger.Global.CreateLoggerForModule("router") + routerLogger := logger.Global.CreateLoggerForModule("rainTreeRouter") routerLogger.Info().Msg("Initializing rainTreeRouter") if err := cfg.IsValid(); err != nil { @@ -156,7 +156,7 @@ func (rtr *rainTreeRouter) sendInternal(data []byte, address cryptoPocket.Addres peer := rtr.peersManager.GetPeerstore().GetPeer(address) if peer == nil { - return fmt.Errorf("no known peer with pokt address %s", address) + return fmt.Errorf("%w: with pokt address %s", typesP2P.ErrUnknownPeer, address) } // debug logging @@ -204,16 +204,14 @@ func (rtr *rainTreeRouter) handleRainTreeMsg(rainTreeMsgBz []byte) error { var rainTreeMsg typesP2P.RainTreeMessage if err := proto.Unmarshal(rainTreeMsgBz, &rainTreeMsg); err != nil { + // TECHDEBT: add telemetry return err } - // TECHDEBT(#763): refactor as "pre-propagation validation" - networkMessage := messaging.PocketEnvelope{} - if err := proto.Unmarshal(rainTreeMsg.Data, &networkMessage); err != nil { - rtr.logger.Error().Err(err).Msg("Error decoding network message") - return err + if err := rtr.validateRainTreeMsg(&rainTreeMsg); err != nil { + // TECHDEBT: add telemetry + return fmt.Errorf("validating raintree message: %w", err) } - // -- // Continue RainTree propagation if rainTreeMsg.Level > 0 { @@ -235,6 +233,13 @@ func (rtr *rainTreeRouter) handleRainTreeMsg(rainTreeMsgBz []byte) error { return nil } +// validateRainTreeMsg ensures that the `data` contained within the RainTree message +// is a valid `PocketEnvelope` by attempting to deserialize it. +func (rtr *rainTreeRouter) validateRainTreeMsg(rainTreeMsg *typesP2P.RainTreeMessage) error { + networkMessage := messaging.PocketEnvelope{} + return proto.Unmarshal(rainTreeMsg.Data, &networkMessage) +} + // GetPeerstore implements the respective member of `typesP2P.Router`. func (rtr *rainTreeRouter) GetPeerstore() typesP2P.Peerstore { return rtr.peersManager.GetPeerstore() diff --git a/p2p/types/errors.go b/p2p/types/errors.go index 0688e5757..632bdd534 100644 --- a/p2p/types/errors.go +++ b/p2p/types/errors.go @@ -1,6 +1,13 @@ package types -import "fmt" +import ( + "errors" + "fmt" +) + +var ( + ErrUnknownPeer = errors.New("unknown peer") +) func ErrUnknownEventType(msg any) error { return fmt.Errorf("unknown event type: %v", msg) From 2b407767acc7328fbe5107102cf1d4946ffffb28 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 12:54:58 +0200 Subject: [PATCH 09/59] refactor: background router --- p2p/background/router.go | 341 ++++++++++++++++++++++++++++------ p2p/background/router_test.go | 2 +- p2p/protocol/protocol.go | 4 - 3 files changed, 288 insertions(+), 59 deletions(-) diff --git a/p2p/background/router.go b/p2p/background/router.go index 199190b21..1ee3dc845 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -9,12 +9,19 @@ import ( dht "github.com/libp2p/go-libp2p-kad-dht" pubsub "github.com/libp2p/go-libp2p-pubsub" libp2pHost "github.com/libp2p/go-libp2p/core/host" + libp2pPeer "github.com/libp2p/go-libp2p/core/peer" + "go.uber.org/multierr" + "google.golang.org/protobuf/proto" + "github.com/pokt-network/pocket/logger" "github.com/pokt-network/pocket/p2p/config" "github.com/pokt-network/pocket/p2p/protocol" + "github.com/pokt-network/pocket/p2p/providers" typesP2P "github.com/pokt-network/pocket/p2p/types" + "github.com/pokt-network/pocket/p2p/unicast" "github.com/pokt-network/pocket/p2p/utils" cryptoPocket "github.com/pokt-network/pocket/shared/crypto" + "github.com/pokt-network/pocket/shared/messaging" "github.com/pokt-network/pocket/shared/modules" "github.com/pokt-network/pocket/shared/modules/base_modules" ) @@ -22,18 +29,30 @@ import ( var ( _ typesP2P.Router = &backgroundRouter{} _ modules.IntegratableModule = &backgroundRouter{} + _ backgroundRouterFactory = &backgroundRouter{} ) +type backgroundRouterFactory = modules.FactoryWithConfig[typesP2P.Router, *config.BackgroundConfig] + // backgroundRouter implements `typesP2P.Router` for use with all P2P participants. type backgroundRouter struct { base_modules.IntegratableModule + unicast.UnicastRouter logger *modules.Logger + // handler is the function to call when a message is received. + handler typesP2P.MessageHandler // host represents a libp2p network node, it encapsulates a libp2p peerstore // & connection manager. `libp2p.New` configures and starts listening // according to options. // (see: https://pkg.go.dev/github.com/libp2p/go-libp2p#section-readme) host libp2pHost.Host + // cancelCtx is the cancel function for the context which is provided to the + // gossipsub subscription. Used to terminate the `readSubscription()` go routine. + cancelCtx context.CancelFunc + + // Fields below are assigned during creation via `#setupDependencies()`. + // gossipSub is used for broadcast communication // (i.e. multiple, unidentified receivers) // TECHDEBT: investigate diff between randomSub and gossipSub @@ -47,96 +66,103 @@ type backgroundRouter struct { kadDHT *dht.IpfsDHT // TECHDEBT: `pstore` will likely be removed in future refactoring / simplification // of the `Router` interface. - // pstore is the background router's peerstore. + // pstore is the background router's peerstore. Assigned in `backgroundRouter#setupPeerstore()`. pstore typesP2P.Peerstore } -// NewBackgroundRouter returns a `backgroundRouter` as a `typesP2P.Router` +// Create returns a `backgroundRouter` as a `typesP2P.Router` // interface using the given configuration. -func NewBackgroundRouter(bus modules.Bus, cfg *config.BackgroundConfig) (typesP2P.Router, error) { - // TECHDEBT(#595): add ctx to interface methods and propagate down. - ctx := context.TODO() +func Create(bus modules.Bus, cfg *config.BackgroundConfig) (typesP2P.Router, error) { + return new(backgroundRouter).Create(bus, cfg) +} +func (*backgroundRouter) Create(bus modules.Bus, cfg *config.BackgroundConfig) (typesP2P.Router, error) { networkLogger := logger.Global.CreateLoggerForModule("backgroundRouter") - networkLogger.Info().Msg("Initializing background router") + networkLogger.Info().Msg("initializing background router") - // seed initial peerstore with current on-chain peer info (i.e. staked actors) - pstore, err := cfg.PeerstoreProvider.GetStakedPeerstoreAtHeight( - cfg.CurrentHeightProvider.CurrentHeight(), - ) - if err != nil { + if err := cfg.IsValid(); err != nil { return nil, err } - // CONSIDERATION: If switching to `NewRandomSub`, there will be a max size - gossipSub, err := pubsub.NewGossipSub(ctx, cfg.Host) - if err != nil { - return nil, fmt.Errorf("creating gossip pubsub: %w", err) - } + // TECHDEBT(#595): add ctx to interface methods and propagate down. + ctx, cancel := context.WithCancel(context.TODO()) - dhtMode := dht.ModeAutoServer - // NB: don't act as a bootstrap node in peer discovery in client debug mode - if isClientDebugMode(bus) { - dhtMode = dht.ModeClient + rtr := &backgroundRouter{ + logger: networkLogger, + handler: cfg.Handler, + host: cfg.Host, + cancelCtx: cancel, } + rtr.SetBus(bus) - kadDHT, err := dht.New(ctx, cfg.Host, dht.Mode(dhtMode)) - if err != nil { - return nil, fmt.Errorf("creating DHT: %w", err) + if err := rtr.setupDependencies(ctx, cfg); err != nil { + return nil, err } - topic, err := gossipSub.Join(protocol.BackgroundTopicStr) - if err != nil { - return nil, fmt.Errorf("joining background topic: %w", err) - } + go rtr.readSubscription(ctx) - // INVESTIGATE: `WithBufferSize` `SubOpt`: - // > WithBufferSize is a Subscribe option to customize the size of the subscribe - // > output buffer. The default length is 32 but it can be configured to avoid - // > dropping messages if the consumer is not reading fast enough. - // (see: https://pkg.go.dev/github.com/libp2p/go-libp2p-pubsub#WithBufferSize) - subscription, err := topic.Subscribe() - if err != nil { - return nil, fmt.Errorf("subscribing to background topic: %w", err) - } + return rtr, nil +} - rtr := &backgroundRouter{ - host: cfg.Host, - gossipSub: gossipSub, - kadDHT: kadDHT, - topic: topic, - subscription: subscription, - logger: networkLogger, - pstore: pstore, +func (rtr *backgroundRouter) Close() error { + rtr.logger.Debug().Msg("closing background router") + + rtr.cancelCtx() + rtr.subscription.Cancel() + + var topicCloseErr error + if err := rtr.topic.Close(); err != context.Canceled { + topicCloseErr = err } - return rtr, nil + return multierr.Append( + topicCloseErr, + rtr.kadDHT.Close(), + ) } // Broadcast implements the respective `typesP2P.Router` interface method. -func (rtr *backgroundRouter) Broadcast(data []byte) error { +func (rtr *backgroundRouter) Broadcast(pocketEnvelopeBz []byte) error { + backgroundMsg := &typesP2P.BackgroundMessage{ + Data: pocketEnvelopeBz, + } + backgroundMsgBz, err := proto.Marshal(backgroundMsg) + if err != nil { + return err + } + // TECHDEBT(#595): add ctx to interface methods and propagate down. - return rtr.topic.Publish(context.TODO(), data) + return rtr.topic.Publish(context.TODO(), backgroundMsgBz) } // Send implements the respective `typesP2P.Router` interface method. -func (rtr *backgroundRouter) Send(data []byte, address cryptoPocket.Address) error { +func (rtr *backgroundRouter) Send(pocketEnvelopeBz []byte, address cryptoPocket.Address) error { + rtr.logger.Warn().Str("address", address.String()).Msg("sending background message to peer") + + backgroundMessage := &typesP2P.BackgroundMessage{ + Data: pocketEnvelopeBz, + } + backgroundMessageBz, err := proto.Marshal(backgroundMessage) + if err != nil { + return fmt.Errorf("marshalling background message: %w", err) + } + peer := rtr.pstore.GetPeer(address) if peer == nil { return fmt.Errorf("peer with address %s not in peerstore", address) } - if err := utils.Libp2pSendToPeer(rtr.host, data, peer); err != nil { + if err := utils.Libp2pSendToPeer( + rtr.host, + protocol.BackgroundProtocolID, + backgroundMessageBz, + peer, + ); err != nil { return err } return nil } -// HandleNetworkData implements the respective `typesP2P.Router` interface method. -func (rtr *backgroundRouter) HandleNetworkData(data []byte) ([]byte, error) { - return data, nil // intentional passthrough -} - // GetPeerstore implements the respective `typesP2P.Router` interface method. func (rtr *backgroundRouter) GetPeerstore() typesP2P.Peerstore { return rtr.pstore @@ -166,6 +192,213 @@ func (rtr *backgroundRouter) RemovePeer(peer typesP2P.Peer) error { return rtr.pstore.RemovePeer(peer.GetAddress()) } +func (rtr *backgroundRouter) setupUnicastRouter() error { + unicastRouterCfg := config.UnicastRouterConfig{ + Logger: rtr.logger, + Host: rtr.host, + ProtocolID: protocol.BackgroundProtocolID, + MessageHandler: rtr.handleBackgroundMsg, + PeerHandler: rtr.AddPeer, + } + + unicastRouter, err := unicast.Create(rtr.GetBus(), &unicastRouterCfg) + if err != nil { + return fmt.Errorf("setting up unicast router: %w", err) + } + + rtr.UnicastRouter = *unicastRouter + return nil +} + +func (rtr *backgroundRouter) setupDependencies(ctx context.Context, cfg *config.BackgroundConfig) error { + if err := rtr.setupUnicastRouter(); err != nil { + return err + } + + if err := rtr.setupPeerDiscovery(ctx); err != nil { + return fmt.Errorf("setting up peer discovery: %w", err) + } + + if err := rtr.setupPubsub(ctx); err != nil { + return fmt.Errorf("setting up pubsub: %w", err) + } + + if err := rtr.setupTopic(); err != nil { + return fmt.Errorf("setting up topic: %w", err) + } + + if err := rtr.setupSubscription(); err != nil { + return fmt.Errorf("setting up subscription: %w", err) + } + + if err := rtr.setupPeerstore( + ctx, + cfg.PeerstoreProvider, + cfg.CurrentHeightProvider, + ); err != nil { + return fmt.Errorf("setting up peerstore: %w", err) + } + return nil +} + +func (rtr *backgroundRouter) setupPeerstore( + ctx context.Context, + pstoreProvider providers.PeerstoreProvider, + currentHeightProvider providers.CurrentHeightProvider, +) (err error) { + // seed initial peerstore with current on-chain peer info (i.e. staked actors) + rtr.pstore, err = pstoreProvider.GetStakedPeerstoreAtHeight( + currentHeightProvider.CurrentHeight(), + ) + if err != nil { + return err + } + + // CONSIDERATION: add `GetPeers` method to `PeerstoreProvider` interface + // to avoid this loop. + for _, peer := range rtr.pstore.GetPeerList() { + if err := utils.AddPeerToLibp2pHost(rtr.host, peer); err != nil { + return err + } + + // TODO: refactor: #bootstrap() + libp2pAddrInfo, err := utils.Libp2pAddrInfoFromPeer(peer) + if err != nil { + return fmt.Errorf( + "converting peer info, pokt address: %s: %w", + peer.GetAddress(), + err, + ) + } + + // don't attempt to connect to self + if rtr.host.ID() == libp2pAddrInfo.ID { + return nil + } + + if err := rtr.host.Connect(ctx, libp2pAddrInfo); err != nil { + return fmt.Errorf("connecting to peer: %w", err) + } + } + return nil +} + +func (rtr *backgroundRouter) setupPeerDiscovery(ctx context.Context) (err error) { + dhtMode := dht.ModeAutoServer + // NB: don't act as a bootstrap node in peer discovery in client debug mode + if isClientDebugMode(rtr.GetBus()) { + dhtMode = dht.ModeClient + } + + rtr.kadDHT, err = dht.New(ctx, rtr.host, dht.Mode(dhtMode)) + return err +} + +func (rtr *backgroundRouter) setupPubsub(ctx context.Context) (err error) { + // TECHDEBT(#730): integrate libp2p tracing via `pubsub.WithEventTracer()`. + + // CONSIDERATION: If switching to `NewRandomSub`, there will be a max size + rtr.gossipSub, err = pubsub.NewGossipSub(ctx, rtr.host) + return err +} + +func (rtr *backgroundRouter) setupTopic() (err error) { + if err := rtr.gossipSub.RegisterTopicValidator( + protocol.BackgroundTopicStr, + rtr.topicValidator, + ); err != nil { + return fmt.Errorf( + "registering topic validator for topic: %q: %w", + protocol.BackgroundTopicStr, err, + ) + } + + if rtr.topic, err = rtr.gossipSub.Join(protocol.BackgroundTopicStr); err != nil { + return fmt.Errorf( + "joining background topic: %q: %w", + protocol.BackgroundTopicStr, err, + ) + } + return nil +} + +func (rtr *backgroundRouter) setupSubscription() (err error) { + // INVESTIGATE: `WithBufferSize` `SubOpt`: + // > WithBufferSize is a Subscribe option to customize the size of the subscribe + // > output buffer. The default length is 32 but it can be configured to avoid + // > dropping messages if the consumer is not reading fast enough. + // (see: https://pkg.go.dev/github.com/libp2p/go-libp2p-pubsub#WithBufferSize) + rtr.subscription, err = rtr.topic.Subscribe() + return err +} + +// topicValidator is used in conjunction with libp2p-pubsub's notion of "topic +// validaton". It is usefed for arbitrary and concurrent pre-propagation validation +// of messages. +// +// (see: https://github.com/libp2p/specs/tree/master/pubsub#topic-validation +// and https://pkg.go.dev/github.com/libp2p/go-libp2p-pubsub#PubSub.RegisterTopicValidator) +// +// Also note: https://pkg.go.dev/github.com/libp2p/go-libp2p-pubsub#BasicSeqnoValidator +func (rtr *backgroundRouter) topicValidator(_ context.Context, _ libp2pPeer.ID, msg *pubsub.Message) bool { + var backgroundMsg typesP2P.BackgroundMessage + if err := proto.Unmarshal(msg.Data, &backgroundMsg); err != nil { + return false + } + + if backgroundMsg.Data == nil { + return false + } + + networkMessage := messaging.PocketEnvelope{} + if err := proto.Unmarshal(backgroundMsg.Data, &networkMessage); err != nil { + rtr.logger.Error().Err(err).Msg("Error decoding network message") + return false + } + + return true +} + +func (rtr *backgroundRouter) readSubscription(ctx context.Context) { + for { + if err := ctx.Err(); err != nil { + if err != context.Canceled { + rtr.logger.Error().Err(err). + Msg("context error while reading subscription") + } + return + } + msg, err := rtr.subscription.Next(ctx) + + if err != nil { + rtr.logger.Error().Err(err). + Msg("error reading from background topic subscription") + continue + } + + // TECHDEBT/DISCUSS: telemetry + if err := rtr.handleBackgroundMsg(msg.Data); err != nil { + rtr.logger.Error().Err(err).Msg("error handling background message") + continue + } + } +} + +func (rtr *backgroundRouter) handleBackgroundMsg(backgroundMsgBz []byte) error { + var backgroundMsg typesP2P.BackgroundMessage + if err := proto.Unmarshal(backgroundMsgBz, &backgroundMsg); err != nil { + return err + } + + // There was no error, but we don't need to forward this to the app-specific bus. + // For example, the message has already been handled by the application. + if backgroundMsg.Data == nil { + return nil + } + + return rtr.handler(backgroundMsg.Data) +} + // isClientDebugMode returns the value of `ClientDebugMode` in the base config func isClientDebugMode(bus modules.Bus) bool { return bus.GetRuntimeMgr().GetConfig().ClientDebugMode diff --git a/p2p/background/router_test.go b/p2p/background/router_test.go index a1a0fe40b..450946f71 100644 --- a/p2p/background/router_test.go +++ b/p2p/background/router_test.go @@ -284,7 +284,7 @@ func newRouterWithSelfPeerAndHost(t *testing.T, selfPeer typesP2P.Peer, host lib err := pstore.AddPeer(selfPeer) require.NoError(t, err) - router, err := NewBackgroundRouter(busMock, &config.BackgroundConfig{ + router, err := Create(busMock, &config.BackgroundConfig{ Addr: selfPeer.GetAddress(), PeerstoreProvider: pstoreProviderMock, CurrentHeightProvider: consensusMock, diff --git a/p2p/protocol/protocol.go b/p2p/protocol/protocol.go index df06603b8..c4463bb45 100644 --- a/p2p/protocol/protocol.go +++ b/p2p/protocol/protocol.go @@ -16,8 +16,4 @@ const ( // BackgroundTopicStr is a "default" pubsub topic string used when // subscribing and broadcasting. BackgroundTopicStr = "pokt/background" - // PeerDiscoveryNamespace used by both advertiser and discoverer to rendezvous - // during peer discovery. Advertiser(s) and discoverer(s) MUST have matching - // discovery namespaces to find one another. - PeerDiscoveryNamespace = "pokt/peer_discovery" ) From 213f294c91e88053faf9dfda09b9e4c4df5c6614 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 12:55:11 +0200 Subject: [PATCH 10/59] refactor: integrate bg router --- p2p/bootstrap.go | 4 +- p2p/module.go | 188 ++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 173 insertions(+), 19 deletions(-) diff --git a/p2p/bootstrap.go b/p2p/bootstrap.go index 75198ee0e..68952d44d 100644 --- a/p2p/bootstrap.go +++ b/p2p/bootstrap.go @@ -76,14 +76,14 @@ func (m *p2pModule) bootstrap() error { for _, peer := range pstore.GetPeerList() { m.logger.Debug().Str("address", peer.GetAddress().String()).Msg("Adding peer to router") - if err := m.router.AddPeer(peer); err != nil { + if err := m.stakedActorRouter.AddPeer(peer); err != nil { m.logger.Error().Err(err). Str("pokt_address", peer.GetAddress().String()). Msg("adding peer") } } - if m.router.GetPeerstore().Size() == 0 { + if m.stakedActorRouter.GetPeerstore().Size() == 0 { return fmt.Errorf("bootstrap failed") } return nil diff --git a/p2p/module.go b/p2p/module.go index d9e164fc7..1c06e5f78 100644 --- a/p2p/module.go +++ b/p2p/module.go @@ -3,14 +3,17 @@ package p2p import ( "errors" "fmt" + "sync/atomic" "github.com/libp2p/go-libp2p" libp2pHost "github.com/libp2p/go-libp2p/core/host" "github.com/multiformats/go-multiaddr" + "go.uber.org/multierr" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/anypb" "github.com/pokt-network/pocket/logger" + "github.com/pokt-network/pocket/p2p/background" "github.com/pokt-network/pocket/p2p/config" "github.com/pokt-network/pocket/p2p/providers" "github.com/pokt-network/pocket/p2p/providers/current_height_provider" @@ -35,6 +38,7 @@ var _ modules.P2PModule = &p2pModule{} type p2pModule struct { base_modules.IntegratableModule + started atomic.Bool address cryptoPocket.Address logger *modules.Logger options []modules.ModuleOption @@ -49,8 +53,9 @@ type p2pModule struct { nonceDeduper *mempool.GenericFIFOSet[uint64, uint64] // Assigned during `#Start()`. TLDR; `host` listens on instantiation. - // and `router` depends on `host`. - router typesP2P.Router + // `stakedActorRouter` and `unstakedActorRouter` depends on `host`. + stakedActorRouter typesP2P.Router + unstakedActorRouter typesP2P.Router // host represents a libp2p network node, it encapsulates a libp2p peerstore // & connection manager. `libp2p.New` configures and starts listening // according to options. Assigned via `#Start()` (starts on instantiation). @@ -62,6 +67,7 @@ func Create(bus modules.Bus, options ...modules.ModuleOption) (modules.Module, e return new(p2pModule).Create(bus, options...) } +// TODO_THIS_COMMIT: rename (WithHost) & consider moving to testutil file // WithHostOption associates an existing (i.e. "started") libp2p `host.Host` // with this module, instead of creating a new one on `#Start()`. // Primarily intended for testing. @@ -137,8 +143,12 @@ func (m *p2pModule) GetModuleName() string { } // Start instantiates and assigns `m.host`, unless one already exists, and -// `m.router` (which depends on `m.host` as a required config field). +// `m.stakedActorRouter` (which depends on `m.host` as a required config field). func (m *p2pModule) Start() (err error) { + if !m.started.CompareAndSwap(false, true) { + return fmt.Errorf("p2p module already started") + } + m.GetBus(). GetTelemetryModule(). GetTimeSeriesAgent(). @@ -162,8 +172,8 @@ func (m *p2pModule) Start() (err error) { } } - if err := m.setupRouter(); err != nil { - return fmt.Errorf("setting up router: %w", err) + if err := m.setupRouters(); err != nil { + return fmt.Errorf("setting up routers: %w", err) } m.GetBus(). @@ -174,38 +184,106 @@ func (m *p2pModule) Start() (err error) { } func (m *p2pModule) Stop() error { - err := m.host.Close() + m.logger.Debug().Msg("stopping P2P module") + + if !m.started.CompareAndSwap(true, false) { + return fmt.Errorf("p2p module already stopped") + } + + var stakedActorRouterCloseErr error + if m.stakedActorRouter != nil { + stakedActorRouterCloseErr = m.stakedActorRouter.Close() + } + + routerCloseErrs := multierr.Append( + m.unstakedActorRouter.Close(), + stakedActorRouterCloseErr, + ) + + err := multierr.Append( + routerCloseErrs, + m.host.Close(), + ) // Don't reuse closed host, `#Start()` will re-create. m.host = nil + m.stakedActorRouter = nil + m.unstakedActorRouter = nil return err } func (m *p2pModule) Broadcast(msg *anypb.Any) error { - c := &messaging.PocketEnvelope{ + isStaked, err := m.isStakedActor() + if err != nil { + return err + } + + if isStaked { + if m.stakedActorRouter == nil { + return fmt.Errorf("staked actor router not started") + } + } + + if m.unstakedActorRouter == nil { + return fmt.Errorf("unstaked actor router not started") + } + + poktEnvelope := &messaging.PocketEnvelope{ Content: msg, Nonce: cryptoPocket.GetNonce(), } - data, err := codec.GetCodec().Marshal(c) + poktEnvelopeBz, err := codec.GetCodec().Marshal(poktEnvelope) if err != nil { return err } - return m.router.Broadcast(data) + var stakedBroadcastErr error + if isStaked { + stakedBroadcastErr = m.stakedActorRouter.Broadcast(poktEnvelopeBz) + } + + unstakedBroadcastErr := m.unstakedActorRouter.Broadcast(poktEnvelopeBz) + + return multierr.Append(stakedBroadcastErr, unstakedBroadcastErr) } func (m *p2pModule) Send(addr cryptoPocket.Address, msg *anypb.Any) error { - c := &messaging.PocketEnvelope{ + poktEnvelope := &messaging.PocketEnvelope{ Content: msg, Nonce: cryptoPocket.GetNonce(), } - data, err := codec.GetCodec().Marshal(c) + poktEnvelopeBz, err := codec.GetCodec().Marshal(poktEnvelope) + if err != nil { + return err + } + + unstakedSendErr := m.unstakedActorRouter.Send(poktEnvelopeBz, addr) + + isStaked, err := m.isStakedActor() if err != nil { return err } - return m.router.Send(data, addr) + var stakedSendErr error + if isStaked { + stakedSendErr = m.stakedActorRouter.Send(poktEnvelopeBz, addr) + } + + if errors.Is(unstakedSendErr, typesP2P.ErrUnknownPeer) && + errors.Is(stakedSendErr, typesP2P.ErrUnknownPeer) { + return typesP2P.ErrUnknownPeer + } + + if errors.Is(unstakedSendErr, typesP2P.ErrUnknownPeer) { + return stakedSendErr + } + + if errors.Is(stakedSendErr, typesP2P.ErrUnknownPeer) { + return unstakedSendErr + } + + return multierr.Append(unstakedSendErr, stakedSendErr) } // TECHDEBT(#348): Define what the node identity is throughout the codebase @@ -292,9 +370,35 @@ func (m *p2pModule) setupNonceDeduper() error { return nil } -// setupRouter instantiates the configured router implementation. -func (m *p2pModule) setupRouter() (err error) { - m.router, err = raintree.NewRainTreeRouter( +// setupRouters instantiates the configured router implementations. +func (m *p2pModule) setupRouters() (err error) { + if err := m.setupStakedRouter(); err != nil { + return err + } + + if err := m.setupUnstakedRouter(); err != nil { + return err + } + return nil +} + +// setupStakedRouter initializes the staked actor router ONLY IF this node is +// a staked actor, exclusively for use between staked actors. +func (m *p2pModule) setupStakedRouter() (err error) { + // `nstakedActorRouter` may already be initialized via a `ModuleOption`. + if m.stakedActorRouter != nil { + m.logger.Debug().Msg("staked actor router already initialized") + return nil + } + + if isStaked, err := m.isStakedActor(); err != nil { + return err + } else if !isStaked { + return nil + } + + m.logger.Debug().Msg("setting up staked actor router") + m.stakedActorRouter, err = raintree.NewRainTreeRouter( m.GetBus(), &config.RainTreeConfig{ Addr: m.address, @@ -304,10 +408,39 @@ func (m *p2pModule) setupRouter() (err error) { Handler: m.handlePocketEnvelope, }, ) - return err + if err != nil { + return fmt.Errorf("setting up staked actor router: %w", err) + } + return nil } -// setupHost creates a new libp2p host and assignes it to `m.host`. Libp2p host +// setupUnstakedRouter initializes the unstaked actor router for use with the +// entire P2P network. +func (m *p2pModule) setupUnstakedRouter() (err error) { + // `unstakedActorRouter` may already be initialized via a `ModuleOption`. + if m.unstakedActorRouter != nil { + m.logger.Debug().Msg("unstaked actor router already initialized") + return nil + } + + m.logger.Debug().Msg("setting up unstaked actor router") + m.unstakedActorRouter, err = background.Create( + m.GetBus(), + &config.BackgroundConfig{ + Addr: m.address, + CurrentHeightProvider: m.currentHeightProvider, + PeerstoreProvider: m.pstoreProvider, + Host: m.host, + Handler: m.handlePocketEnvelope, + }, + ) + if err != nil { + return fmt.Errorf("unstaked actor router: %w", err) + } + return nil +} + +// setupHost creates a new libp2p host and assigns it to `m.host`. Libp2p host // starts listening upon instantiation. func (m *p2pModule) setupHost() (err error) { m.logger.Debug().Msg("creating new libp2p host") @@ -422,3 +555,24 @@ func (m *p2pModule) getMultiaddr() (multiaddr.Multiaddr, error) { "%s:%d", m.cfg.Hostname, m.cfg.Port, )) } + +func (m *p2pModule) getStakedPeerstore() (typesP2P.Peerstore, error) { + return m.pstoreProvider.GetStakedPeerstoreAtHeight( + m.currentHeightProvider.CurrentHeight(), + ) +} + +// isStakedActor returns whether the current node is a staked actor at the current height. +// Return an error if a peerstore can't be provided. +func (m *p2pModule) isStakedActor() (bool, error) { + pstore, err := m.getStakedPeerstore() + if err != nil { + return false, fmt.Errorf("getting staked peerstore: %w", err) + } + + // Ensure self address is present in current height's staked actor set. + if self := pstore.GetPeer(m.address); self != nil { + return true, nil + } + return false, nil +} From 437afc8a2ed7796edc9973b9c7d5a73e9def303e Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 12:55:20 +0200 Subject: [PATCH 11/59] refactor: staked actor router peer discovery --- p2p/event_handler.go | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/p2p/event_handler.go b/p2p/event_handler.go index 48e1a7d73..792737268 100644 --- a/p2p/event_handler.go +++ b/p2p/event_handler.go @@ -3,10 +3,11 @@ package p2p import ( "fmt" + "google.golang.org/protobuf/types/known/anypb" + "github.com/pokt-network/pocket/shared/codec" coreTypes "github.com/pokt-network/pocket/shared/core/types" "github.com/pokt-network/pocket/shared/messaging" - "google.golang.org/protobuf/types/known/anypb" ) // CONSIDERATION(#576): making this part of some new `ConnManager`. @@ -23,21 +24,25 @@ func (m *p2pModule) HandleEvent(event *anypb.Any) error { return fmt.Errorf("failed to cast event to ConsensusNewHeightEvent") } - oldPeerList := m.router.GetPeerstore().GetPeerList() - updatedPeerstore, err := m.pstoreProvider.GetStakedPeerstoreAtHeight(consensusNewHeightEvent.Height) - if err != nil { + if isStaked, err := m.isStakedActor(); err != nil { return err - } - - added, removed := oldPeerList.Delta(updatedPeerstore.GetPeerList()) - for _, add := range added { - if err := m.router.AddPeer(add); err != nil { + } else if isStaked { + oldPeerList := m.stakedActorRouter.GetPeerstore().GetPeerList() + updatedPeerstore, err := m.pstoreProvider.GetStakedPeerstoreAtHeight(consensusNewHeightEvent.Height) + if err != nil { return err } - } - for _, rm := range removed { - if err := m.router.RemovePeer(rm); err != nil { - return err + + added, removed := oldPeerList.Delta(updatedPeerstore.GetPeerList()) + for _, add := range added { + if err := m.stakedActorRouter.AddPeer(add); err != nil { + return err + } + } + for _, rm := range removed { + if err := m.stakedActorRouter.RemovePeer(rm); err != nil { + return err + } } } @@ -50,7 +55,7 @@ func (m *p2pModule) HandleEvent(event *anypb.Any) error { m.logger.Debug().Fields(messaging.TransitionEventToMap(stateMachineTransitionEvent)).Msg("Received state machine transition event") if stateMachineTransitionEvent.NewState == string(coreTypes.StateMachineState_P2P_Bootstrapping) { - if m.router.GetPeerstore().Size() == 0 { + if m.stakedActorRouter.GetPeerstore().Size() == 0 { m.logger.Warn().Msg("No peers in addrbook, bootstrapping") if err := m.bootstrap(); err != nil { From 43cf6715aabafab19a7575a334ff1a5e0d4a4f09 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 22 Jun 2023 13:06:40 +0200 Subject: [PATCH 12/59] test: post-refactor updates --- p2p/background/router_test.go | 149 ++++++++++++++++++++--------- p2p/module.go | 16 +--- p2p/module_raintree_test.go | 36 +++---- p2p/module_test.go | 8 +- p2p/raintree/peers_manager_test.go | 5 + p2p/raintree/router_test.go | 2 + p2p/testutil.go | 40 ++++++++ p2p/transport_encryption_test.go | 12 ++- p2p/utils_test.go | 63 ++++++++++-- 9 files changed, 234 insertions(+), 97 deletions(-) create mode 100644 p2p/testutil.go diff --git a/p2p/background/router_test.go b/p2p/background/router_test.go index 450946f71..0dd33c5b7 100644 --- a/p2p/background/router_test.go +++ b/p2p/background/router_test.go @@ -8,32 +8,41 @@ import ( "time" "github.com/golang/mock/gomock" + pubsub "github.com/libp2p/go-libp2p-pubsub" libp2pCrypto "github.com/libp2p/go-libp2p/core/crypto" libp2pHost "github.com/libp2p/go-libp2p/core/host" libp2pNetwork "github.com/libp2p/go-libp2p/core/network" libp2pPeer "github.com/libp2p/go-libp2p/core/peer" mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" "github.com/multiformats/go-multiaddr" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/anypb" + "github.com/pokt-network/pocket/internal/testutil" "github.com/pokt-network/pocket/p2p/config" + "github.com/pokt-network/pocket/p2p/protocol" typesP2P "github.com/pokt-network/pocket/p2p/types" mock_types "github.com/pokt-network/pocket/p2p/types/mocks" "github.com/pokt-network/pocket/p2p/utils" "github.com/pokt-network/pocket/runtime/configs" "github.com/pokt-network/pocket/runtime/defaults" cryptoPocket "github.com/pokt-network/pocket/shared/crypto" + "github.com/pokt-network/pocket/shared/messaging" mockModules "github.com/pokt-network/pocket/shared/modules/mocks" - "github.com/stretchr/testify/require" ) // https://www.rfc-editor.org/rfc/rfc3986#section-3.2.2 const testIP6ServiceURL = "[2a00:1450:4005:802::2004]:8080" // TECHDEBT(#609): move & de-dup. -var testLocalServiceURL = fmt.Sprintf("127.0.0.1:%d", defaults.DefaultP2PPort) +var ( + testLocalServiceURL = fmt.Sprintf("127.0.0.1:%d", defaults.DefaultP2PPort) + noopHandler = func(data []byte) error { return nil } +) func TestBackgroundRouter_AddPeer(t *testing.T) { - testRouter := newTestRouter(t, nil) + testRouter := newTestRouter(t, nil, noopHandler) libp2pPStore := testRouter.host.Peerstore() // NB: assert initial state @@ -81,7 +90,7 @@ func TestBackgroundRouter_AddPeer(t *testing.T) { } func TestBackgroundRouter_RemovePeer(t *testing.T) { - testRouter := newTestRouter(t, nil) + testRouter := newTestRouter(t, nil, noopHandler) peerstore := testRouter.host.Peerstore() // NB: assert initial state @@ -114,6 +123,73 @@ func TestBackgroundRouter_RemovePeer(t *testing.T) { require.Len(t, existingPeerstoreAddrs, 1) } +func TestBackgroundRouter_Validation(t *testing.T) { + ctx := context.Background() + libp2pMockNet := mocknet.New() + + invalidWireFormatData := []byte("test message") + invalidPocketEnvelope := &anypb.Any{ + TypeUrl: "/test", + Value: invalidWireFormatData, + } + invalidPocketEnvelopeBz, err := proto.Marshal(invalidPocketEnvelope) + require.NoError(t, err) + + invalidMessages := [][]byte{ + invalidWireFormatData, + invalidPocketEnvelopeBz, + } + + receivedChan := make(chan struct{}) + + receiverPrivKey, receiverPeer := newTestPeer(t) + receiverHost := newTestHost(t, libp2pMockNet, receiverPrivKey) + receiverRouter := newRouterWithSelfPeerAndHost(t, receiverPeer, receiverHost, func(data []byte) error { + receivedChan <- struct{}{} + return nil + }) + + t.Cleanup(func() { + err := receiverRouter.Close() + require.NoError(t, err) + }) + + senderPrivKey, _ := newTestPeer(t) + senderHost := newTestHost(t, libp2pMockNet, senderPrivKey) + gossipPubsub, err := pubsub.NewGossipSub(ctx, senderHost) + require.NoError(t, err) + + err = libp2pMockNet.LinkAll() + require.NoError(t, err) + + receiverAddrInfo, err := utils.Libp2pAddrInfoFromPeer(receiverPeer) + require.NoError(t, err) + + err = senderHost.Connect(ctx, receiverAddrInfo) + require.NoError(t, err) + + topic, err := gossipPubsub.Join(protocol.BackgroundTopicStr) + require.NoError(t, err) + + for _, invalidMessageBz := range invalidMessages { + err = topic.Publish(ctx, invalidMessageBz) + require.NoError(t, err) + } + + select { + case <-time.After(time.Second * 2): + // TECHDEBT: find a better way to prove that pre-propagation validation + // works as expected. Ideally, we should be able to distinguish which + // invalid message was received in the event of failure. + // + // CONSIDERATION: we could use the telemetry module mock to set expectations + // for validation failure telemetry calls, which would probably be useful in + // their own right. + case <-receivedChan: + t.Fatal("expected message to not be received") + } +} + func TestBackgroundRouter_Broadcast(t *testing.T) { const ( numPeers = 4 @@ -138,17 +214,31 @@ func TestBackgroundRouter_Broadcast(t *testing.T) { libp2pMockNet = mocknet.New() ) - // setup 4 libp2p hosts to listen for incoming streams from the test backgroundRouter + testPocketEnvelope, err := messaging.PackMessage(&anypb.Any{ + TypeUrl: "/test", + Value: []byte(testMsg), + }) + require.NoError(t, err) + + testPocketEnvelopeBz, err := proto.Marshal(testPocketEnvelope) + require.NoError(t, err) + + // setup 4 receiver routers to listen for incoming messages from the sender router for i := 0; i < numPeers; i++ { broadcastWaitgroup.Add(1) bootstrapWaitgroup.Add(1) - privKey, selfPeer := newTestPeer(t) + privKey, peer := newTestPeer(t) host := newTestHost(t, libp2pMockNet, privKey) testHosts = append(testHosts, host) expectedPeerIDs[i] = host.ID().String() - rtr := newRouterWithSelfPeerAndHost(t, selfPeer, host) - go readSubscription(t, ctx, &broadcastWaitgroup, rtr, &seenMessagesMutext, seenMessages) + newRouterWithSelfPeerAndHost(t, peer, host, func(data []byte) error { + seenMessagesMutext.Lock() + broadcastWaitgroup.Done() + seenMessages[host.ID().String()] = struct{}{} + seenMessagesMutext.Unlock() + return nil + }) } // bootstrap off of arbitrary testHost @@ -156,12 +246,12 @@ func TestBackgroundRouter_Broadcast(t *testing.T) { // set up a test backgroundRouter testRouterHost := newTestHost(t, libp2pMockNet, privKey) - testRouter := newRouterWithSelfPeerAndHost(t, selfPeer, testRouterHost) + testRouter := newRouterWithSelfPeerAndHost(t, selfPeer, testRouterHost, noopHandler) testHosts = append(testHosts, testRouterHost) // simulate network links between each to every other // (i.e. fully-connected network) - err := libp2pMockNet.LinkAll() + err = libp2pMockNet.LinkAll() require.NoError(t, err) // setup notifee/notify BEFORE bootstrapping @@ -189,7 +279,7 @@ func TestBackgroundRouter_Broadcast(t *testing.T) { // broadcast message t.Log("broadcasting...") - err := testRouter.Broadcast([]byte(testMsg)) + err := testRouter.Broadcast(testPocketEnvelopeBz) require.NoError(t, err) // wait for broadcast to be received by all peers @@ -241,7 +331,7 @@ func bootstrap(t *testing.T, ctx context.Context, testHosts []libp2pHost.Host) { } // TECHDEBT(#609): move & de-duplicate -func newTestRouter(t *testing.T, libp2pMockNet mocknet.Mocknet) *backgroundRouter { +func newTestRouter(t *testing.T, libp2pMockNet mocknet.Mocknet, handler typesP2P.MessageHandler) *backgroundRouter { t.Helper() privKey, selfPeer := newTestPeer(t) @@ -256,10 +346,10 @@ func newTestRouter(t *testing.T, libp2pMockNet mocknet.Mocknet) *backgroundRoute require.NoError(t, err) }) - return newRouterWithSelfPeerAndHost(t, selfPeer, host) + return newRouterWithSelfPeerAndHost(t, selfPeer, host, handler) } -func newRouterWithSelfPeerAndHost(t *testing.T, selfPeer typesP2P.Peer, host libp2pHost.Host) *backgroundRouter { +func newRouterWithSelfPeerAndHost(t *testing.T, selfPeer typesP2P.Peer, host libp2pHost.Host, handler typesP2P.MessageHandler) *backgroundRouter { t.Helper() ctrl := gomock.NewController(t) @@ -268,7 +358,7 @@ func newRouterWithSelfPeerAndHost(t *testing.T, selfPeer typesP2P.Peer, host lib P2P: &configs.P2PConfig{ IsClientOnly: false, }, - }) + }).AnyTimes() consensusMock := mockModules.NewMockConsensusModule(ctrl) consensusMock.EXPECT().CurrentHeight().Return(uint64(1)).AnyTimes() @@ -289,6 +379,7 @@ func newRouterWithSelfPeerAndHost(t *testing.T, selfPeer typesP2P.Peer, host lib PeerstoreProvider: pstoreProviderMock, CurrentHeightProvider: consensusMock, Host: host, + Handler: handler, }) require.NoError(t, err) @@ -345,31 +436,3 @@ func newTestHost(t *testing.T, mockNet mocknet.Mocknet, privKey cryptoPocket.Pri // construct mock host return newMockNetHostFromPeer(t, mockNet, privKey, peer) } - -func readSubscription( - t *testing.T, - ctx context.Context, - broadcastWaitGroup *sync.WaitGroup, - rtr *backgroundRouter, - mu *sync.Mutex, - seenMsgs map[string]struct{}, -) { - t.Helper() - - for { - if err := ctx.Err(); err != nil { - if err != context.Canceled || err != context.DeadlineExceeded { - require.NoError(t, err) - } - return - } - - _, err := rtr.subscription.Next(ctx) - require.NoError(t, err) - - mu.Lock() - broadcastWaitGroup.Done() - seenMsgs[rtr.host.ID().String()] = struct{}{} - mu.Unlock() - } -} diff --git a/p2p/module.go b/p2p/module.go index 1c06e5f78..06ad1dc30 100644 --- a/p2p/module.go +++ b/p2p/module.go @@ -67,20 +67,6 @@ func Create(bus modules.Bus, options ...modules.ModuleOption) (modules.Module, e return new(p2pModule).Create(bus, options...) } -// TODO_THIS_COMMIT: rename (WithHost) & consider moving to testutil file -// WithHostOption associates an existing (i.e. "started") libp2p `host.Host` -// with this module, instead of creating a new one on `#Start()`. -// Primarily intended for testing. -func WithHostOption(host libp2pHost.Host) modules.ModuleOption { - return func(m modules.InitializableModule) { - mod, ok := m.(*p2pModule) - if ok { - mod.host = host - mod.logger.Debug().Msg("using host provided via `WithHostOption`") - } - } -} - func (m *p2pModule) Create(bus modules.Bus, options ...modules.ModuleOption) (modules.Module, error) { logger.Global.Debug().Msg("Creating P2P module") *m = p2pModule{ @@ -157,7 +143,7 @@ func (m *p2pModule) Start() (err error) { telemetry.P2P_NODE_STARTED_TIMESERIES_METRIC_DESCRIPTION, ) - // Return early if host has already been started (e.g. via `WithHostOption`) + // Return early if host has already been started (e.g. via `WithHost`) if m.host == nil { // Libp2p hosts provided via `WithHost()` option are destroyed when // `#Stop()`ing the module. Therefore, a new one must be created. diff --git a/p2p/module_raintree_test.go b/p2p/module_raintree_test.go index 9bd873913..3dc98a988 100644 --- a/p2p/module_raintree_test.go +++ b/p2p/module_raintree_test.go @@ -9,18 +9,14 @@ import ( "regexp" "sort" "strconv" - "strings" "sync" "testing" - libp2pNetwork "github.com/libp2p/go-libp2p/core/network" mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/anypb" "github.com/pokt-network/pocket/internal/testutil" - "github.com/pokt-network/pocket/p2p/protocol" - "github.com/pokt-network/pocket/p2p/raintree" ) // TODO(#314): Add the tooling and instructions on how to generate unit tests in this file. @@ -220,11 +216,13 @@ func TestRainTreeNetworkCompleteTwentySevenNodes(t *testing.T) { // 1. It creates and configures a "real" P2P module where all the other components of the node are mocked. // 2. It then triggers a single message and waits for all of the expected messages transmission to complete before announcing failure. func testRainTreeCalls(t *testing.T, origNode string, networkSimulationConfig TestNetworkSimulationConfig) { + var readWriteWaitGroup sync.WaitGroup + // Configure & prepare test module numValidators := len(networkSimulationConfig) runtimeConfigs := createMockRuntimeMgrs(t, numValidators) genesisMock := runtimeConfigs[0].GetGenesis() - busMocks := createMockBuses(t, runtimeConfigs) + busMocks := createMockBuses(t, runtimeConfigs, &readWriteWaitGroup) valIds := make([]string, 0, numValidators) for valId := range networkSimulationConfig { @@ -241,7 +239,6 @@ func testRainTreeCalls(t *testing.T, origNode string, networkSimulationConfig Te // Create connection and bus mocks along with a shared WaitGroup to track the number of expected // reads and writes throughout the mocked local network - var wg sync.WaitGroup for i, valId := range valIds { expectedCall := networkSimulationConfig[valId] expectedReads := expectedCall.numNetworkReads @@ -249,50 +246,41 @@ func testRainTreeCalls(t *testing.T, origNode string, networkSimulationConfig Te log.Printf("[valId: %s] expected reads: %d\n", valId, expectedReads) log.Printf("[valId: %s] expected writes: %d\n", valId, expectedWrites) - wg.Add(expectedReads) - wg.Add(expectedWrites) + readWriteWaitGroup.Add(expectedReads) + readWriteWaitGroup.Add(expectedWrites) persistenceMock := preparePersistenceMock(t, busMocks[i], genesisMock) consensusMock := prepareConsensusMock(t, busMocks[i]) - telemetryMock := prepareTelemetryMock(t, busMocks[i], valId, &wg, expectedWrites) + telemetryMock := prepareTelemetryMock(t, busMocks[i], valId, &readWriteWaitGroup, expectedWrites) prepareBusMock(busMocks[i], persistenceMock, consensusMock, telemetryMock) } libp2pMockNet := mocknet.New() - defer func() { - err := libp2pMockNet.Close() - require.NoError(t, err) - }() // Inject the connection and bus mocks into the P2P modules p2pModules := createP2PModules(t, busMocks, libp2pMockNet) - for serviceURL, p2pMod := range p2pModules { + for _, p2pMod := range p2pModules { err := p2pMod.Start() require.NoError(t, err) - - sURL := strings.Clone(serviceURL) - mod := *p2pMod - p2pMod.host.SetStreamHandler(protocol.PoktProtocolID, func(stream libp2pNetwork.Stream) { - log.Printf("[valID: %s] Read\n", sURL) - (&mod).router.(*raintree.RainTreeRouter).HandleStream(stream) - wg.Done() - }) } // Wait for completion - defer waitForNetworkSimulationCompletion(t, &wg) + defer waitForNetworkSimulationCompletion(t, &readWriteWaitGroup) t.Cleanup(func() { // Stop all p2p modules for _, p2pMod := range p2pModules { err := p2pMod.Stop() require.NoError(t, err) } + + err := libp2pMockNet.Close() + require.NoError(t, err) }) // Send the first message (by the originator) to trigger a RainTree broadcast - p := &anypb.Any{} + p := &anypb.Any{TypeUrl: "test"} p2pMod := p2pModules[origNode] require.NoError(t, p2pMod.Broadcast(p)) } diff --git a/p2p/module_test.go b/p2p/module_test.go index 79cd17066..45477bd1d 100644 --- a/p2p/module_test.go +++ b/p2p/module_test.go @@ -111,7 +111,7 @@ func Test_Create_configureBootstrapNodes(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ctrl := gomock.NewController(t) mockRuntimeMgr := mockModules.NewMockRuntimeMgr(ctrl) - mockBus := createMockBus(t, mockRuntimeMgr) + mockBus := createMockBus(t, mockRuntimeMgr, nil) genesisStateMock := createMockGenesisState(keys) persistenceMock := preparePersistenceMock(t, mockBus, genesisStateMock) @@ -137,7 +137,7 @@ func Test_Create_configureBootstrapNodes(t *testing.T) { } host := newLibp2pMockNetHost(t, privKey, peer) - p2pMod, err := Create(mockBus, WithHostOption(host)) + p2pMod, err := Create(mockBus, WithHost(host)) if (err != nil) != tt.wantErr { t.Errorf("p2pModule.Create() error = %v, wantErr %v", err, tt.wantErr) } @@ -155,7 +155,7 @@ func TestP2pModule_WithHostOption_Restart(t *testing.T) { privKey := cryptoPocket.GetPrivKeySeed(1) mockRuntimeMgr := mockModules.NewMockRuntimeMgr(ctrl) - mockBus := createMockBus(t, mockRuntimeMgr) + mockBus := createMockBus(t, mockRuntimeMgr, nil) genesisStateMock := createMockGenesisState(nil) persistenceMock := preparePersistenceMock(t, mockBus, genesisStateMock) @@ -184,7 +184,7 @@ func TestP2pModule_WithHostOption_Restart(t *testing.T) { } mockNetHost := newLibp2pMockNetHost(t, privKey, peer) - p2pMod, err := Create(mockBus, WithHostOption(mockNetHost)) + p2pMod, err := Create(mockBus, WithHost(mockNetHost)) require.NoError(t, err) mod, ok := p2pMod.(*p2pModule) diff --git a/p2p/raintree/peers_manager_test.go b/p2p/raintree/peers_manager_test.go index 7fa01a3d3..e3c80d541 100644 --- a/p2p/raintree/peers_manager_test.go +++ b/p2p/raintree/peers_manager_test.go @@ -26,6 +26,8 @@ const ( addrAlphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ[" ) +var noopHandler = func(_ []byte) error { return nil } + type ExpectedRainTreeRouterConfig struct { numNodes int numExpectedLevels int @@ -101,6 +103,7 @@ func TestRainTree_Peerstore_HandleUpdate(t *testing.T) { Addr: pubKey.Address(), PeerstoreProvider: pstoreProviderMock, CurrentHeightProvider: currentHeightProviderMock, + Handler: noopHandler, } router, err := NewRainTreeRouter(mockBus, rtCfg) @@ -168,6 +171,7 @@ func BenchmarkPeerstoreUpdates(b *testing.B) { Addr: pubKey.Address(), PeerstoreProvider: pstoreProviderMock, CurrentHeightProvider: currentHeightProviderMock, + Handler: noopHandler, } router, err := NewRainTreeRouter(mockBus, rtCfg) @@ -293,6 +297,7 @@ func testRainTreeMessageTargets(t *testing.T, expectedMsgProp *ExpectedRainTreeM Addr: []byte{expectedMsgProp.orig}, PeerstoreProvider: pstoreProviderMock, CurrentHeightProvider: currentHeightProviderMock, + Handler: noopHandler, } router, err := NewRainTreeRouter(busMock, rtCfg) diff --git a/p2p/raintree/router_test.go b/p2p/raintree/router_test.go index 1ef093a20..2865a584b 100644 --- a/p2p/raintree/router_test.go +++ b/p2p/raintree/router_test.go @@ -57,6 +57,7 @@ func TestRainTreeRouter_AddPeer(t *testing.T) { Addr: selfAddr, PeerstoreProvider: peerstoreProviderMock, CurrentHeightProvider: currentHeightProviderMock, + Handler: noopHandler, } router, err := NewRainTreeRouter(busMock, rtCfg) @@ -119,6 +120,7 @@ func TestRainTreeRouter_RemovePeer(t *testing.T) { Addr: selfAddr, PeerstoreProvider: peerstoreProviderMock, CurrentHeightProvider: currentHeightProviderMock, + Handler: noopHandler, } router, err := NewRainTreeRouter(busMock, rtCfg) diff --git a/p2p/testutil.go b/p2p/testutil.go new file mode 100644 index 000000000..ee922c634 --- /dev/null +++ b/p2p/testutil.go @@ -0,0 +1,40 @@ +//go:build test + +package p2p + +import ( + libp2pHost "github.com/libp2p/go-libp2p/core/host" + typesP2P "github.com/pokt-network/pocket/p2p/types" + "github.com/pokt-network/pocket/shared/modules" +) + +// WithHost associates an existing (i.e. "started") libp2p `host.Host` +// with this module, instead of creating a new one on `#Start()`. +// Primarily intended for testing. +func WithHost(host libp2pHost.Host) modules.ModuleOption { + return func(m modules.InitializableModule) { + mod, ok := m.(*p2pModule) + if ok { + mod.host = host + mod.logger.Debug().Msg("using host provided via `WithHost`") + } + } +} + +func WithUnstakedActorRouter(router typesP2P.Router) modules.ModuleOption { + return func(m modules.InitializableModule) { + mod, ok := m.(*p2pModule) + if ok { + mod.unstakedActorRouter = router + } + } +} + +func WithStakedActorRouter(router typesP2P.Router) modules.ModuleOption { + return func(m modules.InitializableModule) { + mod, ok := m.(*p2pModule) + if ok { + mod.stakedActorRouter = router + } + } +} diff --git a/p2p/transport_encryption_test.go b/p2p/transport_encryption_test.go index d95cb6496..799340a17 100644 --- a/p2p/transport_encryption_test.go +++ b/p2p/transport_encryption_test.go @@ -14,6 +14,7 @@ import ( "github.com/pokt-network/pocket/internal/testutil" "github.com/pokt-network/pocket/p2p/protocol" typesP2P "github.com/pokt-network/pocket/p2p/types" + mock_types "github.com/pokt-network/pocket/p2p/types/mocks" "github.com/pokt-network/pocket/p2p/utils" "github.com/pokt-network/pocket/runtime/configs" "github.com/pokt-network/pocket/runtime/configs/types" @@ -23,7 +24,7 @@ import ( mockModules "github.com/pokt-network/pocket/shared/modules/mocks" ) -func TestP2pModule_Insecure_Error(t *testing.T) { +func TestP2pModule_RainTreeRouter_Insecure_Error(t *testing.T) { // TECHDEBT(#609): refactor mock setup with similar test utilities. ctrl := gomock.NewController(t) hostname := "127.0.0.1" @@ -54,7 +55,7 @@ func TestP2pModule_Insecure_Error(t *testing.T) { telemetryMock.EXPECT().GetEventMetricsAgent().Return(eventMetricsAgentMock).AnyTimes() telemetryMock.EXPECT().GetModuleName().Return(modules.TelemetryModuleName).AnyTimes() - busMock := createMockBus(t, runtimeMgrMock) + busMock := createMockBus(t, runtimeMgrMock, nil) busMock.EXPECT().GetConsensusModule().Return(mockConsensusModule).AnyTimes() busMock.EXPECT().GetRuntimeMgr().Return(runtimeMgrMock).AnyTimes() busMock.EXPECT().GetTelemetryModule().Return(telemetryMock).AnyTimes() @@ -73,7 +74,10 @@ func TestP2pModule_Insecure_Error(t *testing.T) { dnsDone := testutil.PrepareDNSMockFromServiceURLs(t, serviceURLs) t.Cleanup(dnsDone) - p2pMod, err := Create(busMock) + routerMock := mock_types.NewMockRouter(ctrl) + routerMock.EXPECT().Close().Times(1) + + p2pMod, err := Create(busMock, WithUnstakedActorRouter(routerMock)) require.NoError(t, err) err = p2pMod.Start() @@ -114,6 +118,6 @@ func TestP2pModule_Insecure_Error(t *testing.T) { require.NoError(t, err) ctx := context.Background() - _, err = clearNode.NewStream(ctx, libp2pPeerInfo.ID, protocol.PoktProtocolID) + _, err = clearNode.NewStream(ctx, libp2pPeerInfo.ID, protocol.RaintreeProtocolID) require.ErrorContains(t, err, "failed to negotiate security protocol: protocols not supported:") } diff --git a/p2p/utils_test.go b/p2p/utils_test.go index 633ea1425..b9f89fcf2 100644 --- a/p2p/utils_test.go +++ b/p2p/utils_test.go @@ -20,6 +20,7 @@ import ( "github.com/pokt-network/pocket/p2p/providers/current_height_provider" "github.com/pokt-network/pocket/p2p/providers/peerstore_provider" typesP2P "github.com/pokt-network/pocket/p2p/types" + mock_types "github.com/pokt-network/pocket/p2p/types/mocks" "github.com/pokt-network/pocket/p2p/utils" "github.com/pokt-network/pocket/runtime" "github.com/pokt-network/pocket/runtime/configs" @@ -29,6 +30,7 @@ import ( "github.com/pokt-network/pocket/runtime/test_artifacts" coreTypes "github.com/pokt-network/pocket/shared/core/types" cryptoPocket "github.com/pokt-network/pocket/shared/crypto" + "github.com/pokt-network/pocket/shared/messaging" "github.com/pokt-network/pocket/shared/modules" mockModules "github.com/pokt-network/pocket/shared/modules/mocks" "github.com/pokt-network/pocket/telemetry" @@ -107,10 +109,20 @@ func waitForNetworkSimulationCompletion(t *testing.T, wg *sync.WaitGroup) { func createP2PModules(t *testing.T, busMocks []*mockModules.MockBus, netMock mocknet.Mocknet) (p2pModules map[string]*p2pModule) { peerIDs := setupMockNetPeers(t, netMock, len(busMocks)) + ctrl := gomock.NewController(t) + backgroundRouterMock := mock_types.NewMockRouter(ctrl) + backgroundRouterMock.EXPECT().Broadcast(gomock.Any()).Times(1) + backgroundRouterMock.EXPECT().Close().Times(len(busMocks)) + p2pModules = make(map[string]*p2pModule, len(busMocks)) for i := range busMocks { host := netMock.Host(peerIDs[i]) - p2pMod, err := Create(busMocks[i], WithHostOption(host)) + p2pMod, err := Create( + busMocks[i], + WithHost(host), + // mock background router to prevent background message propagation. + WithUnstakedActorRouter(backgroundRouterMock), + ) require.NoError(t, err) p2pModules[validatorId(i+1)] = p2pMod.(*p2pModule) } @@ -180,15 +192,23 @@ func createMockRuntimeMgrs(t *testing.T, numValidators int) []modules.RuntimeMgr return mockRuntimeMgrs } -func createMockBuses(t *testing.T, runtimeMgrs []modules.RuntimeMgr) []*mockModules.MockBus { +func createMockBuses( + t *testing.T, + runtimeMgrs []modules.RuntimeMgr, + readWriteWaitGroup *sync.WaitGroup, +) []*mockModules.MockBus { mockBuses := make([]*mockModules.MockBus, len(runtimeMgrs)) for i := range mockBuses { - mockBuses[i] = createMockBus(t, runtimeMgrs[i]) + mockBuses[i] = createMockBus(t, runtimeMgrs[i], readWriteWaitGroup) } return mockBuses } -func createMockBus(t *testing.T, runtimeMgr modules.RuntimeMgr) *mockModules.MockBus { +func createMockBus( + t *testing.T, + runtimeMgr modules.RuntimeMgr, + readWriteWaitGroup *sync.WaitGroup, +) *mockModules.MockBus { ctrl := gomock.NewController(t) mockBus := mockModules.NewMockBus(ctrl) mockBus.EXPECT().GetRuntimeMgr().Return(runtimeMgr).AnyTimes() @@ -199,6 +219,14 @@ func createMockBus(t *testing.T, runtimeMgr modules.RuntimeMgr) *mockModules.Moc mockModulesRegistry.EXPECT().GetModule(peerstore_provider.ModuleName).Return(nil, runtime.ErrModuleNotRegistered(peerstore_provider.ModuleName)).AnyTimes() mockModulesRegistry.EXPECT().GetModule(current_height_provider.ModuleName).Return(nil, runtime.ErrModuleNotRegistered(current_height_provider.ModuleName)).AnyTimes() mockBus.EXPECT().GetModulesRegistry().Return(mockModulesRegistry).AnyTimes() + mockBus.EXPECT().PublishEventToBus(gomock.AssignableToTypeOf(&messaging.PocketEnvelope{})). + Do(func(envelope *messaging.PocketEnvelope) { + fmt.Println("[valId: unknown] Read") + fmt.Printf("content type: %s\n", envelope.Content.GetTypeUrl()) + if readWriteWaitGroup != nil { + readWriteWaitGroup.Done() + } + }).AnyTimes() // TODO: specific times mockBus.EXPECT().PublishEventToBus(gomock.Any()).AnyTimes() return mockBus } @@ -313,11 +341,32 @@ func prepareEventMetricsAgentMock(t *testing.T, valId string, wg *sync.WaitGroup ctrl := gomock.NewController(t) eventMetricsAgentMock := mockModules.NewMockEventMetricsAgent(ctrl) - eventMetricsAgentMock.EXPECT().EmitEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - eventMetricsAgentMock.EXPECT().EmitEvent(gomock.Any(), gomock.Any(), gomock.Eq(telemetry.P2P_RAINTREE_MESSAGE_EVENT_METRIC_SEND_LABEL), gomock.Any()).Do(func(n, e any, l ...any) { + // DISCUSS_THIS_COMMIT: The number of times each telemetry event is expected + // (below) is dependent on the number of redundant messages all validators see, + // which is a function of the network size. Until this function is derived and + // implemented, we cannot predict the number of times each event is expected. + _ = expectedNumNetworkWrites + + eventMetricsAgentMock.EXPECT().EmitEvent( + gomock.Any(), + gomock.Any(), + gomock.Eq(telemetry.P2P_RAINTREE_MESSAGE_EVENT_METRIC_SEND_LABEL), + gomock.Any(), + ).Do(func(n, e any, l ...any) { + log.Printf("[valId: %s] Write\n", valId) + wg.Done() + }).AnyTimes() // TECHDEBT: expect specific number of non-redundant writes once known. + eventMetricsAgentMock.EXPECT().EmitEvent( + gomock.Any(), + gomock.Eq(telemetry.P2P_BROADCAST_MESSAGE_REDUNDANCY_PER_BLOCK_EVENT_METRIC_NAME), + gomock.Any(), + gomock.Any(), // nonce + gomock.Any(), + gomock.Any(), // blockHeight + ).Do(func(n, e any, l ...any) { log.Printf("[valId: %s] Write\n", valId) wg.Done() - }).Times(expectedNumNetworkWrites) + }).AnyTimes() // TECHDEBT: expect specific number of redundant writes once known. eventMetricsAgentMock.EXPECT().EmitEvent(gomock.Any(), gomock.Any(), gomock.Not(telemetry.P2P_RAINTREE_MESSAGE_EVENT_METRIC_SEND_LABEL), gomock.Any()).AnyTimes() return eventMetricsAgentMock From 048e3065b451fa4029ea461e4f2418a679ade47f Mon Sep 17 00:00:00 2001 From: Bryan White Date: Fri, 23 Jun 2023 11:40:22 +0200 Subject: [PATCH 13/59] fix: gofmt --- p2p/unicast/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/unicast/router.go b/p2p/unicast/router.go index bec52b36b..88eef4929 100644 --- a/p2p/unicast/router.go +++ b/p2p/unicast/router.go @@ -35,7 +35,7 @@ type UnicastRouter struct { // (see: https://pkg.go.dev/github.com/libp2p/go-libp2p#section-readme) host libp2pHost.Host messageHandler typesP2P.MessageHandler - peerHandler func(peer typesP2P.Peer) error + peerHandler func(peer typesP2P.Peer) error } func Create(bus modules.Bus, cfg *config.UnicastRouterConfig) (*UnicastRouter, error) { From 9ab2a5d05da5083446d3fa2d705111d28c4a539c Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 26 Jun 2023 09:02:37 +0200 Subject: [PATCH 14/59] chore: fix typo in comment --- p2p/unicast/logging.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/unicast/logging.go b/p2p/unicast/logging.go index 7e2539c63..1539b9059 100644 --- a/p2p/unicast/logging.go +++ b/p2p/unicast/logging.go @@ -14,7 +14,7 @@ import ( // of too many concurrent streams). // // This could ultimately be actuated from the CLI via flags, configs, and/or env -// vars. Initially, weo could consider coupling to a `--verbose` persistent flag. +// vars. Initially, we could consider coupling to a `--verbose` persistent flag. // // logStream logs the incoming stream and its scope stats From dce1bac973fdc9db3b3084f2fb68a2520f9085d9 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 26 Jun 2023 09:02:51 +0200 Subject: [PATCH 15/59] chore: add debug log --- p2p/raintree/router.go | 1 + 1 file changed, 1 insertion(+) diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index a68fd293d..c399e85aa 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -222,6 +222,7 @@ func (rtr *rainTreeRouter) handleRainTreeMsg(rainTreeMsgBz []byte) error { // There was no error, but we don't need to forward this to the app-specific bus. // For example, the message has already been handled by the application. if rainTreeMsg.Data == nil { + rtr.logger.Debug().Msg("no data in RainTree message") return nil } From 8dc2852875c81cc6b8bb5641d6a686e68da7fabb Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 26 Jun 2023 09:03:05 +0200 Subject: [PATCH 16/59] chore: fix field comment out of place --- p2p/unicast/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/unicast/router.go b/p2p/unicast/router.go index 88eef4929..de8112629 100644 --- a/p2p/unicast/router.go +++ b/p2p/unicast/router.go @@ -28,12 +28,12 @@ type UnicastRouter struct { base_modules.IntegratableModule logger *modules.Logger + host libp2pHost.Host // messageHandler is the function to call when a message is received. // host represents a libp2p network node, it encapsulates a libp2p peerstore // & connection manager. `libp2p.New` configures and starts listening // according to options. // (see: https://pkg.go.dev/github.com/libp2p/go-libp2p#section-readme) - host libp2pHost.Host messageHandler typesP2P.MessageHandler peerHandler func(peer typesP2P.Peer) error } From a6d4b520b1e9947b370c2f770c7c38c54ba3ea0c Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 26 Jun 2023 09:25:51 +0200 Subject: [PATCH 17/59] fix: imports --- p2p/raintree/router.go | 1 + 1 file changed, 1 insertion(+) diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index c399e85aa..60dbcbd86 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -2,6 +2,7 @@ package raintree import ( "fmt" + libp2pHost "github.com/libp2p/go-libp2p/core/host" "github.com/pokt-network/pocket/p2p/unicast" "google.golang.org/protobuf/proto" From d24407b1e73d4a7302c99ace0e31f6d71a4abe62 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 26 Jun 2023 13:03:44 +0200 Subject: [PATCH 18/59] chore: bootstrap refactor / TECHDEBT --- p2p/background/router.go | 62 ++++++++++++++++++++-------------------- p2p/bootstrap.go | 1 + 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/p2p/background/router.go b/p2p/background/router.go index 1ee3dc845..680e3c231 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -232,17 +232,13 @@ func (rtr *backgroundRouter) setupDependencies(ctx context.Context, cfg *config. } if err := rtr.setupPeerstore( - ctx, - cfg.PeerstoreProvider, - cfg.CurrentHeightProvider, - ); err != nil { + cfg.PeerstoreProvider, cfg.CurrentHeightProvider); err != nil { return fmt.Errorf("setting up peerstore: %w", err) } return nil } func (rtr *backgroundRouter) setupPeerstore( - ctx context.Context, pstoreProvider providers.PeerstoreProvider, currentHeightProvider providers.CurrentHeightProvider, ) (err error) { @@ -254,32 +250,6 @@ func (rtr *backgroundRouter) setupPeerstore( return err } - // CONSIDERATION: add `GetPeers` method to `PeerstoreProvider` interface - // to avoid this loop. - for _, peer := range rtr.pstore.GetPeerList() { - if err := utils.AddPeerToLibp2pHost(rtr.host, peer); err != nil { - return err - } - - // TODO: refactor: #bootstrap() - libp2pAddrInfo, err := utils.Libp2pAddrInfoFromPeer(peer) - if err != nil { - return fmt.Errorf( - "converting peer info, pokt address: %s: %w", - peer.GetAddress(), - err, - ) - } - - // don't attempt to connect to self - if rtr.host.ID() == libp2pAddrInfo.ID { - return nil - } - - if err := rtr.host.Connect(ctx, libp2pAddrInfo); err != nil { - return fmt.Errorf("connecting to peer: %w", err) - } - } return nil } @@ -332,6 +302,36 @@ func (rtr *backgroundRouter) setupSubscription() (err error) { return err } +//nolint:unused // TECHDEBT(#859): refactor bootstrapping +func (rtr *backgroundRouter) bootstrap(ctx context.Context) error { + // CONSIDERATION: add `GetPeers` method to `PeerstoreProvider` interface + // to avoid this loop. + for _, peer := range rtr.pstore.GetPeerList() { + if err := utils.AddPeerToLibp2pHost(rtr.host, peer); err != nil { + return err + } + + libp2pAddrInfo, err := utils.Libp2pAddrInfoFromPeer(peer) + if err != nil { + return fmt.Errorf( + "converting peer info, pokt address: %s: %w", + peer.GetAddress(), + err, + ) + } + + // don't attempt to connect to self + if rtr.host.ID() == libp2pAddrInfo.ID { + return nil + } + + if err := rtr.host.Connect(ctx, libp2pAddrInfo); err != nil { + return fmt.Errorf("connecting to peer: %w", err) + } + } + return nil +} + // topicValidator is used in conjunction with libp2p-pubsub's notion of "topic // validaton". It is usefed for arbitrary and concurrent pre-propagation validation // of messages. diff --git a/p2p/bootstrap.go b/p2p/bootstrap.go index 68952d44d..1def39373 100644 --- a/p2p/bootstrap.go +++ b/p2p/bootstrap.go @@ -42,6 +42,7 @@ func (m *p2pModule) configureBootstrapNodes() error { } // bootstrap attempts to bootstrap from a bootstrap node +// TECHDEBT(#859): refactor bootstrapping. func (m *p2pModule) bootstrap() error { var pstore typesP2P.Peerstore From 70ca5732fbcc2fb09c56133628a419ae04f2ad88 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 26 Jun 2023 15:18:38 +0200 Subject: [PATCH 19/59] fix: imports --- app/client/cli/consensus.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/client/cli/consensus.go b/app/client/cli/consensus.go index a74876949..3cb69b32d 100644 --- a/app/client/cli/consensus.go +++ b/app/client/cli/consensus.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "github.com/spf13/cobra" "github.com/pokt-network/pocket/app/client/cli/flags" From 8467f3a832ae26b48ed9ec481a8b99355c849c09 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 26 Jun 2023 15:20:04 +0200 Subject: [PATCH 20/59] chore: remove unused field --- p2p/raintree/router.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index 0de9c92f1..9955f832d 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -16,7 +16,6 @@ import ( "github.com/pokt-network/pocket/p2p/utils" "github.com/pokt-network/pocket/shared/codec" cryptoPocket "github.com/pokt-network/pocket/shared/crypto" - "github.com/pokt-network/pocket/shared/mempool" "github.com/pokt-network/pocket/shared/messaging" "github.com/pokt-network/pocket/shared/modules" "github.com/pokt-network/pocket/shared/modules/base_modules" @@ -48,7 +47,6 @@ type rainTreeRouter struct { peersManager *rainTreePeersManager pstoreProvider peerstore_provider.PeerstoreProvider currentHeightProvider providers.CurrentHeightProvider - nonceDeduper *mempool.GenericFIFOSet[uint64, uint64] } func NewRainTreeRouter(bus modules.Bus, cfg *config.RainTreeConfig) (typesP2P.Router, error) { From c3bc4c7e06e21bd313f88e887f30f26c9a9fa810 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Tue, 27 Jun 2023 15:51:27 +0200 Subject: [PATCH 21/59] chore: cleanup unused test utils --- internal/testutil/proxy.go | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 internal/testutil/proxy.go diff --git a/internal/testutil/proxy.go b/internal/testutil/proxy.go deleted file mode 100644 index a586a6c05..000000000 --- a/internal/testutil/proxy.go +++ /dev/null @@ -1,3 +0,0 @@ -package testutil - -type ProxyFactory[T any] func(target T) (proxy T) From 3fdada607bec530abda34c17d9e1134ec809e05a Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 09:13:18 +0200 Subject: [PATCH 22/59] chore: comment cleanup Co-authored-by: @Olshansk --- p2p/raintree/router.go | 3 +-- p2p/unicast/logging.go | 1 - p2p/unicast/router.go | 8 ++++++++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index 60dbcbd86..01eab6d77 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -211,7 +211,6 @@ func (rtr *rainTreeRouter) handleRainTreeMsg(rainTreeMsgBz []byte) error { rtr.logger.Error().Err(err).Msg("Error decoding network message") return err } - // -- // Continue RainTree propagation if rainTreeMsg.Level > 0 { @@ -227,7 +226,7 @@ func (rtr *rainTreeRouter) handleRainTreeMsg(rainTreeMsgBz []byte) error { return nil } - // call configured handler to forward to app-specific bus + // Call configured message handler with the serialized `PocketEnvelope`. if err := rtr.handler(rainTreeMsg.Data); err != nil { rtr.logger.Error().Err(err).Msg("handling raintree message") } diff --git a/p2p/unicast/logging.go b/p2p/unicast/logging.go index 1539b9059..4a7f4faaf 100644 --- a/p2p/unicast/logging.go +++ b/p2p/unicast/logging.go @@ -15,7 +15,6 @@ import ( // // This could ultimately be actuated from the CLI via flags, configs, and/or env // vars. Initially, we could consider coupling to a `--verbose` persistent flag. -// // logStream logs the incoming stream and its scope stats func (rtr *UnicastRouter) logStream(stream libp2pNetwork.Stream) { diff --git a/p2p/unicast/router.go b/p2p/unicast/router.go index de8112629..ebd5a4e7b 100644 --- a/p2p/unicast/router.go +++ b/p2p/unicast/router.go @@ -53,6 +53,10 @@ func (*UnicastRouter) Create(bus modules.Bus, cfg *config.UnicastRouterConfig) ( messageHandler: cfg.MessageHandler, peerHandler: cfg.PeerHandler, } + + // `UnicastRouter` is not a submodule and therefore does not register with the + // module registry. However, as it does depend on the bus and therefore MUST + // embed the base `IntegrableModule` and call `#SetBus()`. rtr.SetBus(bus) // Don't handle incoming streams in client debug mode. @@ -73,6 +77,9 @@ func (rtr *UnicastRouter) handleStream(stream libp2pNetwork.Stream) { Str("address", peer.GetAddress().String()). Msg("parsing remote peer identity") + // Reset stream to signal the sender to give up and move on. + // NB: failing to reset the stream can easily max out the number of available + // network connections on the receiver's side. if err = stream.Reset(); err != nil { rtr.logger.Error().Err(err).Msg("resetting stream") } @@ -85,6 +92,7 @@ func (rtr *UnicastRouter) handleStream(stream libp2pNetwork.Stream) { Msg("adding remote peer to router") } + // concurrently read messages out of incoming streams for handling. go rtr.readStream(stream) } From 39a78773a9b1012694986afc028591592b169d29 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 09:13:33 +0200 Subject: [PATCH 23/59] chore: add submodule TECHDEBT comments --- p2p/module.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/p2p/module.go b/p2p/module.go index d9e164fc7..5f87d5b6e 100644 --- a/p2p/module.go +++ b/p2p/module.go @@ -43,11 +43,17 @@ type p2pModule struct { identity libp2p.Option listenAddrs libp2p.Option + // TECHDEBT(#810): register the providers to the module registry instead of + // holding a reference in the module struct and passing via router config. + // // Assigned during creation via `#setupDependencies()`. currentHeightProvider providers.CurrentHeightProvider pstoreProvider providers.PeerstoreProvider nonceDeduper *mempool.GenericFIFOSet[uint64, uint64] + // TECHDEBT(#810): register the routers to the module registry instead of + // holding a reference in the module struct. This will improve testability. + // // Assigned during `#Start()`. TLDR; `host` listens on instantiation. // and `router` depends on `host`. router typesP2P.Router @@ -252,6 +258,9 @@ func (m *p2pModule) setupPeerstoreProvider() error { if !ok { return fmt.Errorf("unknown peerstore provider type: %T", pstoreProviderModule) } + + // TECHDEBT(#810): register the provider to the module registry instead of + // holding a reference in the module struct and passing via router config. m.pstoreProvider = pstoreProvider return nil @@ -260,6 +269,7 @@ func (m *p2pModule) setupPeerstoreProvider() error { // setupCurrentHeightProvider attempts to retrieve the current height provider // from the bus registry, falls back to the consensus module if none is registered. func (m *p2pModule) setupCurrentHeightProvider() error { + // TECHDEBT(#810): simplify once submodules are more convenient to retrieve. m.logger.Debug().Msg("setupCurrentHeightProvider") currentHeightProviderModule, err := m.GetBus().GetModulesRegistry().GetModule(current_height_provider.ModuleName) if err != nil { @@ -276,6 +286,9 @@ func (m *p2pModule) setupCurrentHeightProvider() error { if !ok { return fmt.Errorf("unexpected current height provider type: %T", currentHeightProviderModule) } + + // TECHDEBT(#810): register the provider to the module registry instead of + // holding a reference in the module struct and passing via router config. m.currentHeightProvider = currentHeightProvider return nil @@ -294,6 +307,8 @@ func (m *p2pModule) setupNonceDeduper() error { // setupRouter instantiates the configured router implementation. func (m *p2pModule) setupRouter() (err error) { + // TECHDEBT(#810): register the router to the module registry instead of + // holding a reference in the module struct. This will improve testability. m.router, err = raintree.NewRainTreeRouter( m.GetBus(), &config.RainTreeConfig{ From 049cbf5e03fca4831b84cc6cb50a3c6a4c0a038a Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 09:13:44 +0200 Subject: [PATCH 24/59] chore: add missing godoc comments --- p2p/raintree/router.go | 2 ++ p2p/types/router.go | 15 +++++++++++++-- p2p/unicast/router.go | 5 ++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index 01eab6d77..3b0c59b34 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -259,6 +259,7 @@ func (rtr *rainTreeRouter) AddPeer(peer typesP2P.Peer) error { return nil } +// RemovePeer implements the respective member of `typesP2P.Router`. func (rtr *rainTreeRouter) RemovePeer(peer typesP2P.Peer) error { rtr.peersManager.HandleEvent( typesP2P.PeerManagerEvent{ @@ -280,6 +281,7 @@ func shouldSendToTarget(target target) bool { return !target.isSelf } +// setupUnicastRouter configures and assigns `rtr.UnicastRouter`. func (rtr *rainTreeRouter) setupUnicastRouter() error { unicastRouterCfg := config.UnicastRouterConfig{ Logger: rtr.logger, diff --git a/p2p/types/router.go b/p2p/types/router.go index 21a2b8010..37080acbd 100644 --- a/p2p/types/router.go +++ b/p2p/types/router.go @@ -15,10 +15,21 @@ type Router interface { Broadcast(data []byte) error Send(data []byte, address cryptoPocket.Address) error - // Address book helpers - // TECHDEBT: simplify - remove `GetPeerstore` + // GetPeerstore is used by the P2P module to update the staked actor router's + // (`RainTreeRouter`) peerstore. + // + // TECHDEBT(#859+): remove the need for this group of interface methods. + // All peer discovery logic should be encapsulated by the router. + // Adopt `HandleEvent(*anypb.Any) error` here instead and forward events + // from P2P module to routers. + // CONSIDERATION: Utility, Conseneus and P2P modules could share an interface + // containing this method (e.g. `BusEventHandler`). GetPeerstore() Peerstore + // AddPeer is used to add a peer to the routers peerstore. It is intended to + // support peer discovery. AddPeer(peer Peer) error + // RemovePeer is used to remove a peer to the routers peerstore. It is used + // during churn to purge offline peers from the routers peerstore. RemovePeer(peer Peer) error } diff --git a/p2p/unicast/router.go b/p2p/unicast/router.go index ebd5a4e7b..bc461003e 100644 --- a/p2p/unicast/router.go +++ b/p2p/unicast/router.go @@ -35,7 +35,10 @@ type UnicastRouter struct { // according to options. // (see: https://pkg.go.dev/github.com/libp2p/go-libp2p#section-readme) messageHandler typesP2P.MessageHandler - peerHandler func(peer typesP2P.Peer) error + // peerHandler is called whenever a new incoming stream is established. + // TECHDEBT(#749,#747): this may not be needed once we've adopted libp2p + // peer IDs and multiaddr natively. + peerHandler func(peer typesP2P.Peer) error } func Create(bus modules.Bus, cfg *config.UnicastRouterConfig) (*UnicastRouter, error) { From 79a1c6e6f517bd464fa53fe168f91a58c8768639 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 09:14:10 +0200 Subject: [PATCH 25/59] chore: cleanup unused garbage --- p2p/raintree/testutil.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/p2p/raintree/testutil.go b/p2p/raintree/testutil.go index f12ef0a58..e2e81b636 100644 --- a/p2p/raintree/testutil.go +++ b/p2p/raintree/testutil.go @@ -4,28 +4,12 @@ package raintree import ( libp2pNetwork "github.com/libp2p/go-libp2p/core/network" - "github.com/regen-network/gocuke" - - "github.com/pokt-network/pocket/internal/testutil" - typesP2P "github.com/pokt-network/pocket/p2p/types" ) // RainTreeRouter exports `rainTreeRouter` for testing purposes. type RainTreeRouter = rainTreeRouter -type routerHandlerProxyFactory = testutil.ProxyFactory[typesP2P.MessageHandler] - // HandleStream exports `rainTreeRouter#handleStream` for testing purposes. func (rtr *rainTreeRouter) HandleStream(stream libp2pNetwork.Stream) { rtr.UnicastRouter.HandleStream(stream) } -func (rtr *rainTreeRouter) HandlerProxy( - t gocuke.TestingT, - handlerProxyFactory routerHandlerProxyFactory, -) { - t.Helper() - - // pass original handler to proxy factory & replace it with the proxy - origHandler := rtr.handler - rtr.handler = handlerProxyFactory(origHandler) -} From a7c4bf6b4658dc3cb6b95e7da9d67f2ff67f513b Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 09:14:19 +0200 Subject: [PATCH 26/59] fix: return error --- p2p/raintree/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index 3b0c59b34..bd0656ef7 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -228,7 +228,7 @@ func (rtr *rainTreeRouter) handleRainTreeMsg(rainTreeMsgBz []byte) error { // Call configured message handler with the serialized `PocketEnvelope`. if err := rtr.handler(rainTreeMsg.Data); err != nil { - rtr.logger.Error().Err(err).Msg("handling raintree message") + return fmt.Errorf("handling raintree message: %w", err) } return nil } From 8a54f1a13d8e0943c9f7a63ec3a0750f9d0e1b64 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 13:16:56 +0200 Subject: [PATCH 27/59] chore: router logging improvements --- p2p/background/router.go | 11 ++++++++--- p2p/raintree/router.go | 19 +++++++++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/p2p/background/router.go b/p2p/background/router.go index 680e3c231..d6672bf4c 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -77,8 +77,7 @@ func Create(bus modules.Bus, cfg *config.BackgroundConfig) (typesP2P.Router, err } func (*backgroundRouter) Create(bus modules.Bus, cfg *config.BackgroundConfig) (typesP2P.Router, error) { - networkLogger := logger.Global.CreateLoggerForModule("backgroundRouter") - networkLogger.Info().Msg("initializing background router") + bgRouterLogger := logger.Global.CreateLoggerForModule("backgroundRouter") if err := cfg.IsValid(); err != nil { return nil, err @@ -88,13 +87,19 @@ func (*backgroundRouter) Create(bus modules.Bus, cfg *config.BackgroundConfig) ( ctx, cancel := context.WithCancel(context.TODO()) rtr := &backgroundRouter{ - logger: networkLogger, + logger: bgRouterLogger, handler: cfg.Handler, host: cfg.Host, cancelCtx: cancel, } rtr.SetBus(bus) + bgRouterLogger.Info().Fields(map[string]any{ + "host_id": cfg.Host.ID(), + "unicast_protocol_id": protocol.BackgroundProtocolID, + "broadcast_pubsub_topic": protocol.BackgroundTopicStr, + }).Msg("initializing background router") + if err := rtr.setupDependencies(ctx, cfg); err != nil { return nil, err } diff --git a/p2p/raintree/router.go b/p2p/raintree/router.go index ea43de424..d79e5be76 100644 --- a/p2p/raintree/router.go +++ b/p2p/raintree/router.go @@ -54,9 +54,7 @@ func NewRainTreeRouter(bus modules.Bus, cfg *config.RainTreeConfig) (typesP2P.Ro } func (*rainTreeRouter) Create(bus modules.Bus, cfg *config.RainTreeConfig) (typesP2P.Router, error) { - routerLogger := logger.Global.CreateLoggerForModule("rainTreeRouter") - routerLogger.Info().Msg("Initializing rainTreeRouter") - + rainTreeLogger := logger.Global.CreateLoggerForModule("rainTreeRouter") if err := cfg.IsValid(); err != nil { return nil, err } @@ -66,11 +64,24 @@ func (*rainTreeRouter) Create(bus modules.Bus, cfg *config.RainTreeConfig) (type selfAddr: cfg.Addr, pstoreProvider: cfg.PeerstoreProvider, currentHeightProvider: cfg.CurrentHeightProvider, - logger: routerLogger, + logger: rainTreeLogger, handler: cfg.Handler, } rtr.SetBus(bus) + height := rtr.currentHeightProvider.CurrentHeight() + pstore, err := rtr.pstoreProvider.GetStakedPeerstoreAtHeight(height) + if err != nil { + return nil, fmt.Errorf("getting staked peerstore at height %d: %w", height, err) + } + rainTreeLogger.Info().Fields(map[string]any{ + "address": cfg.Addr, + "host_id": cfg.Host.ID(), + "protocol_id": protocol.BackgroundProtocolID, + "current_height": height, + "peerstore_size": pstore.Size(), + }).Msg("initializing raintree router") + if err := rtr.setupDependencies(); err != nil { return nil, err } From 6795c96324bf867f2b3856791c59eb4faf69851f Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 13:18:27 +0200 Subject: [PATCH 28/59] fix: interim background router bootstrapping --- p2p/background/router.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/p2p/background/router.go b/p2p/background/router.go index d6672bf4c..ade9594ea 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -237,6 +237,7 @@ func (rtr *backgroundRouter) setupDependencies(ctx context.Context, cfg *config. } if err := rtr.setupPeerstore( + ctx, cfg.PeerstoreProvider, cfg.CurrentHeightProvider); err != nil { return fmt.Errorf("setting up peerstore: %w", err) } @@ -244,6 +245,7 @@ func (rtr *backgroundRouter) setupDependencies(ctx context.Context, cfg *config. } func (rtr *backgroundRouter) setupPeerstore( + ctx context.Context, pstoreProvider providers.PeerstoreProvider, currentHeightProvider providers.CurrentHeightProvider, ) (err error) { @@ -255,6 +257,10 @@ func (rtr *backgroundRouter) setupPeerstore( return err } + if err := rtr.bootstrap(ctx); err != nil { + return fmt.Errorf("bootstrapping peerstore: %w", err) + } + return nil } @@ -307,7 +313,6 @@ func (rtr *backgroundRouter) setupSubscription() (err error) { return err } -//nolint:unused // TECHDEBT(#859): refactor bootstrapping func (rtr *backgroundRouter) bootstrap(ctx context.Context) error { // CONSIDERATION: add `GetPeers` method to `PeerstoreProvider` interface // to avoid this loop. From 0a3fac1a38e2df362863514b78751018078d2b5a Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 13:18:49 +0200 Subject: [PATCH 29/59] fix: `p2pModule#Send()` routing logic --- p2p/module.go | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/p2p/module.go b/p2p/module.go index 77a3f1f38..a05099a77 100644 --- a/p2p/module.go +++ b/p2p/module.go @@ -250,32 +250,28 @@ func (m *p2pModule) Send(addr cryptoPocket.Address, msg *anypb.Any) error { return err } - unstakedSendErr := m.unstakedActorRouter.Send(poktEnvelopeBz, addr) - isStaked, err := m.isStakedActor() if err != nil { return err } - var stakedSendErr error - if isStaked { - stakedSendErr = m.stakedActorRouter.Send(poktEnvelopeBz, addr) + // Send via the staked actor router both if this node and the peer are staked + // actors; otherwise, send via the unstaked actor router. + if !isStaked { + return m.unstakedActorRouter.Send(poktEnvelopeBz, addr) } - if errors.Is(unstakedSendErr, typesP2P.ErrUnknownPeer) && - errors.Is(stakedSendErr, typesP2P.ErrUnknownPeer) { - return typesP2P.ErrUnknownPeer - } + stakedActorSendErr := m.stakedActorRouter.Send(poktEnvelopeBz, addr) - if errors.Is(unstakedSendErr, typesP2P.ErrUnknownPeer) { - return stakedSendErr - } + // Peer is not a staked actor. + if errors.Is(stakedActorSendErr, typesP2P.ErrUnknownPeer) { + m.logger.Warn(). + Str("address", addr.String()). + Msgf("attempting to send to unstaked actor") - if errors.Is(stakedSendErr, typesP2P.ErrUnknownPeer) { - return unstakedSendErr + return m.unstakedActorRouter.Send(poktEnvelopeBz, addr) } - - return multierr.Append(unstakedSendErr, stakedSendErr) + return nil } // TECHDEBT(#348): Define what the node identity is throughout the codebase From b8f9a1ae6325b182b6b3e84bddbaa4cb572ea9f2 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 13:37:50 +0200 Subject: [PATCH 30/59] chore: improve variable naming Co-authored-by: @Olshansk --- p2p/background/router.go | 16 ++++++++-------- p2p/utils_test.go | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/p2p/background/router.go b/p2p/background/router.go index ade9594ea..0244dfca2 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -47,9 +47,9 @@ type backgroundRouter struct { // according to options. // (see: https://pkg.go.dev/github.com/libp2p/go-libp2p#section-readme) host libp2pHost.Host - // cancelCtx is the cancel function for the context which is provided to the - // gossipsub subscription. Used to terminate the `readSubscription()` go routine. - cancelCtx context.CancelFunc + // cancelReadSubscription is the cancel function for the context which is + // monitored in the `#readSubscription()` go routine. Call to terminate it. + cancelReadSubscription context.CancelFunc // Fields below are assigned during creation via `#setupDependencies()`. @@ -87,10 +87,10 @@ func (*backgroundRouter) Create(bus modules.Bus, cfg *config.BackgroundConfig) ( ctx, cancel := context.WithCancel(context.TODO()) rtr := &backgroundRouter{ - logger: bgRouterLogger, - handler: cfg.Handler, - host: cfg.Host, - cancelCtx: cancel, + logger: bgRouterLogger, + handler: cfg.Handler, + host: cfg.Host, + cancelReadSubscription: cancel, } rtr.SetBus(bus) @@ -112,7 +112,7 @@ func (*backgroundRouter) Create(bus modules.Bus, cfg *config.BackgroundConfig) ( func (rtr *backgroundRouter) Close() error { rtr.logger.Debug().Msg("closing background router") - rtr.cancelCtx() + rtr.cancelReadSubscription() rtr.subscription.Cancel() var topicCloseErr error diff --git a/p2p/utils_test.go b/p2p/utils_test.go index b9f89fcf2..7f5cd8b7f 100644 --- a/p2p/utils_test.go +++ b/p2p/utils_test.go @@ -110,9 +110,9 @@ func createP2PModules(t *testing.T, busMocks []*mockModules.MockBus, netMock moc peerIDs := setupMockNetPeers(t, netMock, len(busMocks)) ctrl := gomock.NewController(t) - backgroundRouterMock := mock_types.NewMockRouter(ctrl) - backgroundRouterMock.EXPECT().Broadcast(gomock.Any()).Times(1) - backgroundRouterMock.EXPECT().Close().Times(len(busMocks)) + noopBackgroundRouterMock := mock_types.NewMockRouter(ctrl) + noopBackgroundRouterMock.EXPECT().Broadcast(gomock.Any()).Times(1) + noopBackgroundRouterMock.EXPECT().Close().Times(len(busMocks)) p2pModules = make(map[string]*p2pModule, len(busMocks)) for i := range busMocks { @@ -121,7 +121,7 @@ func createP2PModules(t *testing.T, busMocks []*mockModules.MockBus, netMock moc busMocks[i], WithHost(host), // mock background router to prevent background message propagation. - WithUnstakedActorRouter(backgroundRouterMock), + WithUnstakedActorRouter(noopBackgroundRouterMock), ) require.NoError(t, err) p2pModules[validatorId(i+1)] = p2pMod.(*p2pModule) From de63d6d6270f2f980c0bd313e5415c6e4a728fd9 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 13:38:24 +0200 Subject: [PATCH 31/59] chore: improve comments --- p2p/testutil.go | 6 ++++++ p2p/types/proto/background.proto | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/p2p/testutil.go b/p2p/testutil.go index ee922c634..50b472118 100644 --- a/p2p/testutil.go +++ b/p2p/testutil.go @@ -21,6 +21,10 @@ func WithHost(host libp2pHost.Host) modules.ModuleOption { } } +// WithUnstakedActorRouter assigns the given router to the P2P modules +// `#unstakedActor` field, used to communicate between unstaked actors +// and the rest of the network, plus as a redundancy to the staked actor +// router when broadcasting. func WithUnstakedActorRouter(router typesP2P.Router) modules.ModuleOption { return func(m modules.InitializableModule) { mod, ok := m.(*p2pModule) @@ -30,6 +34,8 @@ func WithUnstakedActorRouter(router typesP2P.Router) modules.ModuleOption { } } +// WithStakedActorRouter assigns the given router to the P2P modules' +// `#stakedActor` field, exclusively used to communicate between staked actors. func WithStakedActorRouter(router typesP2P.Router) modules.ModuleOption { return func(m modules.InitializableModule) { mod, ok := m.(*p2pModule) diff --git a/p2p/types/proto/background.proto b/p2p/types/proto/background.proto index d5046ff71..da1e138e7 100644 --- a/p2p/types/proto/background.proto +++ b/p2p/types/proto/background.proto @@ -3,6 +3,11 @@ package background; option go_package = "github.com/pokt-network/pocket/p2p/types"; +// BackgroundMessage is intended to be used with the background router for +// communication with unstaked actors. For unstaked actors, this is the only +// means of communication with the network. For staked actors, this functions +// as a redundancy for broadcast propagation (in addition to the staked actor +// router broadcast message - i.e. `RainTreeMessage`). message BackgroundMessage { bytes data = 1; } From 1282e1abc4a6af581fd1395ce983c9bca9658e88 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 13:38:34 +0200 Subject: [PATCH 32/59] chore: improve debug logging --- p2p/testutil.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/p2p/testutil.go b/p2p/testutil.go index 50b472118..808f2d497 100644 --- a/p2p/testutil.go +++ b/p2p/testutil.go @@ -30,6 +30,7 @@ func WithUnstakedActorRouter(router typesP2P.Router) modules.ModuleOption { mod, ok := m.(*p2pModule) if ok { mod.unstakedActorRouter = router + mod.logger.Debug().Msg("using unstaked actor router provided via `WithUnstakeActorRouter`") } } } @@ -41,6 +42,7 @@ func WithStakedActorRouter(router typesP2P.Router) modules.ModuleOption { mod, ok := m.(*p2pModule) if ok { mod.stakedActorRouter = router + mod.logger.Debug().Msg("using staked actor router provided via `WithStakeActorRouter`") } } } From 5793b7fc86597c66ed228379c5b9e7992f45a63a Mon Sep 17 00:00:00 2001 From: Bryan White Date: Wed, 28 Jun 2023 13:45:54 +0200 Subject: [PATCH 33/59] chore: return early Co-authored-by: @Olshansk --- p2p/event_handler.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/p2p/event_handler.go b/p2p/event_handler.go index 792737268..0c5743d18 100644 --- a/p2p/event_handler.go +++ b/p2p/event_handler.go @@ -26,23 +26,25 @@ func (m *p2pModule) HandleEvent(event *anypb.Any) error { if isStaked, err := m.isStakedActor(); err != nil { return err - } else if isStaked { - oldPeerList := m.stakedActorRouter.GetPeerstore().GetPeerList() - updatedPeerstore, err := m.pstoreProvider.GetStakedPeerstoreAtHeight(consensusNewHeightEvent.Height) - if err != nil { - return err - } + } else if !isStaked { + return nil + } - added, removed := oldPeerList.Delta(updatedPeerstore.GetPeerList()) - for _, add := range added { - if err := m.stakedActorRouter.AddPeer(add); err != nil { - return err - } + oldPeerList := m.stakedActorRouter.GetPeerstore().GetPeerList() + updatedPeerstore, err := m.pstoreProvider.GetStakedPeerstoreAtHeight(consensusNewHeightEvent.Height) + if err != nil { + return err + } + + added, removed := oldPeerList.Delta(updatedPeerstore.GetPeerList()) + for _, add := range added { + if err := m.stakedActorRouter.AddPeer(add); err != nil { + return err } - for _, rm := range removed { - if err := m.stakedActorRouter.RemovePeer(rm); err != nil { - return err - } + } + for _, rm := range removed { + if err := m.stakedActorRouter.RemovePeer(rm); err != nil { + return err } } From 95a3948752a1d6b8e9f3b9bdd8e316acd1020392 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 29 Jun 2023 08:51:18 +0200 Subject: [PATCH 34/59] chore: add TECHDEBT comment --- p2p/module.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/p2p/module.go b/p2p/module.go index a05099a77..01e475d1b 100644 --- a/p2p/module.go +++ b/p2p/module.go @@ -333,6 +333,9 @@ func (m *p2pModule) setupCurrentHeightProvider() error { m.logger.Debug().Msg("setupCurrentHeightProvider") currentHeightProviderModule, err := m.GetBus().GetModulesRegistry().GetModule(current_height_provider.ModuleName) if err != nil { + // TECHDEBT(#810): add a `consensusCurrentHeightProvider` submodule to wrap + // the consensus module usage (similar to how `persistencePeerstoreProvider` + // wraps persistence). currentHeightProviderModule = m.GetBus().GetConsensusModule() } From fe42ab3dd7f28e35fe767ca3d79ea24c32b58417 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 29 Jun 2023 09:19:41 +0200 Subject: [PATCH 35/59] test: fix raintree message target test --- p2p/raintree/peers_manager_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/p2p/raintree/peers_manager_test.go b/p2p/raintree/peers_manager_test.go index e3c80d541..9e3fa2502 100644 --- a/p2p/raintree/peers_manager_test.go +++ b/p2p/raintree/peers_manager_test.go @@ -9,8 +9,11 @@ import ( "github.com/foxcpp/go-mockdns" "github.com/golang/mock/gomock" + libp2pPeer "github.com/libp2p/go-libp2p/core/peer" "github.com/libp2p/go-libp2p/p2p/host/peerstore/pstoremem" mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" + "github.com/stretchr/testify/require" + "github.com/pokt-network/pocket/internal/testutil" "github.com/pokt-network/pocket/p2p/config" typesP2P "github.com/pokt-network/pocket/p2p/types" @@ -18,7 +21,6 @@ import ( "github.com/pokt-network/pocket/runtime/configs" cryptoPocket "github.com/pokt-network/pocket/shared/crypto" mockModules "github.com/pokt-network/pocket/shared/modules/mocks" - "github.com/stretchr/testify/require" ) const ( @@ -290,7 +292,8 @@ func testRainTreeMessageTargets(t *testing.T, expectedMsgProp *ExpectedRainTreeM hostMock := mocksP2P.NewMockHost(ctrl) hostMock.EXPECT().Peerstore().Return(libp2pPStore).AnyTimes() - hostMock.EXPECT().SetStreamHandler(gomock.Any(), gomock.Any()).Times(1) + hostMock.EXPECT().SetStreamHandler(gomock.Any(), gomock.Any()).AnyTimes() + hostMock.EXPECT().ID().Return(libp2pPeer.ID("")).AnyTimes() rtCfg := &config.RainTreeConfig{ Host: hostMock, From 60cd2bd1abe26b4eb892be173c845cf346284706 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 29 Jun 2023 11:17:01 +0200 Subject: [PATCH 36/59] docs: update P2P readme --- p2p/README.md | 484 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 406 insertions(+), 78 deletions(-) diff --git a/p2p/README.md b/p2p/README.md index 8263cd563..6a3cdd7d7 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -34,14 +34,11 @@ A structured "gossip" protocol (and implementation) which uses the raintree algo A "gossip" protocol (implementation TBD) which facilitates "gossip" to all P2P participants, including non-staked actors (e.g. full-nodes). -## Interface +## Interface & Integration -This module aims to implement the interface specified in `pocket/shared/modules/p2p_module.go` using the specification above. - -## Implementation - -### P2P Module Architecture +This module aims to implement the interface specified in [`pocket/shared/modules/p2p_module.go`](../shared/modules/p2p_module.go). +_(TODO: diagram legend)_ ```mermaid flowchart TD subgraph P2P["P2P Module"] @@ -67,105 +64,431 @@ flowchart TD class PN pocket_network ``` -`Routers` is where [RainTree](https://github.com/pokt-network/pocket/files/9853354/raintree.pdf) (or the simpler basic approach) is implemented. See `raintree/router.go` for the specific implementation of RainTree, but please refer to the [specifications](https://github.com/pokt-network/pocket-network-protocol/tree/main/p2p) for more details. +`Routers` is where [RainTree](https://github.com/pokt-network/pocket/files/9853354/raintree.pdf) is implemented. +See [`raintree/router.go`](./raintree/router.go) for the specific implementation of RainTree, but please refer to the [specifications](https://github.com/pokt-network/pocket-network-protocol/tree/main/p2p) for more details. -### Raintree Router Architecture +## Module Architecture -_DISCUSS(team): If you feel this needs a diagram, please reach out to the team for additional details._ -_TODO(olshansky, BenVan): Link to RainTree visualizations once it is complete._ +### Legends +```mermaid +flowchart +subgraph Legend + m[[`Method`]] + c[Component] + + m -- "unconditional usage" --> c + m -. "conditional usage" .-> c + m -. "ignored" .-x c +end +``` + +```mermaid +classDiagram +class ConcreteType { + +ExportedField + -unexportedField + +ExportedMethod(argType) returnType + -unexportedMethod() +} + +class InterfaceType { + <> + +Method(argType) (returnType1, returnType2) +} + +ConcreteType --|> InterfaceType : Interface realization + +ConcreteType --> OtherType : Direct usage +ConcreteType --o OtherType : Composition +ConcreteType --* OtherType : Aggregatation +ConcreteType ..* "(cardinality)" OtherType : Indirect (via interface) +``` + +### P2P Module / Router Decoupling -### Message Propagation +The P2P module encapsulates the `RaiTreeRouter` and `BackgroundRouter` submodules. +The P2P module internally refers to these as the `stakedActorRouter` and `unstakedActorRouter`, respectively. -Given `Local P2P Module` has a message that it needs to propagate: +Depending on the necessary routing scheme (unicast / broadcast) and whether the peers involved are staked actors, a node will use one or both of these routers. -
    -
  • 1a. Raintree Router selects targets from the Pokt Peerstore, which only includes staked actors
    • -
  • 1b. Background Router selects targets from the libp2p Peerstore, which includes all P2P participants
    • -
  • 2. Libp2p Host manages opening and closing streams to targeted peers
    • -
  • 3. Remote P2P module's (i.e. receiver's) handleStream is called (having been registered via setStreamHandler())
    • -
  • 4a. handleStream propagates message via Raintree Router
    • -
  • 4b. handleStream propagates message via Background Router
    • -
  • 5a. Repeat step 1a from Remote P2P Module's perspective targeting its next peers
    • -
  • 5b. Repeat step 1b from Remote P2P Module's perspective targeting its next peers
    • -
+**Unicast** + +| Sender | Receiver | Router | Example Usage | +|----------------|----------------|-----------------|------------------------------------------------------| +| Staked Actor | Staked Actor | Raintree only | Consensus (state sync) messages (to validators only) | +| Unstaked Actor | Staked Actor | Background only | Consensus (state sync) messages (to validators only) | +| Unstaked Actor | Unstaked Actor | Background only | Debug messages (debug CLI) | + +**Broadcast** + +| Broadcaster | Receiver | Router | Example Usage | +|----------------|----------------|-----------------------|--------------------------------------------| +| Staked Actor | Staked Actor | Raintree + Background | Utility tx messages | +| Unstaked Actor | Staked Actor | Background only | Utility tx messages (gossipsub redundancy) | +| Unstaked Actor | Unstaked Actor | Background only | Utility tx messages | + +Both router submodule implementations embed a `UnicastRouter` which enables them to send and receive messages directly to/from a single peer. + +**Class Diagram** ```mermaid -flowchart TD - subgraph lMod[Local P2P Module] - subgraph lHost[Libp2p `Host`] +classDiagram + class p2pModule { + -stakedActorRouter Router + -unstakedActorRouter Router + -handlePocketEnvelope([]byte) error + } + + class P2PModule { + <> + GetAddress() (Address, error) + HandleEvent(*anypb.Any) error + Send([]byte, Address) error + Broadcast([]byte) error + } + p2pModule --|> P2PModule + + class RainTreeRouter { + UnicastRouter + -handler MessageHandler + +Broadcast([]byte) error + -handleRainTreeMsg([]byte) error + } + + class BackgroundRouter { + UnicastRouter + -handler MessageHandler + +Broadcast([]byte) error + -handleBackgroundMsg([]byte) error + -readSubscription(subscription *pubsub.Subscription) + } + + class UnicastRouter { + -messageHandler MessageHandler + -peerHandler PeerHandler + +Send([]byte, Address) error + -handleStream(libp2pNetwork.Stream) + -readStream(libp2pNetwork.Stream) + } + RainTreeRouter --* UnicastRouter : (embedded) + BackgroundRouter --* UnicastRouter : (embedded) + + p2pModule --o "2" Router + p2pModule ..* RainTreeRouter : (`stakedActorRouter`) + p2pModule ..* BackgroundRouter : (`unstakedActorRouter`) + + class Router { + <> + +Send([]byte, Address) error + +Broadcast([]byte) error + } + BackgroundRouter --|> Router + RainTreeRouter --|> Router +``` + +### Message Propagation & Handling + +**Unicast** + +```mermaid +flowchart + subgraph lp2p["Local P2P Module (outgoing)"] + lps[[`Send`]] + lps -. "(iff local & remote peer are staked)" ..-> lrtu + lps -. "(if local or remote peer are not staked)" .-> lbgu + + lbgu -- "opens stream\nto target peer" ---> lhost + + lhost[Libp2p Host] + + subgraph lrt[RainTree Router] + subgraph lRTPS[Raintree Peerstore] + lStakedPS([staked actors only]) + end + + lrtu[UnicastRouter] + + lrtu -- "network address lookup" --> lRTPS + end + + lrtu -- "opens a stream\nto target peer" ---> lhost + + subgraph lbg[Background Router] + lbgu[UnicastRouter] + subgraph lBGPS[Background Peerstore] + lNetPS([all P2P participants]) + end + + lbgu -- "network address lookup" --> lBGPS + end + end + + subgraph rp2p["Remote P2P Module (incoming)"] + rhost[Libp2p Host] + + subgraph rrt[RainTree Router] + rrth[[`RainTreeMessage` Handler]] + rrtu[UnicastRouter] + end + + subgraph rbg[Background Router] + rbgh[[`BackgroundMessage` Handler]] + rbgu[UnicastRouter] + rbgu --> rbgh + end + + rp2ph[[`PocketEnvelope` Handler]] + rbus[bus] + rhost -. "new stream" .-> rrtu + rhost -- "new subscription message" --> rbgu + rrtu --> rrth + + rnd[Nonce Deduper] + rp2ph -- "deduplicate msg mempool" --> rnd end - subgraph lRT[Raintree Router] + + + rp2ph -. "(iff not duplicate msg)\npublish event" .-> rbus + + rrth --> rp2ph + rbgh --> rp2ph + + lhost --> rhost +``` + +**Broadcast** + +```mermaid +flowchart + subgraph lp2p["Local P2P Module (outgoing)"] + lpb[[`Broadcast`]] + lpb -. "(iff local & remote peer are staked)" ..-> lrtu + lpb -- "(always)" --> lbggt + + lbggt -- "msg published\n(gossipsub protocol)" ---> lhost + + lhost[Libp2p Host] + + subgraph lrt[RainTree Router] subgraph lRTPS[Raintree Peerstore] lStakedPS([staked actors only]) end - - subgraph lPM[PeerManager] - end - lPM --> lRTPS + + lrtu[UnicastRouter] + + lrtu -- "network address lookup" --> lRTPS end + + lrtu -- "opens a stream\nto target peer" ---> lhost - subgraph lBG[Background Router] + subgraph lbg[Background Router] + lbggt[Gossipsub Topic] subgraph lBGPS[Background Peerstore] lNetPS([all P2P participants]) end + + lbggt -- "network address lookup" --> lBGPS + end + end - subgraph lGossipSub[GossipSub] - end + subgraph rp2p["Remote P2P Module (incoming)"] + rhost[Libp2p Host] - subgraph lDHT[Kademlia DHT] - end + subgraph rrt[RainTree Router] + rrth[[`RainTreeMessage` Handler]] + rrtu[UnicastRouter] + end - lGossipSub --> lBGPS - lDHT --> lBGPS + subgraph rbg[Background Router] + rbgh[[`BackgroundMessage` Handler]] + rbgg[Gossipsub Subscription] + rbggt[Gossipsub Topic] + rbgg --> rbgh + rbgh -- "(background msg\npropagation cont.)" ---> rbggt end - lRT --1a--> lHost - lBG --1b--> lHost + rp2ph[[`PocketEnvelope` Handler]] + rbus[bus] + rhost -. "new stream" ..-> rrtu + rhost -- "new subscription message" --> rbgg + rbggt -- "(background msg\npropagation cont.)" --> rhost + rrtu --> rrth + rrth -. "(iff level > 0)\n(raintree msg\npropagation cont.)" .-> rrtu + rrtu -- "(raintree msg\npropagation cont.)" --> rhost + + rnd[Nonce Deduper] + rp2ph -- "deduplicate msg mempool" --> rnd end -subgraph rMod[Remote P2P Module] -subgraph rHost[Libp2p `Host`] -end -subgraph rRT[Raintree Router] -subgraph rPS[Raintree Peerstore] -rStakedPS([staked actors only]) -end -subgraph rPM[PeerManager] -end + rp2ph -. "(iff not duplicate msg)\npublish event" .-> rbus -rPM --> rStakedPS -end + rrth --> rp2ph + rbgh --> rp2ph -subgraph rBG[Background Router] -subgraph rBGPS[Background Peerstore] -rNetPS([all P2P participants]) -end + lhost --> rhost +``` -subgraph rGossipSub[GossipSub] -end +### Message Deduplication -subgraph rDHT[Kademlia DHT] -end +Messages MUST be deduplicated before broadcasting their respective event over the bus since it is expected that nodes will receive duplicate messages (for multiple reasons). -rGossipSub --> rBGPS -rDHT --> rBGPS -end +The responsibility of deduplication is encapsulated by the P2P module, As such duplicate messages may come from multiple routers in some of these scenarios. -rHost -. "3 (setStreamHandler())" .-> hs[[handleStream]] +```mermaid +classDiagram + class RainTreeMessage { + <> + +Level uint32 + +Data []byte + } + + class BackgroundMessage { + <> + +Data []byte + } + + class PocketEnvelope { + <> + +Content *anypb.Any + +Nonce uint64 + } + + RainTreeMessage --* PocketEnvelope : serialized as `Data` + BackgroundMessage --* PocketEnvelope : serialized as `Data` + + + class p2pModule { + -handlePocketEnvelope([]byte) error + } + + class P2PModule { + <> + GetAddress() (Address, error) + HandleEvent(*anypb.Any) error + Send([]byte, address Address) error + Broadcast([]byte) error + } + p2pModule --|> P2PModule + + class RainTreeRouter { + UnicastRouter + -handler MessageHandler + +Broadcast([]byte) error + -handleRainTreeMsg([]byte) error + } + + class NonceDeduper { + Push(Nonce) error + Contains(Nonce) bool + } + + class Bus { + <> + PublishEventToBus(PocketEnvelope) + GetBusEvent() PocketEnvelope + } + p2pModule --> Bus + + class BackgroundRouter { + UnicastRouter + -handler MessageHandler + +Broadcast([]byte) error + -handleBackgroundMsg([]byte) error + -readSubscription(subscription *pubsub.Subscription) + } + + class UnicastRouter { + -messageHandler MessageHandler + -peerHandler PeerHandler + +Send([]byte, address Address) error + -handleStream(stream libp2pNetwork.Stream) + -readStream(stream libp2pNetwork.Stream) + } + RainTreeRouter --* UnicastRouter : (embedded) + BackgroundRouter --* UnicastRouter : (embedded) + + p2pModule ..* RainTreeRouter + RainTreeRouter --o RainTreeMessage + + p2pModule ..* BackgroundRouter + BackgroundRouter --o BackgroundMessage + + p2pModule --o PocketEnvelope + p2pModule --* NonceDeduper +``` -hs --4a--> rRT -hs --4b--> rBG -rBG --"5a (cont. propagation)"--> rHost -linkStyle 11 stroke:#ff3 -rRT --"5b (cont. propagation)"--> rHost -linkStyle 12 stroke:#ff3 -end +### Peer Discovery +Peer discovery involves pairing peer IDs to their network addresses (multiaddr). +This pairing always has an associated TTL (time-to-live), near the end of which it must +be refreshed. + +In the background gossip overlay network (`backgroundRouter`), peers will re-advertise themselves 7/8th through their TTL. +This refreshes the libp2p peerstore automatically. + +In the raintree gossip overlay network (`raintreeRouter`), the libp2p peerstore is **NOT** currently refreshed _(TODO: [#859](https://github.com/pokt-network/network/isues/859))_. + +```mermaid +flowchart TD + subgraph bus + end + + subgraph pers[Persistence Module] + end + + subgraph cons[Consensus Module] + end -lHost --2--> rHost + cons -- "(staked actor set changed)\npublish event" --> bus + bus --> rPM + rPM -- "get staked actors\nat current height" --> pers + + subgraph p2p["P2P Module"] + host[Libp2p Host] + host -- "incoming\nraintree message" --> rtu + host -- "incoming\nbackground message" --> bgu + host -- "incoming\ntopic message" --> bgr + host -- "DHT peer discovery" --> rDHT + + subgraph rt[RainTree Router] + subgraph rPS[Raintree Peerstore] + rStakedPS([staked actors only]) + end + + subgraph rPM[PeerManager] + end + + rtu[UnicastRouter] + + rPM -- "synchronize\n(add/remove)" --> rPS + rtu -. "(no discovery)" .-x rPS + end + + subgraph bg[Background Router] + subgraph rBGPS[Background Peerstore] + rNetPS([all P2P participants]) + end + + subgraph bgr[GossipSub Topic\nSubscription] + end + + subgraph rDHT[Kademlia DHT] + end + + bgu -- "add if new" --> rBGPS + bgr -- "add if new" --> rBGPS + rDHT -- "continuous import" --> rBGPS + + bgu[UnicastRouter] + end + + end ``` -The `Network Module` is where [RainTree](https://github.com/pokt-network/pocket/files/9853354/raintree.pdf) (or the simpler basic approach) is implemented. See `raintree/network.go` for the specific implementation of RainTree, but please refer to the [specifications](https://github.com/pokt-network/pocket-network-protocol/tree/main/p2p) for more details. +### Raintree Router Architecture + +_DISCUSS(team): If you feel this needs a diagram, please reach out to the team for additional details._ +_TODO(olshansky, BenVan): Link to RainTree visualizations once it is complete._ ### Code Organization @@ -177,6 +500,8 @@ p2p │   └── router_test.go # `BackgroundRouter` functional tests ├── bootstrap.go # `p2pModule` bootstrap related method(s) ├── CHANGELOG.md +├── config +│   └── config.go ├── event_handler.go ├── module.go # `p2pModule` definition ├── module_raintree_test.go # `p2pModule` & `RainTreeRouter` functional tests (routing) @@ -189,21 +514,18 @@ p2p │   ├── peerstore_provider │   └── providers.go ├── raintree -│   ├── nonce_deduper.go -│   ├── nonce_deduper_test.go │   ├── peers_manager.go # `rainTreePeersManager` implementation of `PeersManager` interface │   ├── peers_manager_test.go │   ├── peerstore_utils.go # Raintree routing helpers │   ├── router.go # `RainTreeRouter` implementation of `Router` interface │   ├── router_test.go # `RainTreeRouter` functional tests │   ├── target.go # `target` definition -│   ├── types -│   │   └── proto -│   │   └── raintree.proto +│   ├── testutil.go │   └── utils_test.go -├── README.md +├── testutil.go ├── transport_encryption_test.go # Libp2p transport security integration test ├── types +│   ├── background.pb.go │   ├── errors.go │   ├── libp2p_mocks.go │   ├── mocks @@ -213,12 +535,18 @@ p2p │   ├── peerstore.go # `Peerstore` interface & `PeerAddrMap` implementation definitions │   ├── peers_view.go # `PeersView` interface & `sortedPeersView` implementation definitions │   ├── peers_view_test.go +│   ├── proto │   ├── raintree.pb.go │   └── router.go # `Router` interface definition +├── unicast +│   ├── logging.go +│   ├── router.go +│   └── testutil.go ├── utils -│   ├── config.go # `RouterConfig` definition │   ├── host.go # Helpers for working with libp2p hosts │   ├── logging.go # Helpers for logging +│   ├── nonce_deduper.go +│   ├── nonce_deduper_test.go │   ├── peer_conversion.go # Helpers for converting between "native" and libp2p peer representations │   ├── url_conversion.go # Helpers for converting between "native" and libp2p network address representations │   └── url_conversion_test.go From 8354d7980ee7c6c2b4970541d9215653df295a72 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 29 Jun 2023 15:40:33 +0200 Subject: [PATCH 37/59] docs: update table of contents --- p2p/README.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/p2p/README.md b/p2p/README.md index 6a3cdd7d7..cc5a4a4fe 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -5,11 +5,13 @@ This document is meant to be a supplement to the living specification of [1.0 Po ## Table of Contents - [Definitions](#definitions) -- [Interface](#interface) -- [Implementation](#implementation) - - [Code Architecture - P2P Module](#code-architecture---p2p-module) - - [Code Architecture - Network Module](#code-architecture---network-module) - - [Code Organization](#code-organization) +- [Interface & Integration](#interface--integration) +- [Module Architecture](#module-architecture) + - [P2P Module / Router Decoupling](#p2p-module--router-decoupling) + - [Message Propagation & Handling](#message-propagation--handling) + - [Message Deduplication](#message-deduplication) + - [Peer Discovery](#peer-discovery) + - [Code Organization](#code-organization) - [Testing](#testing) - [Running Unit Tests](#running-unit-tests) - [RainTree testing framework](#raintree-testing-framework) From f7b0202f3e30ddeadb2a6dcb80f21a00a9bb2f0f Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 29 Jun 2023 16:46:14 +0200 Subject: [PATCH 38/59] docs: tweak P2P readme --- p2p/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/README.md b/p2p/README.md index cc5a4a4fe..ccd693d96 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -119,7 +119,7 @@ Depending on the necessary routing scheme (unicast / broadcast) and whether the |----------------|----------------|-----------------|------------------------------------------------------| | Staked Actor | Staked Actor | Raintree only | Consensus (state sync) messages (to validators only) | | Unstaked Actor | Staked Actor | Background only | Consensus (state sync) messages (to validators only) | -| Unstaked Actor | Unstaked Actor | Background only | Debug messages (debug CLI) | +| Unstaked Actor | Unstaked Actor | Background only | Consensus (state sync) & Debug (CLI) messages | **Broadcast** From d2f33a462a838f199366afc2553f7d7f41b9c0b4 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 29 Jun 2023 16:46:24 +0200 Subject: [PATCH 39/59] chore: add godoc comment Co-authored-by: Daniel Olshansky --- p2p/background/router.go | 1 + 1 file changed, 1 insertion(+) diff --git a/p2p/background/router.go b/p2p/background/router.go index 0244dfca2..933d9445b 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -197,6 +197,7 @@ func (rtr *backgroundRouter) RemovePeer(peer typesP2P.Peer) error { return rtr.pstore.RemovePeer(peer.GetAddress()) } +// setupUnicastRouter configures and assigns `rtr.UnicastRouter`. func (rtr *backgroundRouter) setupUnicastRouter() error { unicastRouterCfg := config.UnicastRouterConfig{ Logger: rtr.logger, From b527d910ede3e9ecae2b944bc4aa779f8b08aeb2 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 29 Jun 2023 16:50:59 +0200 Subject: [PATCH 40/59] chore: remove warning log --- p2p/background/router.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/p2p/background/router.go b/p2p/background/router.go index 933d9445b..043052840 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -142,8 +142,6 @@ func (rtr *backgroundRouter) Broadcast(pocketEnvelopeBz []byte) error { // Send implements the respective `typesP2P.Router` interface method. func (rtr *backgroundRouter) Send(pocketEnvelopeBz []byte, address cryptoPocket.Address) error { - rtr.logger.Warn().Str("address", address.String()).Msg("sending background message to peer") - backgroundMessage := &typesP2P.BackgroundMessage{ Data: pocketEnvelopeBz, } From 73da86c7224932c2a90a2016dc9f9566db45b6fd Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 29 Jun 2023 16:54:54 +0200 Subject: [PATCH 41/59] chore: convert `DISCUSS_THIS_COMMIT` to `TECHDEBT` --- p2p/utils_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/utils_test.go b/p2p/utils_test.go index 7f5cd8b7f..3ec09a8cb 100644 --- a/p2p/utils_test.go +++ b/p2p/utils_test.go @@ -341,7 +341,7 @@ func prepareEventMetricsAgentMock(t *testing.T, valId string, wg *sync.WaitGroup ctrl := gomock.NewController(t) eventMetricsAgentMock := mockModules.NewMockEventMetricsAgent(ctrl) - // DISCUSS_THIS_COMMIT: The number of times each telemetry event is expected + // TECHDEBT: The number of times each telemetry event is expected // (below) is dependent on the number of redundant messages all validators see, // which is a function of the network size. Until this function is derived and // implemented, we cannot predict the number of times each event is expected. From 4602283cbd676952b2a5c20f73758305cc833ce5 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 3 Jul 2023 11:19:30 +0200 Subject: [PATCH 42/59] fix: typo Co-authored-by: d7t --- p2p/background/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/background/router.go b/p2p/background/router.go index 043052840..dca6d57f8 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -342,7 +342,7 @@ func (rtr *backgroundRouter) bootstrap(ctx context.Context) error { } // topicValidator is used in conjunction with libp2p-pubsub's notion of "topic -// validaton". It is usefed for arbitrary and concurrent pre-propagation validation +// validaton". It is used for arbitrary and concurrent pre-propagation validation // of messages. // // (see: https://github.com/libp2p/specs/tree/master/pubsub#topic-validation From 65a8c940e2d66728d036d384bc5118f1efe8515c Mon Sep 17 00:00:00 2001 From: Bryan White Date: Tue, 4 Jul 2023 11:12:29 +0200 Subject: [PATCH 43/59] chore: review suggestion improvements Co-authored-by: Daniel Olshansky --- p2p/README.md | 4 ++-- p2p/background/router.go | 10 +++++++++- p2p/event_handler.go | 2 +- p2p/utils_test.go | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/p2p/README.md b/p2p/README.md index ccd693d96..5c4064f67 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -421,6 +421,7 @@ classDiagram ``` ### Peer Discovery + Peer discovery involves pairing peer IDs to their network addresses (multiaddr). This pairing always has an associated TTL (time-to-live), near the end of which it must be refreshed. @@ -489,8 +490,7 @@ flowchart TD ### Raintree Router Architecture -_DISCUSS(team): If you feel this needs a diagram, please reach out to the team for additional details._ -_TODO(olshansky, BenVan): Link to RainTree visualizations once it is complete._ +_NOTE: If you (the reader) feel this needs a diagram, please reach out to the team for additional details._ ### Code Organization diff --git a/p2p/background/router.go b/p2p/background/router.go index dca6d57f8..19dce46fe 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -49,6 +49,7 @@ type backgroundRouter struct { host libp2pHost.Host // cancelReadSubscription is the cancel function for the context which is // monitored in the `#readSubscription()` go routine. Call to terminate it. + // only one read subscription exists per router at any point in time cancelReadSubscription context.CancelFunc // Fields below are assigned during creation via `#setupDependencies()`. @@ -215,6 +216,7 @@ func (rtr *backgroundRouter) setupUnicastRouter() error { } func (rtr *backgroundRouter) setupDependencies(ctx context.Context, cfg *config.BackgroundConfig) error { + // NB: The order in which the internal components are setup below is important if err := rtr.setupUnicastRouter(); err != nil { return err } @@ -237,7 +239,9 @@ func (rtr *backgroundRouter) setupDependencies(ctx context.Context, cfg *config. if err := rtr.setupPeerstore( ctx, - cfg.PeerstoreProvider, cfg.CurrentHeightProvider); err != nil { + cfg.PeerstoreProvider, + cfg.CurrentHeightProvider, + ); err != nil { return fmt.Errorf("setting up peerstore: %w", err) } return nil @@ -263,6 +267,7 @@ func (rtr *backgroundRouter) setupPeerstore( return nil } +// setupPeerDiscovery sets up the Kademlia Distributed Hash Table (DHT) func (rtr *backgroundRouter) setupPeerDiscovery(ctx context.Context) (err error) { dhtMode := dht.ModeAutoServer // NB: don't act as a bootstrap node in peer discovery in client debug mode @@ -274,6 +279,7 @@ func (rtr *backgroundRouter) setupPeerDiscovery(ctx context.Context) (err error) return err } +// setupPubsub sets up a new gossip sub topic using libp2p func (rtr *backgroundRouter) setupPubsub(ctx context.Context) (err error) { // TECHDEBT(#730): integrate libp2p tracing via `pubsub.WithEventTracer()`. @@ -368,6 +374,8 @@ func (rtr *backgroundRouter) topicValidator(_ context.Context, _ libp2pPeer.ID, return true } +// readSubscription is a while loop for receiving and handling messages from the +// subscription. It is intended to be called as a goroutine. func (rtr *backgroundRouter) readSubscription(ctx context.Context) { for { if err := ctx.Err(); err != nil { diff --git a/p2p/event_handler.go b/p2p/event_handler.go index 0c5743d18..93e7fc2da 100644 --- a/p2p/event_handler.go +++ b/p2p/event_handler.go @@ -27,7 +27,7 @@ func (m *p2pModule) HandleEvent(event *anypb.Any) error { if isStaked, err := m.isStakedActor(); err != nil { return err } else if !isStaked { - return nil + return nil // unstaked actors do not use RainTree and therefore do not need to update this router } oldPeerList := m.stakedActorRouter.GetPeerstore().GetPeerList() diff --git a/p2p/utils_test.go b/p2p/utils_test.go index 3ec09a8cb..cb5d2329d 100644 --- a/p2p/utils_test.go +++ b/p2p/utils_test.go @@ -120,7 +120,7 @@ func createP2PModules(t *testing.T, busMocks []*mockModules.MockBus, netMock moc p2pMod, err := Create( busMocks[i], WithHost(host), - // mock background router to prevent background message propagation. + // mock background router to prevent & ignore background message propagation. WithUnstakedActorRouter(noopBackgroundRouterMock), ) require.NoError(t, err) From 519db25cda1bf046d6932173a9d8cd6dea4f4755 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Tue, 4 Jul 2023 14:51:12 +0200 Subject: [PATCH 44/59] fix: gofmt --- p2p/background/router.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/background/router.go b/p2p/background/router.go index 19dce46fe..48df3ee63 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -240,8 +240,8 @@ func (rtr *backgroundRouter) setupDependencies(ctx context.Context, cfg *config. if err := rtr.setupPeerstore( ctx, cfg.PeerstoreProvider, - cfg.CurrentHeightProvider, - ); err != nil { + cfg.CurrentHeightProvider, + ); err != nil { return fmt.Errorf("setting up peerstore: %w", err) } return nil From 7e7e6e70788b74624fef747c5fb521aac39e9a84 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Tue, 4 Jul 2023 15:09:50 +0200 Subject: [PATCH 45/59] docs: README improvements (review feedback) Co-authored-by: Daniel Olshansky --- p2p/README.md | 14 +++++++++++--- p2p/utils_test.go | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/p2p/README.md b/p2p/README.md index 5c4064f67..68eb0fb3e 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -72,6 +72,8 @@ See [`raintree/router.go`](./raintree/router.go) for the specific implementation ## Module Architecture ### Legends + +_NOTE: the architecture design language is based on [UML](https://www.uml-diagrams.org/) but limited by mermaid's constraints (see: [component](https://www.uml-diagrams.org/component-diagrams.html) and [class](https://www.uml-diagrams.org/class-diagrams-overview.html) diagrams)._ ```mermaid flowchart subgraph Legend @@ -89,8 +91,8 @@ classDiagram class ConcreteType { +ExportedField -unexportedField - +ExportedMethod(argType) returnType - -unexportedMethod() + +ExportedMethod(...args) (...returnTypes) + -unexportedMethod(...args) (...returnTypes) } class InterfaceType { @@ -108,7 +110,7 @@ ConcreteType ..* "(cardinality)" OtherType : Indirect (via interface) ### P2P Module / Router Decoupling -The P2P module encapsulates the `RaiTreeRouter` and `BackgroundRouter` submodules. +The P2P module encapsulates the `RainTreeRouter` and `BackgroundRouter` submodules. The P2P module internally refers to these as the `stakedActorRouter` and `unstakedActorRouter`, respectively. Depending on the necessary routing scheme (unicast / broadcast) and whether the peers involved are staked actors, a node will use one or both of these routers. @@ -337,6 +339,8 @@ Messages MUST be deduplicated before broadcasting their respective event over th The responsibility of deduplication is encapsulated by the P2P module, As such duplicate messages may come from multiple routers in some of these scenarios. +The `NondeDeduper` state is not persisted outside of memory and therefore is cleared during node restarts. + ```mermaid classDiagram class RainTreeMessage { @@ -420,6 +424,10 @@ classDiagram p2pModule --* NonceDeduper ``` +#### Configuration + +The size of the `NonceDeduper` queue is configurable via the `P2PConfig.MaxNonces` field. + ### Peer Discovery Peer discovery involves pairing peer IDs to their network addresses (multiaddr). diff --git a/p2p/utils_test.go b/p2p/utils_test.go index cb5d2329d..3247bfe82 100644 --- a/p2p/utils_test.go +++ b/p2p/utils_test.go @@ -226,7 +226,7 @@ func createMockBus( if readWriteWaitGroup != nil { readWriteWaitGroup.Done() } - }).AnyTimes() // TODO: specific times + }).AnyTimes() // TECHDEBT: assert number of times. Consider `waitForEventsInternal` or similar as in consensus. mockBus.EXPECT().PublishEventToBus(gomock.Any()).AnyTimes() return mockBus } From f3437cb77e2b060af996c6192666752c4f9d1b61 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Tue, 4 Jul 2023 15:49:53 +0200 Subject: [PATCH 46/59] docs: add architecture design language section --- p2p/README.md | 60 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/p2p/README.md b/p2p/README.md index 68eb0fb3e..549798b78 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -71,9 +71,22 @@ See [`raintree/router.go`](./raintree/router.go) for the specific implementation ## Module Architecture +### Architecture Design Language + +The architecture design language expressed in this documentation is based on [UML](https://www.uml-diagrams.org/). +Due to limitations in the current version of mermaid, class diagrams are much more adherant to the UML component specification. +Component diagrams however are much more loosely inspired by their UML counterparts. + +Regardless, each architecture diagram should be accompanied by a legend which covers all the design language features used to provide disambiguation. + +References: +- [Class Diagrams](https://www.uml-diagrams.org/class-diagrams-overview.html) +- [Component Diagrams](https://www.uml-diagrams.org/component-diagrams.html) + + _NOTE: mermaid does not support ports, interfaces, ... in component diagrams ("flowcharts)._ + ### Legends -_NOTE: the architecture design language is based on [UML](https://www.uml-diagrams.org/) but limited by mermaid's constraints (see: [component](https://www.uml-diagrams.org/component-diagrams.html) and [class](https://www.uml-diagrams.org/class-diagrams-overview.html) diagrams)._ ```mermaid flowchart subgraph Legend @@ -91,13 +104,13 @@ classDiagram class ConcreteType { +ExportedField -unexportedField - +ExportedMethod(...args) (...returnTypes) - -unexportedMethod(...args) (...returnTypes) + +ExportedMethod(...argTypes) (...returnTypes) + -unexportedMethod(...argTypes) (...returnTypes) } class InterfaceType { <> - +Method(argType) (returnType1, returnType2) + +Method(...argTypes) (...returnTypes) } ConcreteType --|> InterfaceType : Interface realization @@ -108,6 +121,45 @@ ConcreteType --* OtherType : Aggregatation ConcreteType ..* "(cardinality)" OtherType : Indirect (via interface) ``` +#### Interface Realization + +> Realization is a specialized abstraction relationship between two sets of model elements, one representing a specification (the supplier) and the other represents an implementation of the latter (the client). + +> Realization can be used to model stepwise refinement, optimizations, transformations, templates, model synthesis, framework composition, etc. + +_(see: [UML Realization](https://www.uml-diagrams.org/realization.html))_ + +#### Direct Usage + +> Dependency is a directed relationship which is used to show that some UML element or a set of elements requires, needs or depends on other model elements for specification or implementation. Because of this, dependency is called a supplier - client relationship, where supplier provides something to the client, and thus the client is in some sense incomplete while semantically or structurally dependent on the supplier element(s). Modification of the supplier may impact the client elements. + +> Usage is a dependency in which one named element (client) requires another named element (supplier) for its full definition or implementation. + +_(see: [UML Dependency](https://www.uml-diagrams.org/dependency.html))_ + +#### Composition + +> A "strong" form of aggregation + +> If a composite (whole) is deleted, all of its composite parts are "normally" deleted with it. + +_(see: [UML Shared composition](https://www.uml-diagrams.org/composition.html))_ + +#### Aggregation + +> A "weak" form of composition + +> Shared part could be included in several composites, and if some or all of the composites are deleted, shared part may still exist. + +_(see: [UML Shared aggregation](https://www.uml-diagrams.org/aggregation.html))_ + +#### Cardinality + +Cardinality indicates the number or range of simultaneous instances of the classifier at the "cardinality-side" association end that are associated with the classifier at the other end of the given association type. + +_(see: [UML Association](https://www.uml-diagrams.org/association.html#association-end))_ + + ### P2P Module / Router Decoupling The P2P module encapsulates the `RainTreeRouter` and `BackgroundRouter` submodules. From 4f879217cbd947cb98258e447397b7cc8f063884 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Tue, 4 Jul 2023 16:44:53 +0200 Subject: [PATCH 47/59] chore: background router comment and var name cleanup --- p2p/background/router.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/p2p/background/router.go b/p2p/background/router.go index 48df3ee63..3482bcc44 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -319,8 +319,8 @@ func (rtr *backgroundRouter) setupSubscription() (err error) { } func (rtr *backgroundRouter) bootstrap(ctx context.Context) error { - // CONSIDERATION: add `GetPeers` method to `PeerstoreProvider` interface - // to avoid this loop. + // CONSIDERATION: add `GetPeers` method, which returns a map, + // to the `PeerstoreProvider` interface to simplify this loop. for _, peer := range rtr.pstore.GetPeerList() { if err := utils.AddPeerToLibp2pHost(rtr.host, peer); err != nil { return err @@ -362,12 +362,13 @@ func (rtr *backgroundRouter) topicValidator(_ context.Context, _ libp2pPeer.ID, } if backgroundMsg.Data == nil { + rtr.logger.Debug().Msg("no data in Background message") return false } - networkMessage := messaging.PocketEnvelope{} - if err := proto.Unmarshal(backgroundMsg.Data, &networkMessage); err != nil { - rtr.logger.Error().Err(err).Msg("Error decoding network message") + poktEnvelope := messaging.PocketEnvelope{} + if err := proto.Unmarshal(backgroundMsg.Data, &poktEnvelope); err != nil { + rtr.logger.Error().Err(err).Msg("Error decoding Background message") return false } From c113a362b2fc5977b079088af777201a569e1db0 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 6 Jul 2023 09:51:52 +0200 Subject: [PATCH 48/59] chore: review feedback improvements --- p2p/module.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/module.go b/p2p/module.go index 01e475d1b..e4479c080 100644 --- a/p2p/module.go +++ b/p2p/module.go @@ -212,12 +212,12 @@ func (m *p2pModule) Broadcast(msg *anypb.Any) error { if isStaked { if m.stakedActorRouter == nil { - return fmt.Errorf("staked actor router not started") + return fmt.Errorf("broadcasting: staked actor router not started") } } if m.unstakedActorRouter == nil { - return fmt.Errorf("unstaked actor router not started") + return fmt.Errorf("broadcasting: unstaked actor router not started") } poktEnvelope := &messaging.PocketEnvelope{ From 3356f63e129767535b99ce19f891716042294428 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 6 Jul 2023 09:52:38 +0200 Subject: [PATCH 49/59] chore: add issue # to TECHDEBT comment --- p2p/utils_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/utils_test.go b/p2p/utils_test.go index 3247bfe82..26ddbeb80 100644 --- a/p2p/utils_test.go +++ b/p2p/utils_test.go @@ -341,7 +341,7 @@ func prepareEventMetricsAgentMock(t *testing.T, valId string, wg *sync.WaitGroup ctrl := gomock.NewController(t) eventMetricsAgentMock := mockModules.NewMockEventMetricsAgent(ctrl) - // TECHDEBT: The number of times each telemetry event is expected + // TECHDEBT(#886): The number of times each telemetry event is expected // (below) is dependent on the number of redundant messages all validators see, // which is a function of the network size. Until this function is derived and // implemented, we cannot predict the number of times each event is expected. From 157ecb6303f9184b738fb63a8115538ad397d78b Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 6 Jul 2023 09:53:06 +0200 Subject: [PATCH 50/59] docs: update TOC --- p2p/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/p2p/README.md b/p2p/README.md index 549798b78..20f6232cf 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -7,6 +7,8 @@ This document is meant to be a supplement to the living specification of [1.0 Po - [Definitions](#definitions) - [Interface & Integration](#interface--integration) - [Module Architecture](#module-architecture) + - [Architecture Design Language](#architecture-design-language) + - [Legends](#legends) - [P2P Module / Router Decoupling](#p2p-module--router-decoupling) - [Message Propagation & Handling](#message-propagation--handling) - [Message Deduplication](#message-deduplication) From 30cf145bafa47265ce394b87775c48369e839156 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 6 Jul 2023 09:53:51 +0200 Subject: [PATCH 51/59] chore: add TODO README --- p2p/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/p2p/README.md b/p2p/README.md index 20f6232cf..93423393a 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -73,6 +73,8 @@ See [`raintree/router.go`](./raintree/router.go) for the specific implementation ## Module Architecture +_(TODO: move "arch. design lang." & "legends" sections into `shared` to support common usage)_ + ### Architecture Design Language The architecture design language expressed in this documentation is based on [UML](https://www.uml-diagrams.org/). From 40628c47beb3a25e9d162d093a259fe26f328e59 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 6 Jul 2023 09:55:22 +0200 Subject: [PATCH 52/59] docs: improve legend definitions --- p2p/README.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/p2p/README.md b/p2p/README.md index 93423393a..ab00e0a1e 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -127,6 +127,8 @@ ConcreteType ..* "(cardinality)" OtherType : Indirect (via interface) #### Interface Realization +_TL;DR An instance (i.e. client) implements the associated interface (i.e. supplierl)._ + > Realization is a specialized abstraction relationship between two sets of model elements, one representing a specification (the supplier) and the other represents an implementation of the latter (the client). > Realization can be used to model stepwise refinement, optimizations, transformations, templates, model synthesis, framework composition, etc. @@ -135,6 +137,8 @@ _(see: [UML Realization](https://www.uml-diagrams.org/realization.html))_ #### Direct Usage +_TL;DR one instance (i.e. client) is dependent the associated instance(s) (i.e. supplier) to function properly._ + > Dependency is a directed relationship which is used to show that some UML element or a set of elements requires, needs or depends on other model elements for specification or implementation. Because of this, dependency is called a supplier - client relationship, where supplier provides something to the client, and thus the client is in some sense incomplete while semantically or structurally dependent on the supplier element(s). Modification of the supplier may impact the client elements. > Usage is a dependency in which one named element (client) requires another named element (supplier) for its full definition or implementation. @@ -143,6 +147,8 @@ _(see: [UML Dependency](https://www.uml-diagrams.org/dependency.html))_ #### Composition +_TL;DR deleting an instance also deletes the associated instance(s)._ + > A "strong" form of aggregation > If a composite (whole) is deleted, all of its composite parts are "normally" deleted with it. @@ -151,7 +157,10 @@ _(see: [UML Shared composition](https://www.uml-diagrams.org/composition.html))_ #### Aggregation -> A "weak" form of composition + +_TL;DR deleting an instance does not necessarily delete the associated instance(s)._ + +> A "weak" form of aggregation > Shared part could be included in several composites, and if some or all of the composites are deleted, shared part may still exist. @@ -159,7 +168,11 @@ _(see: [UML Shared aggregation](https://www.uml-diagrams.org/aggregation.html))_ #### Cardinality -Cardinality indicates the number or range of simultaneous instances of the classifier at the "cardinality-side" association end that are associated with the classifier at the other end of the given association type. +_TL;DR indicates a number, or range of instances associated (i.e. supplier(s))_ + +Cardinality indicates the number or range of simultaneous instances of supplier that are associated with the client. +Applicable to multiple association types. +Can be expressed arbitrarily (e.g. wildcards, variable, equation, etc.) _(see: [UML Association](https://www.uml-diagrams.org/association.html#association-end))_ From 8c0b8c3b0702bbfe0d920e0f8ff8803ad36acf67 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 6 Jul 2023 09:55:46 +0200 Subject: [PATCH 53/59] docs: clarify broadcast table --- p2p/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/p2p/README.md b/p2p/README.md index ab00e0a1e..4188b941a 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -194,11 +194,11 @@ Depending on the necessary routing scheme (unicast / broadcast) and whether the **Broadcast** -| Broadcaster | Receiver | Router | Example Usage | -|----------------|----------------|-----------------------|--------------------------------------------| -| Staked Actor | Staked Actor | Raintree + Background | Utility tx messages | -| Unstaked Actor | Staked Actor | Background only | Utility tx messages (gossipsub redundancy) | -| Unstaked Actor | Unstaked Actor | Background only | Utility tx messages | +| Broadcaster | Receiver | Router | Example Usage | +|----------------|----------------|-----------------------|---------------------------------------------------| +| Staked Actor | Staked Actor | Raintree + Background | Utility tx messages | +| Unstaked Actor | Staked Actor | Background only | Utility tx messages (libp2p gossipsub redundancy) | +| Unstaked Actor | Unstaked Actor | Background only | Utility tx messages | Both router submodule implementations embed a `UnicastRouter` which enables them to send and receive messages directly to/from a single peer. From 5a4cc80fe122934d291985d19b4f2f464222cd66 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Fri, 7 Jul 2023 09:45:14 +0200 Subject: [PATCH 54/59] docs: fix mistake in peer discovery section --- p2p/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/README.md b/p2p/README.md index 4188b941a..ae5135b98 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -503,7 +503,7 @@ Peer discovery involves pairing peer IDs to their network addresses (multiaddr). This pairing always has an associated TTL (time-to-live), near the end of which it must be refreshed. -In the background gossip overlay network (`backgroundRouter`), peers will re-advertise themselves 7/8th through their TTL. +In the background gossip overlay network (`backgroundRouter`), peers will re-advertise themselves every 3 hours through their TTL (see: [`RoutingDiscovery#Advertise()`](https://github.com/libp2p/go-libp2p/blob/87c2561238cb0340ddb182c61be8dbbc7a12a780/p2p/discovery/routing/routing.go#L34) and [`ProviderManager#AddProvider()`](https://github.com/libp2p/go-libp2p-kad-dht/blob/v0.24.2/providers/providers_manager.go#L255)). This refreshes the libp2p peerstore automatically. In the raintree gossip overlay network (`raintreeRouter`), the libp2p peerstore is **NOT** currently refreshed _(TODO: [#859](https://github.com/pokt-network/network/isues/859))_. From 3e997f565c57bbefc553873ba8053a38ade86c90 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Fri, 7 Jul 2023 09:41:39 +0200 Subject: [PATCH 55/59] test: improve background router validation test Co-authored-by: Daniel Olshansky --- p2p/background/router_test.go | 182 ++++++++++++++++++++++++---------- 1 file changed, 127 insertions(+), 55 deletions(-) diff --git a/p2p/background/router_test.go b/p2p/background/router_test.go index 0dd33c5b7..2a24cde93 100644 --- a/p2p/background/router_test.go +++ b/p2p/background/router_test.go @@ -13,7 +13,7 @@ import ( libp2pHost "github.com/libp2p/go-libp2p/core/host" libp2pNetwork "github.com/libp2p/go-libp2p/core/network" libp2pPeer "github.com/libp2p/go-libp2p/core/peer" - mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" + "github.com/libp2p/go-libp2p/p2p/net/mock" "github.com/multiformats/go-multiaddr" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -33,7 +33,10 @@ import ( ) // https://www.rfc-editor.org/rfc/rfc3986#section-3.2.2 -const testIP6ServiceURL = "[2a00:1450:4005:802::2004]:8080" +const ( + testIP6ServiceURL = "[2a00:1450:4005:802::2004]:8080" + invalidReceiveTimeout = time.Millisecond * 500 +) // TECHDEBT(#609): move & de-dup. var ( @@ -42,7 +45,7 @@ var ( ) func TestBackgroundRouter_AddPeer(t *testing.T) { - testRouter := newTestRouter(t, nil, noopHandler) + testRouter := newTestRouter(t, nil, nil) libp2pPStore := testRouter.host.Peerstore() // NB: assert initial state @@ -90,7 +93,7 @@ func TestBackgroundRouter_AddPeer(t *testing.T) { } func TestBackgroundRouter_RemovePeer(t *testing.T) { - testRouter := newTestRouter(t, nil, noopHandler) + testRouter := newTestRouter(t, nil, nil) peerstore := testRouter.host.Peerstore() // NB: assert initial state @@ -124,69 +127,112 @@ func TestBackgroundRouter_RemovePeer(t *testing.T) { } func TestBackgroundRouter_Validation(t *testing.T) { - ctx := context.Background() - libp2pMockNet := mocknet.New() - - invalidWireFormatData := []byte("test message") - invalidPocketEnvelope := &anypb.Any{ - TypeUrl: "/test", - Value: invalidWireFormatData, + invalidProtoMessage := anypb.Any{ + TypeUrl: "/notADefinedProtobufType", + Value: []byte("not a serialized protobuf"), } - invalidPocketEnvelopeBz, err := proto.Marshal(invalidPocketEnvelope) - require.NoError(t, err) - invalidMessages := [][]byte{ - invalidWireFormatData, - invalidPocketEnvelopeBz, + testCases := []struct { + name string + msgBz []byte + }{ + { + name: "invalid BackgroundMessage", + // NB: `msgBz` would normally be a serialized `BackgroundMessage`. + msgBz: mustMarshal(t, &invalidProtoMessage), + }, + { + name: "empty PocketEnvelope", + msgBz: mustMarshal(t, &typesP2P.BackgroundMessage{ + // NB: `Data` is normally a serialized `PocketEnvelope`. + Data: nil, + }), + }, + { + name: "invalid PoketEnvelope", + msgBz: mustMarshal(t, &typesP2P.BackgroundMessage{ + // NB: `Data` is normally a serialized `PocketEnvelope`. + Data: mustMarshal(t, &invalidProtoMessage), + }), + }, } - receivedChan := make(chan struct{}) + // Set up test router as the receiver. + ctx := context.Background() + libp2pMockNet := mocknet.New() + receivedChan := make(chan []byte, 1) receiverPrivKey, receiverPeer := newTestPeer(t) receiverHost := newTestHost(t, libp2pMockNet, receiverPrivKey) - receiverRouter := newRouterWithSelfPeerAndHost(t, receiverPeer, receiverHost, func(data []byte) error { - receivedChan <- struct{}{} - return nil - }) + receiverRouter := newRouterWithSelfPeerAndHost( + t, receiverPeer, + receiverHost, + func(data []byte) error { + receivedChan <- data + return nil + }, + ) t.Cleanup(func() { err := receiverRouter.Close() require.NoError(t, err) }) - senderPrivKey, _ := newTestPeer(t) - senderHost := newTestHost(t, libp2pMockNet, senderPrivKey) - gossipPubsub, err := pubsub.NewGossipSub(ctx, senderHost) + // Wrap `receiverRouter#topicValidator` to make assertions by. + // Existing topic validator must be unregistered first. + err := receiverRouter.gossipSub.UnregisterTopicValidator(protocol.BackgroundTopicStr) require.NoError(t, err) - err = libp2pMockNet.LinkAll() - require.NoError(t, err) + // Register topic validator wrapper. + err = receiverRouter.gossipSub.RegisterTopicValidator( + protocol.BackgroundTopicStr, + func(ctx context.Context, peerID libp2pPeer.ID, msg *pubsub.Message) bool { + msgIsValid := receiverRouter.topicValidator(ctx, peerID, msg) + require.Falsef(t, msgIsValid, "expected message to be invalid") - receiverAddrInfo, err := utils.Libp2pAddrInfoFromPeer(receiverPeer) - require.NoError(t, err) - - err = senderHost.Connect(ctx, receiverAddrInfo) - require.NoError(t, err) - - topic, err := gossipPubsub.Join(protocol.BackgroundTopicStr) + return msgIsValid + }, + ) require.NoError(t, err) - for _, invalidMessageBz := range invalidMessages { - err = topic.Publish(ctx, invalidMessageBz) - require.NoError(t, err) - } - - select { - case <-time.After(time.Second * 2): - // TECHDEBT: find a better way to prove that pre-propagation validation - // works as expected. Ideally, we should be able to distinguish which - // invalid message was received in the event of failure. - // - // CONSIDERATION: we could use the telemetry module mock to set expectations - // for validation failure telemetry calls, which would probably be useful in - // their own right. - case <-receivedChan: - t.Fatal("expected message to not be received") + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + senderPrivKey, _ := newTestPeer(t) + senderHost := newTestHost(t, libp2pMockNet, senderPrivKey) + gossipPubsub, err := pubsub.NewGossipSub(ctx, senderHost) + require.NoError(t, err) + + err = libp2pMockNet.LinkAll() + require.NoError(t, err) + + receiverAddrInfo, err := utils.Libp2pAddrInfoFromPeer(receiverPeer) + require.NoError(t, err) + + err = senderHost.Connect(ctx, receiverAddrInfo) + require.NoError(t, err) + + topic, err := gossipPubsub.Join(protocol.BackgroundTopicStr) + require.NoError(t, err) + + err = topic.Publish(ctx, testCase.msgBz) + require.NoError(t, err) + + // Destroy previous topic and sender instances to start with new ones + // for each test case. + t.Cleanup(func() { + _ = topic.Close() + _ = senderHost.Close() + }) + + // Ensure no messages were handled at the end of each test case for + // async errors. + select { + case <-receivedChan: + t.Fatal("no messages should have been handled by receiver router") + case <-time.After(invalidReceiveTimeout): + // no error, continue + } + }) } } @@ -234,9 +280,9 @@ func TestBackgroundRouter_Broadcast(t *testing.T) { expectedPeerIDs[i] = host.ID().String() newRouterWithSelfPeerAndHost(t, peer, host, func(data []byte) error { seenMessagesMutext.Lock() - broadcastWaitgroup.Done() + defer seenMessagesMutext.Unlock() seenMessages[host.ID().String()] = struct{}{} - seenMessagesMutext.Unlock() + broadcastWaitgroup.Done() return nil }) } @@ -246,7 +292,7 @@ func TestBackgroundRouter_Broadcast(t *testing.T) { // set up a test backgroundRouter testRouterHost := newTestHost(t, libp2pMockNet, privKey) - testRouter := newRouterWithSelfPeerAndHost(t, selfPeer, testRouterHost, noopHandler) + testRouter := newRouterWithSelfPeerAndHost(t, selfPeer, testRouterHost, nil) testHosts = append(testHosts, testRouterHost) // simulate network links between each to every other @@ -331,7 +377,11 @@ func bootstrap(t *testing.T, ctx context.Context, testHosts []libp2pHost.Host) { } // TECHDEBT(#609): move & de-duplicate -func newTestRouter(t *testing.T, libp2pMockNet mocknet.Mocknet, handler typesP2P.MessageHandler) *backgroundRouter { +func newTestRouter( + t *testing.T, + libp2pMockNet mocknet.Mocknet, + handler typesP2P.MessageHandler, +) *backgroundRouter { t.Helper() privKey, selfPeer := newTestPeer(t) @@ -349,7 +399,12 @@ func newTestRouter(t *testing.T, libp2pMockNet mocknet.Mocknet, handler typesP2P return newRouterWithSelfPeerAndHost(t, selfPeer, host, handler) } -func newRouterWithSelfPeerAndHost(t *testing.T, selfPeer typesP2P.Peer, host libp2pHost.Host, handler typesP2P.MessageHandler) *backgroundRouter { +func newRouterWithSelfPeerAndHost( + t *testing.T, + selfPeer typesP2P.Peer, + host libp2pHost.Host, + handler typesP2P.MessageHandler, +) *backgroundRouter { t.Helper() ctrl := gomock.NewController(t) @@ -374,6 +429,10 @@ func newRouterWithSelfPeerAndHost(t *testing.T, selfPeer typesP2P.Peer, host lib err := pstore.AddPeer(selfPeer) require.NoError(t, err) + if handler == nil { + handler = noopHandler + } + router, err := Create(busMock, &config.BackgroundConfig{ Addr: selfPeer.GetAddress(), PeerstoreProvider: pstoreProviderMock, @@ -423,7 +482,11 @@ func newMockNetHostFromPeer( return host } -func newTestHost(t *testing.T, mockNet mocknet.Mocknet, privKey cryptoPocket.PrivateKey) libp2pHost.Host { +func newTestHost( + t *testing.T, + mockNet mocknet.Mocknet, + privKey cryptoPocket.PrivateKey, +) libp2pHost.Host { t.Helper() // listen on random port on loopback interface @@ -436,3 +499,12 @@ func newTestHost(t *testing.T, mockNet mocknet.Mocknet, privKey cryptoPocket.Pri // construct mock host return newMockNetHostFromPeer(t, mockNet, privKey, peer) } + +func mustMarshal(t *testing.T, msg proto.Message) []byte { + t.Helper() + + msgBz, err := proto.Marshal(msg) + require.NoError(t, err) + + return msgBz +} From abd47897bac2903884d0face4919bf32fbb05b17 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Fri, 7 Jul 2023 09:41:51 +0200 Subject: [PATCH 56/59] chore: add error log --- p2p/background/router.go | 1 + 1 file changed, 1 insertion(+) diff --git a/p2p/background/router.go b/p2p/background/router.go index 3482bcc44..c3770368f 100644 --- a/p2p/background/router.go +++ b/p2p/background/router.go @@ -358,6 +358,7 @@ func (rtr *backgroundRouter) bootstrap(ctx context.Context) error { func (rtr *backgroundRouter) topicValidator(_ context.Context, _ libp2pPeer.ID, msg *pubsub.Message) bool { var backgroundMsg typesP2P.BackgroundMessage if err := proto.Unmarshal(msg.Data, &backgroundMsg); err != nil { + rtr.logger.Error().Err(err).Msg("unmarshalling Background message") return false } From 8c6ac688ef85d03cc6cf8725e04d49fb34a484d4 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Fri, 7 Jul 2023 09:51:47 +0200 Subject: [PATCH 57/59] fix: unstaked actor bootstrapping FSM transition --- p2p/event_handler.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/p2p/event_handler.go b/p2p/event_handler.go index 93e7fc2da..ba0885839 100644 --- a/p2p/event_handler.go +++ b/p2p/event_handler.go @@ -57,13 +57,25 @@ func (m *p2pModule) HandleEvent(event *anypb.Any) error { m.logger.Debug().Fields(messaging.TransitionEventToMap(stateMachineTransitionEvent)).Msg("Received state machine transition event") if stateMachineTransitionEvent.NewState == string(coreTypes.StateMachineState_P2P_Bootstrapping) { - if m.stakedActorRouter.GetPeerstore().Size() == 0 { - m.logger.Warn().Msg("No peers in addrbook, bootstrapping") + staked, err := m.isStakedActor() + if err != nil { + return err + } + if staked { + // TECHDEBT(#859): this will never happen as the peerstore is + // seeded from consensus during P2P module construction. + if m.stakedActorRouter.GetPeerstore().Size() == 0 { + m.logger.Warn().Msg("No peers in peerstore, bootstrapping") - if err := m.bootstrap(); err != nil { - return err + if err := m.bootstrap(); err != nil { + return err + } } } + + // TECHDEBT(#859): for unstaked actors, unstaked actor (background) + // router bootstrapping SHOULD complete before the event below is sent. + m.logger.Info().Bool("TODO", true).Msg("Advertise self to network") if err := m.GetBus().GetStateMachineModule().SendEvent(coreTypes.StateMachineEvent_P2P_IsBootstrapped); err != nil { return err From b76efdfd746460dd16473e4bab7e2d0cf43b3606 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Fri, 7 Jul 2023 10:37:48 +0200 Subject: [PATCH 58/59] fix: goimports --- p2p/background/router_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/background/router_test.go b/p2p/background/router_test.go index 2a24cde93..61cb5f153 100644 --- a/p2p/background/router_test.go +++ b/p2p/background/router_test.go @@ -13,7 +13,7 @@ import ( libp2pHost "github.com/libp2p/go-libp2p/core/host" libp2pNetwork "github.com/libp2p/go-libp2p/core/network" libp2pPeer "github.com/libp2p/go-libp2p/core/peer" - "github.com/libp2p/go-libp2p/p2p/net/mock" + mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" "github.com/multiformats/go-multiaddr" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" From 5aa0a5c44c53f1c4a4987d8cb95f0a3702ecefe8 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Tue, 11 Jul 2023 10:31:50 +0200 Subject: [PATCH 59/59] docs: update unicast/broadcast table --- p2p/README.md | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/p2p/README.md b/p2p/README.md index ae5135b98..cde1a155c 100644 --- a/p2p/README.md +++ b/p2p/README.md @@ -186,19 +186,21 @@ Depending on the necessary routing scheme (unicast / broadcast) and whether the **Unicast** -| Sender | Receiver | Router | Example Usage | -|----------------|----------------|-----------------|------------------------------------------------------| -| Staked Actor | Staked Actor | Raintree only | Consensus (state sync) messages (to validators only) | -| Unstaked Actor | Staked Actor | Background only | Consensus (state sync) messages (to validators only) | -| Unstaked Actor | Unstaked Actor | Background only | Consensus (state sync) & Debug (CLI) messages | +| Sender | Receiver | Router | Example Usage | +|----------------|----------------|-----------------|----------------------------------------------------------------------| +| Staked Actor | Staked Actor | Raintree only | Consensus hotstuff messages (validators only) & state sync responses | +| Staked Actor | Untaked Actor | Background only | Consensus state sync responses | +| Unstaked Actor | Staked Actor | Background only | Consensus state sync responses, debug messages | +| Unstaked Actor | Unstaked Actor | Background only | Consensus state sync responses, debug messages | **Broadcast** -| Broadcaster | Receiver | Router | Example Usage | -|----------------|----------------|-----------------------|---------------------------------------------------| -| Staked Actor | Staked Actor | Raintree + Background | Utility tx messages | -| Unstaked Actor | Staked Actor | Background only | Utility tx messages (libp2p gossipsub redundancy) | -| Unstaked Actor | Unstaked Actor | Background only | Utility tx messages | +| Broadcaster | Receiver | Router | Example Usage | +|----------------|----------------|-----------------------|-----------------------------------------------------------------| +| Staked Actor | Staked Actor | Raintree + Background | Utility tx messages, consensus state sync requests | +| Staked Actor | Untaked Actor | Background only | Utility tx messages (redundancy), consensus state sync requests | +| Unstaked Actor | Staked Actor | Background only | Utility tx messages (redundancy), consensus state sync requests | +| Unstaked Actor | Unstaked Actor | Background only | Utility tx messages, consensus state sync requests | Both router submodule implementations embed a `UnicastRouter` which enables them to send and receive messages directly to/from a single peer.