diff --git a/contractcourt/utxonursery.go b/contractcourt/utxonursery.go index af4a8cd96be..fa0f1868c63 100644 --- a/contractcourt/utxonursery.go +++ b/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 diff --git a/contractcourt/utxonursery_test.go b/contractcourt/utxonursery_test.go index c06301a067b..5dcf8c78142 100644 --- a/contractcourt/utxonursery_test.go +++ b/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) + }) + } +} diff --git a/docs/release-notes/release-notes-0.20.0.md b/docs/release-notes/release-notes-0.20.0.md index 26e45114be3..e91bc168237 100644 --- a/docs/release-notes/release-notes-0.20.0.md +++ b/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)