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.

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

mpp_split: use single nodes for mpp payments over trampoline #7107

Merged
merged 1 commit into from
Mar 17, 2021

Conversation

bitromortac
Copy link
Contributor

@bitromortac bitromortac commented Mar 15, 2021

Adds the node id as an input to mpp_split (extending the short_channel_id key), to be able to filter out split configurations that pay via several nodes. We need to do this in case of trampoline multipart payments, see ACINQ/eclair#1723. Related: #7087.

TestPeer now inherits from TestCaseForTestnet instead of ElectrumTestCase to disable filtering of single nodes in tests, to not break test_multipart_payment_with_trampoline, which we want to keep in its current form (as intended for the future without the above mentioned bug). Let me know if there's a better way to do this.

@ecdsa
Copy link
Member

ecdsa commented Mar 16, 2021

It looks good. Note that we use self.channel_db is None as a way to detect trampoline all over the place. It may be clearer to use an explicit variable name or method, but in that case it should be done everywhere, not just in a single method. I think introducing heterogeneous notations is worse. Also, if a variable is a boolean, the name should reflect it, eg is_trampoline instead of trampoline_payment.

@bitromortac
Copy link
Contributor Author

I reverted to not self.channel_db which is also used in other places, to unify. Also a good point concerning naming of boolean variables.

@ecdsa ecdsa merged commit 8d8f078 into spesmilo:master Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants