Its not uncommon to be multi-homed with different addresses, so we should probably allow nodes to do this. Also, it seems like this is pretty much universally not actually enforced on the network.
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.
ECDSA signatures in Bitcoin are DER-encoded but public keys are not.
The compressed format for public keys is for example standardized in
Sections 2.3.3 and 2.3.4 of
Standards for Efficient Cryptography, SEC 1: Elliptic Curve
Cryptography, Certicom Research, Version 2, 2009,
https://www.secg.org/sec1-v2.pdf
As more nodes on the network use timestamp_filter to block gossip
floods, we've seen some propagation problems. This should avoid it
(and is implemented now by c-lightning).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In this commit, we modify the existing instructions to create the Sphinx
packet to no longer start out with a zero initialize set of 1366 bytes.
Instead, we now instruct the sender to use _random_ bytes derived from a
CSPRG. This fixes a recently discovered privacy leak that allows an
adversarial exit hop to ascertain a lower bound on the true path length.
Note that this doesn't affect packet processing, so this is a backwards
compatible change. Only clients need to update in order to avoid this
privacy leak.
After this change is applied, the test vectors as is don't match the
spec, as they're created using the original all zero starting bytes. We
can either update these with our specified set of random bytes, or leave
them as is, as they're fully deterministic as is.
An alternative path would be to generate more random bytes from the
shared secret as we do elsewhere (the chacha based CSPRNG).
As a final step, we now can remove several of the BOLT 11 writer's
requirements now that it builds on BOLT 9's, particularly:
- setting the even bit if a feature is required.
- only setting a feature if the node supports a given feature.
The lone requirement that remains pertains to setting the `s` value if
and only if the `payment_secret` feature is set.
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.
As reading of commit 6729755f shows, `final_expiry_too_soon` was
17, not PERM|17.
Note that because we folded a previously non-permanent failure into
the now-permanent PERM|15 failure code, modifications to payment
algorithms may now be needed to specificalyl detect this case,
otherwise payment algorithms may give up in some edge cases where
blocks are mined while payments are in-transit between sender and
receiver.
This appears to have been an oversight in the flat features spec,
and is somewhat implicitly relied on for several new feature bits -
if var_onion_optin is set on a node_announcement (its not allowed
on a channel_announcement), then trying to route through that node
using the pre-tlv formt is somewhat nonsensical, and should be
forbidden.
BOLT 4 explicitly indicates var_onion_optin may appear in a BOLT 11
invoice, however, BOLT 9 only indicates it is available in init and
node_announcement contextx. Resolve this conflict in favor of BOLT 4
as there doesn't seem to be much reason to *not* allow it in BOLT
11 invoices.
This means the BOLT11 invoice must offer it (we already say it must
set the field if it offers it), and that the receiving node must
require it (again, we already say it must check it if it requires it).
Without the payment_secret, MPP payments are especially vulnerable to
probing attacks: unlike normal payments (with amounts) they can be
detected with 1msat payment probes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I recently made a cut & paste bug with the protocol tests, and
paid an HTLC of amount 100M msat, but with only a 1M msat `amt_to_forward`
in the hop_data. To my surprise, it was accepted.
This is because we allow overpaying the routing fee (considered 0
for the final hop). This doesn't make sense for the final hop: anything
but exact equality implies a bug, or that the previous node took the
wrong amount from the payment.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The feature fields refer to the properties of the channel/node, not the
message itself, so we can still propagate them (and should, to avoid
splitting the network).
If we want to make an incompatible announcement message, we'll use a
different type, or insert an even TLV type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
When the `p` multiplier is used, the amount MUST be divisible
by 10 since the resolution used internally is millisatoshi.
This addresses but does not close#692.
We might argue this does not apply if you set `minimum_depth` to 0, since
you're assuming trust (TurboChannels-style), but it needs to be specified.
See: CVE-2019-12998 / CVE-2019-12999 / CVE-2019-13000
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 7: fix cut & paste typo.
This is `reply_channel_range_tlvs`: `query_channel_range_tlvs` is defined
above. Somehow this fix got lost in the merge process, and breaks
our spec parsing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 7: add missing `encoding_type` field in query_short_channel_ids_tlvs / reply_channel_range_tlvs
The implementations have it, and the requirements refer to it,
but it's not actually in the description!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* BOLT 7: clarify specification.
As agreed in http://www.erisian.com.au/meetbot/lightning-dev/2019/lightning-dev.2019-08-05-20.03.html,
checksums are not encoded as encoding_type + byte, but as a straight
array. Referring to them as `*byte` is thus underspecifying them:
they are literally `*channel_update_checksums`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This separates out the static remotekey changes from the more ambitious
option_simplified_commitment (which also included pushme outputs and
bring-your-own-fee for HTLC outputs).
As per http://www.erisian.com.au/meetbot/lightning-dev/2019/lightning-dev.2019-09-02-20.06.html
Thanks to everyone for feedback: @araspitzu @roasbeef @bitconner
Suggested-by: @roasbeef
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
