X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fchain%2Fchannelmonitor.rs;h=3c75ec9e99ecd68baa51be9d1f60eb8e39becb2f;hb=9d8d24f6906d6fbdc6c02a88e5e2298c1fa50825;hp=cf368f15259cad2e9f03e3cc3728eb61b80a701b;hpb=002a5db5b07fd317545d3aa4f1a217cd40b608e2;p=rust-lightning

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index cf368f15..3c75ec9e 100644
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -53,7 +53,7 @@ use util::events::Event;
 
 use prelude::*;
 use core::{cmp, mem};
-use std::io::Error;
+use io::{self, Error};
 use core::ops::Deref;
 use sync::Mutex;
 
@@ -88,7 +88,7 @@ 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)?;
@@ -100,7 +100,7 @@ impl Writeable for ChannelMonitorUpdate {
 	}
 }
 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)?;
@@ -114,7 +114,7 @@ impl Readable for ChannelMonitorUpdate {
 }
 
 /// 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,10 +199,12 @@ 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, required),
+	(1, onchain_value_satoshis, option),
 	(2, source, required),
 	(4, payment_preimage, option),
 });
@@ -293,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[..])?;
@@ -311,7 +313,7 @@ impl Writeable for CounterpartyCommitmentTransaction {
 	}
 }
 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));
@@ -385,6 +387,7 @@ enum OnchainEvent {
 	HTLCUpdate {
 		source: HTLCSource,
 		payment_hash: PaymentHash,
+		onchain_value_satoshis: Option<u64>,
 	},
 	MaturingOutput {
 		descriptor: SpendableOutputDescriptor,
@@ -400,6 +403,7 @@ impl_writeable_tlv_based!(OnchainEventEntry, {
 impl_writeable_tlv_based_enum!(OnchainEvent,
 	(0, HTLCUpdate) => {
 		(0, source, required),
+		(1, onchain_value_satoshis, option),
 		(2, payment_hash, required),
 	},
 	(1, MaturingOutput) => {
@@ -434,6 +438,9 @@ pub(crate) enum ChannelMonitorUpdateStep {
 		/// think we've fallen behind!
 		should_broadcast: bool,
 	},
+	ShutdownScript {
+		scriptpubkey: Script,
+	},
 }
 
 impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
@@ -457,6 +464,9 @@ impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
 	(4, ChannelForceClosed) => {
 		(0, should_broadcast, required),
 	},
+	(5, ShutdownScript) => {
+		(0, scriptpubkey, required),
+	},
 ;);
 
 /// A ChannelMonitor handles chain events (blocks connected and disconnected) and generates
@@ -489,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,
@@ -506,6 +516,9 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
 	on_holder_tx_csv: u16,
 
 	commitment_secrets: CounterpartyCommitmentSecrets,
+	/// The set of outpoints in each counterparty commitment transaction. We always need at least
+	/// the payment hash from `HTLCOutputInCommitment` to claim even a revoked commitment
+	/// transaction broadcast as we need to be able to construct the witness script in all cases.
 	counterparty_claimable_outpoints: HashMap<Txid, Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>,
 	/// We cannot identify HTLC-Success or HTLC-Timeout transactions by themselves on the chain.
 	/// Nor can we figure out their commitment numbers without the commitment transaction they are
@@ -665,7 +678,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)?;
@@ -795,7 +811,7 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 }
 
 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,
@@ -804,8 +820,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();
 
@@ -1197,6 +1211,81 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 	}
 }
 
