graph/db+refactor: surface all context.TODO()s

[ellemouton]

Refactor the graph/db methods to all take a context rather than creating context.TODO()s. we instead move the context.TODO()s ot the call-sites or pass in contexts if they are available.

This is a pure refactor

[gemini-code-assist]

Summary of Changes

Hello @ellemouton, I'm Gemini Code Assist¹! 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 is a pure refactor aimed at improving context propagation throughout the codebase, particularly within graph and database-related operations. It modifies numerous functions and interfaces to explicitly accept context.Context as an argument, thereby eliminating the implicit use of context.TODO() within method implementations. Instead, context.TODO() is now passed at the call sites where a specific context is not otherwise available, promoting better cancellation and timeout management.

Highlights

• Context Propagation: Introduced context.Context as the first argument to numerous graph/db methods and their interfaces across various packages, including discovery, graph, netann, routing, and rpcserver.
• Elimination of context.TODO() within methods: Replaced implicit context.TODO() calls within method implementations with the explicitly passed ctx parameter, improving cancellation and timeout management.
• Context at Call Sites: Moved context.TODO() to call sites where a specific context is not readily available, ensuring that a context is always provided.
• New invoiceGraphSource Adapter: Added a new adapter (invoiceGraphSource) to wrap graphdb.ChannelGraph methods, specifically for FetchChannelEdgesByID and IsPublicNode, passing context.TODO() for invoice-related graph queries.

🧠 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.

Changelog

• discovery/ban.go
  • Imported the context package.
  • Updated GraphCloser interface methods PutClosedScid and IsClosedScid to accept context.Context.
  • Modified ScidCloserMan methods PutClosedScid and IsClosedScid to accept and pass context.Context.
• discovery/chan_series.go
  • Passed context.TODO() to calls for ChanUpdatesInHorizon, NodeUpdatesInHorizon, FilterKnownChanIDs, FilterChannelRange, FetchChanInfos, and FetchChannelEdgesByID.
• discovery/gossiper.go
  • Passed context.Context to d.cfg.ScidCloser.IsClosedScid and d.cfg.ScidCloser.PutClosedScid.
• discovery/gossiper_test.go
  • Passed context.Context to tCtx.gossiper.cfg.ScidCloser.IsClosedScid.
• discovery/mock_test.go
  • Imported the context package.
  • Updated mockScidCloser methods PutClosedScid and IsClosedScid to accept context.Context (ignored with _).
• graph/builder.go
  • Passed context.TODO() to calls for b.cfg.Graph.PruneTip, b.cfg.Graph.PruneGraph, b.cfg.Graph.ChannelView, b.cfg.Graph.PruneGraphNodes, b.v1Graph.DeleteChannelEdges, b.cfg.Graph.DisconnectBlockAtHeight, b.cfg.Graph.MarkEdgeZombie, b.cfg.Graph.HasChannelEdge, b.cfg.Graph.HasV1ChannelEdge, b.cfg.Graph.FetchChannelEdgesByID, b.cfg.Graph.AddEdgeProof, b.v1Graph.IsPublicNode, and b.cfg.Graph.MarkEdgeLive.
• graph/builder_test.go
  • Passed t.Context() to ctx.graph.HasChannelEdge.
• graph/db/benchmark_test.go
  • Passed ctx (from testing.B.Context()) to store.NodeUpdatesInHorizon and store.ChanUpdatesInHorizon.
• graph/db/graph.go
  • Introduced context.TODO() in Start() and passed it to populateCache and handleTopologySubscriptions.
  • Updated handleTopologySubscriptions and handleTopologyUpdate to accept context.Context.
  • Added context.Context as an argument to ForEachNodeDirectedChannel, FetchNodeFeatures, GraphSession, MarkEdgeLive, DeleteChannelEdges, DisconnectBlockAtHeight, PruneGraph, PruneGraphNodes, FilterKnownChanIDs, MarkEdgeZombie, NodeUpdatesInHorizon, IsPublicNode, DisabledChannelIDs, HasV1ChannelEdge, HasChannelEdge, AddEdgeProof, ChanUpdatesInHorizon, FilterChannelRange, FetchChanInfos, FetchChannelEdgesByOutpoint, FetchChannelEdgesByID, ChannelView, IsZombieEdge, NumZombies, PutClosedScid, IsClosedScid, and PruneTip methods.
  • Updated VersionedGraph methods to also accept context.Context.
