Merge pull request #2556 from cfromknecht/list-unspent-recovery

lnrpc+rpcserver: return all utxos by default from ListUnspent

cmd/lncli/commands.go

@@ -257,14 +257,17 @@ var listUnspentCommand = cli.Command{
 	Name:      "listunspent",
 	Category:  "On-chain",
 	Usage:     "List utxos available for spending.",
-	ArgsUsage: "min-confs max-confs",
+	ArgsUsage: "[min-confs [max-confs]] [--unconfirmed_only]",
 	Description: `
 	For each spendable utxo currently in the wallet, with at least min_confs
-	confirmations, and at most max_confs confirmations, lists the txid, index,
-	amount, address, address type, scriptPubkey and number of confirmations.
-	Use --min_confs=0 to include unconfirmed coins. To list all coins
-	with at least min_confs confirmations, omit the second argument or flag
-	'--max_confs'.
+	confirmations, and at most max_confs confirmations, lists the txid,
+	index, amount, address, address type, scriptPubkey and number of
+	confirmations.  Use --min_confs=0 to include unconfirmed coins. To list
+	all coins with at least min_confs confirmations, omit the second
+	argument or flag '--max_confs'. To list all confirmed and unconfirmed
+	coins, no arguments are required. To see only unconfirmed coins, use 
+	'--unconfirmed_only' with '--min_confs' and '--max_confs' set to zero or
+	not present.
 	`,
 	Flags: []cli.Flag{
 		cli.Int64Flag{
@@ -275,6 +278,15 @@ var listUnspentCommand = cli.Command{
 			Name:  "max_confs",
 			Usage: "the maximum number of confirmations for a utxo",
 		},
+		cli.BoolFlag{
+			Name: "unconfirmed_only",
+			Usage: "when min_confs and max_confs are zero, " +
+				"setting false implicitly overrides max_confs " +
+				"to be MaxInt32, otherwise max_confs remains " +
+				"zero. An error is returned if the value is " +
+				"true and both min_confs and max_confs are " +
+				"non-zero. (defualt: false)",
+		},
 	},
 	Action: actionDecorator(listUnspent),
 }
@@ -287,11 +299,6 @@ func listUnspent(ctx *cli.Context) error {
 	)
 	args := ctx.Args()
 
-	if ctx.NArg() == 0 && ctx.NumFlags() == 0 {
-		cli.ShowCommandHelp(ctx, "listunspent")
-		return nil
-	}
-
 	if ctx.IsSet("max_confs") && !ctx.IsSet("min_confs") {
 		return fmt.Errorf("max_confs cannot be set without " +
 			"min_confs being set")
@@ -307,8 +314,6 @@ func listUnspent(ctx *cli.Context) error {
 			return nil
 		}
 		args = args.Tail()
-	default:
-		return fmt.Errorf("minimum confirmations argument missing")
 	}
 
 	switch {
@@ -321,15 +326,21 @@ func listUnspent(ctx *cli.Context) error {
 			return nil
 		}
 		args = args.Tail()
-	default:
-		// No maxconfs was specified; we use max as flag;
-		// the default for ctx.Int64 (0) is *not* appropriate here.
-		maxConfirms = math.MaxInt32
 	}
 
-	if minConfirms < 0 || maxConfirms < minConfirms {
-		return fmt.Errorf("maximum confirmations must be greater or " +
-			"equal to minimum confirmations")
+	unconfirmedOnly := ctx.Bool("unconfirmed_only")
+
+	// Force minConfirms and maxConfirms to be zero if unconfirmedOnly is
+	// true.
+	if unconfirmedOnly && (minConfirms != 0 || maxConfirms != 0) {
+		cli.ShowCommandHelp(ctx, "listunspent")
+		return nil
+	}
+
+	// When unconfirmedOnly is inactive, we will override maxConfirms to be
+	// a MaxInt32 to return all confirmed and unconfirmed utxos.
+	if maxConfirms == 0 && !unconfirmedOnly {
+		maxConfirms = math.MaxInt32
 	}
 
 	ctxb := context.Background()

cmd/lncli/types.go

@@ -20,7 +20,7 @@ type Utxo struct {
 	Type          lnrpc.AddressType `json:"address_type"`
 	Address       string            `json:"address"`
 	AmountSat     int64             `json:"amount_sat"`
-	ScriptPubkey  string            `json:"script_pubkey"`
+	PkScript      string            `json:"pk_script"`
 	OutPoint      OutPoint          `json:"outpoint"`
 	Confirmations int64             `json:"confirmations"`
 }
@@ -33,7 +33,7 @@ func NewUtxoFromProto(utxo *lnrpc.Utxo) *Utxo {
 		Type:          utxo.Type,
 		Address:       utxo.Address,
 		AmountSat:     utxo.AmountSat,
-		ScriptPubkey:  utxo.ScriptPubkey,
+		PkScript:      utxo.PkScript,
 		OutPoint:      NewOutPointFromProto(utxo.Outpoint),
 		Confirmations: utxo.Confirmations,
 	}

lnd_test.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"math"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -643,8 +644,8 @@ func testOnchainFundRecovery(net *lntest.NetworkHarness, t *harnessTest) {
 	// method takes the expected value of Carol's balance when using the
 	// given recovery window. Additionally, the caller can specify an action
 	// to perform on the restored node before the node is shutdown.
-	restoreCheckBalance := func(expAmount int64, recoveryWindow int32,
-		fn func(*lntest.HarnessNode)) {
+	restoreCheckBalance := func(expAmount int64, expectedNumUTXOs int,
+		recoveryWindow int32, fn func(*lntest.HarnessNode)) {
 
 		// Restore Carol, passing in the password, mnemonic, and
 		// desired recovery window.
@@ -655,8 +656,12 @@ func testOnchainFundRecovery(net *lntest.NetworkHarness, t *harnessTest) {
 			t.Fatalf("unable to restore node: %v", err)
 		}
 
-		// Query carol for her current wallet balance.
-		var currBalance int64
+		// Query carol for her current wallet balance, and also that we
+		// gain the expected number of UTXOs.
+		var (
+			currBalance  int64
+			currNumUTXOs uint32
+		)
 		err = lntest.WaitPredicate(func() bool {
 			req := &lnrpc.WalletBalanceRequest{}
 			ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
@@ -673,12 +678,27 @@ func testOnchainFundRecovery(net *lntest.NetworkHarness, t *harnessTest) {
 				return false
 			}
 
+			utxoReq := &lnrpc.ListUnspentRequest{
+				MaxConfs: math.MaxInt32,
+			}
+			ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+			utxoResp, err := node.ListUnspent(ctxt, utxoReq)
+			if err != nil {
+				t.Fatalf("unable to query utxos: %v", err)
+			}
+
+			currNumUTXOs := len(utxoResp.Utxos)
+			if currNumUTXOs != expectedNumUTXOs {
+				return false
+			}
+
 			return true
 		}, 15*time.Second)
 		if err != nil {
 			t.Fatalf("expected restored node to have %d satoshis, "+
-				"instead has %d satoshis", expAmount,
-				currBalance)
+				"instead has %d satoshis, expected %d utxos "+
+				"instead has %d", expAmount, currBalance,
+				expectedNumUTXOs, currNumUTXOs)
 		}
 
 		// If the user provided a callback, execute the commands against
@@ -750,37 +770,40 @@ func testOnchainFundRecovery(net *lntest.NetworkHarness, t *harnessTest) {
 	//
 	// After, one BTC is sent to both her first external P2WKH and NP2WKH
 	// addresses.
-	restoreCheckBalance(0, 0, skipAndSend(0))
+	restoreCheckBalance(0, 0, 0, skipAndSend(0))
 
 	// Check that restoring without a look-ahead results in having no funds
 	// in the wallet, even though they exist on-chain.
-	restoreCheckBalance(0, 0, nil)
+	restoreCheckBalance(0, 0, 0, nil)
 
-	// Now, check that using a look-ahead of 1 recovers the balance from the
-	// two transactions above.
+	// Now, check that using a look-ahead of 1 recovers the balance from
+	// the two transactions above. We should also now have 2 UTXOs in the
+	// wallet at the end of the recovery attempt.
 	//
 	// After, we will generate and skip 9 P2WKH and NP2WKH addresses, and
 	// send another BTC to the subsequent 10th address in each derivation
 	// path.
-	restoreCheckBalance(2*btcutil.SatoshiPerBitcoin, 1, skipAndSend(9))
+	restoreCheckBalance(2*btcutil.SatoshiPerBitcoin, 2, 1, skipAndSend(9))
 
 	// Check that using a recovery window of 9 does not find the two most
 	// recent txns.
-	restoreCheckBalance(2*btcutil.SatoshiPerBitcoin, 9, nil)
+	restoreCheckBalance(2*btcutil.SatoshiPerBitcoin, 2, 9, nil)
 
 	// Extending our recovery window to 10 should find the most recent
-	// transactions, leaving the wallet with 4 BTC total.
+	// transactions, leaving the wallet with 4 BTC total. We should also
+	// learn of the two additional UTXOs created above.
 	//
 	// After, we will skip 19 more addrs, sending to the 20th address past
 	// our last found address, and repeat the same checks.
-	restoreCheckBalance(4*btcutil.SatoshiPerBitcoin, 10, skipAndSend(19))
+	restoreCheckBalance(4*btcutil.SatoshiPerBitcoin, 4, 10, skipAndSend(19))
 
 	// Check that recovering with a recovery window of 19 fails to find the
 	// most recent transactions.
-	restoreCheckBalance(4*btcutil.SatoshiPerBitcoin, 19, nil)
+	restoreCheckBalance(4*btcutil.SatoshiPerBitcoin, 4, 19, nil)
 
-	// Ensure that using a recovery window of 20 succeeds.
-	restoreCheckBalance(6*btcutil.SatoshiPerBitcoin, 20, nil)
+	// Ensure that using a recovery window of 20 succeeds with all UTXOs
+	// found and the final balance reflected.
+	restoreCheckBalance(6*btcutil.SatoshiPerBitcoin, 6, 20, nil)
 }
 
 // testBasicChannelFunding performs a test exercising expected behavior from a

lnrpc/rpc.proto

@@ -686,8 +686,8 @@ message Utxo {
     /// The value of the unspent coin in satoshis
     int64 amount_sat = 3 [json_name = "amount_sat"];
 
-    /// The scriptpubkey in hex
-    string script_pubkey = 4 [json_name = "script_pubkey"];
+    /// The pkscript in hex
+    string pk_script = 4 [json_name = "pk_script"];
 
     /// The outpoint in format txid:n
     OutPoint outpoint = 5 [json_name = "outpoint"];
@@ -880,16 +880,15 @@ message SendCoinsResponse {
 }
 
 message ListUnspentRequest {
-	/// The minimum number of confirmations to be included.
-	int32 min_confs = 1;
+    /// The minimum number of confirmations to be included.
+    int32 min_confs = 1;
 
-	/// The maximum number of confirmations to be included.
-	int32 max_confs = 2;
+    /// The maximum number of confirmations to be included.
+    int32 max_confs = 2;
 }
 message ListUnspentResponse {
-	/// A list of utxos
-	repeated Utxo utxos = 1 [json_name = "utxos"];
-
+    /// A list of utxos
+    repeated Utxo utxos = 1 [json_name = "utxos"];
 }
 
 /** 

lnrpc/rpc.swagger.json

@@ -3068,9 +3068,9 @@
           "format": "int64",
           "title": "/ The value of the unspent coin in satoshis"
         },
-        "script_pubkey": {
+        "pk_script": {
           "type": "string",
-          "title": "/ The scriptpubkey in hex"
+          "title": "/ The pkscript in hex"
         },
         "outpoint": {
           "$ref": "#/definitions/lnrpcOutPoint",

lntest/harness.go

@@ -1,6 +1,7 @@
 package lntest
 
 import (
+	"encoding/hex"
 	"errors"
 	"fmt"
 	"io"
@@ -1297,6 +1298,41 @@ func (n *NetworkHarness) sendCoins(ctx context.Context, amt btcutil.Amount,
 		return err
 	}
 
+	// Encode the pkScript in hex as this the format that it will be
+	// returned via rpc.
+	expPkScriptStr := hex.EncodeToString(addrScript)
+
+	// Now, wait for ListUnspent to show the unconfirmed transaction
+	// containing the correct pkscript.
+	err = WaitNoError(func() error {
+		req := &lnrpc.ListUnspentRequest{}
+		resp, err := target.ListUnspent(ctx, req)
+		if err != nil {
+			return err
+		}
+
+		// When using this method, there should only ever be on
+		// unconfirmed transaction.
+		if len(resp.Utxos) != 1 {
+			return fmt.Errorf("number of unconfirmed utxos "+
+				"should be 1, found %d", len(resp.Utxos))
+		}
+
+		// Assert that the lone unconfirmed utxo contains the same
+		// pkscript as the output generated above.
+		pkScriptStr := resp.Utxos[0].PkScript
+		if strings.Compare(pkScriptStr, expPkScriptStr) != 0 {
+			return fmt.Errorf("pkscript mismatch, want: %s, "+
+				"found: %s", expPkScriptStr, pkScriptStr)
+		}
+
+		return nil
+	}, 15*time.Second)
+	if err != nil {
+		return fmt.Errorf("unconfirmed utxo was not found in "+
+			"ListUnspent: %v", err)
+	}
+
 	// If the transaction should remain unconfirmed, then we'll wait until
 	// the target node's unconfirmed balance reflects the expected balance
 	// and exit.

rpcserver.go

@@ -731,7 +731,7 @@ func (r *rpcServer) ListUnspent(ctx context.Context,
 		utxoResp := lnrpc.Utxo{
 			Type:          addrType,
 			AmountSat:     int64(utxo.Value),
-			ScriptPubkey:  hex.EncodeToString(utxo.PkScript),
+			PkScript:      hex.EncodeToString(utxo.PkScript),
 			Outpoint:      outpoint,
 			Confirmations: utxo.Confirmations,
 		}