Make HTLCOutputInCommitment::transaction_output_index an Option
authorMatt Corallo <git@bluematt.me>
Sun, 6 Jan 2019 22:02:53 +0000 (17:02 -0500)
committerMatt Corallo <git@bluematt.me>
Fri, 11 Jan 2019 21:03:40 +0000 (16:03 -0500)
We really shouldn't have split out the with-source HTLCs from the
in-transaction HTLCs when we added back-failing, and will need
almost all of the info in HTLCOutputInCommitment for each HTLC to
fix would_broadcast_at_height, so this is a first step at
recombining them.

src/ln/chan_utils.rs
src/ln/channel.rs
src/ln/channelmonitor.rs

index 2efa9ff5cc3f27a48ce4582882d4b63e44d5c32f..dabc0f4f47b943d60d277524ff5b1bc2276a3f1b 100644 (file)
@@ -147,7 +147,7 @@ pub struct HTLCOutputInCommitment {
        pub amount_msat: u64,
        pub cltv_expiry: u32,
        pub payment_hash: PaymentHash,
-       pub transaction_output_index: u32,
+       pub transaction_output_index: Option<u32>,
 }
 
 #[inline]
@@ -222,12 +222,13 @@ pub fn get_htlc_redeemscript(htlc: &HTLCOutputInCommitment, keys: &TxCreationKey
        get_htlc_redeemscript_with_explicit_keys(htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key)
 }
 
