From e286afd0b07ee6e027a07ad9e097f6d4f5fee09b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 17 Apr 2020 19:31:24 -0400 Subject: [PATCH 1/5] Drop uneccessary indirection in map-updating in 1107ab06c3 1107ab06c33bd360bdee7ee64f4b690e753003f6 added a Vec of future updates to apply during a loop, fixing a borrow checker issue that didn't exist in the merged version of the patch. This simply reverts that small part of the change. --- lightning/src/ln/onchaintx.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 2f08fe291d2..3b8238ddcae 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -713,18 +713,14 @@ impl OnchainTxHandler { // Build, bump and rebroadcast tx accordingly log_trace!(self, "Bumping {} candidates", bump_candidates.len()); - let mut pending_claim_updates = Vec::with_capacity(bump_candidates.len()); for (first_claim_txid, claim_material) in bump_candidates.iter() { if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &claim_material, &*fee_estimator) { log_trace!(self, "Broadcast onchain {}", log_tx!(bump_tx)); broadcaster.broadcast_transaction(&bump_tx); - pending_claim_updates.push((*first_claim_txid, new_timer, new_feerate)); - } - } - for updates in pending_claim_updates { - if let Some(claim_material) = self.pending_claim_requests.get_mut(&updates.0) { - claim_material.height_timer = updates.1; - claim_material.feerate_previous = updates.2; + if let Some(claim_material) = self.pending_claim_requests.get_mut(first_claim_txid) { + claim_material.height_timer = new_timer; + claim_material.feerate_previous = new_feerate; + } } } } From 41b3be7ad00fd65b38a5b55376cb2554f483348b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 17 Apr 2020 21:06:40 -0400 Subject: [PATCH 2/5] Fix new rustc warnings for unnecessary parenthesis --- lightning/src/chain/keysinterface.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 58bdd722609..fb163966f9a 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -620,6 +620,6 @@ impl KeysInterface for KeysManager { let child_privkey = self.channel_id_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(child_ix as u32).expect("key space exhausted")).expect("Your RNG is busted"); sha.input(&child_privkey.private_key.key[..]); - (Sha256::from_engine(sha).into_inner()) + Sha256::from_engine(sha).into_inner() } } From 7e0b57615f568f773a7b742c10997985ecbbbb75 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 17 Apr 2020 21:09:08 -0400 Subject: [PATCH 3/5] Concretize some types in fuzz, addressing new rustc warnings --- fuzz/src/chanmon_consistency.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 3d14808b016..07a632d0a4e 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -73,9 +73,9 @@ impl Writer for VecWriter { } } -pub struct TestChannelMonitor { +struct TestChannelMonitor { pub logger: Arc, - pub simple_monitor: Arc, Arc>>, + pub simple_monitor: Arc, Arc>>, pub update_ret: Mutex>, // If we reload a node with an old copy of ChannelMonitors, the ChannelManager deserialization // logic will automatically force-close our channels for us (as we don't have an up-to-date @@ -86,7 +86,7 @@ pub struct TestChannelMonitor { pub should_update_manager: atomic::AtomicBool, } impl TestChannelMonitor { - pub fn new(chain_monitor: Arc, broadcaster: Arc, logger: Arc, feeest: Arc) -> Self { + pub fn new(chain_monitor: Arc, broadcaster: Arc, logger: Arc, feeest: Arc) -> Self { Self { simple_monitor: Arc::new(channelmonitor::SimpleManyChannelMonitor::new(chain_monitor, broadcaster, logger.clone(), feeest)), logger, From f5b0663f6a8f437c8d86433a07cb5aeca53b8c86 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 17 Apr 2020 21:14:54 -0400 Subject: [PATCH 4/5] Drop std::error::Error impl for DecodeError It appears to be effectively-deprecated in Rust now, and didn't really appear to serve a lot of purpose anyway. --- lightning/src/ln/msgs.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 9698798c22e..b17b4aa083d 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -23,7 +23,6 @@ use bitcoin::blockdata::script::Script; use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures}; -use std::error::Error; use std::{cmp, fmt}; use std::io::Read; use std::result::Result; @@ -688,21 +687,16 @@ pub(crate) struct OnionErrorPacket { pub(crate) data: Vec, } -impl Error for DecodeError { - fn description(&self) -> &str { - match *self { - DecodeError::UnknownVersion => "Unknown realm byte in Onion packet", - DecodeError::UnknownRequiredFeature => "Unknown required feature preventing decode", - DecodeError::InvalidValue => "Nonsense bytes didn't map to the type they were interpreted as", - DecodeError::ShortRead => "Packet extended beyond the provided bytes", - DecodeError::BadLengthDescriptor => "A length descriptor in the packet didn't describe the later data correctly", - DecodeError::Io(ref e) => e.description(), - } - } -} impl fmt::Display for DecodeError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str(self.description()) + match *self { + DecodeError::UnknownVersion => f.write_str("Unknown realm byte in Onion packet"), + DecodeError::UnknownRequiredFeature => f.write_str("Unknown required feature preventing decode"), + DecodeError::InvalidValue => f.write_str("Nonsense bytes didn't map to the type they were interpreted as"), + DecodeError::ShortRead => f.write_str("Packet extended beyond the provided bytes"), + DecodeError::BadLengthDescriptor => f.write_str("A length descriptor in the packet didn't describe the later data correctly"), + DecodeError::Io(ref e) => e.fmt(f), + } } } From c89514c37c8c2632cb32f1fb00764f9d7f2ce7f8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 18 Apr 2020 21:33:54 -0400 Subject: [PATCH 5/5] De-Option<> some fields in ChannelMonitor which are set at init After we moved the ChannelMonitor creation later during Channel init, we never went back and cleaned up ChannelMonitor to remove a number of now-useless Option<>s, so we do that now. --- lightning/src/ln/channelmanager.rs | 4 +- lightning/src/ln/channelmonitor.rs | 120 ++++++++-------------- lightning/src/ln/functional_test_utils.rs | 4 +- lightning/src/ln/functional_tests.rs | 14 +-- lightning/src/util/macro_logger.rs | 7 +- 5 files changed, 57 insertions(+), 92 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f9ccc18a0c7..62ac9866396 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1432,7 +1432,7 @@ impl ChannelMan }; // Because we have exclusive ownership of the channel here we can release the channel_state // lock before add_monitor - if let Err(e) = self.monitor.add_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) { + if let Err(e) = self.monitor.add_monitor(chan_monitor.get_funding_txo(), chan_monitor) { match e { ChannelMonitorUpdateErr::PermanentFailure => { match handle_error!(self, Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure", *temporary_channel_id, chan.force_shutdown(true), None)), chan.get_their_node_id()) { @@ -2275,7 +2275,7 @@ impl ChannelMan }; // Because we have exclusive ownership of the channel here we can release the channel_state // lock before add_monitor - if let Err(e) = self.monitor.add_monitor(monitor_update.get_funding_txo().unwrap(), monitor_update) { + if let Err(e) = self.monitor.add_monitor(monitor_update.get_funding_txo(), monitor_update) { match e { ChannelMonitorUpdateErr::PermanentFailure => { // Note that we reply with the new channel_id in error messages if we gave up on the diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index e88dd33395c..7a6bac9c7fb 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -290,16 +290,9 @@ impl return Err(MonitorUpdateError("Channel monitor for given key is already present")), hash_map::Entry::Vacant(e) => e, }; - match monitor.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); - }, - } + log_trace!(self, "Got new Channel Monitor for channel {}", log_bytes!(monitor.funding_info.0.to_channel_id()[..])); + self.chain_monitor.install_watch_tx(&monitor.funding_info.0.txid, &monitor.funding_info.1); + self.chain_monitor.install_watch_outpoint((monitor.funding_info.0.txid, monitor.funding_info.0.index as u32), &monitor.funding_info.1); for (txid, outputs) in monitor.get_outputs_to_watch().iter() { for (idx, script) in outputs.iter().enumerate() { self.chain_monitor.install_watch_outpoint((*txid, idx as u32), script); @@ -721,19 +714,19 @@ pub struct ChannelMonitor { shutdown_script: Script, keys: ChanSigner, - funding_info: Option<(OutPoint, Script)>, + funding_info: (OutPoint, Script), current_remote_commitment_txid: Option, prev_remote_commitment_txid: Option, - their_htlc_base_key: Option, - their_delayed_payment_base_key: Option, - funding_redeemscript: Option