X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fchain%2Fchannelmonitor.rs;h=6c11703d3158fc86567a49b2c81d771a1b8517b0;hb=6a94ff9daeb6f5f38cbb16b7f46c8480924592cd;hp=eba96ea6a5ceed1bbbcffbe042ccf90bd2e7d8e0;hpb=74717d390c3ac654978d1a22123d9085729529cd;p=rust-lightning

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index eba96ea6..6c11703d 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -37,9 +37,9 @@ use ln::{PaymentHash, PaymentPreimage};
 use ln::msgs::DecodeError;
 use ln::chan_utils;
 use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCType, ChannelTransactionParameters, HolderCommitmentTransaction};
-use ln::channelmanager::{BestBlock, HTLCSource};
+use ln::channelmanager::HTLCSource;
 use chain;
-use chain::WatchedOutput;
+use chain::{BestBlock, WatchedOutput};
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use chain::transaction::{OutPoint, TransactionData};
 use chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface};
@@ -53,9 +53,9 @@ use util::events::Event;
 
 use prelude::*;
 use core::{cmp, mem};
-use std::io::Error;
+use io::{self, Error};
 use core::ops::Deref;
-use std::sync::Mutex;
+use sync::Mutex;
 
 /// An update generated by the underlying Channel itself which contains some new information the
 /// ChannelMonitor should be made aware of.
@@ -88,19 +88,19 @@ pub struct ChannelMonitorUpdate {
 pub const CLOSED_CHANNEL_UPDATE_ID: u64 = core::u64::MAX;
 
 impl Writeable for ChannelMonitorUpdate {
-	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 		write_ver_prefix!(w, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
 		self.update_id.write(w)?;
 		(self.updates.len() as u64).write(w)?;
 		for update_step in self.updates.iter() {
 			update_step.write(w)?;
 		}
-		write_tlv_fields!(w, {}, {});
+		write_tlv_fields!(w, {});
 		Ok(())
 	}
 }
 impl Readable for ChannelMonitorUpdate {
-	fn read<R: ::std::io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+	fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
 		let _ver = read_ver_prefix!(r, SERIALIZATION_VERSION);
 		let update_id: u64 = Readable::read(r)?;
 		let len: u64 = Readable::read(r)?;
@@ -108,13 +108,13 @@ impl Readable for ChannelMonitorUpdate {
 		for _ in 0..len {
 			updates.push(Readable::read(r)?);
 		}
-		read_tlv_fields!(r, {}, {});
+		read_tlv_fields!(r, {});
 		Ok(Self { update_id, updates })
 	}
 }
 
 /// An error enum representing a failure to persist a channel monitor update.
-#[derive(Clone, Debug)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 pub enum ChannelMonitorUpdateErr {
 	/// Used to indicate a temporary failure (eg connection to a watchtower or remote backup of
 	/// our state failed, but is expected to succeed at some point in the future).
@@ -199,14 +199,15 @@ pub enum MonitorEvent {
 pub struct HTLCUpdate {
 	pub(crate) payment_hash: PaymentHash,
 	pub(crate) payment_preimage: Option<PaymentPreimage>,
-	pub(crate) source: HTLCSource
+	pub(crate) source: HTLCSource,
+	pub(crate) onchain_value_satoshis: Option<u64>,
 }
 impl_writeable_tlv_based!(HTLCUpdate, {
-	(0, payment_hash),
-	(2, source),
-}, {
-	(4, payment_preimage)
-}, {});
+	(0, payment_hash, required),
+	(1, onchain_value_satoshis, option),
+	(2, source, required),
+	(4, payment_preimage, option),
+});
 
 /// If an HTLC expires within this many blocks, don't try to claim it in a shared transaction,
 /// instead claiming it in its own individual transaction.
@@ -273,15 +274,14 @@ struct HolderSignedTx {
 	htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
 }
 impl_writeable_tlv_based!(HolderSignedTx, {
-	(0, txid),
-	(2, revocation_key),
-	(4, a_htlc_key),
-	(6, b_htlc_key),
-	(8, delayed_payment_key),
-	(10, per_commitment_point),
-	(12, feerate_per_kw),
-}, {}, {
-	(14, htlc_outputs)
+	(0, txid, required),
+	(2, revocation_key, required),
+	(4, a_htlc_key, required),
+	(6, b_htlc_key, required),
+	(8, delayed_payment_key, required),
+	(10, per_commitment_point, required),
+	(12, feerate_per_kw, required),
+	(14, htlc_outputs, vec_type)
 });
 
 /// We use this to track counterparty commitment transactions and htlcs outputs and