+/// panics if htlc.transaction_output_index.is_none()!
 pub fn build_htlc_transaction(prev_hash: &Sha256dHash, feerate_per_kw: u64, to_self_delay: u16, htlc: &HTLCOutputInCommitment, a_delayed_payment_key: &PublicKey, revocation_key: &PublicKey) -> Transaction {
        let mut txins: Vec<TxIn> = Vec::new();
        txins.push(TxIn {
                previous_output: OutPoint {
                        txid: prev_hash.clone(),
-                       vout: htlc.transaction_output_index,
+                       vout: htlc.transaction_output_index.expect("Can't build an HTLC transaction for a dust output"),
                },
                script_sig: Script::new(),
                sequence: 0,
index ddfb809ccdeaf6f7b542c0c08e2850f7646a80cf..2d579e970cf514805bf10965e810b93df1cbd137 100644 (file)
@@ -132,18 +132,6 @@ struct OutboundHTLCOutput {
        fail_reason: Option<HTLCFailReason>,
 }
 
-macro_rules! get_htlc_in_commitment {
-       ($htlc: expr, $offered: expr) => {
-               HTLCOutputInCommitment {
-                       offered: $offered,
-                       amount_msat: $htlc.amount_msat,
-                       cltv_expiry: $htlc.cltv_expiry,
-                       payment_hash: $htlc.payment_hash,
-                       transaction_output_index: 0
-               }
-       }
-}
-
 /// See AwaitingRemoteRevoke ChannelState for more info
 enum HTLCUpdateAwaitingACK {
        AddHTLC {
@@ -775,38 +763,46 @@ impl Channel {
                };
 
                let mut txouts: Vec<(TxOut, Option<(HTLCOutputInCommitment, Option<&HTLCSource>)>)> = Vec::with_capacity(self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len() + 2);
-               let mut unincluded_htlc_sources: Vec<(PaymentHash, &HTLCSource, Option<u32>)> = Vec::new();
+               let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new();
 
                let dust_limit_satoshis = if local { self.our_dust_limit_satoshis } else { self.their_dust_limit_satoshis };
                let mut remote_htlc_total_msat = 0;
                let mut local_htlc_total_msat = 0;
                let mut value_to_self_msat_offset = 0;
 
+               macro_rules! get_htlc_in_commitment {
+                       ($htlc: expr, $offered: expr) => {
+                               HTLCOutputInCommitment {
+                                       offered: $offered,
+                                       amount_msat: $htlc.amount_msat,
+                                       cltv_expiry: $htlc.cltv_expiry,
+                                       payment_hash: $htlc.payment_hash,
+                                       transaction_output_index: None
+                               }
+                       }
+               }
+
                macro_rules! add_htlc_output {
                        ($htlc: expr, $outbound: expr, $source: expr) => {
                                if $outbound == local { // "offered HTLC output"
+                                       let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
                                        if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
-                                               let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
                                                txouts.push((TxOut {
                                                        script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
                                                        value: $htlc.amount_msat / 1000
                                                }, Some((htlc_in_tx, $source))));
                                        } else {
-                                               if let Some(source) = $source {
-                                                       unincluded_htlc_sources.push(($htlc.payment_hash, source, None));
-                                               }
+                                               included_dust_htlcs.push((htlc_in_tx, $source));
                                        }
                                } else {
+                                       let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
                                        if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) {
-                                               let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
                                                txouts.push((TxOut { // "received HTLC output"
                                                        script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
                                                        value: $htlc.amount_msat / 1000
                                                }, Some((htlc_in_tx, $source))));
                                        } else {
-                                               if let Some(source) = $source {
-                                                       unincluded_htlc_sources.push(($htlc.payment_hash, source, None));
-                                               }
+                                               included_dust_htlcs.push((htlc_in_tx, $source));
                                        }
                                }
                        }
@@ -920,18 +916,22 @@ impl Channel {
 
                let mut outputs: Vec<TxOut> = Vec::with_capacity(txouts.len());
                let mut htlcs_included: Vec<HTLCOutputInCommitment> = Vec::with_capacity(txouts.len());
-               let mut htlc_sources: Vec<(PaymentHash, &HTLCSource, Option<u32>)> = Vec::with_capacity(txouts.len() + unincluded_htlc_sources.len());
+               let mut htlc_sources: Vec<(PaymentHash, &HTLCSource, Option<u32>)> = Vec::with_capacity(txouts.len() + included_dust_htlcs.len());
                for (idx, out) in txouts.drain(..).enumerate() {
                        outputs.push(out.0);
                        if let Some((mut htlc, source_option)) = out.1 {
-                               htlc.transaction_output_index = idx as u32;
+                               htlc.transaction_output_index = Some(idx as u32);
                                if let Some(source) = source_option {
                                        htlc_sources.push((htlc.payment_hash, source, Some(idx as u32)));
                                }
                                htlcs_included.push(htlc);
                        }
                }
-               htlc_sources.append(&mut unincluded_htlc_sources);
+               for (htlc, source_option) in included_dust_htlcs.drain(..) {
+                       if let Some(source) = source_option {
+                               htlc_sources.push((htlc.payment_hash, source, None));
+                       }
+               }
 
                (Transaction {
                        version: 2,
@@ -1720,18 +1720,20 @@ impl Channel {
 
                let mut htlcs_and_sigs = Vec::with_capacity(local_commitment_tx.1.len());
                for (idx, htlc) in local_commitment_tx.1.drain(..).enumerate() {
-                       let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
-                       let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
-                       let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
-                       secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
-                       let htlc_sig = if htlc.offered {
-                               let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, &htlc, &local_keys)?;
-                               new_local_commitment_txn.push(htlc_tx);
-                               htlc_sig
-                       } else {
-                               self.create_htlc_tx_signature(&htlc_tx, &htlc, &local_keys)?.1
-                       };
-                       htlcs_and_sigs.push((htlc, msg.htlc_signatures[idx], htlc_sig));
+                       if let Some(_) = htlc.transaction_output_index {
+                               let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
+                               let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
+                               let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
+                               secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
+                               let htlc_sig = if htlc.offered {
+                                       let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, &htlc, &local_keys)?;
+                                       new_local_commitment_txn.push(htlc_tx);
+                                       htlc_sig
+                               } else {
+                                       self.create_htlc_tx_signature(&htlc_tx, &htlc, &local_keys)?.1
+                               };
+                               htlcs_and_sigs.push((htlc, msg.htlc_signatures[idx], htlc_sig));
+                       }
                }
 
                let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number - 1));
@@ -3295,11 +3297,13 @@ impl Channel {
                let mut htlc_sigs = Vec::new();
 
                for ref htlc in remote_commitment_tx.1.iter() {
-                       let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys, feerate_per_kw);
-                       let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys);
-                       let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
-                       let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
-                       htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key));
+                       if let Some(_) = htlc.transaction_output_index {
+                               let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys, feerate_per_kw);
+                               let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys);
+                               let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
+                               let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
+                               htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key));
+                       }
                }
 
                Ok((msgs::CommitmentSigned {
index ea3e279213c88cca40a7e3fa5c800828fbc20f12..04efd0a8d940d8824f357e68d38613962cdc52df 100644 (file)
@@ -776,8 +776,20 @@ impl ChannelMonitor {
                // Set in initial Channel-object creation, so should always be set by now:
                U48(self.commitment_transaction_number_obscure_factor).write(writer)?;
 
+               macro_rules! write_option {
+                       ($thing: expr) => {
+                               match $thing {
+                                       &Some(ref t) => {
+                                               1u8.write(writer)?;
+                                               t.write(writer)?;
+                                       },
+                                       &None => 0u8.write(writer)?,
+                               }
+                       }
+               }
+
                match self.key_storage {
-                       Storage::Local { ref revocation_base_key, ref htlc_base_key, ref delayed_payment_base_key, ref payment_base_key, ref shutdown_pubkey, ref prev_latest_per_commitment_point, ref latest_per_commitment_point, ref funding_info, current_remote_commitment_txid, prev_remote_commitment_txid } => {
+                       Storage::Local { ref revocation_base_key, ref htlc_base_key, ref delayed_payment_base_key, ref payment_base_key, ref shutdown_pubkey, ref prev_latest_per_commitment_point, ref latest_per_commitment_point, ref funding_info, ref current_remote_commitment_txid, ref prev_remote_commitment_txid } => {
                                writer.write_all(&[0; 1])?;
                                writer.write_all(&revocation_base_key[..])?;
                                writer.write_all(&htlc_base_key[..])?;
@@ -806,18 +818,8 @@ impl ChannelMonitor {
                                                debug_assert!(false, "Try to serialize a useless Local monitor !");
                                        },
                                }
-                               if let Some(ref txid) = current_remote_commitment_txid {
-                                       writer.write_all(&[1; 1])?;
-                                       writer.write_all(&txid[..])?;
-                               } else {
-                                       writer.write_all(&[0; 1])?;
-                               }
-                               if let Some(ref txid) = prev_remote_commitment_txid {
-                                       writer.write_all(&[1; 1])?;
-                                       writer.write_all(&txid[..])?;
-                               } else {
-                                       writer.write_all(&[0; 1])?;
-                               }
+                               write_option!(current_remote_commitment_txid);
+                               write_option!(prev_remote_commitment_txid);
                        },
                        Storage::Watchtower { .. } => unimplemented!(),
                }
@@ -857,7 +859,7 @@ impl ChannelMonitor {
                                writer.write_all(&byte_utils::be64_to_array($htlc_output.amount_msat))?;
                                writer.write_all(&byte_utils::be32_to_array($htlc_output.cltv_expiry))?;
                                writer.write_all(&$htlc_output.payment_hash.0[..])?;
-                               writer.write_all(&byte_utils::be32_to_array($htlc_output.transaction_output_index))?;
+                               write_option!(&$htlc_output.transaction_output_index);
                        }
                }
 
@@ -1142,39 +1144,41 @@ impl ChannelMonitor {
                                inputs.reserve_exact(per_commitment_data.len());
 
                                for (idx, ref htlc) in per_commitment_data.iter().enumerate() {
-                                       let expected_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey);
-                                       if htlc.transaction_output_index as usize >= tx.output.len() ||
-                                                       tx.output[htlc.transaction_output_index as usize].value != htlc.amount_msat / 1000 ||
-                                                       tx.output[htlc.transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
-                                               return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); // Corrupted per_commitment_data, fuck this user
-                                       }
-                                       let input = TxIn {
-                                               previous_output: BitcoinOutPoint {
-                                                       txid: commitment_txid,
-                                                       vout: htlc.transaction_output_index,
-                                               },
-                                               script_sig: Script::new(),
-                                               sequence: 0xfffffffd,
-                                               witness: Vec::new(),
-                                       };
-                                       if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER {
-                                               inputs.push(input);
-                                               htlc_idxs.push(Some(idx));
-                                               values.push(tx.output[htlc.transaction_output_index as usize].value);
-                                               total_value += htlc.amount_msat / 1000;
-                                       } else {
-                                               let mut single_htlc_tx = Transaction {
-                                                       version: 2,
-                                                       lock_time: 0,
-                                                       input: vec![input],
-                                                       output: vec!(TxOut {
-                                                               script_pubkey: self.destination_script.clone(),
-                                                               value: htlc.amount_msat / 1000, //TODO: - fee
-                                                       }),
+                                       if let Some(transaction_output_index) = htlc.transaction_output_index {
+                                               let expected_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey);
+                                               if transaction_output_index as usize >= tx.output.len() ||
+                                                               tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 ||
+                                                               tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
+                                                       return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); // Corrupted per_commitment_data, fuck this user
+                                               }
+                                               let input = TxIn {
+                                                       previous_output: BitcoinOutPoint {
+                                                               txid: commitment_txid,
+                                                               vout: transaction_output_index,
+                                                       },
+                                                       script_sig: Script::new(),
+                                                       sequence: 0xfffffffd,
+                                                       witness: Vec::new(),
                                                };
-                                               let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
-                                               sign_input!(sighash_parts, single_htlc_tx.input[0], Some(idx), htlc.amount_msat / 1000);
-                                               txn_to_broadcast.push(single_htlc_tx);
+                                               if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER {
+                                                       inputs.push(input);
+                                                       htlc_idxs.push(Some(idx));
+                                                       values.push(tx.output[transaction_output_index as usize].value);
+                                                       total_value += htlc.amount_msat / 1000;
+                                               } else {
+                                                       let mut single_htlc_tx = Transaction {
+                                                               version: 2,
+                                                               lock_time: 0,
+                                                               input: vec![input],
+                                                               output: vec!(TxOut {
+                                                                       script_pubkey: self.destination_script.clone(),
+                                                                       value: htlc.amount_msat / 1000, //TODO: - fee
+                                                               }),
+                                                       };
+                                                       let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
+                                                       sign_input!(sighash_parts, single_htlc_tx.input[0], Some(idx), htlc.amount_msat / 1000);
+                                                       txn_to_broadcast.push(single_htlc_tx);
+                                               }
                                        }
                                }
                        }
