discovery: avoid boxing channel update freshness

[JasonColapietro]

Change Description

Fixes #11006.

Store ChannelUpdateInfo freshness values as concrete uint64 fields, using the existing gossip version as the discriminator. Version-specific lnwire.Timestamp values are reconstructed only at the API boundary. This removes the two interface-boxing allocations from each timestamped channel-range entry without changing wire or database formats.

A release-note entry and focused v1/v2 representation tests are included.

Steps to Test

1. Run go test ./graph/db -run '^TestChannelUpdateInfoFreshness$' -count=1.
2. Run go test ./graph/db ./discovery -run '^$' -count=1 to compile both affected packages.

Pull Request Checklist

Testing

• Your PR passes all CI checks.
• Tests covering the positive and negative (error paths) are included where applicable.
• The performance regression has focused representation coverage.

Code Style and Documentation

• The change is substantial.
• The change obeys documentation, commenting, and 80-column guidelines.
• The commit follows the ideal commit structure.
• No logging statements were added.
• No lncli commands were added.
• A change description is included in the 0.22.0 release notes.

[github-actions]

🟠 PR Severity: HIGH

gh pr view | 7 files | 162 lines changed

🟠 High (4 files)

• discovery/syncer.go - discovery/* (gossip protocol)
• graph/builder.go - graph/* (network graph maintenance)
• graph/db/kv_store.go - graph/* (network graph maintenance / persistence)
• graph/db/sql_store.go - graph/* (network graph maintenance / persistence)

🟢 Low (3 files)

• docs/release-notes/release-notes-0.22.0.md - release notes
• graph/db/channel_update_info_test.go - test-only change
• graph/db/graph_test.go - test-only change

Analysis

The highest-severity files touched are in discovery/* and graph/* (graph/builder.go, graph/db/kv_store.go, graph/db/sql_store.go), which govern gossip syncing and network graph persistence — these are classified HIGH. No CRITICAL packages are touched. Excluding tests and docs, the change spans 5 non-test files and ~118 lines, well under the thresholds for a severity bump (>20 files or >500 lines), and does not span multiple distinct CRITICAL packages, so no bump applies.

---

_{To override, add a severity-override-{critical,high,medium,low} label.}

[Lrifton92]

Trading a self-describing interface for a value plus a discriminator is the right call here, and the release-note arithmetic checks out: an interface value is two words, so dropping two of them to uint64 saves exactly the 16 bytes claimed, and the boxing allocations go with it. Two things before this is ready, one of which will fail CI as it stands.

Six added lines exceed the 80-column limit

.golangci.yml enables a custom ll linter with line-length: 80 and tab-width: 8 (lines 209-219). Measured with tabs expanded to 8, the new assignments come out over:

88  chanInfo.Node1Freshness = uint64(edge.LastUpdate.Unix())   graph/db/kv_store.go
88  chanInfo.Node2Freshness = uint64(edge.LastUpdate.Unix())   graph/db/kv_store.go
82  chanInfo.Node1Freshness = uint64(n1Update)                 graph/db/sql_store.go
82  chanInfo.Node1Freshness = uint64(n1Height)                 graph/db/sql_store.go
82  chanInfo.Node2Freshness = uint64(n2Update)                 graph/db/sql_store.go
82  chanInfo.Node2Freshness = uint64(n2Height)                 graph/db/sql_store.go

The two kv_store.go ones are the telling case: the code you replaced was already split across three lines for exactly this reason, and collapsing it to one call put it back over. All six are at deep nesting, so they need wrapping rather than shortening.

freshnessTimestamp has an unchecked default

func (c ChannelUpdateInfo) freshnessTimestamp(value uint64) lnwire.Timestamp {
	if c.Version == lnwire.GossipVersion1 {
		return lnwire.UnixTimestamp(value)
	}

	return lnwire.BlockHeightTimestamp(value)
}

GossipVersion1 is 1 and GossipVersion2 is 2 (lnwire/interfaces.go:17-22), so zero is not a valid version — but it is the zero value of the field. Anything that is not exactly V1 is reported as a block height: a zero-valued ChannelUpdateInfo, and any GossipVersion3 added later.

This is not reachable today — both constructors set Version, and they are the only two places that build the struct literal — so I am raising it as a property of the refactor rather than a live bug. But it is the specific thing the interface used to make impossible: a lnwire.Timestamp either held a UnixTimestamp, held a BlockHeightTimestamp, or was nil and detectably absent. It could not quietly claim to be the wrong one. An explicit switch with a GossipVersion2 case makes the discriminator carry that same guarantee, and gives a future version a compile-time or panic-time signal instead of silently wrong data.

Worth noting the sibling method already treats unknown versions conservatively:

func (c ChannelUpdateInfo) Node1FreshnessTime() time.Time {
	if c.Version == lnwire.GossipVersion1 {
		return time.Unix(int64(c.Node1Freshness), 0)
	}

	return time.Time{}
}

Same condition, opposite default — one falls back to "unknown", the other to "block height". Whichever is intended, they should agree.

The default branch is untested

TestChannelUpdateInfoFreshness covers V1 and V2, which is the useful half. Nothing pins the behaviour for a version that is neither, so whatever you decide above would not be protected by a test. Given the point of the change is that Version now carries the type information, that branch seems worth an assertion.

[JasonColapietro]

Thanks for the careful read — all three points addressed in 692e35f.

Line lengths: all six assignments wrapped. Re-measured every added line in the diff with tabs expanded to 8; nothing exceeds 80 columns now. The kv_store.go pair went back to a multi-line call, and the sql_store.go cases follow the break-after-= shape the replaced code used.

freshnessTimestamp default: now an explicit switch over Version with a GossipVersion2 case and a nil default. You named the exact property that mattered — the interface could be nil and detectably absent, and the refactor's else branch gave that up. Returning nil for anything that isn't a known version restores it, and it agrees with Node1FreshnessTime's conservative zero-time fallback rather than contradicting it. Downstream this is also the safe direction: isTimestampStale type-asserts in both of its branches and treats a failed assertion as stale, so a zero-valued struct or a future GossipVersion3 reads as prunable instead of silently claiming a block height.

Default-branch test: TestChannelUpdateInfoFreshness now constructs a ChannelUpdateInfo with the zero-valued version and a nonzero freshness, and asserts the timestamp comes back nil and the unix-time accessor returns the zero time.

Verified locally: go build ./graph/... ./discovery/..., then go test for TestChannelUpdateInfoFreshness, TestFilterChannelRange, the graph zombie tests, and TestGossipSyncer* in discovery — all pass. Kept the fix as its own commit so the delta is easy to review; happy to squash it into the main commit before merge if that's preferred.

[Lrifton92]

All three points addressed in 692e35f — verified each one:

• Line lengths: the six assignments are wrapped; the kv_store.go pair went back to the multi-line call shape the replaced code used, and the sql_store.go cases break after =. Nothing in the new diff exceeds 80 columns with tabs at 8.
• freshnessTimestamp default: the explicit switch with a GossipVersion2 case and nil default restores the property the interface used to carry — an unknown version is now detectably absent instead of silently claiming to be a block height. I checked the consumer side too: isTimestampStale guards with ts, ok := freshness.(...) on both arms, so a nil freshness reads as stale rather than dereferencing, and the nil default now agrees with Node1FreshnessTime's zero-time fallback instead of contradicting it.
• Default branch test: the new unknown case in TestChannelUpdateInfoFreshness pins both accessors (nil timestamp, zero time) for a version that is neither v1 nor v2, including the zero value.

LGTM.