From b9fa9aa58e09d95b4e8bc34487025792d602587d Mon Sep 17 00:00:00 2001
From: rene <41963722+renaynay@users.noreply.github.com>
Date: Thu, 14 Mar 2024 18:54:51 +0100
Subject: [PATCH] chore(sync)!: Rename `unrecentHead` to `outdatedHead` in
 metrics + clean up `Head` func a bit (#163)

This PR renames a poorly named metric from `unrecent_header` to `outdated_head` which is more descriptive / accurate.

It also cleans up a bit of the code inside of `sync_head.go`.

This PR is meant to override #162 if we agree that the timeout should not be configurable (see [comment](https://github.com/celestiaorg/go-header/issues/155#issuecomment-1983516622)
---
 sync/metrics.go   | 14 +++++++-------
 sync/sync_head.go | 20 +++++++++++---------
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/sync/metrics.go b/sync/metrics.go
index a0916720..590bd5ba 100644
--- a/sync/metrics.go
+++ b/sync/metrics.go
@@ -20,7 +20,7 @@ type metrics struct {
 
 	syncLoopStarted       metric.Int64Counter
 	trustedPeersOutOfSync metric.Int64Counter
-	unrecentHeader        metric.Int64Counter
+	outdatedHeader        metric.Int64Counter
 	subjectiveInit        metric.Int64Counter
 
 	subjectiveHead atomic.Int64
@@ -53,9 +53,9 @@ func newMetrics() (*metrics, error) {
 		return nil, err
 	}
 
-	unrecentHeader, err := meter.Int64Counter(
-		"hdr_sync_unrecent_header_counter",
-		metric.WithDescription("tracks every time Syncer returns an unrecent header"),
+	outdatedHeader, err := meter.Int64Counter(
+		"hdr_sync_outdated_head_counter",
+		metric.WithDescription("tracks every time Syncer returns an outdated head"),
 	)
 	if err != nil {
 		return nil, err
@@ -110,7 +110,7 @@ func newMetrics() (*metrics, error) {
 	m := &metrics{
 		syncLoopStarted:       syncLoopStarted,
 		trustedPeersOutOfSync: trustedPeersOutOfSync,
-		unrecentHeader:        unrecentHeader,
+		outdatedHeader:        outdatedHeader,
 		subjectiveInit:        subjectiveInit,
 		syncLoopDurationHist:  syncLoopDurationHist,
 		syncLoopRunningInst:   syncLoopRunningInst,
@@ -148,9 +148,9 @@ func (m *metrics) syncFinished(ctx context.Context) {
 	})
 }
 
-func (m *metrics) unrecentHead(ctx context.Context) {
+func (m *metrics) outdatedHead(ctx context.Context) {
 	m.observe(ctx, func(ctx context.Context) {
-		m.unrecentHeader.Add(ctx, 1)
+		m.outdatedHeader.Add(ctx, 1)
 	})
 }
 
diff --git a/sync/sync_head.go b/sync/sync_head.go
index ad84344b..a5a97a3a 100644
--- a/sync/sync_head.go
+++ b/sync/sync_head.go
@@ -8,6 +8,10 @@ import (
 	"github.com/celestiaorg/go-header"
 )
 
+// headRequestTimeout is the amount of time the syncer is willing to wait for
+// the exchange to request the head of the chain from the network.
+var headRequestTimeout = time.Second * 2
+
 // Head returns the Network Head.
 //
 // Known subjective head is considered network head if it is recent enough(now-timestamp<=blocktime)
@@ -24,27 +28,25 @@ func (s *Syncer[H]) Head(ctx context.Context, _ ...header.HeadOption[H]) (H, err
 	if isRecent(sbjHead, s.Params.blockTime, s.Params.recencyThreshold) {
 		return sbjHead, nil
 	}
-	// otherwise, request head from the network
-	// TODO: Besides requesting we should listen for new gossiped headers and cancel request if so
-	//
-	// single-flight protection
-	// ensure only one Head is requested at the time
+
+	// 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)
 	}
 	defer s.getter.Unlock()
-	// limit time to get a recent header
-	// if we can't get it - give what we have
-	reqCtx, cancel := context.WithTimeout(ctx, time.Second*2) // TODO(@vgonkivs): make timeout configurable
+
+	s.metrics.outdatedHead(s.ctx)
+
+	reqCtx, cancel := context.WithTimeout(ctx, headRequestTimeout)
 	defer cancel()
-	s.metrics.unrecentHead(s.ctx)
 	netHead, err := s.getter.Head(reqCtx, header.WithTrustedHead[H](sbjHead))
 	if err != nil {
 		log.Warnw("failed to get recent head, returning current subjective", "sbjHead", sbjHead.Height(), "err", err)
 		return s.subjectiveHead(ctx)
 	}
+
 	// process and validate netHead fetched from trusted peers
 	// NOTE: We could trust the netHead like we do during 'automatic subjective initialization'
 	// but in this case our subjective head is not expired, so we should verify netHead