From 521e9e29da321772a48151c40e6a16ca8d1296c8 Mon Sep 17 00:00:00 2001 From: noot Date: Tue, 18 May 2021 21:19:16 -0400 Subject: [PATCH 1/5] update bootstrapPeers to be func() []peer.AddrInfo --- dht.go | 13 +++++++------ dht_options.go | 4 ++-- dht_test.go | 8 ++++++-- fullrt/dht.go | 2 +- internal/config/config.go | 2 +- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/dht.go b/dht.go index ae82c1396..c5fbeeb54 100644 --- a/dht.go +++ b/dht.go @@ -126,10 +126,10 @@ type IpfsDHT struct { autoRefresh bool - // A set of bootstrap peers to fallback on if all other attempts to fix + // A function returning a set of bootstrap peers to fallback on if all other attempts to fix // the routing table fail (or, e.g., this is the first time this node is // connecting to the network). - bootstrapPeers []peer.AddrInfo + bootstrapPeers func() []peer.AddrInfo maxRecordAge time.Duration @@ -473,15 +473,16 @@ func (dht *IpfsDHT) fixLowPeers(ctx context.Context) { // We should first use non-bootstrap peers we knew of from previous // snapshots of the Routing Table before we connect to the bootstrappers. // See https://github.com/libp2p/go-libp2p-kad-dht/issues/387. - if dht.routingTable.Size() == 0 { - if len(dht.bootstrapPeers) == 0 { + if dht.routingTable.Size() == 0 && dht.bootstrapPeers != nil { + bootstrapPeers := dht.bootstrapPeers() + if len(bootstrapPeers) == 0 { // No point in continuing, we have no peers! return } found := 0 - for _, i := range rand.Perm(len(dht.bootstrapPeers)) { - ai := dht.bootstrapPeers[i] + for _, i := range rand.Perm(len(bootstrapPeers)) { + ai := bootstrapPeers[i] err := dht.Host().Connect(ctx, ai) if err == nil { found++ diff --git a/dht_options.go b/dht_options.go index 1f4e47afe..2941b66fe 100644 --- a/dht_options.go +++ b/dht_options.go @@ -267,9 +267,9 @@ func RoutingTableFilter(filter RouteTableFilterFunc) Option { // BootstrapPeers configures the bootstrapping nodes that we will connect to to seed // and refresh our Routing Table if it becomes empty. -func BootstrapPeers(bootstrappers ...peer.AddrInfo) Option { +func BootstrapPeers(getBootstrapPeers func() []peer.AddrInfo) Option { return func(c *dhtcfg.Config) error { - c.BootstrapPeers = bootstrappers + c.BootstrapPeers = getBootstrapPeers return nil } } diff --git a/dht_test.go b/dht_test.go index 3bc45426b..b69b3cb7a 100644 --- a/dht_test.go +++ b/dht_test.go @@ -1991,7 +1991,9 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { testPrefix, NamespacedValidator("v", blankValidator{}), Mode(ModeServer), - BootstrapPeers(bootstrapAddrs[0]), + BootstrapPeers(func() []peer.AddrInfo { + return []peer.AddrInfo{bootstrapAddrs[0]} + }), ) require.NoError(t, err) dht2 := setupDHT(ctx, t, false) @@ -2025,7 +2027,9 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { testPrefix, NamespacedValidator("v", blankValidator{}), Mode(ModeServer), - BootstrapPeers(bootstrapAddrs[1], bootstrapAddrs[2]), + BootstrapPeers(func() []peer.AddrInfo { + return []peer.AddrInfo{bootstrapAddrs[1], bootstrapAddrs[2]} + }), ) require.NoError(t, err) diff --git a/fullrt/dht.go b/fullrt/dht.go index f01662737..ffc00a99f 100644 --- a/fullrt/dht.go +++ b/fullrt/dht.go @@ -141,7 +141,7 @@ func NewFullRT(h host.Host, protocolPrefix protocol.ID, options ...Option) (*Ful var bsPeers []*peer.AddrInfo - for _, ai := range dhtcfg.BootstrapPeers { + for _, ai := range dhtcfg.BootstrapPeers() { tmpai := ai bsPeers = append(bsPeers, &tmpai) } diff --git a/internal/config/config.go b/internal/config/config.go index 8e805688c..75b980e3f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -57,7 +57,7 @@ type Config struct { DiversityFilter peerdiversity.PeerIPGroupFilter } - BootstrapPeers []peer.AddrInfo + BootstrapPeers func() []peer.AddrInfo // test specific Config options DisableFixLowPeers bool From 2ad4f93446369291d8933d0dd246d53e5bd9fab6 Mon Sep 17 00:00:00 2001 From: noot Date: Thu, 27 May 2021 21:43:59 -0400 Subject: [PATCH 2/5] add BootstrapPeersFunc and add test --- dht_options.go | 13 ++++++++++++- dht_test.go | 34 ++++++++++++++++++++++++++++------ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/dht_options.go b/dht_options.go index 2941b66fe..0ef777913 100644 --- a/dht_options.go +++ b/dht_options.go @@ -267,7 +267,18 @@ func RoutingTableFilter(filter RouteTableFilterFunc) Option { // BootstrapPeers configures the bootstrapping nodes that we will connect to to seed // and refresh our Routing Table if it becomes empty. -func BootstrapPeers(getBootstrapPeers func() []peer.AddrInfo) Option { +func BootstrapPeers(bootstrappers ...peer.AddrInfo) Option { + return func(c *dhtcfg.Config) error { + c.BootstrapPeers = func() []peer.AddrInfo { + return bootstrappers + } + return nil + } +} + +// BootstrapPeersFunc configures the function that returns the bootstrapping nodes that we will +// connect to to seed and refresh our Routing Table if it becomes empty. +func BootstrapPeersFunc(getBootstrapPeers func() []peer.AddrInfo) Option { return func(c *dhtcfg.Config) error { c.BootstrapPeers = getBootstrapPeers return nil diff --git a/dht_test.go b/dht_test.go index b69b3cb7a..1b8d249bc 100644 --- a/dht_test.go +++ b/dht_test.go @@ -1991,9 +1991,7 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { testPrefix, NamespacedValidator("v", blankValidator{}), Mode(ModeServer), - BootstrapPeers(func() []peer.AddrInfo { - return []peer.AddrInfo{bootstrapAddrs[0]} - }), + BootstrapPeers(bootstrapAddrs[0]), ) require.NoError(t, err) dht2 := setupDHT(ctx, t, false) @@ -2027,9 +2025,7 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { testPrefix, NamespacedValidator("v", blankValidator{}), Mode(ModeServer), - BootstrapPeers(func() []peer.AddrInfo { - return []peer.AddrInfo{bootstrapAddrs[1], bootstrapAddrs[2]} - }), + BootstrapPeers(bootstrapAddrs[1], bootstrapAddrs[2]), ) require.NoError(t, err) @@ -2052,6 +2048,32 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { }, 5*time.Second, 500*time.Millisecond) } +func TestBootstrapPeersFunc(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + bootstrapFuncA := func() []peer.AddrInfo { + return []peer.AddrInfo{} + } + dhtA := setupDHT(ctx, t, false, BootstrapPeersFunc(bootstrapFuncA)) + + bootstrapPeersB := []peer.AddrInfo{} + bootstrapFuncB := func() []peer.AddrInfo { + return bootstrapPeersB + } + dhtB := setupDHT(ctx, t, false, BootstrapPeersFunc(bootstrapFuncB)) + require.Equal(t, 0, len(dhtB.host.Network().Peers())) + + addrA := peer.AddrInfo{ + ID: dhtA.self, + Addrs: dhtA.host.Addrs(), + } + + bootstrapPeersB = []peer.AddrInfo{addrA} + dhtB.fixLowPeers(ctx) + require.NotEqual(t, 0, len(dhtB.host.Network().Peers())) +} + func TestPreconnectedNodes(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() From e94ddc712d998d856deb461ff1a537e856129732 Mon Sep 17 00:00:00 2001 From: noot Date: Tue, 18 May 2021 21:19:16 -0400 Subject: [PATCH 3/5] update bootstrapPeers to be func() []peer.AddrInfo --- dht.go | 13 +++++++------ dht_options.go | 4 ++-- dht_test.go | 8 ++++++-- fullrt/dht.go | 2 +- internal/config/config.go | 2 +- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/dht.go b/dht.go index ae82c1396..c5fbeeb54 100644 --- a/dht.go +++ b/dht.go @@ -126,10 +126,10 @@ type IpfsDHT struct { autoRefresh bool - // A set of bootstrap peers to fallback on if all other attempts to fix + // A function returning a set of bootstrap peers to fallback on if all other attempts to fix // the routing table fail (or, e.g., this is the first time this node is // connecting to the network). - bootstrapPeers []peer.AddrInfo + bootstrapPeers func() []peer.AddrInfo maxRecordAge time.Duration @@ -473,15 +473,16 @@ func (dht *IpfsDHT) fixLowPeers(ctx context.Context) { // We should first use non-bootstrap peers we knew of from previous // snapshots of the Routing Table before we connect to the bootstrappers. // See https://github.com/libp2p/go-libp2p-kad-dht/issues/387. - if dht.routingTable.Size() == 0 { - if len(dht.bootstrapPeers) == 0 { + if dht.routingTable.Size() == 0 && dht.bootstrapPeers != nil { + bootstrapPeers := dht.bootstrapPeers() + if len(bootstrapPeers) == 0 { // No point in continuing, we have no peers! return } found := 0 - for _, i := range rand.Perm(len(dht.bootstrapPeers)) { - ai := dht.bootstrapPeers[i] + for _, i := range rand.Perm(len(bootstrapPeers)) { + ai := bootstrapPeers[i] err := dht.Host().Connect(ctx, ai) if err == nil { found++ diff --git a/dht_options.go b/dht_options.go index 1f4e47afe..2941b66fe 100644 --- a/dht_options.go +++ b/dht_options.go @@ -267,9 +267,9 @@ func RoutingTableFilter(filter RouteTableFilterFunc) Option { // BootstrapPeers configures the bootstrapping nodes that we will connect to to seed // and refresh our Routing Table if it becomes empty. -func BootstrapPeers(bootstrappers ...peer.AddrInfo) Option { +func BootstrapPeers(getBootstrapPeers func() []peer.AddrInfo) Option { return func(c *dhtcfg.Config) error { - c.BootstrapPeers = bootstrappers + c.BootstrapPeers = getBootstrapPeers return nil } } diff --git a/dht_test.go b/dht_test.go index 3c1f2bca0..cf5902513 100644 --- a/dht_test.go +++ b/dht_test.go @@ -1994,7 +1994,9 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { testPrefix, NamespacedValidator("v", blankValidator{}), Mode(ModeServer), - BootstrapPeers(bootstrapAddrs[0]), + BootstrapPeers(func() []peer.AddrInfo { + return []peer.AddrInfo{bootstrapAddrs[0]} + }), ) require.NoError(t, err) dht2 := setupDHT(ctx, t, false) @@ -2028,7 +2030,9 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { testPrefix, NamespacedValidator("v", blankValidator{}), Mode(ModeServer), - BootstrapPeers(bootstrapAddrs[1], bootstrapAddrs[2]), + BootstrapPeers(func() []peer.AddrInfo { + return []peer.AddrInfo{bootstrapAddrs[1], bootstrapAddrs[2]} + }), ) require.NoError(t, err) diff --git a/fullrt/dht.go b/fullrt/dht.go index 331f324ff..dfee22807 100644 --- a/fullrt/dht.go +++ b/fullrt/dht.go @@ -142,7 +142,7 @@ func NewFullRT(h host.Host, protocolPrefix protocol.ID, options ...Option) (*Ful var bsPeers []*peer.AddrInfo - for _, ai := range dhtcfg.BootstrapPeers { + for _, ai := range dhtcfg.BootstrapPeers() { tmpai := ai bsPeers = append(bsPeers, &tmpai) } diff --git a/internal/config/config.go b/internal/config/config.go index 8e805688c..75b980e3f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -57,7 +57,7 @@ type Config struct { DiversityFilter peerdiversity.PeerIPGroupFilter } - BootstrapPeers []peer.AddrInfo + BootstrapPeers func() []peer.AddrInfo // test specific Config options DisableFixLowPeers bool From 0264964b4063f7dd4dee446da24c825a94d1beda Mon Sep 17 00:00:00 2001 From: noot Date: Thu, 27 May 2021 21:43:59 -0400 Subject: [PATCH 4/5] add BootstrapPeersFunc and add test --- dht_options.go | 13 ++++++++++++- dht_test.go | 34 ++++++++++++++++++++++++++++------ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/dht_options.go b/dht_options.go index 2941b66fe..0ef777913 100644 --- a/dht_options.go +++ b/dht_options.go @@ -267,7 +267,18 @@ func RoutingTableFilter(filter RouteTableFilterFunc) Option { // BootstrapPeers configures the bootstrapping nodes that we will connect to to seed // and refresh our Routing Table if it becomes empty. -func BootstrapPeers(getBootstrapPeers func() []peer.AddrInfo) Option { +func BootstrapPeers(bootstrappers ...peer.AddrInfo) Option { + return func(c *dhtcfg.Config) error { + c.BootstrapPeers = func() []peer.AddrInfo { + return bootstrappers + } + return nil + } +} + +// BootstrapPeersFunc configures the function that returns the bootstrapping nodes that we will +// connect to to seed and refresh our Routing Table if it becomes empty. +func BootstrapPeersFunc(getBootstrapPeers func() []peer.AddrInfo) Option { return func(c *dhtcfg.Config) error { c.BootstrapPeers = getBootstrapPeers return nil diff --git a/dht_test.go b/dht_test.go index cf5902513..ca23e95cc 100644 --- a/dht_test.go +++ b/dht_test.go @@ -1994,9 +1994,7 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { testPrefix, NamespacedValidator("v", blankValidator{}), Mode(ModeServer), - BootstrapPeers(func() []peer.AddrInfo { - return []peer.AddrInfo{bootstrapAddrs[0]} - }), + BootstrapPeers(bootstrapAddrs[0]), ) require.NoError(t, err) dht2 := setupDHT(ctx, t, false) @@ -2030,9 +2028,7 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { testPrefix, NamespacedValidator("v", blankValidator{}), Mode(ModeServer), - BootstrapPeers(func() []peer.AddrInfo { - return []peer.AddrInfo{bootstrapAddrs[1], bootstrapAddrs[2]} - }), + BootstrapPeers(bootstrapAddrs[1], bootstrapAddrs[2]), ) require.NoError(t, err) @@ -2055,6 +2051,32 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { }, 5*time.Second, 500*time.Millisecond) } +func TestBootstrapPeersFunc(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + bootstrapFuncA := func() []peer.AddrInfo { + return []peer.AddrInfo{} + } + dhtA := setupDHT(ctx, t, false, BootstrapPeersFunc(bootstrapFuncA)) + + bootstrapPeersB := []peer.AddrInfo{} + bootstrapFuncB := func() []peer.AddrInfo { + return bootstrapPeersB + } + dhtB := setupDHT(ctx, t, false, BootstrapPeersFunc(bootstrapFuncB)) + require.Equal(t, 0, len(dhtB.host.Network().Peers())) + + addrA := peer.AddrInfo{ + ID: dhtA.self, + Addrs: dhtA.host.Addrs(), + } + + bootstrapPeersB = []peer.AddrInfo{addrA} + dhtB.fixLowPeers(ctx) + require.NotEqual(t, 0, len(dhtB.host.Network().Peers())) +} + func TestPreconnectedNodes(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() From 82cd58909934848215ff296d2e9a70efa29cee9f Mon Sep 17 00:00:00 2001 From: noot Date: Mon, 21 Jun 2021 10:30:04 -0400 Subject: [PATCH 5/5] fix racey test --- dht_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dht_test.go b/dht_test.go index 1b8d249bc..3c1b83d4a 100644 --- a/dht_test.go +++ b/dht_test.go @@ -2051,6 +2051,7 @@ func TestBootStrapWhenRTIsEmpty(t *testing.T) { func TestBootstrapPeersFunc(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() + var lock sync.Mutex bootstrapFuncA := func() []peer.AddrInfo { return []peer.AddrInfo{} @@ -2059,8 +2060,11 @@ func TestBootstrapPeersFunc(t *testing.T) { bootstrapPeersB := []peer.AddrInfo{} bootstrapFuncB := func() []peer.AddrInfo { + lock.Lock() + defer lock.Unlock() return bootstrapPeersB } + dhtB := setupDHT(ctx, t, false, BootstrapPeersFunc(bootstrapFuncB)) require.Equal(t, 0, len(dhtB.host.Network().Peers())) @@ -2069,7 +2073,10 @@ func TestBootstrapPeersFunc(t *testing.T) { Addrs: dhtA.host.Addrs(), } + lock.Lock() bootstrapPeersB = []peer.AddrInfo{addrA} + lock.Unlock() + dhtB.fixLowPeers(ctx) require.NotEqual(t, 0, len(dhtB.host.Network().Peers())) }