Merge pull request #2082 from wpaulino/bump-htlc-resolution-tx-locktime
[rust-lightning] / lightning / src / chain / package.rs
index 5537a56f2f3898bcee470ae1fae87cbf44c5767c..e8886e5a2aa49958fcac012529a3ae4160348230 100644 (file)
@@ -434,7 +434,6 @@ impl PackageSolvingData {
                                let chan_keys = TxCreationKeys::derive_new(&onchain_handler.secp_ctx, &outp.per_commitment_point, &outp.counterparty_delayed_payment_base_key, &outp.counterparty_htlc_base_key, &onchain_handler.signer.pubkeys().revocation_basepoint, &onchain_handler.signer.pubkeys().htlc_basepoint);
                                let witness_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&outp.htlc, onchain_handler.opt_anchors(), &chan_keys.broadcaster_htlc_key, &chan_keys.countersignatory_htlc_key, &chan_keys.revocation_key);
 
-                               bumped_tx.lock_time = PackedLockTime(outp.htlc.cltv_expiry); // Right now we don't aggregate time-locked transaction, if we do we should set lock_time before to avoid breaking hash computation
                                if let Ok(sig) = onchain_handler.signer.sign_counterparty_htlc_transaction(&bumped_tx, i, &outp.htlc.amount_msat / 1000, &outp.per_commitment_point, &outp.htlc, &onchain_handler.secp_ctx) {
                                        let mut ser_sig = sig.serialize_der().to_vec();
                                        ser_sig.push(EcdsaSighashType::All as u8);
@@ -460,18 +459,23 @@ impl PackageSolvingData {
                        _ => { panic!("API Error!"); }
                }
        }
-       fn absolute_tx_timelock(&self, output_conf_height: u32) -> u32 {
-               // Get the absolute timelock at which this output can be spent given the height at which
-               // this output was confirmed. We use `output_conf_height + 1` as a safe default as we can
-               // be confirmed in the next block and transactions with time lock `current_height + 1`
-               // always propagate.
+       fn absolute_tx_timelock(&self, current_height: u32) -> u32 {
+               // We use `current_height + 1` as our default locktime to discourage fee sniping and because
+               // transactions with it always propagate.
                let absolute_timelock = match self {
-                       PackageSolvingData::RevokedOutput(_) => output_conf_height + 1,
-                       PackageSolvingData::RevokedHTLCOutput(_) => output_conf_height + 1,
-                       PackageSolvingData::CounterpartyOfferedHTLCOutput(_) => output_conf_height + 1,
-                       PackageSolvingData::CounterpartyReceivedHTLCOutput(ref outp) => cmp::max(outp.htlc.cltv_expiry, output_conf_height + 1),
-                       PackageSolvingData::HolderHTLCOutput(ref outp) => cmp::max(outp.cltv_expiry, output_conf_height + 1),
-                       PackageSolvingData::HolderFundingOutput(_) => output_conf_height + 1,
+                       PackageSolvingData::RevokedOutput(_) => current_height + 1,
+                       PackageSolvingData::RevokedHTLCOutput(_) => current_height + 1,
+                       PackageSolvingData::CounterpartyOfferedHTLCOutput(_) => current_height + 1,
+                       PackageSolvingData::CounterpartyReceivedHTLCOutput(ref outp) => cmp::max(outp.htlc.cltv_expiry, current_height + 1),
+                       // HTLC timeout/success transactions rely on a fixed timelock due to the counterparty's
+                       // signature.
+                       PackageSolvingData::HolderHTLCOutput(ref outp) => {
+                               if outp.preimage.is_some() {
+                                       debug_assert_eq!(outp.cltv_expiry, 0);
+                               }
+                               outp.cltv_expiry
+                       },
+                       PackageSolvingData::HolderFundingOutput(_) => current_height + 1,
                };
                absolute_timelock
        }
@@ -638,9 +642,36 @@ impl PackageTemplate {
                }
                amounts
        }
-       pub(crate) fn package_timelock(&self) -> u32 {
-               self.inputs.iter().map(|(_, outp)| outp.absolute_tx_timelock(self.height_original))
-                       .max().expect("There must always be at least one output to spend in a PackageTemplate")
+       pub(crate) fn package_locktime(&self, current_height: u32) -> u32 {
+               let locktime = self.inputs.iter().map(|(_, outp)| outp.absolute_tx_timelock(current_height))
+                       .max().expect("There must always be at least one output to spend in a PackageTemplate");
+
+               // If we ever try to aggregate a `HolderHTLCOutput`s with another output type, we'll likely
+               // end up with an incorrect transaction locktime since the counterparty has included it in
+               // its HTLC signature. This should never happen unless we decide to aggregate outputs across
+               // different channel commitments.
+               #[cfg(debug_assertions)] {
+                       if self.inputs.iter().any(|(_, outp)|
+                               if let PackageSolvingData::HolderHTLCOutput(outp) = outp {
+                                       outp.preimage.is_some()
+                               } else {
+                                       false
+                               }
+                       ) {
+                               debug_assert_eq!(locktime, 0);
+                       };
+                       for timeout_htlc_expiry in self.inputs.iter().filter_map(|(_, outp)|
+                               if let PackageSolvingData::HolderHTLCOutput(outp) = outp {
+                                       if outp.preimage.is_none() {
+                                               Some(outp.cltv_expiry)
+                                       } else { None }
+                               } else { None }
+                       ) {
+                               debug_assert_eq!(locktime, timeout_htlc_expiry);
+                       }
+               }
+
+               locktime
        }
        pub(crate) fn package_weight(&self, destination_script: &Script) -> usize {
                let mut inputs_weight = 0;
@@ -676,12 +707,13 @@ impl PackageTemplate {
                htlcs
        }
        pub(crate) fn finalize_malleable_package<L: Deref, Signer: WriteableEcdsaChannelSigner>(
-               &self, onchain_handler: &mut OnchainTxHandler<Signer>, value: u64, destination_script: Script, logger: &L
+               &self, current_height: u32, onchain_handler: &mut OnchainTxHandler<Signer>, value: u64,
+               destination_script: Script, logger: &L
        ) -> Option<Transaction> where L::Target: Logger {
                debug_assert!(self.is_malleable());
                let mut bumped_tx = Transaction {
                        version: 2,
-                       lock_time: PackedLockTime::ZERO,
+                       lock_time: PackedLockTime(self.package_locktime(current_height)),
                        input: vec![],
                        output: vec![TxOut {
                                script_pubkey: destination_script,