@@ -1351,70 +1355,72 @@ impl ChannelMonitor {
                                        }
 
                                        for (idx, ref htlc) in per_commitment_data.0.iter().enumerate() {
-                                               let expected_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey);
-                                               if htlc.transaction_output_index as usize >= tx.output.len() ||
-                                                               tx.output[htlc.transaction_output_index as usize].value != htlc.amount_msat / 1000 ||
-                                                               tx.output[htlc.transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
-                                                       return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); // Corrupted per_commitment_data, fuck this user
-                                               }
-                                               if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
-                                                       let input = TxIn {
-                                                               previous_output: BitcoinOutPoint {
-                                                                       txid: commitment_txid,
-                                                                       vout: htlc.transaction_output_index,
-                                                               },
-                                                               script_sig: Script::new(),
-                                                               sequence: idx as u32, // reset to 0xfffffffd in sign_input
-                                                               witness: Vec::new(),
-                                                       };
-                                                       if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER {
-                                                               inputs.push(input);
-                                                               values.push((tx.output[htlc.transaction_output_index as usize].value, payment_preimage));
-                                                               total_value += htlc.amount_msat / 1000;
-                                                       } else {
-                                                               let mut single_htlc_tx = Transaction {
+                                               if let Some(transaction_output_index) = htlc.transaction_output_index {
+                                                       let expected_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey);
+                                                       if transaction_output_index as usize >= tx.output.len() ||
+                                                                       tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 ||
+                                                                       tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
+                                                               return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); // Corrupted per_commitment_data, fuck this user
+                                                       }
+                                                       if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
+                                                               let input = TxIn {
+                                                                       previous_output: BitcoinOutPoint {
+                                                                               txid: commitment_txid,
+                                                                               vout: transaction_output_index,
+                                                                       },
+                                                                       script_sig: Script::new(),
+                                                                       sequence: idx as u32, // reset to 0xfffffffd in sign_input
+                                                                       witness: Vec::new(),
+                                                               };
+                                                               if htlc.cltv_expiry > height + CLTV_SHARED_CLAIM_BUFFER {
+                                                                       inputs.push(input);
+                                                                       values.push((tx.output[transaction_output_index as usize].value, payment_preimage));
+                                                                       total_value += htlc.amount_msat / 1000;
+                                                               } else {
+                                                                       let mut single_htlc_tx = Transaction {
+                                                                               version: 2,
+                                                                               lock_time: 0,
+                                                                               input: vec![input],
+                                                                               output: vec!(TxOut {
+                                                                                       script_pubkey: self.destination_script.clone(),
+                                                                                       value: htlc.amount_msat / 1000, //TODO: - fee
+                                                                               }),
+                                                                       };
+                                                                       let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
+                                                                       sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec());
+                                                                       spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
+                                                                               outpoint: BitcoinOutPoint { txid: single_htlc_tx.txid(), vout: 0 },
+                                                                               output: single_htlc_tx.output[0].clone(),
+                                                                       });
+                                                                       txn_to_broadcast.push(single_htlc_tx);
+                                                               }
+                                                       }
+                                                       if !htlc.offered {
+                                                               // TODO: If the HTLC has already expired, potentially merge it with the
+                                                               // rest of the claim transaction, as above.
+                                                               let input = TxIn {
+                                                                       previous_output: BitcoinOutPoint {
+                                                                               txid: commitment_txid,
+                                                                               vout: transaction_output_index,
+                                                                       },
+                                                                       script_sig: Script::new(),
+                                                                       sequence: idx as u32,
+                                                                       witness: Vec::new(),
+                                                               };
+                                                               let mut timeout_tx = Transaction {
                                                                        version: 2,
-                                                                       lock_time: 0,
+                                                                       lock_time: htlc.cltv_expiry,
                                                                        input: vec![input],
                                                                        output: vec!(TxOut {
                                                                                script_pubkey: self.destination_script.clone(),
-                                                                               value: htlc.amount_msat / 1000, //TODO: - fee
+                                                                               value: htlc.amount_msat / 1000,
                                                                        }),
                                                                };
-                                                               let sighash_parts = bip143::SighashComponents::new(&single_htlc_tx);
-                                                               sign_input!(sighash_parts, single_htlc_tx.input[0], htlc.amount_msat / 1000, payment_preimage.0.to_vec());
-                                                               spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
-                                                                       outpoint: BitcoinOutPoint { txid: single_htlc_tx.txid(), vout: 0 },
-                                                                       output: single_htlc_tx.output[0].clone(),
-                                                               });
-                                                               txn_to_broadcast.push(single_htlc_tx);
+                                                               let sighash_parts = bip143::SighashComponents::new(&timeout_tx);
+                                                               sign_input!(sighash_parts, timeout_tx.input[0], htlc.amount_msat / 1000, vec![0]);
+                                                               txn_to_broadcast.push(timeout_tx);
                                                        }
                                                }
