From 05347c839263554e609a5275cbe4fc99628f46ec Mon Sep 17 00:00:00 2001
From: Keagan McClelland <keagan.mcclelland@gmail.com>
Date: Wed, 17 Jul 2024 13:35:32 -0700
Subject: [PATCH] lnwallet: bring processFeeUpdate in line with
 process[Add|Remove]Entry

This commit redoes the API and semantics of processFeeUpdate to make
it consistent with the semantics of it's sister functions. This is
part of an ongoing series of commits to remove mutateState arguments
pervasively from the codebase.

As with the previous commit this makes state mutation the caller's
responsibility. This temporarily increases code duplication at the
call-sites, but this will unlock other refactor opportunities.
---
 lnwallet/channel.go      | 74 +++++++++++++++++++++++-----------------
 lnwallet/channel_test.go | 25 ++++++++++++--
 2 files changed, 64 insertions(+), 35 deletions(-)

diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index 13a421458..2157afe03 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -2927,10 +2927,26 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, ourBalance,
 
 		// Process fee updates, updating the current feePerKw.
 		case FeeUpdate:
-			processFeeUpdate(
-				entry, nextHeight, whoseCommitChain,
-				mutateState, newView,
+			h := entry.addCommitHeights.GetForParty(
+				whoseCommitChain,
 			)
+
+			if h == 0 {
+				processFeeUpdate(
+					entry, &newView.FeePerKw, nextHeight,
+					whoseCommitChain,
+				)
+
+				if mutateState {
+					entry.addCommitHeights.SetForParty(
+						whoseCommitChain, nextHeight,
+					)
+
+					entry.removeCommitHeights.SetForParty(
+						whoseCommitChain, nextHeight,
+					)
+				}
+			}
 			continue
 		}
 
@@ -2975,10 +2991,26 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, ourBalance,
 
 		// Process fee updates, updating the current feePerKw.
 		case FeeUpdate:
-			processFeeUpdate(
-				entry, nextHeight, whoseCommitChain,
-				mutateState, newView,
+			h := entry.addCommitHeights.GetForParty(
+				whoseCommitChain,
 			)
+
+			if h == 0 {
+				processFeeUpdate(
+					entry, &newView.FeePerKw, nextHeight,
+					whoseCommitChain,
+				)
+
+				if mutateState {
+					entry.addCommitHeights.SetForParty(
+						whoseCommitChain, nextHeight,
+					)
+
+					entry.removeCommitHeights.SetForParty(
+						whoseCommitChain, nextHeight,
+					)
+				}
+			}
 			continue
 		}
 
@@ -3176,35 +3208,13 @@ func processRemoveEntry(htlc *paymentDescriptor, ourBalance,
 
 // processFeeUpdate processes a log update that updates the current commitment
 // fee.
-func processFeeUpdate(feeUpdate *paymentDescriptor, nextHeight uint64,
-	whoseCommitChain lntypes.ChannelParty, mutateState bool,
-	view *HtlcView) {
-
-	// Fee updates are applied for all commitments after they are
-	// sent/received, so we consider them being added and removed at the
-	// same height.
-	var addHeight *uint64
-	var removeHeight *uint64
-	if whoseCommitChain.IsRemote() {
-		addHeight = &feeUpdate.addCommitHeights.Remote
-		removeHeight = &feeUpdate.removeCommitHeights.Remote
-	} else {
-		addHeight = &feeUpdate.addCommitHeights.Local
-		removeHeight = &feeUpdate.removeCommitHeights.Local
-	}
-
-	if *addHeight != 0 {
-		return
-	}
+func processFeeUpdate(feeUpdate *paymentDescriptor,
+	feeRef *chainfee.SatPerKWeight, nextHeight uint64,
+	whoseCommitChain lntypes.ChannelParty) {
 
 	// If the update wasn't already locked in, update the current fee rate
 	// to reflect this update.
-	view.FeePerKw = chainfee.SatPerKWeight(feeUpdate.Amount.ToSatoshis())
-
-	if mutateState {
-		*addHeight = nextHeight
-		*removeHeight = nextHeight
-	}
+	*feeRef = chainfee.SatPerKWeight(feeUpdate.Amount.ToSatoshis())
 }
 
 // generateRemoteHtlcSigJobs generates a series of HTLC signature jobs for the
diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go
index 997f0c22d..77630d57a 100644
--- a/lnwallet/channel_test.go
+++ b/lnwallet/channel_test.go
@@ -9204,11 +9204,30 @@ func TestProcessFeeUpdate(t *testing.T) {
 			view := &HtlcView{
 				FeePerKw: chainfee.SatPerKWeight(feePerKw),
 			}
-			processFeeUpdate(
-				update, nextHeight, test.whoseCommitChain,
-				test.mutate, view,
+
+			h := update.addCommitHeights.GetForParty(
+				test.whoseCommitChain,
 			)
 
+			if h == 0 {
+				processFeeUpdate(
+					update, &view.FeePerKw, nextHeight,
+					test.whoseCommitChain,
+				)
+
+				if test.mutate {
+					update.addCommitHeights.SetForParty(
+						test.whoseCommitChain,
+						nextHeight,
+					)
+
+					update.removeCommitHeights.SetForParty(
+						test.whoseCommitChain,
+						nextHeight,
+					)
+				}
+			}
+
 			if view.FeePerKw != test.expectedFee {
 				t.Fatalf("expected fee: %v, got: %v",
 					test.expectedFee, feePerKw)