Channeldb: Store HTLC Extra TLVs in Onion Blob Varbytes

[carlaKC]

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

[carlaKC]

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]

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⛵️

[yyforyongyu]

LGTM🏖

[ziggie1984]

Smart workaround, chapeau 🎓 :)

[ziggie1984]

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

[carlaKC]

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 :')

[carlaKC]

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

[lightninglabs-deploy]

@Roasbeef: review reminder

[Torakushi]

Awesome changes! very clear and concise. Well done!

[Roasbeef]

LGTM 🌴