fix(sync): stall headersub until Syncer has started (#317)

A better version of #316. This version should be easier to understand,
but it also addresses a corruption case that I realized #316 doesn't
cover. If, for whatever reason, Tail request during the very first start
takes more time, then the duplicate subjective init, triggered by the
network head from headersub, then the network head will be stored before
tail, breaking the further syncing attempts irrecoverably.

This will be removed with bsync once the requirement to store Tail
before Head is removed.

EDIT:
[f569260](f569260)
completely stalls headersub until started on every start, not just subj
init. During a regular start, a headersub header may frontrun the Start
routine and trigger Tail update first. This is problematic as the node
assumes the Tail is up-to-date after Start finishes, but in the
front-running case, only the headersub routine gets blocked (see #313).

Extensively tested manually.

Author: Wondertan

sync/syncer.go

@@ -53,6 +53,10 @@ type Syncer[H header.Header[H]] struct {
 	cancel context.CancelFunc
 
 	Params *Parameters
+
+	// a hack to prevent headersub interfering during start
+	// TODO(@Wondertan): Remove after bsync.
+	started chan struct{}
 }
 
 // NewSyncer creates a new instance of Syncer.
@@ -86,15 +90,35 @@ func NewSyncer[H header.Header[H]](
 		metrics:     metrics,
 		triggerSync: make(chan struct{}, 1), // should be buffered
 		Params:      &params,
+		started:     make(chan struct{}),
 	}, nil
 }
 
 // Start starts the syncing routine.
 func (s *Syncer[H]) Start(ctx context.Context) error {
 	s.ctx, s.cancel = context.WithCancel(context.Background())
+
 	// register validator for header subscriptions
 	// syncer does not subscribe itself and syncs headers together with validation
 	err := s.sub.SetVerifier(func(ctx context.Context, h H) error {
+		// HACK(@Wondertan):
+		//  During subjective init we request the Head and then the Tail.
+		//  However, we store them in the opposite order, s.t before Tail arrives
+		//  there is a window where Head awaits Tail without being stored.
+		//  During this window, headersub may deliver new network head. As it can't
+		//  see the awaiting Head, it triggers duplicate subjective initialization
+		//  which may frontrun Tail, violating the storing order and corrupting the store.
+		//  This hack basically stalls headersub until Syncer has fully started, but it should
+		//  not be this way.
+		//
+		//  This will be fixed with bsync as Syncer will learn how to verify Tail from Head backwards,
+		//  allowing Head to be stored first and then the Tail.
+		select {
+		case <-s.started:
+		case <-ctx.Done():
+			return ctx.Err()
+		}
+
 		if err := s.incomingNetworkHead(ctx, h); err != nil {
 			return err
 		}
@@ -115,6 +139,11 @@ func (s *Syncer[H]) Start(ctx context.Context) error {
 	}
 	// start syncLoop only if Start is errorless
 	go s.syncLoop()
+	select {
+	case <-s.started:
+	default:
+		close(s.started)
+	}
 	return nil
 }