• graph/db/graph_test.go
  • Passed ctx (from testing.T.Context()) to various graph methods including FetchNodeFeatures, FetchChannelEdgesByID, FetchChannelEdgesByOutpoint, DeleteChannelEdges, IsZombieEdge, PruneGraph, DisconnectBlockAtHeight, HasChannelEdge, ChannelID, AddEdgeProof, ChannelView, ChanUpdatesInHorizon, NodeUpdatesInHorizon, FilterKnownChanIDs, MarkEdgeZombie, MarkEdgeLive, PruneGraphNodes, IsPublicNode, DisabledChannelIDs, NumZombies, PutClosedScid, and IsClosedScid.
• graph/db/interfaces.go
  • Updated method signatures for NodeTraverser (ForEachNodeDirectedChannel, FetchNodeFeatures) and Store (ForEachNodeDirectedChannel, FetchNodeFeatures, NodeUpdatesInHorizon, IsPublicNode, GraphSession, ForEachChannelCacheable, DisabledChannelIDs, HasV1ChannelEdge, HasChannelEdge, DeleteChannelEdges, AddEdgeProof, ChannelID, ChanUpdatesInHorizon, FilterKnownChanIDs, FilterChannelRange, FetchChanInfos, FetchChannelEdgesByOutpoint, FetchChannelEdgesByID, ChannelView, MarkEdgeZombie, MarkEdgeLive, IsZombieEdge, NumZombies, PutClosedScid, IsClosedScid, PruneTip, PruneGraphNodes, PruneGraph, and DisconnectBlockAtHeight) to accept context.Context.
• graph/db/kv_store.go
  • Updated method signatures for numerous KVStore methods to accept context.Context (often ignored with _) including ForEachChannelCacheable, ForEachNodeDirectedChannel, FetchNodeFeatures, DisabledChannelIDs, HasV1ChannelEdge, HasChannelEdge, AddEdgeProof, PruneGraph, PruneGraphNodes, DisconnectBlockAtHeight, PruneTip, DeleteChannelEdges, ChannelID, ChanUpdatesInHorizon, NodeUpdatesInHorizon, FilterKnownChanIDs, FilterChannelRange, FetchChanInfos, FetchChannelEdgesByOutpoint, FetchChannelEdgesByID, IsPublicNode, ChannelView, MarkEdgeZombie, MarkEdgeLive, IsZombieEdge, NumZombies, PutClosedScid, IsClosedScid, and GraphSession.
• graph/db/notifications.go
  • Imported the context package.
  • Updated handleTopologyUpdate and addToTopologyChange to accept context.Context.
  • Passed context.Context to FetchChannelEdgesByID.
• graph/db/sql_store.go
  • Updated method signatures for numerous SQLStore methods to accept context.Context, including FetchNodeFeatures, DisabledChannelIDs, NodeUpdatesInHorizon, ForEachNodeDirectedChannel, ChanUpdatesInHorizon, ForEachChannelCacheable, FilterChannelRange, MarkEdgeZombie, MarkEdgeLive, IsZombieEdge, NumZombies, DeleteChannelEdges, FetchChannelEdgesByID, FetchChannelEdgesByOutpoint, HasV1ChannelEdge, HasChannelEdge, ChannelID, IsPublicNode, FetchChanInfos, FilterKnownChanIDs, PruneGraphNodes, PruneGraph, ChannelView, PruneTip, DisconnectBlockAtHeight, AddEdgeProof, PutClosedScid, IsClosedScid, and GraphSession.
  • Removed context.TODO() from within these methods, as ctx is now passed as an argument.
• invoice_graph_adapter.go
  • Added a new file defining invoiceGraphSource struct.
  • Implemented FetchChannelEdgesByID and IsPublicNode methods for invoiceGraphSource, passing context.TODO() to the underlying graphdb.ChannelGraph methods.
• netann/chan_status_manager.go
  • Imported the context package.
  • Passed context.TODO() to m.cfg.Graph.FetchChannelEdgesByOutpoint.
• netann/chan_status_manager_test.go
  • Imported the context package.
  • Updated mockGraph.FetchChannelEdgesByOutpoint to accept context.Context (ignored with _).
• netann/interface.go
  • Imported the context package.
  • Updated ChannelGraph.FetchChannelEdgesByOutpoint method signature to accept context.Context.
• peer/brontide.go
  • Passed context.TODO() to graph.FetchChannelEdgesByOutpoint.
• routing/bandwidth.go
  • Imported the context package.
  • Passed context.TODO() to graph.ForEachNodeDirectedChannel.
• routing/graph.go
  • Imported the context package.
  • Updated Graph interface methods ForEachNodeDirectedChannel and FetchNodeFeatures to accept context.Context.
  • Updated GraphSessionFactory.GraphSession method signature to accept context.Context.