@@ -295,7 +295,7 @@ struct CounterpartyCommitmentTransaction {
 }
 
 impl Writeable for CounterpartyCommitmentTransaction {
-	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 		w.write_all(&byte_utils::be64_to_array(self.per_htlc.len() as u64))?;
 		for (ref txid, ref htlcs) in self.per_htlc.iter() {
 			w.write_all(&txid[..])?;
@@ -305,15 +305,15 @@ impl Writeable for CounterpartyCommitmentTransaction {
 			}
 		}
 		write_tlv_fields!(w, {
-			(0, self.counterparty_delayed_payment_base_key),
-			(2, self.counterparty_htlc_base_key),
-			(4, self.on_counterparty_tx_csv),
-		}, {});
+			(0, self.counterparty_delayed_payment_base_key, required),
+			(2, self.counterparty_htlc_base_key, required),
+			(4, self.on_counterparty_tx_csv, required),
+		});
 		Ok(())
 	}
 }
 impl Readable for CounterpartyCommitmentTransaction {
-	fn read<R: ::std::io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+	fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
 		let counterparty_commitment_transaction = {
 			let per_htlc_len: u64 = Readable::read(r)?;
 			let mut per_htlc = HashMap::with_capacity(cmp::min(per_htlc_len as usize, MAX_ALLOC_SIZE / 64));
@@ -333,10 +333,10 @@ impl Readable for CounterpartyCommitmentTransaction {
 			let mut counterparty_htlc_base_key = OptionDeserWrapper(None);
 			let mut on_counterparty_tx_csv: u16 = 0;
 			read_tlv_fields!(r, {
-				(0, counterparty_delayed_payment_base_key),
-				(2, counterparty_htlc_base_key),
-				(4, on_counterparty_tx_csv),
-			}, {});
+				(0, counterparty_delayed_payment_base_key, required),
+				(2, counterparty_htlc_base_key, required),
+				(4, on_counterparty_tx_csv, required),
+			});
 			CounterpartyCommitmentTransaction {
 				counterparty_delayed_payment_base_key: counterparty_delayed_payment_base_key.0.unwrap(),
 				counterparty_htlc_base_key: counterparty_htlc_base_key.0.unwrap(),
@@ -372,8 +372,8 @@ impl OnchainEventEntry {
 		conf_threshold
 	}
 
-	fn has_reached_confirmation_threshold(&self, height: u32) -> bool {
-		height >= self.confirmation_threshold()
+	fn has_reached_confirmation_threshold(&self, best_block: &BestBlock) -> bool {
+		best_block.height() >= self.confirmation_threshold()
 	}
 }
 
@@ -387,6 +387,7 @@ enum OnchainEvent {
 	HTLCUpdate {
 		source: HTLCSource,
 		payment_hash: PaymentHash,
+		onchain_value_satoshis: Option<u64>,
 	},
 	MaturingOutput {
 		descriptor: SpendableOutputDescriptor,
@@ -394,19 +395,20 @@ enum OnchainEvent {
 }
 
 impl_writeable_tlv_based!(OnchainEventEntry, {
-	(0, txid),
-	(2, height),
-	(4, event),
-}, {}, {});
+	(0, txid, required),
+	(2, height, required),
+	(4, event, required),
+});
 
 impl_writeable_tlv_based_enum!(OnchainEvent,
 	(0, HTLCUpdate) => {
-		(0, source),
-		(2, payment_hash),
-	}, {}, {},
+		(0, source, required),
+		(1, onchain_value_satoshis, option),
+		(2, payment_hash, required),
+	},
 	(1, MaturingOutput) => {
-		(0, descriptor),
-	}, {}, {},
+		(0, descriptor, required),
+	},
 ;);
 
 #[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
@@ -436,31 +438,35 @@ pub(crate) enum ChannelMonitorUpdateStep {
 		/// think we've fallen behind!
 		should_broadcast: bool,
 	},
