Merge pull request #10451 from lightningnetwork/backport-10449-to-v0.20.x-branch

[v0.20.x-branch] Backport #10449: server: fix timestamp comparison in setSelfNode

Author: yyforyongyu

docs/release-notes/release-notes-0.20.1.md

@@ -36,6 +36,12 @@
   condition](https://github.com/lightningnetwork/lnd/pull/10371) which could
   prevent a node from starting up if two goroutines attempt to update the 
   node's announcement at the same time.
+
+* [Fix timestamp comparison in source node
+  updates](https://github.com/lightningnetwork/lnd/pull/10449) that could still
+  cause "sql: no rows in result set" startup errors. The previous fix (#10371)
+  addressed concurrent updates with equal timestamps, but the seconds-only
+  comparison could still fail when restarting with different minute/hour values.
 
 * [Fix a startup issue in LND when encountering a 
   deserialization issue](https://github.com/lightningnetwork/lnd/pull/10383) 

server.go

@@ -5497,6 +5497,20 @@ func (s *server) AttemptRBFCloseUpdate(ctx context.Context,
 	return updates, nil
 }
 
+// calculateNodeAnnouncementTimestamp returns the timestamp to use for a node
+// announcement, ensuring it's at least one second after the previously
+// persisted timestamp. This ensures BOLT-07 compliance, which requires node
+// announcements to have strictly increasing timestamps.
+func calculateNodeAnnouncementTimestamp(persistedTime,
+	currentTime time.Time) time.Time {
+
+	if persistedTime.Unix() >= currentTime.Unix() {
+		return persistedTime.Add(time.Second)
+	}
+
+	return currentTime
+}
+
 // setSelfNode configures and sets the server's self node. It sets the node
 // announcement, signs it, and updates the source node in the graph. When
 // determining values such as color and alias, the method prioritizes values
@@ -5564,9 +5578,9 @@ func (s *server) setSelfNode(ctx context.Context, nodePub route.Vertex,
 		// If we have a source node persisted in the DB already, then we
 		// just need to make sure that the new LastUpdate time is at
 		// least one second after the last update time.
-		if srcNode.LastUpdate.Second() >= nodeLastUpdate.Second() {
-			nodeLastUpdate = srcNode.LastUpdate.Add(time.Second)
-		}
+		nodeLastUpdate = calculateNodeAnnouncementTimestamp(
+			srcNode.LastUpdate, nodeLastUpdate,
+		)
 
 		// If the color is not changed from default, it means that we
 		// didn't specify a different color in the config. We'll use the

server_test.go

@@ -0,0 +1,141 @@
+package lnd
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestNodeAnnouncementTimestampComparison tests the timestamp comparison
+// logic used in setSelfNode to ensure node announcements have strictly
+// increasing timestamps at second precision (as required by BOLT-07 and
+// enforced by the database storage).
+func TestNodeAnnouncementTimestampComparison(t *testing.T) {
+	t.Parallel()
+
+	// Use a simple base time for the tests.
+	baseTime := int64(1000)
+
+	tests := []struct {
+		name              string
+		srcNodeLastUpdate time.Time
+		nodeLastUpdate    time.Time
+		expectedResult    time.Time
+		description       string
+	}{
+		{
+			name:              "same second different nanoseconds",
+			srcNodeLastUpdate: time.Unix(baseTime, 0),
+			nodeLastUpdate:    time.Unix(baseTime, 500_000_000),
+			expectedResult:    time.Unix(baseTime+1, 0),
+			description: "Edge case: timestamps in same second " +
+				"but different nanoseconds. Must increment " +
+				"to avoid persisting same second-level " +
+				"timestamp.",
+		},
+		{
+			name:              "different seconds",
+			srcNodeLastUpdate: time.Unix(baseTime, 0),
+			nodeLastUpdate:    time.Unix(baseTime+2, 0),
+			expectedResult:    time.Unix(baseTime+2, 0),
+			description: "Normal case: current time is already " +
+				"in a different (later) second. No increment " +
+				"needed.",
+		},
+		{
+			name:              "exactly equal",
+			srcNodeLastUpdate: time.Unix(baseTime, 123456789),
+			nodeLastUpdate:    time.Unix(baseTime, 123456789),
+			expectedResult:    time.Unix(baseTime+1, 123456789),
+			description: "Timestamps are identical. Must " +
+				"increment to ensure strictly greater " +
+				"timestamp.",
+		},
+		{
+			name:              "exactly equal - zero nanoseconds",
+			srcNodeLastUpdate: time.Unix(baseTime, 0),
+			nodeLastUpdate:    time.Unix(baseTime, 0),
+			expectedResult:    time.Unix(baseTime+1, 0),
+			description: "Timestamps are identical at second " +
+				"precision (0 nanoseconds), as would be read " +
+				"from DB. Must increment.",
+		},
+		{
+			name:              "clock skew - persisted is newer",
+			srcNodeLastUpdate: time.Unix(baseTime+5, 0),
+			nodeLastUpdate:    time.Unix(baseTime+3, 0),
+			expectedResult:    time.Unix(baseTime+6, 0),
+			description: "Clock went backwards: persisted " +
+				"timestamp is newer than current time. Must " +
+				"increment from persisted timestamp.",
+		},
+		{
+			name:              "clock skew - same second",
+			srcNodeLastUpdate: time.Unix(baseTime+5, 100_000_000),
+			nodeLastUpdate:    time.Unix(baseTime+5, 900_000_000),
+			expectedResult:    time.Unix(baseTime+6, 100_000_000),
+			description: "Clock skew within same second. Must " +
+				"increment to ensure strictly greater " +
+				"second-level timestamp.",
+		},
+		{
+			name: "same second component different " +
+				"minute",
+			srcNodeLastUpdate: time.Unix(baseTime, 0),
+			nodeLastUpdate:    time.Unix(baseTime+60, 0),
+			expectedResult:    time.Unix(baseTime+60, 0),
+			description: "Same seconds component (:00) but " +
+				"different minutes. Current time is later. " +
+				"Verifies we use .Unix() not .Second().",
+		},
+		{
+			name: "lower second component but " +
+				"later time",
+			srcNodeLastUpdate: time.Unix(baseTime+58, 0),
+			nodeLastUpdate:    time.Unix(baseTime+63, 0),
+			expectedResult:    time.Unix(baseTime+63, 0),
+			description: "Persisted has second=58, current has " +
+				"second=3 (next minute). Current is later " +
+				"overall. Verifies .Unix() not .Second().",
+		},
+		{
+			name: "higher second component but " +
+				"earlier time",
+			srcNodeLastUpdate: time.Unix(baseTime+63, 0),
+			nodeLastUpdate:    time.Unix(baseTime+58, 0),
+			expectedResult:    time.Unix(baseTime+64, 0),
+			description: "Persisted has second=3 (next minute), " +
+				"current has second=58. Persisted is later " +
+				"overall. Verifies .Unix() not .Second().",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			result := calculateNodeAnnouncementTimestamp(
+				tc.srcNodeLastUpdate,
+				tc.nodeLastUpdate,
+			)
+
+			// Verify we got the expected result.
+			require.Equal(
+				t, tc.expectedResult, result,
+				"Unexpected result: %s", tc.description,
+			)
+
+			// Verify result is strictly greater than persisted
+			// timestamp. This is an additional check to ensure
+			// the result is strictly greater than the persisted
+			// timestamp.
+			require.Greater(
+				t, result.Unix(), tc.srcNodeLastUpdate.Unix(),
+				"Result must be strictly greater than "+
+					"persisted timestamp: %s",
+				tc.description,
+			)
+		})
+	}
+}