missioncontrol: add invalid onion blinding handling for blinded paths #8095

carlaKC · 2023-10-13T19:29:35Z

This PR updates mission control to handle InvalidOnionBlinding errors, fixes #7882.

Key ideas in this PR (h/t @bitromortac for digging into edge cases):

Blinded errors should only every originate from an introduction node.
We should not penalize the introduction node, as it is responsible for reporting on behalf of the blinded route.
We should minimize the number of ephemeral blinded keys that we store in mission control, as they're single use (also the case with bolt 11 hop hints).

Key for images:

Yellow = node sending InvalidOnionBlinding
Pink = node penalized
Green = node success

❗ Handling has been updated to also reward the incoming link to the introduction node after discussion on the PR, images are slightly outdated ❗

Regular Blinded Paths

Misuse of Error Code

Final Hop

Final Hop Case:

bitromortac

Nice 👍, I provided some initial feedback, hope my understanding is correct there.

lnwire/onion_error.go

bitromortac · 2023-10-18T14:56:58Z

routing/result_interpretation.go

+		// Everything up until the error source forwarded the payment
+		// correctly.
+		i.successPairRange(route, 0, errorSourceIdx-1)
+		i.failPairRange(


With failPairRange this fails pairs after errorSourceIdx completely, which means that all hops afterwards are discarded for any amount in future attempts, because the failure amount is set to zero (additionally pairs are failed backwards and forwards, which may be unneccesary). I feel like we should use failPairBalance only for the last hop, as this will already invalidate the blinded part of the route for the amount that was tried, but it leaves some room for the splitting algo to retry the blinded path in case of multiple blinded paths? The other benefit of this would be to not pollute mission control with many ephemeral keys (payment successes could be handled similarly).

I think we ideally don't want any ephemeral outgoing pairs associated with the introduction node to be reported failed (as it currently is) nor succeeded. Node reputation is computed on all outgoing results of a node, which is why this would add/remove node penalization (hope my analysis is correct, see https://github.com/lightningnetwork/lnd/blob/master/routing/missioncontrol.go#L335 and https://github.com/lightningnetwork/lnd/blob/master/routing/probability_apriori.go#L180). Otherwise this may be gameable by providing faulty invoices that would worsen/improve introduction nodes' routing reputation.

I feel like we should use failPairBalance only for the last hop, as this will already invalidate the blinded part of the route for the amount that was tried

Oh yeah, this is much nicer. WDYT we should do in the case of a single hop blinded route (Introduction -> Blinded Recipient), because failPairBalance is only effective on intermediate nodes rn? afaik we don't have a way to penalize the blinded recipient without penalizing the intro node, so just set a final failure so that we give up?

Otherwise this may be gameable by providing faulty invoices that would worsen/improve introduction nodes' routing reputation.

I do wonder how much we actually need to worry about this? To "poison" an introduction node, the malicious receiver needs to get the sender to pay that invoice. Which would only happen if the sender wants to make that payment (buy something / send money etc), and then surely the receiver is more incentivized to "shut up and take my money"? Not disagreeing with leaving the intro node untouched (makes sense for node score), just not sure how much of a threat it would be.

Thanks for taking a look!

Oh yeah, this is much nicer. WDYT we should do in the case of a single hop blinded route (Introduction -> Blinded Recipient), because failPairBalance is only effective on intermediate nodes rn? afaik we don't have a way to penalize the blinded recipient without penalizing the intro node, so just set a final failure so that we give up?

Yes, that one is tricky, but it seems reasonable to fail the payment, since the receiver should have added a route hint that has liquidity. Another case which is not handled right now is the case where we see FailInvalidBlinding from a node after the introduction point (spec violated twice?). In that case we probably would like to punish the introduction node for not letting the error originate from it (thereby also invalidating the blinded route). Since all of this is really complicated I tried check any edge cases as well (https://github.com/bitromortac/lnd/tree/re-8095-231018-11-blinded-error).

Yes, that one is tricky, but it seems reasonable to fail the payment, since the receiver should have added a route hint that has liquidity.

I think this might get a little tricky with multi-blinded path support, but I think this is a good start for the single-path case.

Thanks for going through all the edge cases! Updated code to account for single hop case + errors after the introduction node. Also updated PR description with overview of each case - lmk if anything is missing / different to your interpretation.

bitromortac

Nice updates, great code documentation! I think there are two issues left, one is to mark the hop before the intro node successful in some cases and to check error conversion for all error types if it's a blinded route.

routing/result_interpretation.go

routing/result_interpretation_test.go

routing/result_interpretation.go

carlaKC · 2023-11-06T18:42:40Z

Updated handling of blinded errors after the introduction node to a higher level in the decision tree (before we decide to handle intermediate / final). I think that this makes sense because it's a new "class" or error type we need to handle, but also happy to move it into the individual process functions if that's more readable.

bitromortac

LGTM 🎉

lightninglabs-deploy · 2023-12-07T22:21:16Z

@ziggie1984: review reminder

carlaKC · 2023-12-12T14:45:54Z

Latest push just a rebase!

ziggie1984

Very difficult topic and rock solid PR 💪

Had only some nits.

routing/result_interpretation.go

routing/result_interpretation_test.go

routing/result_interpretation.go

routing/result_interpretation_test.go

This commit adds handling for errors that originate after the introduction node when making payment to a blinded route. This indicates that the introduction node is not obeying the spec, so it is punished for the violation.

This commit adds handling for route blinding errors that are reported by the introduction node in a multi-hop blinded route. As the introduction node is always responsible for handling blinded errors, it is not penalized - only the final hop is penalized to discourage the blinded route without filling up mission control with ephemeral results. If this error code is reported by a node that is not an introduction node, we penalize the node because it is returning an error code that it should not be using.

Note: this refactor updates the inequality used from >= 2 to > 1 to align with the rest of this file so that we express this concept consistently throughout the code.

We do not expect blinding errors from the final node: 1. If the introduction is the recipient, they should use regular errors. 2. Otherwise, nodes have no business sending this error when they are not part of a blinded route.

ziggie1984

LGTM ⭐️

EDIT: Maybe it makes sense to create an issue that specifies that the error handling will need to get updated when we are capable of forwarding/failing blinded path payments ?

Roasbeef · 2024-01-03T20:29:36Z

EDIT: Maybe it makes sense to create an issue that specifies that the error handling will need to get updated when we are capable of forwarding/failing blinded path payments ?

Def, can you create this (given current context) @carlaKC?

carlaKC · 2024-01-09T17:14:30Z

Opened #8363 with write up, @ziggie1984 / @bitromortac please give it a peep to make sure I've covered everything properly!

carlaKC force-pushed the 7882-mcblinding branch 2 times, most recently from 152ca56 to dd01d71 Compare October 13, 2023 19:31

Roasbeef added this to the v0.18.0 milestone Oct 13, 2023

Roasbeef added the blinded paths label Oct 13, 2023

Roasbeef requested a review from bitromortac October 13, 2023 19:31

Roasbeef added routing mission control labels Oct 13, 2023

saubyk assigned carlaKC Oct 17, 2023

bitromortac reviewed Oct 18, 2023

View reviewed changes

carlaKC force-pushed the 7882-mcblinding branch from dd01d71 to 4aa871c Compare October 31, 2023 13:56

carlaKC requested a review from bitromortac October 31, 2023 14:25

bitromortac reviewed Nov 2, 2023

View reviewed changes

routing/result_interpretation.go Outdated Show resolved Hide resolved

routing/result_interpretation_test.go Outdated Show resolved Hide resolved

routing/result_interpretation.go Outdated Show resolved Hide resolved

carlaKC force-pushed the 7882-mcblinding branch from 4aa871c to b97ce37 Compare November 6, 2023 18:40

bitromortac self-requested a review November 7, 2023 10:49

bitromortac approved these changes Nov 9, 2023

View reviewed changes

saubyk requested a review from ziggie1984 November 9, 2023 19:46

carlaKC added 2 commits December 12, 2023 09:44

lnwire: add invalid onion blinding error code

fd8381b

lnrpc: surface invalid onion blinding on rpc

d017fe0

carlaKC force-pushed the 7882-mcblinding branch from b97ce37 to 6f67d52 Compare December 12, 2023 14:45

ziggie1984 reviewed Dec 18, 2023

View reviewed changes

carlaKC force-pushed the 7882-mcblinding branch 2 times, most recently from 5a3a4ab to 0075a8e Compare December 18, 2023 17:05

carlaKC added 5 commits December 18, 2023 12:07

routing: handle introduction node failure to convert error

f91589b

This commit adds handling for errors that originate after the introduction node when making payment to a blinded route. This indicates that the introduction node is not obeying the spec, so it is punished for the violation.

routing/refactor: add failNode helper to final outcome processing

7ce3a15

Note: this refactor updates the inequality used from >= 2 to > 1 to align with the rest of this file so that we express this concept consistently throughout the code.

routing: add handling for blinded errors from final node

4071db7

We do not expect blinding errors from the final node: 1. If the introduction is the recipient, they should use regular errors. 2. Otherwise, nodes have no business sending this error when they are not part of a blinded route.

docs: add release notes

b31bab3

carlaKC force-pushed the 7882-mcblinding branch from 0075a8e to b31bab3 Compare December 18, 2023 17:08

ziggie1984 approved these changes Dec 19, 2023

View reviewed changes

Roasbeef merged commit ffd330a into lightningnetwork:master Jan 3, 2024
23 of 25 checks passed

carlaKC mentioned this pull request Jan 9, 2024

[feature]: Support Payment to Multiple Blinded Paths #8363

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

missioncontrol: add invalid onion blinding handling for blinded paths #8095

missioncontrol: add invalid onion blinding handling for blinded paths #8095

carlaKC commented Oct 13, 2023 •

edited

Loading

bitromortac left a comment

bitromortac Oct 18, 2023

carlaKC Oct 20, 2023

bitromortac Oct 25, 2023

carlaKC Oct 31, 2023

bitromortac left a comment

carlaKC commented Nov 6, 2023

bitromortac left a comment

lightninglabs-deploy commented Dec 7, 2023

carlaKC commented Dec 12, 2023

ziggie1984 left a comment

ziggie1984 left a comment •

edited

Loading

Roasbeef commented Jan 3, 2024

carlaKC commented Jan 9, 2024

missioncontrol: add invalid onion blinding handling for blinded paths #8095

missioncontrol: add invalid onion blinding handling for blinded paths #8095

Conversation

carlaKC commented Oct 13, 2023 • edited Loading

Regular Blinded Paths

Misuse of Error Code

Final Hop

bitromortac left a comment

Choose a reason for hiding this comment

bitromortac Oct 18, 2023

Choose a reason for hiding this comment

carlaKC Oct 20, 2023

Choose a reason for hiding this comment

bitromortac Oct 25, 2023

Choose a reason for hiding this comment

carlaKC Oct 31, 2023

Choose a reason for hiding this comment

bitromortac left a comment

Choose a reason for hiding this comment

carlaKC commented Nov 6, 2023

bitromortac left a comment

Choose a reason for hiding this comment

lightninglabs-deploy commented Dec 7, 2023

carlaKC commented Dec 12, 2023

ziggie1984 left a comment

Choose a reason for hiding this comment

ziggie1984 left a comment • edited Loading

Choose a reason for hiding this comment

Roasbeef commented Jan 3, 2024

carlaKC commented Jan 9, 2024

carlaKC commented Oct 13, 2023 •

edited

Loading

ziggie1984 left a comment •

edited

Loading