-                                               if !htlc.offered {
-                                                       // TODO: If the HTLC has already expired, potentially merge it with the
-                                                       // rest of the claim transaction, as above.
-                                                       let input = TxIn {
-                                                               previous_output: BitcoinOutPoint {
-                                                                       txid: commitment_txid,
-                                                                       vout: htlc.transaction_output_index,
-                                                               },
-                                                               script_sig: Script::new(),
-                                                               sequence: idx as u32,
-                                                               witness: Vec::new(),
-                                                       };
-                                                       let mut timeout_tx = Transaction {
-                                                               version: 2,
-                                                               lock_time: htlc.cltv_expiry,
-                                                               input: vec![input],
-                                                               output: vec!(TxOut {
-                                                                       script_pubkey: self.destination_script.clone(),
-                                                                       value: htlc.amount_msat / 1000,
-                                                               }),
-                                                       };
-                                                       let sighash_parts = bip143::SighashComponents::new(&timeout_tx);
-                                                       sign_input!(sighash_parts, timeout_tx.input[0], htlc.amount_msat / 1000, vec![0]);
-                                                       txn_to_broadcast.push(timeout_tx);
-                                               }
                                        }
 
                                        if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs), spendable_outputs, htlc_updated); } // Nothing to be done...probably a false positive/local tx