• routing/mock_graph_test.go
  • Imported the context package.
  • Updated mockGraph.ForEachNodeDirectedChannel, FetchNodeFeatures, and GraphSession methods to accept context.Context (ignored with _).
• routing/pathfind.go
  • Imported the context package.
  • Passed context.TODO() to g.ForEachNodeDirectedChannel and g.graph.FetchNodeFeatures.
• routing/pathfind_test.go
  • Passed t.Context() to graph.FetchChannelEdgesByID.
  • Passed ctx to graph.GraphSession.
• routing/payment_session.go
  • Imported the context package.
  • Passed context.TODO() to p.graphSessFactory.GraphSession.
• routing/payment_session_test.go
  • Imported the context package.
  • Updated sessionGraph.GraphSession to accept context.Context (ignored with _).
• routing/router_test.go
  • Passed t.Context() to ctx.graph.FetchChannelEdgesByID.
• routing/unified_edges.go
  • Imported the context package.
  • Passed context.TODO() to g.ForEachNodeDirectedChannel.
• rpcserver.go
  • Passed context.TODO() to graph.FetchChannelEdgesByID and chanGraph.ChannelID, chanGraph.DeleteChannelEdges.
  • Used invoiceGraphSource adapter for Graph in AddInvoice.
  • Passed ctx to graph.NumZombies, graph.FetchChannelEdgesByID, and graph.FetchChannelEdgesByOutpoint in GetChanInfo.
  • Updated GetChanInfo method signature to accept context.Context.
• server.go
  • Passed context.TODO() to s.graphDB.FetchChannelEdgesByID and s.v1Graph.DeleteChannelEdges.
• subrpcserver_config.go
  • Used invoiceGraphSource adapter for Graph in PopulateDependencies.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request is a pure refactoring that surfaces context.TODO() calls from the graph/db methods to their call sites, or propagates a real context where available. The changes are consistent across the codebase and correctly apply this pattern. The introduction of invoiceGraphSource is a good adapter pattern to handle the interface change for invoicesrpc. The refactoring appears to be well-executed and achieves its stated goal.

[lightninglabs-deploy]

🔴 PR Severity: CRITICAL

Highest-severity file rule + bump conditions met | 21 files (excl. tests) | ~813 lines changed (excl. tests)

🔴 Critical (3 files)

• peer/brontide.go - encrypted peer connection handling (peer/*)
• rpcserver.go - core RPC server coordination
• server.go - core server coordination

🟠 High (16 files)

• discovery/ban.go - gossip protocol (discovery/*)
• discovery/chan_series.go - gossip protocol (discovery/*)
• discovery/gossiper.go - gossip protocol (discovery/*)
• graph/builder.go - network graph maintenance (graph/*)
• graph/db/graph.go - network graph DB (graph/*)
• graph/db/interfaces.go - network graph DB interfaces (graph/*)
• graph/db/kv_store.go - network graph KV store (graph/*)
• graph/db/notifications.go - network graph notifications (graph/*)
• graph/db/sql_store.go - network graph SQL store (graph/*)
• lnrpc/invoicesrpc/addinvoice.go - RPC/API layer (lnrpc/*)
• lnrpc/invoicesrpc/interfaces.go - RPC/API layer (lnrpc/*)
• routing/bandwidth.go - payment pathfinding (routing/*)
• routing/graph.go - payment pathfinding (routing/*)
• routing/pathfind.go - payment pathfinding (routing/*)
• routing/payment_session.go - payment pathfinding (routing/*)
• routing/unified_edges.go - payment pathfinding (routing/*)

🟡 Medium (2 files)

• netann/chan_status_manager.go - channel announcements (netann/*)
• netann/interface.go - channel announcements (netann/*)

Analysis

The highest-severity files are peer/brontide.go, rpcserver.go, and server.go, which fall under CRITICAL categories (peer connections and core server coordination). This alone sets the base severity to CRITICAL.

Additionally, two severity bump conditions are met (though they cannot raise beyond CRITICAL):

• >20 non-test files: 21 source files changed
• >500 non-test lines: approximately 813 lines changed (additions + deletions, excluding test files)

The bulk of the changes are in graph/db/* (network graph database refactoring — 510 lines) and routing/* (pathfinding logic). The PR is tagged as a refactoring/code health effort, but the breadth of changes across critical peer, server, routing, graph, discovery, and RPC layers warrants careful expert review.

---

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

[bhandras]

LGTM, nice!!

[bitromortac]

LGTM ⚡