channeldb: store extra HTLC data in variable onion blob slot

Take advantage of the variable byte encoding for a known length field
to extend HLTCs with additional data.

channeldb/channel.go

@@ -198,6 +198,10 @@ var (
 	// ErrMissingIndexEntry is returned when a caller attempts to close a
 	// channel and the outpoint is missing from the index.
 	ErrMissingIndexEntry = fmt.Errorf("missing outpoint from index")
+
+	// ErrOnionBlobLength is returned is an onion blob with incorrect
+	// length is read from disk.
+	ErrOnionBlobLength = errors.New("onion blob < 1366 bytes")
 )
 
 const (
@@ -2068,11 +2072,39 @@ type HTLC struct {
 	// from the HtlcIndex as this will be incremented for each new log
 	// update added.
 	LogIndex uint64
+
+	// ExtraData contains any additional information that was transmitted
+	// with the HTLC via TLVs. This data *must* already be encoded as a
+	// TLV stream, and may be empty. The length of this data is naturally
+	// limited by the space available to TLVs in update_add_htlc:
+	// = 65535 bytes (bolt 8 maximum message size):
+	// - 2 bytes (bolt 1 message_type)
+	// - 32 bytes (channel_id)
+	// - 8 bytes (id)
+	// - 8 bytes (amount_msat)
+	// - 32 bytes (payment_hash)
+	// - 4 bytes (cltv_expiry
+	// - 1366 bytes (onion_routing_packet)
+	// = 64083 bytes maximum possible TLV stream
+	//
+	// Note that this extra data is stored inline with the OnionBlob for
+	// legacy reasons, see serialization/deserialization functions for
+	// detail.
+	ExtraData []byte
 }
 
 // SerializeHtlcs writes out the passed set of HTLC's into the passed writer
 // using the current default on-disk serialization format.
 //
+// This inline serialization has been extended to allow storage of extra data
+// associated with a HTLC in the following way:
+//   - The known-length onion blob (1366 bytes) is serialized as var bytes in
+//     WriteElements (ie, the length 1366 was written, followed by the 1366
+//     onion bytes).
+//   - To include extra data, we append any extra data present to this one
+//     variable length of data. Since we know that the onion is strictly 1366
+//     bytes, any length after that should be considered to be extra data.
+//
 // NOTE: This API is NOT stable, the on-disk format will likely change in the
 // future.
 func SerializeHtlcs(b io.Writer, htlcs ...HTLC) error {
@@ -2082,9 +2114,17 @@ func SerializeHtlcs(b io.Writer, htlcs ...HTLC) error {
 	}
 
 	for _, htlc := range htlcs {
+		// The onion blob and hltc data are stored as a single var
+		// bytes blob.
+		onionAndExtraData := make(
+			[]byte, lnwire.OnionPacketSize+len(htlc.ExtraData),
+		)
+		copy(onionAndExtraData, htlc.OnionBlob[:])
+		copy(onionAndExtraData[lnwire.OnionPacketSize:], htlc.ExtraData)
+
 		if err := WriteElements(b,
 			htlc.Signature, htlc.RHash, htlc.Amt, htlc.RefundTimeout,
-			htlc.OutputIndex, htlc.Incoming, htlc.OnionBlob[:],
+			htlc.OutputIndex, htlc.Incoming, onionAndExtraData,
 			htlc.HtlcIndex, htlc.LogIndex,
 		); err != nil {
 			return err
@@ -2098,6 +2138,17 @@ func SerializeHtlcs(b io.Writer, htlcs ...HTLC) error {
 // io.Reader. The bytes within the passed reader MUST have been previously
 // written to using the SerializeHtlcs function.
 //
+// This inline deserialization has been extended to allow storage of extra data
+// associated with a HTLC in the following way:
+//   - The known-length onion blob (1366 bytes) and any additional data present
+//     are read out as a single blob of variable byte data.
+//   - They are stored like this to take advantage of the variable space
+//     available for extension without migration (see SerializeHtlcs).
+//   - The first 1366 bytes are interpreted as the onion blob, and any remaining
+//     bytes as extra HTLC data.
+//   - This extra HTLC data is expected to be serialized as a TLV stream, and
+//     its parsing is left to higher layers.
+//
 // NOTE: This API is NOT stable, the on-disk format will likely change in the
 // future.
 func DeserializeHtlcs(r io.Reader) ([]HTLC, error) {
@@ -2113,17 +2164,41 @@ func DeserializeHtlcs(r io.Reader) ([]HTLC, error) {
 
 	htlcs = make([]HTLC, numHtlcs)
 	for i := uint16(0); i < numHtlcs; i++ {
-		var onionBlob []byte
+		var onionAndExtraData []byte
 		if err := ReadElements(r,
 			&htlcs[i].Signature, &htlcs[i].RHash, &htlcs[i].Amt,
 			&htlcs[i].RefundTimeout, &htlcs[i].OutputIndex,
-			&htlcs[i].Incoming, &onionBlob,
+			&htlcs[i].Incoming, &onionAndExtraData,
 			&htlcs[i].HtlcIndex, &htlcs[i].LogIndex,
 		); err != nil {
 			return htlcs, err
 		}
 
-		copy(htlcs[i].OnionBlob[:], onionBlob)
+		// Sanity check that we have at least the onion blob size we
+		// expect.
+		if len(onionAndExtraData) < lnwire.OnionPacketSize {
+			return nil, ErrOnionBlobLength
+		}
+
+		// First OnionPacketSize bytes are our fixed length onion
+		// packet.
+		copy(
+			htlcs[i].OnionBlob[:],
+			onionAndExtraData[0:lnwire.OnionPacketSize],
+		)
+
+		// Any additional bytes belong to extra data. ExtraDataLen
+		// will be >= 0, because we know that we always have a fixed
+		// length onion packet.
+		extraDataLen := len(onionAndExtraData) - lnwire.OnionPacketSize
+		if extraDataLen > 0 {
+			htlcs[i].ExtraData = make([]byte, extraDataLen)
+
+			copy(
+				htlcs[i].ExtraData,
+				onionAndExtraData[lnwire.OnionPacketSize:],
+			)
+		}
 	}
 
 	return htlcs, nil

channeldb/channel_test.go

@@ -1531,3 +1531,105 @@ func TestFinalHtlcs(t *testing.T) {
 	_, err = cdb.LookupFinalHtlc(chanID, unknownHtlcID)
 	require.ErrorIs(t, err, ErrHtlcUnknown)
 }
+
+// TestHTLCsExtraData tests serialization and deserialization of HTLCs
+// combined with extra data.
+func TestHTLCsExtraData(t *testing.T) {
+	t.Parallel()
+
+	mockHtlc := HTLC{
+		Signature:     testSig.Serialize(),
+		Incoming:      false,
+		Amt:           10,
+		RHash:         key,
+		RefundTimeout: 1,
+		OnionBlob:     lnmock.MockOnion(),
+	}
+
+	testCases := []struct {
+		name  string
+		htlcs []HTLC
+	}{
+		{
+			// Serialize multiple HLTCs with no extra data to
+			// assert that there is no regression for HTLCs with
+			// no extra data.
+			name: "no extra data",
+			htlcs: []HTLC{
+				mockHtlc, mockHtlc,
+			},
+		},
+		{
+			name: "mixed extra data",
+			htlcs: []HTLC{
+				mockHtlc,
+				{
+					Signature:     testSig.Serialize(),
+					Incoming:      false,
+					Amt:           10,
+					RHash:         key,
+					RefundTimeout: 1,
+					OnionBlob:     lnmock.MockOnion(),
+					ExtraData:     []byte{1, 2, 3},
+				},
+				mockHtlc,
+				{
+					Signature:     testSig.Serialize(),
+					Incoming:      false,
+					Amt:           10,
+					RHash:         key,
+					RefundTimeout: 1,
+					OnionBlob:     lnmock.MockOnion(),
+					ExtraData: bytes.Repeat(
+						[]byte{9}, 999,
+					),
+				},
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		testCase := testCase
+
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+
+			var b bytes.Buffer
+			err := SerializeHtlcs(&b, testCase.htlcs...)
+			require.NoError(t, err)
+
+			r := bytes.NewReader(b.Bytes())
+			htlcs, err := DeserializeHtlcs(r)
+			require.NoError(t, err)
+			require.Equal(t, testCase.htlcs, htlcs)
+		})
+	}
+}
+
+// TestOnionBlobIncorrectLength tests HTLC deserialization in the case where
+// the OnionBlob saved on disk is of an unexpected length. This error case is
+// only expected in the case of database corruption (or some severe protocol
+// breakdown/bug). A HTLC is manually serialized because we cannot force a
+// case where we write an onion blob of incorrect length.
+func TestOnionBlobIncorrectLength(t *testing.T) {
+	t.Parallel()
+
+	var b bytes.Buffer
+
+	var numHtlcs uint16 = 1
+	require.NoError(t, WriteElement(&b, numHtlcs))
+
+	require.NoError(t, WriteElements(
+		&b,
+		// Number of HTLCs.
+		numHtlcs,
+		// Signature, incoming, amount, Rhash, Timeout.
+		testSig.Serialize(), false, lnwire.MilliSatoshi(10), key,
+		uint32(1),
+		// Write an onion blob that is half of our expected size.
+		bytes.Repeat([]byte{1}, lnwire.OnionPacketSize/2),
+	))
+
+	_, err := DeserializeHtlcs(&b)
+	require.ErrorIs(t, err, ErrOnionBlobLength)
+}