Merge pull request #10273 from ziggie1984/fix-nursery-height-hint

fix height hint Zero issue in utxonursery

Author: Roasbeef

contractcourt/utxonursery.go

@@ -242,6 +242,71 @@ func NewUtxoNursery(cfg *NurseryConfig) *UtxoNursery {
 	}
 }
 
+// patchZeroHeightHint handles the edge case where a crib output has expiry=0
+// due to a historical bug. This should never happen in normal operation, but
+// we provide a fallback mechanism using the channel close height to determine
+// a valid height hint for the chain notifier.
+//
+// This function returns a height hint that ensures we don't miss confirmations
+// while avoiding the chain notifier's requirement that height hints must
+// be > 0.
+func (u *UtxoNursery) patchZeroHeightHint(baby *babyOutput,
+	classHeight uint32) (uint32, error) {
+
+	if classHeight != 0 {
+		// Normal case - return the original height.
+		return classHeight, nil
+	}
+
+	utxnLog.Warnf("Detected crib output %v with expiry=0, "+
+		"attempting to use fallback height hint from channel "+
+		"close summary", baby.OutPoint())
+
+	// Try to get the channel close height as a fallback.
+	chanPoint := baby.OriginChanPoint()
+	closeSummary, err := u.cfg.FetchClosedChannel(chanPoint)
+	if err != nil {
+		return 0, fmt.Errorf("cannot fetch close summary for "+
+			"channel %v to determine fallback height hint: %w",
+			chanPoint, err)
+	}
+
+	heightHint := closeSummary.CloseHeight
+
+	// If the close height is 0, we try to use the short channel ID block
+	// height as a fallback.
+	if heightHint == 0 {
+		if closeSummary.ShortChanID.BlockHeight == 0 {
+			return 0, fmt.Errorf("cannot use fallback height " +
+				"hint: close height is 0 and short " +
+				"channel ID block height is 0")
+		}
+
+		heightHint = closeSummary.ShortChanID.BlockHeight
+	}
+
+	// At this point the height hint should normally be greater than the
+	// conf depth since channels should have a minimum close height of the
+	// segwit activation height and the conf depth which is a config
+	// parameter should be in the single digit range.
+	if heightHint <= u.cfg.ConfDepth {
+		return 0, fmt.Errorf("cannot use fallback height hint: "+
+			"fallback height hint %v <= confirmation depth %v",
+			heightHint, u.cfg.ConfDepth)
+	}
+
+	// Use the close height minus the confirmation depth as a conservative
+	// height hint. This ensures we don't miss the confirmation even if it
+	// happened around the close height.
+	heightHint -= u.cfg.ConfDepth
+
+	utxnLog.Infof("Using fallback height hint %v for crib output "+
+		"%v (channel closed at height %v, conf depth %v)", heightHint,
+		baby.OutPoint(), closeSummary.CloseHeight, u.cfg.ConfDepth)
+
+	return heightHint, nil
+}
+
 // Start launches all goroutines the UtxoNursery needs to properly carry out
 // its duties.
 func (u *UtxoNursery) Start() error {
@@ -967,7 +1032,19 @@ func (u *UtxoNursery) sweepCribOutput(classHeight uint32, baby *babyOutput) erro
 		return err
 	}
 
-	return u.registerTimeoutConf(baby, classHeight)
+	// Determine the height hint to use for the confirmation notification.
+	// In the normal case, we use classHeight (which is the expiry height).
+	// However, due to a historical bug, some outputs were stored with
+	// expiry=0. For these cases, we need to use a fallback height hint
+	// based on the channel close height to avoid errors from the chain
+	// notifier which requires height hints > 0.
+	heightHint, err := u.patchZeroHeightHint(baby, classHeight)
+	if err != nil {
+		return fmt.Errorf("cannot determine height hint for "+
+			"crib output with expiry=0: %w", err)
+	}
+
+	return u.registerTimeoutConf(baby, heightHint)
 }
 
 // registerTimeoutConf is responsible for subscribing to confirmation

contractcourt/utxonursery_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lntest/mock"
 	"github.com/lightningnetwork/lnd/lnwallet"
+	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/sweep"
 	"github.com/stretchr/testify/require"
 )
@@ -1262,3 +1263,197 @@ func TestKidOutputDecode(t *testing.T) {
 		})
 	}
 }
