lnd icon indicating copy to clipboard operation
lnd copied to clipboard

[3/4] Route Blinding: send MPP over multiple blinded paths

Open ellemouton opened this issue 1 year ago • 1 comments

In this PR, we remove the restriction of only being able to send to a single blinded payment path. Nodes will now be able to take advantage of the multiple blinded paths provided for a given payment to perform MPP if needed.

Depends on: https://github.com/lightningnetwork/lnd/pull/8735 Tracking Issue: https://github.com/lightningnetwork/lnd/issues/6690

ellemouton avatar May 16 '24 08:05 ellemouton

[!IMPORTANT]

Review skipped

Auto reviews are limited to specific labels.

Labels to auto review (1)
  • llm-review

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.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

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

CodeRabbit Configuration 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/schema.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 May 16 '24 08:05 coderabbitai[bot]

Finally, there is one more assumption we make here: if a recipient provides us with a set of paths of various lengths but one of those paths is an intro-node-only path, then we just go ahead and discard the rest of the paths since it doesnt make sense to try those since we know that the intro node is the dest node.

I wonder if this would be considered a wrong behaviour of the person providing us the invoice, so maybe we should consider this as an invalid invoice to spot wrong implementations ?

ziggie1984 avatar Jul 21 '24 17:07 ziggie1984

@ellemouton, remember to re-request review from reviewers when ready

lightninglabs-deploy avatar Jul 29 '24 12:07 lightninglabs-deploy

I went to create an invoice with a blinded path on testnet, and ran into this panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x143a82b]

goroutine 4084 [running]:
github.com/lightningnetwork/lnd/routing.(*ChannelRouter).FindBlindedPaths(0xc0022b6f00, {0x2, 0x70, 0x68, 0x5c, 0xa8, 0x1a, 0x8e, 0x4d, 0x4d, ...}, ...)
	github.com/lightningnetwork/[email protected]/routing/router.go:727 +0x64b
github.com/lightningnetwork/lnd.(*rpcServer).AddInvoice.func3(0x20?)
	github.com/lightningnetwork/[email protected]/rpcserver.go:5816 +0x97
github.com/lightningnetwork/lnd/lnrpc/invoicesrpc.buildBlindedPaymentPaths(0xc0070713e0)
	github.com/lightningnetwork/[email protected]/lnrpc/invoicesrpc/addinvoice.go:1054 +0x42
github.com/lightningnetwork/lnd/lnrpc/invoicesrpc.AddInvoice({0x36f9090, 0xc003fc4450}, 0xc004d95080, 0xc0070715c8)
	github.com/lightningnetwork/[email protected]/lnrpc/invoicesrpc/addinvoice.go:517 +0x108e
github.com/lightningnetwork/lnd.(*rpcServer).AddInvoice(0xc00061a600, {0x36f9090, 0xc003fc4450}, 0xc001487980)
	github.com/lightningnetwork/[email protected]/rpcserver.go:5856 +0x6a5
github.com/lightningnetwork/lnd/lnrpc._Lightning_AddInvoice_Handler.func1({0x36f9090?, 0xc003fc4450?}, {0x22965c0?, 0xc001487980?})
	github.com/lightningnetwork/[email protected]/lnrpc/lightning_grpc.pb.go:2610 +0xcb
github.com/lightningnetwork/lnd/rpcperms.(*InterceptorChain).CreateServerOpts.(*InterceptorChain).middlewareUnaryServerInterceptor.func7({0x36f9090, 0xc003fc4450}, {0x22965c0, 0xc001487980}, 0xc00391a660, 0xc002205ec0)
	github.com/lightningnetwork/[email protected]/rpcperms/interceptor.go:832 +0x111
google.golang.org/grpc.getChainUnaryHandler.func1({0x36f9090, 0xc003fc4450}, {0x22965c0, 0xc001487980})
	google.golang.org/[email protected]/server.go:1163 +0xb2
github.com/lightningnetwork/lnd/rpcperms.(*InterceptorChain).CreateServerOpts.(*InterceptorChain).MacaroonUnaryServerInterceptor.func5({0x36f9090, 0xc003fc4450}, {0x22965c0, 0xc001487980}, 0xc00391a660?, 0xc003f8fec0)
	github.com/lightningnetwork/[email protected]/rpcperms/interceptor.go:689 +0x7d
google.golang.org/grpc.getChainUnaryHandler.func1({0x36f9090, 0xc003fc4450}, {0x22965c0, 0xc001487980})
	google.golang.org/[email protected]/server.go:1163 +0xb2
github.com/lightningnetwork/lnd/rpcperms.(*InterceptorChain).CreateServerOpts.(*InterceptorChain).rpcStateUnaryServerInterceptor.func3({0x36f9090, 0xc003fc4450}, {0x22965c0, 0xc001487980}, 0xc00391a660, 0xc002b80900)
	github.com/lightningnetwork/[email protected]/rpcperms/interceptor.go:781 +0xfc
google.golang.org/grpc.getChainUnaryHandler.func1({0x36f9090, 0xc003fc4450}, {0x22965c0, 0xc001487980})
	google.golang.org/[email protected]/server.go:1163 +0xb2
github.com/lightningnetwork/lnd/rpcperms.(*InterceptorChain).CreateServerOpts.errorLogUnaryServerInterceptor.func1({0x36f9090?, 0xc003fc4450?}, {0x22965c0?, 0xc001487980?}, 0xc00391a660, 0xc002205ec0?)
	github.com/lightningnetwork/[email protected]/rpcperms/interceptor.go:605 +0x52
