-
Notifications
You must be signed in to change notification settings - Fork 2.2k
switchrpc: add idempotent external HTLC dispatch via SendOnion #10473
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
base: elle-base-branch-payment-service
Are you sure you want to change the base?
Changes from all commits
2fbe283
d256721
131981a
f33cd8d
df0c8b6
23dbe7f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| package itest | ||
|
|
||
| import ( | ||
| "context" | ||
| "sync" | ||
|
|
||
| "github.com/btcsuite/btcd/btcec/v2" | ||
| "github.com/btcsuite/btcd/btcutil" | ||
| sphinx "github.com/lightningnetwork/lightning-onion" | ||
|
|
@@ -9,9 +12,12 @@ import ( | |
| "github.com/lightningnetwork/lnd/lnrpc/invoicesrpc" | ||
| "github.com/lightningnetwork/lnd/lnrpc/switchrpc" | ||
| "github.com/lightningnetwork/lnd/lntest" | ||
| "github.com/lightningnetwork/lnd/lntest/rpc" | ||
| "github.com/lightningnetwork/lnd/lntypes" | ||
| "github.com/lightningnetwork/lnd/lnwire" | ||
| "github.com/stretchr/testify/require" | ||
| "google.golang.org/grpc/codes" | ||
| "google.golang.org/grpc/status" | ||
| ) | ||
|
|
||
| // testSendOnion tests the basic success case for the SendOnion RPC. It | ||
|
|
@@ -178,13 +184,18 @@ func testSendOnionTwice(ht *lntest.HarnessTest) { | |
| // While the first onion is still in-flight, we'll send the same onion | ||
| // again with the same attempt ID. This should error as our Switch will | ||
| // detect duplicate ADDs for *in-flight* HTLCs. | ||
| resp = alice.RPC.SendOnion(sendReq) | ||
| ht.Logf("SendOnion resp: %+v, code: %v", resp, resp.ErrorCode) | ||
| require.False(ht, resp.Success, "expected failure on onion send") | ||
| require.Equal(ht, resp.ErrorCode, | ||
| switchrpc.ErrorCode_DUPLICATE_HTLC, | ||
| ctxt, cancel := context.WithTimeout(context.Background(), | ||
| rpc.DefaultTimeout) | ||
| defer cancel() | ||
|
|
||
| _, err := alice.RPC.Switch.SendOnion(ctxt, sendReq) | ||
| require.Error(ht, err, "expected failure on onion send") | ||
|
|
||
| // Check that we get the expected gRPC error. | ||
| s, ok := status.FromError(err) | ||
| require.True(ht, ok, "expected gRPC status error") | ||
| require.Equal(ht, codes.AlreadyExists, s.Code(), | ||
| "unexpected error code") | ||
| require.Equal(ht, resp.ErrorMessage, htlcswitch.ErrDuplicateAdd.Error()) | ||
|
|
||
| // Dave settles the invoice. | ||
| dave.RPC.SettleInvoice(preimage[:]) | ||
|
|
@@ -203,14 +214,140 @@ func testSendOnionTwice(ht *lntest.HarnessTest) { | |
| require.Equal(ht, preimage[:], trackResp.Preimage) | ||
|
|
||
| // Now that the original HTLC attempt has settled, we'll send the same | ||
| // onion again with the same attempt ID. | ||
| // | ||
| // NOTE: Currently, this does not error. When we make SendOnion fully | ||
| // duplicate safe, this should be updated to assert an error is | ||
| // returned. | ||
| resp = alice.RPC.SendOnion(sendReq) | ||
| require.True(ht, resp.Success, "expected successful onion send") | ||
| require.Empty(ht, resp.ErrorMessage, "unexpected failure to send onion") | ||
| // onion again with the same attempt ID. Confirm that this is also | ||
| // prevented. | ||
| ctxt, cancel = context.WithTimeout(context.Background(), | ||
| rpc.DefaultTimeout) | ||
| defer cancel() | ||
|
|
||
| _, err = alice.RPC.Switch.SendOnion(ctxt, sendReq) | ||
| require.Error(ht, err, "expected failure on onion send") | ||
|
|
||
| // Check that we get the expected gRPC error. | ||
| s, ok = status.FromError(err) | ||
| require.True(ht, ok, "expected gRPC status error") | ||
| require.Equal(ht, codes.AlreadyExists, s.Code(), | ||
| "unexpected error code") | ||
| } | ||
|
|
||
| // testSendOnionConcurrency simulates a client that crashes and attempts to | ||
| // retry a payment with the same attempt ID concurrently. This test provides a | ||
| // strong guarantee that the SendOnion RPC is idempotent and correctly prevents | ||
| // duplicate payment attempts from succeeding. | ||
| func testSendOnionConcurrency(ht *lntest.HarnessTest) { | ||
| // Create a two-node context consisting of Alice and Bob. | ||
| const chanAmt = btcutil.Amount(100000) | ||
| const numNodes = 2 | ||
| nodeCfgs := make([][]string, numNodes) | ||
| chanPoints, nodes := ht.CreateSimpleNetwork( | ||
| nodeCfgs, lntest.OpenChannelParams{Amt: chanAmt}, | ||
| ) | ||
| alice, bob := nodes[0], nodes[1] | ||
| defer ht.CloseChannel(alice, chanPoints[0]) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is already done by the cleanup harness |
||
|
|
||
| // Make sure Alice knows about the channel. | ||
| aliceBobChan := ht.AssertChannelInGraph(alice, chanPoints[0]) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be part of the simipleNetwork setup or ? |
||
|
|
||
| const paymentAmt = 10000 | ||
|
|
||
| // Request an invoice from Bob so he is expecting payment. | ||
| _, rHashes, invoices := ht.CreatePayReqs(bob, paymentAmt, 1) | ||
| paymentHash := rHashes[0] | ||
|
|
||
| // Query for a route to pay from Alice to Bob. | ||
| routesReq := &lnrpc.QueryRoutesRequest{ | ||
| PubKey: bob.PubKeyStr, | ||
| Amt: paymentAmt, | ||
| } | ||
| routes := alice.RPC.QueryRoutes(routesReq) | ||
| route := routes.Routes[0] | ||
| finalHop := route.Hops[len(route.Hops)-1] | ||
| finalHop.MppRecord = &lnrpc.MPPRecord{ | ||
| PaymentAddr: invoices[0].PaymentAddr, | ||
| TotalAmtMsat: int64(lnwire.NewMSatFromSatoshis(paymentAmt)), | ||
| } | ||
|
|
||
| // Construct the onion for the route. | ||
| onionReq := &switchrpc.BuildOnionRequest{ | ||
| Route: route, | ||
| PaymentHash: paymentHash, | ||
| } | ||
| onionResp := alice.RPC.BuildOnion(onionReq) | ||
|
|
||
| // Create the SendOnion request that all goroutines will use. | ||
| // The AttemptId MUST be the same for all calls. | ||
| sendReq := &switchrpc.SendOnionRequest{ | ||
| FirstHopChanId: aliceBobChan.ChannelId, | ||
| Amount: route.TotalAmtMsat, | ||
| Timelock: route.TotalTimeLock, | ||
| PaymentHash: paymentHash, | ||
| OnionBlob: onionResp.OnionBlob, | ||
| AttemptId: 42, | ||
| } | ||
|
|
||
| const numConcurrentRequests = 50 | ||
| var wg sync.WaitGroup | ||
| wg.Add(numConcurrentRequests) | ||
|
|
||
| // Use channels to collect the results from each goroutine. | ||
| resultsChan := make(chan error, | ||
| numConcurrentRequests) | ||
|
|
||
| // Launch all requests concurrently to simulate a retry storm. | ||
| for i := 0; i < numConcurrentRequests; i++ { | ||
| go func() { | ||
| defer wg.Done() | ||
| ctxt, cancel := context.WithTimeout( | ||
| context.Background(), | ||
| rpc.DefaultTimeout, | ||
| ) | ||
| defer cancel() | ||
|
|
||
| _, err := alice.RPC.Switch.SendOnion(ctxt, sendReq) | ||
| resultsChan <- err | ||
| }() | ||
| } | ||
|
|
||
| wg.Wait() | ||
| close(resultsChan) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whats the meaning of closing this channel ? |
||
|
|
||
| // We expect exactly one successful dispatch and the rest to be | ||
| // rejected as duplicates. | ||
| successCount := 0 | ||
| duplicateCount := 0 | ||
|
|
||
| for err := range resultsChan { | ||
| // A nil error indicates a successful dispatch. | ||
| if err == nil { | ||
| successCount++ | ||
| continue | ||
| } | ||
|
|
||
| // For non-nil errors, we should receive a gRPC status error. | ||
| s, ok := status.FromError(err) | ||
| if !ok { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. require:False() ? |
||
| // If it's not a gRPC status error, it's an unexpected | ||
| // condition. | ||
| ht.Fatalf("unexpected error from SendOnion: %v, "+ | ||
| "code: %v", s.Err().Error(), s.Code()) | ||
| } | ||
|
|
||
| // Check if the error code indicates a duplicate acknowledgment. | ||
| if s.Code() == codes.AlreadyExists { | ||
| duplicateCount++ | ||
| } else { | ||
| ht.Fatalf("unexpected error from SendOnion: %v, "+ | ||
| "code: %v", s.Err().Error(), s.Code()) | ||
| } | ||
| } | ||
|
|
||
| // Confirm that only a single dispatch succeeds. | ||
| require.Equal(ht, 1, successCount, "expected exactly one success") | ||
| require.Equal(ht, numConcurrentRequests-1, duplicateCount, | ||
| "expected all other attempts to be duplicates") | ||
|
|
||
| // The invoice should eventually show as settled for Bob. | ||
| ht.AssertInvoiceSettled(bob, invoices[0].PaymentAddr) | ||
| } | ||
|
|
||
| // testTrackOnion exercises the SwitchRPC server's TrackOnion endpoint, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,9 +10,34 @@ option go_package = "github.com/lightningnetwork/lnd/lnrpc/switchrpc"; | |
| // subsystem of the daemon. | ||
| service Switch { | ||
| /* | ||
| SendOnion attempts to make a payment via the specified onion. This | ||
| method differs from SendPayment in that the instance need not be aware of | ||
| the full details of the payment route. | ||
| SendOnion provides an idempotent API for dispatching a pre-formed onion | ||
| packet, which is the primary entry point for a remote router. | ||
|
|
||
| To safely handle network failures, a client can and should retry this RPC | ||
| after a timeout or disconnection. Retries MUST use the exact same | ||
| attempt_id to allow the server to correctly detect duplicate requests. | ||
|
|
||
| A client interacting with this RPC must handle four distinct categories of | ||
| outcomes, communicated via gRPC status codes: | ||
|
|
||
| 1. SUCCESS (gRPC code OK): A definitive confirmation that the HTLC has | ||
| been successfully dispatched. The client can proceed to track the | ||
| payment's final result via the `TrackOnion` RPC. | ||
|
|
||
| 2. DUPLICATE ACKNOWLEDGMENT (gRPC code AlreadyExists): A definitive | ||
bitromortac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| acknowledgment that a request with the same attempt_id has already | ||
| been successfully processed. A retrying client should interpret this | ||
| as a success and proceed to tracking the payment's result. | ||
|
|
||
| 3. AMBIGUOUS FAILURE (gRPC code Unavailable or DeadlineExceeded): An | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps we should add that context cancelled is also such an error and that this should be the fallback if an error can't be classified in terms of the other cases (we could make this the last point), since it's always safe to retry this RPC.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would almost only keep the documentation at one place, maybe here on the RPC level to avoid that they diverge ? |
||
| ambiguous error occurred (e.g., the server is shutting down or the | ||
| client timed out). The state of the HTLC dispatch is unknown. The | ||
| client MUST retry the exact same request to resolve the ambiguity. | ||
|
|
||
| 4. DEFINITIVE FAILURE (gRPC code FailedPrecondition, InvalidArgument, etc.): | ||
| A definitive failure is a guarantee that the HTLC was not and will not be | ||
| dispatched. The client should fail the attempt and may retry with a new | ||
| route and/or new attempt_id. | ||
| */ | ||
| rpc SendOnion (SendOnionRequest) returns (SendOnionResponse); | ||
|
|
||
|
|
||
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.
gofmt: all arguments in the new line