Truncated integer fields, e.g. tu16 cannot have leading zeros
judging by the implementations of the parsers, which are used
across the different ln implementations.
The `can` imposed that it is not necessary to truncate the fields
to values of information.
Signed-off-by: Peter Neuroth <pet.v.ne@gmail.com>
We generally shouldn't disconnect when sending or receiving warning
messages. Whenever disconnecting after a warning makes sense, it should
be specified in the requirements linked to that specific scenario.
Fixes#1072
It had a blank line and invalid format for tools/extract-formats.py.
And move the format information into the requirements section
(and fix spelling: `node_announement` -> `node_announcement`
Diff for extract-formats.py before and after:
```diff
--- /tmp/before 2022-05-17 10:47:01.583086352 +0930
+++ /tmp/after 2022-05-17 10:51:59.166850111 +0930
@@ -6,6 +6,8 @@
msgdata,init,tlvs,init_tlvs,
tlvtype,init_tlvs,networks,1
tlvdata,init_tlvs,networks,chains,chain_hash,...
+tlvtype,init_tlvs,remote_addr,3
+tlvdata,init_tlvs,remote_addr,data,byte,...
msgtype,error,17
msgdata,error,channel_id,channel_id,
msgdata,error,len,u16,
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This adds the option to report an remote IP address back to a connecting
peer using the `init` message. A node can decide to use that information
to discover a potential update to its public IPv4 address (NAT) and use
that for a `node_announcement` update message containing the new address.
The proposal includes reporting the IPv4 and IPv6 address,
however in IPv6 there are likely no NAT issues. TOR is skipped for
obvious reasons.
Certain approaches to check and use this information are thinkable:
- Wait for multiple peers or a certain fraction to report the
same new address.
- Check some random node known via gossip to also report the new
address.
- Verify this information by making a test connection to itself.
There were valid uses for "I don't want to talk to you anymore" apparently!
Also fixed some tabs -> spaces.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And make most places warn or error. Places where we're operating
on a channel tend to be "warn and close connection" since we want to
forget the mistake they just sent, and closing the connection does that.
We now use the same words everywhere:
1. "fail channel" means to go onchain (if necessary).
2. "send `error`" means to send an error message.
3. "send `warning`" means to send a warning message.
4. "close connection" means close the connection.
These are all spelled out explicitly, rather than having "fail channel"
imply sending an error packet, for example.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Under this spec, an error with an all-zero channel id is ignored.
Warnings, being odd, will be ignored by older nodes too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The test vectors for invalid `init` messages were invalid since we added
the `networks` tlv extension.
They are now fixed and made more future-proof by using tlvs with high
values.
There's not a lot of reason to explicitly rate-limit ping messages
on the sending side, hosts on the internet can *always* send you
as much traffic as they want, its up to you whether you want to
talk back to them.
This seems to have been intended as a cutoff where nodes can skip
responding to pings below a certain rate, but in practice 30
seconds is much too long a time to learn that your peer has
disconnected.
We could reduce the threshold, but its not like this is the only
place in the spec where a peer can request a message response, and
that is unlikely to change, making it of highly dubious value.
* Rename all the 'varint' to 'bigsize'.
Having both is confusing; we chose the name bigsize, so use it
explicitly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 7: use `byte` instead of `u8`.
`u8` isn't a type; see BOLT #1 "Fundamental Types".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 1: promote bigsize to a Fundamental Type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The spec already prepared a hook to add additional information to existing
messages (additional bytes at the end of a message must be ignored).
Since we're using TLV in many places, it makes sense to use that optional
additional space at the end of each message to allow an optional tlv stream.
This requires making a few previously optional fields mandatory:
- channel_reestablish commitment points: it makes sense to always include those
regardless of whether `option_dataloss_protect` or `option_static_remotekey` are set.
- option_upfront_shutdown_script: if you're not using one, just set the length to 0.
That field is moved to a TLV record because luckily, the resulting bytes are the same.
This provides more flexibility to later remove the requirement of making this field mandatory.
No need to change the `channel_update`'s `htlc_maximum_msat` because
the `message_flags` encode its presence/absence.
It can still be either included or omitted without causing issues to the extension stream.
The spec already prepared a hook to add additional information to existing
messages (additional bytes at the end of a message must be ignored).
Since we're using TLV in many places, it makes sense to use that optional
additional space at the end of each message to allow an optional tlv stream.
This requires making a few previously optional fields mandatory:
- channel_reestablish commitment points: it makes sense to always include those
regardless of whether `option_dataloss_protect` or `option_static_remotekey` are set.
- option_upfront_shutdown_script: if you're not using one, just set the length to 0.
That field is moved to a TLV record because luckily, the resulting bytes are the same.
This provides more flexibility to later remove the requirement of making this field mandatory.
No need to change the `channel_update`'s `htlc_maximum_msat` because
the `message_flags` encode its presence/absence.
It can still be either included or omitted without causing issues to the extension stream.
This commit:
- Adds a new Dependencies column to the BOLT 9 feature table
populated with existing feature dependencies.
- Requires that all valid feature vectors set transitive dependencies.
- Requires checking transitive dependencies when validating init
messages and payment request.
- Removes transitive feature requiremetns from the BOLT 11 writer, now
that they are implicit by needing to comply with the BOLT 9 origin
requirements.
We simply specify, in each case, where they will appear ("Context").
Because `globalfeatures` is already in use, we fold that into the
renamed `localfeatures` field to unify them (now called `features`),
but dissuade further use.
Note also: we REQUIRE minimal `features` field in
channel_announcement, since otherwise both sides of channel will not
agree and not be able to create their signatures!
Consider these theoretical future features:
`opt_dlog_chan`: a new channel type which uses a new discrete log HTLC
type, but can't support traditional HTLC:
* `init`: presents as odd (optional) or even (if traditional channels
not supported)
* `node_announcement`: the same as above, so you can seek suitable peers.
* `channel_announcement`: presents as even (compulsory), since users need
to use the new HTLCs.
`opt_wumbochan`: a node which allows channels > 2^24 satoshis:
* `init`: presents as odd (optional), or maybe even (if you only want
giant channels)
* `node_announcement`: the same as above, so you can seek suitable peers.
* `channel_announcement`: not present, since size of channel indicates
capacity.
`opt_wumbohtlc`: a channel which allows HTLCs > 2^32 millisatoshis:
* `init`: presents as odd (optional), or even (compulsory)
* `node_announcement`: the same as above, so you can seek suitable peers.
* `channel_announcement`: odd (optional) since you can use the channel
without understanding what this option means.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Co-Authored-By: Bastien Teinturier <31281497+t-bast@users.noreply.github.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>
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.
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>
Obviously you can't include fields you don't understand, but importantly
if the other side doesn't agree, you don't need to include them.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I named the first option `option209` but that was too ugly to live.
Nonetheless, this wording lived on.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
