Implement PartialEq for ChannelMonitor #1986
Conversation
It turns out `#[derive(PartialEq)]` will automatically bound the `PartialEq` implementation by any bounds on the struct also being `PartialEq`. This means to use an auto-derived `ChannelMonitor` `PartialEq` the `EcdsaSigner` used must also be `PartialEq`, but for the use-cases we have today for a `ChannelMonitor` `PartialEq` it doesn't really matter - we use it internally in tests and downstream users wanted similar test-only usage. Fixes lightningdevkit#1912.
`ChannelMonitor` indirectly already has a context - the `OnchainTxHandler` has one. This makes it trivial to remove the existing one, so we do so for a free memory usage reduction.
Codecov ReportBase: 90.83% // Head: 91.30% // Increases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #1986 +/- ##
==========================================
+ Coverage 90.83% 91.30% +0.46%
==========================================
Files 99 99
Lines 51761 54507 +2746
Branches 51761 54507 +2746
==========================================
+ Hits 47018 49768 +2750
+ Misses 4743 4739 -4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -58,6 +58,12 @@ pub struct EnforcingSigner { | |||
| pub disable_revocation_policy_check: bool, | |||
| } | |||
|
|
|||
| impl PartialEq for EnforcingSigner { | |||
| fn eq(&self, o: &Self) -> bool { | |||
| Arc::ptr_eq(&self.state, &o.state) | |||
There was a problem hiding this comment.
It's interesting when I checked out this function and went down a bit of a rabbit hole and found out that currently for dyn Trait raw pointers, Rust also checks for equality of vtables which has caused what most would naturally consider false negatives as it was unexpected from the docs: rust-lang/rust#106447
We're at least good for EnforcementState structs though.
There was a problem hiding this comment.
Huh! Yea, that's surprising. I mean indeed doesn't impact us here (it snot even a dyn trait), but still.
It turns out
#[derive(PartialEq)]will automatically bound thePartialEqimplementation by any bounds on the struct also beingPartialEq, which I only found out today. This means to use an auto-derivedChannelMonitorPartialEqtheEcdsaSignerused must also bePartialEq, butfor the use-cases we have today for a
ChannelMonitorPartialEqit doesn't really matter - we use it internally in tests and
downstream users wanted similar test-only usage.
Fixes #1912.