Skip to content

Commit 677ffab

Browse files
ziggie1984github-actions[bot]
authored andcommitted
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 865e155)
1 parent eceaadc commit 677ffab

File tree

2 files changed

+158
-3
lines changed

2 files changed

+158
-3
lines changed

server.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5497,6 +5497,20 @@ func (s *server) AttemptRBFCloseUpdate(ctx context.Context,
54975497
return updates, nil
54985498
}
54995499

5500+
// calculateNodeAnnouncementTimestamp returns the timestamp to use for a node
5501+
// announcement, ensuring it's at least one second after the previously
5502+
// persisted timestamp. This ensures BOLT-07 compliance, which requires node
5503+
// announcements to have strictly increasing timestamps.
5504+
func calculateNodeAnnouncementTimestamp(persistedTime,
5505+
currentTime time.Time) time.Time {
5506+
5507+
if persistedTime.Unix() >= currentTime.Unix() {
5508+
return persistedTime.Add(time.Second)
5509+
}
5510+
5511+
return currentTime
5512+
}
5513+
55005514
// setSelfNode configures and sets the server's self node. It sets the node
55015515
// announcement, signs it, and updates the source node in the graph. When
55025516
// 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,
55645578
// If we have a source node persisted in the DB already, then we
55655579
// just need to make sure that the new LastUpdate time is at
55665580
// least one second after the last update time.
5567-
if srcNode.LastUpdate.Second() >= nodeLastUpdate.Second() {
5568-
nodeLastUpdate = srcNode.LastUpdate.Add(time.Second)
5569-
}
5581+
nodeLastUpdate = calculateNodeAnnouncementTimestamp(
5582+
srcNode.LastUpdate, nodeLastUpdate,
5583+
)
55705584

55715585
// If the color is not changed from default, it means that we
55725586
// didn't specify a different color in the config. We'll use the

server_test.go

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package lnd
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
// TestNodeAnnouncementTimestampComparison tests the timestamp comparison
11+
// logic used in setSelfNode to ensure node announcements have strictly
12+
// increasing timestamps at second precision (as required by BOLT-07 and
13+
// enforced by the database storage).
14+
func TestNodeAnnouncementTimestampComparison(t *testing.T) {
15+
t.Parallel()
16+
17+
// Use a simple base time for the tests.
18+
baseTime := int64(1000)
19+
20+
tests := []struct {
21+
name string
22+
srcNodeLastUpdate time.Time
23+
nodeLastUpdate time.Time
24+
expectedResult time.Time
25+
description string
26+
}{
27+
{
28+
name: "same second different nanoseconds",
29+
srcNodeLastUpdate: time.Unix(baseTime, 0),
30+
nodeLastUpdate: time.Unix(baseTime, 500_000_000),
31+
expectedResult: time.Unix(baseTime+1, 0),
32+
description: "Edge case: timestamps in same second " +
33+
"but different nanoseconds. Must increment " +
34+
"to avoid persisting same second-level " +
35+
"timestamp.",
36+
},
37+
{
38+
name: "different seconds",
39+
srcNodeLastUpdate: time.Unix(baseTime, 0),
40+
nodeLastUpdate: time.Unix(baseTime+2, 0),
41+
expectedResult: time.Unix(baseTime+2, 0),
42+
description: "Normal case: current time is already " +
43+
"in a different (later) second. No increment " +
44+
"needed.",
45+
},
46+
{
47+
name: "exactly equal",
48+
srcNodeLastUpdate: time.Unix(baseTime, 123456789),
49+
nodeLastUpdate: time.Unix(baseTime, 123456789),
50+
expectedResult: time.Unix(baseTime+1, 123456789),
51+
description: "Timestamps are identical. Must " +
52+
"increment to ensure strictly greater " +
53+
"timestamp.",
54+
},
55+
{
56+
name: "exactly equal - zero nanoseconds",
57+
srcNodeLastUpdate: time.Unix(baseTime, 0),
58+
nodeLastUpdate: time.Unix(baseTime, 0),
59+
expectedResult: time.Unix(baseTime+1, 0),
60+
description: "Timestamps are identical at second " +
61+
"precision (0 nanoseconds), as would be read " +
62+
"from DB. Must increment.",
63+
},
64+
{
65+
name: "clock skew - persisted is newer",
66+
srcNodeLastUpdate: time.Unix(baseTime+5, 0),
67+
nodeLastUpdate: time.Unix(baseTime+3, 0),
68+
expectedResult: time.Unix(baseTime+6, 0),
69+
description: "Clock went backwards: persisted " +
70+
"timestamp is newer than current time. Must " +
71+
"increment from persisted timestamp.",
72+
},
73+
{
74+
name: "clock skew - same second",
75+
srcNodeLastUpdate: time.Unix(baseTime+5, 100_000_000),
76+
nodeLastUpdate: time.Unix(baseTime+5, 900_000_000),
77+
expectedResult: time.Unix(baseTime+6, 100_000_000),
78+
description: "Clock skew within same second. Must " +
79+
"increment to ensure strictly greater " +
80+
"second-level timestamp.",
81+
},
82+
{
83+
name: "same second component different " +
84+
"minute",
85+
srcNodeLastUpdate: time.Unix(baseTime, 0),
86+
nodeLastUpdate: time.Unix(baseTime+60, 0),
87+
expectedResult: time.Unix(baseTime+60, 0),
88+
description: "Same seconds component (:00) but " +
89+
"different minutes. Current time is later. " +
90+
"Verifies we use .Unix() not .Second().",
91+
},
92+
{
93+
name: "lower second component but " +
94+
"later time",
95+
srcNodeLastUpdate: time.Unix(baseTime+58, 0),
96+
nodeLastUpdate: time.Unix(baseTime+63, 0),
97+
expectedResult: time.Unix(baseTime+63, 0),
98+
description: "Persisted has second=58, current has " +
99+
"second=3 (next minute). Current is later " +
100+
"overall. Verifies .Unix() not .Second().",
101+
},
102+
{
103+
name: "higher second component but " +
104+
"earlier time",
105+
srcNodeLastUpdate: time.Unix(baseTime+63, 0),
106+
nodeLastUpdate: time.Unix(baseTime+58, 0),
107+
expectedResult: time.Unix(baseTime+64, 0),
108+
description: "Persisted has second=3 (next minute), " +
109+
"current has second=58. Persisted is later " +
110+
"overall. Verifies .Unix() not .Second().",
111+
},
112+
}
113+
114+
for _, tc := range tests {
115+
t.Run(tc.name, func(t *testing.T) {
116+
t.Parallel()
117+
118+
result := calculateNodeAnnouncementTimestamp(
119+
tc.srcNodeLastUpdate,
120+
tc.nodeLastUpdate,
121+
)
122+
123+
// Verify we got the expected result.
124+
require.Equal(
125+
t, tc.expectedResult, result,
126+
"Unexpected result: %s", tc.description,
127+
)
128+
129+
// Verify result is strictly greater than persisted
130+
// timestamp. This is an additional check to ensure
131+
// the result is strictly greater than the persisted
132+
// timestamp.
133+
require.Greater(
134+
t, result.Unix(), tc.srcNodeLastUpdate.Unix(),
135+
"Result must be strictly greater than "+
136+
"persisted timestamp: %s",
137+
tc.description,
138+
)
139+
})
140+
}
141+
}

0 commit comments

Comments
 (0)