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

Increase likelihood that payments below dust go through #1751

Closed
wants to merge 1 commit into from

Conversation

luckysori
Copy link
Contributor

With transaction fees going up, certain payments that would normally succeed can fail because they end up below the dust limit.

By increasing the max_dust_htlc_exposure_msat from a dynamic value of 5 * high_priority_feerate_per_kw sats to a fixed value of 10_000_000 sats, we increase the likelihood that payments below dust go through.

For example, in a high transaction fee environment where the high priority fee rate is 200 sat/vByte (50_000 sat/KW), before this patch we would allow up to 50_000 * 5 = 250_000 sats in dust HTLCs, whereas now we would allow up to 10_000_000 sats.

This does come at the risk of losing more funds to transaction fees if dust HTLCs are part of the channel when it is force-closed.

@luckysori luckysori requested review from bonomat and holzeis December 14, 2023 03:27
@luckysori luckysori self-assigned this Dec 14, 2023
@luckysori luckysori force-pushed the fix/payment-below-dust branch from 9d7703a to de6dc3a Compare December 14, 2023 03:28
With transaction fees going up, certain payments that would normally
succeed can fail because they end up below the dust limit.

By increasing the `max_dust_htlc_exposure_msat` dynamic fee rate
multiplier from `5_000` to `100_000`, we increase the likelihood that
payments below dust go through.

For example, in a high transaction fee environment where the high
priority fee rate is 200 sat/vByte (50_000 sat/KW), before this patch
we would allow up to `50_000 * 5 = 250_000` sats in dust HTLCs,
whereas now we would allow `50_000 * 100 = 5_000_000` sats.

This does come at the risk of losing more funds to transaction fees if
dust HTLCs are part of the channel when it is force-closed.
@luckysori luckysori force-pushed the fix/payment-below-dust branch from de6dc3a to bc426d8 Compare December 14, 2023 04:19
holzeis

This comment was marked as outdated.

Copy link
Contributor

@holzeis holzeis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's give this a go!

@holzeis
Copy link
Contributor

holzeis commented Dec 16, 2023

@luckysori In case the negotiated fees on that channel are very high it would mean we can send the max amount of 10k sats right?

@holzeis
Copy link
Contributor

holzeis commented Dec 21, 2023

@luckysori Is there anything left on this PR or can we get it merged?

@luckysori
Copy link
Contributor Author

@luckysori In case the negotiated fees on that channel are very high it would mean we can send the max amount of 10k sats right?

I'm not sure I get you.

The example I left in the docs might be helpful:

/// Fee rate multiplier used to dynamically calculate how many msats we allow to have in incoming,
/// dust HTLCs for a channel.
///
/// The larger this value the less likely we are to reject a payment that would produce a dust HTLC.
///
/// ### Example
///
/// - High priority fee: 200 sat/vByte (50_000 sat/KW).
///
/// - Fee rate multiplier: 100_000.
///
/// Total dust HTLC sats: (50_000 * 100_000) / 1_000 = 5_000_000.
const MAX_DUST_HTLC_EXPOSURE_FEE_RATE_MULTIPLIER: MaxDustHTLCExposure =
    MaxDustHTLCExposure::FeeRateMultiplier(100_000);

We set the max_dust_htlc_exposure to MaxDustHTLCExposure::FeeRateMultiplier(100_000) for both parties, so the app-coordinator channels will have a variable max dust HTLC exposure. E.g. if the current fee rate is 200 sat/vByte (like in the example above) the parties will accept up to 5 million sats in dust HTLCs (including any other inbound, unclaimed dust HTLCs already in the channel).

Also I don't think the fee rate is negotiated. It simply depends on the fee rate source for each party. It allows either party to reject inbound dust HTLCs based on their view of the blockchain transaction fees.

@luckysori
Copy link
Contributor Author

luckysori commented Dec 22, 2023

@luckysori Is there anything left on this PR or can we get it merged?

I don't think there's anything left, but I wasn't sure if we were all happy with the tradeoff. What do you think, @bonomat?

@bonomat
Copy link
Contributor

bonomat commented Dec 29, 2023

@luckysori Is there anything left on this PR or can we get it merged?

I don't think there's anything left, but I wasn't sure if we were all happy with the tradeoff. What do you think, @bonomat?

If I understand it correctly than we will simply keep dust HTLCs in the holding cells until they are resolved. If we were to force close in the mean time, then this amount will go to the miners.
Meaning, if we have a fee of 200 sats/vbyte we might lose up to 5m sats to miners?

@luckysori
Copy link
Contributor Author

Not needed anymore.

@luckysori luckysori closed this Jan 8, 2024
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

Successfully merging this pull request may close these issues.

3 participants