lnd icon indicating copy to clipboard operation
lnd copied to clipboard

[2/3]: Support Forwarding of Blinded Payments

Open carlaKC opened this issue 2 years ago • 3 comments

This PR is the second in a series to add the ability for LND to forward payments to blinded routes.

⚠️ Lots of PR dependencies (do not fear the diff size!!) ⚠️

  • Depends on: #8159
  • Depends on: #8142

👀 First commit in this PR: hop: add function for calculating forwarding amount

Reviewer Notes:

  • This PR does not signal the route blinding feature yet because it does not include error handling for blinded forwards, which is critical to receiver privacy so LND should not advertise the feature until it fully support it.
  • Itests can be squashed, they're broken into multiple commits to aid readability.

Replaces #7376 Fixes #7298

carlaKC avatar Nov 08 '23 19:11 carlaKC

Will this be the last in the series to support forwarding?

Roasbeef avatar Jan 18 '24 16:01 Roasbeef

Will this be the last in the series to support forwarding?

One more (WIP) to add and test error handling - it's quite delicate so I want to separate it out.

carlaKC avatar Jan 18 '24 16:01 carlaKC

[!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

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.
  • Please see the configuration documentation for more information.
  • 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

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Jan 25 '24 14:01 coderabbitai[bot]

Depending on the answer to my question in the first PR, this one might need to be updated to account for next_blinding_override.

Done!

carlaKC avatar Feb 21 '24 16:02 carlaKC

Cross compilation error showing up on other PRs/ I can't reproduce locally so going to assume it's unrelated.

carlaKC avatar Mar 07 '24 22:03 carlaKC

Cross compilation fails with: no space left on device... Oops, probably need to clear out some of the build caches.

EDIT: Cleared out build caches.

guggero avatar Mar 08 '24 09:03 guggero

and then it doesnt know how to handle that due to 2 different htlc's coming in with the same hash

Not sure I'm following this. I can see that we're more at risk of malicious wormholing, but shouldn't we be able to handle that out of the box because we allow MPP payments with the same hash?

carlaKC avatar Mar 08 '24 16:03 carlaKC

Last push is just a rebase to clear conflicts!

carlaKC avatar Mar 08 '24 21:03 carlaKC

Added the ability to disable route blinding - works nicely in validation like @ellemouton suggested 🚀

carlaKC avatar Mar 15 '24 21:03 carlaKC

We'd probably want to merge this only after https://github.com/lightningnetwork/lnd/pull/8485 is final?

I think that we could go ahead and merge this before https://github.com/lightningnetwork/lnd/pull/8485 is finalized because the content of forwarding vs error handling is pretty distinct in the codebase (ie, there are minimal interactions). Could also turn route blinding off by default and merge, then flip it back on in the next PR if we wanted to be extra cautious.

carlaKC avatar Mar 18 '24 21:03 carlaKC

Added a469087932af89e7e38202ca3837789605eb4465 disable route blinding so that this can merge independently of #8485 (which will re-enable it + advertise feature).

carlaKC avatar Mar 22 '24 17:03 carlaKC

Rebase! Also re-confirmed interop testing here.

carlaKC avatar Mar 27 '24 13:03 carlaKC

Needs a rebase!

Roasbeef avatar Mar 28 '24 22:03 Roasbeef

Thanks for the review @Roasbeef, the suggested interface makes things much cleaner 🧹

Addressed some of your post-merge review of #8159 here (where relevant to the existing changes) and will follow with the rest either in #8485 or an independent followup.

carlaKC avatar Apr 02 '24 20:04 carlaKC