Bump dust exposure threshold

[alecchendev]

Addresses #2261. Adds a config knob allowing a user to set a multiplier on the high priority fee estimate to control their maximum dust exposure.

With fee rates rising dramatically in mid-April 2023, thresholds for what is considered dust have risen, often exceeding our previous default dust exposure threshold of 5k sats. This causes all payments and HTLC forwards between 5k sats and new dust thresholds to fail. Having a multiplier allows the maximum dust exposure to scale automatically with changing fee rates.

(This PR originally just bumped the default to 25k sats but has been changed to this new approach.)

[ariard]

The 25k threshold sounds good to me - The ideal balance between dust exposure risks from miner-as-counterparties and HTLC throughput on a channel would be a dynamic limit scaled up from mempool levels of congestion probed at periodic interval…However as the concern is going away with anchor channels, and we should ship them soon(tm) I don’t think we should think too much on the ideal value.

[codecov-commenter]

Codecov Report

Patch coverage: 91.50% and project coverage change: +0.14 🎉

Comparison is base (89aa7ac) 90.32% compared to head (b040335) 90.46%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2354      +/-   ##
==========================================
+ Coverage   90.32%   90.46%   +0.14%     
==========================================
  Files         106      106              
  Lines       54948    58190    +3242     
  Branches    54948    58190    +3242     
==========================================
+ Hits        49630    52643    +3013     
- Misses       5318     5547     +229     

Impacted Files	Coverage Δ
lightning/src/util/config.rs	56.56% <56.25%> (-1.58%)	⬇️
lightning/src/ln/channelmanager.rs	89.98% <94.73%> (+3.71%)	⬆️
lightning/src/ln/functional_tests.rs	98.26% <96.77%> (+<0.01%)	⬆️
lightning/src/ln/channel.rs	89.47% <100.00%> (+0.05%)	⬆️
lightning/src/ln/functional_test_utils.rs	88.10% <100.00%> (+0.27%)	⬆️
lightning/src/ln/onion_route_tests.rs	98.27% <100.00%> (+0.01%)	⬆️
lightning/src/ln/priv_short_conf_tests.rs	97.64% <100.00%> (+<0.01%)	⬆️
lightning/src/util/ser_macros.rs	66.18% <100.00%> (-3.30%)	⬇️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[TheBlueMatt]

With the current hard-limit design I don't think $1.50 is nearly high enough. Sadly, the "right answer" really depends on the fee market - if transaction fees are $0.50 (they are today), $1.50 seems low - you have to pay a third of that to go on chain, why are you worried about losing that to dust? If someone is doing some attack they're gonna do it for more than that. If fees go back down to where transactions cost $0.05, then $1.50 is probably not a crazy level.

The discussion on the issue veered into some other ideas like an HTLC count limit, rather than an HTLC value limit, which may make sense, though that really relies on our counterparty's fee estimator for inbound channels, rather than ours. Another option would be to set the dust limit as some (configurable) multiplier on the HighPriority fee estimate - we have to pay that to get funds back, you should be willing to lose at least a few of those at a time.

[alecchendev]

if transaction fees are $0.50 (they are today), $1.50 seems low - you have to pay a third of that to go on chain, why are you worried about losing that to dust?

Hmm that's a good point

The discussion on the issue veered into some other ideas like an HTLC count limit, rather than an HTLC value limit, which may make sense, though that really relies on our counterparty's fee estimator for inbound channels, rather than ours. Another option would be to set the dust limit as some (configurable) multiplier on the HighPriority fee estimate - we have to pay that to get funds back, you should be willing to lose at least a few of those at a time.

Okay, I'll think about these some more and try to get something up soon, if not I may drop this in favor of someone else picking it up since the milestone is coming up and I initially thought this change was going to be more trivial

[alecchendev]