@@ -1570,40 +1576,42 @@ impl ChannelMonitor {
                }
 
                for &(ref htlc, ref their_sig, ref our_sig) in local_tx.htlc_outputs.iter() {
-                       if htlc.offered {
-                               let mut htlc_timeout_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key);
+                       if let Some(transaction_output_index) = htlc.transaction_output_index {
+                               if htlc.offered {
+                                       let mut htlc_timeout_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key);
 
-                               htlc_timeout_tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
+                                       htlc_timeout_tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
 
-                               htlc_timeout_tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec());
-                               htlc_timeout_tx.input[0].witness[1].push(SigHashType::All as u8);
-                               htlc_timeout_tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec());
-                               htlc_timeout_tx.input[0].witness[2].push(SigHashType::All as u8);
+                                       htlc_timeout_tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec());
+                                       htlc_timeout_tx.input[0].witness[1].push(SigHashType::All as u8);
+                                       htlc_timeout_tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec());
+                                       htlc_timeout_tx.input[0].witness[2].push(SigHashType::All as u8);
 
-                               htlc_timeout_tx.input[0].witness.push(Vec::new());
-                               htlc_timeout_tx.input[0].witness.push(chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key).into_bytes());
+                                       htlc_timeout_tx.input[0].witness.push(Vec::new());
+                                       htlc_timeout_tx.input[0].witness.push(chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key).into_bytes());
 
