address review comments

libp2p · Jun 29, 2023 · 53bc6d2 · 53bc6d2
1 parent 63f56d3
commit 53bc6d2
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 51 deletions.
diff --git a/config/config.go b/config/config.go
@@ -127,8 +127,7 @@ type Config struct {
 
 	DialRanker network.DialRanker
 
-	UDPBlackHoleConfig  *swarm.BlackHoleConfig
-	IPv6BlackHoleConfig *swarm.BlackHoleConfig
+	SwarmOpts []swarm.Option
 }
 
 func (cfg *Config) makeSwarm(eventBus event.Bus, enableMetrics bool) (*swarm.Swarm, error) {
@@ -163,7 +162,7 @@ func (cfg *Config) makeSwarm(eventBus event.Bus, enableMetrics bool) (*swarm.Swa
 		return nil, err
 	}
 
-	opts := make([]swarm.Option, 0, 6)
+	opts := cfg.SwarmOpts
 	if cfg.Reporter != nil {
 		opts = append(opts, swarm.WithMetrics(cfg.Reporter))
 	}
@@ -186,12 +185,6 @@ func (cfg *Config) makeSwarm(eventBus event.Bus, enableMetrics bool) (*swarm.Swa
 	}
 	opts = append(opts, swarm.WithDialRanker(dialRanker))
 
-	if cfg.UDPBlackHoleConfig != nil {
-		opts = append(opts, swarm.WithUDPBlackHoleConfig(cfg.UDPBlackHoleConfig))
-	}
-	if cfg.IPv6BlackHoleConfig != nil {
-		opts = append(opts, swarm.WithIPv6BlackHoleConfig(cfg.IPv6BlackHoleConfig))
-	}
 	if enableMetrics {
 		opts = append(opts,
 			swarm.WithMetricsTracer(swarm.NewMetricsTracer(swarm.WithRegisterer(cfg.PrometheusRegisterer))))

diff --git a/options.go b/options.go
@@ -589,24 +589,10 @@ func DialRanker(d network.DialRanker) Option {
 	}
 }
 
-// UDPBlackHoleConfig configures libp2p to use c as the config for UDP black hole detection
-func UDPBlackHoleConfig(c *swarm.BlackHoleConfig) Option {
+// SwarmOpts configures libp2p to use swarm with opts
+func SwarmOpts(opts ...swarm.Option) Option {
 	return func(cfg *Config) error {
-		if c == nil {
-			return errors.New("udp black hole config cannot be nil")
-		}
-		cfg.UDPBlackHoleConfig = c
-		return nil
-	}
-}
-
-// IPv6BlackHoleConfig configures libp2p to use c as the config for IPv6 black hole detection
-func IPv6BlackHoleConfig(c *swarm.BlackHoleConfig) Option {
-	return func(cfg *Config) error {
-		if c == nil {
-			return errors.New("ipv6 black hole config cannot be nil")
-		}
-		cfg.IPv6BlackHoleConfig = c
+		cfg.SwarmOpts = opts
 		return nil
 	}
 }
diff --git a/p2p/net/swarm/black_hole_detector.go b/p2p/net/swarm/black_hole_detector.go
@@ -241,8 +241,8 @@ func (d *blackHoleDetector) RecordResult(addr ma.Multiaddr, success bool) {
 	}
 }
 
-// BlackHoleConfig is the config used for black hole detection
-type BlackHoleConfig struct {
+// blackHoleConfig is the config used for black hole detection
+type blackHoleConfig struct {
 	// Enabled enables black hole detection
 	Enabled bool
 	// N is the size of the sliding window used to evaluate black hole state
@@ -252,10 +252,10 @@ type BlackHoleConfig struct {
 	MinSuccesses int
 }
 
-func newBlackHoleDetector(udpConfig, ipv6Config *BlackHoleConfig, mt MetricsTracer) *blackHoleDetector {
+func newBlackHoleDetector(udpConfig, ipv6Config blackHoleConfig, mt MetricsTracer) *blackHoleDetector {
 	d := &blackHoleDetector{}
 
-	if udpConfig != nil && udpConfig.Enabled {
+	if udpConfig.Enabled {
 		d.udp = &blackHoleFilter{
 			n:             udpConfig.N,
 			minSuccesses:  udpConfig.MinSuccesses,
@@ -264,7 +264,7 @@ func newBlackHoleDetector(udpConfig, ipv6Config *BlackHoleConfig, mt MetricsTrac
 		}
 	}
 
-	if ipv6Config != nil && ipv6Config.Enabled {
+	if ipv6Config.Enabled {
 		d.ipv6 = &blackHoleFilter{
 			n:             ipv6Config.N,
 			minSuccesses:  ipv6Config.MinSuccesses,

diff --git a/p2p/net/swarm/black_hole_detector_test.go b/p2p/net/swarm/black_hole_detector_test.go
@@ -75,8 +75,8 @@ func TestBlackHoleFilterSuccessFraction(t *testing.T) {
 }
 
 func TestBlackHoleDetectorInApplicableAddress(t *testing.T) {
-	udpConfig := &BlackHoleConfig{Enabled: true, N: 10, MinSuccesses: 5}
-	ipv6Config := &BlackHoleConfig{Enabled: true, N: 10, MinSuccesses: 5}
+	udpConfig := blackHoleConfig{Enabled: true, N: 10, MinSuccesses: 5}
+	ipv6Config := blackHoleConfig{Enabled: true, N: 10, MinSuccesses: 5}
 	bhd := newBlackHoleDetector(udpConfig, ipv6Config, nil)
 	addrs := []ma.Multiaddr{
 		ma.StringCast("/ip4/1.2.3.4/tcp/1234"),
@@ -94,8 +94,8 @@ func TestBlackHoleDetectorInApplicableAddress(t *testing.T) {
 }
 
 func TestBlackHoleDetectorUDPDisabled(t *testing.T) {
-	ipv6Config := &BlackHoleConfig{Enabled: true, N: 10, MinSuccesses: 5}
-	bhd := newBlackHoleDetector(nil, ipv6Config, nil)
+	ipv6Config := blackHoleConfig{Enabled: true, N: 10, MinSuccesses: 5}
+	bhd := newBlackHoleDetector(blackHoleConfig{Enabled: false}, ipv6Config, nil)
 	publicAddr := ma.StringCast("/ip4/1.2.3.4/udp/1234/quic-v1")
 	privAddr := ma.StringCast("/ip4/192.168.1.5/udp/1234/quic-v1")
 	for i := 0; i < 100; i++ {
@@ -106,8 +106,8 @@ func TestBlackHoleDetectorUDPDisabled(t *testing.T) {
 }
 
 func TestBlackHoleDetectorIPv6Disabled(t *testing.T) {
-	udpConfig := &BlackHoleConfig{Enabled: true, N: 10, MinSuccesses: 5}
-	bhd := newBlackHoleDetector(udpConfig, nil, nil)
+	udpConfig := blackHoleConfig{Enabled: true, N: 10, MinSuccesses: 5}
+	bhd := newBlackHoleDetector(udpConfig, blackHoleConfig{Enabled: false}, nil)
 	publicAddr := ma.StringCast("/ip6/1::1/tcp/1234")
 	privAddr := ma.StringCast("/ip6/::1/tcp/1234")
 	addrs := []ma.Multiaddr{publicAddr, privAddr}

diff --git a/p2p/net/swarm/swarm.go b/p2p/net/swarm/swarm.go
@@ -109,23 +109,17 @@ func WithDialRanker(d network.DialRanker) Option {
 }
 
 // WithUDPBlackHoleConfig configures swarm to use c as the config for UDP black hole detection
-func WithUDPBlackHoleConfig(c *BlackHoleConfig) Option {
+func WithUDPBlackHoleConfig(enabled bool, n, min int) Option {
 	return func(s *Swarm) error {
-		if c == nil {
-			return errors.New("udp black hole config cannot be nil")
-		}
-		s.udpBlackHoleConfig = c
+		s.udpBlackHoleConfig = blackHoleConfig{Enabled: enabled, N: n, MinSuccesses: min}
 		return nil
 	}
 }
 
 // WithIPv6BlackHoleConfig configures swarm to use c as the config for IPv6 black hole detection
-func WithIPv6BlackHoleConfig(c *BlackHoleConfig) Option {
+func WithIPv6BlackHoleConfig(enabled bool, n, min int) Option {
 	return func(s *Swarm) error {
-		if c == nil {
-			return errors.New("ipv6 black hole config cannot be nil")
-		}
-		s.ipv6BlackHoleConfig = c
+		s.ipv6BlackHoleConfig = blackHoleConfig{Enabled: enabled, N: n, MinSuccesses: min}
 		return nil
 	}
 }
@@ -196,8 +190,8 @@ type Swarm struct {
 
 	dialRanker network.DialRanker
 
-	udpBlackHoleConfig  *BlackHoleConfig
-	ipv6BlackHoleConfig *BlackHoleConfig
+	udpBlackHoleConfig  blackHoleConfig
+	ipv6BlackHoleConfig blackHoleConfig
 	bhd                 *blackHoleDetector
 }
 
@@ -222,8 +216,8 @@ func NewSwarm(local peer.ID, peers peerstore.Peerstore, eventBus event.Bus, opts
 		// A black hole is a binary property. On a network if UDP dials are blocked or there is
 		// no IPv6 connectivity, all dials will fail. So a low success rate of 5 out 100 dials
 		// is good enough.
-		udpBlackHoleConfig:  &BlackHoleConfig{Enabled: true, N: 100, MinSuccesses: 5},
-		ipv6BlackHoleConfig: &BlackHoleConfig{Enabled: true, N: 10, MinSuccesses: 5},
+		udpBlackHoleConfig:  blackHoleConfig{Enabled: true, N: 100, MinSuccesses: 5},
+		ipv6BlackHoleConfig: blackHoleConfig{Enabled: true, N: 100, MinSuccesses: 5},
 	}
 
 	s.conns.m = make(map[peer.ID][]*Conn)