Skip to content

LSPS5 -> more follow ups #3972

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

Merged
merged 2 commits into from
Jul 29, 2025

Conversation

martinsaposnic
Copy link
Contributor

Addressing comments #3662 (comment), #3662 (comment) and lightning/blips#55 (comment)

Changes:

  • lightning-liquidity README is updated
  • We stop logging at the Error / Info level when the client / manager is dealing with unknown peers, unknown request IDs, missing handlers, etc

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jul 29, 2025

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@martinsaposnic martinsaposnic mentioned this pull request Jul 29, 2025
18 tasks
@tnull tnull requested review from tnull and removed request for valentinewallace July 29, 2025 14:08
Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

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

LGTM

I think at some point we need to more fundamentally rethink our logging scheme (e.g., not sure if we should start logging at ERROR for some of the more unlikely/severe cases). but this is a clear improvement for now.

@ldk-reviews-bot
Copy link

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

@tnull tnull merged commit 8fd7cbf into lightningdevkit:main Jul 29, 2025
23 checks passed
Copy link

codecov bot commented Jul 29, 2025

Codecov Report

❌ Patch coverage is 22.22222% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.93%. Comparing base (bb48bbc) to head (7782e3d).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
lightning-liquidity/src/manager.rs 0.00% 7 Missing ⚠️
lightning-liquidity/src/lsps2/client.rs 25.00% 6 Missing ⚠️
lightning-liquidity/src/lsps5/client.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3972      +/-   ##
==========================================
- Coverage   88.93%   88.93%   -0.01%     
==========================================
  Files         174      174              
  Lines      123875   123880       +5     
  Branches   123875   123880       +5     
==========================================
+ Hits       110169   110171       +2     
- Misses      11253    11259       +6     
+ Partials     2453     2450       -3     
Flag Coverage Δ
fuzzing 22.62% <0.00%> (+0.44%) ⬆️
tests 88.76% <22.22%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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