Repository: lightningnetwork/lnd (language: go)

Issue:
[bug]: `panic: runtime error` resuming HTLC attempt
### Background

On LND 0.19.0 rc2 during startup this panic was observed:
```
2025-04-08 18:51:38.941 [INF] CRTR: Resuming HTLC attempt 35020 for payment 53a3012ec149f3a478e13a7e16617fca658f87e80f26c8b5ffa7091be9ce34a6
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x19c407e]
goroutine 1191 [running]:
github.com/lightningnetwork/lnd/channeldb.(*HTLCAttemptInfo).attachOnionBlobAndCircuit(0xc00775ca08)
    github.com/lightningnetwork/lnd/channeldb/mp_payment.go:123 +0x1e
github.com/lightningnetwork/lnd/channeldb.(*HTLCAttemptInfo).Circuit(...)
    github.com/lightningnetwork/lnd/channeldb/mp_payment.go:111
github.com/lightningnetwork/lnd/routing.(*paymentLifecycle).collectResult(0xc00509a180, 0xc00775ca08)
    github.com/lightningnetwork/lnd/routing/payment_lifecycle.go:508 +0x11a
github.com/lightningnetwork/lnd/routing.(*paymentLifecycle).collectResultAsync.func1()
    github.com/lightningnetwork/lnd/routing/payment_lifecycle.go:464 +0x31
created by github.com/lightningnetwork/lnd/routing.(*paymentLifecycle).collectResultAsync in goroutine 1068
    github.com/lightningnetwork/lnd/routing/payment_lifecycle.go:463 +0x12a
```

Line for ref:
```
    onionBlob, circuit, err := GenerateSphinxPacket(
        &h.Route, h.Hash[:], h.SessionKey(),
    )
```

Based on the valid memory address, `h` itself is not `nil`, so the culprit likely lies in the other fields (i.e. `Route`, `Hash`, etc.)

### Your environment

* version of `lnd`: 0.19 rc2
* which operating system (`uname -a` on *Nix): Linux 5.10.225
* version of `btcd`, `bitcoind`, or other backend: bitcoind

### Steps to reproduce

Upgrade instance to LND 0.19 rc2.

### Expected behaviour

Start up.

### Actual behaviour

Runtime panic.



Discussion / hints:
It looks like the panic is from `h.Hash[:]`, caused by the `[:]` part since `Route` is a concrete struct. So this only happens in 0.19 rc2?
> It looks like the panic is from `h.Hash[:]`, caused by the `[:]` part since `Route` is a concrete struct. So this only happens in 0.19 rc2?

Have not tried to repro in other versions, noticed it during an upgrade from 0.18.3.

Fix patch:
diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md
index 7ebc540375..7eb5150f0a 100644
--- a/docs/release-notes/release-notes-0.19.0.md
+++ b/docs/release-notes/release-notes-0.19.0.md
@@ -111,6 +111,9 @@ keysend payment validation is stricter.
   process of the node won't be interrupted if a non-fatal error is returned from
   the subsystems.
 
