Clean channelmonitor.rs code #3429
Conversation
* Replace deprecated `u64::max_value()` * Use `Option::is_some()` * Use `Iterator::any()` instead of `Iterator::find()`
Seems that it is not related to the changes. Or do you mean something specific? |
| @@ -104,7 +104,7 @@ pub struct ChannelMonitorUpdate { | |||
|
|
|||
| /// LDK prior to 0.1 used this constant as the [`ChannelMonitorUpdate::update_id`] for any | |||
| /// [`ChannelMonitorUpdate`]s which were generated after the channel was closed. | |||
| const LEGACY_CLOSED_CHANNEL_UPDATE_ID: u64 = core::u64::MAX; | |||
There was a problem hiding this comment.
This is gonna conflict with #3413.
There was a problem hiding this comment.
What do you mean here? I only dropped core:: prefix, you are not touching this line in your PR.
There was a problem hiding this comment.
Oh, sorry, I had my PRs confused. The one I was thinking of already landed lol
| @@ -794,10 +794,10 @@ struct IrrevocablyResolvedHTLC { | |||
| // In LDK versions prior to 0.0.111 commitment_tx_output_idx was not Option-al and | |||
| // IrrevocablyResolvedHTLC objects only existed for non-dust HTLCs. This was a bug, but to maintain | |||
| // backwards compatibility we must ensure we always write out a commitment_tx_output_idx field, | |||
| // using `u32::max_value()` as a sentinal to indicate the HTLC was dust. | |||
| // using `u32::MAX` as a sentinal to indicate the HTLC was dust. | |||
There was a problem hiding this comment.
while you're here can you [] link?
There was a problem hiding this comment.
Done. Also made it a Rust comment (prefixed with ///).
| } | ||
| } | ||
| let (onchain_events_reaching_threshold_conf, onchain_events_awaiting_threshold_conf): (Vec<_>, Vec<_>) = | ||
| self.onchain_events_awaiting_threshold_conf.drain(..).partition( |
There was a problem hiding this comment.
This adds an assumption that onchain_events_awaiting_threshold_conf is sorted by when events will reach their confirmation threshold. That may be the case in practice (its not entirely clear to me from the code), but I'm not super happy with adding that assumption here.
There was a problem hiding this comment.
Hm, how? partition() does not make such an assumption:
partition()returns a pair, all of the elements for which it returned true, and all of the elements for which it returned false.
https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.partition
There was a problem hiding this comment.
Ah, that's a terrible name for that function lol.
u64::max_value()Option::is_some()Iterator::any()instead ofIterator::find()