Broken link 2 #1
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| * [`to_local_anchor` and `to_remote_anchor`](#to_local_anchor-and-to_remote_anchor-output-option_anchor_outputs) | ||
| * [`to_local_anchor` and `to_remote_anchor`](#to_local_anchor-and-to_remote_anchor-output-option_anchors) |
There was a problem hiding this comment.
have you tried to remove this and check if the CI will fail?
There was a problem hiding this comment.
it also did not catch this one
There was a problem hiding this comment.
Amazing a url checker that is not able to check the relative url! so maybe you can run your scrip inside the CI?
antonilol
pushed a commit
that referenced
this pull request
Aug 13, 2022
fix and make scripts smaller
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 <[email protected]>
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 ]
Requirements for the htlc_maximum_msat field in channel_update were inadvertently removed by lightning#999 (this PR meant to make this field mandatory, not removed explanations about what it does).
Data is not relevant for failure message generation.
With the tlv payload, the maximum isn't fixed anymore.
…1031) 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.
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.
* 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 lightning#1032
TLV failure message and recommended length to 1024
Make it more obvious to readers of bolt 11 that the maximum length of a Tagged Field is 639 bytes.
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.
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.
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 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 anchors test accordingly.
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).
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.
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.
In lightning#1032 we allowed overshooting the final amount and expiry, but forgot to update the onion error descriptions which make reference thereto.
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 lightning#1072
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 <[email protected]>
Remove requirements to disconnect on warnings
This is a follow-up to lightning#1092 that fixes the following issues: - fix a few typos - remove non-zero-fee anchors test cases - remove `remote_pubkey` rotation
Clean-up: follow-up on removing spec features
Signed-off-by: Rusty Russell <[email protected]> Header from folded patch 'bolt_2__set_an_initiator_in_quiescence.patch': BOLT lightning#2: Set an initiator in quiescence. This is especially useful for protocols such as splicing; for simplified commitment transactions, there is already an implied initiator at each point, so having the negotiation at splicing time would be redundant. Signed-off-by: Rusty Russell <[email protected]> Header from folded patch 'option_quiesce__feature_to_support_stfu_method.patch': option_quiesce: feature to support stfu method. In practice, sftu is useless unless you have something (e.g. channel_upgrade) which uses it, but adding a feature is best practice IMHO. Signed-off-by: Rusty Russell <[email protected]>
…ith HTLCs pending. Signed-off-by: Rusty Russell <[email protected]>
To avoid timing analysis when decrypting failed payments the sender should act as if the failure in the route came for the 27th hop. Also changed the maximum number of hops in the route from 20 (legacy) to 27 (tlv onion).
As noted previously, `channel_update`s in the onion failure packets are massive gaping fingerprintign vulnerabilities - if a node applies them in a publicly-visible way the err'ing node can easily identify the sender of an HTLC. While the updates are still arguably marginally useful for nodes to use in their pathfinding local to retires of the same payment, this too will eventually become an issue with PTLCs. Further, we shouldn't be letting nodes get away with delaying payments by failing to announce the latest channel parameters or enforcing new parameters too soon, so treating the node as having indicated insufficient liquidity (or other general failure) is appropriate in the general case. Thus, here, we begin phasing out the `channel_update` field, requiring nodes ignore it outside of the current payment and making it formally optional (though nodes have been doing this for some time due to various bugs). Because some nodes may want to use update data on mobile when they have stale gossip data, it is left optional.
* BOLT4: include `min_final_cltv_expiry_delta` in `max_cltv_expiry` calc Include `min_final_cltv_expiry_delta` in the `max_cltv_expiry` calculation. Also add a note that indicates that this field may be set for the final node too. This is useful for the final node as then it does not need to persist the path expiry separately and can rely on just checking the `payment_relay` field when the payment arrives. * BOLT4: include calculation for `total_cltv_delta` of a blinded path Include an explicit formula to use for determining the total CLTV delta of a blinded path so that it is clear that it should include the recipient's `min_final_cltv_expiry_delta`. * proposals: fix `max_cltv_expiry` value for final hop in example More info [here](lightning#1174 (comment)) outlining why the example needed to be updated.
This was from the legacy onion, and is no longer present. Signed-off-by: Rusty Russell <[email protected]>
There's currently a *description* of how to decrypt an onion, and some requirements in forwarding. But it also applies to onion messages, so: 1. Turn the description into actual enumerated requirements. 2. Ensure the description covers both payload and messaging onions. 3. Include both methods to apply the blinding tweak. 4. Leave the actual handling of the extracted payload (payment vs messaging onion) to those specific sections (e.g. reporting failure) Signed-off-by: Rusty Russell <[email protected]>
…tlc/onion message requirements. This ties it together, saying what to use as associated data, blinding, and what to do on failure. Signed-off-by: Rusty Russell <[email protected]>
Sure, it's used to derive a secret for blinding, but it's also used to derive the key for encrypted_recipient_data. It's not used as a blinding factor *directly*. Signed-off-by: Rusty Russell <[email protected]>
This commit doesn't change the logic at all, it simply: - removes `realm` from onion test vector - cleans-up markdown formatting and indents - fixes typos and missing parenthesis - consistently uses `_` instead of `-` for field names - fixes math formatting (including changes from lightning#1169 and lightning#1158)
…` type there. It's currently buried in the onion message section, but it applies to payments too. We now have a separate sub-section for the encrypted_data_tlv definition. Signed-off-by: Rusty Russell <[email protected]>
…nstructing. This spec was initially written before the `blinded_path` type existed. Be precise (and we no longer need to say "MUST communicate"!). Signed-off-by: Rusty Russell <[email protected]>
Writer parts: 1. Be explicit that the writer creates a route. 2. Make it clear we create shared secrets, then derive blinded points. 3. Refer explicitly to all `blinded_path` fields. Split reader into the *two* readers: 1. The reader of the blinded path, who uses it to make an onion (which wasn't described at all!) 2. The reader of the encrypted_recipient_data, who decrypts it. In the latter case, we don't have to discuss unblinding the onion since that's now covered in the "Onion Decryption" section. Signed-off-by: Rusty Russell <[email protected]>
It's a bit complex, but try to convey the idea of an introduction point, blinded node ids and encrypted blobs. Since the requirements detail the two ways to reach the introduction node, I handwaved on that a bit. Signed-off-by: Rusty Russell <[email protected]>
It was mentioned in the rationale only. Signed-off-by: Rusty Russell <[email protected]>
It's used for blinded payment paths, too, not just onion messages. Suggested-by: @t-bast Signed-off-by: Rusty Russell <[email protected]>
1. Always use the term `encrypted_recipient_data` for the encrypted field: it's confusing enough without multiple names! 2. Don't give an option for joining blinded payments, since everyone will use an unblinded payment to the introduction node (at least, for now). 3. Avoid the term "payer" and at least note that encrypted_recipient_data can be made by the sender themselves, pointing out that the forwarding node cannot tell. Thanks t-bast and gijswijs for this feedback! Signed-off-by: Rusty Russell <[email protected]>
Bastien points out we didn't update this when we changed terms. Signed-off-by: Rusty Russell <[email protected]>
Fix link reference to point at sub section with minimal link reference
…rt-2 Clarify onions part 2: a bit deeper rework
All the test vectors use static keys now, which are listed above already as the local_payment_basepoint and remote_payment_basepoint. The keys listed here are the pre-static rotated ones: if you use these, the vectors don't work! We actually use the same basepoint for the HTLCs, but never spelled it out. So do that now, and these are the local/remote htlc keys. Signed-off-by: Rusty Russell <[email protected]>
* bolt03: fix link to `bitcoind`'s `policy.cpp` file The original link is broken, we now replace it with a permalink. * bolt03: remove 0 `txout` count in closing tx Txns cannot have zero outputs. * bolt03: remove wrong ref to `remotepubkey` and fix format `remotepubkey` has been placed at the wrong place, also fixes the format so it's easier to read.
This makes more sense given the context of combining bitmaps.
Clarify test vector.
antonilol
force-pushed
the
broken-link-2
branch
from
f4647f6
to
ff7457b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
test gh ci