Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/release-notes/release-notes-0.20.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 17 additions & 3 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5556,6 +5556,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
Expand Down Expand Up @@ -5623,9 +5637,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
Expand Down
141 changes: 141 additions & 0 deletions server_test.go
Original file line number Diff line number Diff line change
@@ -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,
)
})
}
}
Loading