+/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
+/// failing any HTLCs which didn't make it into the broadcasted commitment transaction back
+/// after ANTI_REORG_DELAY blocks.
+///
+/// We always compare against the set of HTLCs in counterparty commitment transactions, as those
+/// are the commitment transactions which are generated by us. The off-chain state machine in
+/// `Channel` will automatically resolve any HTLCs which were never included in a commitment
+/// transaction when it detects channel closure, but it is up to us to ensure any HTLCs which were
+/// included in a remote commitment transaction are failed back if they are not present in the
+/// broadcasted commitment transaction.
+///
+/// Specifically, the removal process for HTLCs in `Channel` is always based on the counterparty
+/// sending a `revoke_and_ack`, which causes us to clear `prev_counterparty_commitment_txid`. Thus,
+/// as long as we examine both the current counterparty commitment transaction and, if it hasn't
+/// been revoked yet, the previous one, we we will never "forget" to resolve an HTLC.
+macro_rules! fail_unbroadcast_htlcs {
+	($self: expr, $commitment_tx_type: expr, $commitment_tx_conf_height: expr, $confirmed_htlcs_list: expr, $logger: expr) => { {
+		macro_rules! check_htlc_fails {
+			($txid: expr, $commitment_tx: expr) => {
+				if let Some(ref latest_outpoints) = $self.counterparty_claimable_outpoints.get($txid) {
+					for &(ref htlc, ref source_option) in latest_outpoints.iter() {
+						if let &Some(ref source) = source_option {
+							// Check if the HTLC is present in the commitment transaction that was
+							// broadcast, but not if it was below the dust limit, which we should
+							// fail backwards immediately as there is no way for us to learn the
+							// payment_preimage.
+							// Note that if the dust limit were allowed to change between
+							// commitment transactions we'd want to be check whether *any*
+							// broadcastable commitment transaction has the HTLC in it, but it
+							// cannot currently change after channel initialization, so we don't
+							// need to here.
+							let confirmed_htlcs_iter: &mut Iterator<Item = (&HTLCOutputInCommitment, Option<&HTLCSource>)> = &mut $confirmed_htlcs_list;
+							let mut matched_htlc = false;
+							for (ref broadcast_htlc, ref broadcast_source) in confirmed_htlcs_iter {
+								if broadcast_htlc.transaction_output_index.is_some() && Some(&**source) == *broadcast_source {
+									matched_htlc = true;
+									break;
+								}
+							}
+							if matched_htlc { continue; }
+							$self.onchain_events_awaiting_threshold_conf.retain(|ref entry| {
+								if entry.height != $commitment_tx_conf_height { return true; }
+								match entry.event {
+									OnchainEvent::HTLCUpdate { source: ref update_source, .. } => {
+										*update_source != **source
+									},
+									_ => true,
+								}
+							});
+							let entry = OnchainEventEntry {
+								txid: *$txid,
+								height: $commitment_tx_conf_height,
+								event: OnchainEvent::HTLCUpdate {
+									source: (**source).clone(),
+									payment_hash: htlc.payment_hash.clone(),
+									onchain_value_satoshis: Some(htlc.amount_msat / 1000),
+								},
+							};
+							log_trace!($logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of {} commitment transaction, waiting for confirmation (at height {})",
+								log_bytes!(htlc.payment_hash.0), $commitment_tx, $commitment_tx_type, entry.confirmation_threshold());
+							$self.onchain_events_awaiting_threshold_conf.push(entry);
+						}
+					}
+				}
+			}
+		}
+		if let Some(ref txid) = $self.current_counterparty_commitment_txid {
+			check_htlc_fails!(txid, "current");
+		}
+		if let Some(ref txid) = $self.prev_counterparty_commitment_txid {
+			check_htlc_fails!(txid, "previous");
+		}
+	} }
+}
+
 impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 	/// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither
 	/// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen
@@ -1432,7 +1521,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;
@@ -1554,42 +1649,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 				}
 				self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
