From 98493464881665c56c600eee31fb2fa07cedb957 Mon Sep 17 00:00:00 2001 From: noot Date: Tue, 8 Jun 2021 10:08:33 -0400 Subject: [PATCH 1/8] sleep after stopping service --- dot/network/service_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/dot/network/service_test.go b/dot/network/service_test.go index fcd64bf0b8..c80ccb6b01 100644 --- a/dot/network/service_test.go +++ b/dot/network/service_test.go @@ -113,6 +113,7 @@ func createTestService(t *testing.T, cfg *Config) (srvc *Service) { t.Cleanup(func() { srvc.Stop() + time.Sleep(time.Second) err = os.RemoveAll(cfg.BasePath) if err != nil { fmt.Printf("failed to remove path %s : %s\n", cfg.BasePath, err) From cc3ea6e863e759f6c453a4533e63999d3c68be97 Mon Sep 17 00:00:00 2001 From: noot Date: Tue, 8 Jun 2021 10:46:13 -0400 Subject: [PATCH 2/8] remove randseed from tests --- dot/network/config.go | 1 - dot/network/service_test.go | 7 ------- 2 files changed, 8 deletions(-) diff --git a/dot/network/config.go b/dot/network/config.go index aa6accfb39..f9270207cf 100644 --- a/dot/network/config.go +++ b/dot/network/config.go @@ -104,7 +104,6 @@ type Config struct { // build checks the configuration, sets up the private key for the network service, // and applies default values where appropriate func (c *Config) build() error { - // check state configuration err := c.checkState() if err != nil { diff --git a/dot/network/service_test.go b/dot/network/service_test.go index c80ccb6b01..64774a61b6 100644 --- a/dot/network/service_test.go +++ b/dot/network/service_test.go @@ -48,7 +48,6 @@ func createServiceHelper(t *testing.T, num int) []*Service { config := &Config{ BasePath: utils.NewTestBasePath(t, fmt.Sprintf("node%d", i)), Port: uint32(7001 + i), - RandSeed: int64(1 + i), NoBootstrap: true, NoMDNS: true, } @@ -146,7 +145,6 @@ func TestBroadcastMessages(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -159,7 +157,6 @@ func TestBroadcastMessages(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -194,7 +191,6 @@ func TestBroadcastDuplicateMessage(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -207,7 +203,6 @@ func TestBroadcastDuplicateMessage(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -320,7 +315,6 @@ func TestHandleConn(t *testing.T) { configA := &Config{ BasePath: utils.NewTestBasePath(t, "nodeA"), Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -330,7 +324,6 @@ func TestHandleConn(t *testing.T) { configB := &Config{ BasePath: utils.NewTestBasePath(t, "nodeB"), Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } From d01e75dfc6cd5c2f66f54a80d8ce6f489607fd05 Mon Sep 17 00:00:00 2001 From: noot Date: Tue, 8 Jun 2021 10:57:59 -0400 Subject: [PATCH 3/8] completely remove randseed usage from tests --- dot/core/test_helpers.go | 1 - dot/network/block_announce_test.go | 2 -- dot/network/connmgr_test.go | 3 --- dot/network/discovery_test.go | 6 ------ dot/network/gossip_test.go | 3 --- dot/network/host_test.go | 13 ------------- dot/network/light_test.go | 2 -- dot/network/mdns_test.go | 2 -- dot/network/notifications_test.go | 6 ------ dot/network/service_test.go | 2 -- dot/network/sync_justification_test.go | 2 -- dot/network/sync_test.go | 4 ---- dot/network/transaction_test.go | 1 - 13 files changed, 47 deletions(-) diff --git a/dot/core/test_helpers.go b/dot/core/test_helpers.go index 5366d4cf8f..17275fbe17 100644 --- a/dot/core/test_helpers.go +++ b/dot/core/test_helpers.go @@ -168,7 +168,6 @@ func NewTestService(t *testing.T, cfg *Config) *Service { config := &network.Config{ BasePath: testDatadirPath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, BlockState: stateSrvc.Block, diff --git a/dot/network/block_announce_test.go b/dot/network/block_announce_test.go index 4010e002b3..216153f359 100644 --- a/dot/network/block_announce_test.go +++ b/dot/network/block_announce_test.go @@ -89,7 +89,6 @@ func TestHandleBlockAnnounceMessage(t *testing.T) { config := &Config{ BasePath: basePath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -110,7 +109,6 @@ func TestValidateBlockAnnounceHandshake(t *testing.T) { configA := &Config{ BasePath: utils.NewTestBasePath(t, "nodeA"), Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } diff --git a/dot/network/connmgr_test.go b/dot/network/connmgr_test.go index 5f6b22bbcb..50e0661b80 100644 --- a/dot/network/connmgr_test.go +++ b/dot/network/connmgr_test.go @@ -33,7 +33,6 @@ func TestMaxPeers(t *testing.T) { config := &Config{ BasePath: utils.NewTestBasePath(t, fmt.Sprintf("node%d", i)), Port: 7000 + uint32(i), - RandSeed: 1 + int64(i), NoBootstrap: true, NoMDNS: true, MaxPeers: max, @@ -91,7 +90,6 @@ func TestPersistentPeers(t *testing.T) { configA := &Config{ BasePath: utils.NewTestBasePath(t, "node-a"), Port: 7000, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -101,7 +99,6 @@ func TestPersistentPeers(t *testing.T) { configB := &Config{ BasePath: utils.NewTestBasePath(t, "node-b"), Port: 7001, - RandSeed: 2, NoMDNS: true, PersistentPeers: []string{addrs[0].String()}, } diff --git a/dot/network/discovery_test.go b/dot/network/discovery_test.go index fac6d57d91..5c19d47600 100644 --- a/dot/network/discovery_test.go +++ b/dot/network/discovery_test.go @@ -37,7 +37,6 @@ func newTestDiscovery(t *testing.T, num int) []*discovery { config := &Config{ BasePath: utils.NewTestBasePath(t, fmt.Sprintf("node%d", i)), Port: uint32(7001 + i), - RandSeed: int64(1 + i), NoBootstrap: true, NoMDNS: true, } @@ -119,7 +118,6 @@ func TestBeginDiscovery(t *testing.T) { configA := &Config{ BasePath: utils.NewTestBasePath(t, "nodeA"), Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -130,7 +128,6 @@ func TestBeginDiscovery(t *testing.T) { configB := &Config{ BasePath: utils.NewTestBasePath(t, "nodeB"), Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -159,7 +156,6 @@ func TestBeginDiscovery_ThreeNodes(t *testing.T) { configA := &Config{ BasePath: utils.NewTestBasePath(t, "nodeA"), Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -170,7 +166,6 @@ func TestBeginDiscovery_ThreeNodes(t *testing.T) { configB := &Config{ BasePath: utils.NewTestBasePath(t, "nodeB"), Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -181,7 +176,6 @@ func TestBeginDiscovery_ThreeNodes(t *testing.T) { configC := &Config{ BasePath: utils.NewTestBasePath(t, "nodeC"), Port: 7003, - RandSeed: 3, NoBootstrap: true, NoMDNS: true, } diff --git a/dot/network/gossip_test.go b/dot/network/gossip_test.go index 94c7b0669e..6e3f383682 100644 --- a/dot/network/gossip_test.go +++ b/dot/network/gossip_test.go @@ -36,7 +36,6 @@ func TestGossip(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -49,7 +48,6 @@ func TestGossip(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -73,7 +71,6 @@ func TestGossip(t *testing.T) { configC := &Config{ BasePath: basePathC, Port: 7003, - RandSeed: 3, NoBootstrap: true, NoMDNS: true, } diff --git a/dot/network/host_test.go b/dot/network/host_test.go index 7330f71441..3409fa3fe4 100644 --- a/dot/network/host_test.go +++ b/dot/network/host_test.go @@ -33,7 +33,6 @@ func TestExternalAddrs(t *testing.T) { config := &Config{ BasePath: utils.NewTestBasePath(t, "node"), Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -64,7 +63,6 @@ func TestConnect(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -77,7 +75,6 @@ func TestConnect(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -123,7 +120,6 @@ func TestBootstrap(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -138,7 +134,6 @@ func TestBootstrap(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, Bootnodes: []string{addrA.String()}, NoMDNS: true, } @@ -180,7 +175,6 @@ func TestSend(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -193,7 +187,6 @@ func TestSend(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -231,7 +224,6 @@ func TestExistingStream(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -248,7 +240,6 @@ func TestExistingStream(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -299,7 +290,6 @@ func TestStreamCloseMetadataCleanup(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -313,7 +303,6 @@ func TestStreamCloseMetadataCleanup(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -373,7 +362,6 @@ func Test_PeerSupportsProtocol(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -384,7 +372,6 @@ func Test_PeerSupportsProtocol(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } diff --git a/dot/network/light_test.go b/dot/network/light_test.go index bc9b2fd862..219d9f0b0e 100644 --- a/dot/network/light_test.go +++ b/dot/network/light_test.go @@ -49,7 +49,6 @@ func TestHandleLightMessage_Response(t *testing.T) { config := &Config{ BasePath: utils.NewTestBasePath(t, "nodeA"), Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -58,7 +57,6 @@ func TestHandleLightMessage_Response(t *testing.T) { configB := &Config{ BasePath: utils.NewTestBasePath(t, "nodeB"), Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } diff --git a/dot/network/mdns_test.go b/dot/network/mdns_test.go index d7e54ea70d..26f589d8fc 100644 --- a/dot/network/mdns_test.go +++ b/dot/network/mdns_test.go @@ -33,7 +33,6 @@ func TestMDNS(t *testing.T) { configA := &Config{ BasePath: basePathA, Port: 7001, - RandSeed: 1, NoBootstrap: true, } @@ -46,7 +45,6 @@ func TestMDNS(t *testing.T) { configB := &Config{ BasePath: basePathB, Port: 7002, - RandSeed: 2, NoBootstrap: true, } diff --git a/dot/network/notifications_test.go b/dot/network/notifications_test.go index 3662943614..4b1ed7093e 100644 --- a/dot/network/notifications_test.go +++ b/dot/network/notifications_test.go @@ -45,7 +45,6 @@ func TestCreateDecoder_BlockAnnounce(t *testing.T) { config := &Config{ BasePath: basePath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -108,7 +107,6 @@ func TestCreateNotificationsMessageHandler_BlockAnnounce(t *testing.T) { config := &Config{ BasePath: basePath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -118,7 +116,6 @@ func TestCreateNotificationsMessageHandler_BlockAnnounce(t *testing.T) { configB := &Config{ BasePath: utils.NewTestBasePath(t, "nodeB"), Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -169,7 +166,6 @@ func TestCreateNotificationsMessageHandler_BlockAnnounceHandshake(t *testing.T) config := &Config{ BasePath: utils.NewTestBasePath(t, "nodeA"), Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -189,7 +185,6 @@ func TestCreateNotificationsMessageHandler_BlockAnnounceHandshake(t *testing.T) configB := &Config{ BasePath: utils.NewTestBasePath(t, "nodeB"), Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, } @@ -251,7 +246,6 @@ func Test_HandshakeTimeout(t *testing.T) { config := &Config{ BasePath: utils.NewTestBasePath(t, "nodeA"), Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } diff --git a/dot/network/service_test.go b/dot/network/service_test.go index 64774a61b6..cdeb36aa86 100644 --- a/dot/network/service_test.go +++ b/dot/network/service_test.go @@ -70,7 +70,6 @@ func createTestService(t *testing.T, cfg *Config) (srvc *Service) { cfg = &Config{ BasePath: basePath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, LogLvl: 4, @@ -272,7 +271,6 @@ func TestService_Health(t *testing.T) { config := &Config{ BasePath: basePath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } diff --git a/dot/network/sync_justification_test.go b/dot/network/sync_justification_test.go index bcb791d9bc..4c3caf41b9 100644 --- a/dot/network/sync_justification_test.go +++ b/dot/network/sync_justification_test.go @@ -35,7 +35,6 @@ func TestSyncQueue_PushResponse_Justification(t *testing.T) { config := &Config{ BasePath: basePath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -74,7 +73,6 @@ func TestSyncQueue_PushResponse_EmptyJustification(t *testing.T) { config := &Config{ BasePath: basePath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } diff --git a/dot/network/sync_test.go b/dot/network/sync_test.go index 4bb96712d7..b95d073504 100644 --- a/dot/network/sync_test.go +++ b/dot/network/sync_test.go @@ -82,7 +82,6 @@ func TestSyncQueue_PushResponse(t *testing.T) { config := &Config{ BasePath: basePath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, } @@ -285,7 +284,6 @@ func TestSyncQueue_ProcessBlockRequests(t *testing.T) { configA := &Config{ BasePath: utils.NewTestBasePath(t, "nodeA"), Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, LogLvl: 4, @@ -297,7 +295,6 @@ func TestSyncQueue_ProcessBlockRequests(t *testing.T) { configB := &Config{ BasePath: utils.NewTestBasePath(t, "nodeB"), Port: 7002, - RandSeed: 2, NoBootstrap: true, NoMDNS: true, LogLvl: 4, @@ -309,7 +306,6 @@ func TestSyncQueue_ProcessBlockRequests(t *testing.T) { configC := &Config{ BasePath: utils.NewTestBasePath(t, "nodeC"), Port: 7003, - RandSeed: 3, NoBootstrap: true, NoMDNS: true, } diff --git a/dot/network/transaction_test.go b/dot/network/transaction_test.go index 07876168d9..1bfa54bde9 100644 --- a/dot/network/transaction_test.go +++ b/dot/network/transaction_test.go @@ -78,7 +78,6 @@ func TestHandleTransactionMessage(t *testing.T) { config := &Config{ BasePath: basePath, Port: 7001, - RandSeed: 1, NoBootstrap: true, NoMDNS: true, TransactionHandler: handler, From 50c00eafde49e7e6db09d803b091dc8603c2274c Mon Sep 17 00:00:00 2001 From: noot Date: Tue, 8 Jun 2021 11:51:49 -0400 Subject: [PATCH 4/8] improve host.addrInfo --- dot/network/discovery_test.go | 24 +++++-------- dot/network/gossip_test.go | 22 +++++------- dot/network/host.go | 14 +++----- dot/network/host_test.go | 60 ++++++++++++------------------- dot/network/light_test.go | 8 ++--- dot/network/notifications_test.go | 16 ++++----- dot/network/service_test.go | 32 +++++++---------- dot/network/sync_test.go | 16 ++++----- 8 files changed, 72 insertions(+), 120 deletions(-) diff --git a/dot/network/discovery_test.go b/dot/network/discovery_test.go index 5c19d47600..c3b9adf01f 100644 --- a/dot/network/discovery_test.go +++ b/dot/network/discovery_test.go @@ -135,13 +135,11 @@ func TestBeginDiscovery(t *testing.T) { nodeB := createTestService(t, configB) nodeB.noGossip = true - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) @@ -184,24 +182,20 @@ func TestBeginDiscovery_ThreeNodes(t *testing.T) { nodeC.noGossip = true // connect A and B - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) // connect A and C - addrInfosC, err := nodeC.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosC[0]) + addrInfoC := nodeC.host.addrInfo() + err = nodeA.host.connect(addrInfoC) if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosC[0]) + err = nodeA.host.connect(addrInfoC) } require.NoError(t, err) diff --git a/dot/network/gossip_test.go b/dot/network/gossip_test.go index 6e3f383682..7cd645ae72 100644 --- a/dot/network/gossip_test.go +++ b/dot/network/gossip_test.go @@ -56,14 +56,12 @@ func TestGossip(t *testing.T) { handlerB := newTestStreamHandler(testBlockAnnounceMessageDecoder) nodeB.host.registerStreamHandler("", handlerB.handleStream) - addrInfosA, err := nodeA.host.addrInfos() - require.NoError(t, err) - - err = nodeB.host.connect(*addrInfosA[0]) + addrInfoA := nodeA.host.addrInfo() + err := nodeB.host.connect(addrInfoA) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeB.host.connect(*addrInfosA[0]) + err = nodeB.host.connect(addrInfoA) } require.NoError(t, err) @@ -79,26 +77,24 @@ func TestGossip(t *testing.T) { handlerC := newTestStreamHandler(testBlockAnnounceMessageDecoder) nodeC.host.registerStreamHandler("", handlerC.handleStream) - err = nodeC.host.connect(*addrInfosA[0]) + err = nodeC.host.connect(addrInfoA) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeC.host.connect(*addrInfosA[0]) + err = nodeC.host.connect(addrInfoA) } require.NoError(t, err) - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeC.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err = nodeC.host.connect(addrInfoB) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeC.host.connect(*addrInfosB[0]) + err = nodeC.host.connect(addrInfoB) } require.NoError(t, err) - _, err = nodeA.host.send(addrInfosB[0].ID, "", testBlockAnnounceMessage) + _, err = nodeA.host.send(addrInfoB.ID, "", testBlockAnnounceMessage) require.NoError(t, err) time.Sleep(TestMessageTimeout) diff --git a/dot/network/host.go b/dot/network/host.go index 84cbd8bcc2..e500f5c708 100644 --- a/dot/network/host.go +++ b/dot/network/host.go @@ -379,16 +379,12 @@ func (h *host) peerCount() int { return len(peers) } -// addrInfos returns the libp2p AddrInfos of the host -func (h *host) addrInfos() (addrInfos []*peer.AddrInfo, err error) { - for _, multiaddr := range h.multiaddrs() { - addrInfo, err := peer.AddrInfoFromP2pAddr(multiaddr) - if err != nil { - return nil, err - } - addrInfos = append(addrInfos, addrInfo) +// addrInfo returns the libp2p peer.AddrInfo of the host +func (h *host) addrInfo() peer.AddrInfo { + return peer.AddrInfo{ + ID: h.h.ID(), + Addrs: h.h.Addrs(), } - return addrInfos, nil } // multiaddrs returns the multiaddresses of the host diff --git a/dot/network/host_test.go b/dot/network/host_test.go index 3409fa3fe4..4cb3895008 100644 --- a/dot/network/host_test.go +++ b/dot/network/host_test.go @@ -39,9 +39,7 @@ func TestExternalAddrs(t *testing.T) { node := createTestService(t, config) - addrInfos, err := node.host.addrInfos() - require.NoError(t, err) - + addrInfo := node.host.addrInfo() privateIPs := ma.NewFilters() for _, cidr := range privateCIDRs { _, ipnet, err := net.ParseCIDR(cidr) //nolint @@ -49,10 +47,8 @@ func TestExternalAddrs(t *testing.T) { privateIPs.AddFilter(*ipnet, ma.ActionDeny) } - for _, info := range addrInfos { - for _, addr := range info.Addrs { - require.False(t, privateIPs.AddrBlocked(addr)) - } + for _, addr := range addrInfo.Addrs { + require.False(t, privateIPs.AddrBlocked(addr)) } } @@ -82,14 +78,12 @@ func TestConnect(t *testing.T) { nodeB := createTestService(t, configB) nodeB.noGossip = true - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) @@ -196,18 +190,16 @@ func TestSend(t *testing.T) { handler := newTestStreamHandler(testBlockRequestMessageDecoder) nodeB.host.registerStreamHandler("", handler.handleStream) - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) - _, err = nodeA.host.send(addrInfosB[0].ID, nodeB.host.protocolID, testBlockRequestMessage) + _, err = nodeA.host.send(addrInfoB.ID, nodeB.host.protocolID, testBlockRequestMessage) require.NoError(t, err) time.Sleep(TestMessageTimeout) @@ -233,9 +225,7 @@ func TestExistingStream(t *testing.T) { handlerA := newTestStreamHandler(testBlockRequestMessageDecoder) nodeA.host.registerStreamHandler("", handlerA.handleStream) - addrInfosA, err := nodeA.host.addrInfos() - require.NoError(t, err) - + addrInfoA := nodeA.host.addrInfo() basePathB := utils.NewTestBasePath(t, "nodeB") configB := &Config{ BasePath: basePathB, @@ -249,19 +239,17 @@ func TestExistingStream(t *testing.T) { handlerB := newTestStreamHandler(testBlockRequestMessageDecoder) nodeB.host.registerStreamHandler("", handlerB.handleStream) - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) // node A opens the stream to send the first message - stream, err := nodeA.host.send(addrInfosB[0].ID, nodeB.host.protocolID, testBlockRequestMessage) + stream, err := nodeA.host.send(addrInfoB.ID, nodeB.host.protocolID, testBlockRequestMessage) require.NoError(t, err) time.Sleep(TestMessageTimeout) @@ -273,7 +261,7 @@ func TestExistingStream(t *testing.T) { require.NotNil(t, handlerB.messages[nodeA.host.id()], "node B timeout waiting for message from node A") // node B opens the stream to send the first message - stream, err = nodeB.host.send(addrInfosA[0].ID, nodeB.host.protocolID, testBlockRequestMessage) + stream, err = nodeB.host.send(addrInfoA.ID, nodeB.host.protocolID, testBlockRequestMessage) require.NoError(t, err) time.Sleep(TestMessageTimeout) @@ -312,14 +300,12 @@ func TestStreamCloseMetadataCleanup(t *testing.T) { handlerB := newTestStreamHandler(testBlockAnnounceHandshakeDecoder) nodeB.host.registerStreamHandler(blockAnnounceID, handlerB.handleStream) - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) @@ -379,14 +365,12 @@ func Test_PeerSupportsProtocol(t *testing.T) { nodeB := createTestService(t, configB) nodeB.noGossip = true - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) diff --git a/dot/network/light_test.go b/dot/network/light_test.go index 219d9f0b0e..268a8eeefa 100644 --- a/dot/network/light_test.go +++ b/dot/network/light_test.go @@ -62,14 +62,12 @@ func TestHandleLightMessage_Response(t *testing.T) { } b := createTestService(t, configB) - addrInfosB, err := b.host.addrInfos() - require.NoError(t, err) - - err = s.host.connect(*addrInfosB[0]) + addrInfoB := b.host.addrInfo() + err := s.host.connect(addrInfoB) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = s.host.connect(*addrInfosB[0]) + err = s.host.connect(addrInfoB) } require.NoError(t, err) diff --git a/dot/network/notifications_test.go b/dot/network/notifications_test.go index 4b1ed7093e..dc85782d67 100644 --- a/dot/network/notifications_test.go +++ b/dot/network/notifications_test.go @@ -126,13 +126,11 @@ func TestCreateNotificationsMessageHandler_BlockAnnounce(t *testing.T) { testPeerID := b.host.id() // connect nodes - addrInfosB, err := b.host.addrInfos() - require.NoError(t, err) - - err = s.host.connect(*addrInfosB[0]) + addrInfoB := b.host.addrInfo() + err := s.host.connect(addrInfoB) if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = s.host.connect(*addrInfosB[0]) + err = s.host.connect(addrInfoB) } require.NoError(t, err) @@ -195,13 +193,11 @@ func TestCreateNotificationsMessageHandler_BlockAnnounceHandshake(t *testing.T) testPeerID := b.host.id() // connect nodes - addrInfosB, err := b.host.addrInfos() - require.NoError(t, err) - - err = s.host.connect(*addrInfosB[0]) + addrInfoB := b.host.addrInfo() + err := s.host.connect(addrInfoB) if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = s.host.connect(*addrInfosB[0]) + err = s.host.connect(addrInfoB) } require.NoError(t, err) diff --git a/dot/network/service_test.go b/dot/network/service_test.go index cdeb36aa86..814abeb90e 100644 --- a/dot/network/service_test.go +++ b/dot/network/service_test.go @@ -166,14 +166,12 @@ func TestBroadcastMessages(t *testing.T) { handler := newTestStreamHandler(testBlockAnnounceHandshakeDecoder) nodeB.host.registerStreamHandler(blockAnnounceID, handler.handleStream) - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) @@ -213,14 +211,12 @@ func TestBroadcastDuplicateMessage(t *testing.T) { handler := newTestStreamHandler(testBlockAnnounceHandshakeDecoder) nodeB.host.registerStreamHandler(blockAnnounceID, handler.handleStream) - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) // retry connect if "failed to dial" error if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) @@ -288,13 +284,11 @@ func TestPersistPeerStore(t *testing.T) { nodeA := nodes[0] nodeB := nodes[1] - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) @@ -328,13 +322,11 @@ func TestHandleConn(t *testing.T) { nodeB := createTestService(t, configB) - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) diff --git a/dot/network/sync_test.go b/dot/network/sync_test.go index b95d073504..f79a8d74f4 100644 --- a/dot/network/sync_test.go +++ b/dot/network/sync_test.go @@ -314,24 +314,20 @@ func TestSyncQueue_ProcessBlockRequests(t *testing.T) { nodeC.noGossip = true // connect A and B - addrInfosB, err := nodeB.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosB[0]) + addrInfoB := nodeB.host.addrInfo() + err := nodeA.host.connect(addrInfoB) if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosB[0]) + err = nodeA.host.connect(addrInfoB) } require.NoError(t, err) // connect A and C - addrInfosC, err := nodeC.host.addrInfos() - require.NoError(t, err) - - err = nodeA.host.connect(*addrInfosC[0]) + addrInfoC := nodeC.host.addrInfo() + err = nodeA.host.connect(addrInfoC) if failedToDial(err) { time.Sleep(TestBackoffTimeout) - err = nodeA.host.connect(*addrInfosC[0]) + err = nodeA.host.connect(addrInfoC) } require.NoError(t, err) From 04b49302be3950b36daf4930c2291d385c147a0c Mon Sep 17 00:00:00 2001 From: noot Date: Tue, 8 Jun 2021 12:41:14 -0400 Subject: [PATCH 5/8] increase connect timeout to 5s --- dot/network/host.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dot/network/host.go b/dot/network/host.go index e500f5c708..d731328818 100644 --- a/dot/network/host.go +++ b/dot/network/host.go @@ -48,6 +48,8 @@ var privateCIDRs = []string{ "169.254.0.0/16", } +var connectTimeout = time.Second * 5 + // host wraps libp2p host with network host configuration and services type host struct { ctx context.Context @@ -241,7 +243,7 @@ func (h *host) registerStreamHandlerWithOverwrite(pid protocol.ID, overwrite boo // connect connects the host to a specific peer address func (h *host) connect(p peer.AddrInfo) (err error) { h.h.Peerstore().AddAddrs(p.ID, p.Addrs, peerstore.PermanentAddrTTL) - ctx, cancel := context.WithTimeout(h.ctx, time.Second*2) + ctx, cancel := context.WithTimeout(h.ctx, connectTimeout) defer cancel() err = h.h.Connect(ctx, p) return err From f9b0642c8633e428f0ab208d466a1bc137ceb51b Mon Sep 17 00:00:00 2001 From: Arijit Das Date: Wed, 9 Jun 2021 00:07:36 +0530 Subject: [PATCH 6/8] Fix handshake timeout test. --- dot/network/notifications.go | 1 - dot/network/notifications_test.go | 79 ++++++++++++++++--------------- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/dot/network/notifications.go b/dot/network/notifications.go index 1cb102d0c5..cb44e68894 100644 --- a/dot/network/notifications.go +++ b/dot/network/notifications.go @@ -248,7 +248,6 @@ func (s *Service) sendData(peer peer.ID, hs Handshake, info *notificationsProtoc _ = stream.Close() info.outboundHandshakeData.Delete(peer) return - case hsResponse := <-s.readHandshake(stream, decodeBlockAnnounceHandshake): hsTimer.Stop() if hsResponse.err != nil { diff --git a/dot/network/notifications_test.go b/dot/network/notifications_test.go index dc85782d67..68bbabcb82 100644 --- a/dot/network/notifications_test.go +++ b/dot/network/notifications_test.go @@ -17,7 +17,6 @@ package network import ( - "context" "fmt" "math/big" "sync" @@ -27,9 +26,6 @@ import ( "github.com/ChainSafe/gossamer/dot/types" "github.com/ChainSafe/gossamer/lib/common" "github.com/ChainSafe/gossamer/lib/utils" - ma "github.com/multiformats/go-multiaddr" - - "github.com/libp2p/go-libp2p" libp2pnetwork "github.com/libp2p/go-libp2p-core/network" "github.com/libp2p/go-libp2p-core/peer" "github.com/stretchr/testify/require" @@ -238,31 +234,50 @@ func TestCreateNotificationsMessageHandler_BlockAnnounceHandshake(t *testing.T) } func Test_HandshakeTimeout(t *testing.T) { - // create service A - config := &Config{ - BasePath: utils.NewTestBasePath(t, "nodeA"), + basePathA := utils.NewTestBasePath(t, "nodeA") + configA := &Config{ + BasePath: basePathA, Port: 7001, NoBootstrap: true, NoMDNS: true, } - ha := createTestService(t, config) + + nodeA := createTestService(t, configA) + nodeA.noGossip = true + + basePathB := utils.NewTestBasePath(t, "nodeB") + configB := &Config{ + BasePath: basePathB, + Port: 7002, + RandSeed: 2, + NoBootstrap: true, + NoMDNS: true, + } + + nodeB := createTestService(t, configB) + nodeB.noGossip = true // create info and handler info := ¬ificationsProtocol{ - protocolID: ha.host.protocolID + blockAnnounceID, - getHandshake: ha.getBlockAnnounceHandshake, - handshakeValidator: ha.validateBlockAnnounceHandshake, + protocolID: nodeA.host.protocolID + blockAnnounceID, + getHandshake: nodeA.getBlockAnnounceHandshake, + handshakeValidator: nodeA.validateBlockAnnounceHandshake, inboundHandshakeData: new(sync.Map), outboundHandshakeData: new(sync.Map), } - // creating host b with will never respond to a handshake - addrB, err := ma.NewMultiaddr(fmt.Sprintf("/ip4/0.0.0.0/tcp/%d", 7002)) - require.NoError(t, err) + nodeB.host.h.SetStreamHandler(info.protocolID, func(stream libp2pnetwork.Stream) { + fmt.Println("never respond a handshake message") + }) - hb, err := libp2p.New( - context.Background(), libp2p.ListenAddrs(addrB), - ) + addrInfosB := nodeB.host.addrInfo() + + err := nodeA.host.connect(addrInfosB) + // retry connect if "failed to dial" error + if failedToDial(err) { + time.Sleep(TestBackoffTimeout) + err = nodeA.host.connect(addrInfosB) + } require.NoError(t, err) testHandshakeMsg := &BlockAnnounceHandshake{ @@ -271,32 +286,18 @@ func Test_HandshakeTimeout(t *testing.T) { BestBlockHash: common.Hash{1}, GenesisHash: common.Hash{2}, } + nodeA.SendMessage(testHandshakeMsg) - hb.SetStreamHandler(info.protocolID, func(stream libp2pnetwork.Stream) { - fmt.Println("never respond a handshake message") - }) - - addrBInfo := peer.AddrInfo{ - ID: hb.ID(), - Addrs: hb.Addrs(), - } - - err = ha.host.connect(addrBInfo) - if failedToDial(err) { - time.Sleep(TestBackoffTimeout) - err = ha.host.connect(addrBInfo) - } - require.NoError(t, err) + go nodeA.sendData(nodeB.host.id(), testHandshakeMsg, info, nil) - go ha.sendData(hb.ID(), testHandshakeMsg, info, nil) + time.Sleep(time.Second) - time.Sleep(handshakeTimeout / 2) - // peer should be stored in handshake data until timeout - _, ok := info.outboundHandshakeData.Load(hb.ID()) + // Verify that handshake data exists. + _, ok := info.getHandshakeData(nodeB.host.id(), false) require.True(t, ok) // a stream should be open until timeout - connAToB := ha.host.h.Network().ConnsToPeer(hb.ID()) + connAToB := nodeA.host.h.Network().ConnsToPeer(nodeB.host.id()) require.Len(t, connAToB, 1) require.Len(t, connAToB[0].GetStreams(), 1) @@ -304,11 +305,11 @@ func Test_HandshakeTimeout(t *testing.T) { time.Sleep(handshakeTimeout) // handshake data should be removed - _, ok = info.outboundHandshakeData.Load(hb.ID()) + _, ok = info.getHandshakeData(nodeB.host.id(), false) require.False(t, ok) // stream should be closed - connAToB = ha.host.h.Network().ConnsToPeer(hb.ID()) + connAToB = nodeA.host.h.Network().ConnsToPeer(nodeB.host.id()) require.Len(t, connAToB, 1) require.Len(t, connAToB[0].GetStreams(), 0) } From 1b7ae67b68ed4b7549e90ac8a75d781c56992b38 Mon Sep 17 00:00:00 2001 From: Arijit Das Date: Wed, 9 Jun 2021 00:08:33 +0530 Subject: [PATCH 7/8] Fix leaky go routines. --- dot/network/connmgr.go | 26 ++++++++++++++++++-------- dot/network/host.go | 24 ++++++++++++++---------- dot/rpc/modules/system_test.go | 7 +++++++ 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/dot/network/connmgr.go b/dot/network/connmgr.go index 276ad0e6e6..c64ee3bad0 100644 --- a/dot/network/connmgr.go +++ b/dot/network/connmgr.go @@ -197,15 +197,25 @@ func (cm *ConnManager) Disconnected(n network.Network, c network.Conn) { } go func() { - for i := 0; i < maxRetries; i++ { - err := cm.host.connect(info) - if err != nil { - logger.Warn("failed to reconnect to persistent peer", "peer", c.RemotePeer(), "error", err) - time.Sleep(time.Minute) - continue + retry := 0 + retryTimer := time.NewTicker(time.Minute) + defer retryTimer.Stop() + for { + select { + case <-cm.host.ctx.Done(): + return + case <-retryTimer.C: + err := cm.host.connect(info) + if err != nil { + logger.Warn("failed to reconnect to persistent peer", "peer", c.RemotePeer(), "error", err) + continue + } + + retry++ + if retry > maxRetries { + return + } } - - return } }() diff --git a/dot/network/host.go b/dot/network/host.go index d731328818..fe0f73ce1c 100644 --- a/dot/network/host.go +++ b/dot/network/host.go @@ -21,6 +21,7 @@ import ( "fmt" "net" "path" + "sync" "time" "github.com/dgraph-io/ristretto" @@ -62,6 +63,7 @@ type host struct { ds *badger.Datastore messageCache *messageCache bwc *metrics.BandwidthCounter + closeSync sync.Once } // newHost creates a host wrapper with a new libp2p host instance @@ -206,17 +208,19 @@ func (h *host) close() error { return err } - err = h.h.Peerstore().Close() - if err != nil { - logger.Error("Failed to close libp2p peerstore", "error", err) - return err - } + h.closeSync.Do(func() { + err = h.h.Peerstore().Close() + if err != nil { + logger.Error("Failed to close libp2p peerstore", "error", err) + return + } - err = h.ds.Close() - if err != nil { - logger.Error("Failed to close libp2p host datastore", "error", err) - return err - } + err = h.ds.Close() + if err != nil { + logger.Error("Failed to close libp2p host datastore", "error", err) + return + } + }) return nil } diff --git a/dot/rpc/modules/system_test.go b/dot/rpc/modules/system_test.go index a632029709..d606e5058e 100644 --- a/dot/rpc/modules/system_test.go +++ b/dot/rpc/modules/system_test.go @@ -17,10 +17,12 @@ package modules import ( + "fmt" "math/big" "os" "path" "testing" + "time" "github.com/ChainSafe/gossamer/dot/network" "github.com/ChainSafe/gossamer/dot/state" @@ -121,6 +123,11 @@ func newNetworkService(t *testing.T) *network.Service { t.Cleanup(func() { _ = srv.Stop() + time.Sleep(time.Second) + err = os.RemoveAll(cfg.BasePath) + if err != nil { + fmt.Printf("failed to remove path %s : %s\n", cfg.BasePath, err) + } }) return srv From ed283b1ff9168efdbfeb0964e47e32cd25d62483 Mon Sep 17 00:00:00 2001 From: Arijit Das Date: Wed, 9 Jun 2021 00:52:37 +0530 Subject: [PATCH 8/8] Fix test. --- dot/network/connmgr.go | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/dot/network/connmgr.go b/dot/network/connmgr.go index c64ee3bad0..769c6c1a8a 100644 --- a/dot/network/connmgr.go +++ b/dot/network/connmgr.go @@ -196,8 +196,26 @@ func (cm *ConnManager) Disconnected(n network.Network, c network.Conn) { Addrs: addrs, } + count := 0 + retry := func() bool { + err := cm.host.connect(info) + if err != nil { + logger.Warn("failed to reconnect to persistent peer", "peer", c.RemotePeer(), "error", err) + return false + } + + count++ + if count > maxRetries { + return true + } + return true + } + go func() { - retry := 0 + if retry() { + return + } + retryTimer := time.NewTicker(time.Minute) defer retryTimer.Stop() for { @@ -205,14 +223,7 @@ func (cm *ConnManager) Disconnected(n network.Network, c network.Conn) { case <-cm.host.ctx.Done(): return case <-retryTimer.C: - err := cm.host.connect(info) - if err != nil { - logger.Warn("failed to reconnect to persistent peer", "peer", c.RemotePeer(), "error", err) - continue - } - - retry++ - if retry > maxRetries { + if retry() { return } }