From ab9a662758eb2abc46fa9d5ac43a257dd78efd8a Mon Sep 17 00:00:00 2001
From: carla <kirkcohenc@gmail.com>
Date: Thu, 22 Apr 2021 09:28:32 +0200
Subject: [PATCH] loop/test: simplify preimage push test to be less dependent
 on height

Our preimage push test previously relied on our dropping down to the
default sweep conf target to mock a drop in chain fees. This makes
our test dependent on height, which makes changes to our sweep logic
regarding when we reveal our preimage break this test. In this commit
that logic is replaced with simply locking our mock and updating fees
on the fly.
---
 loopout_test.go           | 32 ++++++++++----------------------
 test/lnd_services_mock.go |  2 +-
 test/walletkit_mock.go    | 19 ++++++++++++++++---
 3 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/loopout_test.go b/loopout_test.go
index 0e72933..a8faaf8 100644
--- a/loopout_test.go
+++ b/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.
diff --git a/test/lnd_services_mock.go b/test/lnd_services_mock.go
index 7e162a2..7d3466c 100644
--- a/test/lnd_services_mock.go
+++ b/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)
 }
diff --git a/test/walletkit_mock.go b/test/walletkit_mock.go
index b961162..19738a8 100644
--- a/test/walletkit_mock.go
+++ b/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")
 	}