Support future removal of redundant per-HTLC data in `ChanMonUpd`s
[rust-lightning] / lightning / src / ln / channel.rs
index 53e8201ff5d190a22d6730e086dce2934db3a3cc..f883297576abbbd81c932633e0075f8dff14eb0b 100644 (file)
@@ -27,7 +27,7 @@ use crate::ln::features::{ChannelTypeFeatures, InitFeatures};
 use crate::ln::msgs;
 use crate::ln::msgs::{DecodeError, OptionalField, DataLossProtect};
 use crate::ln::script::{self, ShutdownScript};
-use crate::ln::channelmanager::{self, CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSource, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
+use crate::ln::channelmanager::{self, CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
 use crate::ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction};
 use crate::ln::chan_utils;
 use crate::ln::onion_utils::HTLCFailReason;
@@ -192,6 +192,7 @@ enum OutboundHTLCState {
 
 #[derive(Clone)]
 enum OutboundHTLCOutcome {
+       /// LDK version 0.0.105+ will always fill in the preimage here.
        Success(Option<PaymentPreimage>),
        Failure(HTLCFailReason),
 }
@@ -2268,9 +2269,9 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 
        pub fn funding_created<SP: Deref, L: Deref>(
                &mut self, msg: &msgs::FundingCreated, best_block: BestBlock, signer_provider: &SP, logger: &L
-       ) -> Result<(msgs::FundingSigned, ChannelMonitor<<SP::Target as SignerProvider>::Signer>, Option<msgs::ChannelReady>), ChannelError>
+       ) -> Result<(msgs::FundingSigned, ChannelMonitor<Signer>), ChannelError>
        where
-               SP::Target: SignerProvider,
+               SP::Target: SignerProvider<Signer = Signer>,
                L::Target: Logger
        {
                if self.is_outbound() {
@@ -2346,19 +2347,22 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 
                log_info!(logger, "Generated funding_signed for peer for channel {}", log_bytes!(self.channel_id()));
 
+               let need_channel_ready = self.check_get_channel_ready(0).is_some();
+               self.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
+
                Ok((msgs::FundingSigned {
                        channel_id: self.channel_id,
                        signature
-               }, channel_monitor, self.check_get_channel_ready(0)))
+               }, channel_monitor))
        }
 
        /// Handles a funding_signed message from the remote end.
        /// If this call is successful, broadcast the funding transaction (and not before!)
        pub fn funding_signed<SP: Deref, L: Deref>(
                &mut self, msg: &msgs::FundingSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
-       ) -> Result<(ChannelMonitor<<SP::Target as SignerProvider>::Signer>, Transaction, Option<msgs::ChannelReady>), ChannelError>
+       ) -> Result<ChannelMonitor<Signer>, ChannelError>
        where
-               SP::Target: SignerProvider,
+               SP::Target: SignerProvider<Signer = Signer>,
                L::Target: Logger
        {
                if !self.is_outbound() {
@@ -2431,7 +2435,9 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 
                log_info!(logger, "Received funding_signed from peer for channel {}", log_bytes!(self.channel_id()));
 
-               Ok((channel_monitor, self.funding_transaction.as_ref().cloned().unwrap(), self.check_get_channel_ready(0)))
+               let need_channel_ready = self.check_get_channel_ready(0).is_some();
+               self.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
+               Ok(channel_monitor)
        }
 
        /// Handles a channel_ready message from our peer. If we've already sent our channel_ready
@@ -2478,6 +2484,11 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                                        // If they haven't ever sent an updated point, the point they send should match
                                        // the current one.
                                        self.counterparty_cur_commitment_point
+                               } else if self.cur_counterparty_commitment_transaction_number == INITIAL_COMMITMENT_NUMBER - 2 {
+                                       // If we've advanced the commitment number once, the second commitment point is
+                                       // at `counterparty_prev_commitment_point`, which is not yet revoked.
+                                       debug_assert!(self.counterparty_prev_commitment_point.is_some());
+                                       self.counterparty_prev_commitment_point
                                } else {
                                        // If they have sent updated points, channel_ready is always supposed to match
                                        // their "first" point, which we re-derive here.
@@ -3111,9 +3122,24 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                        return Err(ChannelError::Close(format!("Got wrong number of HTLC signatures ({}) from remote. It must be {}", msg.htlc_signatures.len(), commitment_stats.num_nondust_htlcs)));
                }
 
