option_dusty_htlcs_uncounted (Feature 32/33)

[Crypt-iQ]

Quoting from mail conversation:

I think a new proposal which changes this would be possible, with a
feature bit:

  - if `option_dusty_htlcs_uncounted`:
    - we define "counted HTLC offers" as the total number of HTLCs
      committed on the remote peer which have `amount_msat` greater
      or equal to the remote `dust_limit_satoshis`
  - otherwise:
    - we define "counted HTLC offers" as the total number of HTLCs
      committed on the remote peer

  - if "counted HTLC offers" from this HTLC would exceed `max_accepted_htlcs`:
    - MUST NOT add an HTLC.

Similar language on the receiver side.

Since we all have interpreted the spec in the same way (counting all htlc's), we'll need feature bits to upgrade. Thoughts?

[TheBlueMatt]

Discussion today:

<rusty> #topic https://github.com/lightningnetwork/lightning-rfc/issues/873
<BlueMatt> yes, would be nice, someone's gotta do the work :/
<rusty> Someone remind me of the purpose of this?  Is it more a "why not, it's free?"
<BlueMatt> yea, basically, though marginally increases the cost of dos'ing a channel so you have to actually put up non-dust values
<roasbeef> basically that someone can add dust and make your channel less useful 
<rusty> Well, since dust levels don't have to be the same on both sides, this proposal is insufficient?
<BlueMatt> tbh i didnt read the proposed text lol
<rusty> I think it has to be "if either side considers it non-dust, we count it"?
<BlueMatt> yea, probably what you said rusty
<roasbeef> rusty: well the dust is already asymmetric, so I imagine this would be as well?
<roasbeef> at a high level is proposes not counting dust htlcs to the max htlc limit 
<roasbeef> which would then be clamped only by the max dust settings 
<roasbeef> tho maybe just two limits a better? 
<BlueMatt> but if you have one side that adds a billion dust outputs but they aren't dust for the other side, then the other side cant broadcast a transaction cause it has a billion outputs?
<rusty> Well, I guess if you propose 1M dust htlcs which are not dust for yourself, that's on you?
<BlueMatt> cause the dust limit of the broadcaster applies to all the htlcs not just the broadcaster's htlcs
<BlueMatt> wouldnt you be able to prevent someone from broadcasting their commitment tx?
<rusty> BlueMatt: the proposal says you can't add an htlc if it would go over the max for the *other* side.
<roasbeef> BlueMatt: each side says what their dust limit is tho 
<joost_> hi all. won't this open up a new vector where an attacker can send indeed a billion htlcs along the same (26 hop) route into the network? it won't weigh down on the commit txes, but it does on processing
<rusty> BlueMatt: so I think you can only shoot yourself.
<joost_> (1 msat htlcs)
<roasbeef> joost_: yeh so there needs to be another count clamp on dust htlcs themselves 
<BlueMatt> joost_: I dont see why that needs a separate limit? you already have fees and its no different than any other htlc
<roasbeef> I think the ML post has more context here also 
<rusty> BlueMatt: if you set fees to zero you get this problem though, I guess.
<roasbeef> I g2g in 10 
<BlueMatt> rusty: I mean you get that problem if you send a bajillion 1sat htlcs too....
<BlueMatt> rusty: ok, I think I agree that as worded you can only shoot yourself. I'd propose mentioning that you can shoot yourself in the spec.
<roasbeef> BlueMatt: another limit since those would all be dust, you need to clamp dust and non dust in total count of each 
<BlueMatt> roasbeef: why
<BlueMatt> roasbeef: what is wrong with having a ton of dust outputs?
<roasbeef> that PR says don't count dust towards the max htlc number anymore
<rusty> Thinking more, I'm not sure what the actual CPU limitation BigNum dust HTLCs would be.  I suspect we may have some O(N^2) somewhere in our tx construction code maybe?
<roasbeef> what stops a commitment from being fully dusted if that isn't there BlueMatt ?
<BlueMatt> rusty: it doesnt feed into tx construction, though?
<BlueMatt> rusty: you dont have to send signatures for a dust htlc, too!
<rusty> BlueMatt: we discard them though, and turn them into fees.
<BlueMatt> roasbeef: you already have a limit on the total in-flight?
<rusty> BlueMatt: I think that's O(N).
<roasbeef> BlueMatt: that PR says you don't count dust towards total in flight anymore 
<BlueMatt> roasbeef: htlc count, I assumed we'd still count them towards value-in-flight
<rusty> BlueMatt: yeah, but a 1 BTC channel can easily have > 10M dust htlcs.
<BlueMatt> rusty: right, would need to read code carefully, but i was assuming signature creation/checking would basically dwarf you in any case
<BlueMatt> ie even one signature could dwarf 1M simple ifs in a for loop....depending on how the code is
<rusty> BlueMatt: I'd like to benchmark, just to be sure.  We never bothered optimizations for < 1000 htlcs as we have now.
<BlueMatt> yes, agreed
<rusty> OK, so next step is to turn it into actual patch with caveats mentioned here?
<BlueMatt> +1
<roasbeef> i'll ping eugene about that
<roasbeef> (crypt-iq)
<rusty> #action rusty to assign feature bit for #873
<niftynei> seems like setting a "max fees from dust" limit might be a better way to approach this than a cap on the num of 'dust htlcs'
<BlueMatt> let me paste this on the issue
<BlueMatt> niftynei: sgtm
<rusty> OK, I say we end the meeting early, rather than trying to squeeze something in 4 minutes?
<BlueMatt> niftynei: its already hard to pick appropriate dust limits trying to munge a "max fees from dust" limit out of it. having an explicit "max fees from dust" seems strictly better than today in any case.

[Crypt-iQ]

Let A's dust_limit_satoshis > B's dust_limit_satoshis

• If A is forwarded an HTLC under A's dust_limit_satoshis, and above B's dust_limit_satoshis:
  • it will never be on A's commitment transaction so won't count towards A's max_accepted_htlcs
  • it may be on B's commitment transaction depending on fees, and will count towards B's max_accepted_htlcs.
• If A is forwarded an HTLC under A's dust_limit_satoshis, and under B's dust_limit_satoshis:
  • it will never be on either side's commitment transaction, doesn't count towards either max_accepted_htlcs.
• If A is forwarded an HTLC above A's dust_limit_satoshis, and above B's dust_limit_satoshis:
  • it may be on A's commitment transaction, and may be on B's commitment transaction, count towards both sides max_accepted_htlcs.

Joost brings up a good point about dust HTLC spam, and niftynei brings up a good point about dusted fees.
Personally, I think we should limit outstanding number of dust HTLCs as LND has in-memory queues that store the HTLCs (not sure about other impls). If we don't take into account dusted fees, the entire channel balance could be in-flight and dusted, which may not have been possible before depending on the channel's parameters. I think these parameters could be defined in the open_channel/accept_channel TLV segments. Hopefully this addresses the problems mentioned.