-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
htlcswitch: add inbound routing fees receive support #6703
Conversation
Added a scenario that underlines the advantage of inbound fees on the spec pr: lightning/bolts#835 (comment) Lightning Labs, what is your view on this feature? |
Created BLIP for the gossip change: lightning/blips#18 |
6a14f1e
to
776391b
Compare
For the The problem though is the size limit of 256 bytes on the failure message. A 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. |
de3de69
to
e0d6f3f
Compare
31769d8
to
a9f1b24
Compare
Rebased |
a9f1b24
to
9af961c
Compare
9af961c
to
3c52653
Compare
Important Auto Review SkippedAuto 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 To trigger a single review, invoke the 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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configration File (
|
Rebased |
af237f8
to
3888b93
Compare
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ☀️
) | ||
|
||
// Fee represents a fee schedule. | ||
type Fee struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking given the age/lineage of this PR, but I think we should go ahead and add it to the existing ChannelUpdate
struct using the new optional TLV records.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will make a follow up issue to track this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
In this commit, the tlv extension of a channel update message is parsed. If an inbound fee schedule is encountered, it is reported in the graph rpc calls.
5348160
to
9f80df8
Compare
Ensure that negative fees are backwards compatible.
9f80df8
to
ba21ca7
Compare
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 lightning/bolts#835 and blip lightning/blips#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:
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 htlcswitch: relax failure message length check #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 htlcswitch: return inbound channel update #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 #4225