Merge pull request #9221 from yyforyongyu/fix-TestChannelLinkCancelFullCommitment

htlcswitch: fix flake in `TestChannelLinkCancelFullCommitment`

htlcswitch/link.go

@@ -3793,7 +3793,8 @@ func (l *channelLink) processExitHop(add lnwire.UpdateAddHTLC,
 	// ADD, nor will we settle the corresponding invoice or respond with the
 	// preimage.
 	if l.cfg.HodlMask.Active(hodl.ExitSettle) {
-		l.log.Warnf(hodl.ExitSettle.Warning())
+		l.log.Warnf("%s for htlc(rhash=%x,htlcIndex=%v)",
+			hodl.ExitSettle.Warning(), add.PaymentHash, add.ID)
 
 		return nil
 	}

htlcswitch/link_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/channeldb/models"
 	"github.com/lightningnetwork/lnd/contractcourt"
+	"github.com/lightningnetwork/lnd/fn"
 	"github.com/lightningnetwork/lnd/htlcswitch/hodl"
 	"github.com/lightningnetwork/lnd/htlcswitch/hop"
 	"github.com/lightningnetwork/lnd/input"
@@ -864,7 +865,7 @@ func TestChannelLinkCancelFullCommitment(t *testing.T) {
 	)
 
 	// Fill up the commitment from Alice's side with 20 sat payments.
-	count := (input.MaxHTLCNumber / 2)
+	count := int(maxInflightHtlcs)
 	amt := lnwire.NewMSatFromSatoshis(20000)
 
 	htlcAmt, totalTimelock, hopsForwards := generateHops(amt,
@@ -902,17 +903,17 @@ func TestChannelLinkCancelFullCommitment(t *testing.T) {
 
 	// Now make an additional payment from Alice to Bob, this should be
 	// canceled because the commitment in this direction is full.
-	err = <-makePayment(
+	resp := makePayment(
 		n.aliceServer, n.bobServer, firstHop, hopsForwards, amt,
 		htlcAmt, totalTimelock,
-	).err
+	)
-	if err == nil {
+
-		t.Fatalf("overflow payment should have failed")
+	paymentErr, timeoutErr := fn.RecvOrTimeout(resp.err, 30*time.Second)
-	}
+	require.NoError(t, timeoutErr, "timeout receiving payment resp")
-	lerr, ok := err.(*LinkError)
+	require.Error(t, paymentErr, "overflow payment should have failed")
-	if !ok {
+
-		t.Fatalf("expected LinkError, got: %T", err)
+	var lerr *LinkError
-	}
+	require.ErrorAs(t, paymentErr, &lerr)
 
 	msg := lerr.WireMessage()
 	if _, ok := msg.(*lnwire.FailTemporaryChannelFailure); !ok {
@@ -938,10 +939,9 @@ func TestChannelLinkCancelFullCommitment(t *testing.T) {
 
 	// Ensure that all of the payments sent by alice eventually succeed.
 	for errChan := range aliceErrChan {
-		err := <-errChan
+		receivedErr, err := fn.RecvOrTimeout(errChan, 30*time.Second)
-		if err != nil {
+		require.NoError(t, err, "payment timeout")
-			t.Fatalf("alice payment failed: %v", err)
+		require.NoError(t, receivedErr, "alice payment failed")
-		}
 	}
 }
 

htlcswitch/switch_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/lightningnetwork/lnd/htlcswitch/hodl"
 	"github.com/lightningnetwork/lnd/htlcswitch/hop"
 	"github.com/lightningnetwork/lnd/lntest/mock"
+	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/lightningnetwork/lnd/lnwire"
@@ -4289,14 +4290,19 @@ func TestSwitchDustForwarding(t *testing.T) {
 
 	// We'll test that once the default threshold is exceeded on the
 	// Alice -> Bob channel, either side's calls to SendHTLC will fail.
-	//
+	numHTLCs := maxInflightHtlcs
-	// Alice will send 354 HTLC's of 700sats. Bob will also send 354 HTLC's
-	// of 700sats.
-	numHTLCs := 354
 	aliceAttemptID, bobAttemptID := numHTLCs, numHTLCs
 	amt := lnwire.NewMSatFromSatoshis(700)
 	aliceBobFirstHop := n.aliceChannelLink.ShortChanID()
 
+	// We decreased the max number of inflight HTLCs therefore we also need
+	// do decrease the max fee exposure.
+	maxFeeExposure := lnwire.NewMSatFromSatoshis(74500)
+	n.aliceChannelLink.cfg.MaxFeeExposure = maxFeeExposure
+	n.firstBobChannelLink.cfg.MaxFeeExposure = maxFeeExposure
+
+	// Alice will send 50 HTLC's of 700sats. Bob will also send 50 HTLC's
+	// of 700sats.
 	sendDustHtlcs(t, n, true, amt, aliceBobFirstHop, numHTLCs)
 	sendDustHtlcs(t, n, false, amt, aliceBobFirstHop, numHTLCs)
 
@@ -4318,22 +4324,13 @@ func TestSwitchDustForwarding(t *testing.T) {
 		OnionBlob:   blob,
 	}
 
-	checkAlmostDust := func(link *channelLink, mbox MailBox,
+	// This is the expected dust without taking the commitfee into account.
-		whoseCommit lntypes.ChannelParty) bool {
+	expectedDust := maxInflightHtlcs * 2 * amt
 
-		timeout := time.After(15 * time.Second)
+	assertAlmostDust := func(link *channelLink, mbox MailBox,
-		pollInterval := 300 * time.Millisecond
+		whoseCommit lntypes.ChannelParty) {
-		expectedDust := 354 * 2 * amt
-
-		for {
-			<-time.After(pollInterval)
-
-			select {
-			case <-timeout:
-				return false
-			default:
-			}
 
+		err := wait.NoError(func() error {
 			linkDust := link.getDustSum(
 				whoseCommit, fn.None[chainfee.SatPerKWeight](),
 			)
@@ -4347,11 +4344,13 @@ func TestSwitchDustForwarding(t *testing.T) {
 			}
 
 			if totalDust == expectedDust {
-				break
+				return nil
 			}
-		}
 
-		return true
+			return fmt.Errorf("got totalDust=%v, expectedDust=%v",
+				totalDust, expectedDust)
+		}, 15*time.Second)
+		require.NoError(t, err, "timeout checking dust")
 	}
 
 	// Wait until Bob is almost at the fee threshold.
@@ -4359,14 +4358,15 @@ func TestSwitchDustForwarding(t *testing.T) {
 		n.firstBobChannelLink.ChanID(),
 		n.firstBobChannelLink.ShortChanID(),
 	)
-	require.True(
+	assertAlmostDust(n.firstBobChannelLink, bobMbox, lntypes.Local)
-		t, checkAlmostDust(
-			n.firstBobChannelLink, bobMbox, lntypes.Local,
-		),
-	)
 
 	// Sending one more HTLC should fail. SendHTLC won't error, but the
-	// HTLC should be failed backwards.
+	// HTLC should be failed backwards. When sending we only check for the
+	// dust amount without the commitment fee. When the HTLC is added to the
+	// commitment state (link) we also take into account the commitment fee
+	// and with a fee of 6000 sat/kw and a commitment size of 724 (non
+	// anchor channel) we are overexposed in fees (maxFeeExposure) that's
+	// why the HTLC is failed back.
 	err = n.bobServer.htlcSwitch.SendHTLC(
 		aliceBobFirstHop, uint64(bobAttemptID), failingHtlc,
 	)
@@ -4412,9 +4412,7 @@ func TestSwitchDustForwarding(t *testing.T) {
 		aliceBobFirstHop, uint64(bobAttemptID), nondustHtlc,
 	)
 	require.NoError(t, err)
-	require.True(t, checkAlmostDust(
+	assertAlmostDust(n.firstBobChannelLink, bobMbox, lntypes.Local)
-		n.firstBobChannelLink, bobMbox, lntypes.Local,
-	))
 
 	// Check that the HTLC failed.
 	bobResultChan, err = n.bobServer.htlcSwitch.GetAttemptResult(
@@ -4492,11 +4490,7 @@ func TestSwitchDustForwarding(t *testing.T) {
 	aliceMbox := aliceOrch.GetOrCreateMailBox(
 		n.aliceChannelLink.ChanID(), n.aliceChannelLink.ShortChanID(),
 	)
-	require.True(
+	assertAlmostDust(n.aliceChannelLink, aliceMbox, lntypes.Remote)
-		t, checkAlmostDust(
-			n.aliceChannelLink, aliceMbox, lntypes.Remote,
-		),
-	)
 
 	err = n.aliceServer.htlcSwitch.SendHTLC(
 		n.aliceChannelLink.ShortChanID(), uint64(aliceAttemptID),

htlcswitch/test_utils.go

@@ -43,6 +43,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// maxInflightHtlcs specifies the max number of inflight HTLCs. This number is
+// chosen to be smaller than the default 483 so the test can run faster.
+const maxInflightHtlcs = 50
+
 var (
 	alicePrivKey = []byte("alice priv key")
 	bobPrivKey   = []byte("bob priv key")
@@ -145,7 +149,7 @@ func createTestChannel(t *testing.T, alicePrivKey, bobPrivKey []byte,
 			channelCapacity),
 		ChanReserve:      aliceReserve,
 		MinHTLC:          0,
-		MaxAcceptedHtlcs: input.MaxHTLCNumber / 2,
+		MaxAcceptedHtlcs: maxInflightHtlcs,
 	}
 	aliceCommitParams := channeldb.CommitmentParams{
 		DustLimit: btcutil.Amount(200),
@@ -157,7 +161,7 @@ func createTestChannel(t *testing.T, alicePrivKey, bobPrivKey []byte,
 			channelCapacity),
 		ChanReserve:      bobReserve,
 		MinHTLC:          0,
-		MaxAcceptedHtlcs: input.MaxHTLCNumber / 2,
+		MaxAcceptedHtlcs: maxInflightHtlcs,
 	}
 	bobCommitParams := channeldb.CommitmentParams{
 		DustLimit: btcutil.Amount(800),