+	ShutdownScript {
+		scriptpubkey: Script,
+	},
 }
 
 impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
 	(0, LatestHolderCommitmentTXInfo) => {
-		(0, commitment_tx),
-	}, {}, {
-		(2, htlc_outputs),
+		(0, commitment_tx, required),
+		(2, htlc_outputs, vec_type),
 	},
 	(1, LatestCounterpartyCommitmentTXInfo) => {
-		(0, commitment_txid),
-		(2, commitment_number),
-		(4, their_revocation_point),
-	}, {}, {
-		(6, htlc_outputs),
+		(0, commitment_txid, required),
+		(2, commitment_number, required),
+		(4, their_revocation_point, required),
+		(6, htlc_outputs, vec_type),
 	},
 	(2, PaymentPreimage) => {
-		(0, payment_preimage),
-	}, {}, {},
+		(0, payment_preimage, required),
+	},
 	(3, CommitmentSecret) => {
-		(0, idx),
-		(2, secret),
-	}, {}, {},
+		(0, idx, required),
+		(2, secret, required),
+	},
 	(4, ChannelForceClosed) => {
-		(0, should_broadcast),
-	}, {}, {},
+		(0, should_broadcast, required),
+	},
+	(5, ShutdownScript) => {
+		(0, scriptpubkey, required),
+	},
 ;);
 
 /// A ChannelMonitor handles chain events (blocks connected and disconnected) and generates
@@ -493,7 +499,7 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
 	destination_script: Script,
 	broadcasted_holder_revokable_script: Option<(Script, PublicKey, PublicKey)>,
 	counterparty_payment_script: Script,
-	shutdown_script: Script,
+	shutdown_script: Option<Script>,
 
 	channel_keys_id: [u8; 32],
 	holder_revocation_basepoint: PublicKey,
@@ -669,7 +675,10 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 		}
 
 		self.counterparty_payment_script.write(writer)?;
-		self.shutdown_script.write(writer)?;
+		match &self.shutdown_script {
+			Some(script) => script.write(writer)?,
+			None => Script::new().write(writer)?,
+		}
 
 		self.channel_keys_id.write(writer)?;
 		self.holder_revocation_basepoint.write(writer)?;
@@ -792,14 +801,14 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 		self.lockdown_from_offchain.write(writer)?;
 		self.holder_tx_signed.write(writer)?;
 
-		write_tlv_fields!(writer, {}, {});
+		write_tlv_fields!(writer, {});
 
 		Ok(())
 	}
 }
 
 impl<Signer: Sign> ChannelMonitor<Signer> {
-	pub(crate) fn new(secp_ctx: Secp256k1<secp256k1::All>, keys: Signer, shutdown_pubkey: &PublicKey,
+	pub(crate) fn new(secp_ctx: Secp256k1<secp256k1::All>, keys: Signer, shutdown_script: Option<Script>,
 	                  on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
 	                  channel_parameters: &ChannelTransactionParameters,
 	                  funding_redeemscript: Script, channel_value_satoshis: u64,
@@ -808,8 +817,6 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 	                  best_block: BestBlock) -> ChannelMonitor<Signer> {
 
 		assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
-		let our_channel_close_key_hash = WPubkeyHash::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();
 		let payment_key_hash = WPubkeyHash::hash(&keys.pubkeys().payment_point.serialize());
 		let counterparty_payment_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_key_hash[..]).into_script();
 
@@ -1193,6 +1200,12 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 		txids.dedup();
 		txids
 	}
+
+	/// Gets the latest best block which was connected either via the [`chain::Listen`] or
+	/// [`chain::Confirm`] interfaces.
+	pub fn current_best_block(&self) -> BestBlock {
+		self.inner.lock().unwrap().best_block.clone()
+	}
 }
 
 impl<Signer: Sign> ChannelMonitorImpl<Signer> {
@@ -1335,7 +1348,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		macro_rules! claim_htlcs {
 			($commitment_number: expr, $txid: expr) => {
 				let htlc_claim_reqs = self.get_counterparty_htlc_output_claim_reqs($commitment_number, $txid, None);
-				self.onchain_tx_handler.update_claims_view(&Vec::new(), htlc_claim_reqs, self.best_block.height(), broadcaster, fee_estimator, logger);
+				self.onchain_tx_handler.update_claims_view(&Vec::new(), htlc_claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger);
 			}
 		}
 		if let Some(txid) = self.current_counterparty_commitment_txid {
@@ -1357,11 +1370,14 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		// *we* sign a holder commitment transaction, not when e.g. a watchtower broadcasts one of our
 		// holder commitment transactions.
 		if self.broadcasted_holder_revokable_script.is_some() {
-			let (claim_reqs, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, 0);
-			self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, self.best_block.height(), broadcaster, fee_estimator, logger);
+			// Assume that the broadcasted commitment transaction confirmed in the current best
+			// block. Even if not, its a reasonable metric for the bump criteria on the HTLC
+			// transactions.
+			let (claim_reqs, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height());
+			self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger);
 			if let Some(ref tx) = self.prev_holder_signed_commitment_tx {
-				let (claim_reqs, _) = self.get_broadcasted_holder_claims(&tx, 0);
-				self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, self.best_block.height(), broadcaster, fee_estimator, logger);
+				let (claim_reqs, _) = self.get_broadcasted_holder_claims(&tx, self.best_block.height());
+				self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger);
 			}
 		}
 	}
