The clarifications were tacked on after the fact, but they should really be
part of the conventions. I also updated the links to use the reference style,
which results in better text flow and makes it easier to read the source.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
These are based on @t-bast's vectors from #607, with a few more
cases:
1. Explicitly test encodings for 253, 254 and 255.
2. Use BigSize and make sure tests break badly if endian parsing is wrong.'
3. Test wrap-around of type encodings in stream.
Many thanks to @t-bast and @cfromknecht for their contributions and testing
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We didn't explicitly say that the TLV is bad if length exceeds
the message length!
We didn't specify whether to ignore extra bytes: we should.
Similarly, contents of values must be minimal (i.e. tu64 etc).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were swallowing the unused line after `data`, but it's
normal to do:
```
1. tlvs: `n1`
2. types:
1. type: 1 (`tlv1`)
2. data:
* [`tu64`:`amount_msat`]
1. type: 2 (`tlv2`)
2. data:
* [`short_channel_id`:`scid`]
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
(No spec change, just wording)
The "local" and "remote" here are just *confusing*. Each side says
where it's at, and the other side retransmits based on that.
We could call it 'number_of_next_commitment_i_expect_to_receive' and
'number_of_next_revocation_i_expect_to_receive' but that's getting
silly.
These names were a major source of confusion while writing tests!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Mention that `outgoing_cltv_value` has to be equal to
`min_final_cltv_expiry` and `amt_to_forward` has to be equal to
`amount` if the [BOLT #11](11-payment-encoding.md) invoice is used
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.
OP_CHECKLOCKTIMEVERIFY and OP_CSV use an inconsistent naming convention.
Update OP_CSV to match the OP_CHECKLOCKTIMEVERIFY convention as OP_CHECKSEQUENCEVERIFY.
It's trivial to make types->lengths, but not so much the other way.
The types I used here are the ones I found useful in implementation, and
I think add some clarity, though we can certainly argue about them.
There's no normative changes to the spec in here.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Editing the previous mess was horrific. I gave up and rewrote using a
generator.
Changes to output:
1. subtypes and tlvs now handled.
2. The output format now has explicit prefixes, so readers don't have
to rely on number of fields to interpret data.
3. Each field is split into type and count; count is empty if there's
no '*x'.
4. TLV stream typenames are repeated; TLV record type names are not
necessarily unique.
5. The unused offset field is removed.
6. No arguments taken: everything is always printed, and you can grep if you
only want some.
[ Fixup by <niftynei@gmail.com> ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When the notation of the noise protocol framework is being introduced the terms `ck` and `k` are being explained but `e` and `s` are only referred to was public keys. I fixed that by stating what they stand for and noting that `s` is usually the the `nodeid` in the context of the Lightning Network protocol.
Saying "the risk is only to the node *accepting* the HTLC" is confusing
because merely accepting an HTLC is risk-free. The risk comes from
accepting *responsibility to route the payment*, i.e. offering an HTLC
of your own in the next channel on the path, where a too-small
difference in the HTLC values could end up with you cheated out of a
payment.
This revised paragraph hopefully makes that clearer.
Uses more specific and consistent language ("the"/"that" instead of "an"
where possible). Also helps avoid confusion with unrelated HTLCs that
serve other payments but are shared by the same two nodes.
914ebab908 effectively deprecated this, but
left it for "reject if more than 2x expected amount" case.
Leaving it for gross overpayment still leaves an attack on the current
network in practice (all implementations I know of reject grossly excessive
payments), and removing it causes our code to nicely break when regenerating,
since that error code is now not defined anywhere.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We express it has how the outputs are ordered, but the only way you can
detect that is by the htlc_signatures order, which is the part which really
matters.
I finally reproduced this, BTW, which is why I'm digging it up!
Closes: #448
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
