From 67125956183ac4bc38421100bb1c81895e96a928 Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Wed, 8 Sep 2021 13:21:12 +0200
Subject: [PATCH] lnwallet: fix validateFeeRate

The validateFeeRate function uses the availableBalance function to get
the current spendable balance of a channel, adds the old fee and then
ensures that the new fee is not larger than the amount we have available
to spend. This commit also removes the local reserve check in the
validateFeeRate function since the balance returned from
availableBalance already takes the local reserve into acccount.
---
 lnwallet/channel.go      | 16 ++++----------
 lnwallet/channel_test.go | 47 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index ea3a9ca56..a4e9c36f5 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -6821,7 +6821,10 @@ func (lc *LightningChannel) validateFeeRate(feePerKw chainfee.SatPerKWeight) err
 	// be above our reserve balance. Otherwise, we'll reject the fee
 	// update.
 	availableBalance, txWeight := lc.availableBalance()
-	oldFee := lnwire.NewMSatFromSatoshis(lc.localCommitChain.tip().fee)
+
+	oldFee := lnwire.NewMSatFromSatoshis(
+		lc.localCommitChain.tip().feePerKw.FeeForWeight(txWeight),
+	)
 
 	// Our base balance is the total amount of satoshis we can commit
 	// towards fees before factoring in the channel reserve.
@@ -6843,17 +6846,6 @@ func (lc *LightningChannel) validateFeeRate(feePerKw chainfee.SatPerKWeight) err
 			newFee, baseBalance)
 	}
 
-	// If this new balance is below our reserve, then we can't accommodate
-	// the fee change, so we'll reject it.
-	balanceAfterFee := baseBalance - newFee
-	if balanceAfterFee.ToSatoshis() < lc.channelState.LocalChanCfg.ChanReserve {
-		return fmt.Errorf("cannot apply fee_update=%v sat/kw, "+
-			"new balance=%v would dip below channel reserve=%v",
-			int64(feePerKw),
-			balanceAfterFee.ToSatoshis(),
-			lc.channelState.LocalChanCfg.ChanReserve)
-	}
-
 	// TODO(halseth): should fail if fee update is unreasonable,
 	// as specified in BOLT#2.
 	//  * COMMENT(roasbeef): can cross-check with our ideal fee rate
diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go
index a490b9502..e626a22db 100644
--- a/lnwallet/channel_test.go
+++ b/lnwallet/channel_test.go
@@ -5,9 +5,11 @@ import (
 	"container/list"
 	"crypto/sha256"
 	"fmt"
+	"math/rand"
 	"reflect"
 	"runtime"
 	"testing"
+	"testing/quick"
 
 	"github.com/btcsuite/btcd/blockchain"
 	"github.com/btcsuite/btcd/btcec"
@@ -8014,6 +8016,25 @@ func TestChannelMaxFeeRate(t *testing.T) {
 				"allocation of %v, got %v", expFeeRate,
 				maxAlloc, maxFeeRate)
 		}
+
+		if err := c.validateFeeRate(maxFeeRate); err != nil {
+			t.Fatalf("fee rate validation failed: %v", err)
+		}
+	}
+
+	// propertyTest tests that the validateFeeRate function always passes
+	// for the output returned by MaxFeeRate for any valid random inputs
+	// fed to MaxFeeRate.
+	propertyTest := func(c *LightningChannel) func(alloc maxAlloc,
+		anchorMax fee) bool {
+
+		return func(ma maxAlloc, anchorMax fee) bool {
+			maxFeeRate := c.MaxFeeRate(
+				float64(ma), chainfee.SatPerKWeight(anchorMax),
+			)
+
+			return c.validateFeeRate(maxFeeRate) == nil
+		}
 	}
 
 	aliceChannel, _, cleanUp, err := CreateTestChannels(
@@ -8024,6 +8045,10 @@ func TestChannelMaxFeeRate(t *testing.T) {
 	}
 	defer cleanUp()
 
+	if err = quick.Check(propertyTest(aliceChannel), nil); err != nil {
+		t.Fatal(err)
+	}
+
 	assertMaxFeeRate(aliceChannel, 1.0, 0, 676794154)
 	assertMaxFeeRate(aliceChannel, 0.001, 0, 676794)
 	assertMaxFeeRate(aliceChannel, 0.000001, 0, 676)
@@ -8038,6 +8063,10 @@ func TestChannelMaxFeeRate(t *testing.T) {
 	}
 	defer cleanUp()
 
+	if err = quick.Check(propertyTest(anchorChannel), nil); err != nil {
+		t.Fatal(err)
+	}
+
 	// Anchor commitments are heavier, hence will the same allocation lead
 	// to slightly lower fee rates.
 	assertMaxFeeRate(
@@ -8052,6 +8081,24 @@ func TestChannelMaxFeeRate(t *testing.T) {
 	)
 }
 
+type maxAlloc float64
+
+// Generate ensures that the random value generated by the testing quick
+// package for maxAlloc is always a positive float64 between 0 and 1.
+func (maxAlloc) Generate(r *rand.Rand, _ int) reflect.Value {
+	ma := maxAlloc(r.Float64())
+	return reflect.ValueOf(ma)
+}
+
+type fee chainfee.SatPerKWeight
+
+// Generate ensures that the random value generated by the testing quick
+// package for a fee is always a positive int64.
+func (fee) Generate(r *rand.Rand, _ int) reflect.Value {
+	am := fee(r.Int63())
+	return reflect.ValueOf(am)
+}
+
 // TestChannelFeeRateFloor asserts that valid commitments can be proposed and
 // received using chainfee.FeePerKwFloor as the initiator's fee rate.
 func TestChannelFeeRateFloor(t *testing.T) {