Lightning: run-tlvstream failure

[ZmnSCPxj]

When doing a make check on a fresh clone bf3b77a9477054cca6c06c16eff3413fc6bc3c5d on my system I get a failure:

wire/test/run-tlvstream > /dev/null
run-tlvstream: ./wire/tlvstream.c:152: towire_tlvs: Assertion `types[i].type > types[i-1].type' failed.
Aborted
Makefile:456: recipe for target 'unittest/wire/test/run-tlvstream' failed
make: *** [unittest/wire/test/run-tlvstream] Error 134

gdb backtrace:

(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff6fc542a in __GI_abort () at abort.c:89
#2  0x00007ffff6fbce67 in __assert_fail_base (fmt=<optimized out>, assertion=assertion@entry=0x5555555a6358 "types[i].type > types[i-1].type",
    file=file@entry=0x5555555a5f57 "./wire/tlvstream.c", line=line@entry=152,
    function=function@entry=0x5555555a71f8 <__PRETTY_FUNCTION__.6518> "towire_tlvs") at assert.c:92
#3  0x00007ffff6fbcf12 in __GI___assert_fail (assertion=0x5555555a6358 "types[i].type > types[i-1].type", file=0x5555555a5f57 "./wire/tlvstream.c",
    line=152, function=0x5555555a71f8 <__PRETTY_FUNCTION__.6518> "towire_tlvs") at assert.c:101
#4  0x0000555555571a9a in towire_tlvs (pptr=0x7fffffffdaf0, types=0x5555557c8c40 <tlvs_n1>, num_types=4, record=0x5555557d1ad8)
    at ./wire/tlvstream.c:152
#5  0x0000555555573e0f in main () at wire/test/run-tlvstream.c:533

The code being complained is: https://github.com/ElementsProject/lightning/blob/bf3b77a9477054cca6c06c16eff3413fc6bc3c5d/wire/tlvstream.c#L152 which basically just checks that TLVs are sorted by type.

This suggests a bug in the caller, so: https://github.com/ElementsProject/lightning/blob/bf3b77a9477054cca6c06c16eff3413fc6bc3c5d/wire/test/run-tlvstream.c#L533

tlvs_n1 is declared in gen_peer_wire.c, a generated file. On my system it comes up as:

const struct tlv_record_type tlvs_n1[] = {
        { 254, towire_tlv_n1_tlv4, fromwire_tlv_n1_tlv4 },
        { 1, towire_tlv_n1_tlv1, fromwire_tlv_n1_tlv1 },
        { 2, towire_tlv_n1_tlv2, fromwire_tlv_n1_tlv2 },
        { 3, towire_tlv_n1_tlv3, fromwire_tlv_n1_tlv3 },
};

which looks broken to me, since 254 should not be first. In particular, below is where it is generated from: https://github.com/ElementsProject/lightning/blob/bf3b77a9477054cca6c06c16eff3413fc6bc3c5d/wire/extracted_peer_wire_csv#L17-L26

The generation of gen_peer_wire.c is here:

https://github.com/ElementsProject/lightning/blob/bf3b77a9477054cca6c06c16eff3413fc6bc3c5d/wire/Makefile#L76-L77

And BOLT_GEN is defined here: https://github.com/ElementsProject/lightning/blob/bf3b77a9477054cca6c06c16eff3413fc6bc3c5d/Makefile#L185

which is Python code.

So I tried to read the Python code, and anyway, this seems to be the part in the template: https://github.com/ElementsProject/lightning/blob/bf3b77a9477054cca6c06c16eff3413fc6bc3c5d/tools/gen/impl_template#L205-L209

So tlv.messages.values() is returning an unsorted list or array or whatever Python uses for sequenced data. I think the .messages field is this part of class Tlv:

https://github.com/ElementsProject/lightning/blob/bf3b77a9477054cca6c06c16eff3413fc6bc3c5d/tools/generate-wire.py#L359-L362

i.e. it is the built-in Object type (if I understand Python correctly).

So question: does Python actually commit to having values() sorted? In case it is relevant, python3 --version is Python 3.5.3

[niftynei]

Python 'dictionaries' (of which {} is) sorting isn't guaranteed before Python 3.7. This can most likely be fixed by using OrderedDict() instead of {} in the line 362 in generate-wire.py.

[ZmnSCPxj]

Of note is that it seems the expectation is that the dictionary is sorting by order of inclusion, not order of keys. The key seems to be the name of the TLV type, not the type numeric code (I think...?), though as a non-Pythonista it is possible my understanding of the generation code is incorrect.

[ScottTre]

You can try this patch to fix the sorting.

sort.txt

[niftynei]

Of note is that it seems the expectation is that the dictionary is sorting by _order of inclusion_, not order of keys. The key seems to be the name of the TLV type, not the type numeric code (I think...?), though as a non-Pythonista it is possible my understanding of the generation code is incorrect.

Correct. Preserving order of inclusion should be fine for now, since the output we're parsing (in theory) orders them correctly. It's probably better to explicitly sort them by type number though, as that makes us more robust against ordering mistakes in the input files.