Simplify send htlc #3946
Simplify send htlc #3946
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3946 +/- ##
==========================================
- Coverage 89.01% 88.99% -0.02%
==========================================
Files 167 168 +1
Lines 121806 121859 +53
Branches 121806 121859 +53
==========================================
+ Hits 108421 108453 +32
- Misses 10975 10992 +17
- Partials 2410 2414 +4
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:
|
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Simple enough, still some comments though.
While we're here, can we remove the spurious
// Send stuff to our remote peers:
on line 10580?
lightning/src/ln/channel.rs
Outdated
| ) { | ||
| Ok(update_add_msg_opt) => { | ||
| Ok(can_add_htlc) => { | ||
| // `send_htlc` only returns `Ok(None)`, when an update goes into |
There was a problem hiding this comment.
Seems this comment is outdated now if we return a bool.
lightning/src/ln/channel.rs
Outdated
| logger, | ||
| ); | ||
| // All [`LocalHTLCFailureReason`] errors are temporary, so they are [`ChannelError::Ignore`]. | ||
| match send_res.map_err(|(_, msg)| ChannelError::Ignore(msg))? { |
There was a problem hiding this comment.
Can we extract the bool into a variable and make this a simple if rather than a match now?
|
👋 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
simplify-send-htlc
branch
from
93f5e01 to
5e3af3b
Compare
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
LGTM
Simple enough, will land this once CI lets me.
3 participants
Remove message parameter that is used nowhere.