Merge pull request #9770 from bitromortac/2504-mc-migration-fix

routing+migration: fix mission control migration with nil failure msg

Author: guggero

channeldb/migration32/migration_test.go

@@ -118,6 +118,32 @@ var (
 		},
 	}
 
+	resultOld3 = paymentResultOld{
+		id:               3,
+		timeFwd:          time.Unix(0, 4),
+		timeReply:        time.Unix(0, 7),
+		success:          false,
+		failure:          nil,
+		failureSourceIdx: &failureIndex,
+		route: &Route{
+			TotalTimeLock: 101,
+			TotalAmount:   401,
+			SourcePubKey:  testPub2,
+			Hops: []*Hop{
+				{
+					PubKeyBytes:      testPub,
+					ChannelID:        800,
+					OutgoingTimeLock: 4,
+					AmtToForward:     4,
+					BlindingPoint:    pubkey,
+					EncryptedData:    []byte{1, 2, 3},
+					CustomRecords:    customRecord,
+					TotalAmtMsat:     600,
+				},
+			},
+		},
+	}
+
 	//nolint:ll
 	resultNew1Hop1 = &mcHop{
 		channelID:   tlv.NewPrimitiveRecord[tlv.TlvType0, uint64](100),
@@ -182,7 +208,7 @@ var (
 		),
 		failure: tlv.SomeRecordT(
 			tlv.NewRecordT[tlv.TlvType3](
-				*newPaymentFailure(
+				newPaymentFailure(
 					&failureIndex,
 					&lnwire.FailFeeInsufficient{},
 				),
@@ -217,6 +243,31 @@ var (
 			}),
 		}),
 	}
+
+	//nolint:ll
+	resultNew3 = paymentResultNew{
+		id: 3,
+		timeFwd: tlv.NewPrimitiveRecord[tlv.TlvType0, uint64](
+			uint64(time.Unix(0, 4).UnixNano()),
+		),
+		timeReply: tlv.NewPrimitiveRecord[tlv.TlvType1, uint64](
+			uint64(time.Unix(0, 7).UnixNano()),
+		),
+		failure: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType3](
+				newPaymentFailure(
+					&failureIndex, nil,
+				),
+			),
+		),
+		route: tlv.NewRecordT[tlv.TlvType2](mcRoute{
+			sourcePubKey: tlv.NewRecordT[tlv.TlvType0](testPub2),
+			totalAmount:  tlv.NewRecordT[tlv.TlvType1, lnwire.MilliSatoshi](401),
+			hops: tlv.NewRecordT[tlv.TlvType2](mcHops{
+				resultNew2Hop1,
+			}),
+		}),
+	}
 )
 
 // TestMigrateMCRouteSerialisation tests that the MigrateMCRouteSerialisation
