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

More fine grained support for fee diff errors #2815

Merged
merged 2 commits into from Feb 27, 2024
Merged

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Feb 2, 2024

When there is a mismatch between the feerate of a channel and the feerate we get from our estimator, we may want to force-close because that could be exploited by our peer to steal HTLCs. But that's only the case if the feerate is too low, not if it's too high. We previously force-closed in both cases, whereas we only need to do it when the feerate is too low.

This should avoid some unnecessary force-close that we've observed and are due to buggy fee estimators (fee estimation is hard!), or to peers who simply do some smoothing and slightly delay lowering the feerate of our channels.

@t-bast t-bast requested a review from pm47 February 2, 2024 08:20
@codecov-commenter
Copy link

codecov-commenter commented Feb 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.96%. Comparing base (e66e6d2) to head (5c9a5e8).
Report is 12 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    #2815      +/-   ##
==========================================
+ Coverage   85.86%   85.96%   +0.09%     
==========================================
  Files         216      219       +3     
  Lines       18228    18453     +225     
  Branches      772      795      +23     
==========================================
+ Hits        15652    15863     +211     
- Misses       2576     2590      +14     
Files Coverage Δ
...r/acinq/eclair/blockchain/fee/OnChainFeeConf.scala 95.65% <100.00%> (+0.65%) ⬆️
...in/scala/fr/acinq/eclair/channel/Commitments.scala 96.83% <100.00%> (+0.01%) ⬆️
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala 85.49% <100.00%> (-0.31%) ⬇️

... and 37 files with indirect coverage changes

@t-bast t-bast requested a review from sstone February 2, 2024 11:04
When there is a mismatch between the feerate of a channel and the
feerate we get from our estimator, we may want to force-close because
that could be exploited by our peer to steal HTLCs. But that's only the
case if the feerate is too low, not if it's too high. We previously
force-closed in both cases, whereas we only need to do it when the
feerate is too low.

This should avoid some unnecessary force-close that we've observed and
are due to buggy fee estimators (fee estimation is hard!), or to peers
who simply do some smoothing and slightly delay lowering the feerate of
our channels.
sstone
sstone previously approved these changes Feb 8, 2024
@t-bast t-bast merged commit 36a3c88 into master Feb 27, 2024
1 check passed
@t-bast t-bast deleted the feerate-mismatch-warning branch February 27, 2024 12:52
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.

None yet

4 participants