[bug]: mission control migration fails for nil payment failure message

[bitromortac]

Running v0.19.0-rc3 (migrating from 0.18.5-beta), upon startup the mission control migration (#9167) fails with

[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x18cc7a0]

goroutine 1 [running]:
github.com/lightningnetwork/lnd/channeldb/migration/lnwire21.EncodeFailureMessage({0x2d49a80, 0xc00087afc0}, {0x0, 0x0}, 0x0)
        github.com/lightningnetwork/lnd/channeldb/migration/lnwire21/onion_error.go:1354 +0x40
github.com/lightningnetwork/lnd/channeldb/migration32.encodeFailureMessage({0x2d49a80, 0xc00087af90}, {0x24f5ee0?, 0xc00087af20}, 0xc00087af30?)
        github.com/lightningnetwork/lnd/channeldb/migration32/mission_control_store.go:327 +0x76
github.com/lightningnetwork/lnd/channeldb/migration32.(*failureMessage).Record.func1()
        github.com/lightningnetwork/lnd/channeldb/migration32/mission_control_store.go:312 +0x55
github.com/lightningnetwork/lnd/tlv.(*Record).Size(...)
        github.com/lightningnetwork/lnd/tlv@v1.3.0/record.go:86
github.com/lightningnetwork/lnd/tlv.(*Stream).Encode(0xc0019499e0, {0x2d49a80, 0xc00087af30})
        github.com/lightningnetwork/lnd/tlv@v1.3.0/stream.go:98 +0xc9
github.com/lightningnetwork/lnd/channeldb/migration/lnwire21.EncodeRecordsTo({0x2d49a80?, 0xc00087af30?}, {0xc001633180?, 0xc000083c08?, 0x2?})
        github.com/lightningnetwork/lnd/channeldb/migration/lnwire21/custom_records.go:224 +0x156
github.com/lightningnetwork/lnd/channeldb/migration32.encodePaymentFailureInfo({0x2d49a80, 0xc00087af30}, {0x2415b20?, 0xc00087af08?}, 0xc00087aed0?)
        github.com/lightningnetwork/lnd/channeldb/migration32/mission_control_store.go:266 +0xa5

This happens because of a nil pointer dereference here

lnd/channeldb/migration/lnwire21/onion_error.go

Line 1354 in b34afa3

code := uint16(failure.Code())

because failure is a nil FailureMessage interface.

At the moment, msg is expected to be set as a tlv record (failureMessage is a wrapper type around FailureMessage).

lnd/routing/missioncontrol.go

Lines 922 to 926 in b34afa3

	// paymentFailureInfo holds additional information about a payment failure.
	type paymentFailureInfo struct {
	sourceIdx tlv.RecordT[tlv.TlvType0, uint8]
	msg tlv.RecordT[tlv.TlvType1, failureMessage]
	}

I had 8 such cases in the db where payment failures originate for example from the second hop. I'm unsure why the failure message is nil in the first place, which could be a bug by itself.

A potential fix could be to make msg an optional tlv record for the migration to retain any info (but glances over a potential bug), a different one could be to migrate a nil failure message to CodeNone.

[ziggie1984]

We can persist a nil failure message see here:

lnd/routing/payment_lifecycle.go

Lines 858 to 866 in b34afa3

	if errors.Is(sendErr, htlcswitch.ErrUnreadableFailureMessage) {
	log.Warn("Unreadable failure when sending htlc: id=%v, hash=%v",
	attempt.AttemptID, attempt.Hash)
	// Since this error message cannot be decrypted, we will send a
	// nil error message to our mission controller and fail the
	// payment.
	return reportAndFail(nil, nil)
	}

Still checking but I think making the msg optional might work.

[bitromortac]

We can persist a nil failure message see here

I think that in my case that was not the path since a srcIdx of 2 was set.

[ziggie1984]

Can we given the date of the payment somehow check which version of LND was running ?

[bitromortac]

Good point, I have one failure from August 2022, not really worth keeping those :).

[ziggie1984]

So I think we should go with your solution because we might have problems decoding the error:

lnd/htlcswitch/failure.go

Lines 189 to 191 in b34afa3

	if err != nil {
	return NewUnknownForwardingError(failure.SenderIdx), nil
	}

hence the error msg might be nil sometimes.