BOLT01: swap CompactSize for BigSize in TLV format#640
BOLT01: swap CompactSize for BigSize in TLV format#640niftynei merged 2 commits intolightning:masterfrom
Conversation
5eaa876 to
41a50d9
Compare
t-bast
left a comment
There was a problem hiding this comment.
I honestly don't have a strong opinion on the endianness we should use.
Keeping Bitcoin's little-endian encoding or switching to big-endian as you propose both seem ok.
I'll let @rustyrussell chime in to decide.
.aspell.en.pws
Outdated
| tlv | ||
| ExpErr | ||
| Fatalf | ||
| fc |
There was a problem hiding this comment.
It's a bit weird to have to add all of those in the spell-checker...
I think that if we always wrap them in back-ticks (e.g. 0xfc) the spell-checker doesn't look at them, which might be a better solution.
There was a problem hiding this comment.
good point, i gave a shot at modifying the spell checker to ignore code blocks, but not sure that's the best approach
There was a problem hiding this comment.
i'm +1 for ignoring code blocks fwiw
096953b to
770e92c
Compare
This commit modifies the varint encoding used for TLV types and lengths to use a custom format called BigSize. The format is identical to bitcoin's CompactSize, except it replaces the use of little-endian encodings for multi-byte values with big-endian. This is done to prevent mixing endianness within the protocol, since otherwise CompactSize would be the first introduction of little-endian encodings.
770e92c to
8038471
Compare
|
Ack. We ended up not reusing the Bitcoin routines and reimplementing them for this, so using Bitcoin endianness here was a lose. Wasn't sure if other implementations were the same. JSON for testcases is a bit useless without a programmatic extractor IMHO, but I'm just happy to have test vectors at all. We can have a Grand Testcase Unification later... |
|
PS. I've lost count of how many beers I owe @cfromknecht now, but I suspect it may be more than we can drink in Berlin... |
|
ACK. Let's decide on this once and for all, big-endian everywhere it is! |
|
Tested-Ack: 8038471 |
lightning/bolts#640 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Based on: lightning/bolts#640 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Based on: lightning/bolts#640 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit modifies the varint encoding used for TLV types and lengths
to use a custom format called BigSize. The format is identical to
bitcoin's CompactSize, except it replaces the use of little-endian
encodings for multi-byte values with big-endian. This is done to prevent
mixing endianness within the protocol, since otherwise CompactSize would
be the first introduction of little-endian encodings.