Move local commitment tx generation in OnchainTxHandler
authorAntoine Riard <ariard@student.42.fr>
Tue, 3 Mar 2020 23:51:50 +0000 (18:51 -0500)
committerAntoine Riard <ariard@student.42.fr>
Fri, 17 Apr 2020 21:43:50 +0000 (17:43 -0400)
Local Commitment Transaction can't be bumped without anchor outputs
so their generation is one-time for now. We move them in
OnchainTxHandler for simplifying ChannelMonitor and to prepare
storage of keys material behind one external signer interface.

Some tests break due to change in transaction broadcast order but
number of transactions broadcast should stay the same.

lightning/src/ln/channelmonitor.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/onchaintx.rs

index 2fa416edb2dd79f26c8c0f5433b31e60c135bc2b..b3cff4c2b42065406cd7b4b568700de0d80a81f9 100644 (file)
@@ -443,6 +443,9 @@ pub(crate) enum InputMaterial {
                sigs: (Signature, Signature),
                preimage: Option<PaymentPreimage>,
                amount: u64,
+       },
+       Funding {
+               channel_value: u64,
        }
 }
 
@@ -472,6 +475,10 @@ impl Writeable for InputMaterial  {
                                sigs.1.write(writer)?;
                                preimage.write(writer)?;
                                writer.write_all(&byte_utils::be64_to_array(*amount))?;
+                       },
+                       &InputMaterial::Funding { ref channel_value } => {
+                               writer.write_all(&[3; 1])?;
+                               channel_value.write(writer)?;
                        }
                }
                Ok(())
@@ -521,6 +528,12 @@ impl Readable for InputMaterial {
                                        preimage,
                                        amount
                                }
+                       },
+                       3 => {
+                               let channel_value = Readable::read(reader)?;
+                               InputMaterial::Funding {
+                                       channel_value
+                               }
                        }
                        _ => return Err(DecodeError::InvalidValue),
                };
@@ -1894,15 +1907,11 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                let should_broadcast = if let Some(_) = self.current_local_signed_commitment_tx {
                        self.would_broadcast_at_height(height)
                } else { false };
