Add monitor_updating_paused logging#4298
Add monitor_updating_paused logging#4298valentinewallace merged 4 commits intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @valentinewallace as a reviewer! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4298 +/- ##
==========================================
- Coverage 89.38% 86.59% -2.79%
==========================================
Files 180 158 -22
Lines 139834 102093 -37741
Branches 139834 102093 -37741
==========================================
- Hits 124985 88404 -36581
+ Misses 12262 11280 -982
+ Partials 2587 2409 -178
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:
|
lightning/src/ln/channel.rs
Outdated
| &mut self, resend_raa: bool, resend_commitment: bool, resend_channel_ready: bool, | ||
| mut pending_forwards: Vec<(PendingHTLCInfo, u64)>, | ||
| fn monitor_updating_paused<L: Deref>( | ||
| &mut self, logger: &L, resend_raa: bool, resend_commitment: bool, |
There was a problem hiding this comment.
nit: I think we usually have the logger as (one of) the last arguments, which makes sense to me because it's kinda the least important
lightning/src/ln/channel.rs
Outdated
| None => return Err((self, ChannelError::close("Received funding_signed before our first commitment point was available".to_owned()))), | ||
| }; | ||
| self.context.assert_no_commitment_advancement(holder_commitment_point.next_transaction_number(), "funding_signed"); | ||
| let mut holder_commitment_point = |
There was a problem hiding this comment.
Here and elsewhere, this seems more readable:
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 598a9166f..81c5c4c07 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -13818,17 +13818,13 @@ where
ChannelError::close("Received funding_signed in strange state!".to_owned()),
));
}
- let mut holder_commitment_point =
- match self.unfunded_context.holder_commitment_point {
- Some(point) => point,
- None => return Err((
- self,
- ChannelError::close(
- "Received funding_signed before our first commitment point was available"
- .to_owned(),
- ),
- )),
- };
+ let mut holder_commitment_point = match self.unfunded_context.holder_commitment_point {
+ Some(point) => point,
+ None => {
+ let err = "Received funding_signed before our first commitment point was available";
+ return Err((self, ChannelError::close(err.to_owned())));
+ },
+ };
self.context.assert_no_commitment_advancement(
holder_commitment_point.next_transaction_number(),
"funding_signed",There was a problem hiding this comment.
Suggestion applied
|
👋 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. |
joostjager
force-pushed
the
add-monitor-logging
branch
4 times, most recently
from
7c7dc32 to
ed38fe3
Compare
valentinewallace
left a comment
valentinewallace
left a comment
There was a problem hiding this comment.
Should be landable with 1 review, feel free to squash!
joostjager
force-pushed
the
add-monitor-logging
branch
2 times, most recently
from
13a910b to
4c043e2
Compare
|
Fmt squashed |
| &self.signer_provider, | ||
| their_features, | ||
| target_feerate_sats_per_1000_weight, | ||
| override_shutdown_script, | ||
| &self.logger, |
There was a problem hiding this comment.
This change and the above are in the wrong commit
Using extend is slightly cleaner because it doesn't require mut on the parameters.
Extract error message strings into local variables before constructing ChannelError return tuples, reducing nesting and improving readability.
joostjager
force-pushed
the
add-monitor-logging
branch
from
4c043e2 to
51b5ef7
Compare
| pub fn shutdown( | ||
| &mut self, signer_provider: &SP, their_features: &InitFeatures, msg: &msgs::Shutdown, | ||
| pub fn shutdown<L: Deref>( | ||
| &mut self, logger: &L, signer_provider: &SP, their_features: &InitFeatures, |
There was a problem hiding this comment.
Logger is still the first argument here but not gonna block
3 participants
Make the logger better available