Channeldb: Store HTLC Extra TLVs in Onion Blob Varbytes #7710

carlaKC · 2023-05-19T17:49:09Z

Opening this in draft to illustrate the scope of a possible solution for #7297.

This PR demonstrates a workaround for persisting TLV data that is transmitted in update_add_hltc:

HTLCs are serialized inline (so we can't easily add on another field, because we wouldn't know when it is / isn't there easily).
The OnionBlob for HTLCs is serialized as var bytes, but we know this will always be 1366 bytes (as the spec demands).
We can use this variable encoding to squeeze in our TLVs as follows:
- Write OnionBlob + TLVStream as a single var bytes blob where we previously had just OnionBlob
- This will write a lengh prefix of 1366 + len(TLV stream)
- To deserialize, read the full var bytes blob, copy the first 1366 into the OnionBlob and treat the rest as our TLV Stream

When we want to save extra data (as provided in our PaymentDescriptor, all we have to do is encode the TLVs and set channeldb.HTLC.ExtraData (see last commit for example of where we'd need to set this).

Roasbeef

Happy to see you came around to this approach! May not be the ideal path, but it gets the job done and also lets us avoid migrations in this area as well. Before proceeding we should still do a manual comb through and ensure there're no other areas where an exact length is used (not serialized as var bytes).

channeldb/channel.go

carlaKC · 2023-05-24T18:33:31Z

Happy to see you came around to this approach! May not be the ideal path, but it gets the job done and also lets us avoid migrations in this area as well.

Took me a minute to grok, ngl, but seems like the lesser evil (by far)!

Before proceeding we should still do a manual comb through and ensure there're no other areas where an exact length is used (not serialized as var bytes).

Will post separately! WDYT about this PR going in as-is vs combining in a larger preparatory route blinding PR? I lean towards to former, keeping DB workaround PRs more isolated but happy with either approach.

carlaKC · 2023-05-24T18:34:02Z

References to `HTLC.OnionBlob`

channeldb/channel.go

Serialized/Deserialized with updated functions - always stored as varbytes.

File	Line	Reference
channeldb/channel.go	2004	onionHash := sha256.Sum256(htlc.OnionBlob[:])
channeldb/channel.go	2012	onionHash := sha256.Sum256(htlc.OnionBlob[:])
channeldb/channel.go	2104	copy(onionAndExtraData, htlc.OnionBlob[:])
channeldb/channel.go	2162	htlcs[i].OnionBlob[:]

contractcourt

decodePayload reads the HTLC's fixed size blob - these HTLCs are serialized and deserialized with our updated logic as part of commitSet.

Legacy nodes used LogChainActions/FetchChainActions which also serializes and deserializes using var bytes.

File	Line	Reference
contractcourt/htlc_incoming_contest_resolver.go	490	onionReader := bytes.NewReader(h.htlc.OnionBlob[:])

lnwallet/channel.go

HTLCs are stored as var bytes in ChannelCommitment and re-loaded into memory.

File	Line	Reference
lnwallet/channel.go	748	copy(h.OnionBlob[:], htlc.OnionBlob)
lnwallet/channel.go	772	copy(h.OnionBlob[:], htlc.OnionBlob)
lnwallet/channel.go	860	OnionBlob: htlc.OnionBlob[:]

yyforyongyu

Love how small and focused this PR is🙏 Because the onion packet is enforced with the 1366 bytes restriction at the encoding/decoding level, I think it's safe to go with this approach. Nice workaround! Left a few comments and I think this is almost good to go⛵️

channeldb/graph_test.go

channeldb/channel.go

channeldb/channel_test.go

channeldb/channel.go

yyforyongyu

LGTM🏖

channeldb/channel.go

channeldb/channel_test.go

channeldb/channel.go

ziggie1984

Smart workaround, chapeau 🎓 :)

channeldb/channel_test.go

ziggie1984 · 2023-06-01T10:17:15Z

channeldb/channel_test.go

@@ -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))


Q: Wondering where this testcase came from, I think onion-blobs were always 1366 in size also before this change ?

Well previously a OnionBlob was just a byte slice, so they could be less than 1366. They never would be, as we wouldn't be able to read them, but unit tests could get away with junk data.

TL;DR: past selves be short-cuttin :')

channeldb/channel.go

channeldb/channel_test.go

channeldb/channel.go

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).

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

carlaKC · 2023-06-02T15:03:21Z

Latest push just addresses nits and adds some comments, no functionality changes.

channeldb/channel_test.go

lightninglabs-deploy · 2023-06-12T04:08:06Z

@Roasbeef: review reminder

Torakushi

Awesome changes! very clear and concise. Well done!

Roasbeef

LGTM 🌴