-               if let Some(ref mut cur_local_tx) = self.current_local_signed_commitment_tx {
-                       if should_broadcast {
-                               self.onchain_detection.keys.sign_local_commitment(&mut cur_local_tx.tx, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &mut self.secp_ctx);
-                       }
+               if should_broadcast {
+                       claimable_outpoints.push(ClaimRequest { absolute_timelock: height, aggregable: false, outpoint: BitcoinOutPoint { txid: self.onchain_detection.funding_info.as_ref().unwrap().0.txid.clone(), vout: self.onchain_detection.funding_info.as_ref().unwrap().0.index as u32 }, witness_data: InputMaterial::Funding { channel_value: self.channel_value_satoshis.unwrap() }});
                }
                if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
                        if should_broadcast {
-                               log_trace!(self, "Broadcast onchain {}", log_tx!(cur_local_tx.tx.with_valid_witness()));
-                               broadcaster.broadcast_transaction(&cur_local_tx.tx.with_valid_witness());
                                let (txs, new_outputs, _) = self.broadcast_by_local_state(&cur_local_tx);
                                if !new_outputs.is_empty() {
                                        watch_outputs.push((cur_local_tx.txid.clone(), new_outputs));
index 62cae026911acbfd1210a0a195e4b13b34a87b89..0c6bac59cb5c6b7278aad252795557691904352c 100644 (file)
@@ -2342,24 +2342,25 @@ fn claim_htlc_outputs_single_tx() {
                // ChannelMonitor: local commitment + local HTLC-timeout (2)
 
                // Check the pair local commitment and HTLC-timeout broadcast due to HTLC expiration
+               assert_eq!(node_txn[3].input.len(), 1);
+               check_spends!(node_txn[3], chan_1.3);
                assert_eq!(node_txn[0].input.len(), 1);
-               check_spends!(node_txn[0], chan_1.3);
-               assert_eq!(node_txn[1].input.len(), 1);
-               let witness_script = node_txn[1].input[0].witness.last().unwrap();
+               let witness_script = node_txn[0].input[0].witness.last().unwrap();
                assert_eq!(witness_script.len(), OFFERED_HTLC_SCRIPT_WEIGHT); //Spending an offered htlc output
-               check_spends!(node_txn[1], node_txn[0]);
+               check_spends!(node_txn[0], node_txn[3]);
 
-               // Justice transactions are indices 2-3-4
+               // Justice transactions are indices 1-2-4
+               assert_eq!(node_txn[1].input.len(), 1);
                assert_eq!(node_txn[2].input.len(), 1);
-               assert_eq!(node_txn[3].input.len(), 1);
                assert_eq!(node_txn[4].input.len(), 1);
+
+               check_spends!(node_txn[1], revoked_local_txn[0]);
                check_spends!(node_txn[2], revoked_local_txn[0]);
-               check_spends!(node_txn[3], revoked_local_txn[0]);
                check_spends!(node_txn[4], revoked_local_txn[0]);
 
                let mut witness_lens = BTreeSet::new();
+               witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
                witness_lens.insert(node_txn[2].input[0].witness.last().unwrap().len());
-               witness_lens.insert(node_txn[3].input[0].witness.last().unwrap().len());
                witness_lens.insert(node_txn[4].input[0].witness.last().unwrap().len());
                assert_eq!(witness_lens.len(), 3);
                assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
@@ -2611,18 +2612,19 @@ fn test_htlc_on_chain_timeout() {
        {
                let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_txn.len(), 5); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : 2 (local commitment tx + HTLC-timeout), 1 timeout tx
-               assert_eq!(node_txn[0], node_txn[3]);
-               assert_eq!(node_txn[1], node_txn[4]);
 
-               check_spends!(node_txn[2], commitment_tx[0]);
-               assert_eq!(node_txn[2].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
+               assert_eq!(node_txn[2], node_txn[3]);
+               assert_eq!(node_txn[0], node_txn[4]);
 
-               check_spends!(node_txn[0], chan_2.3);
-               check_spends!(node_txn[1], node_txn[0]);
-               assert_eq!(node_txn[0].clone().input[0].witness.last().unwrap().len(), 71);
-               assert_eq!(node_txn[1].clone().input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
+               check_spends!(node_txn[1], commitment_tx[0]);
+               assert_eq!(node_txn[1].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
 
-               timeout_tx = node_txn[2].clone();
+               check_spends!(node_txn[2], chan_2.3);
+               check_spends!(node_txn[0], node_txn[2]);
+               assert_eq!(node_txn[2].clone().input[0].witness.last().unwrap().len(), 71);
+               assert_eq!(node_txn[0].clone().input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
+
+               timeout_tx = node_txn[1].clone();
                node_txn.clear();
        }
 
@@ -7374,11 +7376,11 @@ fn test_set_outpoints_partial_claiming() {
        let partial_claim_tx = {
                let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_txn.len(), 3);
-               check_spends!(node_txn[1], node_txn[0]);
-               check_spends!(node_txn[2], node_txn[0]);
+               check_spends!(node_txn[0], node_txn[2]);
+               check_spends!(node_txn[1], node_txn[2]);
+               assert_eq!(node_txn[0].input.len(), 1);
                assert_eq!(node_txn[1].input.len(), 1);
-               assert_eq!(node_txn[2].input.len(), 1);
-               node_txn[1].clone()
+               node_txn[0].clone()
        };
 
        // Broadcast partial claim on node A, should regenerate a claiming tx with HTLC dropped
index d0d1f29e691f5ea5118d0a06b2e5bf4030fd73aa..2270cdb7c6683e8f82ba71f9bbdb2ed235355a3d 100644 (file)
@@ -52,7 +52,7 @@ enum OnchainEvent {
 pub struct ClaimTxBumpMaterial {
        // At every block tick, used to check if pending claiming tx is taking too
        // much time for confirmation and we need to bump it.
-       height_timer: u32,
+       height_timer: Option<u32>,
        // Tracked in case of reorg to wipe out now-superflous bump material
        feerate_previous: u64,
        // Soonest timelocks among set of outpoints claimed, used to compute
@@ -64,7 +64,7 @@ pub struct ClaimTxBumpMaterial {
 
 impl Writeable for ClaimTxBumpMaterial  {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
-               writer.write_all(&byte_utils::be32_to_array(self.height_timer))?;
+               self.height_timer.write(writer)?;
                writer.write_all(&byte_utils::be64_to_array(self.feerate_previous))?;
                writer.write_all(&byte_utils::be32_to_array(self.soonest_timelock))?;
                writer.write_all(&byte_utils::be64_to_array(self.per_input_material.len() as u64))?;
@@ -357,7 +357,7 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 
        /// Lightning security model (i.e being able to redeem/timeout HTLC or penalize coutnerparty onchain) lays on the assumption of claim transactions getting confirmed before timelock expiration
        /// (CSV or CLTV following cases). In case of high-fee spikes, claim tx may stuck in the mempool, so you need to bump its feerate quickly using Replace-By-Fee or Child-Pay-For-Parent.
-       fn generate_claim_tx<F: Deref>(&self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: F) -> Option<(u32, u64, Transaction)>
+       fn generate_claim_tx<F: Deref>(&self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: F) -> Option<(Option<u32>, u64, Transaction)>
                where F::Target: FeeEstimator
        {
                if cached_claim_datas.per_input_material.len() == 0 { return None } // But don't prune pending claiming request yet, we may have to resurrect HTLCs
@@ -422,9 +422,10 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 
                // Compute new height timer to decide when we need to regenerate a new bumped version of the claim tx (if we
                // didn't receive confirmation of it before, or not enough reorg-safe depth on top of it).
-               let new_timer = Self::get_height_timer(height, cached_claim_datas.soonest_timelock);
+               let new_timer = Some(Self::get_height_timer(height, cached_claim_datas.soonest_timelock));
                let mut inputs_witnesses_weight = 0;
                let mut amt = 0;
+               let mut dynamic_fee = true;
                for per_outp_material in cached_claim_datas.per_input_material.values() {
                        match per_outp_material {
                                &InputMaterial::Revoked { ref witness_script, ref is_htlc, ref amount, .. } => {
@@ -436,71 +437,99 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
                                        amt += *amount;
                                },
                                &InputMaterial::LocalHTLC { .. } => { return None; }
-                       }
-               }
-
-               let predicted_weight = bumped_tx.get_weight() + inputs_witnesses_weight;
-               let mut new_feerate;
-               // If old feerate is 0, first iteration of this claim, use normal fee calculation
-               if cached_claim_datas.feerate_previous != 0 {
-                       if let Some((new_fee, feerate)) = RBF_bump!(amt, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
-                               // If new computed fee is superior at the whole claimable amount burn all in fees
-                               if new_fee > amt {
-                                       bumped_tx.output[0].value = 0;
-                               } else {
-                                       bumped_tx.output[0].value = amt - new_fee;
+                               &InputMaterial::Funding { .. } => {
+                                       dynamic_fee = false;
                                }
-                               new_feerate = feerate;
-                       } else { return None; }
-               } else {
-                       if subtract_high_prio_fee!(self, fee_estimator, amt, predicted_weight, new_feerate) {
-                               bumped_tx.output[0].value = amt;
-                       } else { return None; }
+                       }
                }
-               assert!(new_feerate != 0);
 
-               for (i, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() {
-                       match per_outp_material {
-                               &InputMaterial::Revoked { ref witness_script, ref pubkey, ref key, ref is_htlc, ref amount } => {
-                                       let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
-                                       let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &witness_script, *amount)[..]);
-                                       let sig = self.secp_ctx.sign(&sighash, &key);
-                                       bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
-                                       bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
-                                       if *is_htlc {
-                                               bumped_tx.input[i].witness.push(pubkey.unwrap().clone().serialize().to_vec());
+               if dynamic_fee {
+                       let predicted_weight = bumped_tx.get_weight() + inputs_witnesses_weight;
+                       let mut new_feerate;
+                       // If old feerate is 0, first iteration of this claim, use normal fee calculation
+                       if cached_claim_datas.feerate_previous != 0 {
+                               if let Some((new_fee, feerate)) = RBF_bump!(amt, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
+                                       // If new computed fee is superior at the whole claimable amount burn all in fees
+                                       if new_fee > amt {
+                                               bumped_tx.output[0].value = 0;
                                        } else {
-                                               bumped_tx.input[i].witness.push(vec!(1));
+                                               bumped_tx.output[0].value = amt - new_fee;
                                        }
-                                       bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
-                                       log_trace!(self, "Going to broadcast Penalty Transaction {} claiming revoked {} output {} from {} with new feerate {}...", bumped_tx.txid(), if !is_htlc { "to_local" } else if HTLCType::scriptlen_to_htlctype(witness_script.len()) == Some(HTLCType::OfferedHTLC) { "offered" } else if HTLCType::scriptlen_to_htlctype(witness_script.len()) == Some(HTLCType::AcceptedHTLC) { "received" } else { "" }, outp.vout, outp.txid, new_feerate);
-                               },
-                               &InputMaterial::RemoteHTLC { ref witness_script, ref key, ref preimage, ref amount, ref locktime } => {
-                                       if !preimage.is_some() { bumped_tx.lock_time = *locktime }; // Right now we don't aggregate time-locked transaction, if we do we should set lock_time before to avoid breaking hash computation
-                                       let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
-                                       let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &witness_script, *amount)[..]);
-                                       let sig = self.secp_ctx.sign(&sighash, &key);
-                                       bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
-                                       bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
-                                       if let &Some(preimage) = preimage {
-                                               bumped_tx.input[i].witness.push(preimage.clone().0.to_vec());
-                                       } else {
-                                               bumped_tx.input[i].witness.push(vec![]);
+                                       new_feerate = feerate;
+                               } else { return None; }
+                       } else {
+                               if subtract_high_prio_fee!(self, fee_estimator, amt, predicted_weight, new_feerate) {
+                                       bumped_tx.output[0].value = amt;
+                               } else { return None; }
+                       }
+                       assert!(new_feerate != 0);
+
+                       for (i, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() {
+                               match per_outp_material {
+                                       &InputMaterial::Revoked { ref witness_script, ref pubkey, ref key, ref is_htlc, ref amount } => {
+                                               let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
+                                               let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &witness_script, *amount)[..]);
+                                               let sig = self.secp_ctx.sign(&sighash, &key);
+                                               bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
+                                               bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
+                                               if *is_htlc {
+                                                       bumped_tx.input[i].witness.push(pubkey.unwrap().clone().serialize().to_vec());
+                                               } else {
+                                                       bumped_tx.input[i].witness.push(vec!(1));
+                                               }
+                                               bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
+                                               log_trace!(self, "Going to broadcast Penalty Transaction {} claiming revoked {} output {} from {} with new feerate {}...", bumped_tx.txid(), if !is_htlc { "to_local" } else if HTLCType::scriptlen_to_htlctype(witness_script.len()) == Some(HTLCType::OfferedHTLC) { "offered" } else if HTLCType::scriptlen_to_htlctype(witness_script.len()) == Some(HTLCType::AcceptedHTLC) { "received" } else { "" }, outp.vout, outp.txid, new_feerate);
+                                       },
+                                       &InputMaterial::RemoteHTLC { ref witness_script, ref key, ref preimage, ref amount, ref locktime } => {
+                                               if !preimage.is_some() { bumped_tx.lock_time = *locktime }; // Right now we don't aggregate time-locked transaction, if we do we should set lock_time before to avoid breaking hash computation
+                                               let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
+                                               let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &witness_script, *amount)[..]);
+                                               let sig = self.secp_ctx.sign(&sighash, &key);
+                                               bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
+                                               bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
+                                               if let &Some(preimage) = preimage {
+                                                       bumped_tx.input[i].witness.push(preimage.clone().0.to_vec());
+                                               } else {
+                                                       bumped_tx.input[i].witness.push(vec![]);
+                                               }
+                                               bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
+                                               log_trace!(self, "Going to broadcast Claim Transaction {} claiming remote {} htlc output {} from {} with new feerate {}...", bumped_tx.txid(), if preimage.is_some() { "offered" } else { "received" }, outp.vout, outp.txid, new_feerate);
+                                       },
+                                       _ => unreachable!()
+                               }
+                       }
+                       log_trace!(self, "...with timer {}", new_timer.unwrap());
+                       assert!(predicted_weight >= bumped_tx.get_weight());
+                       return Some((new_timer, new_feerate, bumped_tx))
+               } else {
+                       for (_, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() {
+                               match per_outp_material {
+                                       &InputMaterial::LocalHTLC { .. } => {
+                                               //TODO : Given that Local Commitment Transaction and HTLC-Timeout/HTLC-Success are counter-signed by peer, we can't
+                                               // RBF them. Need a Lightning specs change and package relay modification :
+                                               // https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
+                                               return None;
+                                       },
+                                       &InputMaterial::Funding { ref channel_value } => {
+                                               if self.local_commitment.is_some() {
+                                                       let mut local_commitment = self.local_commitment.clone().unwrap();
+                                                       self.key_storage.sign_local_commitment(&mut local_commitment, &self.funding_redeemscript, *channel_value, &self.secp_ctx);
+                                                       let signed_tx = local_commitment.with_valid_witness().clone();
+                                                       let mut amt_outputs = 0;
+                                                       for outp in signed_tx.output.iter() {
+                                                               amt_outputs += outp.value;
+                                                       }
+                                                       let feerate = (channel_value - amt_outputs) * 1000 / signed_tx.get_weight() as u64;
+                                                       // Timer set to $NEVER given we can't bump tx without anchor outputs
+                                                       log_trace!(self, "Going to broadcast Local Transaction {} claiming funding output {} from {}...", signed_tx.txid(), outp.vout, outp.txid);
+                                                       return Some((None, feerate, signed_tx));
+                                               }
                                        }
-                                       bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
-                                       log_trace!(self, "Going to broadcast Claim Transaction {} claiming remote {} htlc output {} from {} with new feerate {}...", bumped_tx.txid(), if preimage.is_some() { "offered" } else { "received" }, outp.vout, outp.txid, new_feerate);
-                               },
-                               &InputMaterial::LocalHTLC { .. } => {
-                                       //TODO : Given that Local Commitment Transaction and HTLC-Timeout/HTLC-Success are counter-signed by peer, we can't
-                                       // RBF them. Need a Lightning specs change and package relay modification :
-                                       // https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
-                                       return None;
+                                       _ => unreachable!()
                                }
                        }
                }
-               log_trace!(self, "...with timer {}", new_timer);
-               assert!(predicted_weight >= bumped_tx.get_weight());
-               Some((new_timer, new_feerate, bumped_tx))
+               None
        }
 
        pub(super) fn block_connected<B: Deref, F: Deref>(&mut self, txn_matched: &[&Transaction], claimable_outpoints: Vec<ClaimRequest>, height: u32, broadcaster: B, fee_estimator: F)
@@ -535,7 +564,7 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
                // Generate claim transactions and track them to bump if necessary at
                // height timer expiration (i.e in how many blocks we're going to take action).
                for claim in new_claims {
-                       let mut claim_material = ClaimTxBumpMaterial { height_timer: 0, feerate_previous: 0, soonest_timelock: claim.0, per_input_material: claim.1.clone() };
+                       let mut claim_material = ClaimTxBumpMaterial { height_timer: None, feerate_previous: 0, soonest_timelock: claim.0, per_input_material: claim.1.clone() };
                        if let Some((new_timer, new_feerate, tx)) = self.generate_claim_tx(height, &claim_material, &*fee_estimator) {
                                claim_material.height_timer = new_timer;
                                claim_material.feerate_previous = new_feerate;
@@ -653,8 +682,10 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 
                // Check if any pending claim request must be rescheduled
                for (first_claim_txid, ref claim_data) in self.pending_claim_requests.iter() {
-                       if claim_data.height_timer == height {
-                               bump_candidates.insert(*first_claim_txid);
+                       if let Some(h) = claim_data.height_timer {
+                               if h == height {
+                                       bump_candidates.insert(*first_claim_txid);
+                               }
                        }
                }