lnwallet: inline processUpdateFee and remove the function entirely

In this commit we observe that the previous commit reduced the role
of this function to a single assignment statement with numerous newly
irrelevant parameters. This commit makes the choice of inlining it at
the two call-sites within evaluateHTLCView and removing the funciton
definition entirely. This also allows us to drop a huge portion of
newly irrelevant test code.
This commit is contained in:
Keagan McClelland 2024-07-18 17:14:13 -07:00
parent 05347c8392
commit 819239c5c8
No known key found for this signature in database
GPG Key ID: FA7E65C951F12439
2 changed files with 11 additions and 234 deletions

View File

@ -2932,9 +2932,11 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, ourBalance,
)
if h == 0 {
processFeeUpdate(
entry, &newView.FeePerKw, nextHeight,
whoseCommitChain,
// If the update wasn't already locked in,
// update the current fee rate to reflect this
// update.
newView.FeePerKw = chainfee.SatPerKWeight(
entry.Amount.ToSatoshis(),
)
if mutateState {
@ -2996,11 +2998,14 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, ourBalance,
)
if h == 0 {
processFeeUpdate(
entry, &newView.FeePerKw, nextHeight,
whoseCommitChain,
// If the update wasn't already locked in,
// update the current fee rate to reflect this
// update.
newView.FeePerKw = chainfee.SatPerKWeight(
entry.Amount.ToSatoshis(),
)
if mutateState {
entry.addCommitHeights.SetForParty(
whoseCommitChain, nextHeight,
@ -3206,17 +3211,6 @@ func processRemoveEntry(htlc *paymentDescriptor, ourBalance,
}
}
// processFeeUpdate processes a log update that updates the current commitment
// fee.
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.
*feeRef = chainfee.SatPerKWeight(feeUpdate.Amount.ToSatoshis())
}
// generateRemoteHtlcSigJobs generates a series of HTLC signature jobs for the
// sig pool, along with a channel that if closed, will cancel any jobs after
// they have been submitted to the sigPool. This method is to be used when

View File

@ -9021,223 +9021,6 @@ type heights struct {
remoteRemove uint64
}
// TestProcessFeeUpdate tests the applying of fee updates and mutation of
// local and remote add and remove heights on update messages.
func TestProcessFeeUpdate(t *testing.T) {
const (
// height is a non-zero height that can be used for htlcs
// heights.
height = 200
// nextHeight is a constant that we use for the next height in
// all unit tests.
nextHeight = 400
// feePerKw is the fee we start all of our unit tests with.
feePerKw = 1
// ourFeeUpdateAmt is an amount that we update fees to expressed
// in msat.
ourFeeUpdateAmt = 20000
// ourFeeUpdatePerSat is the fee rate *in satoshis* that we
// expect if we update to ourFeeUpdateAmt.
ourFeeUpdatePerSat = chainfee.SatPerKWeight(20)
)
tests := []struct {
name string
startHeights heights
expectedHeights heights
whoseCommitChain lntypes.ChannelParty
mutate bool
expectedFee chainfee.SatPerKWeight
}{
{
// Looking at local chain, local add is non-zero so
// the update has been applied already; no fee change.
name: "non-zero local height, fee unchanged",
startHeights: heights{
localAdd: height,
localRemove: 0,
remoteAdd: 0,
remoteRemove: height,
},
expectedHeights: heights{
localAdd: height,
localRemove: 0,
remoteAdd: 0,
remoteRemove: height,
},
whoseCommitChain: lntypes.Local,
mutate: false,
expectedFee: feePerKw,
},
{
// Looking at local chain, local add is zero so the
// update has not been applied yet; we expect a fee
// update.
name: "zero local height, fee changed",
startHeights: heights{
localAdd: 0,
localRemove: 0,
remoteAdd: height,
remoteRemove: 0,
},
expectedHeights: heights{
localAdd: 0,
localRemove: 0,
remoteAdd: height,
remoteRemove: 0,
},
whoseCommitChain: lntypes.Local,
mutate: false,
expectedFee: ourFeeUpdatePerSat,
},
{
// Looking at remote chain, the remote add height is
// zero, so the update has not been applied so we expect
// a fee change.
name: "zero remote height, fee changed",
startHeights: heights{
localAdd: height,
localRemove: 0,
remoteAdd: 0,
remoteRemove: 0,
},
expectedHeights: heights{
localAdd: height,
localRemove: 0,
remoteAdd: 0,
remoteRemove: 0,
},
whoseCommitChain: lntypes.Remote,
mutate: false,
expectedFee: ourFeeUpdatePerSat,
},
{
// Looking at remote chain, the remote add height is
// non-zero, so the update has been applied so we expect
// no fee change.
name: "non-zero remote height, no fee change",
startHeights: heights{
localAdd: height,
localRemove: 0,
remoteAdd: height,
remoteRemove: 0,
},
expectedHeights: heights{
localAdd: height,
localRemove: 0,
remoteAdd: height,
remoteRemove: 0,
},
whoseCommitChain: lntypes.Remote,
mutate: false,
expectedFee: feePerKw,
},
{
// Local add height is non-zero, so the update has
// already been applied; we do not expect fee to
// change or any mutations to be applied.
name: "non-zero local height, mutation not applied",
startHeights: heights{
localAdd: height,
localRemove: 0,
remoteAdd: 0,
remoteRemove: height,
},
expectedHeights: heights{
localAdd: height,
localRemove: 0,
remoteAdd: 0,
remoteRemove: height,
},
whoseCommitChain: lntypes.Local,
mutate: true,
expectedFee: feePerKw,
},
{
// Local add is zero and we are looking at our local
// chain, so the update has not been applied yet. We
// expect the local add and remote heights to be
// mutated.
name: "zero height, fee changed, mutation applied",
startHeights: heights{
localAdd: 0,
localRemove: 0,
remoteAdd: 0,
remoteRemove: 0,
},
expectedHeights: heights{
localAdd: nextHeight,
localRemove: nextHeight,
remoteAdd: 0,
remoteRemove: 0,
},
whoseCommitChain: lntypes.Local,
mutate: true,
expectedFee: ourFeeUpdatePerSat,
},
}
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
// Create a fee update with add and remove heights as
// set in the test.
heights := test.startHeights
update := &paymentDescriptor{
Amount: ourFeeUpdateAmt,
addCommitHeights: lntypes.Dual[uint64]{
Local: heights.localAdd,
Remote: heights.remoteAdd,
},
removeCommitHeights: lntypes.Dual[uint64]{
Local: heights.localRemove,
Remote: heights.remoteRemove,
},
EntryType: FeeUpdate,
}
view := &HtlcView{
FeePerKw: chainfee.SatPerKWeight(feePerKw),
}
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)
}
checkHeights(t, update, test.expectedHeights)
})
}
}
func checkHeights(t *testing.T, update *paymentDescriptor, expected heights) {
updateHeights := heights{
localAdd: update.addCommitHeights.Local,