Merge pull request #10064 from ellemouton/getChanInfoGrpcErr

rpcserver: return consistent grpc NotFound error for GetChaninfo & LookupInvoice

docs/release-notes/release-notes-0.20.0.md

@@ -115,6 +115,11 @@ circuit. The indices are only available for forwarding events saved after v0.20.
   [fixed](https://github.com/lightningnetwork/lnd/pull/9572), the affected RPCs
   are `SubscribeChannelGraph`, `GetChanInfo`, `GetNodeInfo` and `DescribeGraph`.
 
+* [Fix a bug](https://github.com/lightningnetwork/lnd/pull/10064) where the
+  `GetChanInfo` was not returning the correct gRPC status code in the cases 
+  where the channel is unknown to the node. The same is done for `LookupInvoice`
+  for the case where the DB is kvdb backed and no invoices have yet been added
+  to the database.
 
 ## lncli Updates
 
@@ -189,5 +194,6 @@ reader of a payment request.
 * Funyug
 * Mohamed Awnallah
 * Pins
+* Torkel Rogstad
 * Yong Yu
 * Ziggie

itest/list_on_test.go

@@ -715,6 +715,10 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "peer bootstrapping",
 		TestFunc: testPeerBootstrapping,
 	},
+	{
+		Name:     "grpc not found",
+		TestFunc: testGRPCNotFound,
+	},
 }
 
 // appendPrefixed is used to add a prefix to each test name in the subtests

itest/lnd_misc_test.go

