lnwallet: extract fetchparent for individual testing

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

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)
+			}
+		})
+
+	}
+}