-                               add_dynamic_output!(htlc_timeout_tx, 0);
-                               res.push(htlc_timeout_tx);
-                       } else {
-                               if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
-                                       let mut htlc_success_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key);
+                                       add_dynamic_output!(htlc_timeout_tx, 0);
+                                       res.push(htlc_timeout_tx);
+                               } else {
+                                       if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) {
+                                               let mut htlc_success_tx = chan_utils::build_htlc_transaction(&local_tx.txid, local_tx.feerate_per_kw, self.their_to_self_delay.unwrap(), htlc, &local_tx.delayed_payment_key, &local_tx.revocation_key);
 
-                                       htlc_success_tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
+                                               htlc_success_tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
 
-                                       htlc_success_tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec());
-                                       htlc_success_tx.input[0].witness[1].push(SigHashType::All as u8);
-                                       htlc_success_tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec());
-                                       htlc_success_tx.input[0].witness[2].push(SigHashType::All as u8);
+                                               htlc_success_tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec());
+                                               htlc_success_tx.input[0].witness[1].push(SigHashType::All as u8);
+                                               htlc_success_tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec());
+                                               htlc_success_tx.input[0].witness[2].push(SigHashType::All as u8);
 
-                                       htlc_success_tx.input[0].witness.push(payment_preimage.0.to_vec());
-                                       htlc_success_tx.input[0].witness.push(chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key).into_bytes());
+                                               htlc_success_tx.input[0].witness.push(payment_preimage.0.to_vec());
+                                               htlc_success_tx.input[0].witness.push(chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key).into_bytes());
 
