Merge pull request #10440 from ziggie1984/bugfix/itest-race

funding: fix race in itest for zero-conf funding

Author: ziggie1984

funding/manager.go

@@ -3957,6 +3957,22 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error {
 
 	// Six confirmations have been reached. If this channel is public,
 	// we'll delete some of the alias mappings the gossiper uses.
+	//
+	// Tell the Switch to refresh the relevant ChannelLink so that forwards
+	// under the confirmed SCID are possible. We do this BEFORE updating the
+	// graph to avoid a race where other nodes learn about the confirmed
+	// SCID from gossip before our switch is ready to handle forwards using
+	// it. This is especially important for integration tests.
+	err = f.cfg.ReportShortChanID(c.FundingOutpoint)
+	if err != nil {
+		// This should only fail if the link is not found in the
+		// Switch's linkIndex map. If this is the case, then the peer
+		// has gone offline and the next time the link is loaded, it
+		// will have a refreshed state. Just log an error here.
+		log.Errorf("unable to report scid for zero-conf channel "+
+			"channel: %v", err)
+	}
+
 	isPublic := c.ChannelFlags&lnwire.FFAnnounceChannel != 0
 	if isPublic {
 		err = f.cfg.AliasManager.DeleteSixConfs(c.ShortChannelID)
@@ -3985,19 +4001,6 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error {
 		}
 	}
 
-	// Since we have now marked down the confirmed SCID, we'll also need to
-	// tell the Switch to refresh the relevant ChannelLink so that forwards
-	// under the confirmed SCID are possible if this is a public channel.
-	err = f.cfg.ReportShortChanID(c.FundingOutpoint)
-	if err != nil {
-		// This should only fail if the link is not found in the
-		// Switch's linkIndex map. If this is the case, then the peer
-		// has gone offline and the next time the link is loaded, it
-		// will have a refreshed state. Just log an error here.
-		log.Errorf("unable to report scid for zero-conf channel "+
-			"channel: %v", err)
-	}
-
 	// Update the confirmed transaction's label.
 	f.makeLabelForTx(c)
 

funding/manager_test.go

@@ -4726,14 +4726,9 @@ func testZeroConf(t *testing.T, chanType *lnwire.ChannelType) {
 	assertConfirmationHeight(t, alice, chanID, 1)
 	assertConfirmationHeight(t, bob, chanID, 1)
 
-	// For taproot channels, we don't expect them to be announced atm.
-	if !isTaprootChanType(chanType) {
-		assertChannelAnnouncements(
-			t, alice, bob, fundingAmt, nil, nil, nil, nil,
-		)
-	}
-
-	// Both Alice and Bob should send on reportScidChan.
+	// Both Alice and Bob should call ReportShortChanID first (before
+	// sending announcements) to avoid a race where other nodes learn about
+	// the confirmed SCID before the switch is ready.
 	select {
 	case <-alice.reportScidChan:
 	case <-time.After(time.Second * 5):
@@ -4746,6 +4741,13 @@ func testZeroConf(t *testing.T, chanType *lnwire.ChannelType) {
 		t.Fatalf("did not call ReportShortChanID in time")
 	}
 
+	// For taproot channels, we don't expect them to be announced atm.
+	if !isTaprootChanType(chanType) {
+		assertChannelAnnouncements(
+			t, alice, bob, fundingAmt, nil, nil, nil, nil,
+		)
+	}
+
 	// Send along the 6-confirmation channel so that announcement sigs can
 	// be exchanged.
 	alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{