lnwallet: fix limboMtx/intentMtx lock order inversion in PsbtFundingVerify

docs/release-notes/release-notes-0.21.2.md

@@ -22,6 +22,14 @@
 
 # Bug Fixes
 
+* [Fixed a lock order inversion](https://github.com/lightningnetwork/lnd/pull/11008)
+  between `PsbtFundingVerify` and `handleFundingCancelRequest` in the wallet.
+  With PSBT or batch funding the two could deadlock the wallet's single
+  `requestHandler` goroutine, which permanently disabled all channel funding for
+  the whole node: no new channel could be opened, and channels whose funding
+  transaction confirmed stayed in the `channelReadySent` opening state forever,
+  never added to the graph and never announced. Only a restart recovered.
+
 * [Fixed several bugs](https://github.com/lightningnetwork/lnd/pull/10948)
   in onion message decoding where messages that should have been rejected
   per BOLT 4 were instead accepted, or a valid TLV was dropped.
@@ -87,3 +95,4 @@
 
 * bitromortac
 * Jared Tobin
+* LNBiG

lnwallet/wallet.go

@@ -443,7 +443,12 @@ type LightningWallet struct {
 	// as key in the fundingLimbo map. Used to easily look up a channel
 	// reservation given a pending channel ID.
 	reservationIDs map[[32]byte]uint64
-	limboMtx       sync.RWMutex
+
+	// limboMtx guards fundingLimbo and reservationIDs.
+	//
+	// NOTE: When both limboMtx and intentMtx are needed, limboMtx MUST be
+	// acquired first. See the note on PsbtFundingVerify.
+	limboMtx sync.RWMutex
 
 	// lockedOutPoints is a set of the currently locked outpoint. This
 	// information is kept in order to provide an easy way to unlock all
@@ -746,9 +751,34 @@ func (l *LightningWallet) RegisterFundingIntent(expectedID [32]byte,
 // PsbtFundingVerify looks up a previously registered funding intent by its
 // pending channel ID and tries to advance the state machine by verifying the
 // passed PSBT.
+//
+// NOTE: limboMtx MUST be acquired before intentMtx, never the other way round.
+// handleFundingCancelRequest holds limboMtx for the duration of the call and
+// acquires intentMtx inside it, so acquiring the two in the opposite order here
+// deadlocks the wallet's requestHandler goroutine, which in turn wedges all
+// channel funding for the whole node.
 func (l *LightningWallet) PsbtFundingVerify(pendingChanID [32]byte,
 	packet *psbt.Packet, skipFinalize bool) error {
 
+	// Get the channel reservation that corresponds to this pending channel
+	// ID. This has to happen before intentMtx is acquired, see the note
+	// above.
+	l.limboMtx.Lock()
+	pid, ok := l.reservationIDs[pendingChanID]
+	if !ok {
+		l.limboMtx.Unlock()
+		return fmt.Errorf("no channel reservation found for "+
+			"pendingChannelID(%x)", pendingChanID[:])
+	}
+
+	pendingReservation, ok := l.fundingLimbo[pid]
+	l.limboMtx.Unlock()
+
+	if !ok {
+		return fmt.Errorf("no channel reservation found for "+
+			"reservation ID %v", pid)
+	}
+
 	l.intentMtx.Lock()
 	defer l.intentMtx.Unlock()
 
@@ -772,28 +802,11 @@ func (l *LightningWallet) PsbtFundingVerify(pendingChanID [32]byte,
 		return fmt.Errorf("error verifying PSBT: %w", err)
 	}
 
-	// Get the channel reservation for that corresponds to this pending
-	// channel ID.
-	l.limboMtx.Lock()
-	pid, ok := l.reservationIDs[pendingChanID]
-	if !ok {
-		l.limboMtx.Unlock()
-		return fmt.Errorf("no channel reservation found for "+
-			"pendingChannelID(%x)", pendingChanID[:])
-	}
-
-	pendingReservation, ok := l.fundingLimbo[pid]
-	l.limboMtx.Unlock()
-
-	if !ok {
-		return fmt.Errorf("no channel reservation found for "+
-			"reservation ID %v", pid)
-	}
-
 	// Now the PSBT has been populated and verified, we can again check
 	// whether the value reserved for anchor fee bumping is respected.
 	isPublic := pendingReservation.partialState.ChannelFlags&lnwire.FFAnnounceChannel != 0
 	hasAnchors := pendingReservation.partialState.ChanType.HasAnchors()
+
 	return l.enforceNewReservedValue(intent, isPublic, hasAnchors)
 }