-				macro_rules! check_htlc_fails {
-					($txid: expr, $commitment_tx: expr) => {
-						if let Some(ref outpoints) = self.counterparty_claimable_outpoints.get($txid) {
-							for &(ref htlc, ref source_option) in outpoints.iter() {
-								if let &Some(ref source) = source_option {
-									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: *$txid,
-										height,
-										event: OnchainEvent::HTLCUpdate {
-											source: (**source).clone(),
-											payment_hash: htlc.payment_hash.clone(),
-										},
-									};
-									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());
-									self.onchain_events_awaiting_threshold_conf.push(entry);
-								}
-							}
-						}
-					}
-				}
-				if let Some(ref txid) = self.current_counterparty_commitment_txid {
-					check_htlc_fails!(txid, "current");
-				}
-				if let Some(ref txid) = self.prev_counterparty_commitment_txid {
-					check_htlc_fails!(txid, "counterparty");
-				}
-				// No need to check holder commitment txn, symmetric HTLCSource must be present as per-htlc data on counterparty commitment tx
+				fail_unbroadcast_htlcs!(self, "revoked counterparty", height, [].iter().map(|a| *a), logger);
 			}
 		} else if let Some(per_commitment_data) = per_commitment_option {
 			// While this isn't useful yet, there is a potential race where if a counterparty
@@ -1605,55 +1665,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 			self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
 
 			log_info!(logger, "Got broadcast of non-revoked counterparty commitment transaction {}", commitment_txid);
-
-			macro_rules! check_htlc_fails {
-				($txid: expr, $commitment_tx: expr, $id: tt) => {
-					if let Some(ref latest_outpoints) = self.counterparty_claimable_outpoints.get($txid) {
-						$id: for &(ref htlc, ref source_option) in latest_outpoints.iter() {
-							if let &Some(ref source) = source_option {
-								// Check if the HTLC is present in the commitment transaction that was
-								// broadcast, but not if it was below the dust limit, which we should
-								// fail backwards immediately as there is no way for us to learn the
-								// payment_preimage.
-								// Note that if the dust limit were allowed to change between
-								// commitment transactions we'd want to be check whether *any*
-								// broadcastable commitment transaction has the HTLC in it, but it
-								// cannot currently change after channel initialization, so we don't
-								// need to here.
-								for &(ref broadcast_htlc, ref broadcast_source) in per_commitment_data.iter() {
-									if broadcast_htlc.transaction_output_index.is_some() && Some(source) == broadcast_source.as_ref() {
-										continue $id;
-									}
-								}
-								log_trace!(logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of counterparty commitment transaction", log_bytes!(htlc.payment_hash.0), $commitment_tx);
-								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,
-									}
-								});
-								self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry {
-									txid: *$txid,
-									height,
-									event: OnchainEvent::HTLCUpdate {
-										source: (**source).clone(),
-										payment_hash: htlc.payment_hash.clone(),
-									},
-								});
-							}
-						}
-					}
-				}
-			}
-			if let Some(ref txid) = self.current_counterparty_commitment_txid {
-				check_htlc_fails!(txid, "current", 'current_loop);
-			}
-			if let Some(ref txid) = self.prev_counterparty_commitment_txid {
-				check_htlc_fails!(txid, "previous", 'prev_loop);
-			}
+			fail_unbroadcast_htlcs!(self, "counterparty", height, per_commitment_data.iter().map(|(a, b)| (a, b.as_ref().map(|b| b.as_ref()))), logger);
 
 			let htlc_claim_reqs = self.get_counterparty_htlc_output_claim_reqs(commitment_number, commitment_txid, Some(tx));
 			for req in htlc_claim_reqs {
@@ -1779,27 +1791,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;
@@ -1817,6 +1808,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 			let res = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, height);
 			let mut to_watch = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, tx);
 			append_onchain_update!(res, to_watch);
+			fail_unbroadcast_htlcs!(self, "latest holder", height, self.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger);
 		} else if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx {
 			if holder_tx.txid == commitment_txid {
 				is_holder_tx = true;
@@ -1824,26 +1816,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
 				let res = self.get_broadcasted_holder_claims(holder_tx, height);
 				let mut to_watch = self.get_broadcasted_holder_watch_outputs(holder_tx, tx);
 				append_onchain_update!(res, to_watch);
-			}
-		}
-
-		macro_rules! fail_dust_htlcs_after_threshold_conf {
-			($holder_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());
-						}
-					}
-				}
+				fail_unbroadcast_htlcs!(self, "previous holder", height, holder_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref())), logger);
 			}
 		}
 
 		if is_holder_tx {
-			fail_dust_htlcs_after_threshold_conf!(self.current_holder_commitment_tx);
-			if let &Some(ref holder_tx) = &self.prev_holder_signed_commitment_tx {
-				fail_dust_htlcs_after_threshold_conf!(holder_tx);
-			}
 		}
 
 		(claim_requests, (commitment_txid, watch_outputs))
@@ -2090,7 +2067,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)]
 					{
@@ -2109,9 +2086,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 } => {
@@ -2328,7 +2306,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;
 								}
 							}
@@ -2348,7 +2326,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() {
@@ -2381,7 +2359,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(
@@ -2390,7 +2368,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 {
@@ -2402,7 +2381,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 {
@@ -2418,7 +2398,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);
@@ -2451,7 +2434,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 },
@@ -2464,7 +2448,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(),
@@ -2472,11 +2457,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 {
@@ -2581,7 +2568,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 {
@@ -2608,7 +2595,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)?;
@@ -2840,6 +2830,7 @@ mod tests {
 	use ln::{PaymentPreimage, PaymentHash};
 	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;
@@ -2933,9 +2924,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,