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

Check for spendable_msat on trampoline pay #585

Merged
merged 2 commits into from
Mar 6, 2025

Conversation

nepet
Copy link
Collaborator

@nepet nepet commented Mar 6, 2025

We had some incidents where a trampoline payment created 0msat which lead to failed payments that actually did not fail. In order to avoid this we filter out channels that have a spendable_msat value of 0 or below the minimum_htlc_out_msat.

@nepet nepet requested a review from JssDWt March 6, 2025 12:38
) -> Result<Vec<(cln_rpc::primitives::ShortChannelId, u64)>> {
let mut acc = 0;
let mut choosen = vec![];
while let Some(channel) = channels.pop() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Simpler, but let's keep this as a cleanup, merge this one so we address the issue asap:

for c in channels.iter().filter(|c| c.spendable_msat.unwrap_or_default() > MIN_HTLC_AMOUNT) {
...
}

And we can also add a second .filter() that filters out the ones that have spendable_msat below min_htlc_out_msat, no need to mix those conditions either.

nepet added 2 commits March 6, 2025 13:57
We need to filter out channels that are drained and can not end out an
htlc with an actual amount greater than 0.

Signed-off-by: Peter Neuroth <[email protected]>
@cdecker cdecker force-pushed the 2510-fix-minimum-msat-amount-on-trampoline branch from 16d46a7 to 9477d42 Compare March 6, 2025 12:57
@cdecker cdecker enabled auto-merge (rebase) March 6, 2025 12:57
Comment on lines +367 to +369
let rest = amount_msat - acc;
choosen.push((channel.short_channel_id, rest));
acc += rest;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's still a case where rest does not add up to the minimum htlc amount. Do you know a good way to handle that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, we may want to check and skip those channels as well. 🤔
I'll add a follow up.

@cdecker cdecker merged commit bada31b into main Mar 6, 2025
11 checks passed
@cdecker cdecker deleted the 2510-fix-minimum-msat-amount-on-trampoline branch March 6, 2025 13:09
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