MinError calculation fixes

MinError was being calculated incorrectly. Restored correct
"causality violation" calculation and added better tests.

Author: beevik

ntp.go

@@ -341,12 +341,8 @@ type Response struct {
 	// the current month's last minute.
 	Leap LeapIndicator
 
-	// MinError is a lower bound on the error between the client and server
-	// clocks. When the client and server are not synchronized to the same
-	// clock, the reported timestamps may appear to violate the principle of
-	// causality. In other words, the NTP server's response may indicate
-	// that a message was received before it was sent. In such cases, the
-	// minimum error may be useful.
+	// MinError is a lower bound on the "causality violation" between the
+	// client and server clocks when they are not yet synchronized.
 	MinError time.Duration
 
 	// KissCode is a 4-character string describing the reason for a "kiss of
@@ -754,11 +750,32 @@ func rtt(t1, t2, t3, t4 time.Time) time.Duration {
 	return max(t4.Sub(t1)-t3.Sub(t2), 0)
 }
 
-// minError calculates a lower bound on the error between the client and
-// server clocks using the four NTP timestamps.
+// minError calculates a lower bound on the "causality violation" detected
+// between the client and server clocks when they are not yet synchronized.
 func minError(t1, t2, t3, t4 time.Time) time.Duration {
-	if t2.Before(t1) || t4.Before(t3) {
-		return max(t1.Sub(t2), t3.Sub(t4))
+	// rtt    = (t4 - t1) - (t3 - t2)
+	// offset = ((t2 - t1) + (t3 - t4)) / 2
+	//
+	// t2 - t1 = rtt/2 + offset
+	// t4 - t3 = rtt/2 - offset
+	//
+	// If the client and server clocks are synchronized, then the offset
+	// should be roughly zero and the timestamp differences should be roughly
+	// equal to the one-way trip time (rtt/2). If the clocks are not
+	// synchronized, then the magnitude of the offset relative to the one-way
+	// trip time provides a lower bound on the "causality violation" between
+	// the two clocks.
+
+	offset := offset(t1, t2, t3, t4)
+	if offset < 0 {
+		offset = -offset
+	}
+
+	rtt := rtt(t1, t2, t3, t4)
+	ott := rtt / 2
+
+	if offset > ott {
+		return offset - ott
 	}
 	return 0
 }

ntp4_test.go

@@ -226,37 +226,6 @@ func TestOfflineV4CustomDialerDeprecated(t *testing.T) {
 	assert.True(t, dialerCalled)
 }
 
-func TestOfflineV4MinError(t *testing.T) {
-	start := time.Now()
-
-	for org := 1 * time.Second; org <= 10*time.Second; org += time.Second {
-		for rec := 1 * time.Second; rec <= 10*time.Second; rec += time.Second {
-			for xmt := rec; xmt <= 10*time.Second; xmt += time.Second {
-				for dst := org; dst <= 10*time.Second; dst += time.Second {
-					t1 := start.Add(org)
-					t2 := start.Add(rec)
-					t3 := start.Add(xmt)
-					t4 := start.Add(dst)
-
-					var error0, error1 time.Duration
-					if org >= rec {
-						error0 = org - rec
-					}
-					if xmt >= dst {
-						error1 = xmt - dst
-					}
-
-					var mex time.Duration
-					mex = max(error0, error1)
-
-					m := minError(t1, t2, t3, t4)
-					assert.Equal(t, mex, m)
-				}
-			}
-		}
-	}
-}
-
 func TestOfflineV4OffsetCalculation(t *testing.T) {
 	now := time.Now()
 	t1 := toTimestamp(now)

ntp_test.go

@@ -175,6 +175,85 @@ func TestOfflineFixHostPort(t *testing.T) {
 	}
 }
 
+func TestOfflineMinError(t *testing.T) {
+	const format = "5.000000"
+	base := time.Date(2026, 0, 0, 0, 0, 0, 0, time.UTC)
+
+	cases := []struct {
+		t1, t2, t3, t4 time.Time
+		minError       time.Duration
+	}{
+		{
+			base,
+			base.Add(1 * time.Millisecond),
+			base.Add(1 * time.Millisecond),
+			base.Add(1 * time.Millisecond),
+			0,
+		},
+		{
+			base,
+			base.Add(1 * time.Millisecond),
+			base.Add(1 * time.Millisecond),
+			base.Add(10 * time.Millisecond),
+			0,
+		},
+		{
+			base,
+			base.Add(1 * time.Millisecond),
+			base.Add(1 * time.Millisecond),
+			base.Add(25 * time.Microsecond),
+			975 * time.Microsecond,
+		},
+		{
+			base,
+			base.Add(1 * time.Millisecond),
+			base.Add(1 * time.Millisecond),
+			base.Add(2 * time.Microsecond),
+			998 * time.Microsecond,
+		},
+		{
+			base,
+			base.Add(25 * time.Millisecond),
+			base.Add(25 * time.Millisecond),
+			base.Add(1 * time.Millisecond),
+			24 * time.Millisecond,
+		},
+		{
+			base.Add(78 * time.Millisecond),
+			base.Add(38 * time.Millisecond),
+			base.Add(38 * time.Millisecond),
+			base.Add(94 * time.Millisecond),
+			40 * time.Millisecond,
+		},
+		{
+			base.Add(78 * time.Millisecond),
+			base.Add(38 * time.Millisecond),
+			base.Add(39 * time.Millisecond),
+			base.Add(94 * time.Millisecond),
+			40 * time.Millisecond,
+		},
+		{
+			base.Add(78 * time.Millisecond),
+			base.Add(38 * time.Millisecond),
+			base.Add(39 * time.Millisecond),
+			base.Add(95 * time.Millisecond),
+			40 * time.Millisecond,
+		},
+	}
+
+	for _, c := range cases {
+		m := minError(c.t1, c.t2, c.t3, c.t4)
+		if m != c.minError {
+			t.Errorf("minError(%s, %s, %s, %s) = %s; want %s",
+				c.t1.Format(format),
+				c.t2.Format(format),
+				c.t3.Format(format),
+				c.t4.Format(format),
+				m, c.minError)
+		}
+	}
+}
+
 func TestOfflineKissCode(t *testing.T) {
 	codes := []struct {
 		id  uint32