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

Use remote funding when setting max_htlc_value_in_flight #2980

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Jan 15, 2025

When using dual-funding, both peers may contribute to the funding amount and it usually cannot be known ahead of time how much the remote peer will contribute, which usually leads to underestimating the channel capacity and thus using a lower max_htlc_value_in_flight than what should be used.

However, when we use liquidity ads, we will:

  • contribute to the funding transaction if we're not the opener
  • cancel the funding attempt if we're the opener and our peers doesn't contribute at least the amount we requested

So in that case, we can use a better estimate of the channel capacity when computing our max_htlc_value_in_flight.

@t-bast t-bast requested a review from remyers January 15, 2025 14:51
When using dual-funding, both peers may contribute to the funding amount
and it usually cannot be known ahead of time how much the remote peer
will contribute, which usually leads to underestimating the channel
capacity and thus using a lower `max_htlc_value_in_flight` than what
should be used.

However, when we use liquidity ads, we will:

- contribute to the funding transaction if we're not the opener
- cancel the funding attempt if we're the opener and our peers doesn't
  contribute at least the amount we requested

So in that case, we can use a better estimate of the channel capacity
when computing our `max_htlc_value_in_flight`.
@t-bast t-bast force-pushed the liquidity-ads-max-in-flight branch from 6574090 to a31fd1b Compare January 30, 2025 14:34
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.97%. Comparing base (12df4ce) to head (a31fd1b).
Report is 4 commits behind head on master.

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2980      +/-   ##
==========================================
- Coverage   86.02%   85.97%   -0.05%     
==========================================
  Files         227      228       +1     
  Lines       20371    20442      +71     
  Branches      815      837      +22     
==========================================
+ Hits        17524    17575      +51     
- Misses       2847     2867      +20     
Files with missing lines Coverage Δ
...la/fr/acinq/eclair/io/OpenChannelInterceptor.scala 91.48% <100.00%> (+0.24%) ⬆️

... and 11 files with indirect coverage changes

@t-bast
Copy link
Member Author

t-bast commented Feb 7, 2025

@remyers ping 🙏

Copy link
Contributor

@remyers remyers left a comment

Choose a reason for hiding this comment

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

LGTM!

Changes look good and tested. My only question is why this value can't be updated later.

I think the answer is that this value is only exchanged once when the channel is opened (or spliced) and there just isn't a mechanism to update it short of a splice. It also limits the htlc_maximum_msat in channel_update and channel_announcement so even if it were updated after the open there would be a period of time after the initial announcement when the full capacity couldn't be used.

Am I understanding this correctly?

@t-bast
Copy link
Member Author

t-bast commented Feb 7, 2025

I think the answer is that this value is only exchanged once when the channel is opened (or spliced) and there just isn't a mechanism to update it short of a splice.

Exactly, this currently cannot be updated in the protocol. That's one of the goal of dynamic commitments.

@t-bast t-bast merged commit a8787ab into master Feb 7, 2025
1 check passed
@t-bast t-bast deleted the liquidity-ads-max-in-flight branch February 7, 2025 13:25
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