From 6bbe79053535c39508e8aef396a33d0c68d14eed Mon Sep 17 00:00:00 2001
From: Wilmer Paulino <wilmer.paulino@gmail.com>
Date: Wed, 23 Jun 2021 17:40:36 -0700
Subject: [PATCH] lnwallet: prevent anchor reserve enforcement on legacy
 inbound channel

This commit aims to address a flaw in our anchor reserve enforcement
logic in which an inbound "legacy" channel (i.e. a channel with a
commitment type that precedes anchors) would be rejected by the
recipient if they have at least one opened channel using the anchors
commitment type and do not have enough on-chain funds to meet the
anchors reserve.
---
 lnwallet/wallet.go | 103 +++++++++++++++++++++------------------------
 1 file changed, 47 insertions(+), 56 deletions(-)

diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go
index 14318615..1b1b40b6 100644
--- a/lnwallet/wallet.go
+++ b/lnwallet/wallet.go
@@ -595,33 +595,11 @@ func (l *LightningWallet) PsbtFundingVerify(pendingChanID [32]byte,
 			"reservation ID %v", pid)
 	}
 
-	// Now the the PSBT has been verified, we can again check whether the
-	// value reserved for anchor fee bumping is respected.
-	numAnchors, err := l.currentNumAnchorChans()
-	if err != nil {
-		return err
-	}
-
-	// If this commit type is an anchor channel we add that to our counter,
-	// but only if we are contributing funds to the channel. This is done
-	// to still allow incoming channels even though we have no UTXOs
-	// available, as in bootstrapping phases. We only count public
-	// channels.
+	// Now the 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
-	if pendingReservation.partialState.ChanType.HasAnchors() &&
-		intent.LocalFundingAmt() > 0 && isPublic {
-		numAnchors++
-	}
-
-	// We check the reserve value again, this should already have been
-	// checked for regular FullIntents, but now the PSBT intent is also
-	// populated.
-	return l.WithCoinSelectLock(func() error {
-		_, err := l.CheckReservedValue(
-			intent.Inputs(), intent.Outputs(), numAnchors,
-		)
-		return err
-	})
+	hasAnchors := pendingReservation.partialState.ChanType.HasAnchors()
+	return l.enforceNewReservedValue(intent, isPublic, hasAnchors)
 }
 
 // PsbtFundingFinalize looks up a previously registered funding intent by its
@@ -809,38 +787,15 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg
 
 	// Now that we have a funding intent, we'll check whether funding a
 	// channel using it would violate our reserved value for anchor channel
-	// fee bumping. We first get our current number of anchor channels.
-	numAnchors, err := l.currentNumAnchorChans()
-	if err != nil {
-		fundingIntent.Cancel()
-
-		req.err <- err
-		req.resp <- nil
-		return
-	}
-
-	// If this commit type is an anchor channel we add that to our counter,
-	// but only if we are contributing funds to the channel. This is done
-	// to still allow incoming channels even though we have no UTXOs
-	// available, as in bootstrapping phases. We only count public
-	// channels.
-	isPublic := req.Flags&lnwire.FFAnnounceChannel != 0
-	if req.CommitType == CommitmentTypeAnchorsZeroFeeHtlcTx &&
-		fundingIntent.LocalFundingAmt() > 0 && isPublic {
-		numAnchors++
-	}
-
+	// fee bumping.
+	//
 	// Check the reserved value using the inputs and outputs given by the
-	// intent. Not that for the PSBT intent type we don't yet have the
-	// funding tx ready, so this will always pass. We'll do another check
+	// intent. Note that for the PSBT intent type we don't yet have the
+	// funding tx ready, so this will always pass.  We'll do another check
 	// when the PSBT has been verified.
-	err = l.WithCoinSelectLock(func() error {
-		_, err := l.CheckReservedValue(
-			fundingIntent.Inputs(), fundingIntent.Outputs(),
-			numAnchors,
-		)
-		return err
-	})
+	isPublic := req.Flags&lnwire.FFAnnounceChannel != 0
+	hasAnchors := req.CommitType == CommitmentTypeAnchorsZeroFeeHtlcTx
+	err = l.enforceNewReservedValue(fundingIntent, isPublic, hasAnchors)
 	if err != nil {
 		fundingIntent.Cancel()
 
@@ -893,6 +848,42 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg
 	req.err <- nil
 }
 
+// enforceReservedValue enforces that the wallet, upon a new channel being
+// opened, meets the minimum amount of funds required for each advertised anchor
+// channel.
+//
+// We only enforce the reserve if we are contributing funds to the channel. This
+// is done to still allow incoming channels even though we have no UTXOs
+// available, as in bootstrapping phases.
+func (l *LightningWallet) enforceNewReservedValue(fundingIntent chanfunding.Intent,
+	isPublic, hasAnchors bool) error {
+
+	// Only enforce the reserve when an advertised channel is being opened
+	// in which we are contributing funds to. This ensures we never dip
+	// below the reserve.
+	if !isPublic || fundingIntent.LocalFundingAmt() == 0 {
+		return nil
+	}
+
+	numAnchors, err := l.currentNumAnchorChans()
+	if err != nil {
+		return err
+	}
+
+	// Add the to-be-opened channel.
+	if hasAnchors {
+		numAnchors++
+	}
+
+	return l.WithCoinSelectLock(func() error {
+		_, err := l.CheckReservedValue(
+			fundingIntent.Inputs(), fundingIntent.Outputs(),
+			numAnchors,
+		)
+		return err
+	})
+}
+
 // currentNumAnchorChans returns the current number of non-private anchor
 // channels the wallet should be ready to fee bump if needed.
 func (l *LightningWallet) currentNumAnchorChans() (int, error) {