rust-lightning icon indicating copy to clipboard operation
rust-lightning copied to clipboard

Limit `htlc_maximum_msat` value based on announcement status

Open Sharmalm opened this issue 1 year ago • 4 comments

This or solves #2984

Sharmalm avatar Apr 30 '24 05:04 Sharmalm

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.37%. Comparing base (ac9a2c8) to head (8c5e087). Report is 76 commits behind head on main.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3030      +/-   ##
==========================================
+ Coverage   89.42%   90.37%   +0.94%     
==========================================
  Files         117      118       +1     
  Lines       96290   106036    +9746     
  Branches    96290   106036    +9746     
==========================================
+ Hits        86109    95829    +9720     
- Misses       7962     8032      +70     
+ Partials     2219     2175      -44     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Apr 30 '24 05:04 codecov-commenter

We shouldn't be changing the announced limit, because the announced limit is just reflective of what we can actually send. Instead, we need to change the in-flight limit that we send to our peer during channel opening.

TheBlueMatt avatar May 14 '24 14:05 TheBlueMatt

We shouldn't be changing the announced limit, because the announced limit is just reflective of what we can actually send. Instead, we need to change the in-flight limit that we send to our peer during channel opening.

Huh, I'm confused: in respect to #2851 we discussed that these are two separate issues, i.e., #2851 and #2984?

If we don't change the announced limit, LDK will actually prefer non-LDK nodes when routing as we use the announced limit for our anti_probing_penalty, no?

tnull avatar May 21 '24 10:05 tnull

Right, okay, so the current patch here removes the 90% limit for private channels, which I guess is fine (but I think we always ignore anyway when we're the other side?), but doesn't change anything re: the anti-probing penalty. For that we need to change the public announced limit, which maybe was the intention of this PR?

TheBlueMatt avatar May 21 '24 14:05 TheBlueMatt