From: Matt Corallo Date: Fri, 7 Sep 2018 15:39:37 +0000 (-0400) Subject: f - more descriptive var names + outpoints, not txn X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=refs%2Fheads%2F2018-09-159-clarification-cleanup;p=rust-lightning f - more descriptive var names + outpoints, not txn --- diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index 600342f0b..1d69420ce 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -3297,7 +3297,8 @@ mod tests { nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); { let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(node_txn.len(), 2); + assert_eq!(node_txn.len(), 3); + assert_eq!(node_txn.pop().unwrap(), node_txn[0]); // An outpoint registration will result in a 2nd block_connected assert_eq!(node_txn[0].input.len(), 1); let mut funding_tx_map = HashMap::new(); diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 66780b30c..ddcaa5d24 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -69,9 +69,11 @@ impl ChainListener for SimpleManyChannelMonit fn block_connected(&self, _header: &BlockHeader, height: u32, txn_matched: &[&Transaction], _indexes_of_txn_matched: &[u32]) { let monitors = self.monitors.lock().unwrap(); for monitor in monitors.values() { - let outputs = monitor.block_connected(txn_matched, height, &*self.broadcaster); - for output in outputs { - self.chain_monitor.install_watch_script(&output.script_pubkey); + let txn_outputs = monitor.block_connected(txn_matched, height, &*self.broadcaster); + for (ref txid, ref outputs) in txn_outputs { + for (idx, output) in outputs.iter().enumerate() { + self.chain_monitor.install_watch_outpoint((txid.clone(), idx as u32), &output.script_pubkey); + } } } } @@ -467,7 +469,7 @@ impl ChannelMonitor { /// optional, without it this monitor cannot be used in an SPV client, but you may wish to /// avoid this (or call unset_funding_info) on a monitor you wish to send to a watchtower as it /// provides slightly better privacy. - /// It is the responsability of the caller to register outpoint and script with passing the former + /// It is the responsability of the caller to register outpoint and script with passing the former /// value as key to update current monitor pub(super) fn set_funding_info(&mut self, funding_info: (OutPoint, Script)) { self.funding_txo = Some(funding_info); @@ -912,23 +914,24 @@ impl ChannelMonitor { /// height > height + CLTV_SHARED_CLAIM_BUFFER. In any case, will install monitoring for /// HTLC-Success/HTLC-Timeout transactions, and claim them using the revocation key (if /// applicable) as well. - fn check_spend_remote_transaction(&self, tx: &Transaction, height: u32) -> (Vec, Vec) { + fn check_spend_remote_transaction(&self, tx: &Transaction, height: u32) -> (Vec, (Sha256dHash, Vec)) { // Most secp and related errors trying to create keys means we have no hope of constructing // a spend transaction...so we return no transactions to broadcast let mut txn_to_broadcast = Vec::new(); - let mut outputs = Vec::new(); + let mut watch_outputs = Vec::new(); + + let commitment_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers! + let per_commitment_option = self.remote_claimable_outpoints.get(&commitment_txid); + macro_rules! ignore_error { ( $thing : expr ) => { match $thing { Ok(a) => a, - Err(_) => return (txn_to_broadcast, outputs) + Err(_) => return (txn_to_broadcast, (commitment_txid, watch_outputs)) } }; } - let commitment_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers! - let per_commitment_option = self.remote_claimable_outpoints.get(&commitment_txid); - let commitment_number = 0xffffffffffff - ((((tx.input[0].sequence as u64 & 0xffffff) << 3*8) | (tx.lock_time as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor); if commitment_number >= self.get_min_seen_secret() { let secret = self.get_secret(commitment_number).unwrap(); @@ -947,7 +950,7 @@ impl ChannelMonitor { }; let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.delayed_payment_base_key)); let a_htlc_key = match self.their_htlc_base_key { - None => return (txn_to_broadcast, outputs), + None => return (txn_to_broadcast, (commitment_txid, watch_outputs)), Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &their_htlc_base_key)), }; @@ -1014,7 +1017,7 @@ impl ChannelMonitor { if htlc.transaction_output_index as usize >= tx.output.len() || tx.output[htlc.transaction_output_index as usize].value != htlc.amount_msat / 1000 || tx.output[htlc.transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() { - return (txn_to_broadcast, outputs); // Corrupted per_commitment_data, fuck this user + return (txn_to_broadcast, (commitment_txid, watch_outputs)); // Corrupted per_commitment_data, fuck this user } let input = TxIn { previous_output: BitcoinOutPoint { @@ -1049,10 +1052,10 @@ impl ChannelMonitor { if !inputs.is_empty() || !txn_to_broadcast.is_empty() { // ie we're confident this is actually ours // We're definitely a remote commitment transaction! - outputs.append(&mut tx.output.clone()); + watch_outputs.append(&mut tx.output.clone()); self.remote_commitment_txn_on_chain.lock().unwrap().insert(commitment_txid, commitment_number); } - if inputs.is_empty() { return (txn_to_broadcast, outputs); } // Nothing to be done...probably a false positive/local tx + if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs)); } // Nothing to be done...probably a false positive/local tx let outputs = vec!(TxOut { script_pubkey: self.destination_script.clone(), @@ -1082,7 +1085,7 @@ impl ChannelMonitor { // already processed the block, resulting in the remote_commitment_txn_on_chain entry // not being generated by the above conditional. Thus, to be safe, we go ahead and // insert it here. - outputs.append(&mut tx.output.clone()); + watch_outputs.append(&mut tx.output.clone()); self.remote_commitment_txn_on_chain.lock().unwrap().insert(commitment_txid, commitment_number); if let Some(revocation_points) = self.their_cur_revocation_points { @@ -1103,7 +1106,7 @@ impl ChannelMonitor { }, }; let a_htlc_key = match self.their_htlc_base_key { - None => return (txn_to_broadcast, outputs), + None => return (txn_to_broadcast, (commitment_txid, watch_outputs)), Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &their_htlc_base_key)), }; @@ -1166,7 +1169,7 @@ impl ChannelMonitor { } } - if inputs.is_empty() { return (txn_to_broadcast, outputs); } // Nothing to be done...probably a false positive/local tx + if inputs.is_empty() { return (txn_to_broadcast, (commitment_txid, watch_outputs)); } // Nothing to be done...probably a false positive/local tx let outputs = vec!(TxOut { script_pubkey: self.destination_script.clone(), @@ -1194,7 +1197,7 @@ impl ChannelMonitor { //TODO: For each input check if its in our remote_commitment_txn_on_chain map! } - (txn_to_broadcast, outputs) + (txn_to_broadcast, (commitment_txid, watch_outputs)) } fn broadcast_by_local_state(&self, local_tx: &LocalSignedTx) -> Vec { @@ -1255,13 +1258,15 @@ impl ChannelMonitor { Vec::new() } - fn block_connected(&self, txn_matched: &[&Transaction], height: u32, broadcaster: &BroadcasterInterface) -> Vec { - let mut outputs = Vec::new(); + fn block_connected(&self, txn_matched: &[&Transaction], height: u32, broadcaster: &BroadcasterInterface)-> Vec<(Sha256dHash, Vec)> { + let mut watch_outputs = Vec::new(); for tx in txn_matched { for txin in tx.input.iter() { if self.funding_txo.is_none() || (txin.previous_output.txid == self.funding_txo.as_ref().unwrap().0.txid && txin.previous_output.vout == self.funding_txo.as_ref().unwrap().0.index as u32) { - let (mut txn, out) = self.check_spend_remote_transaction(tx, height); - outputs = out; + let (mut txn, new_outputs) = self.check_spend_remote_transaction(tx, height); + if !new_outputs.1.is_empty() { + watch_outputs.push(new_outputs); + } if txn.is_empty() { txn = self.check_spend_local_transaction(tx, height); } @@ -1288,7 +1293,7 @@ impl ChannelMonitor { } } } - outputs + watch_outputs } pub fn would_broadcast_at_height(&self, height: u32) -> bool {