@@ -1427,7 +1443,13 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 						// shouldn't print the scary warning above.
 						log_info!(logger, "Channel off-chain state closed after we broadcasted our latest commitment transaction.");
 					}
-				}
+				},
+				ChannelMonitorUpdateStep::ShutdownScript { scriptpubkey } => {
+					log_trace!(logger, "Updating ChannelMonitor with shutdown script");
+					if let Some(shutdown_script) = self.shutdown_script.replace(scriptpubkey.clone()) {
+						panic!("Attempted to replace shutdown script {} with {}", shutdown_script, scriptpubkey);
+					}
+				},
 			}
 		}
 		self.latest_update_id = updates.update_id;
@@ -1569,6 +1591,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 										event: OnchainEvent::HTLCUpdate {
 											source: (**source).clone(),
 											payment_hash: htlc.payment_hash.clone(),
+											onchain_value_satoshis: Some(htlc.amount_msat / 1000),
 										},
 									};
 									log_info!(logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of revoked counterparty commitment transaction, waiting for confirmation (at height {})", log_bytes!(htlc.payment_hash.0), $commitment_tx, entry.confirmation_threshold());
@@ -1636,6 +1659,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 									event: OnchainEvent::HTLCUpdate {
 										source: (**source).clone(),
 										payment_hash: htlc.payment_hash.clone(),
+										onchain_value_satoshis: Some(htlc.amount_msat / 1000),
 									},
 								});
 							}
@@ -1728,7 +1752,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 	// Returns (1) `PackageTemplate`s that can be given to the OnChainTxHandler, so that the handler can
 	// broadcast transactions claiming holder HTLC commitment outputs and (2) a holder revokable
 	// script so we can detect whether a holder transaction has been seen on-chain.
-	fn get_broadcasted_holder_claims(&self, holder_tx: &HolderSignedTx, height: u32) -> (Vec<PackageTemplate>, Option<(Script, PublicKey, PublicKey)>) {
+	fn get_broadcasted_holder_claims(&self, holder_tx: &HolderSignedTx, conf_height: u32) -> (Vec<PackageTemplate>, Option<(Script, PublicKey, PublicKey)>) {
 		let mut claim_requests = Vec::with_capacity(holder_tx.htlc_outputs.len());
 
 		let redeemscript = chan_utils::get_revokeable_redeemscript(&holder_tx.revocation_key, self.on_holder_tx_csv, &holder_tx.delayed_payment_key);
@@ -1747,7 +1771,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 						};
 						HolderHTLCOutput::build_accepted(payment_preimage, htlc.amount_msat)
 					};
-				let htlc_package = PackageTemplate::build_package(holder_tx.txid, transaction_output_index, PackageSolvingData::HolderHTLCOutput(htlc_output), height, false, height);
+				let htlc_package = PackageTemplate::build_package(holder_tx.txid, transaction_output_index, PackageSolvingData::HolderHTLCOutput(htlc_output), htlc.cltv_expiry, false, conf_height);
 				claim_requests.push(htlc_package);
 			}
 		}
@@ -1774,27 +1798,6 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		let mut claim_requests = Vec::new();
 		let mut watch_outputs = Vec::new();
 
