Merge pull request #288 from jenyasd209/jenyasd209/fix-race-conditions

CHANGELOG-PENDING.md

@@ -25,3 +25,4 @@ Month, DD, YYYY
 - [go package] (Link to PR) Description @username
 
 - [header] Added missing `err` value in ErrorW logging calls. @jbowen93
+- [service/block, node/p2p] [Fix race conditions in TestExtendedHeaderBroadcast and TestFull_P2P_Streams.](https://github.com/celestiaorg/celestia-node/pull/288) @jenyasd209

node/p2p/addrs.go

@@ -45,17 +45,19 @@ func AddrsFactory(announce []string, noAnnounce []string) func() (_ p2pconfig.Ad
 			maNoAnnounce[string(maddr.Bytes())] = true
 		}
 
-		return func(maListen []ma.Multiaddr) (out []ma.Multiaddr) {
-			// combine maListen and maAnnounce addresses
-			maAnnounce = append(maAnnounce, maListen...)
+		return func(maListen []ma.Multiaddr) []ma.Multiaddr {
+			// copy maAnnounce to out
+			out := make([]ma.Multiaddr, 0, len(maAnnounce)+len(maListen))
+			out = append(out, maAnnounce...)
+
 			// filter out unneeded
 			for _, maddr := range maListen {
 				ok := maNoAnnounce[string(maddr.Bytes())]
 				if !ok {
 					out = append(out, maddr)
 				}
 			}
-			return
+			return out
 		}, nil
 	}
 }

service/block/event_test.go

@@ -2,6 +2,7 @@ package block
 
 import (
 	"context"
+	"sync"
 	"testing"
 	"time"
 
@@ -58,7 +59,9 @@ func TestExtendedHeaderBroadcast(t *testing.T) {
 
 	mockFetcher := &mockFetcher{
 		suite:          suite,
+		commitsLock:    sync.Mutex{},
 		commits:        make(map[int64]*core.Commit),
+		valSetsLock:    sync.Mutex{},
 		valSets:        make(map[int64]*core.ValidatorSet),
 		mockNewBlockCh: make(chan *RawBlock),
 	}
@@ -106,9 +109,14 @@ func TestExtendedHeaderBroadcast(t *testing.T) {
 
 // mockFetcher mocks away the `Fetcher` interface.
 type mockFetcher struct {
-	suite          *header.TestSuite
-	valSets        map[int64]*core.ValidatorSet
-	commits        map[int64]*core.Commit
+	suite *header.TestSuite
+
+	valSetsLock sync.Mutex
+	valSets     map[int64]*core.ValidatorSet
+
+	commitsLock sync.Mutex
+	commits     map[int64]*core.Commit
+
 	mockNewBlockCh chan *RawBlock
 }
 
@@ -117,10 +125,16 @@ func (m *mockFetcher) GetBlock(ctx context.Context, height *int64) (*RawBlock, e
 }
 
 func (m *mockFetcher) Commit(ctx context.Context, height *int64) (*core.Commit, error) {
+	m.commitsLock.Lock()
+	defer m.commitsLock.Unlock()
+
 	return m.commits[*height], nil
 }
 
 func (m *mockFetcher) ValidatorSet(ctx context.Context, height *int64) (*core.ValidatorSet, error) {
+	m.valSetsLock.Lock()
+	defer m.valSetsLock.Unlock()
+
 	return m.valSets[*height], nil
 }
 
@@ -149,8 +163,13 @@ func (m *mockFetcher) generateBlocksWithValidHeaders(t *testing.T, num int) []*R
 
 		rawBlocks[i] = b
 		// store commit and valset at height
+		m.commitsLock.Lock()
 		m.commits[b.Height] = eh.Commit
+		m.commitsLock.Unlock()
+
+		m.valSetsLock.Lock()
 		m.valSets[b.Height] = eh.ValidatorSet
+		m.valSetsLock.Unlock()
 
 		m.mockNewBlockCh <- b
 		prevEH = eh