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.
And `next_per_commitment_point` to explictly `second_per_commitment_point`;
this is particularly important since `channel_ready` can be retransmitted
after the channel has been in use, for example.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And weaken it: the opener doesn't need to respect it.
Note also that the `funding_locked`-can-change-alias refers to the same peer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This lets you add your brand new channel to routehints, and also
means you can use a routehinted channel even if you (later?) have a
real channel.
This supports both trusted and untrusted zero-conf channels: in the
trusted case you can use it immediately like any other channel,
and for the untrusted case you simply use any push_msat they gave you
for outgoing payments, but fail incoming.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The rationale for this is to avoid bad cases like the following one
which was previously allowed:
* sender -> shutdown(script_one) -> receiver
* sender -> shutdown(script_two) -> receiver
* sender <- shutdown(script_one) <- receiver
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
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>
