Merge pull request #372 from carlaKC/sweep-abandon

loopout: do not reveal preimage too close to expiry

loopout.go

@@ -438,6 +438,12 @@ func (s *loopOutSwap) executeSwap(globalCtx context.Context) error {
 		return err
 	}
 
+	// If spend details are nil, we resolved the swap without waiting for
+	// its spend, so we can exit.
+	if spendDetails == nil {
+		return nil
+	}
+
 	// Inspect witness stack to see if it is a success transaction. We
 	// don't just try to match with the hash of our sweep tx, because it
 	// may be swept by a different (fee) sweep tx from a previous run.
@@ -854,6 +860,14 @@ func (s *loopOutSwap) waitForHtlcSpendConfirmed(globalCtx context.Context,
 				return nil, err
 			}
 
+			// If the result of our spend func was that the swap
+			// has reached a final state, then we return nil spend
+			// details, because there is no further action required
+			// for this swap.
+			if s.state.Type() != loopdb.StateTypePending {
+				return nil, nil
+			}
+
 			// If our off chain payment is not yet complete, we
 			// try to push our preimage to the server.
 			if !paymentComplete {
@@ -889,7 +903,9 @@ func (s *loopOutSwap) pushPreimage(ctx context.Context) {
 
 // sweep tries to sweep the given htlc to a destination address. It takes into
 // account the max miner fee and marks the preimage as revealed when it
-// published the tx.
+// published the tx. If the preimage has not yet been revealed, and the time
+// during which we can safely reveal it has passed, the swap will be marked
+// as failed, and the function will return.
 //
 // TODO: Use lnd sweeper?
 func (s *loopOutSwap) sweep(ctx context.Context,
@@ -900,16 +916,36 @@ func (s *loopOutSwap) sweep(ctx context.Context,
 		return s.htlc.GenSuccessWitness(sig, s.Preimage)
 	}
 
+	remainingBlocks := s.CltvExpiry - s.height
+	blocksToLastReveal := remainingBlocks - MinLoopOutPreimageRevealDelta
+	preimageRevealed := s.state == loopdb.StatePreimageRevealed
+
+	// If we have not revealed our preimage, and we don't have time left
+	// to sweep the swap, we abandon the swap because we can no longer
+	// sweep the success path (without potentially having to compete with
+	// the server's timeout sweep), and we have not had any coins pulled
+	// off-chain.
+	if blocksToLastReveal <= 0 && !preimageRevealed {
+		s.log.Infof("Preimage can no longer be safely revealed: "+
+			"expires at: %v, current height: %v", s.CltvExpiry,
+			s.height)
+
+		s.state = loopdb.StateFailTimeout
+		return nil
+	}
+
 	// Calculate the transaction fee based on the confirmation target
 	// required to sweep the HTLC before the timeout. We'll use the
 	// confirmation target provided by the client unless we've come too
 	// close to the expiration height, in which case we'll use the default
 	// if it is better than what the client provided.
 	confTarget := s.SweepConfTarget
-	if s.CltvExpiry-s.height <= DefaultSweepConfTargetDelta &&
+	if remainingBlocks <= DefaultSweepConfTargetDelta &&
 		confTarget > DefaultSweepConfTarget {
+
 		confTarget = DefaultSweepConfTarget
 	}
+
 	fee, err := s.sweeper.GetSweepFee(
 		ctx, s.htlc.AddSuccessToEstimator, s.DestAddr, confTarget,
 	)
@@ -922,7 +958,7 @@ func (s *loopOutSwap) sweep(ctx context.Context,
 		s.log.Warnf("Required fee %v exceeds max miner fee of %v",
 			fee, s.MaxMinerFee)
 
-		if s.state == loopdb.StatePreimageRevealed {
+		if preimageRevealed {
 			// The currently required fee exceeds the max, but we
 			// already revealed the preimage. The best we can do now
 			// is to republish with the max fee.

loopout_test.go

@@ -414,11 +414,7 @@ func TestCustomSweepConfTarget(t *testing.T) {
 // not detected our settle) and settle the off chain htlc, indicating that the
 // server successfully settled using the preimage push. In this test, we need
 // to start with a fee rate that will be too high, then progress to an
-// acceptable one. We do this by starting with a high confirmation target with
-// a high fee, and setting the default confirmation fee (which our swap will
-// drop down to if it is not confirming in time) to a lower fee. This is not
-// intuitive (lower confs having lower fees), but it allows up to mock fee
-// changes.
+// acceptable one.
 func TestPreimagePush(t *testing.T) {
 	defer test.Guard(t)()
 
@@ -426,11 +422,8 @@ func TestPreimagePush(t *testing.T) {
 	ctx := test.NewContext(t, lnd)
 	server := newServerMock(lnd)
 
-	// Start with a high confirmation delta which will have a very high fee
-	// attached to it.
 	testReq := *testRequest
-	testReq.SweepConfTarget = testLoopOutMinOnChainCltvDelta -
-		DefaultSweepConfTargetDelta - 1
+	testReq.SweepConfTarget = 10
 	testReq.Expiry = ctx.Lnd.Height + testLoopOutMinOnChainCltvDelta
 
 	// We set our mock fee estimate for our target sweep confs to be our
@@ -442,11 +435,6 @@ func TestPreimagePush(t *testing.T) {
 		),
 	)
 
-	// We set the fee estimate for our default confirmation target very
-	// low, so that once we drop down to our default confs we will start
-	// trying to sweep the preimage.
-	ctx.Lnd.SetFeeEstimate(DefaultSweepConfTarget, 1)
-
 	cfg := newSwapConfig(
 		&lnd.LndServices, newStoreMock(t), server,
 	)
@@ -520,15 +508,15 @@ func TestPreimagePush(t *testing.T) {
 	// preimage is not revealed, we also do not expect a preimage push.
 	expiryChan <- testTime
 
-	// Now, we notify the height at which the client will start using the
-	// default confirmation target. This has the effect of lowering our fees
-	// so that the client still start sweeping.
-	defaultConfTargetHeight := ctx.Lnd.Height + testLoopOutMinOnChainCltvDelta -
-		DefaultSweepConfTargetDelta
-	blockEpochChan <- defaultConfTargetHeight
+	// Now we decrease our fees for the swap's confirmation target to less
+	// than the maximum miner fee.
+	ctx.Lnd.SetFeeEstimate(testReq.SweepConfTarget, chainfee.SatPerKWeight(
+		testReq.MaxMinerFee/2,
+	))
 
-	// This time when we tick the expiry chan, our fees are lower than the
-	// swap max, so we expect it to prompt a sweep.
+	// Now when we report a new block and tick our expiry fee timer, and
+	// fees are acceptably low so we expect our sweep to be published.
+	blockEpochChan <- ctx.Lnd.Height + 2
 	expiryChan <- testTime
 
 	// Expect a signing request for the HTLC success transaction.
@@ -593,3 +581,119 @@ func TestPreimagePush(t *testing.T) {
 
 	require.NoError(t, <-errChan)
 }
+
+// TestExpiryBeforeReveal tests the case where the on-chain HTLC expires before
+// we have revealed our preimage, demonstrating that we do not reveal our
+// preimage once we've reached our expiry height.
+func TestExpiryBeforeReveal(t *testing.T) {
+	defer test.Guard(t)()
+
+	lnd := test.NewMockLnd()
+	ctx := test.NewContext(t, lnd)
+	server := newServerMock(lnd)
+
+	testReq := *testRequest
+
+	// Set on-chain HTLC CLTV.
+	testReq.Expiry = ctx.Lnd.Height + testLoopOutMinOnChainCltvDelta
+
+	// Set our fee estimate to higher than our max miner fee will allow.
+	lnd.SetFeeEstimate(testReq.SweepConfTarget, chainfee.SatPerKWeight(
+		testReq.MaxMinerFee*2,
+	))
+
+	// Setup the cfg using mock server and init a loop out request.
+	cfg := newSwapConfig(
+		&lnd.LndServices, newStoreMock(t), server,
+	)
+	initResult, err := newLoopOutSwap(
+		context.Background(), cfg, ctx.Lnd.Height, &testReq,
+	)
+	require.NoError(t, err)
+	swap := initResult.swap
+
+	// Set up the required dependencies to execute the swap.
+	sweeper := &sweep.Sweeper{Lnd: &lnd.LndServices}
+	blockEpochChan := make(chan interface{})
+	statusChan := make(chan SwapInfo)
+	expiryChan := make(chan time.Time)
+	timerFactory := func(_ time.Duration) <-chan time.Time {
+		return expiryChan
+	}
+
+	errChan := make(chan error)
+	go func() {
+		err := swap.execute(context.Background(), &executeConfig{
+			statusChan:     statusChan,
+			blockEpochChan: blockEpochChan,
+			timerFactory:   timerFactory,
+			sweeper:        sweeper,
+		}, ctx.Lnd.Height)
+		if err != nil {
+			log.Error(err)
+		}
+		errChan <- err
+	}()
+
+	// The swap should be found in its initial state.
+	cfg.store.(*storeMock).assertLoopOutStored()
+	state := <-statusChan
+	require.Equal(t, loopdb.StateInitiated, state.State)
+
+	// We'll then pay both the swap and prepay invoice, which should trigger
+	// the server to publish the on-chain HTLC.
+	signalSwapPaymentResult := ctx.AssertPaid(swapInvoiceDesc)
+	signalPrepaymentResult := ctx.AssertPaid(prepayInvoiceDesc)
+
+	signalSwapPaymentResult(nil)
+	signalPrepaymentResult(nil)
+
+	// Notify the confirmation notification for the HTLC.
+	ctx.AssertRegisterConf(false, defaultConfirmations)
+
+	// Advance the block height to get the HTLC confirmed.
+	height := ctx.Lnd.Height + 1
+	blockEpochChan <- height
+
+	htlcTx := wire.NewMsgTx(2)
+	htlcTx.AddTxOut(&wire.TxOut{
+		Value:    int64(swap.AmountRequested),
+		PkScript: swap.htlc.PkScript,
+	})
+	ctx.NotifyConf(htlcTx)
+
+	// The client should then register for a spend of the HTLC and attempt
+	// to sweep it using the custom confirmation target.
+	ctx.AssertRegisterSpendNtfn(swap.htlc.PkScript)
+
+	// Assert that we made a query to track our payment, as required for
+	// preimage push tracking.
+	ctx.AssertTrackPayment()
+
+	// Tick the expiry channel. Because our max miner fee is too high, we
+	// won't attempt a sweep at this point.
+	expiryChan <- testTime
+
+	// Now we decrease our conf target to less than our max miner fee.
+	lnd.SetFeeEstimate(testReq.SweepConfTarget, chainfee.SatPerKWeight(
+		testReq.MaxMinerFee/2,
+	))
+
+	// Advance the block height to the point where we would do timeout
+	// instead of pushing the preimage.
+	blockEpochChan <- testReq.Expiry + height
+
+	// Tick our expiry channel again to trigger another sweep attempt.
+	expiryChan <- testTime
+
+	// We should see our swap marked as failed.
+	cfg.store.(*storeMock).assertLoopOutState(
+		loopdb.StateFailTimeout,
+	)
+	status := <-statusChan
+	require.Equal(
+		t, status.State, loopdb.StateFailTimeout,
+	)
+
+	require.Nil(t, <-errChan)
+}

test/lnd_services_mock.go

@@ -258,5 +258,5 @@ func (s *LndMockServices) DecodeInvoice(request string) (*zpay32.Invoice,
 func (s *LndMockServices) SetFeeEstimate(confTarget int32,
 	feeEstimate chainfee.SatPerKWeight) {
 
-	s.WalletKit.(*mockWalletKit).feeEstimates[confTarget] = feeEstimate
+	s.WalletKit.(*mockWalletKit).setFeeEstimate(confTarget, feeEstimate)
 }

test/walletkit_mock.go

@@ -3,6 +3,7 @@ package test
 import (
 	"context"
 	"errors"
+	"sync"
 	"time"
 
 	"github.com/btcsuite/btcd/chaincfg"
@@ -21,9 +22,11 @@ import (
 var DefaultMockFee = chainfee.SatPerKWeight(10000)
 
 type mockWalletKit struct {
-	lnd          *LndMockServices
-	keyIndex     int32
-	feeEstimates map[int32]chainfee.SatPerKWeight
+	lnd      *LndMockServices
+	keyIndex int32
+
+	feeEstimateLock sync.Mutex
+	feeEstimates    map[int32]chainfee.SatPerKWeight
 }
 
 var _ lndclient.WalletKitClient = (*mockWalletKit)(nil)
@@ -118,9 +121,19 @@ func (m *mockWalletKit) SendOutputs(ctx context.Context, outputs []*wire.TxOut,
 	return &tx, nil
 }
 
+func (m *mockWalletKit) setFeeEstimate(confTarget int32, fee chainfee.SatPerKWeight) {
+	m.feeEstimateLock.Lock()
+	defer m.feeEstimateLock.Unlock()
+
+	m.feeEstimates[confTarget] = fee
+}
+
 func (m *mockWalletKit) EstimateFee(ctx context.Context, confTarget int32) (
 	chainfee.SatPerKWeight, error) {
 
+	m.feeEstimateLock.Lock()
+	defer m.feeEstimateLock.Unlock()
+
 	if confTarget <= 1 {
 		return 0, errors.New("conf target must be greater than 1")
 	}