htlcswitch: add inbound routing fees receive support

[joostjager]

This PR implements "receive" support for inbound fees. In short this means that a routing node operator gets to set distinct fee schedules for the movement of funds on their incoming channels. This enables more fine-grained flow control, potentially leading to an increase in capital efficiency.

More discussion on this change can be found in the corresponding bolts issue https://github.com/lightning/bolts/issues/835 and blip https://github.com/lightning/blips/pull/18

Mailing list post that elaborates a bit on the upgrade scenario and how this can be a non-breaking change: https://lists.linuxfoundation.org/pipermail/lightning-dev/2022-July/003643.html

Note that no network-wide upgrade is required for the inbound fee schedule to propagate.

Changes:

• Ability to set an inbound base and proportional fee as part of the channel policy.
• Broadcasting of the inbound fee schedule to the wider network.
• Update the forwarding logic to take into account inbound fees. This does not break older senders as long as the inbound fee is negative.

For send support, see #6934.

Implementation details

• The channel_update extra opaque data is stored in the database without any validation. This means that readers of this data must handle the case where a tlv error is encountered during parsing of the stream.

  An alternative would be to do strict validation in ExtraOpaqueData.Decode when a message is received, but is more involved. It for example breaks the exisitng fuzz tests. Also, there may already be malformed tlv data in the database.

• When a fee_insufficient failure happens on an intermediate node, still only the outgoing channel update is returned. The 256-byte failure message isn't able to accommodate both the incoming and outgoing channel policies until https://github.com/lightningnetwork/lnd/pull/6913 is widely deployed. For now, this means that senders can only receive updates to the inbound policy via gossip.

  For code that shows a future extended fee_insufficient, see https://github.com/lightningnetwork/lnd/pull/6967.

• Forwards will be declined if the total of inbound and outbound fee is negative. Negative inbound fees can be set, but it must be made sure that it is sufficiently offset by a positive outbound fee.

• Inbound fees can be set on private channels, but note must be taken that those fees cannot be communicated in bolt11 hop hints. For a hinted route that spans more than a single hop and where any hop except for the final hop has an inbound fee set, the inbound fee will be ignored by the sender. For a negative inbound fee this means the sender will miss out on the discount. For a positive inbound fee, the payment will fail.

Fixes https://github.com/lightningnetwork/lnd/issues/4225

[joostjager]

Added a scenario that underlines the advantage of inbound fees on the spec pr: https://github.com/lightning/bolts/issues/835#issuecomment-1222376127

Lightning Labs, what is your view on this feature?

[joostjager]

Created BLIP for the gossip change: https://github.com/lightning/blips/pull/18

[joostjager]

For the fee_insufficient failure message, I think we'd want to return not just the outgoing channel update, but also the incoming channel update. That way the sender has up to date fees for the pair of channels and the next attempt should work.

The problem though is the size limit of 256 bytes on the failure message. A fee_insufficient message with two channel updates attached is 274+ bytes.

Maybe a probabilistic fix is to randomly return either the incoming or the outgoing channel update? And perhaps there is an upside to disincentivizing high frequency channel updates too. See also #6883.

[joostjager]

@JssDWt I've tried out your suggestion https://github.com/lightning/blips/pull/18#issuecomment-1240469519 in a tacked-on commit.

[JssDWt]

@JssDWt I've tried out your suggestion lightning/blips#18 (comment) in a tacked-on commit.

Ah I see, that's great. I actually didn't realize you could still split up the fee computation in the pathfinding when using my suggestion there. I thought you couldn't reliably compute the fee to a node anymore, because you'd need information about the previous edge to calculate the fee for the current edge. But clearly the way you've implemented shows that it is possible.

[joostjager]

Ah I see, that's great. I actually didn't realize you could still split up the fee computation in the pathfinding when using my suggestion there. I thought you couldn't reliably compute the fee to a node anymore, because you'd need information about the previous edge to calculate the fee for the current edge. But clearly the way you've implemented shows that it is possible.

