switchrpc: improve TrackOnion error handling

docs/release-notes/release-notes-0.21.0.md

@@ -99,6 +99,17 @@
 
 ## RPC Updates
 
+* The `switchrpc.TrackOnion` RPC has been
+  [overhauled](https://github.com/lightningnetwork/lnd/pull/10472) to provide a
+  more robust and type-safe error handling mechanism. The `TrackOnionResponse`
+  message now uses a top-level `oneof` to enforce a compile-time guarantee that
+  a response contains either a `preimage` (for success) or structured
+  `FailureDetails` (for a payment failure). This replaces the previous
+  string-based error reporting. Application-level payment failures are now
+  clearly separated from RPC-level failures (e.g., attempt not found), which are
+  communicated via standard gRPC status codes. This is a **breaking change** for
+  any clients of the `TrackOnion` RPC.
+
 ## lncli Updates
 
 ## Breaking Changes

itest/lnd_sendonion_test.go

@@ -93,7 +93,7 @@ func testSendOnion(ht *lntest.HarnessTest) {
 		HopPubkeys:  onionResp.HopPubkeys,
 	}
 	trackResp := alice.RPC.TrackOnion(trackReq)
-	require.Equal(ht, invoices[0].RPreimage, trackResp.Preimage)
+	require.Equal(ht, invoices[0].RPreimage, trackResp.GetPreimage())
 
 	// The invoice should show as settled for Dave.
 	ht.AssertInvoiceSettled(dave, invoices[0].PaymentAddr)
@@ -200,7 +200,7 @@ func testSendOnionTwice(ht *lntest.HarnessTest) {
 		HopPubkeys:  onionResp.HopPubkeys,
 	}
 	trackResp := alice.RPC.TrackOnion(trackReq)
-	require.Equal(ht, preimage[:], trackResp.Preimage)
+	require.Equal(ht, preimage[:], trackResp.GetPreimage())
 
 	// Now that the original HTLC attempt has settled, we'll send the same
 	// onion again with the same attempt ID.
@@ -275,9 +275,6 @@ func testTrackOnion(ht *lntest.HarnessTest) {
 	require.True(ht, resp.Success, "expected successful onion send")
 	require.Empty(ht, resp.ErrorMessage, "unexpected failure to send onion")
 
-	serverErrorStr := ""
-	clientErrorStr := ""
-
 	// Track the payment providing all necessary information to delegate
 	// error decryption to the server. We expect this to fail as Dave is not
 	// expecting payment.
@@ -288,10 +285,11 @@ func testTrackOnion(ht *lntest.HarnessTest) {
 		HopPubkeys:  onionResp.HopPubkeys,
 	}
 	trackResp := alice.RPC.TrackOnion(trackReq)
-	require.NotEmpty(ht, trackResp.ErrorMessage,
-		"expected onion tracking error")
+	serverFailure := trackResp.GetFailureDetails()
+	require.NotNil(ht, serverFailure, "expected onion tracking error")
 
-	serverErrorStr = trackResp.ErrorMessage
+	serverFwdFailure := serverFailure.GetForwardingFailure()
+	require.NotNil(ht, serverFwdFailure, "expected forwarding failure")
 
 	// Now we'll track the same payment attempt, but we'll specify that
 	// we want to handle the error decryption ourselves client side.
@@ -300,16 +298,18 @@ func testTrackOnion(ht *lntest.HarnessTest) {
 		PaymentHash: paymentHash,
 	}
 	trackResp = alice.RPC.TrackOnion(trackReq)
-	require.NotNil(ht, trackResp.EncryptedError, "expected encrypted error")
+	clientFailure := trackResp.GetFailureDetails()
+	require.NotNil(ht, clientFailure, "expected client tracking error")
+
+	encryptedErrorBytes := clientFailure.GetEncryptedErrorData()
+	require.NotNil(ht, encryptedErrorBytes, "expected encrypted error")
 
 	// Decrypt and inspect the error from the TrackOnion RPC response.
 	sessionKey, _ := btcec.PrivKeyFromBytes(onionResp.SessionKey)
 	var pubKeys []*btcec.PublicKey
 	for _, keyBytes := range onionResp.HopPubkeys {
 		pubKey, err := btcec.ParsePubKey(keyBytes)
-		if err != nil {
-			ht.Fatalf("Failed to parse public key: %v", err)
-		}
+		require.NoError(ht, err, "Failed to parse public key")
 		pubKeys = append(pubKeys, pubKey)
 	}
 
@@ -323,14 +323,19 @@ func testTrackOnion(ht *lntest.HarnessTest) {
 	}
 
 	// Simulate an RPC client decrypting the onion error.
-	encryptedError := lnwire.OpaqueReason(trackResp.EncryptedError)
-	forwardingError, err := errorDecryptor.DecryptError(encryptedError)
-	require.Nil(ht, err, "unable to decrypt error")
-
-	clientErrorStr = forwardingError.Error()
+	encryptedError := lnwire.OpaqueReason(encryptedErrorBytes)
+	clientFwdErr, err := errorDecryptor.DecryptError(encryptedError)
+	require.NoError(ht, err, "unable to decrypt error")
+
+	// Finally, assert that the structured forwarding failure is the same
+	// whether it was decrypted on the server or on the client.
+	serverFwdErr, err := switchrpc.UnmarshallForwardingError(
+		serverFwdFailure,
+	)
+	require.NoError(ht, err, "unable to decode server forwarding failure")
 
-	serverFwdErr, err := switchrpc.ParseForwardingError(serverErrorStr)
-	require.Nil(ht, err, "expected to parse forwarding error from server")
-	require.Equal(ht, serverFwdErr.Error(), clientErrorStr, "expect error "+
-		"message to match whether handled by client or server")
+	require.Equal(ht, serverFwdFailure.FailureSourceIndex,
+		uint32(clientFwdErr.FailureSourceIdx), "source index mismatch")
+	require.Equal(ht, serverFwdErr.WireMessage(),
+		clientFwdErr.WireMessage(), "wire message mismatch")
 }