@@ -225,10 +276,10 @@ var (
 func TestMigrateMCRouteSerialisation(t *testing.T) {
 	var (
 		resultsOld = []*paymentResultOld{
-			&resultOld1, &resultOld2,
+			&resultOld1, &resultOld2, &resultOld3,
 		}
 		expectedResultsNew = []*paymentResultNew{
-			&resultNew1, &resultNew2,
+			&resultNew1, &resultNew2, &resultNew3,
 		}
 	)
 

channeldb/migration32/mission_control_store.go

@@ -93,9 +93,12 @@ func deserializeOldResult(k, v []byte) (*paymentResultOld, error) {
 
 // convertPaymentResult converts a paymentResultOld to a paymentResultNew.
 func convertPaymentResult(old *paymentResultOld) *paymentResultNew {
-	var failure *paymentFailure
+	var failure fn.Option[paymentFailure]
 	if !old.success {
-		failure = newPaymentFailure(old.failureSourceIdx, old.failure)
+		failure = fn.Some(newPaymentFailure(
+			old.failureSourceIdx,
+			old.failure,
+		))
 	}
 
 	return newPaymentResult(
@@ -106,7 +109,7 @@ func convertPaymentResult(old *paymentResultOld) *paymentResultNew {
 
 // newPaymentResult constructs a new paymentResult.
 func newPaymentResult(id uint64, rt *mcRoute, timeFwd, timeReply time.Time,
-	failure *paymentFailure) *paymentResultNew {
+	failure fn.Option[paymentFailure]) *paymentResultNew {
 
 	result := &paymentResultNew{
 		id: id,
@@ -119,11 +122,11 @@ func newPaymentResult(id uint64, rt *mcRoute, timeFwd, timeReply time.Time,
 		route: tlv.NewRecordT[tlv.TlvType2](*rt),
 	}
 
-	if failure != nil {
+	failure.WhenSome(func(f paymentFailure) {
 		result.failure = tlv.SomeRecordT(
-			tlv.NewRecordT[tlv.TlvType3](*failure),
+			tlv.NewRecordT[tlv.TlvType3](f),
 		)
-	}
+	})
 
 	return result
 }
@@ -142,33 +145,49 @@ type paymentResultNew struct {
 	failure tlv.OptionalRecordT[tlv.TlvType3, paymentFailure]
 }
 
-// paymentFailure represents the presence of a payment failure. It may or may
-// not include additional information about said failure.
-type paymentFailure struct {
-	info tlv.OptionalRecordT[tlv.TlvType0, paymentFailureInfo]
-}
-
 // newPaymentFailure constructs a new paymentFailure struct. If the source
 // index is nil, then an empty paymentFailure is returned. This represents a
 // failure with unknown details. Otherwise, the index and failure message are
 // used to populate the info field of the paymentFailure.
 func newPaymentFailure(sourceIdx *int,
-	failureMsg lnwire.FailureMessage) *paymentFailure {
+	failureMsg lnwire.FailureMessage) paymentFailure {
 
+	// If we can't identify a failure source, we also won't have a decrypted
+	// failure message. In this case we return an empty payment failure.
 	if sourceIdx == nil {
-		return &paymentFailure{}
+		return paymentFailure{}
 	}
 
-	info := paymentFailureInfo{
-		sourceIdx: tlv.NewPrimitiveRecord[tlv.TlvType0](
-			uint8(*sourceIdx),
+	info := paymentFailure{
+		sourceIdx: tlv.SomeRecordT(
+			tlv.NewPrimitiveRecord[tlv.TlvType0](
+				uint8(*sourceIdx),
+			),
 		),
-		msg: tlv.NewRecordT[tlv.TlvType1](failureMessage{failureMsg}),
 	}
 
-	return &paymentFailure{
-		info: tlv.SomeRecordT(tlv.NewRecordT[tlv.TlvType0](info)),
+	if failureMsg != nil {
+		info.msg = tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType1](
+				failureMessage{failureMsg},
+			),
+		)
 	}
+
+	return info
+}
+
+// paymentFailure holds additional information about a payment failure.
+type paymentFailure struct {
+	// sourceIdx is the hop the error was reported from. In order to be able
+	// to decrypt the error message, we need to know the source, which is
+	// why an error message can only be present if the source is known.
+	sourceIdx tlv.OptionalRecordT[tlv.TlvType0, uint8]
+
+	// msg is the error why a payment failed. If we identify the failure of
+	// a certain hop at the above index, but aren't able to decode the
+	// failure message we indicate this by not setting this field.
+	msg tlv.OptionalRecordT[tlv.TlvType1, failureMessage]
 }
 
 // Record returns a TLV record that can be used to encode/decode a
@@ -194,14 +213,27 @@ func (r *paymentFailure) Record() tlv.Record {
 func encodePaymentFailure(w io.Writer, val interface{}, _ *[8]byte) error {
 	if v, ok := val.(*paymentFailure); ok {
 		var recordProducers []tlv.RecordProducer
-		v.info.WhenSome(
-			func(r tlv.RecordT[tlv.TlvType0, paymentFailureInfo]) {
-				recordProducers = append(recordProducers, &r)
+
+		v.sourceIdx.WhenSome(
+			func(r tlv.RecordT[tlv.TlvType0, uint8]) {
+				recordProducers = append(
+					recordProducers, &r,
+				)
+			},
+		)
+
+		v.msg.WhenSome(
+			func(r tlv.RecordT[tlv.TlvType1, failureMessage]) {
+				recordProducers = append(
+					recordProducers, &r,
+				)
 			},
 		)
 
 		return lnwire.EncodeRecordsTo(
-			w, lnwire.ProduceRecordsSorted(recordProducers...),
+			w, lnwire.ProduceRecordsSorted(
+				recordProducers...,
+			),
 		)
 	}
 
@@ -210,81 +242,27 @@ func encodePaymentFailure(w io.Writer, val interface{}, _ *[8]byte) error {
 
 func decodePaymentFailure(r io.Reader, val interface{}, _ *[8]byte,
 	l uint64) error {
-
 	if v, ok := val.(*paymentFailure); ok {
 		var h paymentFailure
 
-		info := tlv.ZeroRecordT[tlv.TlvType0, paymentFailureInfo]()
+		sourceIdx := tlv.ZeroRecordT[tlv.TlvType0, uint8]()
+		msg := tlv.ZeroRecordT[tlv.TlvType1, failureMessage]()
+
 		typeMap, err := lnwire.DecodeRecords(
-			r, lnwire.ProduceRecordsSorted(&info)...,
+			r,
+			lnwire.ProduceRecordsSorted(&sourceIdx, &msg)...,
 		)
+
 		if err != nil {
 			return err
 		}
 
-		if _, ok := typeMap[h.info.TlvType()]; ok {
-			h.info = tlv.SomeRecordT(info)
+		if _, ok := typeMap[h.sourceIdx.TlvType()]; ok {
+			h.sourceIdx = tlv.SomeRecordT(sourceIdx)
 		}
 
-		*v = h
-
-		return nil
-	}
-
-	return tlv.NewTypeForDecodingErr(val, "routing.paymentFailure", l, l)
-}
-
-// paymentFailureInfo holds additional information about a payment failure.
-type paymentFailureInfo struct {
-	sourceIdx tlv.RecordT[tlv.TlvType0, uint8]
-	msg       tlv.RecordT[tlv.TlvType1, failureMessage]
-}
-
-// Record returns a TLV record that can be used to encode/decode a
-// paymentFailureInfo to/from a TLV stream.
-func (r *paymentFailureInfo) Record() tlv.Record {
-	recordSize := func() uint64 {
-		var (
-			b   bytes.Buffer
-			buf [8]byte
-		)
-		if err := encodePaymentFailureInfo(&b, r, &buf); err != nil {
-			panic(err)
-		}
-
-		return uint64(len(b.Bytes()))
-	}
-
-	return tlv.MakeDynamicRecord(
-		0, r, recordSize, encodePaymentFailureInfo,
-		decodePaymentFailureInfo,
-	)
-}
-
-func encodePaymentFailureInfo(w io.Writer, val interface{}, _ *[8]byte) error {
-	if v, ok := val.(*paymentFailureInfo); ok {
-		return lnwire.EncodeRecordsTo(
-			w, lnwire.ProduceRecordsSorted(
-				&v.sourceIdx, &v.msg,
-			),
-		)
-	}
-
-	return tlv.NewTypeForEncodingErr(val, "routing.paymentFailureInfo")
-}
-
-func decodePaymentFailureInfo(r io.Reader, val interface{}, _ *[8]byte,
-	l uint64) error {
-
-	if v, ok := val.(*paymentFailureInfo); ok {
-		var h paymentFailureInfo
-
-		_, err := lnwire.DecodeRecords(
-			r,
-			lnwire.ProduceRecordsSorted(&h.sourceIdx, &h.msg)...,
-		)
-		if err != nil {
-			return err
+		if _, ok := typeMap[h.msg.TlvType()]; ok {
+			h.msg = tlv.SomeRecordT(msg)
 		}
 
 		*v = h
@@ -293,7 +271,7 @@ func decodePaymentFailureInfo(r io.Reader, val interface{}, _ *[8]byte,
 	}
 
 	return tlv.NewTypeForDecodingErr(
-		val, "routing.paymentFailureInfo", l, l,
+		val, "routing.paymentFailure", l, l,
 	)
 }
 

docs/release-notes/release-notes-0.19.0.md

@@ -484,10 +484,12 @@ the on going rate we'll permit.
 
 ## Database
 
-* [Migrate the mission control 
-  store](https://github.com/lightningnetwork/lnd/pull/8911) to use a more 
-  minimal encoding for payment attempt routes as well as use [pure TLV 
-  encoding](https://github.com/lightningnetwork/lnd/pull/9167).
+* [Migrate the mission control
+  store](https://github.com/lightningnetwork/lnd/pull/8911) to use a more
+  minimal encoding for payment attempt routes as well as use [pure TLV
+  encoding](https://github.com/lightningnetwork/lnd/pull/9167). [A
+  fix](https://github.com/lightningnetwork/lnd/pull/9770) was added to handle
+  nil routing failure messages and the serialization was optimized.
 
 * [Migrate the mission control 
   store](https://github.com/lightningnetwork/lnd/pull/9001) so that results are