carlaKC mentioned this pull request May 19, 2023

[feature]: Availability of UpdateAddHTLC ExtraData Across Restarts #7297

Closed

Roasbeef reviewed May 23, 2023

View reviewed changes

channeldb/channel.go Show resolved Hide resolved

channeldb/channel.go Outdated Show resolved Hide resolved

Roasbeef added onion routing HTLC tlv blinded paths labels May 23, 2023

carlaKC force-pushed the 7297-updateaddtlvs branch from 5c9a182 to a629a22 Compare May 24, 2023 17:35

carlaKC force-pushed the 7297-updateaddtlvs branch from a629a22 to 2850989 Compare May 24, 2023 18:58

carlaKC marked this pull request as ready for review May 25, 2023 16:57

yyforyongyu reviewed May 25, 2023

View reviewed changes

channeldb/graph_test.go Outdated Show resolved Hide resolved

channeldb/channel.go Show resolved Hide resolved

channeldb/channel_test.go Show resolved Hide resolved

channeldb/channel.go Show resolved Hide resolved

channeldb/channel.go Show resolved Hide resolved

carlaKC mentioned this pull request May 25, 2023

multi: Forward Blinded Payments #7376

Closed

carlaKC force-pushed the 7297-updateaddtlvs branch 2 times, most recently from 724d177 to 08ede0f Compare May 26, 2023 18:51

yyforyongyu added this to the v0.17.0 milestone May 30, 2023

yyforyongyu approved these changes May 30, 2023

View reviewed changes

channeldb/channel.go Show resolved Hide resolved

channeldb/channel_test.go Show resolved Hide resolved

channeldb/channel.go Show resolved Hide resolved

carlaKC force-pushed the 7297-updateaddtlvs branch from 08ede0f to d1b5c0a Compare May 31, 2023 14:35

carlaKC requested a review from Roasbeef May 31, 2023 14:37

ziggie1984 approved these changes Jun 1, 2023

View reviewed changes

carlaKC added 3 commits June 2, 2023 11:01

channeldb: store extra HTLC data in variable onion blob slot

7b1a288

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

docs: add release notes

8a9dd10

carlaKC force-pushed the 7297-updateaddtlvs branch from d1b5c0a to 8a9dd10 Compare June 2, 2023 15:02

yyforyongyu approved these changes Jun 5, 2023

View reviewed changes

channeldb/channel_test.go Show resolved Hide resolved

saubyk assigned carlaKC Jun 14, 2023

saubyk requested a review from ellemouton June 15, 2023 17:10

Torakushi approved these changes Jun 16, 2023

View reviewed changes

Roasbeef approved these changes Jun 16, 2023

View reviewed changes

Roasbeef merged commit 7f95810 into lightningnetwork:master Jun 16, 2023

calvinrzachman mentioned this pull request Mar 15, 2024

route-blinding: recover blinding point from forwarding packages carlaKC/lnd#37

Closed

carlaKC mentioned this pull request May 20, 2024

Enhance update_add_htlc with remote peer's custom records #8660

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Channeldb: Store HTLC Extra TLVs in Onion Blob Varbytes #7710

Channeldb: Store HTLC Extra TLVs in Onion Blob Varbytes #7710

carlaKC commented May 19, 2023

Roasbeef left a comment

carlaKC commented May 24, 2023

carlaKC commented May 24, 2023

yyforyongyu left a comment

yyforyongyu left a comment

ziggie1984 left a comment

ziggie1984 Jun 1, 2023

carlaKC Jun 2, 2023

carlaKC commented Jun 2, 2023

lightninglabs-deploy commented Jun 12, 2023

Torakushi left a comment

Roasbeef left a comment

Channeldb: Store HTLC Extra TLVs in Onion Blob Varbytes #7710

Channeldb: Store HTLC Extra TLVs in Onion Blob Varbytes #7710

Conversation

carlaKC commented May 19, 2023

Roasbeef left a comment

Choose a reason for hiding this comment

carlaKC commented May 24, 2023

carlaKC commented May 24, 2023

References to HTLC.OnionBlob

channeldb/channel.go

contractcourt

lnwallet/channel.go

yyforyongyu left a comment

Choose a reason for hiding this comment

yyforyongyu left a comment

Choose a reason for hiding this comment

ziggie1984 left a comment

Choose a reason for hiding this comment

ziggie1984 Jun 1, 2023

Choose a reason for hiding this comment

carlaKC Jun 2, 2023

Choose a reason for hiding this comment

carlaKC commented Jun 2, 2023

lightninglabs-deploy commented Jun 12, 2023

Torakushi left a comment

Choose a reason for hiding this comment

Roasbeef left a comment

Choose a reason for hiding this comment

References to `HTLC.OnionBlob`