From a05e46697d54e5bb62b314cbd7b5dd57a7e3323f Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 15 Jan 2020 09:55:22 +0100 Subject: [PATCH 1/3] test: assert raw output signing requests --- client_test.go | 3 +++ loopin_test.go | 3 +++ loopout_test.go | 6 ++++++ test/lnd_services_mock.go | 11 +++++++++++ test/signer_mock.go | 5 +++++ 5 files changed, 28 insertions(+) diff --git a/client_test.go b/client_test.go index 94195213..16750097 100644 --- a/client_test.go +++ b/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() diff --git a/loopin_test.go b/loopin_test.go index 7ba1385d..1ffae363 100644 --- a/loopin_test.go +++ b/loopin_test.go @@ -175,6 +175,9 @@ func TestLoopInTimeout(t *testing.T) { // Let htlc expire. ctx.blockEpochChan <- swap.LoopInContract.CltvExpiry + // Expect a signing request. + <-ctx.lnd.SignOutputRawChannel + // Expect timeout tx to be published. timeoutTx := <-ctx.lnd.TxPublishChannel diff --git a/loopout_test.go b/loopout_test.go index 77e5259a..af56c52a 100644 --- a/loopout_test.go +++ b/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) diff --git a/test/lnd_services_mock.go b/test/lnd_services_mock.go index 0687bb6b..3e2540a7 100644 --- a/test/lnd_services_mock.go +++ b/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 diff --git a/test/signer_mock.go b/test/signer_mock.go index e0d008f8..992da787 100644 --- a/test/signer_mock.go +++ b/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 From edac1a4054798f1d0dad84157d0e720b152d4222 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 15 Jan 2020 10:07:26 +0100 Subject: [PATCH 2/3] loopin/test: extend test with external htlcs --- loopin_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/loopin_test.go b/loopin_test.go index 1ffae363..b1b94b33 100644 --- a/loopin_test.go +++ b/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,8 +209,15 @@ func TestLoopInTimeout(t *testing.T) { // Let htlc expire. ctx.blockEpochChan <- swap.LoopInContract.CltvExpiry - // Expect a signing request. - <-ctx.lnd.SignOutputRawChannel + // Expect a signing request for the htlc tx output value. + // + // TODO(joostjager): FIX BUG WHERE WE ALWAYS SIGN FOR THE HTLC AMOUNT. + signReq := <-ctx.lnd.SignOutputRawChannel + if signReq.SignDescriptors[0].Output.Value != + int64(testLoopInRequest.Amount) { + + t.Fatal("invalid signing amount") + } // Expect timeout tx to be published. timeoutTx := <-ctx.lnd.TxPublishChannel From 9aaef3fd07b32bf3e006e66359b88b0f5138cf75 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 15 Jan 2020 09:17:01 +0100 Subject: [PATCH 3/3] loopin: use discovered htlc value for timeout tx --- loopin.go | 10 +++++----- loopin_test.go | 5 +---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/loopin.go b/loopin.go index c7632f4c..f2f98468 100644 --- a/loopin.go +++ b/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 diff --git a/loopin_test.go b/loopin_test.go index b1b94b33..f74ac756 100644 --- a/loopin_test.go +++ b/loopin_test.go @@ -210,11 +210,8 @@ func testLoopInTimeout(t *testing.T, externalValue int64) { ctx.blockEpochChan <- swap.LoopInContract.CltvExpiry // Expect a signing request for the htlc tx output value. - // - // TODO(joostjager): FIX BUG WHERE WE ALWAYS SIGN FOR THE HTLC AMOUNT. signReq := <-ctx.lnd.SignOutputRawChannel - if signReq.SignDescriptors[0].Output.Value != - int64(testLoopInRequest.Amount) { + if signReq.SignDescriptors[0].Output.Value != htlcTx.TxOut[0].Value { t.Fatal("invalid signing amount") }