Merge pull request #7710 from carlaKC/7297-updateaddtlvs

Channeldb: Store HTLC Extra TLVs in Onion Blob Varbytes

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 (
@@ -2001,7 +2005,7 @@ func (c *OpenChannel) ActiveHtlcs() []HTLC {
 	// which ones are present on their commitment.
 	remoteHtlcs := make(map[[32]byte]struct{})
 	for _, htlc := range c.RemoteCommitment.Htlcs {
-		onionHash := sha256.Sum256(htlc.OnionBlob)
+		onionHash := sha256.Sum256(htlc.OnionBlob[:])
 		remoteHtlcs[onionHash] = struct{}{}
 	}
 
@@ -2009,7 +2013,7 @@ func (c *OpenChannel) ActiveHtlcs() []HTLC {
 	// as active if *we* know them as well.
 	activeHtlcs := make([]HTLC, 0, len(remoteHtlcs))
 	for _, htlc := range c.LocalCommitment.Htlcs {
-		onionHash := sha256.Sum256(htlc.OnionBlob)
+		onionHash := sha256.Sum256(htlc.OnionBlob[:])
 		if _, ok := remoteHtlcs[onionHash]; !ok {
 			continue
 		}
@@ -2056,7 +2060,7 @@ type HTLC struct {
 
 	// OnionBlob is an opaque blob which is used to complete multi-hop
 	// routing.
-	OnionBlob []byte
+	OnionBlob [lnwire.OnionPacketSize]byte
 
 	// HtlcIndex is the HTLC counter index of this active, outstanding
 	// HTLC. This differs from the LogIndex, as the HtlcIndex is only
@@ -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,14 +2164,41 @@ func DeserializeHtlcs(r io.Reader) ([]HTLC, error) {
 
 	htlcs = make([]HTLC, numHtlcs)
 	for i := uint16(0); i < numHtlcs; i++ {
+		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, &htlcs[i].OnionBlob,
+			&htlcs[i].Incoming, &onionAndExtraData,
 			&htlcs[i].HtlcIndex, &htlcs[i].LogIndex,
 		); err != nil {
 			return htlcs, err
 		}
+
+		// 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

@@ -19,6 +19,7 @@ import (
 	"github.com/lightningnetwork/lnd/clock"
 	"github.com/lightningnetwork/lnd/keychain"
 	"github.com/lightningnetwork/lnd/kvdb"
+	"github.com/lightningnetwork/lnd/lnmock"
 	"github.com/lightningnetwork/lnd/lntest/channels"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/shachain"
@@ -366,12 +367,13 @@ func TestOpenChannelPutGetDelete(t *testing.T) {
 	// Create the test channel state, with additional htlcs on the local
 	// and remote commitment.
 	localHtlcs := []HTLC{
-		{Signature: testSig.Serialize(),
+		{
+			Signature:     testSig.Serialize(),
 			Incoming:      true,
 			Amt:           10,
 			RHash:         key,
 			RefundTimeout: 1,
-			OnionBlob:     []byte("onionblob"),
+			OnionBlob:     lnmock.MockOnion(),
 		},
 	}
 
@@ -382,7 +384,7 @@ func TestOpenChannelPutGetDelete(t *testing.T) {
 			Amt:           10,
 			RHash:         key,
 			RefundTimeout: 1,
-			OnionBlob:     []byte("onionblob"),
+			OnionBlob:     lnmock.MockOnion(),
 		},
 	}
 
@@ -612,8 +614,10 @@ func TestChannelStateTransition(t *testing.T) {
 			LogIndex:      uint64(i * 2),
 			HtlcIndex:     uint64(i),
 		}
-		htlc.OnionBlob = make([]byte, 10)
-		copy(htlc.OnionBlob[:], bytes.Repeat([]byte{2}, 10))
+		copy(
+			htlc.OnionBlob[:],
+			bytes.Repeat([]byte{2}, lnwire.OnionPacketSize),
+		)
 		htlcs = append(htlcs, htlc)
 		htlcAmt += htlc.Amt
 	}
@@ -1527,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)
+}

contractcourt/briefcase_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/kvdb"
+	"github.com/lightningnetwork/lnd/lnmock"
 	"github.com/lightningnetwork/lnd/lntest/channels"
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/stretchr/testify/require"
@@ -745,7 +746,7 @@ func TestCommitSetStorage(t *testing.T) {
 	activeHTLCs := []channeldb.HTLC{
 		{
 			Amt:       1000,
-			OnionBlob: make([]byte, 0),
+			OnionBlob: lnmock.MockOnion(),
 			Signature: make([]byte, 0),
 		},
 	}

contractcourt/htlc_incoming_contest_resolver.go

@@ -487,7 +487,7 @@ func (h *htlcIncomingContestResolver) Supplement(htlc channeldb.HTLC) {
 func (h *htlcIncomingContestResolver) decodePayload() (*hop.Payload,
 	[]byte, error) {
 
-	onionReader := bytes.NewReader(h.htlc.OnionBlob)
+	onionReader := bytes.NewReader(h.htlc.OnionBlob[:])
 	iterator, err := h.OnionProcessor.ReconstructHopIterator(
 		onionReader, h.htlc.RHash[:],
 	)

contractcourt/htlc_incoming_resolver_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/lightningnetwork/lnd/htlcswitch/hop"
 	"github.com/lightningnetwork/lnd/invoices"
 	"github.com/lightningnetwork/lnd/kvdb"
+	"github.com/lightningnetwork/lnd/lnmock"
 	"github.com/lightningnetwork/lnd/lntest/mock"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet"
@@ -29,7 +30,6 @@ var (
 	testResPreimage         = lntypes.Preimage{1, 2, 3}
 	testResHash             = testResPreimage.Hash()
 	testResCircuitKey       = models.CircuitKey{}
-	testOnionBlob           = []byte{4, 5, 6}
 	testAcceptHeight  int32 = 1234
 	testHtlcAmount          = 2300
 )
@@ -139,8 +139,9 @@ func TestHtlcIncomingResolverExitSettle(t *testing.T) {
 
 	ctx.waitForResult(true)
 
+	expetedOnion := lnmock.MockOnion()
 	if !bytes.Equal(
-		ctx.onionProcessor.offeredOnionBlob, testOnionBlob,
+		ctx.onionProcessor.offeredOnionBlob, expetedOnion[:],
 	) {
 
 		t.Fatal("unexpected onion blob")
@@ -375,7 +376,7 @@ func newIncomingResolverTestContext(t *testing.T, isExit bool) *incomingResolver
 			htlc: channeldb.HTLC{
 				Amt:       lnwire.MilliSatoshi(testHtlcAmount),
 				RHash:     testResHash,
-				OnionBlob: testOnionBlob,
+				OnionBlob: lnmock.MockOnion(),
 			},
 		},
 		htlcExpiry: testHtlcExpiry,

contractcourt/htlc_outgoing_contest_resolver_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/kvdb"
+	"github.com/lightningnetwork/lnd/lnmock"
 	"github.com/lightningnetwork/lnd/lntest/mock"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet"
@@ -183,7 +184,7 @@ func newOutgoingResolverTestContext(t *testing.T) *outgoingResolverTestContext {
 			htlc: channeldb.HTLC{
 				Amt:       lnwire.MilliSatoshi(testHtlcAmount),
 				RHash:     testResHash,
-				OnionBlob: testOnionBlob,
+				OnionBlob: lnmock.MockOnion(),
 			},
 		},
 	}

contractcourt/htlc_success_resolver_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/kvdb"
+	"github.com/lightningnetwork/lnd/lnmock"
 	"github.com/lightningnetwork/lnd/lntest/mock"
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwire"
@@ -110,7 +111,7 @@ func newHtlcResolverTestContext(t *testing.T,
 
 	htlc := channeldb.HTLC{
 		RHash:     testResHash,
-		OnionBlob: testOnionBlob,
+		OnionBlob: lnmock.MockOnion(),
 		Amt:       testHtlcAmt,
 	}
 

docs/release-notes/release-notes-0.17.0.md

@@ -36,6 +36,9 @@ package](https://github.com/lightningnetwork/lnd/pull/7356)
 * [Fix unit test flake (TestLightningWallet) in the neutrino package via
   version bump of btcsuite/btcwallet](https://github.com/lightningnetwork/lnd/pull/7049)
 
+* [HTLC serialization updated](https://github.com/lightningnetwork/lnd/pull/7710) 
+  to allow storing extra data transmitted in TLVs.
+
 ## RPC
 
 * [SendOutputs](https://github.com/lightningnetwork/lnd/pull/7631) now adheres

lnmock/routing.go

@@ -0,0 +1,16 @@
+package lnmock
+
+import (
+	"bytes"
+
+	"github.com/lightningnetwork/lnd/lnwire"
+)
+
+// MockOnion returns a mock onion payload.
+func MockOnion() [lnwire.OnionPacketSize]byte {
+	var onion [lnwire.OnionPacketSize]byte
+	onionBlob := bytes.Repeat([]byte{1}, lnwire.OnionPacketSize)
+	copy(onion[:], onionBlob)
+
+	return onion
+}

lnwallet/channel.go

@@ -745,7 +745,6 @@ func (c *commitment) toDiskCommit(ourCommit bool) *channeldb.ChannelCommitment {
 			LogIndex:      htlc.LogIndex,
 			Incoming:      false,
 		}
-		h.OnionBlob = make([]byte, len(htlc.OnionBlob))
 		copy(h.OnionBlob[:], htlc.OnionBlob)
 
 		if ourCommit && htlc.sig != nil {
@@ -770,7 +769,6 @@ func (c *commitment) toDiskCommit(ourCommit bool) *channeldb.ChannelCommitment {
 			LogIndex:      htlc.LogIndex,
 			Incoming:      true,
 		}
-		h.OnionBlob = make([]byte, len(htlc.OnionBlob))
 		copy(h.OnionBlob[:], htlc.OnionBlob)
 
 		if ourCommit && htlc.sig != nil {
@@ -859,7 +857,7 @@ func (lc *LightningChannel) diskHtlcToPayDesc(feeRate chainfee.SatPerKWeight,
 		EntryType:          Add,
 		HtlcIndex:          htlc.HtlcIndex,
 		LogIndex:           htlc.LogIndex,
-		OnionBlob:          htlc.OnionBlob,
+		OnionBlob:          htlc.OnionBlob[:],
 		localOutputIndex:   localOutputIndex,
 		remoteOutputIndex:  remoteOutputIndex,
 		ourPkScript:        ourP2WSH,