routing: globalize mc#3298
Merged
Merged
Conversation
joostjager
requested review from
Roasbeef,
cfromknecht and
halseth
as code owners
joostjager
force-pushed
the
globalize-mc
branch
2 times, most recently
from
4065566 to
8955870
Compare
This was referenced Jul 11, 2019
cfromknecht
reviewed
Jul 11, 2019
Contributor
Author
|
Tacked on one more commit that cleans up the payment session, making the scope of this PR a bit more complete. |
Contributor
Author
cfromknecht
approved these changes
Jul 12, 2019
Contributor
cfromknecht
left a comment
cfromknecht
left a comment
There was a problem hiding this comment.
@joostjager LGTM apart from nits on interface exporting ✅
If nodes return a channel policy related failure, they may get a second chance. Our graph may not be up to date. Previously this logic was contained in the payment session. This commit moves that into global mission control and thereby removes the last mission control state that was kept on the payment level. Because mission control is not aware of the relation between payment attempts and payments, the second chance logic is no longer based tracking second chances given per payment. Instead a time based approach is used. If a node reports a policy failure that prevents forwarding to its peer, it will get a second chance. But it will get it only if the previous second chance was long enough ago. Also those second chances are no longer dependent on whether an associated channel update is valid. It will get the second chance regardless, to prevent creating a dependency between mission control and the graph. This would interfer with (future) replay of history, because the graph may not be the same anymore at that point.
As there is no more state kept in the payment session, failure reporting can go straight to mission control.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR moves the last payment-specific mission control state into the global mission control instances.
Builds off of #3188
The PR is a base for #3221 and #3164