Improve logging around redundant force close monitor updates
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index 82fae49f7ce5374b33f80ec9b556d99f1dcb8eb4..e3be6bff2fa88b4191d9b3fdc8bccf665d4ac580 100644 (file)
@@ -42,7 +42,7 @@ use crate::chain;
 use crate::chain::{BestBlock, WatchedOutput};
 use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator};
 use crate::chain::transaction::{OutPoint, TransactionData};
-use crate::chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, WriteableEcdsaChannelSigner, SignerProvider, EntropySource};
+use crate::sign::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, WriteableEcdsaChannelSigner, SignerProvider, EntropySource};
 #[cfg(anchors)]
 use crate::chain::onchaintx::ClaimEvent;
 use crate::chain::onchaintx::OnchainTxHandler;
@@ -606,6 +606,10 @@ pub enum Balance {
                /// The height at which the counterparty may be able to claim the balance if we have not
                /// done so.
                timeout_height: u32,
+               /// The payment hash that locks this HTLC.
+               payment_hash: PaymentHash,
+               /// The preimage that can be used to claim this HTLC.
+               payment_preimage: PaymentPreimage,
        },
        /// HTLCs which we sent to our counterparty which are claimable after a timeout (less on-chain
        /// fees) if the counterparty does not know the preimage for the HTLCs. These are somewhat
@@ -617,6 +621,8 @@ pub enum Balance {
                /// The height at which we will be able to claim the balance if our counterparty has not
                /// done so.
                claimable_height: u32,
+               /// The payment hash whose preimage our counterparty needs to claim this HTLC.
+               payment_hash: PaymentHash,
        },
        /// HTLCs which we received from our counterparty which are claimable with a preimage which we
        /// do not currently have. This will only be claimable if we receive the preimage from the node
@@ -628,6 +634,8 @@ pub enum Balance {
                /// The height at which our counterparty will be able to claim the balance if we have not
                /// yet received the preimage and claimed it ourselves.
                expiry_height: u32,
+               /// The payment hash whose preimage we need to claim this HTLC.
+               payment_hash: PaymentHash,
        },
        /// The channel has been closed, and our counterparty broadcasted a revoked commitment
        /// transaction.
@@ -1467,6 +1475,27 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
        pub fn current_best_block(&self) -> BestBlock {
                self.inner.lock().unwrap().best_block.clone()
        }
+
+       /// Triggers rebroadcasts/fee-bumps of pending claims from a force-closed channel. This is
+       /// crucial in preventing certain classes of pinning attacks, detecting substantial mempool
+       /// feerate changes between blocks, and ensuring reliability if broadcasting fails. We recommend
+       /// invoking this every 30 seconds, or lower if running in an environment with spotty
+       /// connections, like on mobile.
+       pub fn rebroadcast_pending_claims<B: Deref, F: Deref, L: Deref>(
+               &self, broadcaster: B, fee_estimator: F, logger: L,
+       )
+       where
+               B::Target: BroadcasterInterface,
+               F::Target: FeeEstimator,
+               L::Target: Logger,
+       {
+               let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
+               let mut inner = self.inner.lock().unwrap();
+               let current_height = inner.best_block.height;
+               inner.onchain_tx_handler.rebroadcast_pending_claims(
+                       current_height, &broadcaster, &fee_estimator, &logger,
+               );
+       }
 }
 
 impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
@@ -1602,9 +1631,10 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                return Some(Balance::MaybeTimeoutClaimableHTLC {
                                        claimable_amount_satoshis: htlc.amount_msat / 1000,
                                        claimable_height: htlc.cltv_expiry,
+                                       payment_hash: htlc.payment_hash,
                                });
                        }
-               } else if self.payment_preimages.get(&htlc.payment_hash).is_some() {
+               } else if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
                        // Otherwise (the payment was inbound), only expose it as claimable if
                        // we know the preimage.
                        // Note that if there is a pending claim, but it did not use the
@@ -1620,12 +1650,15 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                return Some(Balance::ContentiousClaimable {
                                        claimable_amount_satoshis: htlc.amount_msat / 1000,
                                        timeout_height: htlc.cltv_expiry,
+                                       payment_hash: htlc.payment_hash,
+                                       payment_preimage: *payment_preimage,
                                });
                        }
                } else if htlc_resolved.is_none() {
                        return Some(Balance::MaybePreimageClaimableHTLC {
                                claimable_amount_satoshis: htlc.amount_msat / 1000,
                                expiry_height: htlc.cltv_expiry,
+                               payment_hash: htlc.payment_hash,
                        });
                }
                None
