Skip to content

graph: fix graph-cache issue#10378

Merged
bhandras merged 3 commits intolightningnetwork:masterfrom
ziggie1984:bugfix/graph-cache
Nov 19, 2025
Merged

graph: fix graph-cache issue#10378
bhandras merged 3 commits intolightningnetwork:masterfrom
ziggie1984:bugfix/graph-cache

Conversation

@ziggie1984
Copy link
Collaborator

@ziggie1984 ziggie1984 commented Nov 18, 2025

The Problem (Before the Fix)

Initial Population:

  • In graph_cache.go, the old AddChannel function would return early if both policies were disabled (before this fix)
  • This meant the channel structure was never added to the graph cache
  • However, the channel still existed in the database

When a Policy Update Arrives Later:

  1. Builder.updateEdge() in builder.go:1137 is called
  2. It calls HasChannelEdge() which checks the database (not the graph cache) - see kv_store.go:1265
  3. Since the channel exists in the DB, exists=true is returned
  4. The policy update proceeds and updates the database successfully
  5. Then ChannelGraph.UpdateEdgePolicy() is called (graph.go:581)
  6. It calls graphCache.UpdatePolicy() (graph.go:590)
  7. But - UpdatePolicy() in graph_cache.go:193-206 checks if the channel exists in the graph cache:
    channel, ok := c.nodeChannels[nodeKey][policy.ChannelID]
    if !ok {
    log.Warnf("Channel=%v not found in graph cache", policy.ChannelID)
    return // ← Channel never gets added!
    }
  8. The channel is not found, so it just logs a warning and returns
  9. The policy update never makes it into the graph cache

Why AddEdge Won't Help:

You're also correct that Builder.addEdge() won't re-add it because:

  • Lines 1048-1061 in builder.go check HasChannelEdge() which checks the DB
  • If the channel exists in the DB, it returns early with ErrIgnored
  • So AddChannelEdge() is never called, and the graph cache never gets the channel structure

The Fix (After the Commit)

The fix correctly:

  1. Always adds the channel structure to the graph cache (lines 128-140)
  2. Then checks if both policies are disabled and skips adding the policies (lines 142-153)
  3. But the channel structure is already in the cache by this point
  4. When a policy update arrives later, UpdatePolicy() finds the channel and successfully updates it

Fixes: #10342

@ziggie1984 ziggie1984 added this to the v0.20.1 milestone Nov 19, 2025
@ziggie1984 ziggie1984 self-assigned this Nov 19, 2025
…abled

Fix a bug where channels with both policies disabled were not added to
the graph cache during startup. When a policy update later re-enabled
one of the directions, the update would succeed in the database but fail
to update the graph cache (since the channel structure was never added),
preventing the channel from being used for routing.
@ziggie1984 ziggie1984 marked this pull request as ready for review November 19, 2025 09:13
@ziggie1984
Copy link
Collaborator Author

/gemini review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request provides a well-reasoned fix for a subtle bug in the graph cache. The issue, where channels with initially disabled policies were not cached correctly, is resolved by ensuring the channel structure is always added. The changes in graph/db/graph_cache.go are clear and effective. My main feedback is to add a regression test to prevent this issue from resurfacing in the future. I've also included a minor formatting suggestion for the new release notes file.

@bhandras
Copy link
Collaborator

Looks good! Could you please cover this case with a small test? So we don't accidentally regress.

@bhandras bhandras self-requested a review November 19, 2025 09:20
@ziggie1984 ziggie1984 force-pushed the bugfix/graph-cache branch 2 times, most recently from 4e7a1de to 0ed4503 Compare November 19, 2025 09:37
Copy link
Collaborator

@bitromortac bitromortac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice catch ⚡

Copy link
Collaborator

@bhandras bhandras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you! 🎉

@yyforyongyu
Copy link
Member

Pending linter fix, otherwise can be merged!

@ziggie1984 ziggie1984 added the back port candidate pr which should be back ported to last major release label Nov 19, 2025
@bhandras bhandras merged commit 194a9f7 into lightningnetwork:master Nov 19, 2025
37 of 39 checks passed
Roasbeef added a commit that referenced this pull request Nov 19, 2025
Backport graph cache fix #10378 to minor release branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

back port candidate pr which should be back ported to last major release bug fix graph

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: Local Channel all of a sudden not in local cache

4 participants

Comments