-
Notifications
You must be signed in to change notification settings - Fork 417
LSPS5 -> More follow ups #3987
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
LSPS5 -> More follow ups #3987
Conversation
👋 Thanks for assigning @tnull as a reviewer! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3987 +/- ##
==========================================
- Coverage 88.98% 88.93% -0.06%
==========================================
Files 174 174
Lines 124222 124538 +316
Branches 124222 124538 +316
==========================================
+ Hits 110543 110760 +217
- Misses 11201 11283 +82
- Partials 2478 2495 +17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a rebase.
9d975ae
to
669a065
Compare
669a065
to
681656f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we can remove these clones here too for consistency ?
diff --git a/lightning-liquidity/src/lsps2/msgs.rs b/lightning-liquidity/src/lsps2/msgs.rs
index 84875d4ab..22a0d8116 100644
--- a/lightning-liquidity/src/lsps2/msgs.rs
+++ b/lightning-liquidity/src/lsps2/msgs.rs
@@ -72,7 +72,7 @@ impl LSPS2RawOpeningFeeParams {
LSPS2OpeningFeeParams {
min_fee_msat: self.min_fee_msat,
proportional: self.proportional,
- valid_until: self.valid_until.clone(),
+ valid_until: self.valid_until,
min_lifetime: self.min_lifetime,
max_client_to_self_delay: self.max_client_to_self_delay,
min_payment_size_msat: self.min_payment_size_msat,
@@ -235,7 +235,7 @@ mod tests {
let raw = LSPS2RawOpeningFeeParams {
min_fee_msat,
proportional,
- valid_until: valid_until.clone().into(),
+ valid_until,
min_lifetime,
max_client_to_self_delay,
min_payment_size_msat,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, mod the pending comments.
681656f
to
552d8e4
Compare
thanks for the review @tankyleo ! I just force pushed this change: diff --git a/lightning-liquidity/src/lsps2/msgs.rs b/lightning-liquidity/src/lsps2/msgs.rs
index 84875d4ab..8fb9536b6 100644
--- a/lightning-liquidity/src/lsps2/msgs.rs
+++ b/lightning-liquidity/src/lsps2/msgs.rs
@@ -72,7 +72,7 @@ impl LSPS2RawOpeningFeeParams {
LSPS2OpeningFeeParams {
min_fee_msat: self.min_fee_msat,
proportional: self.proportional,
- valid_until: self.valid_until.clone(),
+ valid_until: self.valid_until,
min_lifetime: self.min_lifetime,
max_client_to_self_delay: self.max_client_to_self_delay,
min_payment_size_msat: self.min_payment_size_msat,
@@ -235,7 +235,7 @@ mod tests {
let raw = LSPS2RawOpeningFeeParams {
min_fee_msat,
proportional,
- valid_until: valid_until.clone().into(),
+ valid_until: valid_until.into(),
min_lifetime,
max_client_to_self_delay,
min_payment_size_msat, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Simple enough, landing this.
Addressing LSPS5 follow ups, specifically these comments #3662 and #3662
This is finished but will mark this as draft because it depends on #3969. Once that's merged I will rebase this PR and undraft it.Done ✅Commits to review here are 27e5350 and 9d975ae