lnwallet: add new test to ensure channel is able to update fees in both directions

Closes #1145.

lnwallet/channel.go

@@ -5583,7 +5583,11 @@ func (lc *LightningChannel) validateFeeRate(feePerKw SatPerKWeight) error {
 	// be above our reserve balance. Otherwise, we'll reject the fee
 	// update.
 	availableBalance, txWeight := lc.availableBalance()
-	oldFee := lnwire.NewMSatFromSatoshis(lc.CalcFee(SatPerKWeight(lc.channelState.LocalCommitment.FeePerKw)))
+	oldFee := lnwire.NewMSatFromSatoshis(lc.localCommitChain.tip().fee)
+
+	// Our base balance is the total amount of satoshis we can commit
+	// towards fees before factoring in the channel reserve.
+	baseBalance := availableBalance + oldFee
 
 	// Using the weight of the commitment transaction if we were to create
 	// a commitment now, we'll compute our remaining balance if we apply
@@ -5592,22 +5596,24 @@ func (lc *LightningChannel) validateFeeRate(feePerKw SatPerKWeight) error {
 		feePerKw.FeeForWeight(txWeight),
 	)
 
-	// If the total fee exceeds our available balance, then we'll reject
+	// If the total fee exceeds our available balance (taking into account
-	// this update as it would mean we need to trim our entire output.
+	// the fee from the last state), then we'll reject this update as it
-	if newFee > availableBalance+oldFee {
+	// would mean we need to trim our entire output.
+	if newFee > baseBalance {
 		return fmt.Errorf("cannot apply fee_update=%v sat/kw, new fee "+
 			"of %v is greater than balance of %v", int64(feePerKw),
-			newFee, availableBalance+oldFee)
+			newFee, baseBalance)
 	}
 
 	// If this new balance is below our reserve, then we can't accommodate
 	// the fee change, so we'll reject it.
-	balanceAfterFee := availableBalance + oldFee - newFee
+	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)
+			balanceAfterFee.ToSatoshis(),
+			lc.channelState.LocalChanCfg.ChanReserve)
 	}
 
 	// TODO(halseth): should fail if fee update is unreasonable,

lnwallet/channel_test.go

@@ -2278,6 +2278,61 @@ func TestCooperativeCloseDustAdherence(t *testing.T) {
 	}
 }
 
+// TestUpdateFeeAdjustments tests that the state machine is able to properly
+// accept valid fee changes, as well as reject any invalid fee updates.
+func TestUpdateFeeAdjustments(t *testing.T) {
+	t.Parallel()
+
+	aliceChannel, bobChannel, cleanUp, err := createTestChannels(1)
+	if err != nil {
+		t.Fatalf("unable to create test channels: %v", err)
+	}
+	defer cleanUp()
+
+	// First, we'll grab the current base fee rate as we'll be using this
+	// to make relative adjustments int he fee rate.
+	baseFeeRate := aliceChannel.channelState.LocalCommitment.FeePerKw
+
+	// We'll first try to increase the fee rate 5x, this should be able to
+	// be committed without any issue.
+	newFee := SatPerKWeight(baseFeeRate * 5)
+
+	if err := aliceChannel.UpdateFee(newFee); err != nil {
+		t.Fatalf("unable to alice update fee: %v", err)
+	}
+	if err := bobChannel.ReceiveUpdateFee(newFee); err != nil {
+		t.Fatalf("unable to bob update fee: %v", err)
+	}
+
+	// With the fee updates applied, we'll now initiate a state transition
+	// to ensure the fee update is locked in.
+	if err := forceStateTransition(aliceChannel, bobChannel); err != nil {
+		t.Fatalf("unable to create new commitment: %v", err)
+	}
+
+	// We'll now attempt to increase the fee rate 1,000,000x of the base
+	// fee.  This should result in an error as Alice won't be able to pay
+	// this new fee rate.
+	newFee = SatPerKWeight(baseFeeRate * 1000000)
+	if err := aliceChannel.UpdateFee(newFee); err == nil {
+		t.Fatalf("alice should reject the fee rate")
+	}
+
+	// Finally, we'll attempt to adjust the fee down and use a fee which is
+	// smaller than the initial base fee rate. The fee application and
+	// state transition should proceed without issue.
+	newFee = SatPerKWeight(baseFeeRate / 100)
+	if err := aliceChannel.UpdateFee(newFee); err != nil {
+		t.Fatalf("unable to alice update fee: %v", err)
+	}
+	if err := bobChannel.ReceiveUpdateFee(newFee); err != nil {
+		t.Fatalf("unable to bob update fee: %v", err)
+	}
+	if err := forceStateTransition(aliceChannel, bobChannel); err != nil {
+		t.Fatalf("unable to create new commitment: %v", err)
+	}
+}
+
 // TestUpdateFeeFail tests that the signature verification will fail if they
 // fee updates are out of sync.
 func TestUpdateFeeFail(t *testing.T) {
@@ -4776,36 +4831,38 @@ func TestMaxPendingAmount(t *testing.T) {
 	}
 }
 
