Skip to content

Commit 194a9f7

Browse files
authored
Merge pull request #10378 from ziggie1984/bugfix/graph-cache
graph: fix graph-cache issue
2 parents 31452a6 + 4f40d45 commit 194a9f7

File tree

3 files changed

+187
-7
lines changed

3 files changed

+187
-7
lines changed
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# Release Notes
2+
- [Bug Fixes](#bug-fixes)
3+
- [New Features](#new-features)
4+
- [Functional Enhancements](#functional-enhancements)
5+
- [RPC Additions](#rpc-additions)
6+
- [lncli Additions](#lncli-additions)
7+
- [Improvements](#improvements)
8+
- [Functional Updates](#functional-updates)
9+
- [RPC Updates](#rpc-updates)
10+
- [lncli Updates](#lncli-updates)
11+
- [Breaking Changes](#breaking-changes)
12+
- [Performance Improvements](#performance-improvements)
13+
- [Deprecations](#deprecations)
14+
- [Technical and Architectural Updates](#technical-and-architectural-updates)
15+
- [BOLT Spec Updates](#bolt-spec-updates)
16+
- [Testing](#testing)
17+
- [Database](#database)
18+
- [Code Health](#code-health)
19+
- [Tooling and Documentation](#tooling-and-documentation)
20+
- [Contributors (Alphabetical Order)](#contributors)
21+
22+
# Bug Fixes
23+
24+
* Fix bug where channels with both [policies disabled at startup could never
25+
be used for routing](https://github.com/lightningnetwork/lnd/pull/10378)
26+
27+
# New Features
28+
29+
## Functional Enhancements
30+
31+
## RPC Additions
32+
33+
## lncli Additions
34+
35+
# Improvements
36+
## Functional Updates
37+
38+
## RPC Updates
39+
40+
## lncli Updates
41+
42+
## Breaking Changes
43+
44+
## Performance Improvements
45+
46+
## Deprecations
47+
48+
# Technical and Architectural Updates
49+
## BOLT Spec Updates
50+
51+
## Testing
52+
53+
## Database
54+
55+
## Code Health
56+
57+
## Tooling and Documentation
58+
59+
# Contributors (Alphabetical Order)
60+
61+
* Ziggie

graph/db/graph_cache.go

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,9 @@ func (c *GraphCache) AddChannel(info *models.CachedEdgeInfo,
121121
return
122122
}
123123

124-
if policy1 != nil && policy1.IsDisabled() &&
125-
policy2 != nil && policy2.IsDisabled() {
126-
127-
return
128-
}
129-
130-
// Create the edge entry for both nodes.
124+
// Create the edge entry for both nodes. We always add the channel
125+
// structure to the cache, even if both policies are currently disabled,
126+
// so that later policy updates can find and update the channel entry.
131127
c.mtx.Lock()
132128
c.updateOrAddEdge(info.NodeKey1Bytes, &DirectedChannel{
133129
ChannelID: info.ChannelID,
@@ -143,6 +139,19 @@ func (c *GraphCache) AddChannel(info *models.CachedEdgeInfo,
143139
})
144140
c.mtx.Unlock()
145141

142+
// Skip adding policies if both are disabled, as the channel is
143+
// currently unusable for routing. However, we still add the channel
144+
// structure above so that policy updates can later enable it.
145+
if policy1 != nil && policy1.IsDisabled() &&
146+
policy2 != nil && policy2.IsDisabled() {
147+
148+
log.Debugf("Skipping policies for channel %v: both "+
149+
"policies are disabled (channel structure still "+
150+
"cached for future updates)", info.ChannelID)
151+
152+
return
153+
}
154+
146155
// The policy's node is always the to_node. So if policy 1 has to_node
147156
// of node 2 then we have the policy 1 as seen from node 1.
148157
if policy1 != nil {

graph/db/graph_cache_test.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,113 @@ func assertCachedPolicyEqual(t *testing.T, original,
139139
require.Equal(t, original.ToNodePubKey(), cached.ToNodePubKey())
140140
}
141141
}
142+
143+
// TestGraphCacheDisabledPoliciesRegression is a regression test for the bug
144+
// where channels with both policies disabled were not added to the graph cache
145+
// during population, preventing future policy updates from working.
146+
//
147+
// The bug flow was:
148+
// 1. Channel with both policies disabled exists in DB.
149+
// 2. populateCache skips adding it to graph cache entirely.
150+
// 3. Later, a policy update arrives enabling one direction.
151+
// 4. UpdateEdgePolicy updates the DB successfully.
152+
// 5. UpdateEdgePolicy tries to update graph cache but channel not found.
153+
// 6. Channel never becomes usable for routing.
154+
func TestGraphCacheDisabledPoliciesRegression(t *testing.T) {
155+
t.Parallel()
156+
157+
// Create a simple cache instance.
158+
cache := NewGraphCache(10)
159+
160+
// Simulate a channel with both policies disabled.
161+
chanID := uint64(12345)
162+
node1 := pubKey1
163+
node2 := pubKey2
164+
165+
edgeInfo := &models.CachedEdgeInfo{
166+
ChannelID: chanID,
167+
NodeKey1Bytes: node1,
168+
NodeKey2Bytes: node2,
169+
Capacity: 1000000,
170+
}
171+
172+
// Create two disabled policies.
173+
disabledPolicy1 := &models.CachedEdgePolicy{
174+
ChannelID: chanID,
175+
ChannelFlags: lnwire.ChanUpdateDisabled,
176+
}
177+
disabledPolicy2 := &models.CachedEdgePolicy{
178+
ChannelID: chanID,
179+
ChannelFlags: lnwire.ChanUpdateDisabled |
180+
lnwire.ChanUpdateDirection,
181+
}
182+
183+
// Add the channel with both policies disabled (simulating
184+
// populateCache).
185+
cache.AddChannel(edgeInfo, disabledPolicy1, disabledPolicy2)
186+
187+
// Verify the channel structure was added to cache.
188+
var foundChannels []*DirectedChannel
189+
err := cache.ForEachChannel(node1, func(c *DirectedChannel) error {
190+
if c.ChannelID == chanID {
191+
foundChannels = append(foundChannels, c)
192+
}
193+
194+
return nil
195+
})
196+
require.NoError(t, err)
197+
require.Len(t, foundChannels, 1,
198+
"channel structure should be in cache even when both "+
199+
"policies are disabled")
200+
201+
// Verify policies were NOT added (both disabled).
202+
require.False(t, foundChannels[0].OutPolicySet,
203+
"disabled outgoing policy should not be set in cache")
204+
require.Nil(t, foundChannels[0].InPolicy,
205+
"disabled incoming policy should not be set in cache")
206+
207+
// Now simulate receiving a fresh update enabling one direction.
208+
enabledPolicy1 := &models.CachedEdgePolicy{
209+
ChannelID: chanID,
210+
ChannelFlags: 0, // NOT disabled anymore
211+
TimeLockDelta: 40,
212+
MinHTLC: lnwire.MilliSatoshi(1000),
213+
}
214+
215+
// Update the policy (simulating what UpdateEdgePolicy does).
216+
cache.UpdatePolicy(enabledPolicy1, node1, node2)
217+
218+
// Verify the policy update succeeded. Before the fix, UpdatePolicy
219+
// would log "Channel not found in graph cache" and return early,
220+
// so the policy would never be added.
221+
foundChannels = nil
222+
err = cache.ForEachChannel(node1, func(c *DirectedChannel) error {
223+
if c.ChannelID == chanID {
224+
foundChannels = append(foundChannels, c)
225+
}
226+
227+
return nil
228+
})
229+
require.NoError(t, err)
230+
require.Len(t, foundChannels, 1)
231+
232+
// The policy should now be set.
233+
require.True(t, foundChannels[0].OutPolicySet,
234+
"REGRESSION: policy update should work even for channels that "+
235+
"had both policies disabled initially")
236+
237+
// Verify we can also see it from node2's perspective.
238+
foundChannels = nil
239+
err = cache.ForEachChannel(node2, func(c *DirectedChannel) error {
240+
if c.ChannelID == chanID {
241+
foundChannels = append(foundChannels, c)
242+
}
243+
244+
return nil
245+
})
246+
require.NoError(t, err)
247+
require.Len(t, foundChannels, 1)
248+
require.NotNil(t, foundChannels[0].InPolicy,
249+
"incoming policy should be set after policy update")
250+
require.Equal(t, uint16(40), foundChannels[0].InPolicy.TimeLockDelta)
251+
}

0 commit comments

Comments
 (0)