Skip to content

Commit fe9a3e1

Browse files
committed
funding: fix race in itest for zero-conf funding
Fix a race condition where forwarding through a public zero-conf channel could fail with UnknownNextPeer when using the confirmed SCID. The issue occurred because ReportShortChanID (which updates the switch's baseIndex to handle the confirmed SCID) was called AFTER addToGraph (which announces the confirmed SCID to the network). With slow backends like postgres, addToGraph takes significant time, creating a window where other nodes learn about the confirmed SCID from gossip and attempt to route through it, but the receiving node's switch hasn't been updated yet to handle forwards using the confirmed SCID. The fix reorders operations to call ReportShortChanID before addToGraph, ensuring the switch is ready to handle the confirmed SCID before it's announced to the network. Forwards using either the alias or confirmed SCID will work since getLinkByMapping uses baseIndex to map both to the same link in forwardingIndex. Fixes flaky test: zero_conf-channel_policy_update_public_zero_conf
1 parent cb3991e commit fe9a3e1

File tree

2 files changed

+26
-21
lines changed

2 files changed

+26
-21
lines changed

funding/manager.go

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3957,6 +3957,22 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error {
39573957

39583958
// Six confirmations have been reached. If this channel is public,
39593959
// we'll delete some of the alias mappings the gossiper uses.
3960+
//
3961+
// Tell the Switch to refresh the relevant ChannelLink so that forwards
3962+
// under the confirmed SCID are possible. We do this BEFORE updating the
3963+
// graph to avoid a race where other nodes learn about the confirmed
3964+
// SCID from gossip before our switch is ready to handle forwards using
3965+
// it. This is especially important for integration tests.
3966+
err = f.cfg.ReportShortChanID(c.FundingOutpoint)
3967+
if err != nil {
3968+
// This should only fail if the link is not found in the
3969+
// Switch's linkIndex map. If this is the case, then the peer
3970+
// has gone offline and the next time the link is loaded, it
3971+
// will have a refreshed state. Just log an error here.
3972+
log.Errorf("unable to report scid for zero-conf channel "+
3973+
"channel: %v", err)
3974+
}
3975+
39603976
isPublic := c.ChannelFlags&lnwire.FFAnnounceChannel != 0
39613977
if isPublic {
39623978
err = f.cfg.AliasManager.DeleteSixConfs(c.ShortChannelID)
@@ -3985,19 +4001,6 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error {
39854001
}
39864002
}
39874003

3988-
// Since we have now marked down the confirmed SCID, we'll also need to
3989-
// tell the Switch to refresh the relevant ChannelLink so that forwards
3990-
// under the confirmed SCID are possible if this is a public channel.
3991-
err = f.cfg.ReportShortChanID(c.FundingOutpoint)
3992-
if err != nil {
3993-
// This should only fail if the link is not found in the
3994-
// Switch's linkIndex map. If this is the case, then the peer
3995-
// has gone offline and the next time the link is loaded, it
3996-
// will have a refreshed state. Just log an error here.
3997-
log.Errorf("unable to report scid for zero-conf channel "+
3998-
"channel: %v", err)
3999-
}
4000-
40014004
// Update the confirmed transaction's label.
40024005
f.makeLabelForTx(c)
40034006

funding/manager_test.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4726,14 +4726,9 @@ func testZeroConf(t *testing.T, chanType *lnwire.ChannelType) {
47264726
assertConfirmationHeight(t, alice, chanID, 1)
47274727
assertConfirmationHeight(t, bob, chanID, 1)
47284728

4729-
// For taproot channels, we don't expect them to be announced atm.
4730-
if !isTaprootChanType(chanType) {
4731-
assertChannelAnnouncements(
4732-
t, alice, bob, fundingAmt, nil, nil, nil, nil,
4733-
)
4734-
}
4735-
4736-
// Both Alice and Bob should send on reportScidChan.
4729+
// Both Alice and Bob should call ReportShortChanID first (before
4730+
// sending announcements) to avoid a race where other nodes learn about
4731+
// the confirmed SCID before the switch is ready.
47374732
select {
47384733
case <-alice.reportScidChan:
47394734
case <-time.After(time.Second * 5):
@@ -4746,6 +4741,13 @@ func testZeroConf(t *testing.T, chanType *lnwire.ChannelType) {
47464741
t.Fatalf("did not call ReportShortChanID in time")
47474742
}
47484743

4744+
// For taproot channels, we don't expect them to be announced atm.
4745+
if !isTaprootChanType(chanType) {
4746+
assertChannelAnnouncements(
4747+
t, alice, bob, fundingAmt, nil, nil, nil, nil,
4748+
)
4749+
}
4750+
47494751
// Send along the 6-confirmation channel so that announcement sigs can
47504752
// be exchanged.
47514753
alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{

0 commit comments

Comments
 (0)