Merge pull request #5428 from wpaulino/enforce-anchor-reserve

lnwallet: prevent anchor reserve enforcement on legacy inbound channel

lntest/itest/lnd_onchain_test.go

@@ -179,7 +179,8 @@ func testAnchorReservedValue(net *lntest.NetworkHarness, t *harnessTest) {
 	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
 	net.ConnectNodes(ctxt, t.t, alice, bob)
 
-	// Send just enough coins for Alice to open a channel without a change output.
+	// Send just enough coins for Alice to open a channel without a change
+	// output.
 	const (
 		chanAmt = 1000000
 		feeEst  = 8000
@@ -205,20 +206,45 @@ func testAnchorReservedValue(net *lntest.NetworkHarness, t *harnessTest) {
 	// Alice opens a smaller channel. This works since it will have a
 	// change output.
 	ctxt, _ = context.WithTimeout(context.Background(), defaultTimeout)
-	aliceChanPoint := openChannelAndAssert(
+	aliceChanPoint1 := openChannelAndAssert(
 		ctxt, t, net, alice, bob,
 		lntest.OpenChannelParams{
-			Amt: chanAmt / 2,
+			Amt: chanAmt / 4,
 		},
 	)
 
+	// If Alice tries to open another anchor channel to Bob, Bob should not
+	// reject it as he is not contributing any funds.
+	aliceChanPoint2 := openChannelAndAssert(
+		ctxt, t, net, alice, bob, lntest.OpenChannelParams{
+			Amt: chanAmt / 4,
+		},
+	)
+
+	// Similarly, if Alice tries to open a legacy channel to Bob, Bob should
+	// not reject it as he is not contributing any funds. We'll restart Bob
+	// to remove his support for anchors.
+	err = net.RestartNode(bob, nil)
+	require.NoError(t.t, err)
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
-	err = alice.WaitForNetworkChannelOpen(ctxt, aliceChanPoint)
+	aliceChanPoint3 := openChannelAndAssert(
+		ctxt, t, net, alice, bob, lntest.OpenChannelParams{
+			Amt: chanAmt / 4,
+		},
+	)
+
+	chanPoints := []*lnrpc.ChannelPoint{
+		aliceChanPoint1, aliceChanPoint2, aliceChanPoint3,
+	}
+	for _, chanPoint := range chanPoints {
+		ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+		err = alice.WaitForNetworkChannelOpen(ctxt, chanPoint)
 		require.NoError(t.t, err)
 
 		ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
-	err = bob.WaitForNetworkChannelOpen(ctxt, aliceChanPoint)
+		err = bob.WaitForNetworkChannelOpen(ctxt, chanPoint)
 		require.NoError(t.t, err)
+	}
 
 	// Alice tries to send all coins to an internal address. This is
 	// allowed, since the final wallet balance will still be above the
@@ -316,7 +342,9 @@ func testAnchorReservedValue(net *lntest.NetworkHarness, t *harnessTest) {
 
 	// Alice closes channel, should now be allowed to send everything to an
 	// external address.
-	closeChannelAndAssert(ctxt, t, net, alice, aliceChanPoint, false)
+	for _, chanPoint := range chanPoints {
+		closeChannelAndAssert(ctxt, t, net, alice, chanPoint, false)
+	}
 
 	newBalance := waitForConfirmedBalance()
 	if newBalance <= aliceBalance {
@@ -338,11 +366,11 @@ func testAnchorReservedValue(net *lntest.NetworkHarness, t *harnessTest) {
 	// generated above.
 	block = mineBlocks(t, net, 1, 1)[0]
 
-	// The sweep transaction should have two inputs, the change output from
+	// The sweep transaction should have four inputs, the change output from
-	// the previous sweep, and the output from the coop closed channel.
+	// the previous sweep, and the outputs from the coop closed channels.
 	sweepTx = block.Transactions[1]
-	if len(sweepTx.TxIn) != 2 {
+	if len(sweepTx.TxIn) != 4 {
-		t.Fatalf("expected 2 inputs instead have %v", len(sweepTx.TxIn))
+		t.Fatalf("expected 4 inputs instead have %v", len(sweepTx.TxIn))
 	}
 
 	// It should have a single output.

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
+	// Now the the PSBT has been populated and verified, we can again check
-	// value reserved for anchor fee bumping is respected.
+	// 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.
 	isPublic := pendingReservation.partialState.ChannelFlags&lnwire.FFAnnounceChannel != 0
-	if pendingReservation.partialState.ChanType.HasAnchors() &&
+	hasAnchors := pendingReservation.partialState.ChanType.HasAnchors()
-		intent.LocalFundingAmt() > 0 && isPublic {
+	return l.enforceNewReservedValue(intent, isPublic, hasAnchors)
-		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
-	})
 }
 
 // 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.
+	// fee bumping.
-	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++
-	}
-
 	// 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
+	// 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 {
+	isPublic := req.Flags&lnwire.FFAnnounceChannel != 0
-		_, err := l.CheckReservedValue(
+	hasAnchors := req.CommitType == CommitmentTypeAnchorsZeroFeeHtlcTx
-			fundingIntent.Inputs(), fundingIntent.Outputs(),
+	err = l.enforceNewReservedValue(fundingIntent, isPublic, hasAnchors)
-			numAnchors,
-		)
-		return err
-	})
 	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) {