multi: fix verbose output for listsweeps and check all against wallet

Previously, the verbose output of listsweeps would fail if we did not
find some sweeps in our wallet's listtransactions output. This could be
the case for sweeps that were rbf-ed, so the endpoint would fail. This
commit also updates the listsweeps endpoint to always check against the
wallet, so that we do not return these discarded sweeps that never
confirmed.

lnrpc/walletrpc/walletkit.pb.go

@@ -1117,7 +1117,8 @@ var xxx_messageInfo_BumpFeeResponse proto.InternalMessageInfo
 type ListSweepsRequest struct {
 	//
 	//Retrieve the full sweep transaction details. If false, only the sweep txids
-	//will be returned.
+	//will be returned. Note that some sweeps that LND publishes will have been
+	//replaced-by-fee, so will not be included in this output.
 	Verbose              bool     `protobuf:"varint,1,opt,name=verbose,proto3" json:"verbose,omitempty"`
 	XXX_NoUnkeyedLiteral struct{} `json:"-"`
 	XXX_unrecognized     []byte   `json:"-"`

lnrpc/walletrpc/walletkit.proto

@@ -464,7 +464,8 @@ message BumpFeeResponse {
 message ListSweepsRequest {
     /*
     Retrieve the full sweep transaction details. If false, only the sweep txids
-    will be returned.
+    will be returned. Note that some sweeps that LND publishes will have been
+    replaced-by-fee, so will not be included in this output.
     */
     bool verbose = 1;
 }

lnrpc/walletrpc/walletkit.swagger.json

@@ -299,7 +299,7 @@
         "parameters": [
           {
             "name": "verbose",
-            "description": "Retrieve the full sweep transaction details. If false, only the sweep txids\nwill be returned.",
+            "description": "Retrieve the full sweep transaction details. If false, only the sweep txids\nwill be returned. Note that some sweeps that LND publishes will have been\nreplaced-by-fee, so will not be included in this output.",
             "in": "query",
             "required": false,
             "type": "boolean",

lnrpc/walletrpc/walletkit_server.go

@@ -782,29 +782,14 @@ func (w *WalletKit) ListSweeps(ctx context.Context,
 	}
 
 	sweepTxns := make(map[string]bool)
-
-	txids := make([]string, len(sweeps))
-	for i, sweep := range sweeps {
+	for _, sweep := range sweeps {
 		sweepTxns[sweep.String()] = true
-		txids[i] = sweep.String()
 	}
 
-	// If the caller does not want verbose output, just return the set of
-	// sweep txids.
-	if !in.Verbose {
-		txidResp := &ListSweepsResponse_TransactionIDs{
-			TransactionIds: txids,
-		}
-
-		return &ListSweepsResponse{
-			Sweeps: &ListSweepsResponse_TransactionIds{
-				TransactionIds: txidResp,
-			},
-		}, nil
-	}
-
-	// If the caller does want full transaction lookups, query our wallet
-	// for all transactions, including unconfirmed transactions.
+	// Some of our sweeps could have been replaced by fee, or dropped out
+	// of the mempool. Here, we lookup our wallet transactions so that we
+	// can match our list of sweeps against the list of transactions that
+	// the wallet is still tracking.
 	transactions, err := w.cfg.Wallet.ListTransactionDetails(
 		0, btcwallet.UnconfirmedHeight,
 	)
@@ -812,26 +797,41 @@ func (w *WalletKit) ListSweeps(ctx context.Context,
 		return nil, err
 	}
 
-	var sweepTxDetails []*lnwallet.TransactionDetail
+	var (
+		txids     []string
+		txDetails []*lnwallet.TransactionDetail
+	)
+
 	for _, tx := range transactions {
 		_, ok := sweepTxns[tx.Hash.String()]
 		if !ok {
 			continue
 		}
 
-		sweepTxDetails = append(sweepTxDetails, tx)
+		// Add the txid or full tx details depending on whether we want
+		// verbose output or not.
+		if in.Verbose {
+			txDetails = append(txDetails, tx)
+		} else {
+			txids = append(txids, tx.Hash.String())
+		}
 	}
 
-	// Fail if we have not retrieved all of our sweep transactions from the
-	// wallet.
-	if len(sweepTxDetails) != len(txids) {
-		return nil, fmt.Errorf("not all sweeps found by list "+
-			"transactions: %v, %v", len(sweepTxDetails), len(txids))
+	if in.Verbose {
+		return &ListSweepsResponse{
+			Sweeps: &ListSweepsResponse_TransactionDetails{
+				TransactionDetails: lnrpc.RPCTransactionDetails(
+					txDetails,
+				),
+			},
+		}, nil
 	}
 
 	return &ListSweepsResponse{
-		Sweeps: &ListSweepsResponse_TransactionDetails{
-			TransactionDetails: lnrpc.RPCTransactionDetails(transactions),
+		Sweeps: &ListSweepsResponse_TransactionIds{
+			TransactionIds: &ListSweepsResponse_TransactionIDs{
+				TransactionIds: txids,
+			},
 		},
 	}, nil
 }

lntest/itest/lnd_test.go

@@ -3975,11 +3975,8 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest,
 	}
 
 	// Check that we can find the commitment sweep in our set of known
-	// sweeps.
-	err = findSweep(ctxb, alice, sweepingTXID)
-	if err != nil {
-		t.Fatalf("csv sweep not found: %v", err)
-	}
+	// sweeps, using the simple transaction id ListSweeps output.
+	assertSweepFound(ctxb, t.t, alice, sweepingTXID.String(), false)
 
 	// Restart Alice to ensure that she resumes watching the finalized
 	// commitment sweep txid.
@@ -4368,11 +4365,9 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest,
 		}
 	}
 
-	// Check that we can find the htlc sweep in our set of sweeps.
-	err = findSweep(ctxb, alice, htlcSweepTx.Hash())
-	if err != nil {
-		t.Fatalf("htlc sweep not found: %v", err)
-	}
+	// Check that we can find the htlc sweep in our set of sweeps using
+	// the verbose output of the listsweeps output.
+	assertSweepFound(ctxb, t.t, alice, htlcSweepTx.Hash().String(), true)
 
 	// The following restart checks to ensure that the nursery store is
 	// storing the txid of the previously broadcast htlc sweep txn, and that
@@ -4571,33 +4566,60 @@ func assertReports(ctxb context.Context, t *harnessTest,
 	}
 }
 
-// findSweep looks up a sweep in a nodes list of broadcast sweeps.
-func findSweep(ctx context.Context, node *lntest.HarnessNode,
-	sweep *chainhash.Hash) error {
+// assertSweepFound looks up a sweep in a nodes list of broadcast sweeps.
+func assertSweepFound(ctx context.Context, t *testing.T, node *lntest.HarnessNode,
+	sweep string, verbose bool) {
 
 	// List all sweeps that alice's node had broadcast.
 	ctx, _ = context.WithTimeout(ctx, defaultTimeout)
 	sweepResp, err := node.WalletKitClient.ListSweeps(
 		ctx, &walletrpc.ListSweepsRequest{
-			Verbose: false,
-		})
-	if err != nil {
-		return fmt.Errorf("list sweeps error: %v", err)
+			Verbose: verbose,
+		},
+	)
+	require.NoError(t, err)
+
+	var found bool
+	if verbose {
+		found = findSweepInDetails(t, sweep, sweepResp)
+	} else {
+		found = findSweepInTxids(t, sweep, sweepResp)
 	}
 
-	sweepTxIDs, ok := sweepResp.Sweeps.(*walletrpc.ListSweepsResponse_TransactionIds)
-	if !ok {
-		return errors.New("expected sweep txids in response")
-	}
+	require.True(t, found, "sweep: %v not found", sweep)
+}
+
+func findSweepInTxids(t *testing.T, sweepTxid string,
+	sweepResp *walletrpc.ListSweepsResponse) bool {
+
+	sweepTxIDs := sweepResp.GetTransactionIds()
+	require.NotNil(t, sweepTxIDs, "expected transaction ids")
+	require.Nil(t, sweepResp.GetTransactionDetails())
 
 	// Check that the sweep tx we have just produced is present.
-	for _, tx := range sweepTxIDs.TransactionIds.TransactionIds {
-		if tx == sweep.String() {
-			return nil
+	for _, tx := range sweepTxIDs.TransactionIds {
+		if tx == sweepTxid {
+			return true
 		}
 	}
 
-	return fmt.Errorf("sweep: %v not found", sweep.String())
+	return false
+}
+
+func findSweepInDetails(t *testing.T, sweepTxid string,
+	sweepResp *walletrpc.ListSweepsResponse) bool {
+
+	sweepDetails := sweepResp.GetTransactionDetails()
+	require.NotNil(t, sweepDetails, "expected transaction details")
+	require.Nil(t, sweepResp.GetTransactionIds())
+
+	for _, tx := range sweepDetails.Transactions {
+		if tx.TxHash == sweepTxid {
+			return true
+		}
+	}
+
+	return false
 }
 
 // assertAmountSent generates a closure which queries listchannels for sndr and