Merge pull request #5991 from ethereum-optimism/aj/optional-score-config

op-node: Require peer scoring params to be valid if set

op-e2e/system_test.go

@@ -816,7 +816,7 @@ func TestSystemDenseTopology(t *testing.T) {
 		params, err := p2p.GetPeerScoreParams("light", 2)
 		require.NoError(t, err)
 		node.P2P = &p2p.Config{
-			PeerScoring:    params,
+			PeerScoring:    &params,
 			BanningEnabled: false,
 		}
 	}

op-node/p2p/cli/load_config.go

@@ -98,7 +98,7 @@ func loadTopicScoringParams(conf *p2p.Config, ctx *cli.Context, blockTime uint64
 		if err != nil {
 			return err
 		}
-		conf.TopicScoring = topicScoreParams
+		conf.TopicScoring = &topicScoreParams
 	}
 
 	return nil
@@ -114,7 +114,7 @@ func loadPeerScoringParams(conf *p2p.Config, ctx *cli.Context, blockTime uint64)
 		if err != nil {
 			return err
 		}
-		conf.PeerScoring = peerScoreParams
+		conf.PeerScoring = &peerScoreParams
 	}
 
 	return nil

op-node/p2p/config.go

@@ -63,8 +63,8 @@ type Config struct {
 	AltSync bool
 
 	// Pubsub Scoring Parameters
-	PeerScoring  pubsub.PeerScoreParams
-	TopicScoring pubsub.TopicScoreParams
+	PeerScoring  *pubsub.PeerScoreParams
+	TopicScoring *pubsub.TopicScoreParams
 
 	// Whether to ban peers based on their [PeerScoring] score. Should be negative.
 	BanningEnabled bool
@@ -135,7 +135,7 @@ func (conf *Config) Disabled() bool {
 }
 
 func (conf *Config) PeerScoringParams() *pubsub.PeerScoreParams {
-	return &conf.PeerScoring
+	return conf.PeerScoring
 }
 
 func (conf *Config) BanPeers() bool {
@@ -151,7 +151,7 @@ func (conf *Config) BanDuration() time.Duration {
 }
 
 func (conf *Config) TopicScoringParams() *pubsub.TopicScoreParams {
-	return &conf.TopicScoring
+	return conf.TopicScoring
 }
 
 func (conf *Config) ReqRespSyncEnabled() bool {

op-node/p2p/gossip.go

@@ -443,10 +443,7 @@ func JoinGossip(p2pCtx context.Context, self peer.ID, topicScoreParams *pubsub.T
 	}
 	go LogTopicEvents(p2pCtx, log.New("topic", "blocks"), blocksTopicEvents)
 
-	// A [TimeInMeshQuantum] value of 0 means the topic score is disabled.
-	// If we passed a topicScoreParams with [TimeInMeshQuantum] set to 0,
-	// libp2p errors since the params will be rejected.
-	if topicScoreParams != nil && topicScoreParams.TimeInMeshQuantum != 0 {
+	if topicScoreParams != nil {
 		if err = blocksTopic.SetScoreParams(topicScoreParams); err != nil {
 			return nil, fmt.Errorf("failed to set topic score params: %w", err)
 		}

op-node/p2p/host.go

@@ -146,7 +146,15 @@ func (conf *Config) Host(log log.Logger, reporter metrics.Reporter, metrics Host
 	}
 
 	peerScoreParams := conf.PeerScoringParams()
-	ps, err := store.NewExtendedPeerstore(context.Background(), log, clock.SystemClock, basePs, conf.Store, peerScoreParams.RetainScore)
+	var scoreRetention time.Duration
+	if peerScoreParams != nil {
+		// Use the same retention period as gossip will if available
+		scoreRetention = peerScoreParams.RetainScore
+	} else {
+		// Disable score GC if peer scoring is disabled
+		scoreRetention = 0
+	}
+	ps, err := store.NewExtendedPeerstore(context.Background(), log, clock.SystemClock, basePs, conf.Store, scoreRetention)
 	if err != nil {
 		return nil, fmt.Errorf("failed to open extended peerstore: %w", err)
 	}

op-node/p2p/peer_scores.go

@@ -12,14 +12,13 @@ func ConfigurePeerScoring(gossipConf GossipSetupConfigurables, scorer Scorer, lo
 	peerScoreParams := gossipConf.PeerScoringParams()
 	peerScoreThresholds := NewPeerScoreThresholds()
 	opts := []pubsub.Option{}
-	// Check the app specific score since libp2p doesn't export it's [validate] function :/
-	if peerScoreParams != nil && peerScoreParams.AppSpecificScore != nil {
+	if peerScoreParams != nil {
 		opts = []pubsub.Option{
 			pubsub.WithPeerScore(peerScoreParams, &peerScoreThresholds),
 			pubsub.WithPeerScoreInspect(scorer.SnapshotHook(), peerScoreInspectFrequency),
 		}
 	} else {
-		log.Warn("Proceeding with no peer scoring...\nMissing AppSpecificScore in peer scoring params")
+		log.Info("Peer scoring disabled")
 	}
 	return opts
 }

op-node/p2p/peer_scores_test.go

@@ -102,7 +102,7 @@ func newGossipSubs(testSuite *PeerScoresTestSuite, ctx context.Context, hosts []
 			&rollup.Config{L2ChainID: big.NewInt(123)},
 			extPeerStore, testSuite.mockMetricer, logger)
 		opts = append(opts, ConfigurePeerScoring(&Config{
-			PeerScoring: pubsub.PeerScoreParams{
+			PeerScoring: &pubsub.PeerScoreParams{
 				AppSpecificScore: func(p peer.ID) float64 {
 					if p == hosts[0].ID() {
 						return -1000