Routing to blinded payment paths

[valentinewallace]

Support finding a route to a recipient who is behind blinded payment paths, which are provided in BOLT12 invoices.

• finish tests

Based on #2258, #2305.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.61756% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (d78dd48) to head (6c3ca55).
⚠️ Report is 5192 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/router.rs	94.61%	18 Missing ⚠️
lightning/src/offers/invoice.rs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2120 +/- ##
==========================================
+ Coverage 90.48% 91.30% +0.81% 
==========================================
Files 104 106 +2 Lines 53920 63000 +9080 Branches 53920 63000 +9080 ==========================================
+ Hits 48792 57525 +8733 - Misses 5128 5475 +347 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We should definitely land at least the new serialization backwards-incompatible hints tlvs for the next release.

[TheBlueMatt]

Should this get some kind of TODO? Or should we just always use this path directly?

[valentinewallace]

We can't use these paths atm because get_route doesn't have the ability to "advance" the blinded path to the next hop (and we can't pathfind to ourselves, ofc). Previously discussed here: #2146 (comment)

[TheBlueMatt]

Ah, I misunderstood that comment, so I think there's another way to handle this - let get_route return early with a 0-hop unblinded path portion and the blinded tail as-is. Then the paying code would handle it by detecting this case and doing the advancing itself.

[valentinewallace]

Hmm, I don't see how that would work if the max_htlc of the 1 blinded hint isn't sufficient for the entire payment?

[TheBlueMatt]

Mmm right, I suppose we could pre-select it as a path and then run the router to select more paths if needed? That seems like it would work pretty easy.

[valentinewallace]

Yeah that sounds good! We might want to have another prefactor, though, because there's still assumptions that path.hops.len() > 0 scattered around. Will look into that.

[valentinewallace]

FYI, two more notes on this approach:

1. We may end up double-booking our own channel liquidity, since we don't know which of our channels is being used inside the blinded path (double-booking liquidity is a general problem for blinded pathfinding anyway, though)
2. ISTM we'll always want to go through the process of selecting additional paths, because the blinded path where we are the intro node may be the most expensive option

For (1), we could decrement each of our available channel balances by the amount used on the path. Not sure it's if worth the additional complexity.

[valentinewallace]

I'm gonna address this in a follow-up since this PR is growing. IMO our offers code should still advance blinded paths before pathfinding to them, though adding the behavior you describe is good for keeping find_route general-purpose

[valentinewallace]

Rebased to fix CI, also squashed.

[alecchendev]

More of a comprehension question - if a recipient includes a dummy hop, we'll send extra sats for this dummy hop's fees that will go to the recipient right? Is there anything stopping a recipient from always making a sender slightly overpay as long as there's a route with enough liquidity?

[valentinewallace]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments, based on the non-dummy hops' feerates. I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

[alecchendev]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments

Oh I see, thanks

I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

I realize reading more of the proposal that recipients are even encouraged to in order to avoid probing, although I also saw this section where the recipient pays the blinded fees which was interesting. But yea, seems like nothing's really stopping a recipient from getting the sender to overpay as long as its not outrageous...🤷

[valentinewallace]

Squashed.

[jkczyz]

Largely looks good though I need to do a more detailed pass at the last commit still.

[TheBlueMatt]

LGTM, I think, feel free to squash fixups.

[TheBlueMatt]

One small thing worth fixing in a followup, though I'm not even sure the code is actually reachable.

[valentinewallace]

Support finding a route to a recipient who is behind blinded payment paths, which are provided in BOLT12 invoices.

• finish tests

Based on #2258, #2305.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.61756% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (d78dd48) to head (6c3ca55).
⚠️ Report is 5192 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/router.rs	94.61%	18 Missing ⚠️
lightning/src/offers/invoice.rs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2120 +/- ##
==========================================
+ Coverage 90.48% 91.30% +0.81% 
==========================================
Files 104 106 +2 Lines 53920 63000 +9080 Branches 53920 63000 +9080 ==========================================
+ Hits 48792 57525 +8733 - Misses 5128 5475 +347 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We should definitely land at least the new serialization backwards-incompatible hints tlvs for the next release.

[TheBlueMatt]

Should this get some kind of TODO? Or should we just always use this path directly?

[valentinewallace]

We can't use these paths atm because get_route doesn't have the ability to "advance" the blinded path to the next hop (and we can't pathfind to ourselves, ofc). Previously discussed here: #2146 (comment)

[TheBlueMatt]

Ah, I misunderstood that comment, so I think there's another way to handle this - let get_route return early with a 0-hop unblinded path portion and the blinded tail as-is. Then the paying code would handle it by detecting this case and doing the advancing itself.

[valentinewallace]

Hmm, I don't see how that would work if the max_htlc of the 1 blinded hint isn't sufficient for the entire payment?

[TheBlueMatt]

Mmm right, I suppose we could pre-select it as a path and then run the router to select more paths if needed? That seems like it would work pretty easy.

[valentinewallace]

Yeah that sounds good! We might want to have another prefactor, though, because there's still assumptions that path.hops.len() > 0 scattered around. Will look into that.

[valentinewallace]

FYI, two more notes on this approach:

1. We may end up double-booking our own channel liquidity, since we don't know which of our channels is being used inside the blinded path (double-booking liquidity is a general problem for blinded pathfinding anyway, though)
2. ISTM we'll always want to go through the process of selecting additional paths, because the blinded path where we are the intro node may be the most expensive option

For (1), we could decrement each of our available channel balances by the amount used on the path. Not sure it's if worth the additional complexity.

[valentinewallace]

I'm gonna address this in a follow-up since this PR is growing. IMO our offers code should still advance blinded paths before pathfinding to them, though adding the behavior you describe is good for keeping find_route general-purpose

[valentinewallace]

Rebased to fix CI, also squashed.

[alecchendev]

More of a comprehension question - if a recipient includes a dummy hop, we'll send extra sats for this dummy hop's fees that will go to the recipient right? Is there anything stopping a recipient from always making a sender slightly overpay as long as there's a route with enough liquidity?

[valentinewallace]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments, based on the non-dummy hops' feerates. I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

[alecchendev]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments

Oh I see, thanks

I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

I realize reading more of the proposal that recipients are even encouraged to in order to avoid probing, although I also saw this section where the recipient pays the blinded fees which was interesting. But yea, seems like nothing's really stopping a recipient from getting the sender to overpay as long as its not outrageous...🤷

[valentinewallace]

Squashed.

[jkczyz]

Largely looks good though I need to do a more detailed pass at the last commit still.

[TheBlueMatt]

LGTM, I think, feel free to squash fixups.

[TheBlueMatt]

One small thing worth fixing in a followup, though I'm not even sure the code is actually reachable.

[valentinewallace]

Support finding a route to a recipient who is behind blinded payment paths, which are provided in BOLT12 invoices.

• finish tests

Based on #2258, #2305.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.61756% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (d78dd48) to head (6c3ca55).
⚠️ Report is 5192 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/router.rs	94.61%	18 Missing ⚠️
lightning/src/offers/invoice.rs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2120 +/- ##
==========================================
+ Coverage 90.48% 91.30% +0.81% 
==========================================
Files 104 106 +2 Lines 53920 63000 +9080 Branches 53920 63000 +9080 ==========================================
+ Hits 48792 57525 +8733 - Misses 5128 5475 +347 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We should definitely land at least the new serialization backwards-incompatible hints tlvs for the next release.

[TheBlueMatt]

Should this get some kind of TODO? Or should we just always use this path directly?

[valentinewallace]

We can't use these paths atm because get_route doesn't have the ability to "advance" the blinded path to the next hop (and we can't pathfind to ourselves, ofc). Previously discussed here: #2146 (comment)

[TheBlueMatt]

Ah, I misunderstood that comment, so I think there's another way to handle this - let get_route return early with a 0-hop unblinded path portion and the blinded tail as-is. Then the paying code would handle it by detecting this case and doing the advancing itself.

[valentinewallace]

Hmm, I don't see how that would work if the max_htlc of the 1 blinded hint isn't sufficient for the entire payment?

[TheBlueMatt]

Mmm right, I suppose we could pre-select it as a path and then run the router to select more paths if needed? That seems like it would work pretty easy.

[valentinewallace]

Yeah that sounds good! We might want to have another prefactor, though, because there's still assumptions that path.hops.len() > 0 scattered around. Will look into that.

[valentinewallace]

FYI, two more notes on this approach:

1. We may end up double-booking our own channel liquidity, since we don't know which of our channels is being used inside the blinded path (double-booking liquidity is a general problem for blinded pathfinding anyway, though)
2. ISTM we'll always want to go through the process of selecting additional paths, because the blinded path where we are the intro node may be the most expensive option

For (1), we could decrement each of our available channel balances by the amount used on the path. Not sure it's if worth the additional complexity.

[valentinewallace]

I'm gonna address this in a follow-up since this PR is growing. IMO our offers code should still advance blinded paths before pathfinding to them, though adding the behavior you describe is good for keeping find_route general-purpose

[valentinewallace]

Rebased to fix CI, also squashed.

[alecchendev]

More of a comprehension question - if a recipient includes a dummy hop, we'll send extra sats for this dummy hop's fees that will go to the recipient right? Is there anything stopping a recipient from always making a sender slightly overpay as long as there's a route with enough liquidity?

[valentinewallace]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments, based on the non-dummy hops' feerates. I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

[alecchendev]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments

Oh I see, thanks

I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

I realize reading more of the proposal that recipients are even encouraged to in order to avoid probing, although I also saw this section where the recipient pays the blinded fees which was interesting. But yea, seems like nothing's really stopping a recipient from getting the sender to overpay as long as its not outrageous...🤷

[valentinewallace]

Squashed.

[jkczyz]

Largely looks good though I need to do a more detailed pass at the last commit still.

[TheBlueMatt]

LGTM, I think, feel free to squash fixups.

[TheBlueMatt]

One small thing worth fixing in a followup, though I'm not even sure the code is actually reachable.

[valentinewallace]

Support finding a route to a recipient who is behind blinded payment paths, which are provided in BOLT12 invoices.

• finish tests

Based on #2258, #2305.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.61756% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (d78dd48) to head (6c3ca55).
⚠️ Report is 5192 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/router.rs	94.61%	18 Missing ⚠️
lightning/src/offers/invoice.rs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2120 +/- ##
==========================================
+ Coverage 90.48% 91.30% +0.81% 
==========================================
Files 104 106 +2 Lines 53920 63000 +9080 Branches 53920 63000 +9080 ==========================================
+ Hits 48792 57525 +8733 - Misses 5128 5475 +347 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We should definitely land at least the new serialization backwards-incompatible hints tlvs for the next release.

[TheBlueMatt]

Should this get some kind of TODO? Or should we just always use this path directly?

[valentinewallace]

We can't use these paths atm because get_route doesn't have the ability to "advance" the blinded path to the next hop (and we can't pathfind to ourselves, ofc). Previously discussed here: #2146 (comment)

[TheBlueMatt]

Ah, I misunderstood that comment, so I think there's another way to handle this - let get_route return early with a 0-hop unblinded path portion and the blinded tail as-is. Then the paying code would handle it by detecting this case and doing the advancing itself.

[valentinewallace]

Hmm, I don't see how that would work if the max_htlc of the 1 blinded hint isn't sufficient for the entire payment?

[TheBlueMatt]

Mmm right, I suppose we could pre-select it as a path and then run the router to select more paths if needed? That seems like it would work pretty easy.

[valentinewallace]

Yeah that sounds good! We might want to have another prefactor, though, because there's still assumptions that path.hops.len() > 0 scattered around. Will look into that.

[valentinewallace]

FYI, two more notes on this approach:

1. We may end up double-booking our own channel liquidity, since we don't know which of our channels is being used inside the blinded path (double-booking liquidity is a general problem for blinded pathfinding anyway, though)
2. ISTM we'll always want to go through the process of selecting additional paths, because the blinded path where we are the intro node may be the most expensive option

For (1), we could decrement each of our available channel balances by the amount used on the path. Not sure it's if worth the additional complexity.

[valentinewallace]

I'm gonna address this in a follow-up since this PR is growing. IMO our offers code should still advance blinded paths before pathfinding to them, though adding the behavior you describe is good for keeping find_route general-purpose

[valentinewallace]

Rebased to fix CI, also squashed.

[alecchendev]

More of a comprehension question - if a recipient includes a dummy hop, we'll send extra sats for this dummy hop's fees that will go to the recipient right? Is there anything stopping a recipient from always making a sender slightly overpay as long as there's a route with enough liquidity?

[valentinewallace]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments, based on the non-dummy hops' feerates. I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

[alecchendev]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments

Oh I see, thanks

I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

I realize reading more of the proposal that recipients are even encouraged to in order to avoid probing, although I also saw this section where the recipient pays the blinded fees which was interesting. But yea, seems like nothing's really stopping a recipient from getting the sender to overpay as long as its not outrageous...🤷

[valentinewallace]

Squashed.

[jkczyz]

Largely looks good though I need to do a more detailed pass at the last commit still.

[TheBlueMatt]

LGTM, I think, feel free to squash fixups.

[TheBlueMatt]

One small thing worth fixing in a followup, though I'm not even sure the code is actually reachable.

[valentinewallace]

Support finding a route to a recipient who is behind blinded payment paths, which are provided in BOLT12 invoices.

• finish tests

Based on #2258, #2305.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.61756% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (d78dd48) to head (6c3ca55).
⚠️ Report is 5192 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/router.rs	94.61%	18 Missing ⚠️
lightning/src/offers/invoice.rs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2120 +/- ##
==========================================
+ Coverage 90.48% 91.30% +0.81% 
==========================================
Files 104 106 +2 Lines 53920 63000 +9080 Branches 53920 63000 +9080 ==========================================
+ Hits 48792 57525 +8733 - Misses 5128 5475 +347 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We should definitely land at least the new serialization backwards-incompatible hints tlvs for the next release.

[TheBlueMatt]

Should this get some kind of TODO? Or should we just always use this path directly?

[valentinewallace]

We can't use these paths atm because get_route doesn't have the ability to "advance" the blinded path to the next hop (and we can't pathfind to ourselves, ofc). Previously discussed here: #2146 (comment)

[TheBlueMatt]

Ah, I misunderstood that comment, so I think there's another way to handle this - let get_route return early with a 0-hop unblinded path portion and the blinded tail as-is. Then the paying code would handle it by detecting this case and doing the advancing itself.

[valentinewallace]

Hmm, I don't see how that would work if the max_htlc of the 1 blinded hint isn't sufficient for the entire payment?

[TheBlueMatt]

Mmm right, I suppose we could pre-select it as a path and then run the router to select more paths if needed? That seems like it would work pretty easy.

[valentinewallace]

Yeah that sounds good! We might want to have another prefactor, though, because there's still assumptions that path.hops.len() > 0 scattered around. Will look into that.

[valentinewallace]

FYI, two more notes on this approach:

1. We may end up double-booking our own channel liquidity, since we don't know which of our channels is being used inside the blinded path (double-booking liquidity is a general problem for blinded pathfinding anyway, though)
2. ISTM we'll always want to go through the process of selecting additional paths, because the blinded path where we are the intro node may be the most expensive option

For (1), we could decrement each of our available channel balances by the amount used on the path. Not sure it's if worth the additional complexity.

[valentinewallace]

I'm gonna address this in a follow-up since this PR is growing. IMO our offers code should still advance blinded paths before pathfinding to them, though adding the behavior you describe is good for keeping find_route general-purpose

[valentinewallace]

Rebased to fix CI, also squashed.

[alecchendev]

More of a comprehension question - if a recipient includes a dummy hop, we'll send extra sats for this dummy hop's fees that will go to the recipient right? Is there anything stopping a recipient from always making a sender slightly overpay as long as there's a route with enough liquidity?

[valentinewallace]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments, based on the non-dummy hops' feerates. I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

[alecchendev]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments

Oh I see, thanks

I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

I realize reading more of the proposal that recipients are even encouraged to in order to avoid probing, although I also saw this section where the recipient pays the blinded fees which was interesting. But yea, seems like nothing's really stopping a recipient from getting the sender to overpay as long as its not outrageous...🤷

[valentinewallace]

Squashed.

[jkczyz]

Largely looks good though I need to do a more detailed pass at the last commit still.

[TheBlueMatt]

LGTM, I think, feel free to squash fixups.

[TheBlueMatt]

One small thing worth fixing in a followup, though I'm not even sure the code is actually reachable.

[valentinewallace]

Support finding a route to a recipient who is behind blinded payment paths, which are provided in BOLT12 invoices.

• finish tests

Based on #2258, #2305.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.61756% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (d78dd48) to head (6c3ca55).
⚠️ Report is 5192 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/router.rs	94.61%	18 Missing ⚠️
lightning/src/offers/invoice.rs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2120 +/- ##
==========================================
+ Coverage 90.48% 91.30% +0.81% 
==========================================
Files 104 106 +2 Lines 53920 63000 +9080 Branches 53920 63000 +9080 ==========================================
+ Hits 48792 57525 +8733 - Misses 5128 5475 +347 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We should definitely land at least the new serialization backwards-incompatible hints tlvs for the next release.

[TheBlueMatt]

Should this get some kind of TODO? Or should we just always use this path directly?

[valentinewallace]

We can't use these paths atm because get_route doesn't have the ability to "advance" the blinded path to the next hop (and we can't pathfind to ourselves, ofc). Previously discussed here: #2146 (comment)

[TheBlueMatt]

Ah, I misunderstood that comment, so I think there's another way to handle this - let get_route return early with a 0-hop unblinded path portion and the blinded tail as-is. Then the paying code would handle it by detecting this case and doing the advancing itself.

[valentinewallace]

Hmm, I don't see how that would work if the max_htlc of the 1 blinded hint isn't sufficient for the entire payment?

[TheBlueMatt]

Mmm right, I suppose we could pre-select it as a path and then run the router to select more paths if needed? That seems like it would work pretty easy.

[valentinewallace]

Yeah that sounds good! We might want to have another prefactor, though, because there's still assumptions that path.hops.len() > 0 scattered around. Will look into that.

[valentinewallace]

FYI, two more notes on this approach:

1. We may end up double-booking our own channel liquidity, since we don't know which of our channels is being used inside the blinded path (double-booking liquidity is a general problem for blinded pathfinding anyway, though)
2. ISTM we'll always want to go through the process of selecting additional paths, because the blinded path where we are the intro node may be the most expensive option

For (1), we could decrement each of our available channel balances by the amount used on the path. Not sure it's if worth the additional complexity.

[valentinewallace]

I'm gonna address this in a follow-up since this PR is growing. IMO our offers code should still advance blinded paths before pathfinding to them, though adding the behavior you describe is good for keeping find_route general-purpose

[valentinewallace]

Rebased to fix CI, also squashed.

[alecchendev]

More of a comprehension question - if a recipient includes a dummy hop, we'll send extra sats for this dummy hop's fees that will go to the recipient right? Is there anything stopping a recipient from always making a sender slightly overpay as long as there's a route with enough liquidity?

[valentinewallace]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments, based on the non-dummy hops' feerates. I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

[alecchendev]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments

Oh I see, thanks

I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

I realize reading more of the proposal that recipients are even encouraged to in order to avoid probing, although I also saw this section where the recipient pays the blinded fees which was interesting. But yea, seems like nothing's really stopping a recipient from getting the sender to overpay as long as its not outrageous...🤷

[valentinewallace]

Squashed.

[jkczyz]

Largely looks good though I need to do a more detailed pass at the last commit still.

[TheBlueMatt]

LGTM, I think, feel free to squash fixups.

[TheBlueMatt]

One small thing worth fixing in a followup, though I'm not even sure the code is actually reachable.

[valentinewallace]

Support finding a route to a recipient who is behind blinded payment paths, which are provided in BOLT12 invoices.

• finish tests

Based on #2258, #2305.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.61756% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (d78dd48) to head (6c3ca55).
⚠️ Report is 5192 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/router.rs	94.61%	18 Missing ⚠️
lightning/src/offers/invoice.rs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2120 +/- ##
==========================================
+ Coverage 90.48% 91.30% +0.81% 
==========================================
Files 104 106 +2 Lines 53920 63000 +9080 Branches 53920 63000 +9080 ==========================================
+ Hits 48792 57525 +8733 - Misses 5128 5475 +347 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We should definitely land at least the new serialization backwards-incompatible hints tlvs for the next release.

[TheBlueMatt]

Should this get some kind of TODO? Or should we just always use this path directly?

[valentinewallace]

We can't use these paths atm because get_route doesn't have the ability to "advance" the blinded path to the next hop (and we can't pathfind to ourselves, ofc). Previously discussed here: #2146 (comment)

[TheBlueMatt]

Ah, I misunderstood that comment, so I think there's another way to handle this - let get_route return early with a 0-hop unblinded path portion and the blinded tail as-is. Then the paying code would handle it by detecting this case and doing the advancing itself.

[valentinewallace]

Hmm, I don't see how that would work if the max_htlc of the 1 blinded hint isn't sufficient for the entire payment?

[TheBlueMatt]

Mmm right, I suppose we could pre-select it as a path and then run the router to select more paths if needed? That seems like it would work pretty easy.

[valentinewallace]

Yeah that sounds good! We might want to have another prefactor, though, because there's still assumptions that path.hops.len() > 0 scattered around. Will look into that.

[valentinewallace]

FYI, two more notes on this approach:

1. We may end up double-booking our own channel liquidity, since we don't know which of our channels is being used inside the blinded path (double-booking liquidity is a general problem for blinded pathfinding anyway, though)
2. ISTM we'll always want to go through the process of selecting additional paths, because the blinded path where we are the intro node may be the most expensive option

For (1), we could decrement each of our available channel balances by the amount used on the path. Not sure it's if worth the additional complexity.

[valentinewallace]

I'm gonna address this in a follow-up since this PR is growing. IMO our offers code should still advance blinded paths before pathfinding to them, though adding the behavior you describe is good for keeping find_route general-purpose

[valentinewallace]

Rebased to fix CI, also squashed.

[alecchendev]

More of a comprehension question - if a recipient includes a dummy hop, we'll send extra sats for this dummy hop's fees that will go to the recipient right? Is there anything stopping a recipient from always making a sender slightly overpay as long as there's a route with enough liquidity?

[valentinewallace]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments, based on the non-dummy hops' feerates. I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

[alecchendev]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments

Oh I see, thanks

I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

I realize reading more of the proposal that recipients are even encouraged to in order to avoid probing, although I also saw this section where the recipient pays the blinded fees which was interesting. But yea, seems like nothing's really stopping a recipient from getting the sender to overpay as long as its not outrageous...🤷

[valentinewallace]

Squashed.

[jkczyz]

Largely looks good though I need to do a more detailed pass at the last commit still.

[TheBlueMatt]

LGTM, I think, feel free to squash fixups.

[TheBlueMatt]

One small thing worth fixing in a followup, though I'm not even sure the code is actually reachable.

[valentinewallace]

Support finding a route to a recipient who is behind blinded payment paths, which are provided in BOLT12 invoices.

• finish tests

Based on #2258, #2305.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.61756% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (d78dd48) to head (6c3ca55).
⚠️ Report is 5192 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/routing/router.rs	94.61%	18 Missing ⚠️
lightning/src/offers/invoice.rs	94.11%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #2120 +/- ##
==========================================
+ Coverage 90.48% 91.30% +0.81% 
==========================================
Files 104 106 +2 Lines 53920 63000 +9080 Branches 53920 63000 +9080 ==========================================
+ Hits 48792 57525 +8733 - Misses 5128 5475 +347 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheBlueMatt]

We should definitely land at least the new serialization backwards-incompatible hints tlvs for the next release.

[TheBlueMatt]

Should this get some kind of TODO? Or should we just always use this path directly?

[valentinewallace]

We can't use these paths atm because get_route doesn't have the ability to "advance" the blinded path to the next hop (and we can't pathfind to ourselves, ofc). Previously discussed here: #2146 (comment)

[TheBlueMatt]

Ah, I misunderstood that comment, so I think there's another way to handle this - let get_route return early with a 0-hop unblinded path portion and the blinded tail as-is. Then the paying code would handle it by detecting this case and doing the advancing itself.

[valentinewallace]

Hmm, I don't see how that would work if the max_htlc of the 1 blinded hint isn't sufficient for the entire payment?

[TheBlueMatt]

Mmm right, I suppose we could pre-select it as a path and then run the router to select more paths if needed? That seems like it would work pretty easy.

[valentinewallace]

Yeah that sounds good! We might want to have another prefactor, though, because there's still assumptions that path.hops.len() > 0 scattered around. Will look into that.

[valentinewallace]

FYI, two more notes on this approach:

1. We may end up double-booking our own channel liquidity, since we don't know which of our channels is being used inside the blinded path (double-booking liquidity is a general problem for blinded pathfinding anyway, though)
2. ISTM we'll always want to go through the process of selecting additional paths, because the blinded path where we are the intro node may be the most expensive option

For (1), we could decrement each of our available channel balances by the amount used on the path. Not sure it's if worth the additional complexity.

[valentinewallace]

I'm gonna address this in a follow-up since this PR is growing. IMO our offers code should still advance blinded paths before pathfinding to them, though adding the behavior you describe is good for keeping find_route general-purpose

[valentinewallace]

Rebased to fix CI, also squashed.

[alecchendev]

More of a comprehension question - if a recipient includes a dummy hop, we'll send extra sats for this dummy hop's fees that will go to the recipient right? Is there anything stopping a recipient from always making a sender slightly overpay as long as there's a route with enough liquidity?

[valentinewallace]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments, based on the non-dummy hops' feerates. I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

[alecchendev]

Dummy hops don't cost extra fees, since agregated fees for use of the entire blinded path are calculated by the recipient per https://github.com/lightning/bolts/blob/master/proposals/route-blinding.md#blinded-payments

Oh I see, thanks

I think the recipient could make the sender overpay, though, if they just added extra fees on top of the aggregated fees?

I realize reading more of the proposal that recipients are even encouraged to in order to avoid probing, although I also saw this section where the recipient pays the blinded fees which was interesting. But yea, seems like nothing's really stopping a recipient from getting the sender to overpay as long as its not outrageous...🤷

[valentinewallace]

Squashed.

[jkczyz]

Largely looks good though I need to do a more detailed pass at the last commit still.

[TheBlueMatt]

LGTM, I think, feel free to squash fixups.

[TheBlueMatt]

One small thing worth fixing in a followup, though I'm not even sure the code is actually reachable.