LSPS1: Service refactor - Part 1 #3864
Conversation
|
👋 Hi! This PR is now in draft status. |
- Remove dead / unnecesary functions, dependencies, etc. - Add the first LSPS1 integration test. There are no breaking API changes, ldk-node should function as usual
martinsaposnic
force-pushed
the
lsps1-service-refactor-first-pass
branch
from
1febb34 to
27be2eb
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3864 +/- ##
==========================================
- Coverage 89.75% 89.73% -0.03%
==========================================
Files 164 164
Lines 132834 132832 -2
Branches 132834 132832 -2
==========================================
- Hits 119229 119192 -37
- Misses 10926 10951 +25
- Partials 2679 2689 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Thanks! Might be good to hold work on this for a bit until we concluded the larger discussions around lightning-liquidity design.
So far mostly looks good though, but please keep the generics/references to ChainSource as we added them on purpose since they'll be needed when tracking onchain payments. AChannelManager might be more debatable, but probably can't hurt to leave the generics in place until we're certain we don't need them.
| { | ||
| /// Constructs a `LSPS1ServiceHandler`. | ||
| pub(crate) fn new( | ||
| entropy_source: ES, pending_messages: Arc<MessageQueue>, pending_events: Arc<EventQueue>, | ||
| channel_manager: CM, chain_source: Option<C>, config: LSPS1ServiceConfig, |
There was a problem hiding this comment.
Please leave these object references here, as we'll need them for when we implement tracking of on-chain payments.
|
👋 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. |
Starting to work on this issue #3480
This is the first of (I think) many PRs with multiple refactors on LSPS1/service
In this PR:
There are no breaking API changes, ldk-node should keep functioning as usual