fix(syncer): don't Head recursively (#248)

We don't need recursion here. If recent head request fails, all the
waiters should simply return any known subjective head, like with other
error cases.

Closes #247

Author: Wondertan

sync/sync_head.go

@@ -31,9 +31,8 @@ func (s *Syncer[H]) Head(ctx context.Context, _ ...header.HeadOption[H]) (H, err
 
 	// single-flight protection ensure only one Head is requested at the time
 	if !s.getter.Lock() {
-		// means that other routine held the lock and set the subjective head for us,
-		// so just recursively get it
-		return s.Head(ctx)
+		// means that other routine held the lock and set the subjective head
+		return s.subjectiveHead(ctx)
 	}
 	defer s.getter.Unlock()
 

sync/sync_head_test.go

@@ -2,6 +2,7 @@ package sync
 
 import (
 	"context"
+	"fmt"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -15,6 +16,35 @@ import (
 	"github.com/celestiaorg/go-header/local"
 )
 
+func TestSyncer_HeadConcurrencyError(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
+	t.Cleanup(cancel)
+
+	suite := headertest.NewTestSuite(t)
+	store := headertest.NewStore[*headertest.DummyHeader](t, suite, 1)
+
+	syncer, err := NewSyncer[*headertest.DummyHeader](
+		errorGetter{},
+		store,
+		headertest.NewDummySubscriber(),
+		WithRecencyThreshold(time.Nanosecond), // force recent requests
+	)
+	require.NoError(t, err)
+
+	var wg sync.WaitGroup
+	for i := 0; i < 1000; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			_, err := syncer.Head(ctx)
+			require.NoError(t, err)
+		}()
+	}
+
+	wg.Wait()
+}
+
 func TestSyncer_incomingNetworkHeadRaces(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
 	t.Cleanup(cancel)
@@ -121,12 +151,10 @@ func (t *wrappedGetter) Head(ctx context.Context, options ...header.HeadOption[*
 }
 
 func (t *wrappedGetter) Get(ctx context.Context, hash header.Hash) (*headertest.DummyHeader, error) {
-	// TODO implement me
 	panic("implement me")
 }
 
 func (t *wrappedGetter) GetByHeight(ctx context.Context, u uint64) (*headertest.DummyHeader, error) {
-	// TODO implement me
 	panic("implement me")
 }
 
@@ -135,6 +163,24 @@ func (t *wrappedGetter) GetRangeByHeight(
 	from *headertest.DummyHeader,
 	to uint64,
 ) ([]*headertest.DummyHeader, error) {
-	// TODO implement me
+	panic("implement me")
+}
+
+type errorGetter struct{}
+
+func (e errorGetter) Head(ctx context.Context, h ...header.HeadOption[*headertest.DummyHeader]) (*headertest.DummyHeader, error) {
+	time.Sleep(time.Millisecond * 1)
+	return nil, fmt.Errorf("error")
+}
+
+func (e errorGetter) Get(ctx context.Context, hash header.Hash) (*headertest.DummyHeader, error) {
+	panic("implement me")
+}
+
+func (e errorGetter) GetByHeight(ctx context.Context, u uint64) (*headertest.DummyHeader, error) {
+	panic("implement me")
+}
+
+func (e errorGetter) GetRangeByHeight(ctx context.Context, from *headertest.DummyHeader, to uint64) ([]*headertest.DummyHeader, error) {
 	panic("implement me")
 }