Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Multipart FSM v2 #1439

Merged
merged 9 commits into from
Jun 23, 2020
Merged

Multipart FSM v2 #1439

merged 9 commits into from
Jun 23, 2020

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented May 27, 2020

This is based on #1427, so please ignore the first commits.

mpp-fsm-v2

We re-work the MultiPartPaymentLifecycle state machine to leverage the new MPP split algorithm exposed by the router. This greatly simplifies the FSM, removes a few hacks that were added to the PaymentLifecycle, and should make MPP split better for both mobile and server nodes.

It also allows re-enabling the use of MPP for trampoline relays to non-trampoline nodes (yay!).
I have done a lot of regtest testing to simulate Phoenix/Eclair-Mobile scenario, but nothing beats the real thing. Once this is approved, we should create testnet builds for Phoenix and Eclair-Mobile and extensively test that everything works.

Since it was already a lot of changes, I refrained from refactoring the RouteRequest part. We can improve this later by cleaning up some types / creating nice type hierarchies.

NB: the failing test is because of #1434

@t-bast t-bast marked this pull request as ready for review May 27, 2020 12:18
@t-bast t-bast requested a review from pm47 May 27, 2020 12:18
pm47
pm47 reviewed May 28, 2020
View reviewed changes
Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

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

Looks okay to me, just a few nits and suggestions.

Agree that we should probably merge asap and test in real life conditions.

@t-bast t-bast mentioned this pull request Jun 2, 2020
Merged
pm47
pm47 previously approved these changes Jun 3, 2020
View reviewed changes
@t-bast t-bast mentioned this pull request Jun 3, 2020
Merged
t-bast added 7 commits June 22, 2020 10:11
@t-bast
PaymentLifecycle cleanup
4b4b9cb 
Remove temporary hooks added for first version of MPP (route prefix,
empty routes, etc).

Allow specifying the whole route (not only nodeIds) in SendPaymentToRoute.
@t-bast
Rework MultiPartPaymentLifecycle
a22795c 
Use the Router's new MPP RouteRequest.
Remove the "blind" split based on channel balances.
Reactivate trampoline relay to MPP non-trampoline recipient.
@t-bast
Add MPP payment metrics
bb663da
@t-bast
Activate MPP feature by default
92a8d5e
@t-bast
Small changes/cleanup
58d00e5 
* Improve comments
* Add metric to track effectiveness of retrying failed channels
* Small refactoring
@t-bast
Group ignoreNodes and ignoreChannels
da1df39
@t-bast
Fix config features test
8e9e337
@t-bast t-bast force-pushed the multipart-fsm-v2 branch from 2c070f9 to 8e9e337 Compare June 22, 2020 08:19
@t-bast t-bast requested a review from pm47 June 22, 2020 08:25
pm47
pm47 reviewed Jun 22, 2020
View reviewed changes
@t-bast t-bast merged commit d9f257a into master Jun 23, 2020
@t-bast t-bast deleted the multipart-fsm-v2 branch June 23, 2020 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pm47 pm47 pm47 approved these changes

Assignees
No one assigned
Labels
Optech Make Me Famous!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@t-bast @pm47