routing: prune single direction#2243
Conversation
joostjager
force-pushed
the
prune-single-direction
branch
from
526f5eb to
d748c67
Compare
joostjager
force-pushed
the
prune-single-direction
branch
2 times, most recently
from
3f0b47e to
b563ed7
Compare
halseth
left a comment
halseth
left a comment
There was a problem hiding this comment.
Nice! Looks more or less LGTM, only a few nits.
There was a problem hiding this comment.
Feel like we might as well remove this methods now?
There was a problem hiding this comment.
Only spot I see it used now is the tests.
There was a problem hiding this comment.
It was also used in getFailedChannelID. But good suggestion, removed the map, the unit test and updated getFailedChannelID.
There was a problem hiding this comment.
to reduce commit size this could be introduced in a separate commit :)
There was a problem hiding this comment.
This is too granular for me and I am also not sure if it makes sense to review a data structure without knowing what it will be used for.
There was a problem hiding this comment.
It is just a personal preference of mine, as I tend to jump back and forth between commits anyway :)
Yes at this should indicate that the channel is completely unusable (say the commitment transaction has been broadcast). |
Roasbeef
left a comment
Roasbeef
left a comment
There was a problem hiding this comment.
I dig the set of changes, always nice to be more precise. After a rebase, and the style nits addressed, this should be mostly good to go.
There was a problem hiding this comment.
Only spot I see it used now is the tests.
There is the general assumption that channel edge policy nodes are ordered such that the node1 pubkey is smaller than the key of node 2. In the test graph, this assumption didn't hold. This commit fixes the test graph and also adds a check to prevent this from happening again.
joostjager
force-pushed
the
prune-single-direction
branch
3 times, most recently
from
3404a15 to
f821a2a
Compare
Added bidirectional pruning in case of |
|
Comments processed |
There was a problem hiding this comment.
style nit: continue in case errSource != hop.PubKeyBytes to avoid indentatin.
Hop maps were used in a test to verify the population of the hop map itself and further only in a single function (getFailedChannelID). Rewrote that function and removed the hop maps completely.
In this commit we introduce pruning of channel edges instead of channels. Channel failures apply to a single direction and it is unnecessarily restricting to prune both directions.
joostjager
force-pushed
the
prune-single-direction
branch
from
f821a2a to
3ed1683
Compare
| // continue with the rest of the routes. | ||
| case *lnwire.FailPermanentChannelFailure: | ||
| paySession.ReportChannelFailure(failedChanID) | ||
| paySession.ReportEdgeFailure(&edgeLocator{ |
4 participants
In this PR we introduce pruning of channel edges instead of channels. Channel failures apply to a single direction and it is unnecessarily restricting to prune both directions
Open question: for PermanentChannelFailure, should both directions be pruned?