-                                       add_dynamic_output!(htlc_success_tx, 0);
-                                       res.push(htlc_success_tx);
+                                               add_dynamic_output!(htlc_success_tx, 0);
+                                               res.push(htlc_success_tx);
+                                       }
                                }
+                               watch_outputs.push(local_tx.tx.output[transaction_output_index as usize].clone());
                        }
-                       watch_outputs.push(local_tx.tx.output[htlc.transaction_output_index as usize].clone());
                }
 
                (res, spendable_outputs, watch_outputs)
@@ -1876,7 +1884,7 @@ impl ChannelMonitor {
                                        }
                                        if payment_data.is_none() {
                                                for htlc_output in $htlc_outputs {
-                                                       if input.previous_output.vout == htlc_output.transaction_output_index {
+                                                       if Some(input.previous_output.vout) == htlc_output.transaction_output_index {
                                                                log_claim!($source, $local_tx, $local_tx == htlc_output.offered, htlc_output.payment_hash, false);
                                                                continue 'outer_loop;
                                                        }
@@ -1935,6 +1943,13 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                                }
                        }
                }
+               macro_rules! read_option { () => {
+                       match <u8 as Readable<R>>::read(reader)? {
+                               0 => None,
+                               1 => Some(Readable::read(reader)?),
+                               _ => return Err(DecodeError::InvalidValue),
+                       }
+               } }
 
                let _ver: u8 = Readable::read(reader)?;
                let min_ver: u8 = Readable::read(reader)?;
@@ -1968,16 +1983,8 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                                        index: Readable::read(reader)?,
                                };
                                let funding_info = Some((outpoint, Readable::read(reader)?));
-                               let current_remote_commitment_txid = match <u8 as Readable<R>>::read(reader)? {
-                                       0 => None,
-                                       1 => Some(Readable::read(reader)?),
-                                       _ => return Err(DecodeError::InvalidValue),
-                               };
-                               let prev_remote_commitment_txid = match <u8 as Readable<R>>::read(reader)? {
-                                       0 => None,
-                                       1 => Some(Readable::read(reader)?),
-                                       _ => return Err(DecodeError::InvalidValue),
-                               };
+                               let current_remote_commitment_txid = read_option!();
+                               let prev_remote_commitment_txid = read_option!();
                                Storage::Local {
                                        revocation_base_key,
                                        htlc_base_key,
@@ -2028,7 +2035,7 @@ impl<R: ::std::io::Read> ReadableArgs<R, Arc<Logger>> for (Sha256dHash, ChannelM
                                        let amount_msat: u64 = Readable::read(reader)?;
                                        let cltv_expiry: u32 = Readable::read(reader)?;
                                        let payment_hash: PaymentHash = Readable::read(reader)?;
-                                       let transaction_output_index: u32 = Readable::read(reader)?;
+                                       let transaction_output_index: Option<u32> = read_option!();
 
                                        HTLCOutputInCommitment {
                                                offered, amount_msat, cltv_expiry, payment_hash, transaction_output_index
@@ -2616,7 +2623,7 @@ mod tests {
                                                        amount_msat: 0,
                                                        cltv_expiry: 0,
                                                        payment_hash: preimage.1.clone(),
-                                                       transaction_output_index: idx as u32,
+                                                       transaction_output_index: Some(idx as u32),
                                                });
                                        }
                                        res