From: Antoine Riard Date: Tue, 3 Mar 2020 23:51:50 +0000 (-0500) Subject: Move local commitment tx generation in OnchainTxHandler X-Git-Tag: v0.0.12~84^2~13 X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=2be1f72005d407ed0183b383deeaca6b88becc31;p=rust-lightning Move local commitment tx generation in OnchainTxHandler 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. --- diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 2fa416edb..b3cff4c2b 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -443,6 +443,9 @@ pub(crate) enum InputMaterial { sigs: (Signature, Signature), preimage: Option, 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 ChannelMonitor { 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)); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 62cae0269..0c6bac59c 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -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 diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index d0d1f29e6..2270cdb7c 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -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, // 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(&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 OnchainTxHandler { /// 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(&self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: F) -> Option<(u32, u64, Transaction)> + fn generate_claim_tx(&self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: F) -> Option<(Option, 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 OnchainTxHandler { // 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 OnchainTxHandler { 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(&mut self, txn_matched: &[&Transaction], claimable_outpoints: Vec, height: u32, broadcaster: B, fee_estimator: F) @@ -535,7 +564,7 @@ impl OnchainTxHandler { // 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 OnchainTxHandler { // 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); + } } }