From 677ffabed8a8b9aee6de5049e8d9df7e8971b170 Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 15 Dec 2025 14:07:40 +0100 Subject: [PATCH] server: fix timestamp comparison in setSelfNode Fix bug where setSelfNode compared only the seconds component of timestamps instead of the full timestamp. This caused the node to attempt persisting an older timestamp than what existed in the database during restart, resulting in "sql: no rows in result set" errors. (cherry picked from commit 865e1556d46168d47f3a5745e07c873bc09a78df) --- server.go | 20 +++++-- server_test.go | 141 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 server_test.go diff --git a/server.go b/server.go index b9b6bba3f..4c141a321 100644 --- a/server.go +++ b/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 diff --git a/server_test.go b/server_test.go new file mode 100644 index 000000000..0cb364318 --- /dev/null +++ b/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, + ) + }) + } +}