google.golang.org/grpc.NewServer.chainUnaryServerInterceptors.chainUnaryInterceptors.func1({0x36f9090, 0xc003fc4450}, {0x22965c0, 0xc001487980}, 0xc00391a660, 0x78?)
	google.golang.org/[email protected]/server.go:1154 +0x85
github.com/lightningnetwork/lnd/lnrpc._Lightning_AddInvoice_Handler({0x22d29c0, 0xc00061a600}, {0x36f9090, 0xc003fc4450}, 0xc004b3b280, 0xc0007747a0)
	github.com/lightningnetwork/[email protected]/lnrpc/lightning_grpc.pb.go:2612 +0x143
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0002661e0, {0x36f9090, 0xc003ed0f30}, {0x3708e60, 0xc004871a00}, 0xc002440240, 0xc000769770, 0x4a551d8, 0x0)
	google.golang.org/[email protected]/server.go:1343 +0xdd1
google.golang.org/grpc.(*Server).handleStream(0xc0002661e0, {0x3708e60, 0xc004871a00}, 0xc002440240)
	google.golang.org/[email protected]/server.go:1737 +0xc47
google.golang.org/grpc.(*Server).serveStreams.func1.1()
	google.golang.org/[email protected]/server.go:986 +0x86
created by google.golang.org/grpc.(*Server).serveStreams.func1 in goroutine 4083
	google.golang.org/[email protected]/server.go:997 +0x136

Perhaps it's the case of a missing edge policy: https://github.com/ellemouton/lnd/blob/53dab50d393f3acf80d846e5bdcbdad8ee4b6cb5/routing/router.go#L725-L728

Roasbeef avatar Jul 30 '24 01:07 Roasbeef

Can be rebased to point to master now!

Roasbeef avatar Jul 30 '24 02:07 Roasbeef

Hmm seems like we need to check that the inpolicy is not nil as we already do in other parts in the routing codebase ?

// If there is no edge policy for this candidate node, skip.
		// Note that we are searching backwards so this node would have
		// come prior to the pivot node in the route.
		if channel.InPolicy == nil {
			return nil
		}

ziggie1984 avatar Jul 30 '24 15:07 ziggie1984

@ziggie1984 yep was thinking the same thing, I'll add that then try again. FWIW, this is a very very very old node, which makes it great for testing!

Roasbeef avatar Jul 30 '24 18:07 Roasbeef

oops - I accidentally clicked re-request @ziggie1984 🙈 sorry about that

ellemouton avatar Jul 31 '24 08:07 ellemouton

Tried out the latest branch, and I was able to make an initial blinded paths payment!

Ran into some other issues along the way:

  • When trying to pay with default settings, I got INVALID_ONION_BLINDING each time.
  • That was with a node that may have been added within the blinded path as I had a direct channel with it.
  • I then tried to reduce max-num-paths to 1, initially thinking it woudl give me only 1 hop routes, but I understand now that means only have a single path.
  • After that I got some errors where it was unable to make a path at all:
     tlncli addinvoice --amt=100 --blind
    [lncli] rpc error: code = Unknown desc = could not collect blinded path relay info: no channel updates found from node 
    022fb1230aefbd135141997f03343f5410e6f1ab11d337af6df92cb67d5a856748 for channel 2681963946846388225
     tlncli addinvoice --amt=100 --blind
    [lncli] rpc error: code = Unknown desc = could not build any blinded paths
    
    • Perhaps we should have an inner retry loop here. I did the command a few more times until one of them finally worked.
  • I tried again with only a single blinded path ( max-num-paths=1):
    • The first time around, it was trying to find very long routes to the dest, and failing: TEMPORARY_CHANNEL_FAILURE @ 2nd hop | 34.580 | 36.342 | 100 | 2.707 | 2871717 | 3157069518315978752 | 030f375d8aecdddc8523->24hr Drive Thru->POSTECHCCBR->TuDuD->lnd-test.24h.to->SOMBERMASTER->039c25->025eac
    • I tried again, with the path succeeding with a route of similar length:
    | SUCCEEDED                           |        1.229 |        6.271 | 100          | 3.106 |  2871679 | 3157069
    518315978752 | 030f375d8aecdddc8523->aranguren.org->OCEAN_TESTNET_WK1->03fef5->022060 |
    +-------------------------------------+--------------+--------------+--------------+-------+----------+------------ 
    ---------+------------------------------------------------------------------------+
    Amount + fee:   100 + 3.106 sat
    Payment hash:   ab44ac84be151fde822a7dda35a7acf908d431ad719850ddfddccdb05220d38e
    Payment status: SUCCEEDED, preimage: 
    b6243fdf1869bae1c962c5b528b8ba563845bad51320bebd921635cbb2044ed2
    

Roasbeef avatar Aug 01 '24 02:08 Roasbeef

I think the instances of INVALID_ONION_BLINDING might be older nodes on testnet that aren't running the latest version of blinded paths.

Some other featrure requests after trying things out:

  • Command to unblind blinded paths as the creator. Can be useful for debugging.
  • All dynamically setting values like num paths and path length via lncli addinvoice. During testing I had to restart each time I wanted to mess with a different value.
  • Add ability to omit a node from the blinded path. Would've been useful to pinpoint if it's just certain nodes sending INVALID_ONION_BLINDING errors.

Roasbeef avatar Aug 01 '24 02:08 Roasbeef