-		macro_rules! wait_threshold_conf {
-			($source: expr, $commitment_tx: expr, $payment_hash: expr) => {
-				self.onchain_events_awaiting_threshold_conf.retain(|ref entry| {
-					if entry.height != height { return true; }
-					match entry.event {
-						OnchainEvent::HTLCUpdate { source: ref update_source, .. } => {
-							*update_source != $source
-						},
-						_ => true,
-					}
-				});
-				let entry = OnchainEventEntry {
-					txid: commitment_txid,
-					height,
-					event: OnchainEvent::HTLCUpdate { source: $source, payment_hash: $payment_hash },
-				};
-				log_trace!(logger, "Failing HTLC with payment_hash {} from {} holder commitment tx due to broadcast of transaction, waiting confirmation (at height{})", log_bytes!($payment_hash.0), $commitment_tx, entry.confirmation_threshold());
-				self.onchain_events_awaiting_threshold_conf.push(entry);
-			}
-		}
-
 		macro_rules! append_onchain_update {
 			($updates: expr, $to_watch: expr) => {
 				claim_requests = $updates.0;
@@ -1823,11 +1826,30 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		}
 
 		macro_rules! fail_dust_htlcs_after_threshold_conf {
-			($holder_tx: expr) => {
+			($holder_tx: expr, $commitment_tx: expr) => {
 				for &(ref htlc, _, ref source) in &$holder_tx.htlc_outputs {
 					if htlc.transaction_output_index.is_none() {
 						if let &Some(ref source) = source {
-							wait_threshold_conf!(source.clone(), "lastest", htlc.payment_hash.clone());
+							self.onchain_events_awaiting_threshold_conf.retain(|ref entry| {
+								if entry.height != height { return true; }
+								match entry.event {
+									OnchainEvent::HTLCUpdate { source: ref update_source, .. } => {
+										update_source != source
+									},
+									_ => true,
+								}
+							});
+							let entry = OnchainEventEntry {
+								txid: commitment_txid,
+								height,
+								event: OnchainEvent::HTLCUpdate {
+									source: source.clone(), payment_hash: htlc.payment_hash,
+									onchain_value_satoshis: Some(htlc.amount_msat / 1000)
+								},
+							};
+							log_trace!(logger, "Failing HTLC with payment_hash {} from {} holder commitment tx due to broadcast of transaction, waiting confirmation (at height{})",
+								log_bytes!(htlc.payment_hash.0), $commitment_tx, entry.confirmation_threshold());
+							self.onchain_events_awaiting_threshold_conf.push(entry);
 						}
 					}
 				}
@@ -1835,9 +1857,9 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		}
 
 		if is_holder_tx {
-			fail_dust_htlcs_after_threshold_conf!(self.current_holder_commitment_tx);
+			fail_dust_htlcs_after_threshold_conf!(self.current_holder_commitment_tx, "latest");
 			if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx {
-				fail_dust_htlcs_after_threshold_conf!(holder_tx);
+				fail_dust_htlcs_after_threshold_conf!(holder_tx, "previous");
 			}
 		}
 
@@ -1860,7 +1882,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 				} else if htlc.0.cltv_expiry > self.best_block.height() + 1 {
 					// Don't broadcast HTLC-Timeout transactions immediately as they don't meet the
 					// current locktime requirements on-chain. We will broadcast them in
-					// `block_confirmed` when `would_broadcast_at_height` returns true.
+					// `block_confirmed` when `should_broadcast_holder_commitment_txn` returns true.
 					// Note that we add + 1 as transactions are broadcastable when they can be
 					// confirmed in the next block.
 					continue;
@@ -1930,13 +1952,13 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 
 		if height > self.best_block.height() {
 			self.best_block = BestBlock::new(block_hash, height);
-			self.block_confirmed(height, vec![], vec![], vec![], broadcaster, fee_estimator, logger)
-		} else {
+			self.block_confirmed(height, vec![], vec![], vec![], &broadcaster, &fee_estimator, &logger)
+		} else if block_hash != self.best_block.block_hash() {
 			self.best_block = BestBlock::new(block_hash, height);
 			self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.height <= height);
 			self.onchain_tx_handler.block_disconnected(height + 1, broadcaster, fee_estimator, logger);
 			Vec::new()
-		}
+		} else { Vec::new() }
 	}
 
 	fn transactions_confirmed<B: Deref, F: Deref, L: Deref>(
@@ -2008,33 +2030,49 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 			self.is_paying_spendable_output(&tx, height, &logger);
 		}
 
