routing: ignore ChannelUpdates for unknown channels#2549
routing: ignore ChannelUpdates for unknown channels#2549halseth merged 3 commits intolightningnetwork:masterfrom
Conversation
d4eb3e8 to
883314b
Compare
routing/router_test.go
Outdated
| t.Parallel() | ||
|
|
||
| const startingBlockHeight = 101 | ||
| ctx, cleanUp, err := createTestCtxFromFile(startingBlockHeight, |
There was a problem hiding this comment.
Nit: function args should be formatted on their own line.
routing/router_test.go
Outdated
|
|
||
| // Add the edge between the two unknown nodes to the graph, and check | ||
| // that the nodes are found after the fact. | ||
| fundingTx, _, chanID, err := createChannelEdge(ctx, |
448945b to
29bd228
Compare
|
PTAL @wpaulino |
29bd228 to
e5ac668
Compare
routing/router.go
Outdated
There was a problem hiding this comment.
We return a slightly different error object than before. Is it okay with the callers?
There was a problem hiding this comment.
Good question! I looked into this, and the error will eventually be delivered back to the gossiper.
Previously we would log the error and start rejecting this channel:
Line 2040 in 351865a
While we now will just ignore it, and process it as normal if we later receive it.
Note however, that this is situation most likely will never occur, since the gossiper does its own existence check first:
Line 1942 in 351865a
There was a problem hiding this comment.
Can the situation occur if something happens in between the two db queries for the channel?
There was a problem hiding this comment.
Even if it does, the error is handled correctly by the caller here:
Line 2034 in 351865a
There was a problem hiding this comment.
Ok, clear.
The other use of UpdateEdge is in ChannelRouter.applyChannelUpdate. But I think the behaviour improves there too?
e5ac668 to
69ad94c
Compare
|
PTAL @joostjager @wpaulino |
69ad94c to
01ea797
Compare
Since we should always get a
ChanInfobefore a channel update, we can exit early if we receive aChannelUpdatefor an edge not in the graph.Earlier we would attempt to
GetUTXOin the case we couldn't find it in the graph, which is a heavy operation for some backends, with the subsequentUpdateEdgePolicyfailing regardless.Broken off of #2279