-// TestChanReserve tests that the ErrBelowChanReserve error is thrown when
+// TestChanReserve tests that the ErrBelowChanReserve error is thrown when an
-// an HTLC is added that causes a node's balance to dip below its channel
+// HTLC is added that causes a node's balance to dip below its channel reserve
-// reserve limit.
+// limit.
 func TestChanReserve(t *testing.T) {
 	t.Parallel()
 
 	setupChannels := func() (*LightningChannel, *LightningChannel, func()) {
-		// We'll kick off the test by creating our channels which both are
+		// We'll kick off the test by creating our channels which both
-		// loaded with 5 BTC each.
+		// are loaded with 5 BTC each.
 		aliceChannel, bobChannel, cleanUp, err := createTestChannels(1)
 		if err != nil {
 			t.Fatalf("unable to create test channels: %v", err)
 		}
 
 		// We set the remote required ChanReserve to 0.5 BTC. We will
-		// attempt to cause Alice's balance to dip below this amount and test
+		// attempt to cause Alice's balance to dip below this amount
-		// whether it triggers the ErrBelowChanReserve error.
+		// and test whether it triggers the ErrBelowChanReserve error.
-		aliceMinReserve := btcutil.Amount(0.5 * btcutil.SatoshiPerBitcoin)
+		aliceMinReserve := btcutil.Amount(0.5 *
+			btcutil.SatoshiPerBitcoin)
 
-		// Alice will need to keep her reserve above aliceMinReserve, so
+		// Alice will need to keep her reserve above aliceMinReserve,
-		// set this limit to here local config.
+		// so set this limit to here local config.
 		aliceChannel.localChanCfg.ChanReserve = aliceMinReserve
 
-		// During channel opening Bob will also get to know Alice's minimum
+		// During channel opening Bob will also get to know Alice's
-		// reserve, and this will be found in his remote config.
+		// minimum reserve, and this will be found in his remote
+		// config.
 		bobChannel.remoteChanCfg.ChanReserve = aliceMinReserve
 
-		// We set Bob's channel reserve to a value that is larger than his
+		// We set Bob's channel reserve to a value that is larger than
-		// current balance in the channel. This will ensure that after a
+		// his current balance in the channel. This will ensure that
-		// channel is first opened, Bob can still receive HTLCs
+		// after a channel is first opened, Bob can still receive HTLCs
 		// even though his balance is less than his channel reserve.
 		bobMinReserve := btcutil.Amount(6 * btcutil.SatoshiPerBitcoin)
 		bobChannel.localChanCfg.ChanReserve = bobMinReserve
@@ -4819,10 +4876,9 @@ func TestChanReserve(t *testing.T) {
 	aliceIndex := 0
 	bobIndex := 0
 
-	// Add an HTLC that will increase Bob's balance. This should
+	// Add an HTLC that will increase Bob's balance. This should succeed,
-	// succeed, since Alice stays above her channel reserve, and
+	// since Alice stays above her channel reserve, and Bob increases his
-	// Bob increases his balance (while still being below his
+	// balance (while still being below his channel reserve).
-	// channel reserve).
 	// Resulting balances:
 	//	Alice:	4.5
 	//	Bob:	5.5
@@ -4836,15 +4892,14 @@ func TestChanReserve(t *testing.T) {
 		t.Fatalf("unable to recv htlc: %v", err)
 	}
 
-	// Force a state transation, making sure this HTLC is considered
+	// Force a state transition, making sure this HTLC is considered valid
-	// valid even though the channel reserves are not met.
+	// even though the channel reserves are not met.
 	if err := forceStateTransition(aliceChannel, bobChannel); err != nil {
 		t.Fatalf("unable to complete state update: %v", err)
 	}
 
-	// Now let Bob try to add an HTLC. This should fail, since it
+	// Now let Bob try to add an HTLC. This should fail, since it will
-	// will decrease his balance, which is already below the channel
+	// decrease his balance, which is already below the channel reserve.
-	// reserve.
 	// Resulting balances:
 	//	Alice:	4.5
 	//	Bob:	5.5
@@ -4872,8 +4927,8 @@ func TestChanReserve(t *testing.T) {
 	aliceIndex = 0
 	bobIndex = 0
 
-	// Now we'll add HTLC of 3.5 BTC to Alice's commitment, this should
+	// Now we'll add HTLC of 3.5 BTC to Alice's commitment, this should put
-	// put Alice's balance at 1.5 BTC.
+	// Alice's balance at 1.5 BTC.
 	// Resulting balances:
 	//	Alice:	1.5
 	//	Bob:	9.5
@@ -4890,9 +4945,9 @@ func TestChanReserve(t *testing.T) {
 	}
 
 	// Add a second HTLC of 1 BTC. This should fail because it will take
-	// Alice's balance all the way down to her channel reserve, but
+	// Alice's balance all the way down to her channel reserve, but since
-	// since she is the initiator the additional transaction fee makes
+	// she is the initiator the additional transaction fee makes her
-	// her balance dip below.
+	// balance dip below.
 	htlcAmt = lnwire.NewMSatFromSatoshis(1 * btcutil.SatoshiPerBitcoin)
 	htlc, _ = createHTLC(aliceIndex, htlcAmt)
 	aliceIndex++
@@ -4942,8 +4997,8 @@ func TestChanReserve(t *testing.T) {
 	}
 
 	// And now let Bob add an HTLC of 1 BTC. This will take Bob's balance
-	// all the way down to his channel reserve, but since he is not paying the
+	// all the way down to his channel reserve, but since he is not paying
-	// fee this is okay.
+	// the fee this is okay.
 	htlcAmt = lnwire.NewMSatFromSatoshis(1 * btcutil.SatoshiPerBitcoin)
 	htlc, _ = createHTLC(bobIndex, htlcAmt)
 	bobIndex++
@@ -4953,6 +5008,7 @@ func TestChanReserve(t *testing.T) {
 	if _, err := aliceChannel.ReceiveHTLC(htlc); err != nil {
 		t.Fatalf("unable to recv htlc: %v", err)
 	}
+
 	// Do a last state transition, which should succeed.
 	if err := forceStateTransition(aliceChannel, bobChannel); err != nil {
 		t.Fatalf("unable to complete state update: %v", err)