Include block hash for watched transaction output

When registering a watched transaction output, any in-block descendant
transactions spending the output must be supplied. Give the block hash
when registering such outputs such that this is possible. Otherwise,
spends from other blocks may be returned inadvertently.

Author: jkczyz

lightning/src/chain/chainmonitor.rs

@@ -26,7 +26,7 @@
 use bitcoin::blockdata::block::{Block, BlockHeader};
 
 use chain;
-use chain::Filter;
+use chain::{Filter, WatchedOutput};
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use chain::channelmonitor;
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, MonitorEvent, Persist};
@@ -87,9 +87,14 @@ where C::Target: chain::Filter,
 			let mut txn_outputs = monitor.block_connected(header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
 
 			if let Some(ref chain_source) = self.chain_source {
+				let block_hash = header.block_hash();
 				for (txid, outputs) in txn_outputs.drain(..) {
 					for (idx, output) in outputs.iter() {
-						chain_source.register_output(&OutPoint { txid, index: *idx as u16 }, &output.script_pubkey);
+						chain_source.register_output(WatchedOutput {
+							block_hash: Some(block_hash),
+							outpoint: OutPoint { txid, index: *idx as u16 },
+							script_pubkey: output.script_pubkey.clone(),
+						});
 					}
 				}
 			}

lightning/src/chain/channelmonitor.rs

@@ -40,6 +40,7 @@ use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLC
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
 use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 use chain;
+use chain::WatchedOutput;
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use chain::transaction::{OutPoint, TransactionData};
 use chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface};
@@ -1174,7 +1175,11 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 		for (txid, outputs) in lock.get_outputs_to_watch().iter() {
 			for (index, script_pubkey) in outputs.iter() {
 				assert!(*index <= u16::max_value() as u32);
-				filter.register_output(&OutPoint { txid: *txid, index: *index as u16 }, script_pubkey);
+				filter.register_output(WatchedOutput {
+					block_hash: None,
+					outpoint: OutPoint { txid: *txid, index: *index as u16 },
+					script_pubkey: script_pubkey.clone(),
+				});
 			}
 		}
 	}

lightning/src/chain/mod.rs

@@ -129,12 +129,38 @@ pub trait Filter: Send + Sync {
 	/// a spending condition.
 	fn register_tx(&self, txid: &Txid, script_pubkey: &Script);
 
-	/// Registers interest in spends of a transaction output identified by `outpoint` having
-	/// `script_pubkey` as the spending condition.
+	/// Registers interest in spends of a transaction output.
 	///
-	/// Optionally, returns any transaction dependent on the output. This is useful for Electrum
-	/// clients to facilitate registering in-block descendants.
-	fn register_output(&self, outpoint: &OutPoint, script_pubkey: &Script) -> Option<(usize, Transaction)>;
+	/// Optionally, when `output.block_hash` is set, should return any transaction spending the
+	/// output that is found in the corresponding block along with its index.
+	///
+	/// This return value is useful for Electrum clients in order to supply in-block descendant
+	/// transactions which otherwise were not included. This is not necessary for other clients if
+	/// such descendant transactions were already included (e.g., when a BIP 157 client provides the
+	/// full block).
+	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)>;
+}
+
+/// A transaction output watched by a [`ChannelMonitor`] for spends on-chain.
+///
+/// Used to convey to a [`Filter`] such an output with a given spending condition. Any transaction
+/// spending the output must be given to [`ChannelMonitor::block_connected`] either directly or via
+/// the return value of [`Filter::register_output`].
+///
+/// If `block_hash` is `Some`, this indicates the output was created in the corresponding block and
+/// may have been spent there. See [`Filter::register_output`] for details.
+///
+/// [`ChannelMonitor`]: channelmonitor::ChannelMonitor
+/// [`ChannelMonitor::block_connected`]: channelmonitor::ChannelMonitor::block_connected
+pub struct WatchedOutput {
+	/// First block where the transaction output may have been spent.
+	pub block_hash: Option<BlockHash>,
+
+	/// Outpoint identifying the transaction output.
+	pub outpoint: OutPoint,
+
+	/// Spending condition of the transaction output.
+	pub script_pubkey: Script,
 }
 
 impl<T: Listen> Listen for std::ops::Deref<Target = T> {

lightning/src/util/test_utils.rs

@@ -8,6 +8,7 @@
 // licenses.
 
 use chain;
+use chain::WatchedOutput;
 use chain::chaininterface;
 use chain::chaininterface::ConfirmationTarget;
 use chain::chainmonitor;
@@ -546,8 +547,8 @@ impl chain::Filter for TestChainSource {
 		self.watched_txn.lock().unwrap().insert((*txid, script_pubkey.clone()));
 	}
 
-	fn register_output(&self, outpoint: &OutPoint, script_pubkey: &Script) -> Option<(usize, Transaction)> {
-		self.watched_outputs.lock().unwrap().insert((*outpoint, script_pubkey.clone()));
+	fn register_output(&self, output: WatchedOutput) -> Option<(usize, Transaction)> {
+		self.watched_outputs.lock().unwrap().insert((output.outpoint, output.script_pubkey));
 		None
 	}
 }