Merge pull request #158 from guggero/max-quote-fix

Don't fail loop in quote if balance is insufficient for the miner fee estimation

Author: guggero

client.go

@@ -5,6 +5,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -59,6 +60,11 @@ var (
 	globalCallTimeout = serverRPCTimeout + lsat.PaymentTimeout
 
 	republishDelay = 10 * time.Second
+
+	// MinerFeeEstimationFailed is a magic number that is returned in a
+	// quote call as the miner fee if the fee estimation in lnd's wallet
+	// failed because of insufficient funds.
+	MinerFeeEstimationFailed btcutil.Amount = -1
 )
 
 // Client performs the client side part of swaps. This interface exists to be
@@ -505,10 +511,24 @@ func (s *Client) LoopInQuote(ctx context.Context,
 		}, nil
 	}
 
-	// Get estimate for miner fee.
+	// Get estimate for miner fee. If estimating the miner fee for the
+	// requested amount is not possible because lnd's wallet cannot
+	// construct a sample TX, we just return zero instead of failing the
+	// quote. The user interface should inform the user that fee estimation
+	// was not possible.
+	//
+	// TODO(guggero): Thread through error code from lnd to avoid string
+	// matching.
 	minerFee, err := s.lndServices.Client.EstimateFeeToP2WSH(
 		ctx, request.Amount, request.HtlcConfTarget,
 	)
+	if err != nil && strings.Contains(err.Error(), "insufficient funds") {
+		return &LoopInQuote{
+			SwapFee:   swapFee,
+			MinerFee:  MinerFeeEstimationFailed,
+			CltvDelta: quote.CltvDelta,
+		}, nil
+	}
 	if err != nil {
 		return nil, err
 	}

cmd/loop/loopin.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/btcsuite/btcutil"
+	"github.com/lightninglabs/loop"
 	"github.com/lightninglabs/loop/looprpc"
 	"github.com/lightninglabs/loop/swap"
 	"github.com/lightningnetwork/lnd/routing/route"
@@ -76,6 +77,18 @@ func loopIn(ctx *cli.Context) error {
 		return err
 	}
 
+	// For loop in, the fee estimation is handed to lnd which tries to
+	// construct a real transaction to sample realistic fees to pay to the
+	// HTLC. If the wallet doesn't have enough funds to create this TX, we
+	// know it won't have enough to pay the real transaction either. It
+	// makes sense to abort the loop in this case.
+	if !external && quote.MinerFee == int64(loop.MinerFeeEstimationFailed) {
+		return fmt.Errorf("miner fee estimation not " +
+			"possible, lnd has insufficient funds to " +
+			"create a sample transaction for selected " +
+			"amount")
+	}
+
 	limits := getInLimits(amt, quote)
 	err = displayLimits(swap.TypeIn, amt, limits, external, "")
 	if err != nil {

cmd/loop/main.go

@@ -159,9 +159,8 @@ func displayLimits(swapType swap.Type, amt btcutil.Amount, l *limits,
 			"wallet.\n\n")
 	}
 