+* [Fixed](https://github.com/lightningnetwork/lnd/pull/9703) a possible panic
+  when reloading legacy inflight payments which don't have the MPP feature.
+
 # New Features
 
 * Add support for [archiving channel backup](https://github.com/lightningnetwork/lnd/pull/9232)
diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go
index 9a74140793..a64c8c87e6 100644
--- a/routing/payment_lifecycle.go
+++ b/routing/payment_lifecycle.go
@@ -14,6 +14,7 @@ import (
 	"github.com/lightningnetwork/lnd/graph/db/models"
 	"github.com/lightningnetwork/lnd/htlcswitch"
 	"github.com/lightningnetwork/lnd/lntypes"
+	"github.com/lightningnetwork/lnd/lnutils"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/routing/route"
 	"github.com/lightningnetwork/lnd/routing/shards"
@@ -500,7 +501,8 @@ func (p *paymentLifecycle) collectResultAsync(attempt *channeldb.HTLCAttempt) {
 func (p *paymentLifecycle) collectResult(
 	attempt *channeldb.HTLCAttempt) (*htlcswitch.PaymentResult, error) {
 
-	log.Tracef("Collecting result for attempt %v", spew.Sdump(attempt))
+	log.Tracef("Collecting result for attempt %v",
+		lnutils.SpewLogClosure(attempt))
 
 	result := &htlcswitch.PaymentResult{}
 
@@ -1060,6 +1062,38 @@ func marshallError(sendError error, time time.Time) *channeldb.HTLCFailInfo {
 	return response
 }
 
+// patchLegacyPaymentHash will make a copy of the passed attempt and sets its
+// Hash field to be the payment hash if it's nil.
+//
+// NOTE: For legacy payments, which were created before the AMP feature was
+// enabled, the `Hash` field in their HTLC attempts is nil. In that case, we use
+// the payment hash as the `attempt.Hash` as they are identical.
+func (p *paymentLifecycle) patchLegacyPaymentHash(
+	a channeldb.HTLCAttempt) channeldb.HTLCAttempt {
+
+	// Exit early if this is not a legacy attempt.
+	if a.Hash != nil {
+		return a
+	}
+
+	// Log a warning if the user is still using legacy payments, which has
+	// weaker support.
+	log.Warnf("Found legacy htlc attempt %v in payment %v", a.AttemptID,
+		p.identifier)
+
+	// Set the attempt's hash to be the payment hash, which is the payment's
+	// `PaymentHash`` in the `PaymentCreationInfo`. For legacy payments
+	// before AMP feature, the `Hash` field was not set so we use the
+	// payment hash instead.
+	//
+	// NOTE: During the router's startup, we have a similar logic in
+	// `resumePayments`, in which we will use the payment hash instead if
+	// the attempt's hash is nil.
+	a.Hash = &p.identifier
+
+	return a
+}
+
 // reloadInflightAttempts is called when the payment lifecycle is resumed after
 // a restart. It reloads all inflight attempts from the control tower and
 // collects the results of the attempts that have been sent before.
@@ -1075,6 +1109,10 @@ func (p *paymentLifecycle) reloadInflightAttempts() (DBMPPayment, error) {
 		log.Infof("Resuming HTLC attempt %v for payment %v",
 			a.AttemptID, p.identifier)
 
+		// Potentially attach the payment hash to the `Hash` field if
+		// it's a legacy payment.
+		a = p.patchLegacyPaymentHash(a)
+
 		p.resultCollector(&a)
 	}
 


Test patch:
diff --git a/routing/payment_lifecycle_test.go b/routing/payment_lifecycle_test.go
index b04b04d4be..9df02ec328 100644
--- a/routing/payment_lifecycle_test.go
+++ b/routing/payment_lifecycle_test.go
@@ -1809,3 +1809,61 @@ func TestHandleAttemptResultSuccess(t *testing.T) {
 	require.NoError(t, err, "expected no error")
 	require.Equal(t, attempt, attemptResult.attempt)
 }
+
+// TestReloadInflightAttemptsLegacy checks that when handling a legacy HTLC
+// attempt, `collectResult` behaves as expected.
+func TestReloadInflightAttemptsLegacy(t *testing.T) {
+	t.Parallel()
+
+	// Create a test paymentLifecycle with the initial two calls mocked.
+	p, m := newTestPaymentLifecycle(t)
+
+	// Mount the resultCollector to check the full call path.
+	p.resultCollector = p.collectResultAsync
+
+	// Create testing params.
+	paymentAmt := 10_000
+	preimage := lntypes.Preimage{1}
+	attempt := makeSettledAttempt(t, paymentAmt, preimage)
+
+	// Make the attempt.Hash to be nil to mock a legacy payment.
+	attempt.Hash = nil
+
+	// Create a mock result returned from the switch.
+	result := &htlcswitch.PaymentResult{
+		Preimage: preimage,
+	}
+
+	// 1. calls `FetchPayment` and return the payment.
+	m.control.On("FetchPayment", p.identifier).Return(m.payment, nil).Once()
+
+	// 2. calls `InFlightHTLCs` and return the attempt.
+	attempts := []channeldb.HTLCAttempt{*attempt}
+	m.payment.On("InFlightHTLCs").Return(attempts).Once()
+
+	// 3. Mock the htlcswitch to return a the result chan.
+	resultChan := make(chan *htlcswitch.PaymentResult, 1)
+	m.payer.On("GetAttemptResult",
+		attempt.AttemptID, p.identifier, mock.Anything,
+	).Return(resultChan, nil).Once().Run(func(args mock.Arguments) {
+		// Send the preimage to the result chan.
+		resultChan <- result
+	})
+
+	// Now call the method under test.
+	payment, err := p.reloadInflightAttempts()
+	require.NoError(t, err)
+	require.Equal(t, m.payment, payment)
+
+	var r *switchResult
+
+	// Assert the result is returned within testTimeout.
+	waitErr := wait.NoError(func() error {
+		r = <-p.resultCollected
+		return nil
+	}, testTimeout)
+	require.NoError(t, waitErr, "timeout waiting for result")
+
+	// Assert the result is received as expected.
+	require.Equal(t, result, r.result)
+}