-		self.block_confirmed(height, txn_matched, watch_outputs, claimable_outpoints, broadcaster, fee_estimator, logger)
+		if height > self.best_block.height() {
+			self.best_block = BestBlock::new(block_hash, height);
+		}
+
+		self.block_confirmed(height, txn_matched, watch_outputs, claimable_outpoints, &broadcaster, &fee_estimator, &logger)
 	}
 
+	/// Update state for new block(s)/transaction(s) confirmed. Note that the caller must update
+	/// `self.best_block` before calling if a new best blockchain tip is available. More
+	/// concretely, `self.best_block` must never be at a lower height than `conf_height`, avoiding
+	/// complexity especially in `OnchainTx::update_claims_view`.
+	///
+	/// `conf_height` should be set to the height at which any new transaction(s)/block(s) were
+	/// confirmed at, even if it is not the current best height.
 	fn block_confirmed<B: Deref, F: Deref, L: Deref>(
 		&mut self,
-		height: u32,
+		conf_height: u32,
 		txn_matched: Vec<&Transaction>,
 		mut watch_outputs: Vec<TransactionOutputs>,
 		mut claimable_outpoints: Vec<PackageTemplate>,
-		broadcaster: B,
-		fee_estimator: F,
-		logger: L,
+		broadcaster: &B,
+		fee_estimator: &F,
+		logger: &L,
 	) -> Vec<TransactionOutputs>
 	where
 		B::Target: BroadcasterInterface,
 		F::Target: FeeEstimator,
 		L::Target: Logger,
 	{
-		let should_broadcast = self.would_broadcast_at_height(height, &logger);
+		debug_assert!(self.best_block.height() >= conf_height);
+
+		let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);
 		if should_broadcast {
 			let funding_outp = HolderFundingOutput::build(self.funding_redeemscript.clone());
-			let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), height, false, height);
+			let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), self.best_block.height(), false, self.best_block.height());
 			claimable_outpoints.push(commitment_package);
 			self.pending_monitor_events.push(MonitorEvent::CommitmentTxBroadcasted(self.funding_info.0));
 			let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
 			self.holder_tx_signed = true;
-			let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, height);
+			// Because we're broadcasting a commitment transaction, we should construct the package
+			// assuming it gets confirmed in the next block. Sadly, we have code which considers
+			// "not yet confirmed" things as discardable, so we cannot do that here.
+			let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height());
 			let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &commitment_tx);
 			if !new_outputs.is_empty() {
 				watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs));