@@ -1787,6 +1820,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                                        res.push(Balance::MaybeTimeoutClaimableHTLC {
                                                claimable_amount_satoshis: htlc.amount_msat / 1000,
                                                claimable_height: htlc.cltv_expiry,
+                                               payment_hash: htlc.payment_hash,
                                        });
                                } else if us.payment_preimages.get(&htlc.payment_hash).is_some() {
                                        claimable_inbound_htlc_value_sat += htlc.amount_msat / 1000;
@@ -1796,6 +1830,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                                        res.push(Balance::MaybePreimageClaimableHTLC {
                                                claimable_amount_satoshis: htlc.amount_msat / 1000,
                                                expiry_height: htlc.cltv_expiry,
+                                               payment_hash: htlc.payment_hash,
                                        });
                                }
                        }
@@ -2304,8 +2339,16 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                F::Target: FeeEstimator,
                L::Target: Logger,
        {
-               log_info!(logger, "Applying update to monitor {}, bringing update_id from {} to {} with {} changes.",
-                       log_funding_info!(self), self.latest_update_id, updates.update_id, updates.updates.len());
+               if self.latest_update_id == CLOSED_CHANNEL_UPDATE_ID && updates.update_id == CLOSED_CHANNEL_UPDATE_ID {
+                       log_info!(logger, "Applying post-force-closed update to monitor {} with {} change(s).",
+                               log_funding_info!(self), updates.updates.len());
+               } else if updates.update_id == CLOSED_CHANNEL_UPDATE_ID {
+                       log_info!(logger, "Applying force close update to monitor {} with {} change(s).",
+                               log_funding_info!(self), updates.updates.len());
+               } else {
+                       log_info!(logger, "Applying update to monitor {}, bringing update_id from {} to {} with {} change(s).",
+                               log_funding_info!(self), self.latest_update_id, updates.update_id, updates.updates.len());
+               }
                // ChannelMonitor updates may be applied after force close if we receive a preimage for a
                // broadcasted commitment transaction HTLC output that we'd like to claim on-chain. If this
                // is the case, we no longer have guaranteed access to the monitor's update ID, so we use a
@@ -2372,6 +2415,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                                                _ => false,
                                                        }).is_some();
                                                if detected_funding_spend {
+                                                       log_trace!(logger, "Avoiding commitment broadcast, already detected confirmed spend onchain");
                                                        continue;
                                                }
                                                self.broadcast_latest_holder_commitment_txn(broadcaster, logger);
@@ -2422,7 +2466,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
                self.latest_update_id = updates.update_id;
 
-               if ret.is_ok() && self.funding_spend_seen {
+               // Refuse updates after we've detected a spend onchain, but only if we haven't processed a
+               // force closed monitor update yet.
+               if ret.is_ok() && self.funding_spend_seen && self.latest_update_id != CLOSED_CHANNEL_UPDATE_ID {
                        log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
                        Err(())
                } else { ret }
@@ -4062,7 +4108,7 @@ mod tests {
        use crate::chain::channelmonitor::ChannelMonitor;
        use crate::chain::package::{weight_offered_htlc, weight_received_htlc, weight_revoked_offered_htlc, weight_revoked_received_htlc, WEIGHT_REVOKED_OUTPUT};
        use crate::chain::transaction::OutPoint;
-       use crate::chain::keysinterface::InMemorySigner;
+       use crate::sign::InMemorySigner;
        use crate::events::ClosureReason;
        use crate::ln::{PaymentPreimage, PaymentHash};
        use crate::ln::chan_utils;
@@ -4148,7 +4194,7 @@ mod tests {
                replay_update.updates.push(ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage: payment_preimage_1 });
                replay_update.updates.push(ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage: payment_preimage_2 });
 
-               let broadcaster = TestBroadcaster::new(Arc::clone(&nodes[1].blocks));
+               let broadcaster = TestBroadcaster::with_blocks(Arc::clone(&nodes[1].blocks));
                assert!(
                        pre_update_monitor.update_monitor(&replay_update, &&broadcaster, &chanmon_cfgs[1].fee_estimator, &nodes[1].logger)
                        .is_err());
@@ -4174,10 +4220,7 @@ mod tests {
        fn test_prune_preimages() {
                let secp_ctx = Secp256k1::new();
                let logger = Arc::new(TestLogger::new());
-               let broadcaster = Arc::new(TestBroadcaster {
-                       txn_broadcasted: Mutex::new(Vec::new()),
-                       blocks: Arc::new(Mutex::new(Vec::new()))
-               });
+               let broadcaster = Arc::new(TestBroadcaster::new(Network::Testnet));
                let fee_estimator = TestFeeEstimator { sat_per_kw: Mutex::new(253) };
 
                let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
@@ -4235,6 +4278,7 @@ mod tests {
                        [41; 32],
                        0,
                        [0; 32],
+                       [0; 32],
                );
 
                let counterparty_pubkeys = ChannelPublicKeys {