From bf6b64ac0fdfcc8cbeabb2ee9fb0b0fa6fea9d23 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Tue, 3 Mar 2020 17:35:36 -0500 Subject: [PATCH 1/7] Remove duplicata for local commitment+HTLC txn Previously, we would regenerate this class of txn twice due to block-rescan triggered by new watching outputs registered. This commmit doesn't change behavior, it only tweaks TestBroadcaster to ensure we modify cleanly tests anticipating next commit refactor. --- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests.rs | 60 +++++++---------------- lightning/src/util/test_utils.rs | 15 ++++++ 3 files changed, 33 insertions(+), 44 deletions(-) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index bd8394d1472..67c3b666946 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -974,7 +974,7 @@ pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: pub fn create_chanmon_cfgs(node_count: usize) -> Vec { let mut chan_mon_cfgs = Vec::new(); for _ in 0..node_count { - let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())}; + let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), broadcasted_txn: Mutex::new(HashMap::new())}; let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 }; chan_mon_cfgs.push(TestChanMonCfg{ tx_broadcaster, fee_estimator }); } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index bd4244091c7..487d7e6a49f 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2327,30 +2327,13 @@ fn claim_htlc_outputs_single_tx() { } let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(node_txn.len(), 21); + assert_eq!(node_txn.len(), 9); // ChannelMonitor: justice tx revoked offered htlc, justice tx revoked received htlc, justice tx revoked to_local (3) // ChannelManager: local commmitment + local HTLC-timeout (2) - // ChannelMonitor: bumped justice tx (4), after one increase, bumps on HTLC aren't generated not being substantial anymore - // ChannelMonito r: local commitment + local HTLC-timeout (14) - - assert_eq!(node_txn[0], node_txn[5]); - assert_eq!(node_txn[0], node_txn[7]); - assert_eq!(node_txn[0], node_txn[9]); - assert_eq!(node_txn[0], node_txn[13]); - assert_eq!(node_txn[0], node_txn[15]); - assert_eq!(node_txn[0], node_txn[17]); - assert_eq!(node_txn[0], node_txn[19]); - - assert_eq!(node_txn[1], node_txn[6]); - assert_eq!(node_txn[1], node_txn[8]); - assert_eq!(node_txn[1], node_txn[10]); - assert_eq!(node_txn[1], node_txn[14]); - assert_eq!(node_txn[1], node_txn[16]); - assert_eq!(node_txn[1], node_txn[18]); - assert_eq!(node_txn[1], node_txn[20]); - - - // Check the pair local commitment and HTLC-timeout broadcast due to HTLC expiration and present 8 times (rebroadcast at every block from 200 to 206) + // ChannelMonitor: bumped justice tx (2), after one increase, bumps on HTLC aren't generated not being substantial anymore, bump on revoked to_local isn't generated due to more room for expiration + // ChannelMonitor: local commitment + local HTLC-timeout (2) + + // Check the pair local commitment and HTLC-timeout broadcast due to HTLC expiration assert_eq!(node_txn[0].input.len(), 1); check_spends!(node_txn[0], chan_1.3); assert_eq!(node_txn[1].input.len(), 1); @@ -2429,12 +2412,10 @@ fn test_htlc_on_chain_success() { nodes[2].block_notifier.block_connected(&Block { header, txdata: vec![commitment_tx[0].clone()]}, 1); check_closed_broadcast!(nodes[2], false); - let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx, 2*htlc-success tx), ChannelMonitor : 4 (2*2 * HTLC-Success tx) - assert_eq!(node_txn.len(), 7); + let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 3 (commitment tx, 2*htlc-success tx), ChannelMonitor : 4 (2 * HTLC-Success tx) + assert_eq!(node_txn.len(), 5); assert_eq!(node_txn[0], node_txn[3]); assert_eq!(node_txn[1], node_txn[4]); - assert_eq!(node_txn[0], node_txn[5]); - assert_eq!(node_txn[1], node_txn[6]); assert_eq!(node_txn[2], commitment_tx[0]); check_spends!(node_txn[0], commitment_tx[0]); check_spends!(node_txn[1], commitment_tx[0]); @@ -2473,15 +2454,11 @@ fn test_htlc_on_chain_success() { macro_rules! check_tx_local_broadcast { ($node: expr, $htlc_offered: expr, $commitment_tx: expr, $chan_tx: expr) => { { let mut node_txn = $node.tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(node_txn.len(), if $htlc_offered { 7 } else { 5 }); + assert_eq!(node_txn.len(), 5); // Node[1]: ChannelManager: 3 (commitment tx, 2*HTLC-Timeout tx), ChannelMonitor: 2 (timeout tx) - // Node[0]: ChannelManager: 3 (commtiemtn tx, 2*HTLC-Timeout tx), ChannelMonitor: 2 HTLC-timeout * 2 (block-rescan) - check_spends!(node_txn[0], $commitment_tx); - check_spends!(node_txn[1], $commitment_tx); - if $htlc_offered { - assert_eq!(node_txn[0], node_txn[5]); - assert_eq!(node_txn[1], node_txn[6]); - } + // Node[0]: ChannelManager: 3 (commtiemtn tx, 2*HTLC-Timeout tx), ChannelMonitor: 2 HTLC-timeout + check_spends!(node_txn[0], $commitment_tx.clone()); + check_spends!(node_txn[1], $commitment_tx.clone()); assert_ne!(node_txn[0].lock_time, 0); assert_ne!(node_txn[1].lock_time, 0); if $htlc_offered { @@ -2615,11 +2592,9 @@ fn test_htlc_on_chain_timeout() { let timeout_tx; { let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(node_txn.len(), 7); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : (local commitment tx + HTLC-timeout) * 2 (block-rescan), timeout tx + assert_eq!(node_txn.len(), 5); // ChannelManager : 2 (commitment tx, HTLC-Timeout tx), ChannelMonitor : (local commitment tx + HTLC-timeout), timeout tx assert_eq!(node_txn[0], node_txn[3]); - assert_eq!(node_txn[0], node_txn[5]); assert_eq!(node_txn[1], node_txn[4]); - assert_eq!(node_txn[1], node_txn[6]); check_spends!(node_txn[2], commitment_tx[0]); assert_eq!(node_txn[2].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); @@ -4286,9 +4261,8 @@ fn test_onchain_to_onchain_claim() { check_closed_broadcast!(nodes[2], false); let c_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); // ChannelManager : 2 (commitment tx, HTLC-Success tx), ChannelMonitor : 1 (HTLC-Success tx) - assert_eq!(c_txn.len(), 4); + assert_eq!(c_txn.len(), 3); assert_eq!(c_txn[0], c_txn[2]); - assert_eq!(c_txn[0], c_txn[3]); assert_eq!(commitment_tx[0], c_txn[1]); check_spends!(c_txn[1], chan_2.3); check_spends!(c_txn[2], c_txn[1]); @@ -4401,11 +4375,11 @@ fn test_duplicate_payment_hash_one_failure_one_success() { _ => panic!("Unexepected event"), } let htlc_success_txn: Vec<_> = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); - assert_eq!(htlc_success_txn.len(), 7); - check_spends!(htlc_success_txn[2], chan_2.3); + assert_eq!(htlc_success_txn.len(), 5); // ChannelMonitor: HTLC-Success txn (*2 due to 2-HTLC outputs), ChannelManager: local commitment tx + HTLC-Success txn (*2 due to 2-HTLC outputs) + check_spends!(htlc_success_txn[2], chan_2.3.clone()); check_spends!(htlc_success_txn[3], htlc_success_txn[2]); check_spends!(htlc_success_txn[4], htlc_success_txn[2]); - assert_eq!(htlc_success_txn[0], htlc_success_txn[5]); + assert_eq!(htlc_success_txn[0], htlc_success_txn[3]); assert_eq!(htlc_success_txn[0].input.len(), 1); assert_eq!(htlc_success_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); assert_eq!(htlc_success_txn[1], htlc_success_txn[4]); @@ -6607,7 +6581,7 @@ fn test_data_loss_protect() { let logger: Arc = Arc::new(test_utils::TestLogger::with_id(format!("node {}", 0))); let mut chan_monitor = <(Sha256dHash, ChannelMonitor)>::read(&mut ::std::io::Cursor::new(previous_chan_monitor_state.0), Arc::clone(&logger)).unwrap().1; let chain_monitor = Arc::new(ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&logger))); - tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())}; + tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), broadcasted_txn: Mutex::new(HashMap::new())}; fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 }; keys_manager = test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::clone(&logger)); monitor = test_utils::TestChannelMonitor::new(chain_monitor.clone(), &tx_broadcaster, logger.clone(), &fee_estimator); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 78b81fa637b..ae45473a94a 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -108,9 +108,24 @@ impl<'a> channelmonitor::ManyChannelMonitor for TestChanne pub struct TestBroadcaster { pub txn_broadcasted: Mutex>, + pub broadcasted_txn: Mutex> // Temporary field while refactoring out tx duplication } impl chaininterface::BroadcasterInterface for TestBroadcaster { fn broadcast_transaction(&self, tx: &Transaction) { + let mut already = false; + { + if let Some(counter) = self.broadcasted_txn.lock().unwrap().get_mut(&tx.txid()) { + match counter { + 0 => { *counter = 1; already = true }, // We still authorize at least 2 duplicata for a given TXID to account ChannelManager/ChannelMonitor broadcast + 1 => return, + _ => panic!() + } + } + } + if !already { + self.broadcasted_txn.lock().unwrap().insert(tx.txid(), 0); + } + print!("\nFRESH BROADCAST {}\n\n", tx.txid()); self.txn_broadcasted.lock().unwrap().push(tx.clone()); } } From ee5949ead571ef45ea4cee741370b307666b1dfc Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 18 Mar 2020 01:15:28 -0400 Subject: [PATCH 2/7] Remove Watchtower mode from Storage enum and make it a struct Watchtower will be supported through external signer interface where a watchtower implementation may differ from a local one by the scope of key access and pre-signed datas. --- lightning/src/ln/channelmonitor.rs | 495 ++++++++++------------------- lightning/src/util/macro_logger.rs | 9 +- 2 files changed, 177 insertions(+), 327 deletions(-) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 514a95d27db..7cfeab71a9e 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -281,22 +281,15 @@ impl return Err(MonitorUpdateError("Channel monitor for given key is already present")), hash_map::Entry::Vacant(e) => e, }; - match monitor.key_storage { - Storage::Local { ref funding_info, .. } => { - match funding_info { - &None => { - return Err(MonitorUpdateError("Try to update a useless monitor without funding_txo !")); - }, - &Some((ref outpoint, ref script)) => { - log_trace!(self, "Got new Channel Monitor for channel {}", log_bytes!(outpoint.to_channel_id()[..])); - self.chain_monitor.install_watch_tx(&outpoint.txid, script); - self.chain_monitor.install_watch_outpoint((outpoint.txid, outpoint.index as u32), script); - }, - } + match monitor.key_storage.funding_info { + None => { + return Err(MonitorUpdateError("Try to update a useless monitor without funding_txo !")); + }, + Some((ref outpoint, ref script)) => { + log_trace!(self, "Got new Channel Monitor for channel {}", log_bytes!(outpoint.to_channel_id()[..])); + self.chain_monitor.install_watch_tx(&outpoint.txid, script); + self.chain_monitor.install_watch_outpoint((outpoint.txid, outpoint.index as u32), script); }, - Storage::Watchtower { .. } => { - self.chain_monitor.watch_all_txn(); - } } for (txid, outputs) in monitor.get_outputs_to_watch().iter() { for (idx, script) in outputs.iter().enumerate() { @@ -389,45 +382,23 @@ pub(crate) const LATENCY_GRACE_PERIOD_BLOCKS: u32 = 3; /// keeping bumping another claim tx to solve the outpoint. pub(crate) const ANTI_REORG_DELAY: u32 = 6; -enum Storage { - Local { - keys: ChanSigner, - funding_key: SecretKey, - revocation_base_key: SecretKey, - htlc_base_key: SecretKey, - delayed_payment_base_key: SecretKey, - payment_base_key: SecretKey, - shutdown_pubkey: PublicKey, - funding_info: Option<(OutPoint, Script)>, - current_remote_commitment_txid: Option, - prev_remote_commitment_txid: Option, - }, - Watchtower { - revocation_base_key: PublicKey, - htlc_base_key: PublicKey, - } +struct Storage { + keys: ChanSigner, + funding_key: SecretKey, + revocation_base_key: SecretKey, + htlc_base_key: SecretKey, + delayed_payment_base_key: SecretKey, + payment_base_key: SecretKey, + shutdown_pubkey: PublicKey, + funding_info: Option<(OutPoint, Script)>, + current_remote_commitment_txid: Option, + prev_remote_commitment_txid: Option, } #[cfg(any(test, feature = "fuzztarget"))] impl PartialEq for Storage { fn eq(&self, other: &Self) -> bool { - match *self { - Storage::Local { ref keys, .. } => { - let k = keys; - match *other { - Storage::Local { ref keys, .. } => keys.pubkeys() == k.pubkeys(), - Storage::Watchtower { .. } => false, - } - }, - Storage::Watchtower {ref revocation_base_key, ref htlc_base_key} => { - let (rbk, hbk) = (revocation_base_key, htlc_base_key); - match *other { - Storage::Local { .. } => false, - Storage::Watchtower {ref revocation_base_key, ref htlc_base_key} => - revocation_base_key == rbk && htlc_base_key == hbk, - } - }, - } + self.keys.pubkeys() == other.keys.pubkeys() } } @@ -855,31 +826,25 @@ impl ChannelMonitor { // Set in initial Channel-object creation, so should always be set by now: U48(self.commitment_transaction_number_obscure_factor).write(writer)?; - match self.key_storage { - Storage::Local { ref keys, ref funding_key, ref revocation_base_key, ref htlc_base_key, ref delayed_payment_base_key, ref payment_base_key, ref shutdown_pubkey, ref funding_info, ref current_remote_commitment_txid, ref prev_remote_commitment_txid } => { - writer.write_all(&[0; 1])?; - keys.write(writer)?; - writer.write_all(&funding_key[..])?; - writer.write_all(&revocation_base_key[..])?; - writer.write_all(&htlc_base_key[..])?; - writer.write_all(&delayed_payment_base_key[..])?; - writer.write_all(&payment_base_key[..])?; - writer.write_all(&shutdown_pubkey.serialize())?; - match funding_info { - &Some((ref outpoint, ref script)) => { - writer.write_all(&outpoint.txid[..])?; - writer.write_all(&byte_utils::be16_to_array(outpoint.index))?; - script.write(writer)?; - }, - &None => { - debug_assert!(false, "Try to serialize a useless Local monitor !"); - }, - } - current_remote_commitment_txid.write(writer)?; - prev_remote_commitment_txid.write(writer)?; + self.key_storage.keys.write(writer)?; + writer.write_all(&self.key_storage.funding_key[..])?; + writer.write_all(&self.key_storage.revocation_base_key[..])?; + writer.write_all(&self.key_storage.htlc_base_key[..])?; + writer.write_all(&self.key_storage.delayed_payment_base_key[..])?; + writer.write_all(&self.key_storage.payment_base_key[..])?; + writer.write_all(&self.key_storage.shutdown_pubkey.serialize())?; + match self.key_storage.funding_info { + Some((ref outpoint, ref script)) => { + writer.write_all(&outpoint.txid[..])?; + writer.write_all(&byte_utils::be16_to_array(outpoint.index))?; + script.write(writer)?; + }, + None => { + debug_assert!(false, "Try to serialize a useless Local monitor !"); }, - Storage::Watchtower { .. } => unimplemented!(), } + self.key_storage.current_remote_commitment_txid.write(writer)?; + self.key_storage.prev_remote_commitment_txid.write(writer)?; writer.write_all(&self.their_htlc_base_key.as_ref().unwrap().serialize())?; writer.write_all(&self.their_delayed_payment_base_key.as_ref().unwrap().serialize())?; @@ -1086,7 +1051,7 @@ impl ChannelMonitor { latest_update_id: 0, commitment_transaction_number_obscure_factor, - key_storage: Storage::Local { + key_storage: Storage { keys, funding_key, revocation_base_key, @@ -1143,11 +1108,9 @@ impl ChannelMonitor { // Prune HTLCs from the previous remote commitment tx so we don't generate failure/fulfill // events for now-revoked/fulfilled HTLCs. - if let Storage::Local { ref mut prev_remote_commitment_txid, .. } = self.key_storage { - if let Some(txid) = prev_remote_commitment_txid.take() { - for &mut (_, ref mut source) in self.remote_claimable_outpoints.get_mut(&txid).unwrap() { - *source = None; - } + if let Some(txid) = self.key_storage.prev_remote_commitment_txid.take() { + for &mut (_, ref mut source) in self.remote_claimable_outpoints.get_mut(&txid).unwrap() { + *source = None; } } @@ -1202,10 +1165,8 @@ impl ChannelMonitor { let new_txid = unsigned_commitment_tx.txid(); log_trace!(self, "Tracking new remote commitment transaction with txid {} at commitment number {} with {} HTLC outputs", new_txid, commitment_number, htlc_outputs.len()); log_trace!(self, "New potential remote commitment transaction: {}", encode::serialize_hex(unsigned_commitment_tx)); - if let Storage::Local { ref mut current_remote_commitment_txid, ref mut prev_remote_commitment_txid, .. } = self.key_storage { - *prev_remote_commitment_txid = current_remote_commitment_txid.take(); - *current_remote_commitment_txid = Some(new_txid); - } + self.key_storage.prev_remote_commitment_txid = self.key_storage.current_remote_commitment_txid.take(); + self.key_storage.current_remote_commitment_txid = Some(new_txid); self.remote_claimable_outpoints.insert(new_txid, htlc_outputs); self.current_remote_commitment_number = commitment_number; //TODO: Merge this into the other per-remote-transaction output storage stuff @@ -1230,18 +1191,13 @@ impl ChannelMonitor { } pub(super) fn provide_rescue_remote_commitment_tx_info(&mut self, their_revocation_point: PublicKey) { - match self.key_storage { - Storage::Local { ref payment_base_key, ref keys, .. } => { - if let Ok(payment_key) = chan_utils::derive_public_key(&self.secp_ctx, &their_revocation_point, &keys.pubkeys().payment_basepoint) { - let to_remote_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0) - .push_slice(&Hash160::hash(&payment_key.serialize())[..]) - .into_script(); - if let Ok(to_remote_key) = chan_utils::derive_private_key(&self.secp_ctx, &their_revocation_point, &payment_base_key) { - self.to_remote_rescue = Some((to_remote_script, to_remote_key)); - } - } - }, - Storage::Watchtower { .. } => {} + if let Ok(payment_key) = chan_utils::derive_public_key(&self.secp_ctx, &their_revocation_point, &self.key_storage.keys.pubkeys().payment_basepoint) { + let to_remote_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0) + .push_slice(&Hash160::hash(&payment_key.serialize())[..]) + .into_script(); + if let Ok(to_remote_key) = chan_utils::derive_private_key(&self.secp_ctx, &their_revocation_point, &self.key_storage.payment_base_key) { + self.to_remote_rescue = Some((to_remote_script, to_remote_key)); + } } } @@ -1329,17 +1285,10 @@ impl ChannelMonitor { /// Gets the funding transaction outpoint of the channel this ChannelMonitor is monitoring for. pub fn get_funding_txo(&self) -> Option { - match self.key_storage { - Storage::Local { ref funding_info, .. } => { - match funding_info { - &Some((outpoint, _)) => Some(outpoint), - &None => None - } - }, - Storage::Watchtower { .. } => { - return None; - } + if let Some((outp, _)) = self.key_storage.funding_info { + return Some(outp) } + None } /// Gets a list of txids, with their output scripts (in the order they appear in the @@ -1430,18 +1379,11 @@ impl ChannelMonitor { if commitment_number >= self.get_min_seen_secret() { let secret = self.get_secret(commitment_number).unwrap(); let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret)); - let (revocation_pubkey, revocation_key, b_htlc_key, local_payment_key) = match self.key_storage { - Storage::Local { ref keys, ref revocation_base_key, ref payment_base_key, .. } => { - let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key); - (ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &keys.pubkeys().revocation_basepoint)), - ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key)), - ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &keys.pubkeys().htlc_basepoint)), - Some(ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, &per_commitment_point, &payment_base_key)))) - }, - Storage::Watchtower { .. } => { - unimplemented!() - }, - }; + let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key); + let revocation_pubkey = ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &self.key_storage.keys.pubkeys().revocation_basepoint)); + let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &self.key_storage.revocation_base_key)); + let b_htlc_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &self.key_storage.keys.pubkeys().htlc_basepoint)); + let local_payment_key = Some(ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, &per_commitment_point, &self.key_storage.payment_base_key))); let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key.unwrap())); let a_htlc_key = match self.their_htlc_base_key { None => return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_outputs), @@ -1522,13 +1464,11 @@ impl ChannelMonitor { } } } - if let Storage::Local { ref current_remote_commitment_txid, ref prev_remote_commitment_txid, .. } = self.key_storage { - if let &Some(ref txid) = current_remote_commitment_txid { - check_htlc_fails!(txid, "current"); - } - if let &Some(ref txid) = prev_remote_commitment_txid { - check_htlc_fails!(txid, "remote"); - } + if let Some(ref txid) = self.key_storage.current_remote_commitment_txid { + check_htlc_fails!(txid, "current"); + } + if let Some(ref txid) = self.key_storage.prev_remote_commitment_txid { + check_htlc_fails!(txid, "remote"); } // No need to check local commitment txn, symmetric HTLCSource must be present as per-htlc data on remote commitment tx } @@ -1586,13 +1526,11 @@ impl ChannelMonitor { } } } - if let Storage::Local { ref current_remote_commitment_txid, ref prev_remote_commitment_txid, .. } = self.key_storage { - if let &Some(ref txid) = current_remote_commitment_txid { - check_htlc_fails!(txid, "current", 'current_loop); - } - if let &Some(ref txid) = prev_remote_commitment_txid { - check_htlc_fails!(txid, "previous", 'prev_loop); - } + if let Some(ref txid) = self.key_storage.current_remote_commitment_txid { + check_htlc_fails!(txid, "current", 'current_loop); + } + if let Some(ref txid) = self.key_storage.prev_remote_commitment_txid { + check_htlc_fails!(txid, "previous", 'prev_loop); } if let Some(revocation_points) = self.their_cur_revocation_points { @@ -1602,14 +1540,9 @@ impl ChannelMonitor { if revocation_points.0 == commitment_number + 1 { Some(point) } else { None } } else { None }; if let Some(revocation_point) = revocation_point_option { - let (revocation_pubkey, b_htlc_key, htlc_privkey) = match self.key_storage { - Storage::Local { ref keys, ref htlc_base_key, .. } => { - (ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, revocation_point, &keys.pubkeys().revocation_basepoint)), - ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &keys.pubkeys().htlc_basepoint)), - ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &htlc_base_key))) - }, - Storage::Watchtower { .. } => { unimplemented!() } - }; + let revocation_pubkey = ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, revocation_point, &self.key_storage.keys.pubkeys().revocation_basepoint)); + let b_htlc_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &self.key_storage.keys.pubkeys().htlc_basepoint)); + let htlc_privkey = ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, revocation_point, &self.key_storage.htlc_base_key)); let a_htlc_key = match self.their_htlc_base_key { None => return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_outputs), Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &their_htlc_base_key)), @@ -1618,17 +1551,12 @@ impl ChannelMonitor { // First, mark as spendable our to_remote output for (idx, outp) in tx.output.iter().enumerate() { if outp.script_pubkey.is_v0_p2wpkh() { - match self.key_storage { - Storage::Local { ref payment_base_key, .. } => { - if let Ok(local_key) = chan_utils::derive_private_key(&self.secp_ctx, &revocation_point, &payment_base_key) { - spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH { - outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, - key: local_key, - output: outp.clone(), - }); - } - }, - Storage::Watchtower { .. } => {} + if let Ok(local_key) = chan_utils::derive_private_key(&self.secp_ctx, &revocation_point, &self.key_storage.payment_base_key) { + spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH { + outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, + key: local_key, + output: outp.clone(), + }); } break; // Only to_remote ouput is claimable } @@ -1686,13 +1614,8 @@ impl ChannelMonitor { let secret = if let Some(secret) = self.get_secret(commitment_number) { secret } else { return (Vec::new(), None); }; let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret)); let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key); - let (revocation_pubkey, revocation_key) = match self.key_storage { - Storage::Local { ref keys, ref revocation_base_key, .. } => { - (ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &keys.pubkeys().revocation_basepoint)), - ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, revocation_base_key))) - }, - Storage::Watchtower { .. } => { unimplemented!() } - }; + let revocation_pubkey = ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &self.key_storage.keys.pubkeys().revocation_basepoint)); + let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &self.key_storage.revocation_base_key)); let delayed_key = match self.their_delayed_payment_base_key { None => return (Vec::new(), None), Some(their_delayed_payment_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &their_delayed_payment_base_key)), @@ -1733,46 +1656,44 @@ impl ChannelMonitor { } } - if let &Storage::Local { ref htlc_base_key, .. } = &self.key_storage { - for &(ref htlc, ref sigs, _) in local_tx.htlc_outputs.iter() { - if let Some(transaction_output_index) = htlc.transaction_output_index { - if let &Some(ref their_sig) = sigs { - if htlc.offered { - log_trace!(self, "Broadcasting HTLC-Timeout transaction against local commitment transactions"); - 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); + for &(ref htlc, ref sigs, _) in local_tx.htlc_outputs.iter() { + if let Some(transaction_output_index) = htlc.transaction_output_index { + if let &Some(ref their_sig) = sigs { + if htlc.offered { + log_trace!(self, "Broadcasting HTLC-Timeout transaction against local commitment transactions"); + 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); + let (our_sig, htlc_script) = match + chan_utils::sign_htlc_transaction(&mut htlc_timeout_tx, their_sig, &None, htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.key_storage.htlc_base_key, &self.secp_ctx) { + Ok(res) => res, + Err(_) => continue, + }; + + add_dynamic_output!(htlc_timeout_tx, 0); + let mut per_input_material = HashMap::with_capacity(1); + per_input_material.insert(htlc_timeout_tx.input[0].previous_output, InputMaterial::LocalHTLC { witness_script: htlc_script, sigs: (*their_sig, our_sig), preimage: None, amount: htlc.amount_msat / 1000}); + //TODO: with option_simplified_commitment track outpoint too + log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_timeout_tx.input[0].previous_output.vout, htlc_timeout_tx.input[0].previous_output.txid); + res.push(htlc_timeout_tx); + } else { + if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) { + log_trace!(self, "Broadcasting HTLC-Success transaction against local commitment transactions"); + 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); let (our_sig, htlc_script) = match - chan_utils::sign_htlc_transaction(&mut htlc_timeout_tx, their_sig, &None, htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, htlc_base_key, &self.secp_ctx) { + chan_utils::sign_htlc_transaction(&mut htlc_success_tx, their_sig, &Some(*payment_preimage), htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.key_storage.htlc_base_key, &self.secp_ctx) { Ok(res) => res, Err(_) => continue, }; - add_dynamic_output!(htlc_timeout_tx, 0); + add_dynamic_output!(htlc_success_tx, 0); let mut per_input_material = HashMap::with_capacity(1); - per_input_material.insert(htlc_timeout_tx.input[0].previous_output, InputMaterial::LocalHTLC { witness_script: htlc_script, sigs: (*their_sig, our_sig), preimage: None, amount: htlc.amount_msat / 1000}); + per_input_material.insert(htlc_success_tx.input[0].previous_output, InputMaterial::LocalHTLC { witness_script: htlc_script, sigs: (*their_sig, our_sig), preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000}); //TODO: with option_simplified_commitment track outpoint too - log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_timeout_tx.input[0].previous_output.vout, htlc_timeout_tx.input[0].previous_output.txid); - res.push(htlc_timeout_tx); - } else { - if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) { - log_trace!(self, "Broadcasting HTLC-Success transaction against local commitment transactions"); - 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); - let (our_sig, htlc_script) = match - chan_utils::sign_htlc_transaction(&mut htlc_success_tx, their_sig, &Some(*payment_preimage), htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, htlc_base_key, &self.secp_ctx) { - Ok(res) => res, - Err(_) => continue, - }; - - add_dynamic_output!(htlc_success_tx, 0); - let mut per_input_material = HashMap::with_capacity(1); - per_input_material.insert(htlc_success_tx.input[0].previous_output, InputMaterial::LocalHTLC { witness_script: htlc_script, sigs: (*their_sig, our_sig), preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000}); - //TODO: with option_simplified_commitment track outpoint too - log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_success_tx.input[0].previous_output.vout, htlc_success_tx.input[0].previous_output.txid); - res.push(htlc_success_tx); - } + log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_success_tx.input[0].previous_output.vout, htlc_success_tx.input[0].previous_output.txid); + res.push(htlc_success_tx); } - watch_outputs.push(local_tx.tx.without_valid_witness().output[transaction_output_index as usize].clone()); - } else { panic!("Should have sigs for non-dust local tx outputs!") } - } + } + watch_outputs.push(local_tx.tx.without_valid_witness().output[transaction_output_index as usize].clone()); + } else { panic!("Should have sigs for non-dust local tx outputs!") } } } @@ -1823,12 +1744,7 @@ impl ChannelMonitor { if let &mut Some(ref mut local_tx) = &mut self.current_local_signed_commitment_tx { if local_tx.txid == commitment_txid { - match self.key_storage { - Storage::Local { ref funding_key, .. } => { - local_tx.tx.add_local_sig(funding_key, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); - }, - _ => {}, - } + local_tx.tx.add_local_sig(&self.key_storage.funding_key, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); } } if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx { @@ -1836,23 +1752,13 @@ impl ChannelMonitor { is_local_tx = true; log_trace!(self, "Got latest local commitment tx broadcast, searching for available HTLCs to claim"); assert!(local_tx.tx.has_local_sig()); - match self.key_storage { - Storage::Local { ref delayed_payment_base_key, .. } => { - let mut res = self.broadcast_by_local_state(local_tx, delayed_payment_base_key); - append_onchain_update!(res); - }, - Storage::Watchtower { .. } => { } - } + let mut res = self.broadcast_by_local_state(local_tx, &self.key_storage.delayed_payment_base_key); + append_onchain_update!(res); } } if let &mut Some(ref mut local_tx) = &mut self.prev_local_signed_commitment_tx { if local_tx.txid == commitment_txid { - match self.key_storage { - Storage::Local { ref funding_key, .. } => { - local_tx.tx.add_local_sig(funding_key, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); - }, - _ => {}, - } + local_tx.tx.add_local_sig(&self.key_storage.funding_key, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); } } if let &Some(ref local_tx) = &self.prev_local_signed_commitment_tx { @@ -1860,13 +1766,8 @@ impl ChannelMonitor { is_local_tx = true; log_trace!(self, "Got previous local commitment tx broadcast, searching for available HTLCs to claim"); assert!(local_tx.tx.has_local_sig()); - match self.key_storage { - Storage::Local { ref delayed_payment_base_key, .. } => { - let mut res = self.broadcast_by_local_state(local_tx, delayed_payment_base_key); - append_onchain_update!(res); - }, - Storage::Watchtower { .. } => { } - } + let mut res = self.broadcast_by_local_state(local_tx, &self.key_storage.delayed_payment_base_key); + append_onchain_update!(res); } } @@ -1897,22 +1798,14 @@ impl ChannelMonitor { /// Generate a spendable output event when closing_transaction get registered onchain. fn check_spend_closing_transaction(&self, tx: &Transaction) -> Option { if tx.input[0].sequence == 0xFFFFFFFF && !tx.input[0].witness.is_empty() && tx.input[0].witness.last().unwrap().len() == 71 { - match self.key_storage { - Storage::Local { ref shutdown_pubkey, .. } => { - let our_channel_close_key_hash = Hash160::hash(&shutdown_pubkey.serialize()); - let shutdown_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_close_key_hash[..]).into_script(); - for (idx, output) in tx.output.iter().enumerate() { - if shutdown_script == output.script_pubkey { - return Some(SpendableOutputDescriptor::StaticOutput { - outpoint: BitcoinOutPoint { txid: tx.txid(), vout: idx as u32 }, - output: output.clone(), - }); - } - } - } - Storage::Watchtower { .. } => { - //TODO: we need to ensure an offline client will generate the event when it - // comes back online after only the watchtower saw the transaction + let our_channel_close_key_hash = Hash160::hash(&self.key_storage.shutdown_pubkey.serialize()); + let shutdown_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_close_key_hash[..]).into_script(); + for (idx, output) in tx.output.iter().enumerate() { + if shutdown_script == output.script_pubkey { + return Some(SpendableOutputDescriptor::StaticOutput { + outpoint: BitcoinOutPoint { txid: tx.txid(), vout: idx as u32 }, + output: output.clone(), + }); } } } @@ -1931,23 +1824,13 @@ impl ChannelMonitor { pub fn get_latest_local_commitment_txn(&mut self) -> Vec { log_trace!(self, "Getting signed latest local commitment transaction!"); if let &mut Some(ref mut local_tx) = &mut self.current_local_signed_commitment_tx { - match self.key_storage { - Storage::Local { ref funding_key, .. } => { - local_tx.tx.add_local_sig(funding_key, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); - }, - _ => {}, - } + local_tx.tx.add_local_sig(&self.key_storage.funding_key, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); } if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx { let mut res = vec![local_tx.tx.with_valid_witness().clone()]; - match self.key_storage { - Storage::Local { ref delayed_payment_base_key, .. } => { - res.append(&mut self.broadcast_by_local_state(local_tx, delayed_payment_base_key).0); - // We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do. - // The data will be re-generated and tracked in check_spend_local_transaction if we get a confirmation. - }, - _ => panic!("Can only broadcast by local channelmonitor"), - }; + res.append(&mut self.broadcast_by_local_state(local_tx, &self.key_storage.delayed_payment_base_key).0); + // We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do. + // The data will be re-generated and tracked in check_spend_local_transaction if we get a confirmation. res } else { Vec::new() @@ -1983,14 +1866,7 @@ impl ChannelMonitor { // which is an easy way to filter out any potential non-matching txn for lazy // filters. let prevout = &tx.input[0].previous_output; - let funding_txo = match self.key_storage { - Storage::Local { ref funding_info, .. } => { - funding_info.clone() - } - Storage::Watchtower { .. } => { - unimplemented!(); - } - }; + let funding_txo = self.key_storage.funding_info.clone(); if funding_txo.is_none() || (prevout.txid == funding_txo.as_ref().unwrap().0.txid && prevout.vout == funding_txo.as_ref().unwrap().0.index as u32) { if (tx.input[0].sequence >> 8*3) as u8 == 0x80 && (tx.lock_time >> 8*3) as u8 == 0x20 { let (mut new_outpoints, new_outputs, mut spendable_output) = self.check_spend_remote_transaction(&tx, height); @@ -2036,31 +1912,21 @@ impl ChannelMonitor { } else { false }; if let Some(ref mut cur_local_tx) = self.current_local_signed_commitment_tx { if should_broadcast { - match self.key_storage { - Storage::Local { ref funding_key, .. } => { - cur_local_tx.tx.add_local_sig(funding_key, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); - }, - _ => {} - } + cur_local_tx.tx.add_local_sig(&self.key_storage.funding_key, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); } } 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()); - match self.key_storage { - Storage::Local { ref delayed_payment_base_key, .. } => { - let (txs, mut spendable_output, new_outputs) = self.broadcast_by_local_state(&cur_local_tx, delayed_payment_base_key); - spendable_outputs.append(&mut spendable_output); - if !new_outputs.is_empty() { - watch_outputs.push((cur_local_tx.txid.clone(), new_outputs)); - } - for tx in txs { - log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); - broadcaster.broadcast_transaction(&tx); - } - }, - Storage::Watchtower { .. } => { }, + let (txs, mut spendable_output, new_outputs) = self.broadcast_by_local_state(&cur_local_tx, &self.key_storage.delayed_payment_base_key); + spendable_outputs.append(&mut spendable_output); + if !new_outputs.is_empty() { + watch_outputs.push((cur_local_tx.txid.clone(), new_outputs)); + } + for tx in txs { + log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); + broadcaster.broadcast_transaction(&tx); } } } @@ -2161,16 +2027,14 @@ impl ChannelMonitor { scan_commitment!(cur_local_tx.htlc_outputs.iter().map(|&(ref a, _, _)| a), true); } - if let Storage::Local { ref current_remote_commitment_txid, ref prev_remote_commitment_txid, .. } = self.key_storage { - if let &Some(ref txid) = current_remote_commitment_txid { - if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) { - scan_commitment!(htlc_outputs.iter().map(|&(ref a, _)| a), false); - } + if let Some(ref txid) = self.key_storage.current_remote_commitment_txid { + if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) { + scan_commitment!(htlc_outputs.iter().map(|&(ref a, _)| a), false); } - if let &Some(ref txid) = prev_remote_commitment_txid { - if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) { - scan_commitment!(htlc_outputs.iter().map(|&(ref a, _)| a), false); - } + } + if let Some(ref txid) = self.key_storage.prev_remote_commitment_txid { + if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) { + scan_commitment!(htlc_outputs.iter().map(|&(ref a, _)| a), false); } } @@ -2211,7 +2075,7 @@ impl ChannelMonitor { macro_rules! check_htlc_valid_remote { ($remote_txid: expr, $htlc_output: expr) => { - if let &Some(txid) = $remote_txid { + if let Some(txid) = $remote_txid { for &(ref pending_htlc, ref pending_source) in self.remote_claimable_outpoints.get(&txid).unwrap() { if pending_htlc.payment_hash == $htlc_output.payment_hash && pending_htlc.amount_msat == $htlc_output.amount_msat { if let &Some(ref source) = pending_source { @@ -2238,13 +2102,9 @@ impl ChannelMonitor { // resolve the source HTLC with the original sender. payment_data = Some(((*source).clone(), htlc_output.payment_hash)); } else if !$local_tx { - if let Storage::Local { ref current_remote_commitment_txid, .. } = self.key_storage { - check_htlc_valid_remote!(current_remote_commitment_txid, htlc_output); - } + check_htlc_valid_remote!(self.key_storage.current_remote_commitment_txid, htlc_output); if payment_data.is_none() { - if let Storage::Local { ref prev_remote_commitment_txid, .. } = self.key_storage { - check_htlc_valid_remote!(prev_remote_commitment_txid, htlc_output); - } + check_htlc_valid_remote!(self.key_storage.prev_remote_commitment_txid, htlc_output); } } if payment_data.is_none() { @@ -2337,38 +2197,35 @@ impl ReadableArgs> for (Sha256dH let latest_update_id: u64 = Readable::read(reader)?; let commitment_transaction_number_obscure_factor = ::read(reader)?.0; - let key_storage = match ::read(reader)? { - 0 => { - let keys = Readable::read(reader)?; - let funding_key = Readable::read(reader)?; - let revocation_base_key = Readable::read(reader)?; - let htlc_base_key = Readable::read(reader)?; - let delayed_payment_base_key = Readable::read(reader)?; - let payment_base_key = Readable::read(reader)?; - let shutdown_pubkey = Readable::read(reader)?; - // Technically this can fail and serialize fail a round-trip, but only for serialization of - // barely-init'd ChannelMonitors that we can't do anything with. - let outpoint = OutPoint { - txid: Readable::read(reader)?, - index: Readable::read(reader)?, - }; - let funding_info = Some((outpoint, Readable::read(reader)?)); - let current_remote_commitment_txid = Readable::read(reader)?; - let prev_remote_commitment_txid = Readable::read(reader)?; - Storage::Local { - keys, - funding_key, - revocation_base_key, - htlc_base_key, - delayed_payment_base_key, - payment_base_key, - shutdown_pubkey, - funding_info, - current_remote_commitment_txid, - prev_remote_commitment_txid, - } - }, - _ => return Err(DecodeError::InvalidValue), + let key_storage = { + let keys = Readable::read(reader)?; + let funding_key = Readable::read(reader)?; + let revocation_base_key = Readable::read(reader)?; + let htlc_base_key = Readable::read(reader)?; + let delayed_payment_base_key = Readable::read(reader)?; + let payment_base_key = Readable::read(reader)?; + let shutdown_pubkey = Readable::read(reader)?; + // Technically this can fail and serialize fail a round-trip, but only for serialization of + // barely-init'd ChannelMonitors that we can't do anything with. + let outpoint = OutPoint { + txid: Readable::read(reader)?, + index: Readable::read(reader)?, + }; + let funding_info = Some((outpoint, Readable::read(reader)?)); + let current_remote_commitment_txid = Readable::read(reader)?; + let prev_remote_commitment_txid = Readable::read(reader)?; + Storage { + keys, + funding_key, + revocation_base_key, + htlc_base_key, + delayed_payment_base_key, + payment_base_key, + shutdown_pubkey, + funding_info, + current_remote_commitment_txid, + prev_remote_commitment_txid, + } }; let their_htlc_base_key = Some(Readable::read(reader)?); diff --git a/lightning/src/util/macro_logger.rs b/lightning/src/util/macro_logger.rs index e3a431ed54f..992636671d7 100644 --- a/lightning/src/util/macro_logger.rs +++ b/lightning/src/util/macro_logger.rs @@ -65,14 +65,7 @@ impl<'a, T> std::fmt::Display for DebugFundingInfo<'a, T> { } macro_rules! log_funding_info { ($key_storage: expr) => { - match $key_storage { - Storage::Local { ref funding_info, .. } => { - ::util::macro_logger::DebugFundingInfo(&funding_info) - }, - Storage::Watchtower { .. } => { - ::util::macro_logger::DebugFundingInfo(&None) - } - } + ::util::macro_logger::DebugFundingInfo(&$key_storage.funding_info) } } From e1f5e9bf7324771a7276f728ec09c164a72d356c Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Wed, 18 Mar 2020 00:29:26 -0400 Subject: [PATCH 3/7] Add Storage in OnchainTxHandler Going further between splitting detection and transaction generation, we endow OnchainTxHandler with keys access. That way, in latter commits, we may remove keys entirely from ChannelMonitor. --- lightning/src/ln/channelmonitor.rs | 60 +++++++++++++------------ lightning/src/ln/onchaintx.rs | 72 ++++++++++++++++++++++++++---- 2 files changed, 95 insertions(+), 37 deletions(-) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 7cfeab71a9e..8bb6b974a38 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -382,17 +382,18 @@ pub(crate) const LATENCY_GRACE_PERIOD_BLOCKS: u32 = 3; /// keeping bumping another claim tx to solve the outpoint. pub(crate) const ANTI_REORG_DELAY: u32 = 6; -struct Storage { - keys: ChanSigner, - funding_key: SecretKey, - revocation_base_key: SecretKey, - htlc_base_key: SecretKey, - delayed_payment_base_key: SecretKey, - payment_base_key: SecretKey, - shutdown_pubkey: PublicKey, - funding_info: Option<(OutPoint, Script)>, - current_remote_commitment_txid: Option, - prev_remote_commitment_txid: Option, +#[derive(Clone)] +pub(crate) struct Storage { + pub(crate) keys: ChanSigner, + pub(crate) funding_key: SecretKey, + pub(crate) revocation_base_key: SecretKey, + pub(crate) htlc_base_key: SecretKey, + pub(crate) delayed_payment_base_key: SecretKey, + pub(crate) payment_base_key: SecretKey, + pub(crate) shutdown_pubkey: PublicKey, + pub(crate) funding_info: Option<(OutPoint, Script)>, + pub(crate) current_remote_commitment_txid: Option, + pub(crate) prev_remote_commitment_txid: Option, } #[cfg(any(test, feature = "fuzztarget"))] @@ -763,9 +764,9 @@ pub struct ChannelMonitor { outputs_to_watch: HashMap>, #[cfg(test)] - pub onchain_tx_handler: OnchainTxHandler, + pub onchain_tx_handler: OnchainTxHandler, #[cfg(not(test))] - onchain_tx_handler: OnchainTxHandler, + onchain_tx_handler: OnchainTxHandler, // We simply modify last_block_hash in Channel's block_connected so that serialization is // consistent but hopefully the users' copy handles block_connected in a consistent way. @@ -1047,22 +1048,23 @@ impl ChannelMonitor { let htlc_base_key = keys.htlc_base_key().clone(); let delayed_payment_base_key = keys.delayed_payment_base_key().clone(); let payment_base_key = keys.payment_base_key().clone(); + let key_storage = Storage { + keys, + funding_key, + revocation_base_key, + htlc_base_key, + delayed_payment_base_key, + payment_base_key, + shutdown_pubkey: shutdown_pubkey.clone(), + funding_info: Some(funding_info), + current_remote_commitment_txid: None, + prev_remote_commitment_txid: None, + }; ChannelMonitor { latest_update_id: 0, commitment_transaction_number_obscure_factor, - key_storage: Storage { - keys, - funding_key, - revocation_base_key, - htlc_base_key, - delayed_payment_base_key, - payment_base_key, - shutdown_pubkey: shutdown_pubkey.clone(), - funding_info: Some(funding_info), - current_remote_commitment_txid: None, - prev_remote_commitment_txid: None, - }, + key_storage: key_storage.clone(), their_htlc_base_key: Some(their_htlc_base_key.clone()), their_delayed_payment_base_key: Some(their_delayed_payment_base_key.clone()), funding_redeemscript: Some(funding_redeemscript), @@ -1090,7 +1092,7 @@ impl ChannelMonitor { onchain_events_waiting_threshold_conf: HashMap::new(), outputs_to_watch: HashMap::new(), - onchain_tx_handler: OnchainTxHandler::new(destination_script.clone(), logger.clone()), + onchain_tx_handler: OnchainTxHandler::new(destination_script.clone(), key_storage, logger.clone()), last_block_hash: Default::default(), secp_ctx: Secp256k1::new(), @@ -2694,7 +2696,7 @@ mod tests { for (idx, inp) in claim_tx.input.iter_mut().zip(inputs_des.iter()).enumerate() { sign_input!(sighash_parts, inp.0, idx as u32, 0, inp.1, sum_actual_sigs); } - assert_eq!(base_weight + OnchainTxHandler::get_witnesses_weight(&inputs_des[..]), claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() - sum_actual_sigs)); + assert_eq!(base_weight + OnchainTxHandler::::get_witnesses_weight(&inputs_des[..]), claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() - sum_actual_sigs)); // Claim tx with 1 offered HTLCs, 3 received HTLCs claim_tx.input.clear(); @@ -2716,7 +2718,7 @@ mod tests { for (idx, inp) in claim_tx.input.iter_mut().zip(inputs_des.iter()).enumerate() { sign_input!(sighash_parts, inp.0, idx as u32, 0, inp.1, sum_actual_sigs); } - assert_eq!(base_weight + OnchainTxHandler::get_witnesses_weight(&inputs_des[..]), claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() - sum_actual_sigs)); + assert_eq!(base_weight + OnchainTxHandler::::get_witnesses_weight(&inputs_des[..]), claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_des.len() - sum_actual_sigs)); // Justice tx with 1 revoked HTLC-Success tx output claim_tx.input.clear(); @@ -2736,7 +2738,7 @@ mod tests { for (idx, inp) in claim_tx.input.iter_mut().zip(inputs_des.iter()).enumerate() { sign_input!(sighash_parts, inp.0, idx as u32, 0, inp.1, sum_actual_sigs); } - assert_eq!(base_weight + OnchainTxHandler::get_witnesses_weight(&inputs_des[..]), claim_tx.get_weight() + /* max_length_isg */ (73 * inputs_des.len() - sum_actual_sigs)); + assert_eq!(base_weight + OnchainTxHandler::::get_witnesses_weight(&inputs_des[..]), claim_tx.get_weight() + /* max_length_isg */ (73 * inputs_des.len() - sum_actual_sigs)); } // Further testing is done in the ChannelManager integration tests. diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 3f21d4c1f64..9af6a5d6e2b 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -14,10 +14,11 @@ use secp256k1::Secp256k1; use secp256k1; use ln::msgs::DecodeError; -use ln::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest}; +use ln::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest, Storage}; use ln::chan_utils::HTLCType; use chain::chaininterface::{FeeEstimator, BroadcasterInterface, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT}; -use chain::keysinterface::SpendableOutputDescriptor; +use chain::transaction::OutPoint; +use chain::keysinterface::{SpendableOutputDescriptor, ChannelKeys}; use util::logger::Logger; use util::ser::{ReadableArgs, Readable, Writer, Writeable}; use util::byte_utils; @@ -140,9 +141,11 @@ macro_rules! subtract_high_prio_fee { /// OnchainTxHandler receives claiming requests, aggregates them if it's sound, broadcast and /// do RBF bumping if possible. #[derive(Clone)] -pub struct OnchainTxHandler { +pub struct OnchainTxHandler { destination_script: Script, + key_storage: Storage, + // Used to track claiming requests. If claim tx doesn't confirm before height timer expiration we need to bump // it (RBF or CPFP). If an input has been part of an aggregate tx at first claim try, we need to keep it within // another bumped aggregate tx to comply with RBF rules. We may have multiple claiming txn in the flight for the @@ -176,10 +179,30 @@ pub struct OnchainTxHandler { logger: Arc } -impl Writeable for OnchainTxHandler { - fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { +impl OnchainTxHandler { + pub(crate) fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { self.destination_script.write(writer)?; + self.key_storage.keys.write(writer)?; + writer.write_all(&self.key_storage.funding_key[..])?; + writer.write_all(&self.key_storage.revocation_base_key[..])?; + writer.write_all(&self.key_storage.htlc_base_key[..])?; + writer.write_all(&self.key_storage.delayed_payment_base_key[..])?; + writer.write_all(&self.key_storage.payment_base_key[..])?; + writer.write_all(&self.key_storage.shutdown_pubkey.serialize())?; + match self.key_storage.funding_info { + Some((ref outpoint, ref script)) => { + writer.write_all(&outpoint.txid[..])?; + writer.write_all(&byte_utils::be16_to_array(outpoint.index))?; + script.write(writer)?; + }, + None => { + debug_assert!(false, "Try to serialize a useless Local monitor !"); + }, + } + self.key_storage.current_remote_commitment_txid.write(writer)?; + self.key_storage.prev_remote_commitment_txid.write(writer)?; + writer.write_all(&byte_utils::be64_to_array(self.pending_claim_requests.len() as u64))?; for (ref ancestor_claim_txid, claim_tx_data) in self.pending_claim_requests.iter() { ancestor_claim_txid.write(writer)?; @@ -215,10 +238,41 @@ impl Writeable for OnchainTxHandler { } } -impl ReadableArgs> for OnchainTxHandler { +impl ReadableArgs> for OnchainTxHandler { fn read(reader: &mut R, logger: Arc) -> Result { let destination_script = Readable::read(reader)?; + let key_storage = { + let keys = Readable::read(reader)?; + let funding_key = Readable::read(reader)?; + let revocation_base_key = Readable::read(reader)?; + let htlc_base_key = Readable::read(reader)?; + let delayed_payment_base_key = Readable::read(reader)?; + let payment_base_key = Readable::read(reader)?; + let shutdown_pubkey = Readable::read(reader)?; + // Technically this can fail and serialize fail a round-trip, but only for serialization of + // barely-init'd ChannelMonitors that we can't do anything with. + let outpoint = OutPoint { + txid: Readable::read(reader)?, + index: Readable::read(reader)?, + }; + let funding_info = Some((outpoint, Readable::read(reader)?)); + let current_remote_commitment_txid = Readable::read(reader)?; + let prev_remote_commitment_txid = Readable::read(reader)?; + Storage { + keys, + funding_key, + revocation_base_key, + htlc_base_key, + delayed_payment_base_key, + payment_base_key, + shutdown_pubkey, + funding_info, + current_remote_commitment_txid, + prev_remote_commitment_txid, + } + }; + let pending_claim_requests_len: u64 = Readable::read(reader)?; let mut pending_claim_requests = HashMap::with_capacity(cmp::min(pending_claim_requests_len as usize, MAX_ALLOC_SIZE / 128)); for _ in 0..pending_claim_requests_len { @@ -264,6 +318,7 @@ impl ReadableArgs> for OnchainTxHandler { Ok(OnchainTxHandler { destination_script, + key_storage, claimable_outpoints, pending_claim_requests, onchain_events_waiting_threshold_conf, @@ -273,10 +328,11 @@ impl ReadableArgs> for OnchainTxHandler { } } -impl OnchainTxHandler { - pub(super) fn new(destination_script: Script, logger: Arc) -> Self { +impl OnchainTxHandler { + pub(super) fn new(destination_script: Script, key_storage: Storage, logger: Arc) -> Self { OnchainTxHandler { destination_script, + key_storage, pending_claim_requests: HashMap::new(), claimable_outpoints: HashMap::new(), onchain_events_waiting_threshold_conf: HashMap::new(), From 23d358b1d231f446f1b55384fa1115b5d50d8e7e Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Tue, 3 Mar 2020 18:51:50 -0500 Subject: [PATCH 4/7] 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. --- lightning/src/ln/channelmonitor.rs | 24 +++- lightning/src/ln/functional_tests.rs | 54 +++++---- lightning/src/ln/onchaintx.rs | 165 ++++++++++++++++----------- 3 files changed, 151 insertions(+), 92 deletions(-) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 8bb6b974a38..8295508e264 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -441,6 +441,10 @@ pub(crate) enum InputMaterial { sigs: (Signature, Signature), preimage: Option, amount: u64, + }, + Funding { + local_tx_remote_signed: LocalCommitmentTransaction, + channel_value: u64, } } @@ -470,6 +474,11 @@ impl Writeable for InputMaterial { sigs.1.write(writer)?; preimage.write(writer)?; writer.write_all(&byte_utils::be64_to_array(*amount))?; + }, + &InputMaterial::Funding { ref local_tx_remote_signed, ref channel_value } => { + writer.write_all(&[3; 1])?; + local_tx_remote_signed.write(writer)?; + channel_value.write(writer)?; } } Ok(()) @@ -519,6 +528,14 @@ impl Readable for InputMaterial { preimage, amount } + }, + 3 => { + let local_tx_remote_signed = Readable::read(reader)?; + let channel_value = Readable::read(reader)?; + InputMaterial::Funding { + local_tx_remote_signed, + channel_value + } } _ => return Err(DecodeError::InvalidValue), }; @@ -1915,12 +1932,15 @@ impl ChannelMonitor { if let Some(ref mut cur_local_tx) = self.current_local_signed_commitment_tx { if should_broadcast { cur_local_tx.tx.add_local_sig(&self.key_storage.funding_key, self.funding_redeemscript.as_ref().unwrap(), self.channel_value_satoshis.unwrap(), &self.secp_ctx); + //TODO: move key access behind KeysInterface inside OnchainTxHandler + // If would_broadcast_at_height determine this local tx should be broadcast, absolute_timelock is set to current_height, + // because it indicates that this tx confirmation is urgent. It's not going to change anything because we can't bump + // local_commitment before anchor_outputs, should be rethought afterwards + claimable_outpoints.push(ClaimRequest { absolute_timelock: height, aggregable: false, outpoint: BitcoinOutPoint { txid: self.key_storage.funding_info.as_ref().unwrap().0.txid.clone(), vout: self.key_storage.funding_info.as_ref().unwrap().0.index as u32 }, witness_data: InputMaterial::Funding { local_tx_remote_signed: cur_local_tx.tx.clone(), 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, mut spendable_output, new_outputs) = self.broadcast_by_local_state(&cur_local_tx, &self.key_storage.delayed_payment_base_key); spendable_outputs.append(&mut spendable_output); if !new_outputs.is_empty() { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 487d7e6a49f..b6b1c29bc87 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2334,24 +2334,32 @@ 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.clone()); 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].clone()); - // 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[2], revoked_local_txn[0]); - check_spends!(node_txn[3], revoked_local_txn[0]); - check_spends!(node_txn[4], revoked_local_txn[0]); + + fn get_txout(out_point: &BitcoinOutPoint, tx: &Transaction) -> Option { + if out_point.txid == tx.txid() { + tx.output.get(out_point.vout as usize).cloned() + } else { + None + } + } + node_txn[1].verify(|out|get_txout(out, &revoked_local_txn[0])).unwrap(); + node_txn[2].verify(|out|get_txout(out, &revoked_local_txn[0])).unwrap(); + node_txn[4].verify(|out|get_txout(out, &revoked_local_txn[0])).unwrap(); 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 @@ -2593,18 +2601,18 @@ 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 : (local commitment tx + HTLC-timeout), timeout tx - assert_eq!(node_txn[0], node_txn[3]); - assert_eq!(node_txn[1], node_txn[4]); + assert_eq!(node_txn[2], node_txn[3]); + assert_eq!(node_txn[0], 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); + check_spends!(node_txn[1], commitment_tx[0]); + assert_eq!(node_txn[1].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); - 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[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[2].clone(); + timeout_tx = node_txn[1].clone(); node_txn.clear(); } @@ -7168,11 +7176,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].clone()); + check_spends!(node_txn[1], node_txn[2].clone()); + 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() }; nodes[1].node.get_and_clear_pending_msg_events(); diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 9af6a5d6e2b..80a78139578 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -53,7 +53,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 @@ -65,7 +65,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))?; @@ -388,7 +388,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 @@ -453,9 +453,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, .. } => { @@ -467,71 +468,97 @@ 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![0]); + 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![0]); + } + 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 local_tx_remote_signed, ref channel_value } => { + let mut local_tx = local_tx_remote_signed.clone(); + local_tx.add_local_sig(&self.key_storage.funding_key, &self.key_storage.funding_info.as_ref().unwrap().1, *channel_value, &self.secp_ctx); + let signed_tx = local_tx.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) -> Vec @@ -567,10 +594,16 @@ 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; + if claim_material.height_timer.is_some() { + spendable_outputs.push(SpendableOutputDescriptor::StaticOutput { + outpoint: BitcoinOutPoint { txid: tx.txid(), vout: 0 }, + output: tx.output[0].clone(), + }); + } let txid = tx.txid(); self.pending_claim_requests.insert(txid, claim_material); for k in claim.1.keys() { @@ -578,10 +611,6 @@ impl OnchainTxHandler { self.claimable_outpoints.insert(k.clone(), (txid, height)); } log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); - spendable_outputs.push(SpendableOutputDescriptor::StaticOutput { - outpoint: BitcoinOutPoint { txid: tx.txid(), vout: 0 }, - output: tx.output[0].clone(), - }); broadcaster.broadcast_transaction(&tx); } } @@ -689,8 +718,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); + } } } From d44f0b1dddb1aa636e4edb5ecc3c37142e40014a Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Tue, 3 Mar 2020 14:41:13 -0500 Subject: [PATCH 5/7] Add logger for SpendableOutputDescriptor --- lightning/src/ln/channelmonitor.rs | 4 ++++ lightning/src/util/macro_logger.rs | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 8295508e264..114181475a9 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -1974,6 +1974,10 @@ impl ChannelMonitor { self.outputs_to_watch.insert(txid.clone(), output_scripts.iter().map(|o| o.script_pubkey.clone()).collect()); } + for spend in spendable_outputs.iter() { + log_trace!(self, "{}", log_spendable!(spend)); + } + if spendable_outputs.len() > 0 { self.pending_events.push(events::Event::SpendableOutputs { outputs: spendable_outputs, diff --git a/lightning/src/util/macro_logger.rs b/lightning/src/util/macro_logger.rs index 992636671d7..d16bd48aebb 100644 --- a/lightning/src/util/macro_logger.rs +++ b/lightning/src/util/macro_logger.rs @@ -1,4 +1,5 @@ use chain::transaction::OutPoint; +use chain::keysinterface::SpendableOutputDescriptor; use bitcoin_hashes::sha256d::Hash as Sha256dHash; use bitcoin::blockdata::transaction::Transaction; @@ -121,6 +122,30 @@ macro_rules! log_tx { } } +pub(crate) struct DebugSpendable<'a>(pub &'a SpendableOutputDescriptor); +impl<'a> std::fmt::Display for DebugSpendable<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { + match self.0 { + &SpendableOutputDescriptor::StaticOutput { ref outpoint, .. } => { + write!(f, "StaticOutput {}:{} marked for spending", outpoint.txid, outpoint.vout)?; + } + &SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, .. } => { + write!(f, "DynamicOutputP2WSH {}:{} marked for spending", outpoint.txid, outpoint.vout)?; + } + &SpendableOutputDescriptor::DynamicOutputP2WPKH { ref outpoint, .. } => { + write!(f, "DynamicOutputP2WPKH {}:{} marked for spending", outpoint.txid, outpoint.vout)?; + } + } + Ok(()) + } +} + +macro_rules! log_spendable { + ($obj: expr) => { + ::util::macro_logger::DebugSpendable(&$obj) + } +} + macro_rules! log_internal { ($self: ident, $lvl:expr, $($arg:tt)+) => ( &$self.logger.log(&::util::logger::Record::new($lvl, format_args!($($arg)+), module_path!(), file!(), line!())); From 7d75e7e9936eaacb456ff77c12b3a255f50813af Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Mon, 9 Mar 2020 18:15:35 -0400 Subject: [PATCH 6/7] Move HTLC tx generation in OnchainTxHandler HTLC Transaction can't be bumped without sighash changes so their gneeration is one-time for nwo. 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 broadcaster order. Number of transactions may vary because of temporary anti-duplicata tweak can't dissociate between 2- broadcast from different origins (ChannelMonitor, ChannelManager) and 2-broadcast from same component. --- lightning/src/ln/channelmonitor.rs | 93 ++++++++++++++-------------- lightning/src/ln/functional_tests.rs | 48 +++++++------- lightning/src/ln/onchaintx.rs | 35 ++++++++--- 3 files changed, 99 insertions(+), 77 deletions(-) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 114181475a9..a78e934a24d 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -437,10 +437,15 @@ pub(crate) enum InputMaterial { locktime: u32, }, LocalHTLC { - witness_script: Script, - sigs: (Signature, Signature), + their_sig: Signature, preimage: Option, amount: u64, + feerate_per_kw: u64, + their_to_self_delay: u16, + htlc: HTLCOutputInCommitment, + per_commitment_point: PublicKey, + their_htlc_key: PublicKey, + their_revocation_key: PublicKey, }, Funding { local_tx_remote_signed: LocalCommitmentTransaction, @@ -467,13 +472,17 @@ impl Writeable for InputMaterial { writer.write_all(&byte_utils::be64_to_array(*amount))?; writer.write_all(&byte_utils::be32_to_array(*locktime))?; }, - &InputMaterial::LocalHTLC { ref witness_script, ref sigs, ref preimage, ref amount } => { + &InputMaterial::LocalHTLC { ref their_sig, ref preimage, ref amount, ref feerate_per_kw, ref their_to_self_delay, ref htlc, ref per_commitment_point, ref their_htlc_key, ref their_revocation_key } => { writer.write_all(&[2; 1])?; - witness_script.write(writer)?; - sigs.0.write(writer)?; - sigs.1.write(writer)?; + their_sig.write(writer)?; preimage.write(writer)?; writer.write_all(&byte_utils::be64_to_array(*amount))?; + writer.write_all(&byte_utils::be64_to_array(*feerate_per_kw))?; + writer.write_all(&byte_utils::be16_to_array(*their_to_self_delay))?; + htlc.write(writer)?; + per_commitment_point.write(writer)?; + their_htlc_key.write(writer)?; + their_revocation_key.write(writer)?; }, &InputMaterial::Funding { ref local_tx_remote_signed, ref channel_value } => { writer.write_all(&[3; 1])?; @@ -517,16 +526,25 @@ impl Readable for InputMaterial { } }, 2 => { - let witness_script = Readable::read(reader)?; let their_sig = Readable::read(reader)?; - let our_sig = Readable::read(reader)?; let preimage = Readable::read(reader)?; let amount = Readable::read(reader)?; + let feerate_per_kw = Readable::read(reader)?; + let their_to_self_delay = Readable::read(reader)?; + let htlc = Readable::read(reader)?; + let per_commitment_point = Readable::read(reader)?; + let their_htlc_key = Readable::read(reader)?; + let their_revocation_key = Readable::read(reader)?; InputMaterial::LocalHTLC { - witness_script, - sigs: (their_sig, our_sig), + their_sig, preimage, - amount + amount, + feerate_per_kw, + their_to_self_delay, + htlc, + per_commitment_point, + their_htlc_key, + their_revocation_key, } }, 3 => { @@ -1647,8 +1665,9 @@ impl ChannelMonitor { (claimable_outpoints, Some((htlc_txid, tx.output.clone()))) } - fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx, delayed_payment_base_key: &SecretKey) -> (Vec, Vec, Vec) { + fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx, delayed_payment_base_key: &SecretKey) -> (Vec, Vec, Vec, Vec) { let mut res = Vec::with_capacity(local_tx.htlc_outputs.len()); + let mut claim_requests = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut spendable_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut watch_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); @@ -1679,35 +1698,25 @@ impl ChannelMonitor { if let Some(transaction_output_index) = htlc.transaction_output_index { if let &Some(ref their_sig) = sigs { if htlc.offered { - log_trace!(self, "Broadcasting HTLC-Timeout transaction against local commitment transactions"); 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); - let (our_sig, htlc_script) = match - chan_utils::sign_htlc_transaction(&mut htlc_timeout_tx, their_sig, &None, htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.key_storage.htlc_base_key, &self.secp_ctx) { - Ok(res) => res, + match chan_utils::sign_htlc_transaction(&mut htlc_timeout_tx, their_sig, &None, htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.key_storage.htlc_base_key, &self.secp_ctx) { + Ok(_) => {}, Err(_) => continue, }; add_dynamic_output!(htlc_timeout_tx, 0); - let mut per_input_material = HashMap::with_capacity(1); - per_input_material.insert(htlc_timeout_tx.input[0].previous_output, InputMaterial::LocalHTLC { witness_script: htlc_script, sigs: (*their_sig, our_sig), preimage: None, amount: htlc.amount_msat / 1000}); - //TODO: with option_simplified_commitment track outpoint too - log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_timeout_tx.input[0].previous_output.vout, htlc_timeout_tx.input[0].previous_output.txid); + claim_requests.push(ClaimRequest { absolute_timelock: ::std::u32::MAX, aggregable: false, outpoint: BitcoinOutPoint { txid: local_tx.txid, vout: transaction_output_index as u32 }, witness_data: InputMaterial::LocalHTLC { their_sig: *their_sig, preimage: None, amount: htlc.amount_msat / 1000, feerate_per_kw: local_tx.feerate_per_kw, their_to_self_delay: self.their_to_self_delay.unwrap(), htlc: htlc.clone(), per_commitment_point: local_tx.per_commitment_point, their_htlc_key: self.their_htlc_base_key.unwrap(), their_revocation_key: local_tx.revocation_key }}); res.push(htlc_timeout_tx); } else { if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) { - log_trace!(self, "Broadcasting HTLC-Success transaction against local commitment transactions"); 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); - let (our_sig, htlc_script) = match - chan_utils::sign_htlc_transaction(&mut htlc_success_tx, their_sig, &Some(*payment_preimage), htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.key_storage.htlc_base_key, &self.secp_ctx) { - Ok(res) => res, + match chan_utils::sign_htlc_transaction(&mut htlc_success_tx, their_sig, &Some(*payment_preimage), htlc, &local_tx.a_htlc_key, &local_tx.b_htlc_key, &local_tx.revocation_key, &local_tx.per_commitment_point, &self.key_storage.htlc_base_key, &self.secp_ctx) { + Ok(_) => {}, Err(_) => continue, }; add_dynamic_output!(htlc_success_tx, 0); - let mut per_input_material = HashMap::with_capacity(1); - per_input_material.insert(htlc_success_tx.input[0].previous_output, InputMaterial::LocalHTLC { witness_script: htlc_script, sigs: (*their_sig, our_sig), preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000}); - //TODO: with option_simplified_commitment track outpoint too - log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_success_tx.input[0].previous_output.vout, htlc_success_tx.input[0].previous_output.txid); + claim_requests.push(ClaimRequest { absolute_timelock: ::std::u32::MAX, aggregable: false, outpoint: BitcoinOutPoint { txid: local_tx.txid, vout: transaction_output_index as u32 }, witness_data: InputMaterial::LocalHTLC { their_sig: *their_sig, preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000, feerate_per_kw: local_tx.feerate_per_kw, their_to_self_delay: self.their_to_self_delay.unwrap(), htlc: htlc.clone(), per_commitment_point: local_tx.per_commitment_point, their_htlc_key: self.their_htlc_base_key.unwrap(), their_revocation_key: local_tx.revocation_key}}); res.push(htlc_success_tx); } } @@ -1716,15 +1725,15 @@ impl ChannelMonitor { } } - (res, spendable_outputs, watch_outputs) + (res, claim_requests, spendable_outputs, watch_outputs) } /// Attempts to claim any claimable HTLCs in a commitment transaction which was not (yet) /// revoked using data in local_claimable_outpoints. /// Should not be used if check_spend_revoked_transaction succeeds. - fn check_spend_local_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec, Vec, (Sha256dHash, Vec)) { + fn check_spend_local_transaction(&mut self, tx: &Transaction, height: u32) -> (Vec, Vec, (Sha256dHash, Vec)) { let commitment_txid = tx.txid(); - let mut local_txn = Vec::new(); + let mut claim_requests = Vec::new(); let mut spendable_outputs = Vec::new(); let mut watch_outputs = Vec::new(); @@ -1752,9 +1761,9 @@ impl ChannelMonitor { macro_rules! append_onchain_update { ($updates: expr) => { - local_txn.append(&mut $updates.0); - spendable_outputs.append(&mut $updates.1); - watch_outputs.append(&mut $updates.2); + claim_requests = $updates.1; + spendable_outputs.append(&mut $updates.2); + watch_outputs.append(&mut $updates.3); } } @@ -1811,7 +1820,7 @@ impl ChannelMonitor { } } - (local_txn, spendable_outputs, (commitment_txid, watch_outputs)) + (claim_requests, spendable_outputs, (commitment_txid, watch_outputs)) } /// Generate a spendable output event when closing_transaction get registered onchain. @@ -1894,15 +1903,12 @@ impl ChannelMonitor { watch_outputs.push(new_outputs); } if new_outpoints.is_empty() { - let (local_txn, mut spendable_output, new_outputs) = self.check_spend_local_transaction(&tx, height); + let (mut new_outpoints, mut spendable_output, new_outputs) = self.check_spend_local_transaction(&tx, height); spendable_outputs.append(&mut spendable_output); - for tx in local_txn.iter() { - log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); - broadcaster.broadcast_transaction(tx); - } if !new_outputs.1.is_empty() { watch_outputs.push(new_outputs); } + claimable_outpoints.append(&mut new_outpoints); } claimable_outpoints.append(&mut new_outpoints); } @@ -1941,15 +1947,12 @@ impl ChannelMonitor { } if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx { if should_broadcast { - let (txs, mut spendable_output, new_outputs) = self.broadcast_by_local_state(&cur_local_tx, &self.key_storage.delayed_payment_base_key); + let (_, mut new_outpoints, mut spendable_output, new_outputs) = self.broadcast_by_local_state(&cur_local_tx, &self.key_storage.delayed_payment_base_key); spendable_outputs.append(&mut spendable_output); if !new_outputs.is_empty() { watch_outputs.push((cur_local_tx.txid.clone(), new_outputs)); } - for tx in txs { - log_trace!(self, "Broadcast onchain {}", log_tx!(tx)); - broadcaster.broadcast_transaction(&tx); - } + claimable_outpoints.append(&mut new_outpoints); } } if let Some(events) = self.onchain_events_waiting_threshold_conf.remove(&height) { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index b6b1c29bc87..9518d44c997 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2334,16 +2334,16 @@ 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[2].input.len(), 1); + check_spends!(node_txn[2], chan_1.3.clone()); assert_eq!(node_txn[3].input.len(), 1); - check_spends!(node_txn[3], chan_1.3.clone()); - assert_eq!(node_txn[0].input.len(), 1); - let witness_script = node_txn[0].input[0].witness.last().unwrap(); + let witness_script = node_txn[3].input[0].witness.last().unwrap(); assert_eq!(witness_script.len(), OFFERED_HTLC_SCRIPT_WEIGHT); //Spending an offered htlc output - check_spends!(node_txn[0], node_txn[3].clone()); + check_spends!(node_txn[3], node_txn[2].clone()); // Justice transactions are indices 1-2-4 + assert_eq!(node_txn[0].input.len(), 1); assert_eq!(node_txn[1].input.len(), 1); - assert_eq!(node_txn[2].input.len(), 1); assert_eq!(node_txn[4].input.len(), 1); fn get_txout(out_point: &BitcoinOutPoint, tx: &Transaction) -> Option { @@ -2353,13 +2353,13 @@ fn claim_htlc_outputs_single_tx() { None } } + node_txn[0].verify(|out|get_txout(out, &revoked_local_txn[0])).unwrap(); node_txn[1].verify(|out|get_txout(out, &revoked_local_txn[0])).unwrap(); - node_txn[2].verify(|out|get_txout(out, &revoked_local_txn[0])).unwrap(); node_txn[4].verify(|out|get_txout(out, &revoked_local_txn[0])).unwrap(); let mut witness_lens = BTreeSet::new(); + witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len()); 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[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 @@ -2601,18 +2601,18 @@ 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 : (local commitment tx + HTLC-timeout), timeout tx - assert_eq!(node_txn[2], node_txn[3]); - assert_eq!(node_txn[0], node_txn[4]); + assert_eq!(node_txn[1], node_txn[3]); + assert_eq!(node_txn[2], node_txn[4]); - check_spends!(node_txn[1], commitment_tx[0]); - assert_eq!(node_txn[1].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); + check_spends!(node_txn[0], commitment_tx[0]); + assert_eq!(node_txn[0].clone().input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); - 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); + check_spends!(node_txn[1], chan_2.3); + check_spends!(node_txn[2], node_txn[1]); + assert_eq!(node_txn[1].clone().input[0].witness.last().unwrap().len(), 71); + assert_eq!(node_txn[2].clone().input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); - timeout_tx = node_txn[1].clone(); + timeout_tx = node_txn[0].clone(); node_txn.clear(); } @@ -4155,8 +4155,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() { check_closed_broadcast!(nodes[0], false); let revoked_htlc_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(revoked_htlc_txn.len(), 3); - assert_eq!(revoked_htlc_txn[0], revoked_htlc_txn[2]); + assert_eq!(revoked_htlc_txn.len(), 2); assert_eq!(revoked_htlc_txn[0].input.len(), 1); assert_eq!(revoked_htlc_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]); @@ -4206,8 +4205,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() { check_closed_broadcast!(nodes[1], false); let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(revoked_htlc_txn.len(), 3); - assert_eq!(revoked_htlc_txn[0], revoked_htlc_txn[2]); + assert_eq!(revoked_htlc_txn.len(), 2); assert_eq!(revoked_htlc_txn[0].input.len(), 1); assert_eq!(revoked_htlc_txn[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]); @@ -6922,7 +6920,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() { check_closed_broadcast!(nodes[1], false); let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(revoked_htlc_txn.len(), 6); + assert_eq!(revoked_htlc_txn.len(), 4); if revoked_htlc_txn[0].input[0].witness.last().unwrap().len() == ACCEPTED_HTLC_SCRIPT_WEIGHT { assert_eq!(revoked_htlc_txn[0].input.len(), 1); check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]); @@ -7176,11 +7174,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[0], node_txn[2].clone()); - check_spends!(node_txn[1], node_txn[2].clone()); - assert_eq!(node_txn[0].input.len(), 1); + check_spends!(node_txn[1], node_txn[0].clone()); + check_spends!(node_txn[2], node_txn[0].clone()); assert_eq!(node_txn[1].input.len(), 1); - node_txn[0].clone() + assert_eq!(node_txn[2].input.len(), 1); + node_txn[1].clone() }; nodes[1].node.get_and_clear_pending_msg_events(); diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 80a78139578..02f2c6728a6 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -11,11 +11,13 @@ use bitcoin::util::bip143; use bitcoin_hashes::sha256d::Hash as Sha256dHash; use secp256k1::Secp256k1; +use secp256k1::key::PublicKey; use secp256k1; use ln::msgs::DecodeError; use ln::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest, Storage}; use ln::chan_utils::HTLCType; +use ln::chan_utils; use chain::chaininterface::{FeeEstimator, BroadcasterInterface, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT}; use chain::transaction::OutPoint; use chain::keysinterface::{SpendableOutputDescriptor, ChannelKeys}; @@ -467,13 +469,14 @@ impl OnchainTxHandler { inputs_witnesses_weight += Self::get_witnesses_weight(if preimage.is_some() { &[InputDescriptors::OfferedHTLC] } else { &[InputDescriptors::ReceivedHTLC] }); amt += *amount; }, - &InputMaterial::LocalHTLC { .. } => { return None; } + &InputMaterial::LocalHTLC { .. } => { + dynamic_fee = false; + }, &InputMaterial::Funding { .. } => { dynamic_fee = false; } } } - if dynamic_fee { let predicted_weight = bumped_tx.get_weight() + inputs_witnesses_weight; let mut new_feerate; @@ -535,11 +538,29 @@ impl OnchainTxHandler { } 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::LocalHTLC { ref their_sig, ref preimage, ref amount, ref feerate_per_kw, ref their_to_self_delay, ref htlc, ref per_commitment_point, ref their_htlc_key, ref their_revocation_key } => { + macro_rules! ignore_error { + ( $thing : expr ) => { + match $thing { + Ok(a) => a, + Err(_) => return None, + } + }; + } + + let delayed_payment_key = ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, &per_commitment_point, &self.key_storage.delayed_payment_base_key)); + let delayed_payment_pubkey = PublicKey::from_secret_key(&self.secp_ctx, &delayed_payment_key); + let mut htlc_tx = chan_utils::build_htlc_transaction(&outp.txid, *feerate_per_kw, *their_to_self_delay, htlc, &delayed_payment_pubkey, &their_revocation_key); + let a_htlc_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &self.key_storage.keys.pubkeys().htlc_basepoint)); + let b_htlc_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &their_htlc_key)); + match chan_utils::sign_htlc_transaction(&mut htlc_tx, their_sig, preimage, htlc, &a_htlc_key, &b_htlc_key, &their_revocation_key, &per_commitment_point, &self.key_storage.htlc_base_key, &self.secp_ctx) { + Ok(res) => res, + Err(_) => continue, + }; + let feerate = (amount - htlc_tx.output[0].value) * 1000 / htlc_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 HTLC-{} claiming HTLC output {} from {}...", if preimage.is_some() { "Success" } else { "Timeout" }, outp.vout, outp.txid); + return Some((None, feerate, htlc_tx)); }, &InputMaterial::Funding { ref local_tx_remote_signed, ref channel_value } => { let mut local_tx = local_tx_remote_signed.clone(); From 2fea5578142edcc7bd23db3bfe161e8966a1d669 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Mon, 9 Mar 2020 19:56:42 -0400 Subject: [PATCH 7/7] Remove temporary anti-duplicata logic --- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests.rs | 2 +- lightning/src/util/test_utils.rs | 15 --------------- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 67c3b666946..bd8394d1472 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -974,7 +974,7 @@ pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: pub fn create_chanmon_cfgs(node_count: usize) -> Vec { let mut chan_mon_cfgs = Vec::new(); for _ in 0..node_count { - let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), broadcasted_txn: Mutex::new(HashMap::new())}; + let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())}; let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 }; chan_mon_cfgs.push(TestChanMonCfg{ tx_broadcaster, fee_estimator }); } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 9518d44c997..2350078eb3f 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -6587,7 +6587,7 @@ fn test_data_loss_protect() { let logger: Arc = Arc::new(test_utils::TestLogger::with_id(format!("node {}", 0))); let mut chan_monitor = <(Sha256dHash, ChannelMonitor)>::read(&mut ::std::io::Cursor::new(previous_chan_monitor_state.0), Arc::clone(&logger)).unwrap().1; let chain_monitor = Arc::new(ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&logger))); - tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), broadcasted_txn: Mutex::new(HashMap::new())}; + tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())}; fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 }; keys_manager = test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet, Arc::clone(&logger)); monitor = test_utils::TestChannelMonitor::new(chain_monitor.clone(), &tx_broadcaster, logger.clone(), &fee_estimator); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index ae45473a94a..78b81fa637b 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -108,24 +108,9 @@ impl<'a> channelmonitor::ManyChannelMonitor for TestChanne pub struct TestBroadcaster { pub txn_broadcasted: Mutex>, - pub broadcasted_txn: Mutex> // Temporary field while refactoring out tx duplication } impl chaininterface::BroadcasterInterface for TestBroadcaster { fn broadcast_transaction(&self, tx: &Transaction) { - let mut already = false; - { - if let Some(counter) = self.broadcasted_txn.lock().unwrap().get_mut(&tx.txid()) { - match counter { - 0 => { *counter = 1; already = true }, // We still authorize at least 2 duplicata for a given TXID to account ChannelManager/ChannelMonitor broadcast - 1 => return, - _ => panic!() - } - } - } - if !already { - self.broadcasted_txn.lock().unwrap().insert(tx.txid(), 0); - } - print!("\nFRESH BROADCAST {}\n\n", tx.txid()); self.txn_broadcasted.lock().unwrap().push(tx.clone()); } }