diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 131d86327..a83e4dbbd 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2374,9 +2374,12 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, // these balances will be *before* taking a commitment fee from the // initiator. htlcView := lc.fetchHTLCView(theirLogIndex, ourLogIndex) - ourBalance, theirBalance, _, filteredHTLCView := lc.computeView( + ourBalance, theirBalance, _, filteredHTLCView, err := lc.computeView( htlcView, remoteChain, true, ) + if err != nil { + return nil, err + } feePerKw := filteredHTLCView.feePerKw // Actually generate unsigned commitment transaction for this view. @@ -2444,7 +2447,7 @@ func fundingTxIn(chanState *channeldb.OpenChannel) wire.TxIn { // method. func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, theirBalance *lnwire.MilliSatoshi, nextHeight uint64, - remoteChain, mutateState bool) *htlcView { + remoteChain, mutateState bool) (*htlcView, error) { // We initialize the view's fee rate to the fee rate of the unfiltered // view. If any fee updates are found when evaluating the view, it will @@ -2460,6 +2463,57 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, skipUs := make(map[uint64]struct{}) skipThem := make(map[uint64]struct{}) + // fetchParentEntry is a helper method that will fetch the parent of + // entry from the corresponding update log. + fetchParentEntry := func(entry *PaymentDescriptor, + remoteLog bool) (*PaymentDescriptor, error) { + + var ( + updateLog *updateLog + logName string + ) + + if remoteLog { + updateLog = lc.remoteUpdateLog + logName = "remote" + } else { + updateLog = lc.localUpdateLog + logName = "local" + } + + addEntry := updateLog.lookupHtlc(entry.ParentIndex) + + switch { + // We check if the parent entry is not found at this point. + // This could happen for old versions of lnd, and we return an + // error to gracefully shut down the state machine if such an + // entry is still in the logs. + case addEntry == nil: + return nil, fmt.Errorf("unable to find parent entry "+ + "%d in %v update log: %v\nUpdatelog: %v", + entry.ParentIndex, logName, + newLogClosure(func() string { + return spew.Sdump(entry) + }), newLogClosure(func() string { + return spew.Sdump(updateLog) + }), + ) + + // The parent add height should never be zero at this point. If + // that's the case we probably forgot to send a new commitment. + case remoteChain && addEntry.addCommitHeightRemote == 0: + return nil, fmt.Errorf("parent entry %d for update %d "+ + "had zero remote add height", entry.ParentIndex, + entry.LogIndex) + case !remoteChain && addEntry.addCommitHeightLocal == 0: + return nil, fmt.Errorf("parent entry %d for update %d "+ + "had zero local add height", entry.ParentIndex, + entry.LogIndex) + } + + return addEntry, nil + } + // First we run through non-add entries in both logs, populating the // skip sets and mutating the current chain state (crediting balances, // etc) to reflect the settle/timeout entry encountered. @@ -2486,21 +2540,9 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, lc.channelState.TotalMSatReceived += entry.Amount } - addEntry := lc.remoteUpdateLog.lookupHtlc(entry.ParentIndex) - - // We check if the parent entry is not found at this point. We - // have seen this happening a few times and panic with some - // addtitional info to figure out why. - // TODO(halseth): remove when bug is fixed. - if addEntry == nil { - panic(fmt.Sprintf("unable to find parent entry %d "+ - "in remote update log: %v\nUpdatelog: %v", - entry.ParentIndex, newLogClosure(func() string { - return spew.Sdump(entry) - }), newLogClosure(func() string { - return spew.Sdump(lc.remoteUpdateLog) - }), - )) + addEntry, err := fetchParentEntry(entry, true) + if err != nil { + return nil, err } skipThem[addEntry.HtlcIndex] = struct{}{} @@ -2531,21 +2573,9 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, lc.channelState.TotalMSatSent += entry.Amount } - addEntry := lc.localUpdateLog.lookupHtlc(entry.ParentIndex) - - // We check if the parent entry is not found at this point. We - // have seen this happening a few times and panic with some - // addtitional info to figure out why. - // TODO(halseth): remove when bug is fixed. - if addEntry == nil { - panic(fmt.Sprintf("unable to find parent entry %d "+ - "in local update log: %v\nUpdatelog: %v", - entry.ParentIndex, newLogClosure(func() string { - return spew.Sdump(entry) - }), newLogClosure(func() string { - return spew.Sdump(lc.localUpdateLog) - }), - )) + addEntry, err := fetchParentEntry(entry, false) + if err != nil { + return nil, err } skipUs[addEntry.HtlcIndex] = struct{}{} @@ -2577,7 +2607,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, newView.theirUpdates = append(newView.theirUpdates, entry) } - return newView + return newView, nil } // processAddEntry evaluates the effect of an add entry within the HTLC log. @@ -3099,9 +3129,12 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, ourInitialBalance := commitChain.tip().ourBalance theirInitialBalance := commitChain.tip().theirBalance - ourBalance, theirBalance, commitWeight, filteredView := lc.computeView( + ourBalance, theirBalance, commitWeight, filteredView, err := lc.computeView( view, remoteChain, false, ) + if err != nil { + return err + } feePerKw := filteredView.feePerKw // Calculate the commitment fee, and subtract it from the initiator's @@ -3192,7 +3225,7 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, // First check that the remote updates won't violate it's channel // constraints. - err := validateUpdates( + err = validateUpdates( filteredView.theirUpdates, &lc.channelState.RemoteChanCfg, ) if err != nil { @@ -3688,7 +3721,7 @@ func (lc *LightningChannel) ProcessChanSyncMsg( // HTLCs will be set to the next commitment height. func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, updateState bool) (lnwire.MilliSatoshi, lnwire.MilliSatoshi, int64, - *htlcView) { + *htlcView, error) { commitChain := lc.localCommitChain dustLimit := lc.channelState.LocalChanCfg.DustLimit @@ -3727,8 +3760,11 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, // channel constraints to the final commitment state. If any fee // updates are found in the logs, the commitment fee rate should be // changed, so we'll also set the feePerKw to this new value. - filteredHTLCView := lc.evaluateHTLCView(view, &ourBalance, + filteredHTLCView, err := lc.evaluateHTLCView(view, &ourBalance, &theirBalance, nextHeight, remoteChain, updateState) + if err != nil { + return 0, 0, 0, nil, err + } feePerKw := filteredHTLCView.feePerKw // Now go through all HTLCs at this stage, to calculate the total @@ -3752,7 +3788,7 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, } totalCommitWeight := input.CommitWeight + totalHtlcWeight - return ourBalance, theirBalance, totalCommitWeight, filteredHTLCView + return ourBalance, theirBalance, totalCommitWeight, filteredHTLCView, nil } // genHtlcSigValidationJobs generates a series of signatures verification jobs @@ -5957,8 +5993,12 @@ func (lc *LightningChannel) availableBalance() (lnwire.MilliSatoshi, int64) { lc.localUpdateLog.logIndex) // Then compute our current balance for that view. - ourBalance, _, commitWeight, filteredView := + ourBalance, _, commitWeight, filteredView, err := lc.computeView(htlcView, false, false) + if err != nil { + lc.log.Errorf("Unable to fetch available balance: %v", err) + return 0, 0 + } // If we are the channel initiator, we must remember to subtract the // commitment fee from our available balance. diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 1e871ab68..d34862ed7 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -5922,8 +5922,8 @@ func TestNewBreachRetributionSkipsDustHtlcs(t *testing.T) { // With the HTLC's applied to both update logs, we'll initiate a state // transition from Alice. - if err := ForceStateTransition(bobChannel, aliceChannel); err != nil { - t.Fatalf("unable to complete bob's state transition: %v", err) + if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("unable to complete alice's state transition: %v", err) } // At this point, we'll capture the current state number, as well as