Actually there was an error in the previous revision. For your suggestion, it is indeed required to carry over the amount for the next edge. Made that change.

[joostjager]

Isolated receiver-side support in a separate commit.

The sender-side support is a bit complicated by BuildRoute with its three passes. I've been experimenting with reducing the number of passes (#6920) and going one step further by re-routing buildroute to pathfinding (#6921).

[joostjager]

Removed send support commits and moved into separate pr #6934

[joostjager]

Added commit to convert to fees based on the outgoing htlc amount plus outbound fee. See https://github.com/lightning/blips/pull/18#issuecomment-1261410529

[joostjager]

I feel a change like this would benefit from an integration test case though, to bring confidence in the calculations end up at what we expect in a multi hop scenario.

I've modified the existing multi-hop payment test to include a routing node that charges a negative inbound fee. Pathfinding isn't aware of inbound fees yet and builds the route as before, but this does cover backwards compatibility of the change.

[joostjager]

Added commit to record inbound fee collected by exit hops in the forwarding log.

[joostjager]

Dropped exit hop inbound fees. I am not convinced that it is only good, see https://github.com/lightning/blips/pull/18#issuecomment-1284437820.

This also makes for a smaller delta, and requires no changes to store exit fees earned in the forwarding log.

Saved the exit fee code here: https://github.com/lightningnetwork/lnd/compare/master...bottlepay:lnd:inbound-fees-exit-hop

[lightninglabs-deploy]

@jssdwt: review reminder

[TheBlueMatt]

Before landing this, its probably worth reading the extensive conversation on the bLIP. Notably, this approach has several drawbacks that other approaches do not - namely it requires much broader upgrade across the network and is restricted to negative fees only.

Frankly, I'm kinda gobsmacked that y'all seem intent on going a substantially worse route for some pretty trivial lnd-specific code cleanliness concerns. It's just not how I've ever seen protocol design work and in this case leads to some pretty huge and entirely unnecessary drawbacks.

[halseth]

@saubyk Given that this is a pre-requisite for the send support, should we tag it for v0.17?

[saubyk]

@saubyk Given that this is a pre-requisite for the send support, should we tag it for v0.17?

Hi @halseth tagged it for 0.17, but unsure if we'll have enough review bandwidth to get to it. If not, it may go in a point release after 17

[TheBlueMatt]

We'd also agreed to table this until it gets closer in a previous spec meeting. That conversation should happen before something ships.

[joostjager]

Rebased PR on top of latest master

[lightninglabs-deploy]

@bitromortac: review reminder

[joostjager]

!lightninglabs-deploy mute

[joostjager]

Thanks for the thorough review @ziggie1984.

1. It seems to me that with this change we potentially allow a node to earn negative fees on a forward or did I not see the preemptive check where we avoid exactly this case?

Reply in https://github.com/lightningnetwork/lnd/pull/6703/commits/feac8f0fce812c3bf9248daad7f59a17f8a47ff3#r1286726939

2. When sending through a private channel (not receiving) will the peer be ever able to communicate his inbound fee schedule to us? Imo that is not possible with this design or?

Reply in https://github.com/lightningnetwork/lnd/pull/6703/commits/feac8f0fce812c3bf9248daad7f59a17f8a47ff3#r1286737092

3. Did we come to conclusion how we wanna propagate the channel_upate msg when a payment fails (talking about the insufficient fee error here)? Are we gonna send back the outgoing and incoming feeupdate as well, seems like we do have the support for bigger failure msgs added in lnd.16? But what will the other implementations do if they are part of the route?

It is indeed the plan to send back both in and out fee policy with the insufficient fee error. But we need to know if the sender supports the longer failure message. In #6967, senders signal this in the forward onion tlv, but it is up for discussion how to do this exactly. LND 0.16 supports it (#6913 - it will just ignore the tlv extension), but there may be older senders. Perhaps the older senders' share of the user base is small enough to accept that they will no longer be able to interpret the failure message (specifically the out policy). This would avoid the signaling all together.

Afaik, other implementations didn't have this 256 byte limitation in the first place.

[joostjager]

Will we enforce inbound fees also when we send funds to a direct peer or will we exclude this case ? Refering to this comment in your introduction description: "Update the exit hop logic to apply an inbound fee. This does not break older senders as long as the inbound fee is negative." I did not really see any change in the exit hop logic?

This is indeed outdated, removed from PR description.

[feelancer21]

Policy updates with a positive InboundFee should not be enabled by default, but only by explicitly setting an command line flag. As long as sender support in lnd is not available and other implementations have not caught up, this change is breaking for the network.

Furthermore, I believe that positive InboundFees for routing nodes are not necessary at all, since one could always adjust inbound and outbound fees in such a way that all inbound fees are non-positive, outbound fees are positive, and the routing costs for each channel combination remain the same. One would simply need to reduce all InboundFees by the largest amount and increase all outbound fees accordingly. This would not have a negative impact on the network but would even encourage other LN implementations to adopt the feature.

Whether positive InboundFees are necessary for pure receivers for liquidity management, I cannot judge.

Therefore, I believe that only negative InboundFees should be enabled by default in the standard. Positive fees should only be activated explicitly, with a prominent warning in the sample-lnd.conf to inform node operators of the consequences.

[joostjager]

Scenario 1: A charges 3000 PPM because the channel is depleted and B is malicious, sets -3000 PPM to generate failures for A->B. A would be blamed in current pathfinding (assume B has opened many channels to strong sinks). B can do this because payments can't settle via the channel and B is not at risk of losing on inbound fees. We would need to account for this scenario in pathfinding, so that's more a penalization issue.

I am assuming that B is advertising a discount and then when the htlc arrives, B returns fee insufficient to the sender? In this case, lnd penalizes the A-B link. Can't B already return this error in the current situation too, without a negative inbound fee, when B's balance is zero?

[bitromortac]

Scenario 1: A charges 3000 PPM because the channel is depleted and B is malicious, sets -3000 PPM to generate failures for A->B. A would be blamed in current pathfinding (assume B has opened many channels to strong sinks). B can do this because payments can't settle via the channel and B is not at risk of losing on inbound fees. We would need to account for this scenario in pathfinding, so that's more a penalization issue.

I am assuming that B is advertising a discount and then when the htlc arrives, B returns fee insufficient to the sender? In this case, lnd penalizes the A-B link. Can't B already return this error in the current situation too, without a negative inbound fee, when B's balance is zero?

Interesting, never thought about how insufficient fee errors are penalized, this seems to be similar. I was rather thinking of B attracting traffic to the A-B channel with a discount but A fails back with a temporary channel failure because they can't forward.

[joostjager]

Interesting, never thought about how insufficient fee errors are penalized, this seems to be similar. I was rather thinking of B attracting traffic to the A-B channel with a discount but A fails back with a temporary channel failure because they can't forward.

Ah got it, that's a different one. The temp chan fail is still going to hit B too, because it penalizes the A-B link. But yes, it seems the ability for B to set a negative fee allows them to attract more traffic. It is floored at zero though. B can set a enormous negative fee, but it won't make the link A-B more attractive compared to a zero total fee. Implemented here: https://github.com/lightningnetwork/lnd/pull/6934/commits/e142c8d60aa867c5795f8b29f376f6e797282ce7#r1028989648

[joostjager]

Rebased after move to models package in master.

[joostjager]

Rebased

[coderabbitai[bot]]

[!IMPORTANT]

Auto Review Skipped

Auto reviews are limited to the following labels: llm-review. Please add one of these labels to enable auto reviews.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[joostjager]

Rebased

[joostjager]

Rebased