From 8311bc56e584697b57ba5c9fc6fc6871a123c6f4 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Fri, 25 Oct 2024 23:24:24 +0800 Subject: [PATCH] htlcswitch: fix flake in `TestChannelLinkCancelFullCommitment` We bring down the max number of inflight HTLCs in the link's unit tests to speed up the tests. --- htlcswitch/link_test.go | 28 ++++++++++---------- htlcswitch/switch_test.go | 54 +++++++++++++-------------------------- htlcswitch/test_utils.go | 8 ++++-- 3 files changed, 38 insertions(+), 52 deletions(-) diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 5bfb79ee05..78bb99d04e 100644 --- a/htlcswitch/link_test.go +++ b/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") - } - lerr, ok := err.(*LinkError) - if !ok { - t.Fatalf("expected LinkError, got: %T", err) - } + ) + + paymentErr, timeoutErr := fn.RecvOrTimeout(resp.err, 30*time.Second) + require.NoError(t, timeoutErr, "timeout receiving payment resp") + require.Error(t, paymentErr, "overflow payment should have failed") + + 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 - if err != nil { - t.Fatalf("alice payment failed: %v", err) - } + receivedErr, err := fn.RecvOrTimeout(errChan, 30*time.Second) + require.NoError(t, err, "payment timeout") + require.NoError(t, receivedErr, "alice payment failed") } } diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 825ee6c652..ae5275a995 100644 --- a/htlcswitch/switch_test.go +++ b/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,16 +4290,15 @@ 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. - // - // Alice will send 354 HTLC's of 700sats. Bob will also send 354 HTLC's - // of 700sats. - numHTLCs := 354 + numHTLCs := maxInflightHtlcs aliceAttemptID, bobAttemptID := numHTLCs, numHTLCs amt := lnwire.NewMSatFromSatoshis(700) aliceBobFirstHop := n.aliceChannelLink.ShortChanID() - sendDustHtlcs(t, n, true, amt, aliceBobFirstHop, numHTLCs) - sendDustHtlcs(t, n, false, amt, aliceBobFirstHop, numHTLCs) + // Alice will send 51 HTLC's of 700sats. Bob will also send 51 HTLC's + // of 700sats. + sendDustHtlcs(t, n, true, amt, aliceBobFirstHop, numHTLCs+1) + sendDustHtlcs(t, n, false, amt, aliceBobFirstHop, numHTLCs+1) // Generate the parameters needed for Bob to send another dust HTLC. _, timelock, hops := generateHops( @@ -4318,22 +4318,12 @@ func TestSwitchDustForwarding(t *testing.T) { OnionBlob: blob, } - checkAlmostDust := func(link *channelLink, mbox MailBox, - whoseCommit lntypes.ChannelParty) bool { + expectedDust := maxInflightHtlcs * 2 * amt - timeout := time.After(15 * time.Second) - pollInterval := 300 * time.Millisecond - expectedDust := 354 * 2 * amt - - for { - <-time.After(pollInterval) - - select { - case <-timeout: - return false - default: - } + assertAlmostDust := func(link *channelLink, mbox MailBox, + whoseCommit lntypes.ChannelParty) { + err := wait.NoError(func() error { linkDust := link.getDustSum( whoseCommit, fn.None[chainfee.SatPerKWeight](), ) @@ -4347,11 +4337,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,11 +4351,7 @@ func TestSwitchDustForwarding(t *testing.T) { n.firstBobChannelLink.ChanID(), n.firstBobChannelLink.ShortChanID(), ) - require.True( - t, checkAlmostDust( - n.firstBobChannelLink, bobMbox, lntypes.Local, - ), - ) + assertAlmostDust(n.firstBobChannelLink, bobMbox, lntypes.Local) // Sending one more HTLC should fail. SendHTLC won't error, but the // HTLC should be failed backwards. @@ -4412,9 +4400,7 @@ func TestSwitchDustForwarding(t *testing.T) { aliceBobFirstHop, uint64(bobAttemptID), nondustHtlc, ) require.NoError(t, err) - require.True(t, checkAlmostDust( - n.firstBobChannelLink, bobMbox, lntypes.Local, - )) + assertAlmostDust(n.firstBobChannelLink, bobMbox, lntypes.Local) // Check that the HTLC failed. bobResultChan, err = n.bobServer.htlcSwitch.GetAttemptResult( @@ -4492,11 +4478,7 @@ func TestSwitchDustForwarding(t *testing.T) { aliceMbox := aliceOrch.GetOrCreateMailBox( n.aliceChannelLink.ChanID(), n.aliceChannelLink.ShortChanID(), ) - require.True( - t, checkAlmostDust( - n.aliceChannelLink, aliceMbox, lntypes.Remote, - ), - ) + assertAlmostDust(n.aliceChannelLink, aliceMbox, lntypes.Remote) err = n.aliceServer.htlcSwitch.SendHTLC( n.aliceChannelLink.ShortChanID(), uint64(aliceAttemptID), diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 427dc8cb58..3719d7ae4c 100644 --- a/htlcswitch/test_utils.go +++ b/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),