-               // TODO: Sadly, we pass HTLCs twice to ChannelMonitor: once via the HolderCommitmentTransaction and once via the update
+               // Up to LDK 0.0.115, HTLC information was required to be duplicated in the
+               // `htlcs_and_sigs` vec and in the `holder_commitment_tx` itself, both of which were passed
+               // in the `ChannelMonitorUpdate`. In 0.0.115, support for having a separate set of
+               // outbound-non-dust-HTLCSources in the `ChannelMonitorUpdate` was added, however for
+               // backwards compatibility, we never use it in production. To provide test coverage, here,
+               // we randomly decide (in test/fuzzing builds) to use the new vec sometimes.
+               #[allow(unused_assignments, unused_mut)]
+               let mut separate_nondust_htlc_sources = false;
+               #[cfg(all(feature = "std", any(test, fuzzing)))] {
+                       use core::hash::{BuildHasher, Hasher};
+                       // Get a random value using the only std API to do so - the DefaultHasher
+                       let rand_val = std::collections::hash_map::RandomState::new().build_hasher().finish();
+                       separate_nondust_htlc_sources = rand_val % 2 == 0;
+               }
+
+               let mut nondust_htlc_sources = Vec::with_capacity(htlcs_cloned.len());
                let mut htlcs_and_sigs = Vec::with_capacity(htlcs_cloned.len());
-               for (idx, (htlc, source)) in htlcs_cloned.drain(..).enumerate() {
+               for (idx, (htlc, mut source_opt)) in htlcs_cloned.drain(..).enumerate() {
                        if let Some(_) = htlc.transaction_output_index {
                                let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_stats.feerate_per_kw,
                                        self.get_counterparty_selected_contest_delay().unwrap(), &htlc, self.opt_anchors(),
@@ -3128,10 +3154,18 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                                if let Err(_) = self.secp_ctx.verify_ecdsa(&htlc_sighash, &msg.htlc_signatures[idx], &keys.countersignatory_htlc_key) {
                                        return Err(ChannelError::Close("Invalid HTLC tx signature from peer".to_owned()));
                                }
-                               htlcs_and_sigs.push((htlc, Some(msg.htlc_signatures[idx]), source));
+                               if !separate_nondust_htlc_sources {
+                                       htlcs_and_sigs.push((htlc, Some(msg.htlc_signatures[idx]), source_opt.take()));
+                               }
                        } else {
-                               htlcs_and_sigs.push((htlc, None, source));
+                               htlcs_and_sigs.push((htlc, None, source_opt.take()));
                        }
+                       if separate_nondust_htlc_sources {
+                               if let Some(source) = source_opt.take() {
+                                       nondust_htlc_sources.push(source);
+                               }
+                       }
+                       debug_assert!(source_opt.is_none(), "HTLCSource should have been put somewhere");
                }
 
                let holder_commitment_tx = HolderCommitmentTransaction::new(
@@ -3154,15 +3188,6 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                        }
                }
 
-               self.latest_monitor_update_id += 1;
-               let mut monitor_update = ChannelMonitorUpdate {
-                       update_id: self.latest_monitor_update_id,
-                       updates: vec![ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo {
-                               commitment_tx: holder_commitment_tx,
-                               htlc_outputs: htlcs_and_sigs
-                       }]
-               };
-
                for htlc in self.pending_inbound_htlcs.iter_mut() {
                        let new_forward = if let &InboundHTLCState::RemoteAnnounced(ref forward_info) = &htlc.state {
                                Some(forward_info.clone())
@@ -3174,6 +3199,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                                need_commitment = true;
                        }
                }
