Merge pull request #135 from joostjager/loopin-timeout-sig

loopin: fix handling of incorrect amount in external htlc tx

Author: joostjager

client_test.go

@@ -265,6 +265,9 @@ func testSuccess(ctx *testContext, amt btcutil.Amount, hash lntypes.Hash,
 	// Publish tick.
 	ctx.expiryChan <- testTime
 
+	// Expect a signing request.
+	<-ctx.Lnd.SignOutputRawChannel
+
 	if !preimageRevealed {
 		ctx.assertStatus(loopdb.StatePreimageRevealed)
 		ctx.assertStorePreimageReveal()

loopin.go

@@ -419,7 +419,7 @@ func (s *loopInSwap) publishOnChainHtlc(ctx context.Context) (bool, error) {
 // the swap invoice is either settled or canceled. If the htlc times out, the
 // timeout tx will be published.
 func (s *loopInSwap) waitForSwapComplete(ctx context.Context,
-	htlc *wire.OutPoint, htlcValue btcutil.Amount) error {
+	htlcOutpoint *wire.OutPoint, htlcValue btcutil.Amount) error {
 
 	// Register the htlc spend notification.
 	rpcCtx, cancel := context.WithCancel(ctx)
@@ -445,7 +445,7 @@ func (s *loopInSwap) waitForSwapComplete(ctx context.Context,
 	// checkTimeout publishes the timeout tx if the contract has expired.
 	checkTimeout := func() error {
 		if s.height >= s.LoopInContract.CltvExpiry {
-			return s.publishTimeoutTx(ctx, htlc)
+			return s.publishTimeoutTx(ctx, htlcOutpoint, htlcValue)
 		}
 
 		return nil
@@ -572,7 +572,7 @@ func (s *loopInSwap) processHtlcSpend(ctx context.Context,
 // publishTimeoutTx publishes a timeout tx after the on-chain htlc has expired.
 // The swap failed and we are reclaiming our funds.
 func (s *loopInSwap) publishTimeoutTx(ctx context.Context,
-	htlc *wire.OutPoint) error {
+	htlcOutpoint *wire.OutPoint, htlcValue btcutil.Amount) error {
 
 	if s.timeoutAddr == nil {
 		var err error
@@ -596,8 +596,8 @@ func (s *loopInSwap) publishTimeoutTx(ctx context.Context,
 	}
 
 	timeoutTx, err := s.sweeper.CreateSweepTx(
-		ctx, s.height, s.htlc, *htlc, s.SenderKey, witnessFunc,
-		s.LoopInContract.AmountRequested, fee, s.timeoutAddr,
+		ctx, s.height, s.htlc, *htlcOutpoint, s.SenderKey, witnessFunc,
+		htlcValue, fee, s.timeoutAddr,
 	)
 	if err != nil {
 		return err

loopin_test.go

@@ -113,9 +113,25 @@ func TestLoopInSuccess(t *testing.T) {
 	}
 }
 
-// TestLoopInTimeout tests the scenario where the server doesn't sweep the htlc
+// TestLoopInTimeout tests scenarios where the server doesn't sweep the htlc
 // and the client is forced to reclaim the funds using the timeout tx.
 func TestLoopInTimeout(t *testing.T) {
+	testAmt := int64(testLoopInRequest.Amount)
+	t.Run("internal htlc", func(t *testing.T) {
+		testLoopInTimeout(t, 0)
+	})
+	t.Run("external htlc", func(t *testing.T) {
+		testLoopInTimeout(t, testAmt)
+	})
+	t.Run("external amount too high", func(t *testing.T) {
+		testLoopInTimeout(t, testAmt+1)
+	})
+	t.Run("external amount too low", func(t *testing.T) {
+		testLoopInTimeout(t, testAmt-1)
+	})
+}
+
+func testLoopInTimeout(t *testing.T, externalValue int64) {
 	defer test.Guard(t)()
 
 	ctx := newLoopInTestContext(t)
@@ -128,9 +144,14 @@ func TestLoopInTimeout(t *testing.T) {
 		server: ctx.server,
 	}
 
+	req := testLoopInRequest
+	if externalValue != 0 {
+		req.ExternalHtlc = true
+	}
+
 	swap, err := newLoopInSwap(
 		context.Background(), cfg,
-		height, &testLoopInRequest,
+		height, &req,
 	)
 	if err != nil {
 		t.Fatal(err)
@@ -152,8 +173,21 @@ func TestLoopInTimeout(t *testing.T) {
 	ctx.assertState(loopdb.StateHtlcPublished)
 	ctx.store.assertLoopInState(loopdb.StateHtlcPublished)
 
-	// Expect htlc to be published.
-	htlcTx := <-ctx.lnd.SendOutputsChannel
+	var htlcTx wire.MsgTx
+	if externalValue == 0 {
+		// Expect htlc to be published.
+		htlcTx = <-ctx.lnd.SendOutputsChannel
+	} else {
+		// Create an external htlc publish tx.
+		htlcTx = wire.MsgTx{
+			TxOut: []*wire.TxOut{
+				{
+					PkScript: swap.htlc.PkScript,
+					Value:    externalValue,
+				},
+			},
+		}
+	}
 
 	// Expect register for htlc conf.
 	<-ctx.lnd.RegisterConfChannel
@@ -175,6 +209,13 @@ func TestLoopInTimeout(t *testing.T) {
 	// Let htlc expire.
 	ctx.blockEpochChan <- swap.LoopInContract.CltvExpiry
 
+	// Expect a signing request for the htlc tx output value.
+	signReq := <-ctx.lnd.SignOutputRawChannel
+	if signReq.SignDescriptors[0].Output.Value != htlcTx.TxOut[0].Value {
+
+		t.Fatal("invalid signing amount")
+	}
+
 	// Expect timeout tx to be published.
 	timeoutTx := <-ctx.lnd.TxPublishChannel
 

loopout_test.go

@@ -192,6 +192,9 @@ func TestCustomSweepConfTarget(t *testing.T) {
 
 	expiryChan <- time.Now()
 
+	// Expect a signing request for the HTLC success transaction.
+	<-ctx.Lnd.SignOutputRawChannel
+
 	cfg.store.(*storeMock).assertLoopOutState(loopdb.StatePreimageRevealed)
 	status := <-statusChan
 	if status.State != loopdb.StatePreimageRevealed {
@@ -247,6 +250,9 @@ func TestCustomSweepConfTarget(t *testing.T) {
 	blockEpochChan <- int32(defaultConfTargetHeight)
 	expiryChan <- time.Now()
 
+	// Expect another signing request.
+	<-ctx.Lnd.SignOutputRawChannel
+
 	// We should expect to see another sweep using the higher fee since the
 	// spend hasn't been confirmed yet.
 	sweepTx := assertSweepTx(DefaultSweepConfTarget)

test/lnd_services_mock.go

@@ -9,6 +9,7 @@ import (
 	"github.com/btcsuite/btcd/wire"
 	"github.com/lightninglabs/loop/lndclient"
 	"github.com/lightningnetwork/lnd/chainntnfs"
+	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/lightningnetwork/lnd/zpay32"
@@ -57,6 +58,8 @@ func NewMockLnd() *LndMockServices {
 		RouterSendPaymentChannel: make(chan RouterPaymentChannelMessage),
 		TrackPaymentChannel:      make(chan TrackPaymentMessage),
 
+		SignOutputRawChannel: make(chan SignOutputRawRequest),
+
 		FailInvoiceChannel: make(chan lntypes.Hash, 2),
 		epochChannel:       make(chan int32),
 		Height:             testStartingHeight,
@@ -109,6 +112,12 @@ type SingleInvoiceSubscription struct {
 	Err    chan error
 }
 
+// SignOutputRawRequest contains input data for a tx signing request.
+type SignOutputRawRequest struct {
+	Tx              *wire.MsgTx
+	SignDescriptors []*input.SignDescriptor
+}
+
 // LndMockServices provides a full set of mocked lnd services.
 type LndMockServices struct {
 	lndclient.LndServices
@@ -130,6 +139,8 @@ type LndMockServices struct {
 	RouterSendPaymentChannel chan RouterPaymentChannelMessage
 	TrackPaymentChannel      chan TrackPaymentMessage
 
+	SignOutputRawChannel chan SignOutputRawRequest
+
 	Height       int32
 	NodePubkey   string
 	Signature    []byte

test/signer_mock.go

@@ -16,6 +16,11 @@ type mockSigner struct {
 func (s *mockSigner) SignOutputRaw(ctx context.Context, tx *wire.MsgTx,
 	signDescriptors []*input.SignDescriptor) ([][]byte, error) {
 
+	s.lnd.SignOutputRawChannel <- SignOutputRawRequest{
+		Tx:              tx,
+		SignDescriptors: signDescriptors,
+	}
+
 	rawSigs := [][]byte{{1, 2, 3}}
 
 	return rawSigs, nil