diff --git a/CHANGELOG-PENDING.md b/CHANGELOG-PENDING.md index 3f2d86529f..9d4a9a8067 100644 --- a/CHANGELOG-PENDING.md +++ b/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 diff --git a/node/p2p/addrs.go b/node/p2p/addrs.go index 949c65b5d6..70a9fa8fde 100644 --- a/node/p2p/addrs.go +++ b/node/p2p/addrs.go @@ -45,9 +45,11 @@ 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())] @@ -55,7 +57,7 @@ func AddrsFactory(announce []string, noAnnounce []string) func() (_ p2pconfig.Ad out = append(out, maddr) } } - return + return out }, nil } } diff --git a/service/block/event_test.go b/service/block/event_test.go index a193966d83..6dda54a1db 100644 --- a/service/block/event_test.go +++ b/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