Merge pull request #179 from carlaKC/loopin-addconftarget

loopin: allow user to configure confirmation target

Author: carlaKC

.gitignore

@@ -24,6 +24,9 @@ _testmain.go
 *.test
 *.prof
 
+/loop-debug
+/loopd-debug
+
 output*.log
 
 loop

.travis.yml

@@ -18,7 +18,7 @@ sudo: required
 
 script:
   - export GO111MODULE=on
-  - make lint unit
+  - make lint unit build
 
 after_script:
   - echo "Uploading to termbin.com..." && find *.log | xargs -I{} sh -c "cat {} | nc termbin.com 9999 | xargs -r0 printf '{} uploaded to %s'"

Makefile

@@ -3,6 +3,11 @@ PKG := github.com/lightninglabs/loop
 GOTEST := GO111MODULE=on go test -v
 
 GO_BIN := ${GOPATH}/bin
+GOBUILD := GO111MODULE=on go build -v
+GOINSTALL := GO111MODULE=on go install -v
+
+COMMIT := $(shell git describe --abbrev=40 --dirty)
+LDFLAGS := -ldflags "-X $(PKG)/build.Commit=$(COMMIT)"
 
 GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*")
 GOLIST := go list $(PKG)/... | grep -v '/vendor/'
@@ -33,4 +38,18 @@ fmt:
 
 lint: $(LINT_BIN)
 	@$(call print, "Linting source.")
-	$(LINT)
+	$(LINT)
+
+# ============
+# INSTALLATION
+# ============
+
+build:
+	@$(call print, "Building debug loop and loopd.")
+	$(GOBUILD) -o loop-debug $(LDFLAGS) $(PKG)/cmd/loop
+	$(GOBUILD) -o loopd-debug $(LDFLAGS) $(PKG)/cmd/loopd
+
+install:
+	@$(call print, "Installing loop and loopd.")
+	$(GOINSTALL) $(LDFLAGS) $(PKG)/cmd/loop
+	$(GOINSTALL) $(LDFLAGS) $(PKG)/cmd/loopd

README.md

@@ -195,8 +195,9 @@ DESCRIPTION:
     Send the amount in satoshis specified by the amt argument off-chain.
 
 OPTIONS:
-   --amt value  the amount in satoshis to loop in (default: 0)
-   --external   expect htlc to be published externally
+   --amt value    the amount in satoshis to loop in (default: 0)
+   --external     expect htlc to be published externally
+   --conf_target  the confirmation target for the on chain htlc, if not being published externally  
 ```
 
 The `--external` argument allows the on-chain HTLC transacting to be published

cmd/loop/loopin.go

@@ -18,12 +18,29 @@ var (
 		Usage: "the pubkey of the last hop to use for this swap",
 	}
 
+	confTargetFlag = cli.Uint64Flag{
+		Name: "conf_target",
+		Usage: "the target number of blocks the on-chain " +
+			"htlc broadcast by the swap client should " +
+			"confirm within",
+	}
+
 	loopInCommand = cli.Command{
 		Name:      "in",
 		Usage:     "perform an on-chain to off-chain swap (loop in)",
 		ArgsUsage: "amt",
 		Description: `
