Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lightning Specification Meeting 2022/10/10 #1030

Closed
2 of 26 tasks
t-bast opened this issue Oct 6, 2022 · 12 comments
Closed
2 of 26 tasks

Lightning Specification Meeting 2022/10/10 #1030

t-bast opened this issue Oct 6, 2022 · 12 comments

Comments

@t-bast
Copy link
Collaborator

t-bast commented Oct 6, 2022

The meeting will take place on Monday 2022/10/10 at 7pm UTC (5:30am Adelaide time) on Libera Chat IRC #lightning-dev. It is open to the public.

A video link is available for higher bandwidth communication: https://meet.jit.si/Lightning-Spec-Meeting

Pull Request Review

Waiting for interop

Long Term Updates

Backlog

The following are topics that we should discuss at some point, so if we have time to discuss them great, otherwise they slip to the next meeting.

@t-bast t-bast pinned this issue Oct 6, 2022
@t-bast
Copy link
Collaborator Author

t-bast commented Oct 6, 2022

We're entering DST mess, not 100% sure I got the meeting time right, I may update it at the last minute if it's wrong, watch closely.

@t-bast
Copy link
Collaborator Author

t-bast commented Oct 7, 2022

There's a topic I'd like to pick everyone's brain on as well, that came up in #765 (comment)

Why do we include outgoing_cltv in the final hop onion payload? As @thomash-acinq pointed out, this feels unnecessary and isn't associated with a requirement.

EDIT: we figured it out, posting here for others wondering why it's done this way. This is to protect against probing that would let the next-to-last node discover that they are the next-to-last node (leaking the identity of the recipient).

@joostjager
Copy link
Collaborator

Is that reason still valid now that we have the payment secret?

@t-bast
Copy link
Collaborator Author

t-bast commented Oct 10, 2022

I don't see how the payment_secret would protect against that?

@joostjager
Copy link
Collaborator

I thought that you were referring to the probing as described in #608. What would the type of probing that you mention look like assuming there wouldn't be an outgoing_cltv in the final hop onion?

@thomash-acinq
Copy link

Without outgoing_cltv for the last hop, a node in the route could try setting a CLTV that's off by one and get a different result if the next node is the last or not:

  • a relay node would check incoming_cltv - cltv_delta = outgoing_cltv and would fail if it is off by one.
  • the final node would just check incoming_cltv >= min_final_cltv and would probably still accept it if it is only off by one.

@joostjager
Copy link
Collaborator

But is it indeed the case that a relay node would fail? In LND I see:

https://github.com/lightningnetwork/lnd/blob/406fc86f38a6e45bf1a04f27a010c6c339ddd244/htlcswitch/link.go#L2539

Looks like forwards are also accepted if the delta is bigger. So maybe the opposite is currently the case, at least with lnd? Relay nodes accept off by one, where as final nodes don't.

@t-bast
Copy link
Collaborator Author

t-bast commented Oct 10, 2022

Good catch, I believe we actually have a probing issue today because the intermediate relaying behavior doesn't match the final node behavior...what you're highlighting of lnd's relaying behavior matches the specification and what eclair does, but the final node checks for equality of the htlc expiry and the onion payload expiry, since Bolt 4 says (where cltv_expiry_delta is probably a mistake and should be outgoing_cltv_value):

if it is the final node:
    - MUST treat total_msat as if it were equal to amt_to_forward if it is not present.
    - MUST return an error if:
        - incoming amount_msat != amt_to_forward.
        - incoming cltv_expiry != cltv_expiry_delta.

That indicates that amt_to_forward also is a probing vector, so it would make sense to verify only an inequality as you suggest in #1031

A good topic to discuss during today's meeting!

@t-bast
Copy link
Collaborator Author

t-bast commented Oct 10, 2022

I detailed a sample scenario to showcase this, my current conclusion is that @joostjager is right that we should allow a higher amount_msat in htlcs than indicated by the onion (see #1025) for context. Let me know if I'm missing something here.

Alice sends a payment to Carol with the following path: Alice -> Bob -> Carol -> Dave.
Alice creates an onion containing the following payloads:

  • Bob:
    • amt_to_forward = 25_001_000 msat
    • outgoing_cltv_value = 600
  • Carol:
    • amt_to_forward = 25_000_000 msat
    • outgoing_cltv_value = 500
  • Dave:
    • amt_to_forward = 25_000_000 msat
    • outgoing_cltv_value = 500

Bob wants to probe and creates an htlc for Carol with cltv_expiry = 601 instead of 600.
Carol does relay the payment since the corresponding cltv_expiry_delta = 101 is greater than expected.
Since Carol is honest, she sends an htlc to Dave with cltv_expiry = 500, which he accepts.

If instead Carol wants to probe and creates an htlc for Dave with cltv_expiry = 501, Dave will reject it and reveal that he's the recipient.
Dave should check that update_add_htlc.cltv_expiry >= onion_payload.outgoing_cltv_value instead of equality.

This probing also works with amounts.
If Bob wants to probe, he will take 1 msat from his fee and create an htlc for Carol with amount_msat = 25_001_001 instead of 25_001_000.
Carol does relay the payment since the fee it pays is greater than expected.
Since she's honest, she follows the onion instructions and sends an htlc to Dave with amount_msat = 25_000_000, which he accepts.

If instead Carol wants to probe and creates an htlc for Dave with amount_msat = 25_000_001, Dave will reject it and reveal that he's the recipient.

@joostjager
Copy link
Collaborator

That's a nice addition. I was focusing on the sender overpaying the fee to meet a min_htlc policy, but what you're saying doesn't even require that. The routing node just taking an msat out of his fee.

@Roasbeef
Copy link
Collaborator

forwarding instructions are too strict:

  • Few different things here:
    • When you route an HTLC at HTLC min, then you might need to overpay somewhat to satisfy that check.
      • Seems ppl haven't hit this, but can arise in theory
    • Other thread of if you should allow general overpayment for the MPP total_msat value
      • Probing vector: a node can overshoot a last hop by trying to send a larger value for amount to see if things get rejected or not?
    • Other thread on what value you should put in the onion if you are overpaying and if/how all the cases can be properly identified
      • Ideally want to send back fee insufficient error, otherwise you need to wait for the set to complete or the timeout. Seems not able to create errors as granular as one would like
        * Another idea: can also insert an "amount should have received" into the onion, which lets nodes catch others taking advantage of the slack
    • Simplest thing: accept overpayment at the final hop -- ppl already do this?

inherited ID thing as an alternative to APO:

  • t-bast re-read the paper, thought it was interesting
  • what are the tradeoffs?
    • seems to require another index w.r.t this other txid?
    • allows similar functionality
  • newer versions since then, maybe worth checking out

taproot chans:

  • musig 1.0 is now a thing!
  • means now able to start to do proper cross-impl testing
  • question re nonces:
    • attempt to never commit nonces to disk
    • have a few other options: counter based, and deterministic
      • counter needs state on disk
      • deterministic can maybe work assuming something somethign shachain or other information

@t-bast
Copy link
Collaborator Author

t-bast commented Oct 11, 2022

Thanks @Roasbeef for the notes!

@t-bast t-bast closed this as completed Oct 19, 2022
@t-bast t-bast unpinned this issue Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants