OutputSweeper: Delay pruning until monitors have likely been archived

Previously, we would prune tracked descriptors once we see a spend hit
`ANTI_REORG_DELAY = 6` confirmations. However, this could lead to a
scenario where lingering `ChannelMonitor`s waiting to be archived would
still regenerate and replay `Event::SpendableOutput`s, i.e., we would
re-add the same (now unspendable due to be actually being already spent)
outputs again after having intially pruned them.

Here, we therefore keep the tracked descriptors around for longer, in
particular at least `ARCHIVAL_DELAY_BLOCKS + ANTI_REORG_DELAY = 4038`
confirmations, at which point we assume the lingering monitors to have
been likely archived, and it's 'safe' for us to also forget about the
descriptors.

Author: tnull

lightning-background-processor/src/lib.rs

@@ -1099,7 +1099,7 @@ mod tests {
 		SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
 	};
 	use lightning::util::ser::Writeable;
-	use lightning::util::sweep::{OutputSpendStatus, OutputSweeper};
+	use lightning::util::sweep::{OutputSpendStatus, OutputSweeper, PRUNE_DELAY_BLOCKS};
 	use lightning::util::test_utils;
 	use lightning::{get_event, get_event_msg};
 	use lightning_persister::fs_store::FilesystemStore;
@@ -2282,8 +2282,8 @@ mod tests {
 		}
 
 		// Check we stop tracking the spendable outputs when one of the txs reaches
-		// ANTI_REORG_DELAY confirmations.
-		confirm_transaction_depth(&mut nodes[0], &sweep_tx_0, ANTI_REORG_DELAY);
+		// PRUNE_DELAY_BLOCKS confirmations.
+		confirm_transaction_depth(&mut nodes[0], &sweep_tx_0, PRUNE_DELAY_BLOCKS);
 		assert_eq!(nodes[0].sweeper.tracked_spendable_outputs().len(), 0);
 
 		if !std::thread::panicking() {

lightning/src/util/sweep.rs

@@ -9,7 +9,7 @@
 //! sweeping them.
 
 use crate::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
-use crate::chain::channelmonitor::ANTI_REORG_DELAY;
+use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS};
 use crate::chain::{self, BestBlock, Confirm, Filter, Listen, WatchedOutput};
 use crate::io;
 use crate::ln::msgs::DecodeError;
@@ -32,6 +32,9 @@ use bitcoin::{BlockHash, Transaction, Txid};
 
 use core::ops::Deref;
 
+/// The number of blocks we wait before we prune the tracked spendable outputs.
+pub const PRUNE_DELAY_BLOCKS: u32 = ARCHIVAL_DELAY_BLOCKS + ANTI_REORG_DELAY;
+
 /// The state of a spendable output currently tracked by an [`OutputSweeper`].
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct TrackedSpendableOutput {
@@ -101,7 +104,11 @@ pub enum OutputSpendStatus {
 		latest_spending_tx: Transaction,
 	},
 	/// A transaction spending the output has been confirmed on-chain but will be tracked until it
-	/// reaches [`ANTI_REORG_DELAY`] confirmations.
+	/// reaches at least [`PRUNE_DELAY_BLOCKS`] confirmations to ensure [`Event::SpendableOutputs`]
+	/// stemming from lingering [`ChannelMonitor`]s can safely be replayed.
+	///
+	/// [`Event::SpendableOutputs`]: crate::events::Event::SpendableOutputs
+	/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
 	PendingThresholdConfirmations {
 		/// The hash of the chain tip when we first broadcast a transaction spending this output.
 		first_broadcast_hash: BlockHash,
@@ -524,7 +531,9 @@ where
 		// Prune all outputs that have sufficient depth by now.
 		sweeper_state.outputs.retain(|o| {
 			if let Some(confirmation_height) = o.status.confirmation_height() {
-				if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1 {
+				// We wait at least `PRUNE_DELAY_BLOCKS` as before that
+				// `Event::SpendableOutputs` from lingering monitors might get replayed.
+				if cur_height >= confirmation_height + PRUNE_DELAY_BLOCKS - 1 {
 					log_debug!(self.logger,
 						"Pruning swept output as sufficiently confirmed via spend in transaction {:?}. Pruned descriptor: {:?}",
 						o.status.latest_spending_tx().map(|t| t.compute_txid()), o.descriptor