Handle self payments

[vladimirfomene]

This PR solves issue #2462. If we asked to pay an invoice that we generated ourselves. We generate PaymentSent and PaymentClaimable event and mark the payment as fulfilled in our set of outbound payments.

This PR is important because we realized users can easily screw up self payments in a custodial service when they implement it themselves. Read more here: https://lists.linuxfoundation.org/pipermail/lightning-dev/2023-June/003983.html

[TheBlueMatt]

I'd really like for this to be transparent from the ChannelManager users' perspective, rather than doing it in lightning-invoice. This means a bit more code (we'll have to detect it and handle it in find_route and have some special magic format for a Route) but would make it a bit more transparent to all our users and give us the potential to do it for BOLT12 as well (which won't use lightning-invoice's payer).

The other thing we need here is that list_recent_payments needs to return a set which includes the self-payment, which means we need to store information about it in outbound_payment for a minute or two.

[vladimirfomene]

Please, this is ready for another round of reviews.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 89.04110% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 89.93%. Comparing base (aa3dbe8) to head (3ca0328). Report is 32 commits behind head on main.

Files	Patch %	Lines
lightning/src/ln/outbound_payment.rs	83.33%	4 Missing and 2 partials :warning:
lightning/src/ln/channelmanager.rs	93.75%	4 Missing and 1 partial :warning:
lightning/src/ln/payment_tests.rs	83.33%	0 Missing and 5 partials :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2573      +/-   ##
==========================================
+ Coverage   89.15%   89.93%   +0.77%     
==========================================
  Files         117      117              
  Lines       94934    99484    +4550     
  Branches    94934    99484    +4550     
==========================================
+ Hits        84641    89468    +4827     
+ Misses       7811     7570     -241     
+ Partials     2482     2446      -36     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[vladimirfomene]

@TheBlueMatt sorry I have been busy into other repos. Will look at your feedback this week.

[vladimirfomene]

Hi @TheBlueMatt ! I decided to try a new approach which I think might be an improvement on the previous approach. Let me know what you think. I also had a question. Don't we want to allow self payment in the case of a rebalance for example. Will this implementation not break our rebalancing functionality if we have one?

[TheBlueMatt]

I decided to try a new approach which I think might be an improvement on the previous approach. Let me know what you think

Looks much cleaner, thanks! However, I realized I gave you bad advice at the outset here, we don't really need/want to have a special-case Route, we can just handle the whole thing in send_payment_internal and call it a day (basically move your current code to after we fetch the route, if it errors). I don't think we need to handle the find_route_and_send_payment case yet - its only used for BOLT12 and retries. Obviously retries aren't a thing here, and for BOLT12 if we have a blinded path we'll want to still send an HTLC, so its only the current one-hop blinded path that we'll need to handle there, and that can come later.

Don't we want to allow self payment in the case of a rebalance for example. Will this implementation not break our rebalancing functionality if we have one?

Yes, we should allow users to pay with a route if they want. We currently have no problem handling this (AFAIK, at least it works with the manual path sending flow), so we'll want to maintain this. If a user returns a route from the router which goes back to them, we should use that, we should only shortcut the self-payment if we get an error, I think.

[vladimirfomene]

@TheBlueMatt, thanks for the direction. I have updated as recommended.

[vladimirfomene]

@valentinewallace @TheBlueMatt find in bd3f20cafc934bcbfe46daba13bdfeed28cfd1f1 the new approach. Let me know what you think. If this is good by you, I will improve the test.

[vladimirfomene]

@TheBlueMatt @valentinewallace I have updated the code following your last comments. Let me know if everything looks good now so that I can go ahead and implement test for this functionality.

[TheBlueMatt]

Looks like CI is failing to build this.

[vladimirfomene]

Looks like CI is failing to build this.

Working on fixing it asap

[vladimirfomene]

@TheBlueMatt thanks for the feedback. I really appreciate. Allow me to enhance based on your recommendation.

[coderabbitai[bot]]

Walkthrough

The recent update introduces the capability to manage self payments within the Lightning network. It encompasses the addition of structures to track and handle claimable self payments, alongside modifications in the payment process to support this feature. The changes span across several files, enhancing the ChannelManager for better self payment management, updating routing and payment logic, and ensuring comprehensive testing for the new functionality.

Changes

Files	Change Summary
.../channelmanager.rs, .../outbound_payment.rs, .../payment_tests.rs	Introduced structures and logic for handling self claimable payments, updated routing and payment processing to support self payments, and added tests for the new self payment functionality.
.../channelmanager.rs	Updated serialization and deserialization to include new self payment handling details.

🐇✨
In the world of lightning, a new feature blooms,
Self payments glide, in digital rooms.
Across the network, with speed they dash,
Handled with care, in a lightning flash.
🌩️💼 For every rabbit, a chance to claim,
Their own payments, in their own name.
A hop, a skip, in code we trust,
Bringing self payments, to the forefront, we must.
✨🐇

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-tests 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 tests 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 tests.
  • @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.

[vladimirfomene]

Hi @TheBlueMatt ! Will love to get your feedback on the new code.

[vladimirfomene]

Thanks @TheBlueMatt for the review. I will amend asap.

[vladimirfomene]

Hi @TheBlueMatt, please have a look at the updated implementation when you have some bandwidth.

[TheBlueMatt]

Hey @vladimirfomene any desire to keep working on this?