mirror of
https://github.com/lightningnetwork/lnd.git
synced 2025-02-22 14:22:37 +01:00
lnwallet: inline and remove process[Add|Remove]Entry
This commit observes that processAddEntry and processRemoveEntry are only invoked at a single call-site. Here we inline them at their call-sites, which will unlock further simplifications of the code that will allow us to remove pointer mutations in favor of explicit expression oriented programming. We also delete the tests associated with these functions, the overall functionality is implicitly tested by the TestEvaluateHTLCView tests.
This commit is contained in:
Keagan McClelland
parent
1222cb8b10
commit
f0eecfa2cd
2 changed files with 51 additions and 494 deletions
|
|
@ -2957,10 +2957,44 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, ourBalance,
|
|||
whoseCommitChain,
|
||||
)
|
||||
if rmvHeight == 0 {
|
||||
processRemoveEntry(
|
||||
entry, ourBalance, theirBalance,
|
||||
party.CounterParty(),
|
||||
)
|
||||
switch {
|
||||
// If an incoming HTLC is being settled, then
|
||||
// this means that we've received the preimage
|
||||
// either from another subsystem, or the
|
||||
// upstream peer in the route. Therefore, we
|
||||
// increase our balance by the HTLC amount.
|
||||
case party.CounterParty() == lntypes.Remote &&
|
||||
entry.EntryType == Settle:
|
||||
|
||||
*ourBalance += entry.Amount
|
||||
|
||||
// Otherwise, this HTLC is being failed out,
|
||||
// therefore the value of the HTLC should
|
||||
// return to the remote party.
|
||||
case party.CounterParty() == lntypes.Remote &&
|
||||
entry.EntryType != Settle:
|
||||
|
||||
*theirBalance += entry.Amount
|
||||
|
||||
// If an outgoing HTLC is being settled, then
|
||||
// this means that the downstream party
|
||||
// resented the preimage or learned of it via a
|
||||
// downstream peer. In either case, we credit
|
||||
// their settled value with the value of the
|
||||
// HTLC.
|
||||
case party.CounterParty() == lntypes.Local &&
|
||||
entry.EntryType == Settle:
|
||||
|
||||
*theirBalance += entry.Amount
|
||||
|
||||
// Otherwise, one of our outgoing HTLC's has
|
||||
// timed out, so the value of the HTLC should
|
||||
// be returned to our settled balance.
|
||||
case party.CounterParty() == lntypes.Local &&
|
||||
entry.EntryType != Settle:
|
||||
|
||||
*ourBalance += entry.Amount
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -2981,9 +3015,19 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, ourBalance,
|
|||
whoseCommitChain,
|
||||
)
|
||||
if addHeight == 0 {
|
||||
processAddEntry(
|
||||
entry, ourBalance, theirBalance, party,
|
||||
)
|
||||
if party == lntypes.Remote {
|
||||
// If this is a new incoming
|
||||
// (un-committed) HTLC, then we need
|
||||
// to update their balance accordingly
|
||||
// by subtracting the amount of the
|
||||
// HTLC that are funds pending.
|
||||
*theirBalance -= entry.Amount
|
||||
} else {
|
||||
// Similarly, we need to debit our
|
||||
// balance if this is an out going HTLC
|
||||
// to reflect the pending balance.
|
||||
*ourBalance -= entry.Amount
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -3071,62 +3115,6 @@ func (lc *LightningChannel) fetchParent(entry *paymentDescriptor,
|
|||
return addEntry, nil
|
||||
}
|
||||
|
||||
// processAddEntry evaluates the effect of an add entry within the HTLC log.
|
||||
// If the HTLC hasn't yet been committed in either chain, then the height it
|
||||
// was committed is updated. Keeping track of this inclusion height allows us to
|
||||
// later compact the log once the change is fully committed in both chains.
|
||||
func processAddEntry(htlc *paymentDescriptor, ourBalance,
|
||||
theirBalance *lnwire.MilliSatoshi, originator lntypes.ChannelParty) {
|
||||
|
||||
if originator == lntypes.Remote {
|
||||
// If this is a new incoming (un-committed) HTLC, then we need
|
||||
// to update their balance accordingly by subtracting the
|
||||
// amount of the HTLC that are funds pending.
|
||||
*theirBalance -= htlc.Amount
|
||||
} else {
|
||||
// Similarly, we need to debit our balance if this is an out
|
||||
// going HTLC to reflect the pending balance.
|
||||
*ourBalance -= htlc.Amount
|
||||
}
|
||||
}
|
||||
|
||||
// processRemoveEntry processes a log entry which settles or times out a
|
||||
// previously added HTLC. If the removal entry has already been processed, it
|
||||
// is skipped.
|
||||
func processRemoveEntry(htlc *paymentDescriptor, ourBalance,
|
||||
theirBalance *lnwire.MilliSatoshi, originator lntypes.ChannelParty) {
|
||||
|
||||
switch {
|
||||
// If an incoming HTLC is being settled, then this means that we've
|
||||
// received the preimage either from another subsystem, or the
|
||||
// upstream peer in the route. Therefore, we increase our balance by
|
||||
// the HTLC amount.
|
||||
case originator == lntypes.Remote && htlc.EntryType == Settle:
|
||||
*ourBalance += htlc.Amount
|
||||
|
||||
// Otherwise, this HTLC is being failed out, therefore the value of the
|
||||
// HTLC should return to the remote party.
|
||||
case originator == lntypes.Remote &&
|
||||
(htlc.EntryType == Fail || htlc.EntryType == MalformedFail):
|
||||
|
||||
*theirBalance += htlc.Amount
|
||||
|
||||
// If an outgoing HTLC is being settled, then this means that the
|
||||
// downstream party resented the preimage or learned of it via a
|
||||
// downstream peer. In either case, we credit their settled value with
|
||||
// the value of the HTLC.
|
||||
case originator == lntypes.Local && htlc.EntryType == Settle:
|
||||
*theirBalance += htlc.Amount
|
||||
|
||||
// Otherwise, one of our outgoing HTLC's has timed out, so the value of
|
||||
// the HTLC should be returned to our settled balance.
|
||||
case originator == lntypes.Local &&
|
||||
(htlc.EntryType == Fail || htlc.EntryType == MalformedFail):
|
||||
|
||||
*ourBalance += htlc.Amount
|
||||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
|
|
|
|||
|
|
@ -9055,437 +9055,6 @@ type heights struct {
|
|||
remoteRemove uint64
|
||||
}
|
||||
|
||||
func checkHeights(t *testing.T, update *paymentDescriptor, expected heights) {
|
||||
updateHeights := heights{
|
||||
localAdd: update.addCommitHeights.Local,
|
||||
localRemove: update.removeCommitHeights.Local,
|
||||
remoteAdd: update.addCommitHeights.Remote,
|
||||
remoteRemove: update.removeCommitHeights.Remote,
|
||||
}
|
||||
|
||||
if !reflect.DeepEqual(updateHeights, expected) {
|
||||
t.Fatalf("expected: %v, got: %v", expected, updateHeights)
|
||||
}
|
||||
}
|
||||
|
||||
// TestProcessAddRemoveEntry tests the updating of our and their balances when
|
||||
// we process adds, settles and fails. It also tests the mutating of add and
|
||||
// remove heights.
|
||||
func TestProcessAddRemoveEntry(t *testing.T) {
|
||||
const (
|
||||
// addHeight is a non-zero addHeight that is used for htlc
|
||||
// add heights.
|
||||
addHeight = 100
|
||||
|
||||
// removeHeight is a non-zero removeHeight that is used for
|
||||
// htlc remove heights.
|
||||
removeHeight = 200
|
||||
|
||||
// nextHeight is a constant that we use for the nextHeight in
|
||||
// all unit tests.
|
||||
nextHeight = 400
|
||||
|
||||
// updateAmount is the amount that the update is set to.
|
||||
updateAmount = lnwire.MilliSatoshi(10)
|
||||
|
||||
// startBalance is a balance we start both sides out with
|
||||
// so that balances can be incremented.
|
||||
startBalance = lnwire.MilliSatoshi(100)
|
||||
)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
startHeights heights
|
||||
whoseCommitChain lntypes.ChannelParty
|
||||
originator lntypes.ChannelParty
|
||||
mutateState bool
|
||||
ourExpectedBalance lnwire.MilliSatoshi
|
||||
theirExpectedBalance lnwire.MilliSatoshi
|
||||
expectedHeights heights
|
||||
updateType updateType
|
||||
}{
|
||||
{
|
||||
name: "add, remote chain, already processed",
|
||||
startHeights: heights{
|
||||
localAdd: 0,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Remote,
|
||||
originator: lntypes.Local,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: 0,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Add,
|
||||
},
|
||||
{
|
||||
name: "add, local chain, already processed",
|
||||
startHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: 0,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Local,
|
||||
originator: lntypes.Local,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: 0,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Add,
|
||||
},
|
||||
{
|
||||
name: "incoming add, local chain, not mutated",
|
||||
startHeights: heights{
|
||||
localAdd: 0,
|
||||
remoteAdd: 0,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Local,
|
||||
originator: lntypes.Remote,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance,
|
||||
theirExpectedBalance: startBalance - updateAmount,
|
||||
expectedHeights: heights{
|
||||
localAdd: 0,
|
||||
remoteAdd: 0,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Add,
|
||||
},
|
||||
{
|
||||
name: "incoming add, local chain, mutated",
|
||||
startHeights: heights{
|
||||
localAdd: 0,
|
||||
remoteAdd: 0,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Local,
|
||||
originator: lntypes.Remote,
|
||||
mutateState: true,
|
||||
ourExpectedBalance: startBalance,
|
||||
theirExpectedBalance: startBalance - updateAmount,
|
||||
expectedHeights: heights{
|
||||
localAdd: nextHeight,
|
||||
remoteAdd: 0,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Add,
|
||||
},
|
||||
|
||||
{
|
||||
name: "outgoing add, remote chain, not mutated",
|
||||
startHeights: heights{
|
||||
localAdd: 0,
|
||||
remoteAdd: 0,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Remote,
|
||||
originator: lntypes.Local,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance - updateAmount,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: 0,
|
||||
remoteAdd: 0,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Add,
|
||||
},
|
||||
{
|
||||
name: "outgoing add, remote chain, mutated",
|
||||
startHeights: heights{
|
||||
localAdd: 0,
|
||||
remoteAdd: 0,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Remote,
|
||||
originator: lntypes.Local,
|
||||
mutateState: true,
|
||||
ourExpectedBalance: startBalance - updateAmount,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: 0,
|
||||
remoteAdd: nextHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Add,
|
||||
},
|
||||
{
|
||||
name: "settle, remote chain, already processed",
|
||||
startHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: removeHeight,
|
||||
},
|
||||
whoseCommitChain: lntypes.Remote,
|
||||
originator: lntypes.Local,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: removeHeight,
|
||||
},
|
||||
updateType: Settle,
|
||||
},
|
||||
{
|
||||
name: "settle, local chain, already processed",
|
||||
startHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: removeHeight,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Local,
|
||||
originator: lntypes.Local,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: removeHeight,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Settle,
|
||||
},
|
||||
{
|
||||
// Remote chain, and not processed yet. Incoming settle,
|
||||
// so we expect our balance to increase.
|
||||
name: "incoming settle",
|
||||
startHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Remote,
|
||||
originator: lntypes.Remote,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance + updateAmount,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Settle,
|
||||
},
|
||||
{
|
||||
// Remote chain, and not processed yet. Incoming settle,
|
||||
// so we expect our balance to increase.
|
||||
name: "outgoing settle",
|
||||
startHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Remote,
|
||||
originator: lntypes.Local,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance,
|
||||
theirExpectedBalance: startBalance + updateAmount,
|
||||
expectedHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Settle,
|
||||
},
|
||||
{
|
||||
// Remote chain, and not processed yet. Incoming fail,
|
||||
// so we expect their balance to increase.
|
||||
name: "incoming fail",
|
||||
startHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Remote,
|
||||
originator: lntypes.Remote,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance,
|
||||
theirExpectedBalance: startBalance + updateAmount,
|
||||
expectedHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Fail,
|
||||
},
|
||||
{
|
||||
// Remote chain, and not processed yet. Outgoing fail,
|
||||
// so we expect our balance to increase.
|
||||
name: "outgoing fail",
|
||||
startHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Remote,
|
||||
originator: lntypes.Local,
|
||||
mutateState: false,
|
||||
ourExpectedBalance: startBalance + updateAmount,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Fail,
|
||||
},
|
||||
{
|
||||
// Local chain, and not processed yet. Incoming settle,
|
||||
// so we expect our balance to increase. Mutate is
|
||||
// true, so we expect our remove removeHeight to have
|
||||
// changed.
|
||||
name: "fail, our remove height mutated",
|
||||
startHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Local,
|
||||
originator: lntypes.Remote,
|
||||
mutateState: true,
|
||||
ourExpectedBalance: startBalance + updateAmount,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: nextHeight,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
updateType: Settle,
|
||||
},
|
||||
{
|
||||
// Remote chain, and not processed yet. Incoming settle,
|
||||
// so we expect our balance to increase. Mutate is
|
||||
// true, so we expect their remove removeHeight to have
|
||||
// changed.
|
||||
name: "fail, their remove height mutated",
|
||||
startHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: 0,
|
||||
},
|
||||
whoseCommitChain: lntypes.Remote,
|
||||
originator: lntypes.Remote,
|
||||
mutateState: true,
|
||||
ourExpectedBalance: startBalance + updateAmount,
|
||||
theirExpectedBalance: startBalance,
|
||||
expectedHeights: heights{
|
||||
localAdd: addHeight,
|
||||
remoteAdd: addHeight,
|
||||
localRemove: 0,
|
||||
remoteRemove: nextHeight,
|
||||
},
|
||||
updateType: Settle,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
test := test
|
||||
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
heights := test.startHeights
|
||||
update := &paymentDescriptor{
|
||||
Amount: updateAmount,
|
||||
addCommitHeights: lntypes.Dual[uint64]{
|
||||
Local: heights.localAdd,
|
||||
Remote: heights.remoteAdd,
|
||||
},
|
||||
removeCommitHeights: lntypes.Dual[uint64]{
|
||||
Local: heights.localRemove,
|
||||
Remote: heights.remoteRemove,
|
||||
},
|
||||
EntryType: test.updateType,
|
||||
}
|
||||
|
||||
var (
|
||||
// Start both parties off with an initial
|
||||
// balance. Copy by value here so that we do
|
||||
// not mutate the startBalance constant.
|
||||
ourBalance, theirBalance = startBalance,
|
||||
startBalance
|
||||
)
|
||||
|
||||
// Choose the processing function we need based on the
|
||||
// update type. Process remove is used for settles,
|
||||
// fails and malformed htlcs.
|
||||
process := processRemoveEntry
|
||||
heightDual := &update.removeCommitHeights
|
||||
if test.updateType == Add {
|
||||
process = processAddEntry
|
||||
heightDual = &update.addCommitHeights
|
||||
}
|
||||
|
||||
if heightDual.GetForParty(test.whoseCommitChain) == 0 {
|
||||
process(
|
||||
update, &ourBalance, &theirBalance,
|
||||
test.originator,
|
||||
)
|
||||
|
||||
if test.mutateState {
|
||||
heightDual.SetForParty(
|
||||
test.whoseCommitChain,
|
||||
nextHeight,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Check that balances were updated as expected.
|
||||
if ourBalance != test.ourExpectedBalance {
|
||||
t.Fatalf("expected our balance: %v, got: %v",
|
||||
test.ourExpectedBalance, ourBalance)
|
||||
}
|
||||
|
||||
if theirBalance != test.theirExpectedBalance {
|
||||
t.Fatalf("expected their balance: %v, got: %v",
|
||||
test.theirExpectedBalance, theirBalance)
|
||||
}
|
||||
|
||||
// Check that heights on the update are as expected.
|
||||
checkHeights(t, update, test.expectedHeights)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestChannelUnsignedAckedFailure tests that unsigned acked updates are
|
||||
// properly restored after signing for them and disconnecting.
|
||||
//
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue