From 2fc8e9d617270bb7fde31951367d1ad8553272c7 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Fri, 19 May 2023 10:58:05 -0400 Subject: [PATCH 1/3] multi: update channel db HTLC OnionBlob to array We know that onion blobs in lightning are _exactly_ 1366 bytes in lightning, but they are currently expressed as a byte slice in channeldb's HTLC struct. Blobs are currently serialized as var bytes, so we can take advantage of this known length and variable length to add additional data to the inline serialization of our HTLCs, which are otherwise not easily extensible (without creating a new bucket). --- channeldb/channel.go | 11 +++++++---- channeldb/channel_test.go | 14 +++++++++----- contractcourt/briefcase_test.go | 3 ++- contractcourt/htlc_incoming_contest_resolver.go | 2 +- contractcourt/htlc_incoming_resolver_test.go | 7 ++++--- .../htlc_outgoing_contest_resolver_test.go | 3 ++- contractcourt/htlc_success_resolver_test.go | 3 ++- lnmock/routing.go | 16 ++++++++++++++++ lnwallet/channel.go | 4 +--- 9 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 lnmock/routing.go diff --git a/channeldb/channel.go b/channeldb/channel.go index dc0bf0058f..f986321964 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -2001,7 +2001,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 +2009,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 +2056,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 @@ -2113,14 +2113,17 @@ func DeserializeHtlcs(r io.Reader) ([]HTLC, error) { htlcs = make([]HTLC, numHtlcs) for i := uint16(0); i < numHtlcs; i++ { + var onionBlob []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, &onionBlob, &htlcs[i].HtlcIndex, &htlcs[i].LogIndex, ); err != nil { return htlcs, err } + + copy(htlcs[i].OnionBlob[:], onionBlob) } return htlcs, nil diff --git a/channeldb/channel_test.go b/channeldb/channel_test.go index ccb6b10a87..7896bcaf44 100644 --- a/channeldb/channel_test.go +++ b/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 } diff --git a/contractcourt/briefcase_test.go b/contractcourt/briefcase_test.go index 8bc0209f13..140a85c220 100644 --- a/contractcourt/briefcase_test.go +++ b/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), }, } diff --git a/contractcourt/htlc_incoming_contest_resolver.go b/contractcourt/htlc_incoming_contest_resolver.go index 79f29e4943..f3df57b10d 100644 --- a/contractcourt/htlc_incoming_contest_resolver.go +++ b/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[:], ) diff --git a/contractcourt/htlc_incoming_resolver_test.go b/contractcourt/htlc_incoming_resolver_test.go index 620acdc201..e8532abe58 100644 --- a/contractcourt/htlc_incoming_resolver_test.go +++ b/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, diff --git a/contractcourt/htlc_outgoing_contest_resolver_test.go b/contractcourt/htlc_outgoing_contest_resolver_test.go index 751f8b9718..f83d177497 100644 --- a/contractcourt/htlc_outgoing_contest_resolver_test.go +++ b/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(), }, }, } diff --git a/contractcourt/htlc_success_resolver_test.go b/contractcourt/htlc_success_resolver_test.go index 1303291d0e..d2c9bf0516 100644 --- a/contractcourt/htlc_success_resolver_test.go +++ b/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, } diff --git a/lnmock/routing.go b/lnmock/routing.go new file mode 100644 index 0000000000..260cce6ded --- /dev/null +++ b/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 +} diff --git a/lnwallet/channel.go b/lnwallet/channel.go index d715d5ed66..ad08557a7a 100644 --- a/lnwallet/channel.go +++ b/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, From 7b1a2883202dd13445a65b692090839b7e97c2cd Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Fri, 19 May 2023 11:58:30 -0400 Subject: [PATCH 2/3] 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 | 83 +++++++++++++++++++++++++++++-- channeldb/channel_test.go | 102 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 4 deletions(-) diff --git a/channeldb/channel.go b/channeldb/channel.go index f986321964..59eb357c0c 100644 --- a/channeldb/channel.go +++ b/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 diff --git a/channeldb/channel_test.go b/channeldb/channel_test.go index 7896bcaf44..bfebad824b 100644 --- a/channeldb/channel_test.go +++ b/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) +} From 8a9dd105fcc9dfa5483af697e52f60c64fe36446 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 24 May 2023 15:01:13 -0400 Subject: [PATCH 3/3] docs: add release notes --- docs/release-notes/release-notes-0.17.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.17.0.md b/docs/release-notes/release-notes-0.17.0.md index 5d2a3aef84..f1d58214fd 100644 --- a/docs/release-notes/release-notes-0.17.0.md +++ b/docs/release-notes/release-notes-0.17.0.md @@ -26,6 +26,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