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

Enable multipart trampoline payments via several different trampoline nodes #7604

Closed
bitromortac opened this issue Dec 22, 2021 · 1 comment · Fixed by #7623
Closed

Enable multipart trampoline payments via several different trampoline nodes #7604

bitromortac opened this issue Dec 22, 2021 · 1 comment · Fixed by #7623

Comments

@bitromortac
Copy link
Contributor

bitromortac commented Dec 22, 2021

Since ACINQ/eclair#1723 was merged since a while I think we could enable splitting payments via different trampoline nodes. Currently, this is prevented:

use_singe_node = constants.net is constants.BitcoinMainnet

On testnet, I have two channels, each with a different trampoline. I try to send an amount (400000 sat) that forces splitting (legacy multipart trampoline payment). On testnet, the above restriction is not active, so it should work to send via different trampoline nodes. The payment reaches the trampoline nodes, but at the receiver side (an LND instance) I get:

2021-12-20 16:05:50.486 [DBG] INVC: Invoice(pay_hash=XXX, pay_addr=XXX): failure resolution result outcome: set total too low for invoice, at accept height: 2132478, amt=270211462 mSAT, expiry=2132518, circuit=(Chan ID=XXX, HTLC ID=24), mpp=total=270211462 mSAT, addr=XXX, amp=<nil>

This means that the trampoline payments didn't include the correct total_msat value for the invoice, which is why it makes sense that the client receives INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS. From what I can see, is that Electrum includes the correct total_msat value, so it could be a bug in eclair, but I'm unsure.

Anyhow, enabling mpp trampoline payments would further increase robustness and usefulness having channels with different trampolines.

The trampoline payloads look like this for instance:

I | trampoline | payload 0 {'amt_to_forward': {'amt_to_forward': 261693790}, 'outgoing_cltv_value': {'outgoing_cltv_value': 2133093}, 'outgoing_node_id': {'outgoing_node_id': pubkeyT1}}
I | trampoline | payload 1 {'amt_to_forward': {'amt_to_forward': 261664624}, 'outgoing_cltv_value': {'outgoing_cltv_value': 2132517}, 'outgoing_node_id': {'outgoing_node_id': pubkeyT2}, 'invoice_features': {'invoice_features': <LnFeatures.BASIC_MPP_OPT|PAYMENT_SECRET_REQ|VAR_ONION_OPT: 147968>}, 'invoice_routing_info': {'invoice_routing_info': b''}, 'payment_data': {'payment_secret': XXX1, 'total_msat': 400000000}}
I | trampoline | payload 2 {'amt_to_forward': {'amt_to_forward': 261664624}, 'outgoing_cltv_value': {'outgoing_cltv_value': 2132517}, 'payment_data': {'payment_secret': XXX1, 'total_msat': 400000000}}
@bitromortac
Copy link
Contributor Author

bitromortac commented Jan 6, 2022

The legacy splitting is an Eclair bug (private communication), I'm going to open an issue over there.

Further tests for E2E are encouraging:

E2E Scenario split via two trampolines (working):
S -C1-> T1 -C3-> R
S -C2-> T2 -C4-> R

E2E Scenario 2 split via two trampolines, T1 aggregates (working):
S -C1-> T1 -C3-> R
S -C2-> T1
S -C4-> T2 -C5-> R

There are some issues with using second trampoline hops. I think we should remove those and reintroduce in a later PR with proper error accounting (fees).

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 a pull request may close this issue.

1 participant