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>
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>
Do not reply with a node_announcement if the query includes an optional query flag that does not request it.
The current wording could be interpreted as "always follow with node announcements whenever
you reply with a channel announcements" which defeats the point of using query flags (if you want the node
announcements just set the corresponding bits).
We use the more tool-friendly `...*` description for TLV extensions.
Checksums are now serialized as raw arrays, as using zlib compression here would not help.
Since some can be zero (missing updates), it's probably worth
doing the compression thing optionally.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Formatting changes only.
This make tools/extract-formats.py work (well, it misses some stuff
until the tlv-testcases merge, but then it's OK).
We use `tlvs` (for tlv stream), and we refer to TLV records as "being
included" rather than re-using the TLV name.
We even use subtypes for the pairs of checksums and timestamps.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Nodes that support extended queries will append an additional extended query flag to
their `query_channel_range` queries. If the receiver supports extended queries and
understands this flag, it will append the required additional data to its
`reply_channel_range` message.
There is currently only one type of additional data: one timestamp and one checksum
per `channel_update`.
The checksum is a CRC32 checksum computed over the `channel_update`
with `timestamp` and `signature` omitted.
Along with query_short_channel_ids extension, this can be used to
avoid querying `channel_updates` that are older than the ones you
already have, or that are newer but don't include new information.
Nodes can append additional data to their `query_short_channel_ids`
messages, which consists in one flag per short channel id and
specifies what they would like to receive (`node_announcement`,
`channel_announcement`, or/and one `channel_update` or both).
The specification currently doesn't specify the case where the onion per-hop
payload can't be correctly decoded.
This is somewhat fine with the fixed frames because every field of the payload
can always be interpreted as a numeric value from the input bytes, so it leads
to application errors in upper layers when those values are actually
interpreted (and we realize that for instance we have an invalid
short_channel_id` value).
With variable-length tlv streams in the onion payloads, we will encounter
decoding errors (duplicate tlv types, invalid ordering, etc) and the spec
should define the failure code to use in that case.
Most obviously, we want this for BASE AMP, but it's useful in future.
Even though even bits won't cause existing implementations to know
they can't pay the invoice, it will allow it in future once everyone
has upgraded.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If a node sends its own `channel_update` to a peer node before receiving a `funding_lock`, the peer node may discard because it has not `short_channel_id` yet.
* Added descriptions of how a 2-of-2 multisignature verification is used for enforcing timelocks when timing out on-chain offered HTLCs as well as spending on-chain received HTLCs in the success case.
In commit 914ebab908 the
incorrect_payment_amount error was merged into
incorrect_or_unknown_payment_details to prevent a probing attack
that allowed intermediate nodes to learn the final destination of
a payment.
A similar attack is possible using the htlc expiry value. By trying
payments with the correct amount but low expiry values to candidate
destinations, an incorrect_or_unknown_payment_details error can be
elicited. This would signal that the probe payment was sent to the
final destination.
For the intermediate node to determine the correct amount, an estimate
must be calculated. A logical choice would be the outgoing amount of the
intermediate node plus some allowance for routing fees that would
otherwise be paid to subsequent nodes along the path.
Picking a low enough - but not too low - expiry value is more tricky.
A reasonable guess could be made using external knowledge of the
final destination's implementation defaults or the type of invoice that
is paid to. Especially in the case of an hodl invoice that typically has
a large expiry delta, it is easier to make a correct guess.
This form of attack is arguably harder to realize than the amount probe
that was previously possible. The attacker may accidentally
pay the invoice if the expiry value guess satisfies the invoice
final cltv requirement. In that case, the attacker still has the
incoming htlc to pull which limits the loss.
In practice, using '...*type' is the clearest and simplest way to specify
the common case of "the rest of the TLV is an array of 'type'", rather
than some arbitrary expression with a made-up length field.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