@@ -1,6 +1,7 @@
 package itest
 
 import (
+	"crypto/rand"
 	"encoding/hex"
 	"fmt"
 	"os"
@@ -21,7 +22,9 @@ import (
 	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/lightningnetwork/lnd/routing/route"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
 )
 
 // testSphinxReplayPersistence verifies that replayed onion packets are
@@ -1344,3 +1347,51 @@ func testSendSelectedCoinsChannelReserve(ht *lntest.HarnessTest) {
 	// Alice should have one reserved UTXO now.
 	ht.AssertNumUTXOs(alice, 1)
 }
+
+// testGRPCNotFound verifies that the expected grpc NotFound status code is
+// returned when querying various rpc endpoints.
+func testGRPCNotFound(ht *lntest.HarnessTest) {
+	var (
+		notFoundErr = codes.NotFound.String()
+		unknownPub  = "0286098b97bc843372b4426d4b276cea9aa2f48f0428d6" +
+			"f5b66ae101befc14f8b4"
+		rHash = make([]byte, 32)
+	)
+	unknownPubBytes, err := route.NewVertexFromStr(unknownPub)
+	require.NoError(ht, err)
+
+	_, err = rand.Read(rHash)
+	require.NoError(ht, err)
+
+	alice := ht.NewNode("Alice", []string{
+		// We add this flag so that we can test the
+		// LookupHTLCResolutionAssertErr endpoint.
+		"--store-final-htlc-resolutions",
+	}).RPC
+
+	alice.GetChanInfoAssertErr(&lnrpc.ChanInfoRequest{
+		// Use a random channel ID that doesn't exist.
+		ChanId: 949807622323240961,
+	}, notFoundErr)
+
+	alice.GetNodeInfoAssertErr(&lnrpc.NodeInfoRequest{
+		// Use a random pubkey that doesn't exist.
+		PubKey: unknownPub,
+	}, notFoundErr)
+
+	alice.SendCustomMessageAssertErr(&lnrpc.SendCustomMessageRequest{
+		// Use a random pubkey that doesn't exist.
+		Peer: unknownPubBytes[:],
+		Data: []byte("test message"),
+	}, notFoundErr)
+
+	alice.LookupHTLCResolutionAssertErr(&lnrpc.LookupHtlcResolutionRequest{
+		ChanId:    400000,
+		HtlcIndex: 300,
+	}, notFoundErr)
+
+	alice.LookupInvoiceAssertErr(&lnrpc.PaymentHash{
+		// Use a random payment hash that doesn't exist.
+		RHash: rHash,
+	}, notFoundErr)
+}

lntest/rpc/lnd.go

@@ -303,6 +303,66 @@ func (h *HarnessRPC) AddInvoiceAssertErr(req *lnrpc.Invoice, errStr string) {
 	require.ErrorContains(h, err, errStr)
 }
 
+// GetChanInfoAssertErr makes an RPC call to GetChanInfo and asserts an error
+// has returned with a specific error message.
+func (h *HarnessRPC) GetChanInfoAssertErr(req *lnrpc.ChanInfoRequest,
+	errStr string) {
+
+	ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout)
+	defer cancel()
+
+	_, err := h.LN.GetChanInfo(ctxt, req)
+	require.ErrorContains(h, err, errStr)
+}
+
+// GetNodeInfoAssertErr makes an RPC call to GetNodeInfo and asserts an error
+// has returned with a specific error message.
+func (h *HarnessRPC) GetNodeInfoAssertErr(req *lnrpc.NodeInfoRequest,
+	errStr string) {
+
+	ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout)
+	defer cancel()
+
+	_, err := h.LN.GetNodeInfo(ctxt, req)
+	require.ErrorContains(h, err, errStr)
+}
+
+// SendCustomMessageAssertErr makes an RPC call to SendCustomMessage and asserts
+// an error has returned with a specific error message.
+func (h *HarnessRPC) SendCustomMessageAssertErr(
+	req *lnrpc.SendCustomMessageRequest, errStr string) {
+
+	ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout)
+	defer cancel()
+
+	_, err := h.LN.SendCustomMessage(ctxt, req)
+	require.ErrorContains(h, err, errStr)
+}
+
+// LookupInvoiceAssertErr makes an RPC call to LookupInvoice and asserts an
+// error has returned with a specific error message.
+func (h *HarnessRPC) LookupInvoiceAssertErr(req *lnrpc.PaymentHash,
+	errStr string) {
+
+	ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout)
+	defer cancel()
+
+	_, err := h.LN.LookupInvoice(ctxt, req)
+	require.ErrorContains(h, err, errStr)
+}
+
+// LookupHTLCResolutionAssertErr makes an RPC call to LookupHTLCResolution and
+// asserts an error has returned with a specific error message.
+func (h *HarnessRPC) LookupHTLCResolutionAssertErr(
+	req *lnrpc.LookupHtlcResolutionRequest, errStr string) {
+
+	ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout)
+	defer cancel()
+
+	_, err := h.LN.LookupHtlcResolution(ctxt, req)
+	require.ErrorContains(h, err, errStr)
+}
+
 // AbandonChannel makes a RPC call to AbandonChannel and asserts.
 func (h *HarnessRPC) AbandonChannel(
 	req *lnrpc.AbandonChannelRequest) *lnrpc.AbandonChannelResponse {

rpcserver.go

@@ -6445,7 +6445,9 @@ func (r *rpcServer) LookupInvoice(ctx context.Context,
 
 	invoice, err := r.server.invoices.LookupInvoice(ctx, payHash)
 	switch {
-	case errors.Is(err, invoices.ErrInvoiceNotFound):
+	case errors.Is(err, invoices.ErrInvoiceNotFound) ||
+		errors.Is(err, invoices.ErrNoInvoicesCreated):
+
 		return nil, status.Error(codes.NotFound, err.Error())
 	case err != nil:
 		return nil, err
@@ -6989,7 +6991,10 @@ func (r *rpcServer) GetChanInfo(_ context.Context,
 	default:
 		return nil, fmt.Errorf("specify either chan_id or chan_point")
 	}
-	if err != nil {
+	switch {
+	case errors.Is(err, graphdb.ErrEdgeNotFound):
+		return nil, status.Error(codes.NotFound, err.Error())
+	case err != nil:
 		return nil, err
 	}