One for uppercase, and one with should-be-ignored fields.
The first of these addresses #659 (#677 directly changes the text
to make it clear this is allowed, and should also be applied).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The introductory paragraph describes node discovery and channel
discovery, but changes the ordering. Keep the same ordering throughout
the paragraph for readability.
Add an additional "reserve" for funders on top of the real reserve to
avoid getting in a state where the channel is unusable because
of the increased commit tx cost of a new HTLC.
Requirements are only added for the funder sending an HTLC.
Fundee receiving HTLCs may choose to verify that funders apply
this, but it may lead to an unusable UX.
Fixes#728.
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.
* Rename `complete` flag to `full_information` and provide rationale
This was confusing: the flag name made implementers *think* they
knew what it was for.
* Insist that reply_channel_range be adjacent and ordered
The current loose constraints causes confusion (and now all major
implementations meet the stricter requirements anyway).
You are allowed to provide more blocks than requested, but you have
to be complete and in order, and each reply has to have some overlap
with the requested range.
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>
