Merge pull request #4762 from carlaKC/4748-sweeps

multi: update listsweeps to allow sweeps that are not in ListTransactions

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