+               let mut claimed_htlcs = Vec::new();
                for htlc in self.pending_outbound_htlcs.iter_mut() {
                        if let &mut OutboundHTLCState::RemoteRemoved(ref mut outcome) = &mut htlc.state {
                                log_trace!(logger, "Updating HTLC {} to AwaitingRemoteRevokeToRemove due to commitment_signed in channel {}.",
@@ -3181,11 +3207,31 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                                // Grab the preimage, if it exists, instead of cloning
                                let mut reason = OutboundHTLCOutcome::Success(None);
                                mem::swap(outcome, &mut reason);
+                               if let OutboundHTLCOutcome::Success(Some(preimage)) = reason {
+                                       // If a user (a) receives an HTLC claim using LDK 0.0.104 or before, then (b)
+                                       // upgrades to LDK 0.0.114 or later before the HTLC is fully resolved, we could
+                                       // have a `Success(None)` reason. In this case we could forget some HTLC
+                                       // claims, but such an upgrade is unlikely and including claimed HTLCs here
+                                       // fixes a bug which the user was exposed to on 0.0.104 when they started the
+                                       // claim anyway.
+                                       claimed_htlcs.push((SentHTLCId::from_source(&htlc.source), preimage));
+                               }
                                htlc.state = OutboundHTLCState::AwaitingRemoteRevokeToRemove(reason);
                                need_commitment = true;
                        }
                }
 
+               self.latest_monitor_update_id += 1;
+               let mut monitor_update = ChannelMonitorUpdate {
+                       update_id: self.latest_monitor_update_id,
+                       updates: vec![ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo {
+                               commitment_tx: holder_commitment_tx,
+                               htlc_outputs: htlcs_and_sigs,
+                               claimed_htlcs,
+                               nondust_htlc_sources,
+                       }]
+               };
+
                self.cur_holder_commitment_transaction_number -= 1;
                // Note that if we need_commitment & !AwaitingRemoteRevoke we'll call
                // build_commitment_no_status_check() next which will reset this to RAAFirst.
@@ -3738,15 +3784,17 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
        }
 
        /// Indicates that a ChannelMonitor update is in progress and has not yet been fully persisted.
-       /// This must be called immediately after the [`chain::Watch`] call which returned
-       /// [`ChannelMonitorUpdateStatus::InProgress`].
+       /// This must be called before we return the [`ChannelMonitorUpdate`] back to the
+       /// [`ChannelManager`], which will call [`Self::monitor_updating_restored`] once the monitor
+       /// update completes (potentially immediately).
        /// The messages which were generated with the monitor update must *not* have been sent to the
        /// remote end, and must instead have been dropped. They will be regenerated when
        /// [`Self::monitor_updating_restored`] is called.
        ///
+       /// [`ChannelManager`]: super::channelmanager::ChannelManager
        /// [`chain::Watch`]: crate::chain::Watch
        /// [`ChannelMonitorUpdateStatus::InProgress`]: crate::chain::ChannelMonitorUpdateStatus::InProgress
-       pub fn monitor_updating_paused(&mut self, resend_raa: bool, resend_commitment: bool,
+       fn monitor_updating_paused(&mut self, resend_raa: bool, resend_commitment: bool,
                resend_channel_ready: bool, mut pending_forwards: Vec<(PendingHTLCInfo, u64)>,
                mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
                mut pending_finalized_claimed_htlcs: Vec<HTLCSource>
@@ -5958,9 +6006,16 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                        return Err(APIError::ChannelUnavailable{err: "Cannot begin shutdown while peer is disconnected or we're waiting on a monitor update, maybe force-close instead?".to_owned()});
                }
 
+               // If we haven't funded the channel yet, we don't need to bother ensuring the shutdown
+               // script is set, we just force-close and call it a day.
+               let mut chan_closed = false;
+               if self.channel_state < ChannelState::FundingSent as u32 {
+                       chan_closed = true;
+               }
+
                let update_shutdown_script = match self.shutdown_scriptpubkey {
                        Some(_) => false,
-                       None => {
+                       None if !chan_closed => {
                                let shutdown_scriptpubkey = signer_provider.get_shutdown_scriptpubkey();
                                if !shutdown_scriptpubkey.is_compatible(their_features) {
                                        return Err(APIError::IncompatibleShutdownScript { script: shutdown_scriptpubkey.clone() });
@@ -5968,6 +6023,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
                                self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey);
                                true
                        },
+                       None => false,
                };
 
                // From here on out, we may not fail!
@@ -7160,7 +7216,7 @@ mod tests {
                let secp_ctx = Secp256k1::new();
                let seed = [42; 32];
                let network = Network::Testnet;
-               let best_block = BestBlock::from_genesis(network);
+               let best_block = BestBlock::from_network(network);
                let chain_hash = best_block.block_hash();
                let keys_provider = test_utils::TestKeysInterface::new(&seed, network);
 
@@ -7187,7 +7243,7 @@ mod tests {
                }]};
                let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
                let funding_created_msg = node_a_chan.get_outbound_funding_created(tx.clone(), funding_outpoint, &&logger).unwrap();
-               let (funding_signed_msg, _, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&keys_provider, &&logger).unwrap();
+               let (funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg, best_block, &&keys_provider, &&logger).unwrap();
 
                // Node B --> Node A: funding signed
                let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&keys_provider, &&logger);