op-node: Combine p2p.scoring.peers and p2p.scoring.topics flags as p2p.scoring

We don't need the complexity of supporting independent scoring setups for general peer behaviour and per-topic behaviour.

op-e2e/system_test.go

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

op-node/flags/p2p_flags.go

 package flags
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/urfave/cli"
@@ -25,15 +26,19 @@ var (
 		Required: false,
 		EnvVar:   p2pEnv("NO_DISCOVERY"),
 	}
-	PeerScoring = cli.StringFlag{
-		Name: "p2p.scoring.peers",
-		Usage: "Sets the peer scoring strategy for the P2P stack. " +
-			"Can be one of: none or light." +
-			"Custom scoring strategies can be defined in the config file.",
+	Scoring = cli.StringFlag{
+		Name:     "p2p.scoring",
+		Usage:    "Sets the peer scoring strategy for the P2P stack. Can be one of: none or light.",
 		Required: false,
 		Value:    "none",
 		EnvVar:   p2pEnv("PEER_SCORING"),
 	}
+	PeerScoring = cli.StringFlag{
+		Name:     "p2p.scoring.peers",
+		Usage:    fmt.Sprintf("Deprecated: Use %v instead", Scoring.Name),
+		Required: false,
+		Hidden:   true,
+	}
 	PeerScoreBands = cli.StringFlag{
 		Name:     "p2p.score.bands",
 		Usage:    "Deprecated. This option is ignored and is only present for backwards compatibility.",
@@ -66,12 +71,9 @@ var (
 
 	TopicScoring = cli.StringFlag{
 		Name:     "p2p.scoring.topics",
-		Usage: "Sets the topic scoring strategy. " +
-			"Can be one of: none or light." +
-			"Custom scoring strategies can be defined in the config file.",
+		Usage:    fmt.Sprintf("Deprecated: Use %v instead", Scoring.Name),
 		Required: false,
-		Value:    "none",
-		EnvVar:   p2pEnv("TOPIC_SCORING"),
+		Hidden:   true,
 	}
 	P2PPrivPath = cli.StringFlag{
 		Name: "p2p.priv.path",
@@ -303,6 +305,7 @@ var p2pFlags = []cli.Flag{
 	NoDiscovery,
 	P2PPrivPath,
 	P2PPrivRaw,
+	Scoring,
 	PeerScoring,
 	PeerScoreBands,
 	Banning,

op-node/p2p/cli/load_config.go

@@ -54,7 +54,7 @@ func NewConfig(ctx *cli.Context, blockTime uint64) (*p2p.Config, error) {
 		return nil, fmt.Errorf("failed to load p2p gossip options: %w", err)
 	}
 
-	if err := loadPeerScoringParams(conf, ctx, blockTime); err != nil {
+	if err := loadScoringParams(conf, ctx, blockTime); err != nil {
 		return nil, fmt.Errorf("failed to load p2p peer scoring options: %w", err)
 	}
 
@@ -62,10 +62,6 @@ func NewConfig(ctx *cli.Context, blockTime uint64) (*p2p.Config, error) {
 		return nil, fmt.Errorf("failed to load banning option: %w", err)
 	}
 
-	if err := loadTopicScoringParams(conf, ctx, blockTime); err != nil {
-		return nil, fmt.Errorf("failed to load p2p topic scoring options: %w", err)
-	}
-
 	conf.EnableReqRespSync = ctx.GlobalBool(flags.SyncReqRespFlag.Name)
 
 	return conf, nil
@@ -84,37 +80,29 @@ func validatePort(p uint) (uint16, error) {
 	return uint16(p), nil
 }
 
-// loadTopicScoringParams loads the topic scoring options from the CLI context.
-//
-// If the topic scoring options are not set, then the default topic scoring.
-func loadTopicScoringParams(conf *p2p.Config, ctx *cli.Context, blockTime uint64) error {
-	scoringLevel := ctx.GlobalString(flags.TopicScoring.Name)
-	if scoringLevel != "" {
-		// Set default block topic scoring parameters
-		// See prysm: https://github.com/prysmaticlabs/prysm/blob/develop/beacon-chain/p2p/gossip_scoring_params.go
-		// And research from lighthouse: https://gist.github.com/blacktemplar/5c1862cb3f0e32a1a7fb0b25e79e6e2c
-		// And docs: https://github.com/libp2p/specs/blob/master/pubsub/gossipsub/gossipsub-v1.1.md#topic-parameter-calculation-and-decay
-		topicScoreParams, err := p2p.GetTopicScoreParams(scoringLevel, blockTime)
-		if err != nil {
-			return err
+// loadScoringParams loads the peer scoring options from the CLI context.
+func loadScoringParams(conf *p2p.Config, ctx *cli.Context, blockTime uint64) error {
+	scoringLevel := ctx.GlobalString(flags.Scoring.Name)
+	// Check old names for backwards compatibility
+	if scoringLevel == "" {
+		scoringLevel = ctx.GlobalString(flags.PeerScoring.Name)
 	}
-		conf.TopicScoring = &topicScoreParams
+	if scoringLevel == "" {
+		scoringLevel = ctx.GlobalString(flags.TopicScoring.Name)
 	}
-
-	return nil
-}
-
-// loadPeerScoringParams loads the scoring options from the CLI context.
-//
-// If the scoring level is not set, no scoring is enabled.
-func loadPeerScoringParams(conf *p2p.Config, ctx *cli.Context, blockTime uint64) error {
-	scoringLevel := ctx.GlobalString(flags.PeerScoring.Name)
 	if scoringLevel != "" {
 		peerScoreParams, err := p2p.GetPeerScoreParams(scoringLevel, blockTime)
 		if err != nil {
 			return err
 		}
-		conf.PeerScoring = &peerScoreParams
+		topicScoreParams, err := p2p.GetTopicScoreParams(scoringLevel, blockTime)
+		if err != nil {
+			return err
+		}
+		conf.ScoringParams = &p2p.ScoringParams{
+			PeerScoring:  peerScoreParams,
+			TopicScoring: topicScoreParams,
+		}
 	}
 
 	return nil

op-node/p2p/config.go

@@ -51,6 +51,12 @@ type SetupP2P interface {
 	ReqRespSyncEnabled() bool
 }
 
+// ScoringParams defines the various types of peer scoring parameters.
+type ScoringParams struct {
+	PeerScoring  pubsub.PeerScoreParams
+	TopicScoring pubsub.TopicScoreParams
+}
+
 // Config sets up a p2p host and discv5 service from configuration.
 // This implements SetupP2P.
 type Config struct {
@@ -62,9 +68,7 @@ type Config struct {
 	// Enable P2P-based alt-syncing method (req-resp protocol, not gossip)
 	AltSync bool
 
-	// Pubsub Scoring Parameters
-	PeerScoring  *pubsub.PeerScoreParams
-	TopicScoring *pubsub.TopicScoreParams
+	ScoringParams *ScoringParams
 
 	// Whether to ban peers based on their [PeerScoring] score. Should be negative.
 	BanningEnabled bool
@@ -135,7 +139,10 @@ func (conf *Config) Disabled() bool {
 }
 
 func (conf *Config) PeerScoringParams() *pubsub.PeerScoreParams {
-	return conf.PeerScoring
+	if conf.ScoringParams == nil {
+		return nil
+	}
+	return &conf.ScoringParams.PeerScoring
 }
 
 func (conf *Config) BanPeers() bool {
@@ -151,7 +158,10 @@ func (conf *Config) BanDuration() time.Duration {
 }
 
 func (conf *Config) TopicScoringParams() *pubsub.TopicScoreParams {
-	return conf.TopicScoring
+	if conf.ScoringParams == nil {
+		return nil
+	}
+	return &conf.ScoringParams.TopicScoring
 }
 
 func (conf *Config) ReqRespSyncEnabled() bool {

op-node/p2p/peer_scores_test.go

@@ -102,7 +102,8 @@ 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{
+			ScoringParams: &ScoringParams{
+				PeerScoring: pubsub.PeerScoreParams{
 					AppSpecificScore: func(p peer.ID) float64 {
 						if p == hosts[0].ID() {
 							return -1000
@@ -114,6 +115,8 @@ func newGossipSubs(testSuite *PeerScoresTestSuite, ctx context.Context, hosts []
 					DecayInterval:     time.Second,
 					DecayToZero:       0.01,
 				},
+				TopicScoring: DisabledTopicScoreParams(1),
+			},
 		}, scorer, logger)...)
 		ps, err := pubsub.NewGossipSubWithRouter(ctx, h, rt, opts...)
 		if err != nil {