Pass supported_protocols to LSPS0 (fix ListProtocols support) #3785
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3785 +/- ##
==========================================
- Coverage 89.79% 89.77% -0.02%
==========================================
Files 159 159
Lines 128672 128672
Branches 128672 128672
==========================================
- Hits 115540 115519 -21
- Misses 10463 10468 +5
- Partials 2669 2685 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| protocols, | ||
| }) => { | ||
| assert_eq!(counterparty_node_id, client_node_id); | ||
| assert_eq!(protocols, vec![2]); |
There was a problem hiding this comment.
Ah, note that you'll account for cfg(lsps1_service) here, too.
|
👋 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. |
|
✅ Added second reviewer: @wpaulino |
martinsaposnic
force-pushed
the
lsps0-supported-protocols
branch
from
edc4688 to
88c2ea0
Compare
| let lsps1_service_config = LSPS1ServiceConfig { supported_options: None, token: None }; | ||
| let service_config = LiquidityServiceConfig { | ||
| #[cfg(lsps1_service)] | ||
| lsps1_service_config, |
There was a problem hiding this comment.
This is also still slightly off, having CI fail:
error[E0308]: mismatched types
--> lightning-liquidity/tests/lsps0_integration_tests.rs:27:3
|
27 | lsps1_service_config,
| ^^^^^^^^^^^^^^^^^^^^ expected `Option<LSPS1ServiceConfig>`, found `LSPS1ServiceConfig`
|
= note: expected enum `std::option::Option<LSPS1ServiceConfig>`
found struct `LSPS1ServiceConfig`
help: try wrapping the expression in `Some`
|
27 | lsps1_service_config: Some(lsps1_service_config),
| +++++++++++++++++++++++++++ +
For more information about this error, try `rustc --explain E0308`.
error: could not compile `lightning-liquidity` (test "lsps0_integration_tests") due to 1 previous error
There was a problem hiding this comment.
yeah, sorry. just fixed it and amended the commit 822fa89#diff-2af4dd28828897ca6747fecb526fcec3c755151c242cb49fad2a2a52f4ce05cbR26-R27
martinsaposnic
force-pushed
the
lsps0-supported-protocols
branch
from
88c2ea0 to
822fa89
Compare
This PR fixes an issue where LSPS0 was always receiving an empty vector instead of the actual supported_protocols list. Now, LSPS0 gets the correct supported_protocols, so it can properly handle ListProtocols requests.
It also adds an integration test to verify that LSPS0::ListProtocols works as expected.
(Not sure if the empty vector was intentional or just overlooked? cc @tnull)