Fixup: Add advertisement to protocol server registration

Author: martintomazic

go/oasis-node/cmd/debug/byzantine/node.go

@@ -155,7 +155,6 @@ func initializeAndRegisterByzantineNode(
 		return nil, fmt.Errorf("initializing storage node failed: %w", err)
 	}
 	b.p2p.service.RegisterProtocolServer(storageP2P.NewServer(b.chainContext, b.runtimeID, storage))
-	b.p2p.service.AdvertiseProtocol(storageP2P.ProtocolID(b.chainContext, b.runtimeID))
 
 	b.storage = storage
 

go/oasis-node/cmd/node/node.go

@@ -549,7 +549,6 @@ func NewNode() (node *Node, err error) { // nolint: gocyclo
 
 	// Register consensus light client P2P protocol server.
 	node.P2P.RegisterProtocolServer(consensusLightP2P.NewServer(node.P2P, node.chainContext, node.Consensus.Core()))
-	node.P2P.AdvertiseProtocol(consensusLightP2P.ProtocolID(node.chainContext))
 
 	// Register the consensus service with the peer registry.
 	if mgr := node.P2P.PeerManager(); mgr != nil {

go/p2p/api/api.go

@@ -68,6 +68,11 @@ type Service interface {
 	Publish(ctx context.Context, topic string, msg any)
 
 	// RegisterHandler registers a message handler for the specified runtime and topic kind.
+	//
+	// In addition, it triggers advertisement of the host's readiness to serve the specified topic,
+	// allowing remote peers without existing connection to discover it.
+	//
+	// Ensure your server is indeed ready, to avoid advertising prematurely.
 	RegisterHandler(topic string, handler Handler)
 
 	// BlockPeer blocks a specific peer from being used by the local node.
@@ -84,14 +89,11 @@ type Service interface {
 
 	// RegisterProtocolServer registers a protocol server for the given protocol.
 	//
-	// Normally, registered protocol server should be also advertised (AdvertiseProtocol),
-	// so that remote peers find the host node.
-	RegisterProtocolServer(srv rpc.Server)
-
-	// AdvertiseProtocol starts advertising the host as available for serving the specified protocol.
+	// In addition, it triggers advertisement of the host's readiness to serve the specified protocol,
+	// allowing remote peers without existing connection to discover it.
 	//
-	// It should be called only after the protocol server is running and ready to serve.
-	AdvertiseProtocol(p core.ProtocolID)
+	// Ensure your server is indeed ready, to avoid advertising prematurely.
+	RegisterProtocolServer(srv rpc.Server)
 
 	// GetMinRepublishInterval returns the minimum republish interval that needs to be respected by
 	// the caller when publishing the same message. If Publish is called for the same message more

go/p2p/p2p.go

@@ -340,16 +340,13 @@ func (p *p2p) RegisterProtocolServer(srv rpc.Server) {
 
 	p.host.SetStreamHandler(srv.Protocol(), srv.HandleStream)
 
+	p.peerMgr.AdvertiseProtocol(srv.Protocol())
+
 	p.logger.Info("registered protocol server",
 		"protocol_id", srv.Protocol(),
 	)
 }
 
-// Implements api.Service.
-func (p *p2p) AdvertiseProtocol(protocol core.ProtocolID) {
-	p.peerMgr.AdvertiseProtocol(protocol)
-}
-
 // Implements api.Service.
 func (p *p2p) GetMinRepublishInterval() time.Duration {
 	return seenMessagesTTL + 5*time.Second

go/p2p/peermgmt/peermgr.go

@@ -196,6 +196,8 @@ func (m *PeerManager) RegisterProtocol(p core.ProtocolID, minPeers int, totalPee
 }
 
 // AdvertiseProtocol starts advertising readiness to serve the specified protocol.
+//
+// This enables remote peers without existing connection to find the host node.
 func (m *PeerManager) AdvertiseProtocol(p core.ProtocolID) {
 	m.discovery.startAdvertising(string(p))
 	m.logger.Debug("triggered protocol advertisement",

go/worker/common/committee/node.go

@@ -956,7 +956,6 @@ func NewNode(
 
 	// Register transaction sync service.
 	p2pHost.RegisterProtocolServer(txsync.NewServer(chainContext, runtime.ID(), n.TxPool))
-	p2pHost.AdvertiseProtocol(txsync.ProtocolID(chainContext, runtime.ID()))
 
 	return n, nil
 }

go/worker/keymanager/init.go

@@ -126,7 +126,6 @@ func New(
 
 	// Register keymanager service.
 	commonWorker.P2P.RegisterProtocolServer(p2p.NewServer(commonWorker.ChainContext, w.runtimeID, w))
-	commonWorker.P2P.AdvertiseProtocol(p2p.RuntimeProtocolID(commonWorker.ChainContext, w.runtimeID))
 
 	return w, nil
 }

go/worker/storage/committee/node.go

@@ -274,13 +274,11 @@ func NewNode(
 
 	// Register storage sync service.
 	commonNode.P2P.RegisterProtocolServer(storageSync.NewServer(commonNode.ChainContext, commonNode.Runtime.ID(), localStorage))
-	commonNode.P2P.AdvertiseProtocol(storageSync.ProtocolID(commonNode.ChainContext, commonNode.Runtime.ID()))
 	n.storageSync = storageSync.NewClient(commonNode.P2P, commonNode.ChainContext, commonNode.Runtime.ID())
 
 	// Register storage pub service if configured.
 	if rpcRoleProvider != nil {
 		commonNode.P2P.RegisterProtocolServer(storagePub.NewServer(commonNode.ChainContext, commonNode.Runtime.ID(), localStorage))
-		commonNode.P2P.AdvertiseProtocol(storagePub.ProtocolID(commonNode.ChainContext, commonNode.Runtime.ID()))
 	}
 
 	return n, nil