@@ -2047,7 +2085,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 			self.onchain_events_awaiting_threshold_conf.drain(..).collect::<Vec<_>>();
 		let mut onchain_events_reaching_threshold_conf = Vec::new();
 		for entry in onchain_events_awaiting_threshold_conf {
-			if entry.has_reached_confirmation_threshold(height) {
+			if entry.has_reached_confirmation_threshold(&self.best_block) {
 				onchain_events_reaching_threshold_conf.push(entry);
 			} else {
 				self.onchain_events_awaiting_threshold_conf.push(entry);
@@ -2069,7 +2107,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		// Produce actionable events from on-chain events having reached their threshold.
 		for entry in onchain_events_reaching_threshold_conf.drain(..) {
 			match entry.event {
-				OnchainEvent::HTLCUpdate { ref source, payment_hash } => {
+				OnchainEvent::HTLCUpdate { ref source, payment_hash, onchain_value_satoshis } => {
 					// Check for duplicate HTLC resolutions.
 					#[cfg(debug_assertions)]
 					{
@@ -2088,9 +2126,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 
 					log_debug!(logger, "HTLC {} failure update has got enough confirmations to be passed upstream", log_bytes!(payment_hash.0));
 					self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
-						payment_hash: payment_hash,
+						payment_hash,
 						payment_preimage: None,
 						source: source.clone(),
+						onchain_value_satoshis,
 					}));
 				},
 				OnchainEvent::MaturingOutput { descriptor } => {
@@ -2102,7 +2141,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 			}
 		}
 
-		self.onchain_tx_handler.update_claims_view(&txn_matched, claimable_outpoints, height, &&*broadcaster, &&*fee_estimator, &&*logger);
+		self.onchain_tx_handler.update_claims_view(&txn_matched, claimable_outpoints, conf_height, self.best_block.height(), broadcaster, fee_estimator, logger);
 
 		// Determine new outputs to watch by comparing against previously known outputs to watch,
 		// updating the latter in the process.
@@ -2204,7 +2243,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		false
 	}
 
-	fn would_broadcast_at_height<L: Deref>(&self, height: u32, logger: &L) -> bool where L::Target: Logger {
+	fn should_broadcast_holder_commitment_txn<L: Deref>(&self, logger: &L) -> bool where L::Target: Logger {
 		// We need to consider all HTLCs which are:
 		//  * in any unrevoked counterparty commitment transaction, as they could broadcast said
 		//    transactions and we'd end up in a race, or
@@ -2215,6 +2254,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 		// to the source, and if we don't fail the channel we will have to ensure that the next
 		// updates that peer sends us are update_fails, failing the channel if not. It's probably
 		// easier to just fail the channel as this case should be rare enough anyway.
+		let height = self.best_block.height();
 		macro_rules! scan_commitment {
 			($htlcs: expr, $holder_tx: expr) => {
 				for ref htlc in $htlcs {
@@ -2306,7 +2346,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 							if pending_htlc.payment_hash == $htlc_output.payment_hash && pending_htlc.amount_msat == $htlc_output.amount_msat {
 								if let &Some(ref source) = pending_source {
 									log_claim!("revoked counterparty commitment tx", false, pending_htlc, true);
-									payment_data = Some(((**source).clone(), $htlc_output.payment_hash));
+									payment_data = Some(((**source).clone(), $htlc_output.payment_hash, $htlc_output.amount_msat));
 									break;
 								}
 							}
@@ -2326,7 +2366,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 								// transaction. This implies we either learned a preimage, the HTLC
 								// has timed out, or we screwed up. In any case, we should now
 								// resolve the source HTLC with the original sender.
-								payment_data = Some(((*source).clone(), htlc_output.payment_hash));
+								payment_data = Some(((*source).clone(), htlc_output.payment_hash, htlc_output.amount_msat));
 							} else if !$holder_tx {
 									check_htlc_valid_counterparty!(self.current_counterparty_commitment_txid, htlc_output);
 								if payment_data.is_none() {
@@ -2359,7 +2399,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 
 			// Check that scan_commitment, above, decided there is some source worth relaying an
 			// HTLC resolution backwards to and figure out whether we learned a preimage from it.
-			if let Some((source, payment_hash)) = payment_data {
+			if let Some((source, payment_hash, amount_msat)) = payment_data {
 				let mut payment_preimage = PaymentPreimage([0; 32]);
 				if accepted_preimage_claim {
 					if !self.pending_monitor_events.iter().any(
@@ -2368,7 +2408,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 						self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
 							source,
 							payment_preimage: Some(payment_preimage),
-							payment_hash
+							payment_hash,
+							onchain_value_satoshis: Some(amount_msat / 1000),
 						}));
 					}
 				} else if offered_preimage_claim {
@@ -2380,7 +2421,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 						self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
 							source,
 							payment_preimage: Some(payment_preimage),
-							payment_hash
+							payment_hash,
+							onchain_value_satoshis: Some(amount_msat / 1000),
 						}));
 					}
 				} else {
@@ -2396,7 +2438,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 					let entry = OnchainEventEntry {
 						txid: tx.txid(),
 						height,
-						event: OnchainEvent::HTLCUpdate { source: source, payment_hash: payment_hash },
+						event: OnchainEvent::HTLCUpdate {
+							source, payment_hash,
+							onchain_value_satoshis: Some(amount_msat / 1000),
+						},
 					};
 					log_info!(logger, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height {})", log_bytes!(payment_hash.0), entry.confirmation_threshold());
 					self.onchain_events_awaiting_threshold_conf.push(entry);
@@ -2429,7 +2474,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 					output: outp.clone(),
 				});
 				break;
-			} else if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script {
+			}
+			if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script {
 				if broadcasted_holder_revokable_script.0 == outp.script_pubkey {
 					spendable_output =  Some(SpendableOutputDescriptor::DelayedPaymentOutput(DelayedPaymentOutputDescriptor {
 						outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
@@ -2442,7 +2488,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 					}));
 					break;
 				}
-			} else if self.counterparty_payment_script == outp.script_pubkey {
+			}
+			if self.counterparty_payment_script == outp.script_pubkey {
 				spendable_output = Some(SpendableOutputDescriptor::StaticPaymentOutput(StaticPaymentOutputDescriptor {
 					outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
 					output: outp.clone(),
@@ -2450,11 +2497,13 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 					channel_value_satoshis: self.channel_value_satoshis,
 				}));
 				break;
-			} else if outp.script_pubkey == self.shutdown_script {
+			}
+			if self.shutdown_script.as_ref() == Some(&outp.script_pubkey) {
 				spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
 					outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
 					output: outp.clone(),
 				});
