tlv: add new RecordT[T] utility type

[Roasbeef]

In this commit, we add a new type, Record[T] to reduce some of the common boiler plate for TLV types. This time lets you take either a primitive type, or an existing Record, and gain common methods used to create tlv streams.

It also serves as extra type annotation as well, since wire structs can use this to wrap any existing type and gain the relevant record methods.

[ProofOfKeags]

I'd like to suggest a different approach:

We have a new interface I will refer to as LPSerializable and then we constrain the T in the Record def like so Record[T LPSerializable].

The interface for LPSerializable would be as you'd expect: a pair of functions T -> []byte and []byte -> T, error, and possibly a size function T -> uint16.

Thoughts?

[Roasbeef]

We have a new interface I will refer to as LPSerializable and then we constrain the T in the Record def like so Record[T LPSerializable].

What's LP mean in this context?

What would this look like? We have Record already which is the base type used specifically for the TLV encoding, sounds like you want something even higher level that operates on entire structs? Record is for a single field, or a field which is itself a series of nested records.

My goal here was just carve out just something enough to fill a gap we perceived when trying to make new structs that use the pure TLV encoding. This PR lets a user take a primitive type, or something else that already implements the Record interface used in lnwire and minimize some of the boiler plate needed. So a small step towards improving the ergonomics of the tlv package and the existing patterns we use throughout the codebase today.

[ProofOfKeags]

What's LP mean in this context?

Length-Prefixed.

I am not in any way attached to the name.

To carry out such an overhaul, we'd need to do away with the current encode/decode functions eg (tlv.EUint8), and instead make them into methods (abstracted via a new interface), with those changes reverberating through the entire project.

My commentary was on the fact that the existing RecordProducer interface takes responsibility for assigning a type number and I'd love for it not to. The structure you define here does a great job of taking responsibility for the type number annotation. Given that, it'd be cool if we could remove the type number assignment from the RecordProducer. However, as you point out, this would reverberate throughout the project. Instead, we could define a new typeclass LPSerializable which is only responsible for serialization/sizing, and not type number assignment.

[ellemouton]

Nice!

[lightninglabs-deploy]

@Roasbeef, remember to re-request review from reviewers when ready

[Roasbeef]

Ok, take 3 of this PR:

• Now based on Record[T, V]. V is the actual type, and T is a new struct that maps 1:1 between TLV types.
• This let's you declare things like Record[TlvType10, uint16]-- which'll map to something with TLV type 10, and value 16.
• It ends up using go generate to achieve this. We won't need to run it again, as due to implementation limits, we can only have 100 members in a union struct. If we need more, we can break it up (I think?).
• There's one small quirk that if you want to use something that already has the RecordProducer implementation, then it needs to be a pointer, so RecordT[TlvType2, *wireCsv], not RecordT[TlvType2, wireCsv]. This is the classic "method has a pointer receiver error". Not sure if there's a good way around this, but maybe we just need it less since the type TLV is now enforced at the compile level.

[ellemouton]

My main question is around why we cant just let any uint be the TLV type? ie, do we need the code gen?

added a templating solution suggestion to the code gen

[ProofOfKeags]

Excited about this. Looks like you kinda hacked Phantom Types into go by exploiting the implicit zero constructor 👍🏼

[Roasbeef]

My main question is around why we cant just let any uint be the TLV type? ie, do we need the code gen?

@ellemouton

Go doesn't allow normal primitive types as type param parameter. In C++, you could do something like RecordT[5, wireCsv]and it would work properly. We use the code generation to enable RecordT[TlvType5, wireCsv], which is possible as we have GetTypeVal that maps TlvType5 -> 5. This is passed into MakePrimitiveRecord, which makes the entire thing work.

An example in C++ would be like:

template<unsigned int SIZE = 3>
struct Vector {
    unsigned char buffer[SIZE];
};

Vector<3> test;

[ProofOfKeags]

Go doesn't allow normal primitive types as type param parameter.

This is more than just a language limitation. It's a limitation of the type theory that underlies most languages. To do something like that you need limited support for dependent types and most things won't have that and even if they do you'll run into numerous edge cases where things don't behave as expected unless the language supports full-blown DT's. So, even if you could do it that way, I'd caution against it.

[Roasbeef]

Went to remove the limit for the first 100 TLV types, but ran into something where it'll only generate up to 32767. Will just do 1000 and call it a day for now.

[Roasbeef]

Newest version up, PTAL. There's no true limit on the first 100 types now (see above comment), and I fixed an issue that made it harder to user as a caller (needed to fully populate all the attrs in a new struct, instead of being able to rely on the zero values).

Code generation now is also just for the generated types, no need for those helper functions or the type constraint union.

[ProofOfKeags]

Do you want to be committing the binary?

[Roasbeef]

Nope, removed

[ProofOfKeags]

Oh tight, I wasn't sure if we could do partial inference.

[Roasbeef]

Yeah pretty nice for cases like this.

[ProofOfKeags]

Send it 🚀

[Roasbeef]

Will land this, then push a new tag for tlv. Leaving off the release notes as this isn't a user facing feature.