While reviewing a patch on lnprototest, I encountered a scenario
where the BOLT 9 specification needed to provide clear guidance.
As a result, this commit adds specific requirements to
determine the appropriate behaviour when a node specifies
both optional and required features.
Additionally, if this situation appears to be an
implementation bug, it will be taken care of accordingly.
Reported-by: lnprototest
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@thomash-acinq points out:
1. We absolutely can put other fields in `encrypted_data_tlv`, esp. padding, and test vectors do this.
2. Presumably it was supposed to refer to onionmsg_tlv, so fix that.
3. And of course we need to allow payload fields!
These use onion encoding for simple one-way messaging: there are no error returns.
However, every onion uses route blinding *even if it doesn't need to*.
You can prove what path was used to reach you by including `path_id` in the
encrypted_data_tlv.
Note that this doesn't actually define the payload we're transporting:
that's explictly defined to be payloads in the 64-255 range.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When paying a blinded path, we don't have a CLTV delta at each hop
available, but rather only a total CLTV delta for the entire
blinded path.
However, the onion format currently still requires that we specify
an `outgoing_cltv_value` for the final hop. As the sender, we don't
have a sensible value to put there, as we don't know which part of
the total CLTV delta belongs to the recipient.
The sender is instructed to use the values that are known to them
when setting `outgoing_cltv_value` for the final hop:
- The current block height.
- Any additional delta added to account for block propagation and
improve privacy.
This change reflects the behavior of some implementations at the time
of writing.
In most cases the `onion_hash` isn't actionable in the case of blinded
payments and it's wasteful to keep track of the incoming onion, so we
allow setting it to an all zero value.
lnmessage got this wrong! It would pass our test vectors, but actually fail
in real usage, since it used the same `ck`.
Also, nonce rotation happens after 1000 encryptions, which is when the nonce
reaches 1000 (since it's zero based!), not when it *exceeds* 1000.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
6fee63fc34 made `htlc_maximum_msat`
compulsory, which was the last "optional" field. All future options
will be TLVs, so we never need to parse the "] (optionxxx)" format
for fields again.
We allow options for messages, but only for documentation (we have
some messages which are only for `gossip_queries`), so we don't
put it in the output.
Here's the before/after diff of the output for all the spec files:
```diff
--- /tmp/before 2023-04-27 14:21:07.417170213 +0930
+++ /tmp/after 2023-04-27 14:19:39.184573086 +0930
@@ -273,7 +273,7 @@
msgdata,channel_update,fee_base_msat,u32,
msgdata,channel_update,fee_proportional_millionths,u32,
msgdata,channel_update,htlc_maximum_msat,u64,
-msgtype,query_short_channel_ids,261,gossip_queries
+msgtype,query_short_channel_ids,261
msgdata,query_short_channel_ids,chain_hash,chain_hash,
msgdata,query_short_channel_ids,len,u16,
msgdata,query_short_channel_ids,encoded_short_ids,byte,len
@@ -281,17 +281,17 @@
tlvtype,query_short_channel_ids_tlvs,query_flags,1
tlvdata,query_short_channel_ids_tlvs,query_flags,encoding_type,byte,
tlvdata,query_short_channel_ids_tlvs,query_flags,encoded_query_flags,byte,...
-msgtype,reply_short_channel_ids_end,262,gossip_queries
+msgtype,reply_short_channel_ids_end,262
msgdata,reply_short_channel_ids_end,chain_hash,chain_hash,
msgdata,reply_short_channel_ids_end,full_information,byte,
-msgtype,query_channel_range,263,gossip_queries
+msgtype,query_channel_range,263
msgdata,query_channel_range,chain_hash,chain_hash,
msgdata,query_channel_range,first_blocknum,u32,
msgdata,query_channel_range,number_of_blocks,u32,
msgdata,query_channel_range,tlvs,query_channel_range_tlvs,
tlvtype,query_channel_range_tlvs,query_option,1
tlvdata,query_channel_range_tlvs,query_option,query_option_flags,bigsize,
-msgtype,reply_channel_range,264,gossip_queries
+msgtype,reply_channel_range,264
msgdata,reply_channel_range,chain_hash,chain_hash,
msgdata,reply_channel_range,first_blocknum,u32,
msgdata,reply_channel_range,number_of_blocks,u32,
@@ -310,7 +310,7 @@
subtype,channel_update_checksums
subtypedata,channel_update_checksums,checksum_node_id_1,u32,
subtypedata,channel_update_checksums,checksum_node_id_2,u32,
-msgtype,gossip_timestamp_filter,265,gossip_queries
+msgtype,gossip_timestamp_filter,265
msgdata,gossip_timestamp_filter,chain_hash,chain_hash,
msgdata,gossip_timestamp_filter,first_timestamp,u32,
msgdata,gossip_timestamp_filter,timestamp_range,u32,
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Adds a comment to the `onion-test.json` file to clarify that the
payloads specified for each hop in the test already include the variable
length encodings.
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
Add specification requirements for using route blinding to make payments
while preserving recipient anonymity. Implementers must ensure they
understand all those requirements, there are subtle attacks that could let
malicious senders deanonymize the route if incompletely implemented.
Add specification requirements for creating and using blinded routes.
This commit contains the low-level details of the route blinding scheme,
decoupled from how it can be used by high-level components such as onion
messages or payments.
Route blinding allows a recipient to provide a blinded route to
potential payers. Each node_id in the route is tweaked, and dummy
hops may be included.
This is an alternative to rendezvous to preserve recipient anonymity.
It has a different set of trade-offs: onions are re-usable, but the
privacy guarantees are a bit weaker and require more work (e.g. when
handling payment fees and errors).
The `commitment tx with 3 htlc outputs, 2 offered having the same amount
and preimage` test was not correctly updated after the value of test
htlc 6 was changed to 5000001 and the cltv expiry of test htlc 5 was
changed to 506. This commit updates the anchors test accordingly.
The `commitment tx with 3 htlc outputs, 2 offered having the same amount
and preimage` test was not correctly updated after the value of test
htlc 6 was changed to 5000001 and the cltv expiry of test htlc 5 was
changed to 506. This commit updates the static-remote test accordingly.
The `commitment tx with 3 htlc outputs, 2 offered having the same amount
and preimage` test was not correctly updated after the value of test
htlc 6 was changed to 5000001 and the cltv expiry of test htlc 5 was
changed to 506. This commit updates the legacy test accordingly.
The commitment transaction tests are all meant to use the same funding
transaction which has an amount of 10000000000 msat. The LocalBalance
and RemoteBalance along with the value of any htlcs should always add up
to this amount.
This commit updates the `simple commitment tx with no HTLCs and single
anchor` anchors test to comply with the above.
These are the same test vectors as those found under Appendix F, except
that each HTLC has a zero fee transaction instead, resulting in a
signature change.
* Use onion amount in MPP set calculation
The sender chooses the amounts that are set in the onion payload
(`amt_to_forward`) but cannot predict what amounts will be set in the
HTLCs (`amount_msat`) since intermediate nodes are allowed to overpay.
* Fix error requirements for final node
These requirements were missed when integrating #1032
When nodes receive HTLCs, they verify that the contents of those HTLCs
match the intructions that the sender provided in the onion. It is
important to ensure that intermediate nodes and final nodes have similar
requirements, otherwise a malicious intermediate node could easily probe
whether the next node is the final recipient or not.
Unfortunately, the requirements for intermediate nodes were more lenient
than the requirements for final nodes. Intermediate nodes allowed overpaying
and increasing the CLTV expiry, whereas final nodes required a perfect
equality between the HTLC values and the onion values.
This provided a trivial way of probing: when relaying an HTLC, nodes could
relay 1 msat more than what the onion instructed (or increase the outgoing
expiry by 1). If the next node was an intermediate node, they would accept
this HTLC, but if the next node was the recipient, they would reject it.
We update those requirements to fix this probing attack vector.
We also clarify `min_final_cltv_expiry`: this is actually a cltv_expiry_delta,
not an absolute cltv_expiry, so the field name should reflect that.
Recipients require incoming HTLC expiry to comply with that expiry delta.
When a node retires a failed path as part of a larger MPP payment,
the node may wish to use a path which is constrained by an
`htlc_minimum_msat` value. In this case, the node is forced to
overpay, likely overshooting the `total_msat` it set in the earlier
onions for the same MPP payment.
There are two possible solutions to this - either allow the
`total_msat` value to change in later HTLCs or allow the node to
(slightly) overshoot the `total_msat` value.
Allowing `total_msat` to change across HTLCs is nontrivial to
implement - HTLCs may arrive out-of-order, causing the receiving
node to have to track all seen `total_msat` values and accept a
set of HTLCs which meet any of the seen `total_msat` values.
Instead, this commit changes the MPP logic to simply allow a sender
to overshoot the stated `total_msat`.
Sadly the backwards-compatibility story for this is not great.
There doesn't seem to be a good way to resolve this issue in a
backwards-compatible way. Instead we just bite the bullet and make
the incompatible change, hoping the overshooting is rare enough
that it's not a major issue.
Requirements for the htlc_maximum_msat field in channel_update were
inadvertently removed by #999 (this PR meant to make this field mandatory,
not removed explanations about what it does).
To only use valid tlv payloads instead of fixed-size legacy ones and
invalid tlv streams.
[ Minor typo change: third payload is 275 not 256 bytes long --RR ]
My measurements a few weeks ago reveal that only 5 nodes do not
advertize this feature, of over 17000. I have a patch to
remove support from c-lightning, too.
[ 6 months later: t-bast notes that they only see 0.2% of htlcs using
legacy, and my node hasn't seen one for 2 months w/ 12000 htlcs --RR ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The order of operations is now more clearly specified as:
HTLC output amount = (`amount_msat` / 1000) - (fees in satoshis) where all
divisions are rounded down.
This is required to avoid issues in rounding if we were to take
HTLC output amount = (`amount_msat` - (`feerate_per_kw` * weight)) / 1000 and
then rounded down.
Since #910, nodes are allowed to use aliases instead of real scids. It is
helpful to make it explicit that updates using such aliases must not be
forwarded to other nodes by setting a flag in `channel_update`.
This flag is also generally useful for unannounced channels, regardless
of whether they use an scid alias or not.
We also make the `htlc_maximum_msat` field mandatory: every node on the
network currently sets it, so we can simplify the spec.
This commit ensures closing_signed can only begin if there are
no dangling commitments. It also clarifies update_fee requirements
if it is sent after shutdown.
When a node creates a new `channel_update` to change its channel parameters,
it will take some time to propagate through the network and payers may use
older parameters. It is recommended to keep accepting older parameters for a
while to improve payment latency and reliability.