+				break;
 			}
 		}
 		if let Some(spendable_output) = spendable_output {
@@ -2559,7 +2608,7 @@ const MAX_ALLOC_SIZE: usize = 64*1024;
 
 impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 		for (BlockHash, ChannelMonitor<Signer>) {
-	fn read<R: ::std::io::Read>(reader: &mut R, keys_manager: &'a K) -> Result<Self, DecodeError> {
+	fn read<R: io::Read>(reader: &mut R, keys_manager: &'a K) -> Result<Self, DecodeError> {
 		macro_rules! unwrap_obj {
 			($key: expr) => {
 				match $key {
@@ -2586,7 +2635,10 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 			_ => return Err(DecodeError::InvalidValue),
 		};
 		let counterparty_payment_script = Readable::read(reader)?;
-		let shutdown_script = Readable::read(reader)?;
+		let shutdown_script = {
+			let script = <Script as Readable>::read(reader)?;
+			if script.is_empty() { None } else { Some(script) }
+		};
 
 		let channel_keys_id = Readable::read(reader)?;
 		let holder_revocation_basepoint = Readable::read(reader)?;
@@ -2740,7 +2792,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 		let lockdown_from_offchain = Readable::read(reader)?;
 		let holder_tx_signed = Readable::read(reader)?;
 
-		read_tlv_fields!(reader, {}, {});
+		read_tlv_fields!(reader, {});
 
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
@@ -2811,17 +2863,18 @@ mod tests {
 	use bitcoin::hash_types::Txid;
 	use bitcoin::network::constants::Network;
 	use hex;
+	use chain::BestBlock;
 	use chain::channelmonitor::ChannelMonitor;
 	use chain::package::{WEIGHT_OFFERED_HTLC, WEIGHT_RECEIVED_HTLC, WEIGHT_REVOKED_OFFERED_HTLC, WEIGHT_REVOKED_RECEIVED_HTLC, WEIGHT_REVOKED_OUTPUT};
 	use chain::transaction::OutPoint;
 	use ln::{PaymentPreimage, PaymentHash};
-	use ln::channelmanager::BestBlock;
 	use ln::chan_utils;
 	use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
+	use ln::script::ShutdownScript;
 	use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
 	use bitcoin::secp256k1::key::{SecretKey,PublicKey};
 	use bitcoin::secp256k1::Secp256k1;
-	use std::sync::{Arc, Mutex};
+	use sync::{Arc, Mutex};
 	use chain::keysinterface::InMemorySigner;
 	use prelude::*;
 
@@ -2830,7 +2883,7 @@ mod tests {
 		let secp_ctx = Secp256k1::new();
 		let logger = Arc::new(TestLogger::new());
 		let broadcaster = Arc::new(TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))});
-		let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: 253 });
+		let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: Mutex::new(253) });
 
 		let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let dummy_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };
@@ -2911,9 +2964,10 @@ mod tests {
 		};
 		// Prune with one old state and a holder commitment tx holding a few overlaps with the
 		// old state.
+		let shutdown_pubkey = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 		let best_block = BestBlock::from_genesis(Network::Testnet);
 		let monitor = ChannelMonitor::new(Secp256k1::new(), keys,
-		                                  &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
+		                                  Some(ShutdownScript::new_p2wpkh_from_pubkey(shutdown_pubkey).into_inner()), 0, &Script::new(),
 		                                  (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
 		                                  &channel_parameters,
 		                                  Script::new(), 46, 0,