Okay, I chose to go with having a configurable multiplier of the high priority feerate to set the dust exposure threshold since it seems the most straight forward without introducing any possible new issues IIUC (open to another approach if people think it's better however). I've mostly implemented it, just need to adjusts/write tests (done now).

I also saw in the issue some discussion about making it a global dust exposure limit rather than per-channel at the cost of some parallelism - is that worth implementing here as well?

[TheBlueMatt]

Didnt review the test changes but in general I think this looks great!

[alecchendev]

Rebased, fixed fuzzing, switched to using an enum instead of an extra field which I think is much nicer.

[TheBlueMatt]

Ugh, needs rebase again sorry about that.

[alecchendev]

Rebased

[ariard]

Moving to a real-time feerate-adjusted max_dust_htlc_exposure_msat sounds good, note the caveat about LDK clients relying on third-party fee-estimators.

[ariard]

If we would like to be informative towards LDK-based routing hops operators, I think we can indicate the risk of fee griefing attacks, where counterparties in collusion hold a HTLC payment path through a target and force-close the incoming commitment transaction to burn the non-settled dust HTLCs.

See the original description of dust exposure here: https://lists.linuxfoundation.org/pipermail/lightning-dev/2020-May/002714.html

[alecchendev]

I slightly reworded and added the link - let me know if you think that's enough or not

[ariard]

Thanks for doing it, well new comment missed the point of dissociating the two types of dust exposure risks:

• one by counterparties with mining capabilities both payment senders/routing hops
• one by counterparties without mining capabilities for routing hops only.

Enhanced the comment here ariard@ed9eabb (feel free to take it, squash it)

There is an added link to the disclosure post of October 2021, that is adding more context for node operators.

[alecchendev]

Oh I see, thanks.

There is an added link to the disclosure post of October 2021, that is adding more context for node operators.

In ariard@ed9eabb it looks like the two links are the same?

[ariard]

In ariard@ed9eabb it looks like the two links are the same?

Fuck my bad, here In ariard@ed9eabb

The correct link is this one https://lists.linuxfoundation.org/pipermail/lightning-dev/2021-October/003257.html, which is the most informative source on the mailing list on dust exposure, to the best of my knowledge.

[TheBlueMatt]

I think this basically LGTM, feel free to squash the fields.

[ariard]

Code Review ACK 6fb3fa0 modulo the better comment on “dust exposure”.

Verified the high_priority_feerate_per_kw * value computation for FeeRateMultiplier in get_max_dust_htlc_exposure_msat and how it’s used through the codebase, though I didn’t check the tests changes.

[TheBlueMatt]

Some trivial comments, mostly docs, feel free to squash when you address them, will find another reviewer.

[wpaulino]

Hmm, on second thought, maybe we shouldn't have gone the enum route to keep both around and have the feerate multiplier only apply to pre-anchor channels.

[TheBlueMatt]

Yea, that's something I was wondering about, but even if we're using anchors it's not unreasonable to want our dust exposure limit to float upwards. In the future we should probably think about the protocol letting us tweak our (but not our counterparty's) dust limit for anchor channels and then we'll want this.

[TheBlueMatt]

And in the mean time users are free to set a different limit on different types of channels.

[wpaulino]

And in the mean time users are free to set a different limit on different types of channels.

Yeah that's what I figured, but it doesn't seem very clear that users can do so. Maybe we add a sentence or two to let them know setting the feerate multiplier variant is mostly intended for pre-anchor channels in it's current form?

[alecchendev]

Maybe we add a sentence or two to let them know setting the feerate multiplier variant is mostly intended for pre-anchor channels in it's current form?

Added a comment on the config knob and the variant's docs.

Will anchor channels be the default in 0.0.116? If they are, does it still make sense for the default value here to be the multiplier variant?

[wpaulino]

Will anchor channels be the default in 0.0.116? If they are, does it still make sense for the default value here to be the multiplier variant?

Probably won't happen for another 2 releases or so. We can consider changing the default max exposure then.

[ariard]

Code Review ACK b040335712d, looked again into how we’re checking dust exposure in send_htlc(), send_update_fee() and update_fee(). Didn’t check at all backward compatibility issues.

Nice if we can add the good link on the most informative explanation of “dust htlc exposure”, though can be done as follow-up to avoid invalidating reviews so far.