diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 0f37a46c..41b807ef 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2494,57 +2494,6 @@ 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. @@ -2571,7 +2520,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, lc.channelState.TotalMSatReceived += entry.Amount } - addEntry, err := fetchParentEntry(entry, true) + addEntry, err := lc.fetchParent(entry, remoteChain, true) if err != nil { return nil, err } @@ -2604,7 +2553,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, lc.channelState.TotalMSatSent += entry.Amount } - addEntry, err := fetchParentEntry(entry, false) + addEntry, err := lc.fetchParent(entry, remoteChain, false) if err != nil { return nil, err } @@ -2641,6 +2590,57 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, return newView, nil } +// getFetchParent is a helper that looks up update log parent entries in the +// appropriate log. +func (lc *LightningChannel) fetchParent(entry *PaymentDescriptor, + remoteChain, 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 +} + // 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 diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 9f10be16..f890f118 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -7676,3 +7676,231 @@ func TestChannelFeeRateFloor(t *testing.T) { err) } } + +// TestFetchParent tests lookup of an entry's parent in the appropriate log. +func TestFetchParent(t *testing.T) { + tests := []struct { + name string + remoteChain bool + remoteLog bool + localEntries []*PaymentDescriptor + remoteEntries []*PaymentDescriptor + + // parentIndex is the parent index of the entry that we will + // lookup with fetch parent. + parentIndex uint64 + + // expectErr indicates that we expect fetch parent to fail. + expectErr bool + + // expectedIndex is the htlc index that we expect the parent + // to have. + expectedIndex uint64 + }{ + { + name: "not found in remote log", + localEntries: nil, + remoteEntries: nil, + remoteChain: true, + remoteLog: true, + parentIndex: 0, + expectErr: true, + }, + { + name: "not found in local log", + localEntries: nil, + remoteEntries: nil, + remoteChain: false, + remoteLog: false, + parentIndex: 0, + expectErr: true, + }, + { + name: "remote log + chain, remote add height 0", + localEntries: nil, + remoteEntries: []*PaymentDescriptor{ + // This entry will be added at log index =0. + { + HtlcIndex: 1, + addCommitHeightLocal: 100, + addCommitHeightRemote: 100, + }, + // This entry will be added at log index =1, it + // is the parent entry we are looking for. + { + HtlcIndex: 2, + addCommitHeightLocal: 100, + addCommitHeightRemote: 0, + }, + }, + remoteChain: true, + remoteLog: true, + parentIndex: 1, + expectErr: true, + }, + { + name: "remote log, local chain, local add height 0", + remoteEntries: []*PaymentDescriptor{ + // This entry will be added at log index =0. + { + HtlcIndex: 1, + addCommitHeightLocal: 100, + addCommitHeightRemote: 100, + }, + // This entry will be added at log index =1, it + // is the parent entry we are looking for. + { + HtlcIndex: 2, + addCommitHeightLocal: 0, + addCommitHeightRemote: 100, + }, + }, + localEntries: nil, + remoteChain: false, + remoteLog: true, + parentIndex: 1, + expectErr: true, + }, + { + name: "local log + chain, local add height 0", + localEntries: []*PaymentDescriptor{ + // This entry will be added at log index =0. + { + HtlcIndex: 1, + addCommitHeightLocal: 100, + addCommitHeightRemote: 100, + }, + // This entry will be added at log index =1, it + // is the parent entry we are looking for. + { + HtlcIndex: 2, + addCommitHeightLocal: 0, + addCommitHeightRemote: 100, + }, + }, + remoteEntries: nil, + remoteChain: false, + remoteLog: false, + parentIndex: 1, + expectErr: true, + }, + + { + name: "local log + remote chain, remote add height 0", + localEntries: []*PaymentDescriptor{ + // This entry will be added at log index =0. + { + HtlcIndex: 1, + addCommitHeightLocal: 100, + addCommitHeightRemote: 100, + }, + // This entry will be added at log index =1, it + // is the parent entry we are looking for. + { + HtlcIndex: 2, + addCommitHeightLocal: 100, + addCommitHeightRemote: 0, + }, + }, + remoteEntries: nil, + remoteChain: true, + remoteLog: false, + parentIndex: 1, + expectErr: true, + }, + { + name: "remote log found", + localEntries: nil, + remoteEntries: []*PaymentDescriptor{ + // This entry will be added at log index =0. + { + HtlcIndex: 1, + addCommitHeightLocal: 100, + addCommitHeightRemote: 0, + }, + // This entry will be added at log index =1, it + // is the parent entry we are looking for. + { + HtlcIndex: 2, + addCommitHeightLocal: 100, + addCommitHeightRemote: 100, + }, + }, + remoteChain: true, + remoteLog: true, + parentIndex: 1, + expectErr: false, + expectedIndex: 2, + }, + { + name: "local log found", + localEntries: []*PaymentDescriptor{ + // This entry will be added at log index =0. + { + HtlcIndex: 1, + addCommitHeightLocal: 0, + addCommitHeightRemote: 100, + }, + // This entry will be added at log index =1, it + // is the parent entry we are looking for. + { + HtlcIndex: 2, + addCommitHeightLocal: 100, + addCommitHeightRemote: 100, + }, + }, + remoteEntries: nil, + remoteChain: false, + remoteLog: false, + parentIndex: 1, + expectErr: false, + expectedIndex: 2, + }, + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + // Create a lightning channel with newly initialized + // local and remote logs. + lc := LightningChannel{ + localUpdateLog: newUpdateLog(0, 0), + remoteUpdateLog: newUpdateLog(0, 0), + } + + // Add the local and remote entries to update logs. + for _, entry := range test.localEntries { + lc.localUpdateLog.appendHtlc(entry) + } + for _, entry := range test.remoteEntries { + lc.remoteUpdateLog.appendHtlc(entry) + } + + parent, err := lc.fetchParent( + &PaymentDescriptor{ + ParentIndex: test.parentIndex, + }, + test.remoteChain, + test.remoteLog, + ) + gotErr := err != nil + if test.expectErr != gotErr { + t.Fatalf("expected error: %v, got: %v, "+ + "error:%v", test.expectErr, gotErr, err) + } + + // If our lookup failed, we do not need to check parent + // index. + if err != nil { + return + } + + if parent.HtlcIndex != test.expectedIndex { + t.Fatalf("expected parent index: %v, got: %v", + test.parentIndex, parent.HtlcIndex) + } + }) + + } +}