Make ChannelMonitor round-trip tests more robust

During testsing, we check that a `ChannelMonitor` will round-trip
through serialization exactly. However, we recently added a fix to
change a value in `PackageTemplate` on reload to fix some issues in
the field in 0.1. This can cause the round-trip tests to fail as a
field is modified during read.

We fix it here by simply exempting the field from the equality test
in the condition where it would be updated on read.

We also make the `ChannelMonitor` `PartialEq` trait implementation
non-public as weird workarounds like this make clear that such a
comparison is a britle API at best.

Backport of a8ec966

Resolved `use` and `rustfmt` conflicts in:
 * lightning/src/chain/channelmonitor.rs

In the upstream version of this commit, the `PartialEq`
implementation for `ChannelMonitor` was made test-only however to
avoid breaking a public API we do not do so here. However, the
changes to `PartialEq` for `PackageTemplate` are `test`-only, so
it shouldn't result in any behavioral change (not that the marginal
`PartialEq` changes are likely to impact downstream crates in any
case).

Author: TheBlueMatt

lightning/src/chain/channelmonitor.rs

@@ -59,7 +59,7 @@ use crate::prelude::*;
 use core::{cmp, mem};
 use crate::io::{self, Error};
 use core::ops::Deref;
-use crate::sync::{Mutex, LockTestExt};
+use crate::sync::Mutex;
 
 /// An update generated by the underlying channel itself which contains some new information the
 /// [`ChannelMonitor`] should be made aware of.
@@ -1040,12 +1040,21 @@ pub type TransactionOutputs = (Txid, Vec<(u32, TxOut)>);
 
 impl<Signer: EcdsaChannelSigner> PartialEq for ChannelMonitor<Signer> where Signer: PartialEq {
 	fn eq(&self, other: &Self) -> bool {
+		use crate::sync::LockTestExt;
 		// We need some kind of total lockorder. Absent a better idea, we sort by position in
 		// memory and take locks in that order (assuming that we can't move within memory while a
 		// lock is held).
 		let ord = ((self as *const _) as usize) < ((other as *const _) as usize);
-		let a = if ord { self.inner.unsafe_well_ordered_double_lock_self() } else { other.inner.unsafe_well_ordered_double_lock_self() };
-		let b = if ord { other.inner.unsafe_well_ordered_double_lock_self() } else { self.inner.unsafe_well_ordered_double_lock_self() };
+		let a = if ord {
+			self.inner.unsafe_well_ordered_double_lock_self()
+		} else {
+			other.inner.unsafe_well_ordered_double_lock_self()
+		};
+		let b = if ord {
+			other.inner.unsafe_well_ordered_double_lock_self()
+		} else {
+			self.inner.unsafe_well_ordered_double_lock_self()
+		};
 		a.eq(&b)
 	}
 }

lightning/src/chain/package.rs

@@ -820,7 +820,7 @@ enum PackageMalleability {
 ///
 /// As packages are time-sensitive, we fee-bump and rebroadcast them at scheduled intervals.
 /// Failing to confirm a package translate as a loss of funds for the user.
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, Eq)]
 pub struct PackageTemplate {
 	// List of onchain outputs and solving data to generate satisfying witnesses.
 	inputs: Vec<(BitcoinOutPoint, PackageSolvingData)>,
@@ -849,6 +849,50 @@ pub struct PackageTemplate {
 	height_timer: u32,
 }
 
+impl PartialEq for PackageTemplate {
+	fn eq(&self, o: &Self) -> bool {
+		if self.inputs != o.inputs
+			|| self.malleability != o.malleability
+			|| self.feerate_previous != o.feerate_previous
+			|| self.height_timer != o.height_timer
+		{
+			return false;
+		}
+		#[cfg(test)]
+		{
+			// In some cases we may reset `counterparty_spendable_height` to zero on reload, which
+			// can cause our test assertions that ChannelMonitors round-trip exactly to trip. Here
+			// we allow exactly the same case as we tweak in the `PackageTemplate` `Readable`
+			// implementation.
+			if self.counterparty_spendable_height == 0 {
+				for (_, input) in self.inputs.iter() {
+					if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput {
+						htlc, ..
+					}) = input
+					{
+						if !htlc.offered && htlc.cltv_expiry != 0 {
+							return true;
+						}
+					}
+				}
+			}
+			if o.counterparty_spendable_height == 0 {
+				for (_, input) in o.inputs.iter() {
+					if let PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput {
+						htlc, ..
+					}) = input
+					{
+						if !htlc.offered && htlc.cltv_expiry != 0 {
+							return true;
+						}
+					}
+				}
+			}
+		}
+		self.counterparty_spendable_height == o.counterparty_spendable_height
+	}
+}
+
 impl PackageTemplate {
 	pub(crate) fn can_merge_with(&self, other: &PackageTemplate, cur_height: u32) -> bool {
 		match (self.malleability, other.malleability) {