Skip to content

Commit ae9fd3f

Browse files
committed
peer: make the final cooperative close update cancellable
Send the final close update through a helper that also observes the request context and peer shutdown. Remove the channel closer afterward regardless of whether the update was delivered. Add regression tests covering request cancellation and peer shutdown.
1 parent 9591630 commit ae9fd3f

2 files changed

Lines changed: 91 additions & 4 deletions

File tree

peer/brontide.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3931,6 +3931,24 @@ func ChooseAddr(addr lnwire.DeliveryAddress) fn.Option[lnwire.DeliveryAddress] {
39313931
return fn.Some(addr)
39323932
}
39333933

3934+
// sendFinalCloseUpdate notifies the close caller that its transaction has
3935+
// confirmed. The caller may have stopped reading its bounded update channel,
3936+
// so cancellation of either the request or peer must always release this send.
3937+
func sendFinalCloseUpdate(closeReq *htlcswitch.ChanClose,
3938+
closingTxid chainhash.Hash, peerQuit <-chan struct{}) {
3939+
3940+
select {
3941+
case closeReq.Updates <- &ChannelCloseUpdate{
3942+
ClosingTxid: closingTxid[:],
3943+
Success: true,
3944+
}:
3945+
3946+
case <-closeReq.Ctx.Done():
3947+
3948+
case <-peerQuit:
3949+
}
3950+
}
3951+
39343952
// observeRbfCloseUpdates observes the channel for any updates that may
39353953
// indicate that a new txid has been broadcasted, or the channel fully closed
39363954
// on chain.
@@ -4055,10 +4073,14 @@ func (p *Brontide) observeRbfCloseUpdates(chanCloser *chancloser.RbfChanCloser,
40554073
// update to the client.
40564074
closingTxid := closeState.ConfirmedTx.TxHash()
40574075
if closeReq != nil {
4058-
closeReq.Updates <- &ChannelCloseUpdate{
4059-
ClosingTxid: closingTxid[:],
4060-
Success: true,
4061-
}
4076+
// Updates is bounded. The caller may no
4077+
// longer be reading. Stop waiting when
4078+
// the request or peer shuts down, so
4079+
// cleanup can proceed.
4080+
sendFinalCloseUpdate(
4081+
closeReq, closingTxid,
4082+
p.cg.Done(),
4083+
)
40624084
}
40634085
chanID := lnwire.NewChanIDFromOutPoint(
40644086
*closeReq.ChanPoint,

peer/brontide_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/btcsuite/btcd/btcec/v2"
1212
"github.com/btcsuite/btcd/btcutil/v2"
1313
"github.com/btcsuite/btcd/chaincfg/v2"
14+
"github.com/btcsuite/btcd/chainhash/v2"
1415
"github.com/btcsuite/btcd/txscript/v2"
1516
"github.com/btcsuite/btcd/wire/v2"
1617
"github.com/lightningnetwork/lnd/chainntnfs"
@@ -1585,6 +1586,70 @@ func TestReadHandlerPanicRecovery(t *testing.T) {
15851586
}
15861587
}
15871588

1589+
// TestCloseFinNotificationUnblocks asserts that a full client update channel
1590+
// cannot hold peer teardown open after either the request or peer is canceled.
1591+
func TestCloseFinNotificationUnblocks(t *testing.T) {
1592+
t.Parallel()
1593+
1594+
tests := []struct {
1595+
name string
1596+
shutdownPeer bool
1597+
}{
1598+
{
1599+
name: "request canceled",
1600+
},
1601+
{
1602+
name: "peer stopped",
1603+
shutdownPeer: true,
1604+
},
1605+
}
1606+
1607+
for _, test := range tests {
1608+
t.Run(test.name, func(t *testing.T) {
1609+
t.Parallel()
1610+
1611+
ctx, cancel := context.WithCancel(t.Context())
1612+
defer cancel()
1613+
updates := make(chan interface{}, 1)
1614+
updates <- struct{}{}
1615+
peerQuit := make(chan struct{})
1616+
closeReq := &htlcswitch.ChanClose{
1617+
Updates: updates,
1618+
Ctx: ctx,
1619+
}
1620+
1621+
done := make(chan struct{})
1622+
go func() {
1623+
defer close(done)
1624+
1625+
sendFinalCloseUpdate(
1626+
closeReq, chainhash.Hash{}, peerQuit,
1627+
)
1628+
}()
1629+
1630+
select {
1631+
case <-done:
1632+
t.Fatal("notification returned while no exit " +
1633+
"condition was ready")
1634+
case <-time.After(50 * time.Millisecond):
1635+
}
1636+
1637+
if test.shutdownPeer {
1638+
close(peerQuit)
1639+
} else {
1640+
cancel()
1641+
}
1642+
1643+
select {
1644+
case <-done:
1645+
case <-time.After(timeout):
1646+
t.Fatal("final close notification blocked " +
1647+
"teardown")
1648+
}
1649+
})
1650+
}
1651+
}
1652+
15881653
// TestMessageSummaryPingIncludesNumPongBytes ensures the debug summary for a
15891654
// ping exposes the requested pong size, which makes ignored no-reply pings
15901655
// visible without requiring trace-level logging.

0 commit comments

Comments
 (0)