-		Send the amount in satoshis specified by the amt argument off-chain.`,
+		Send the amount in satoshis specified by the amt argument 
+		off-chain.
+		
+		By default the swap client will create and broadcast the 
+		on-chain htlc. The fee priority of this transaction can 
+		optionally be set using the conf_target flag. 
+
+		The external flag can be set to publish the on chain htlc 
+		independently. Note that this flag cannot be set with the 
+		conf_target flag.
+		`,
 		Flags: []cli.Flag{
 			cli.Uint64Flag{
 				Name:  "amt",
@@ -33,6 +50,7 @@ var (
 				Name:  "external",
 				Usage: "expect htlc to be published externally",
 			},
+			confTargetFlag,
 			lastHopFlag,
 		},
 		Action: loopIn,
@@ -66,10 +84,21 @@ func loopIn(ctx *cli.Context) error {
 	defer cleanup()
 
 	external := ctx.Bool("external")
+	htlcConfTarget := int32(ctx.Uint64(confTargetFlag.Name))
+
+	// External and confirmation target are mutually exclusive; either the
+	// on chain htlc is being externally broadcast, or we are creating the
+	// on chain htlc with a desired confirmation target. Fail if both are
+	// set.
+	if external && htlcConfTarget != 0 {
+		return fmt.Errorf("external and conf_target both set")
+	}
+
 	quote, err := client.GetLoopInQuote(
 		context.Background(),
 		&looprpc.QuoteRequest{
 			Amt:          int64(amt),
+			ConfTarget:   htlcConfTarget,
 			ExternalHtlc: external,
 		},
 	)
@@ -96,10 +125,11 @@ func loopIn(ctx *cli.Context) error {
 	}
 
 	req := &looprpc.LoopInRequest{
-		Amt:          int64(amt),
-		MaxMinerFee:  int64(limits.maxMinerFee),
-		MaxSwapFee:   int64(limits.maxSwapFee),
-		ExternalHtlc: external,
+		Amt:            int64(amt),
+		MaxMinerFee:    int64(limits.maxMinerFee),
+		MaxSwapFee:     int64(limits.maxSwapFee),
+		ExternalHtlc:   external,
+		HtlcConfTarget: htlcConfTarget,
 	}
 
 	if ctx.IsSet(lastHopFlag.Name) {

cmd/loop/quote.go

@@ -22,16 +22,8 @@ var quoteInCommand = cli.Command{
 	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,
+	Flags:       []cli.Flag{confTargetFlag},
+	Action:      quoteIn,
 }
 
 func quoteIn(ctx *cli.Context) error {
@@ -89,7 +81,7 @@ var quoteOutCommand = cli.Command{
 			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,
+			Value: uint64(loop.DefaultSweepConfTarget),
 		},
 		cli.BoolFlag{
 			Name: "fast",

loopd/start.go

@@ -14,10 +14,7 @@ import (
 	"github.com/lightningnetwork/lnd/build"
 )
 
-const (
-	defaultConfTarget     = int32(6)
-	defaultConfigFilename = "loopd.conf"
-)
+const defaultConfigFilename = "loopd.conf"
 
 // RPCConfig holds optional options that can be used to make the loop daemon
 // communicate on custom connections.

loopd/swapclient_server.go

@@ -361,9 +361,16 @@ func (s *swapClientServer) GetLoopInQuote(ctx context.Context,
 
 	log.Infof("Loop in quote request received")
 
+	htlcConfTarget, err := validateLoopInRequest(
+		req.ConfTarget, req.ExternalHtlc,
+	)
+	if err != nil {
+		return nil, err
+	}
+
 	quote, err := s.impl.LoopInQuote(ctx, &loop.LoopInQuoteRequest{
 		Amount:         btcutil.Amount(req.Amt),
-		HtlcConfTarget: defaultConfTarget,
+		HtlcConfTarget: htlcConfTarget,
 		ExternalHtlc:   req.ExternalHtlc,
 	})
 	if err != nil {
@@ -381,11 +388,18 @@ func (s *swapClientServer) LoopIn(ctx context.Context,
 
 	log.Infof("Loop in request received")
 
+	htlcConfTarget, err := validateLoopInRequest(
+		in.HtlcConfTarget, in.ExternalHtlc,
+	)
+	if err != nil {
+		return nil, err
+	}
+
 	req := &loop.LoopInRequest{
 		Amount:         btcutil.Amount(in.Amt),
 		MaxMinerFee:    btcutil.Amount(in.MaxMinerFee),
 		MaxSwapFee:     btcutil.Amount(in.MaxSwapFee),
-		HtlcConfTarget: defaultConfTarget,
+		HtlcConfTarget: htlcConfTarget,
 		ExternalHtlc:   in.ExternalHtlc,
 	}
 	if in.LastHop != nil {
@@ -477,6 +491,9 @@ func (s *swapClientServer) processStatusUpdates(mainCtx context.Context) {
 // isn't specified (0 value), then the default target is used.
 func validateConfTarget(target, defaultTarget int32) (int32, error) {
 	switch {
+	case target == 0:
+		return defaultTarget, nil
+
 	// Ensure the target respects our minimum threshold.
 	case target < minConfTarget:
 		return 0, fmt.Errorf("a confirmation target of at least %v "+
@@ -486,3 +503,22 @@ func validateConfTarget(target, defaultTarget int32) (int32, error) {
 		return target, nil
 	}
 }
+
+// validateLoopInRequest fails if the mutually exclusive conf target and
+// external parameters are both set.
+func validateLoopInRequest(htlcConfTarget int32, external bool) (int32, error) {
+	// If the htlc is going to be externally set, the htlcConfTarget should
+	// not be set, because it has no relevance when the htlc is external.
+	if external && htlcConfTarget != 0 {
+		return 0, errors.New("external and htlc conf target cannot " +
+			"both be set")
+	}
+
+	// If the htlc is being externally published, we do not need to set a
+	// confirmation target.
+	if external {
+		return 0, nil
+	}
+
+	return validateConfTarget(htlcConfTarget, loop.DefaultHtlcConfTarget)
+}

loopd/swapclient_server_test.go

@@ -0,0 +1,145 @@
+package loopd
+
+import (
+	"testing"
+
+	"github.com/lightninglabs/loop"
+)
+
+// TestValidateConfTarget tests all failure and success cases for our conf
+// target validation function, including the case where we replace a zero
+// target with the default provided.
+func TestValidateConfTarget(t *testing.T) {
+	const (
+		// Various input confirmation values for tests.
+		zeroConf int32 = 0
+		oneConf  int32 = 1
+		twoConf  int32 = 2
+		fiveConf int32 = 5
+
+		// defaultConf is the default confirmation target we use for
+		// all tests.
+		defaultConf = 6
+	)
+
+	tests := []struct {
+		name           string
+		confTarget     int32
+		expectedTarget int32
+		expectErr      bool
+	}{
+		{
+			name:           "zero conf, get default",
+			confTarget:     zeroConf,
+			expectedTarget: defaultConf,
+			expectErr:      false,
+		},
+		{
+			name:       "one conf, get error",
+			confTarget: oneConf,
+			expectErr:  true,
+		},
+		{
+			name:           "two conf, ok",
+			confTarget:     twoConf,
+			expectedTarget: twoConf,
+			expectErr:      false,
+		},
+		{
+			name:           "five conf, ok",
+			confTarget:     fiveConf,
+			expectedTarget: fiveConf,
+			expectErr:      false,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+
+		t.Run(test.name, func(t *testing.T) {
+			target, err := validateConfTarget(
+				test.confTarget, defaultConf,
+			)
+
+			haveErr := err != nil
+			if haveErr != test.expectErr {
+				t.Fatalf("expected err: %v, got: %v",
+					test.expectErr, err)
+			}
+
+			if target != test.expectedTarget {
+				t.Fatalf("expected: %v, got: %v",
+					test.expectedTarget, target)
+			}
+		})
+	}
+}
+
+// TestValidateLoopInRequest tests validation of loop in requests.
+func TestValidateLoopInRequest(t *testing.T) {
+	tests := []struct {
+		name           string
+		external       bool
+		confTarget     int32
+		expectErr      bool
+		expectedTarget int32
+	}{
+		{
+			name:           "external and htlc conf set",
+			external:       true,
+			confTarget:     1,
+			expectErr:      true,
+			expectedTarget: 0,
+		},
+		{
+			name:           "external and no conf",
+			external:       true,
+			confTarget:     0,
+			expectErr:      false,
+			expectedTarget: 0,
+		},
+		{
+			name:           "not external, zero conf",
+			external:       false,
+			confTarget:     0,
+			expectErr:      false,
+			expectedTarget: loop.DefaultHtlcConfTarget,
+		},
+		{
+			name:           "not external, bad conf",
+			external:       false,
+			confTarget:     1,
+			expectErr:      true,
+			expectedTarget: 0,
+		},
+		{
+			name:           "not external, ok conf",
+			external:       false,
+			confTarget:     5,
+			expectErr:      false,
+			expectedTarget: 5,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+
+		t.Run(test.name, func(t *testing.T) {
+			external := test.external
+			conf, err := validateLoopInRequest(
+				test.confTarget, external,
+			)
+
+			haveErr := err != nil
+			if haveErr != test.expectErr {
+				t.Fatalf("expected err: %v, got: %v",
+					test.expectErr, err)
+			}
+
+			if conf != test.expectedTarget {
+				t.Fatalf("expected: %v, got: %v",
+					test.expectedTarget, conf)
+			}
+		})
+	}
+}