graph/db: version horizon queries for v1/v2 gossip#10691
graph/db: version horizon queries for v1/v2 gossip#10691yyforyongyu merged 9 commits intolightningnetwork:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces versioned graph horizon queries to support both v1 (time-based) and v2 (block-height-based) gossip ranges. It standardizes end-time semantics to be exclusive, updates the Store interface to enforce version-correct bounds at the type level, and adds necessary SQL infrastructure for efficient v2 queries. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request versions graph horizon queries to support both v1 (time-based) and v2 (block-height-based) gossip ranges. It introduces ChanUpdateRange and NodeUpdateRange types, updates the Store interface, and implements v2 height-based queries in the SQL backend with new indexes. It also adjusts v1 queries to use exclusive end-time bounds per BOLT 07. Feedback identifies several compilation errors due to the use of undefined constants and suggests using the %v format specifier for better type printing in error messages.
| } | ||
|
|
||
| default: | ||
| return fmt.Errorf("unknown gossip version: %d", v) |
There was a problem hiding this comment.
For consistency with ChanUpdateRange.validateForVersion and to correctly print the lnwire.GossipVersion type, please use the %v format specifier instead of %d.
| return fmt.Errorf("unknown gossip version: %d", v) | |
| return fmt.Errorf("unknown gossip version: %v", v) |
ellemouton
force-pushed
the
g175-horizon
branch
from
97af89d to
9621f26
Compare
ellemouton
force-pushed
the
g175-horizon
branch
from
9621f26 to
8de0709
Compare
bitromortac
left a comment
bitromortac
left a comment
There was a problem hiding this comment.
Two comments, but otherwise LGTM 🎉
| }, func() { | ||
| batch = nil | ||
| edgesSeen = make( | ||
| map[uint64]struct{}, | ||
| ) | ||
| edgesToCache = make( | ||
| map[uint64]ChannelEdge, | ||
| ) | ||
| }, |
There was a problem hiding this comment.
I'm not sure this is a problem, but it seems like we would need to track the cursor from before the ExecTx, to reset the advancement from processRow on failure, otherwise we could miss some entries. If this is an issue, other functions that use a cursor have the same pattern. Non-blocking, but could be addressed in a follow up if needed.
There was a problem hiding this comment.
good point - will leave for a followup since it is pre-existing 👍
BOLT 07 specifies that gossip_timestamp_filter range semantics are "greater or equal to first_timestamp, and less than first_timestamp plus timestamp_range", i.e. [start, end). Three of the four implementations (KV ChanUpdatesInHorizon, KV NodeUpdatesInHorizon, SQL NodeUpdatesInHorizon) were incorrectly using an inclusive end time (<= instead of <). Only SQL ChanUpdatesInHorizon was correct. This commit fixes the KV store's fetchNextChanUpdateBatch and fetchNextNodeBatch to use >= (instead of >) for the end time break condition, and < (instead of <=) for the hasMore check. It also fixes the SQL GetNodesByLastUpdateRange query to use < instead of <= on the end_time bound. All godocs are updated to reference the BOLT 07 spec language and explicitly document the [start, end) range semantics. New dedicated tests (TestNodeUpdatesInHorizonExclusiveEnd and TestChanUpdatesInHorizonExclusiveEnd) verify that items at exactly the end time are excluded while items at the start time are included.
Add version-aware range types for channel and node update horizon queries. V1 gossip uses unix timestamps for ordering while v2 uses block heights, so each range type validates that the correct bound type is provided for the requested gossip version. These types will be used in follow-up commits to version the NodeUpdatesInHorizon and ChanUpdatesInHorizon Store methods.
Replace the (startTime, endTime time.Time) parameters on NodeUpdatesInHorizon and ChanUpdatesInHorizon with (v GossipVersion, r NodeUpdateRange/ChanUpdateRange). The range types enforce version-correct bounds at the type level: v1 uses unix timestamps, v2 will use block heights. The KV store rejects non-v1 versions since it only stores v1 data. The SQL store dispatches to version-specific helpers (nodeUpdatesInHorizonV1, chanUpdatesInHorizonV1); the v2 block-height paths return an error for now and will be wired up in follow-up commits. VersionedGraph wrappers supply the version from the embedded field, so callers only pass the range.
The ChannelGraph.NodeUpdatesInHorizon and ChannelGraph.ChanUpdatesInHorizon methods were only used in tests. All production callers already use VersionedGraph (which supplies the gossip version from its embedded field). Remove the ChannelGraph wrappers and update tests to instantiate a VersionedGraph via NewVersionedGraph(MakeTestGraph(t), v1) instead, dropping the explicit version parameter from horizon calls.
Replace the unused chainhash.Hash parameter in ChannelGraphTimeSeries.UpdatesInHorizon with context.Context. The chain parameter was never consulted by the implementation since the graph is not chain-scoped. The context is threaded through to the underlying graph DB queries that need it.
Add composite indexes on graph_nodes and graph_channel_policies for the upcoming v2 block-height-based horizon queries. The v2 gossip protocol uses block heights instead of unix timestamps for ordering node announcements and channel updates. The v2 NodeUpdatesInHorizon and ChanUpdatesInHorizon query paths will filter on WHERE version = @v AND block_height >= start AND block_height < end. Without these indexes, those queries would require full table scans. For nodes, the index is (version, block_height, pub_key). Including pub_key covers the ORDER BY (block_height, pub_key) clause and allows direct cursor seeks for pagination, avoiding an extra sort. For channel policies, the index is (version, block_height). The pagination cursor uses a CASE expression across two joined policy rows so the index cannot cover the ORDER BY — the two leading columns are sufficient for the range scan.
Add GetNodesByBlockHeightRange SQL query and wire it into SQLStore.nodeUpdatesInHorizonV2. This mirrors the existing v1 time-based query but filters on (version, block_height) instead of last_update, using the same [start, end) exclusive-end semantics and (block_height, pub_key) compound cursor pagination. The public-node filter for v2 checks for channels with a non-empty channel announcement signature (c.signature), matching the v2 protocol's public channel indicator.
Add GetChannelsByPolicyBlockRange SQL query and wire it into SQLStore.chanUpdatesInHorizonV2. This mirrors the existing v1 time-based query but filters on policy block_height instead of last_update, using the same [start, end) exclusive-end semantics and (max_block_height, channel_id) compound cursor pagination. Also adds extractMaxBlockHeight helper (returns the max of both policies' block heights for cursor tracking) and buildChannelFromBlockRangeRow (structurally identical to the v1 variant but accepts the distinct sqlc-generated row type). The extractChannelPolicies type-switch is extended with a case for the new GetChannelsByPolicyBlockRangeRow type.
ellemouton
force-pushed
the
g175-horizon
branch
from
8de0709 to
f38daf8
Compare
|
ready for override merge - test flake is unrelated & being addressed here: #10695 |
5 participants
Summary
This PR versions the
NodeUpdatesInHorizonandChanUpdatesInHorizongraph DB methods to support both v1 (time-based) and v2 (block-height-based) gossip ranges. It branches off the v2 graph schema work in #10656.Key changes:
Fix v1 end-time semantics: The end time bound on horizon queries was inconsistently inclusive in 3 of 4 implementations (KV
NodeUpdates, KVChanUpdates, SQLNodeUpdates). Per BOLT 07'sgossip_timestamp_filterspec ("greater or equal tofirst_timestamp, and less thanfirst_timestampplustimestamp_range"), the end must be exclusive. All are now[start, end).Version the Store interface:
NodeUpdatesInHorizonandChanUpdatesInHorizonnow take(v GossipVersion, r NodeUpdateRange/ChanUpdateRange)instead of(startTime, endTime time.Time). The range types enforce version-correct bounds at the type level: v1 uses unix timestamps, v2 uses block heights.Add v2 SQL queries and indexes: New
GetNodesByBlockHeightRangeandGetChannelsByPolicyBlockRangeSQL queries mirror the v1 equivalents but filter on(version, block_height). Composite indexes(version, block_height, pub_key)ongraph_nodesand(version, block_height)ongraph_channel_policiessupport efficient range scans.Clean up callers:
ChannelGraphhorizon wrappers (only used in tests) are removed in favor ofVersionedGraph. TheChanSeries.UpdatesInHorizonmethod replaces its unusedchainparameter withctx context.Context.