Skip to content

Commit 5a72d52

Browse files
committed
htlcswitch+itest: catch link quit signal when processing hodlQueue
This commit makes sure when processing resolutions, e.g, settling invoices, when the link is already broken, the process would exit with an error. This fixes the issue we found in the itest, where an unexpected empty remote pending commitment was created although the remote peer is already offline.
1 parent 0892b59 commit 5a72d52

File tree

3 files changed

+25
-0
lines changed

3 files changed

+25
-0
lines changed

contractcourt/channel_arbitrator.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2155,6 +2155,20 @@ func (c *ChannelArbitrator) checkRemoteDanglingActions(
21552155
continue
21562156
}
21572157

2158+
preimageAvailable, err := c.isPreimageAvailable(htlc.RHash)
2159+
if err != nil {
2160+
log.Errorf("ChannelArbitrator(%v): failed to query "+
2161+
"preimage for dangling htlc=%x from remote "+
2162+
"commitments diff", c.cfg.ChanPoint,
2163+
htlc.RHash[:])
2164+
2165+
continue
2166+
}
2167+
2168+
if preimageAvailable {
2169+
continue
2170+
}
2171+
21582172
// Dust htlcs can be canceled back even before the commitment
21592173
// transaction confirms. Dust htlcs are not enforceable onchain.
21602174
// If another version of the commit tx would confirm we either

htlcswitch/link.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1582,6 +1582,11 @@ loop:
15821582
select {
15831583
case item := <-l.hodlQueue.ChanOut():
15841584
htlcResolution = item.(invoices.HtlcResolution)
1585+
1586+
// No need to process it if the link is broken.
1587+
case <-l.cg.Done():
1588+
return ErrLinkShuttingDown
1589+
15851590
default:
15861591
break loop
15871592
}

itest/lnd_multi-hop_force_close_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,7 @@ func runMultiHopReceiverPreimageClaim(ht *lntest.HarnessTest,
744744

745745
// Stop Bob so he won't be able to settle the incoming htlc.
746746
restartBob := ht.SuspendNode(bob)
747+
ht.AssertPeerNotConnected(carol, bob)
747748

748749
// Settle invoice. This will just mark the invoice as settled, as there
749750
// is no link anymore to remove the htlc from the commitment tx. For
@@ -1690,6 +1691,7 @@ func runLocalClaimIncomingHTLC(ht *lntest.HarnessTest,
16901691

16911692
// Suspend Bob to force Carol to go to chain.
16921693
restartBob := ht.SuspendNode(bob)
1694+
ht.AssertPeerNotConnected(carol, bob)
16931695

16941696
// Settle invoice. This will just mark the invoice as settled, as there
16951697
// is no link anymore to remove the htlc from the commitment tx. For
@@ -1967,6 +1969,7 @@ func runLocalClaimIncomingHTLCLeased(ht *lntest.HarnessTest,
19671969

19681970
// Suspend Bob to force Carol to go to chain.
19691971
restartBob := ht.SuspendNode(bob)
1972+
ht.AssertPeerNotConnected(carol, bob)
19701973

19711974
// Settle invoice. This will just mark the invoice as settled, as there
19721975
// is no link anymore to remove the htlc from the commitment tx. For
@@ -2307,6 +2310,7 @@ func runLocalPreimageClaim(ht *lntest.HarnessTest,
23072310

23082311
// Suspend bob, so Carol is forced to go on chain.
23092312
restartBob := ht.SuspendNode(bob)
2313+
ht.AssertPeerNotConnected(carol, bob)
23102314

23112315
// Settle invoice. This will just mark the invoice as settled, as there
23122316
// is no link anymore to remove the htlc from the commitment tx. For
@@ -2547,6 +2551,7 @@ func runLocalPreimageClaimLeased(ht *lntest.HarnessTest,
25472551

25482552
// Suspend bob, so Carol is forced to go on chain.
25492553
restartBob := ht.SuspendNode(bob)
2554+
ht.AssertPeerNotConnected(carol, bob)
25502555

25512556
// Settle invoice. This will just mark the invoice as settled, as there
25522557
// is no link anymore to remove the htlc from the commitment tx. For
@@ -2964,6 +2969,7 @@ func runHtlcAggregation(ht *lntest.HarnessTest,
29642969
// close. However, Carol will cancel her invoices to prevent force
29652970
// closes, so we shut her down for now.
29662971
restartCarol := ht.SuspendNode(carol)
2972+
ht.AssertPeerNotConnected(bob, carol)
29672973

29682974
// We'll now mine enough blocks to trigger Bob's broadcast of his
29692975
// commitment transaction due to the fact that the Carol's HTLCs are

0 commit comments

Comments
 (0)