+
+// TestPatchZeroHeightHint tests the patchZeroHeightHint function to ensure
+// it correctly handles both normal cases and the edge case where classHeight
+// is zero due to a historical bug.
+func TestPatchZeroHeightHint(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		classHeight    uint32
+		closeHeight    uint32
+		confDepth      uint32
+		shortChanID    lnwire.ShortChannelID
+		fetchError     error
+		expectedHeight uint32
+		expectError    bool
+		errorContains  string
+	}{
+		{
+			name:           "normal case - non-zero class height",
+			classHeight:    100,
+			closeHeight:    200,
+			confDepth:      6,
+			shortChanID:    lnwire.ShortChannelID{BlockHeight: 50},
+			expectedHeight: 100,
+			expectError:    false,
+		},
+		{
+			name: "zero class height - fetch closed " +
+				"channel error",
+			classHeight:   0,
+			closeHeight:   100,
+			confDepth:     6,
+			shortChanID:   lnwire.ShortChannelID{BlockHeight: 50},
+			fetchError:    fmt.Errorf("channel not found"),
+			expectError:   true,
+			errorContains: "cannot fetch close summary",
+		},
+		{
+			name: "zero class height - both close " +
+				"height and short chan ID = 0",
+			classHeight:    0,
+			closeHeight:    0,
+			confDepth:      6,
+			shortChanID:    lnwire.ShortChannelID{BlockHeight: 0},
+			expectedHeight: 0,
+			expectError:    true,
+			errorContains: "cannot use fallback height hint: " +
+				"close height is 0 and short channel " +
+				"ID block height is 0",
+		},
+		{
+			name: "zero class height - fallback height hint " +
+				"= conf depth",
+			classHeight:    0,
+			closeHeight:    6,
+			confDepth:      6,
+			shortChanID:    lnwire.ShortChannelID{BlockHeight: 50},
+			expectedHeight: 0,
+			expectError:    true,
+			errorContains: "fallback height hint 6 <= " +
+				"confirmation depth 6",
+		},
+		{
+			name: "zero class height - fallback height hint " +
+				"< conf depth",
+			classHeight:    0,
+			closeHeight:    3,
+			confDepth:      6,
+			shortChanID:    lnwire.ShortChannelID{BlockHeight: 50},
+			expectedHeight: 0,
+			expectError:    true,
+			errorContains: "fallback height hint 3 <= " +
+				"confirmation depth 6",
+		},
+		{
+			name: "zero class height - close " +
+				"height = 0, fallback height hint = conf depth",
+			classHeight: 0,
+			closeHeight: 0,
+			confDepth:   6,
+			shortChanID: lnwire.ShortChannelID{BlockHeight: 6},
+			expectError: true,
+			errorContains: "fallback height hint 6 <= " +
+				"confirmation depth 6",
+		},
+		{
+			name: "zero class height - close " +
+				"height = 0, fallback height hint < conf depth",
+			classHeight:    0,
+			closeHeight:    0,
+			confDepth:      6,
+			shortChanID:    lnwire.ShortChannelID{BlockHeight: 3},
+			expectedHeight: 0,
+			expectError:    true,
+			errorContains: "fallback height hint 3 <= " +
+				"confirmation depth 6",
+		},
+		{
+			name: "zero class height, fallback height is " +
+				"valid",
+			classHeight: 0,
+			closeHeight: 100,
+			confDepth:   6,
+			shortChanID: lnwire.ShortChannelID{BlockHeight: 50},
+			// heightHint - confDepth = 100 - 6 = 94.
+			expectedHeight: 94,
+			expectError:    false,
+		},
+		{
+			name: "zero class height - close " +
+				"height = 0, fallback height is valid",
+			classHeight: 0,
+			closeHeight: 0,
+			confDepth:   6,
+			shortChanID: lnwire.ShortChannelID{BlockHeight: 50},
+			// heightHint - confDepth = 50 - 6 = 44.
+			expectedHeight: 44,
+			expectError:    false,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			// Create a mock baby output.
+			chanPoint := &wire.OutPoint{
+				Hash: [chainhash.HashSize]byte{
+					0x51, 0xb6, 0x37, 0xd8, 0xfc, 0xd2,
+					0xc6, 0xda, 0x48, 0x59, 0xe6, 0x96,
+					0x31, 0x13, 0xa1, 0x17, 0x2d, 0xe7,
+					0x93, 0xe4, 0xb7, 0x25, 0xb8, 0x4d,
+					0x1f, 0xb, 0x4c, 0xf9, 0x9e, 0xc5,
+					0x8c, 0xe9,
+				},
+				Index: 9,
+			}
+
+			baby := &babyOutput{
+				expiry: tc.classHeight,
+				kidOutput: kidOutput{
+					breachedOutput: breachedOutput{
+						outpoint: *chanPoint,
+					},
+					originChanPoint: *chanPoint,
+				},
+			}
+
+			cfg := &NurseryConfig{
+				ConfDepth: tc.confDepth,
+				FetchClosedChannel: func(
+					chanID *wire.OutPoint) (
+					*channeldb.ChannelCloseSummary,
+					error) {
+
+					if tc.fetchError != nil {
+						return nil, tc.fetchError
+					}
+
+					return &channeldb.ChannelCloseSummary{
+						CloseHeight: tc.closeHeight,
+						ShortChanID: tc.shortChanID,
+					}, nil
+				},
+			}
+
+			nursery := &UtxoNursery{
+				cfg: cfg,
+			}
+
+			resultHeight, err := nursery.patchZeroHeightHint(
+				baby, tc.classHeight,
+			)
+
+			if tc.expectError {
+				require.Error(t, err)
+				if tc.errorContains != "" {
+					require.Contains(
+						t, err.Error(),
+						tc.errorContains,
+					)
+				}
+
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedHeight, resultHeight)
+		})
+	}
+}

docs/release-notes/release-notes-0.20.0.md

@@ -46,6 +46,10 @@
   sweeper where some outputs would not be resolved due to an error string
   mismatch.
 
+- [Fixed](https://github.com/lightningnetwork/lnd/pull/10273) a case in the
+  utxonursery (the legacy sweeper) where htlcs with a locktime of 0 would not
+  be swept.
+
 # New Features
 
 * Use persisted [nodeannouncement](https://github.com/lightningnetwork/lnd/pull/8825)