-	fmt.Printf("Max swap fees for %d Loop %v: %d\n",
-		amt, swapType, totalSuccessMax,
-	)
+	fmt.Printf("Max swap fees for %d Loop %v: %d\n", amt, swapType,
+		totalSuccessMax)
 
 	if warning != "" {
 		fmt.Println(warning)

cmd/loop/quote.go

@@ -2,15 +2,85 @@ package main
 
 import (
 	"context"
+	"fmt"
+	"os"
 	"time"
 
+	"github.com/lightninglabs/loop"
 	"github.com/lightninglabs/loop/looprpc"
 	"github.com/urfave/cli"
 )
 
 var quoteCommand = cli.Command{
 	Name:        "quote",
 	Usage:       "get a quote for the cost of a swap",
+	Subcommands: []cli.Command{quoteInCommand, quoteOutCommand},
+}
+
+var quoteInCommand = cli.Command{
+	Name:        "in",
+	Usage:       "get a quote for the cost of a loop in swap",
+	ArgsUsage:   "amt",
+	Description: "Allows to determine the cost of a swap up front",
+	Flags: []cli.Flag{
+		cli.Uint64Flag{
+			Name: "conf_target",
+			Usage: "the number of blocks from the swap " +
+				"initiation height that the on-chain HTLC " +
+				"should be swept within in a Loop Out",
+			Value: 6,
+		},
+	},
+	Action: quoteIn,
+}
+
+func quoteIn(ctx *cli.Context) error {
+	// Show command help if the incorrect number arguments was provided.
+	if ctx.NArg() != 1 {
+		return cli.ShowCommandHelp(ctx, "in")
+	}
+
+	args := ctx.Args()
+	amt, err := parseAmt(args[0])
+	if err != nil {
+		return err
+	}
+
+	client, cleanup, err := getClient(ctx)
+	if err != nil {
+		return err
+	}
+	defer cleanup()
+
+	ctxb := context.Background()
+	quoteReq := &looprpc.QuoteRequest{
+		Amt:        int64(amt),
+		ConfTarget: int32(ctx.Uint64("conf_target")),
+	}
+	quoteResp, err := client.GetLoopInQuote(ctxb, quoteReq)
+	if err != nil {
+		return err
+	}
+
+	// For loop in, the fee estimation is handed to lnd which tries to
+	// construct a real transaction to sample realistic fees to pay to the
+	// HTLC. If the wallet doesn't have enough funds to create this TX, we
+	// don't want to fail the quote. But the user should still be informed
+	// why the fee shows as -1.
+	if quoteResp.MinerFee == int64(loop.MinerFeeEstimationFailed) {
+		_, _ = fmt.Fprintf(os.Stderr, "Warning: Miner fee estimation "+
+			"not possible, lnd has insufficient funds to "+
+			"create a sample transaction for selected "+
+			"amount.\n")
+	}
+
+	printRespJSON(quoteResp)
+	return nil
+}
+
+var quoteOutCommand = cli.Command{
+	Name:        "out",
+	Usage:       "get a quote for the cost of a loop out swap",
 	ArgsUsage:   "amt",
 	Description: "Allows to determine the cost of a swap up front",
 	Flags: []cli.Flag{
@@ -32,13 +102,13 @@ var quoteCommand = cli.Command{
 				"swap fee.",
 		},
 	},
-	Action: quote,
+	Action: quoteOut,
 }
 
-func quote(ctx *cli.Context) error {
+func quoteOut(ctx *cli.Context) error {
 	// Show command help if the incorrect number arguments was provided.
 	if ctx.NArg() != 1 {
-		return cli.ShowCommandHelp(ctx, "quote")
+		return cli.ShowCommandHelp(ctx, "out")
 	}
 
 	args := ctx.Args()
@@ -60,15 +130,16 @@ func quote(ctx *cli.Context) error {
 	}
 
 	ctxb := context.Background()
-	resp, err := client.LoopOutQuote(ctxb, &looprpc.QuoteRequest{
+	quoteReq := &looprpc.QuoteRequest{
 		Amt:                     int64(amt),
 		ConfTarget:              int32(ctx.Uint64("conf_target")),
 		SwapPublicationDeadline: uint64(swapDeadline.Unix()),
-	})
+	}
+	quoteResp, err := client.LoopOutQuote(ctxb, quoteReq)
 	if err != nil {
 		return err
 	}
 
-	printRespJSON(resp)
+	printRespJSON(quoteResp)
 	return nil
 }

looprpc/client.pb.go

@@ -412,7 +412,7 @@ message QuoteRequest {
     publishing the HTLC on chain. Setting this to a larger value will give the
     server the opportunity to batch multiple swaps together, and wait for
     low-fee periods before publishing the HTLC, potentially resulting in a
-    lower total swap fee.
+    lower total swap fee. This only has an effect on loop out quotes.
     */
     uint64 swap_publication_deadline = 4;
 }
@@ -429,7 +429,13 @@ message QuoteResponse {
     int64 prepay_amt = 2;
 
     /**
-    An estimate of the on-chain fee that needs to be paid to sweep the HTLC.
+    An estimate of the on-chain fee that needs to be paid to sweep the HTLC for
+    a loop out or to pay to the HTLC for loop in. If a miner fee of 0 is
+    returned, it means the external_htlc flag was set for a loop in and the fee
+    estimation was skipped. If a miner fee of -1 is returned, it means lnd's
+    wallet tried to estimate the fee but was unable to create a sample
+    estimation transaction because not enough funds are available. An
+    information message should be shown to the user in this case.
     */
     int64 miner_fee = 3;
 

looprpc/client.proto

@@ -81,7 +81,7 @@
           },
           {
             "name": "swap_publication_deadline",
-            "description": "*\nThe latest time (in unix seconds) we allow the server to wait before\npublishing the HTLC on chain. Setting this to a larger value will give the\nserver the opportunity to batch multiple swaps together, and wait for\nlow-fee periods before publishing the HTLC, potentially resulting in a\nlower total swap fee.",
+            "description": "*\nThe latest time (in unix seconds) we allow the server to wait before\npublishing the HTLC on chain. Setting this to a larger value will give the\nserver the opportunity to batch multiple swaps together, and wait for\nlow-fee periods before publishing the HTLC, potentially resulting in a\nlower total swap fee. This only has an effect on loop out quotes.",
             "in": "query",
             "required": false,
             "type": "string",
@@ -176,7 +176,7 @@
           },
           {
             "name": "swap_publication_deadline",
-            "description": "*\nThe latest time (in unix seconds) we allow the server to wait before\npublishing the HTLC on chain. Setting this to a larger value will give the\nserver the opportunity to batch multiple swaps together, and wait for\nlow-fee periods before publishing the HTLC, potentially resulting in a\nlower total swap fee.",
+            "description": "*\nThe latest time (in unix seconds) we allow the server to wait before\npublishing the HTLC on chain. Setting this to a larger value will give the\nserver the opportunity to batch multiple swaps together, and wait for\nlow-fee periods before publishing the HTLC, potentially resulting in a\nlower total swap fee. This only has an effect on loop out quotes.",
             "in": "query",
             "required": false,
             "type": "string",
@@ -424,7 +424,7 @@
         "miner_fee": {
           "type": "string",
           "format": "int64",
-          "description": "*\nAn estimate of the on-chain fee that needs to be paid to sweep the HTLC."
+          "description": "*\nAn estimate of the on-chain fee that needs to be paid to sweep the HTLC for\na loop out or to pay to the HTLC for loop in. If a miner fee of 0 is\nreturned, it means the external_htlc flag was set for a loop in and the fee\nestimation was skipped. If a miner fee of -1 is returned, it means lnd's\nwallet tried to estimate the fee but was unable to create a sample\nestimation transaction because not enough funds are available. An\ninformation message should be shown to the user in this case."
         },
         "swap_payment_dest": {
           "type": "string",