Merge pull request #2272 from benthecarman/package-broadcast

Support broadcasting multiple transactions at once

Author: TheBlueMatt

fuzz/src/chanmon_consistency.rs

@@ -100,7 +100,7 @@ impl Router for FuzzRouter {
 
 pub struct TestBroadcaster {}
 impl BroadcasterInterface for TestBroadcaster {
-	fn broadcast_transaction(&self, _tx: &Transaction) { }
+	fn broadcast_transactions(&self, _txs: &[&Transaction]) { }
 }
 
 pub struct VecWriter(pub Vec<u8>);

fuzz/src/full_stack.rs

@@ -144,8 +144,9 @@ struct TestBroadcaster {
 	txn_broadcasted: Mutex<Vec<Transaction>>,
 }
 impl BroadcasterInterface for TestBroadcaster {
-	fn broadcast_transaction(&self, tx: &Transaction) {
-		self.txn_broadcasted.lock().unwrap().push(tx.clone());
+	fn broadcast_transactions(&self, txs: &[&Transaction]) {
+		let owned_txs: Vec<Transaction> = txs.iter().map(|tx| (*tx).clone()).collect();
+		self.txn_broadcasted.lock().unwrap().extend(owned_txs);
 	}
 }
 

lightning/src/chain/chaininterface.rs

@@ -19,8 +19,20 @@ use bitcoin::blockdata::transaction::Transaction;
 
 /// An interface to send a transaction to the Bitcoin network.
 pub trait BroadcasterInterface {
-	/// Sends a transaction out to (hopefully) be mined.
-	fn broadcast_transaction(&self, tx: &Transaction);
+	/// Sends a list of transactions out to (hopefully) be mined.
+	/// This only needs to handle the actual broadcasting of transactions, LDK will automatically
+	/// rebroadcast transactions that haven't made it into a block.
+	///
+	/// In some cases LDK may attempt to broadcast a transaction which double-spends another
+	/// and this isn't a bug and can be safely ignored.
+	///
+	/// If more than one transaction is given, these transactions should be considered to be a
+	/// package and broadcast together. Some of the transactions may or may not depend on each other,
+	/// be sure to manage both cases correctly.
+	///
+	/// Bitcoin transaction packages are defined in BIP 331 and here:
+	/// https://github.com/bitcoin/bitcoin/blob/master/doc/policy/packages.md
+	fn broadcast_transactions(&self, txs: &[&Transaction]);
 }
 
 /// An enum that represents the speed at which we want a transaction to confirm used for feerate

lightning/src/chain/channelmonitor.rs

@@ -2327,10 +2327,13 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		where B::Target: BroadcasterInterface,
 					L::Target: Logger,
 	{
-		for tx in self.get_latest_holder_commitment_txn(logger).iter() {
+		let commit_txs = self.get_latest_holder_commitment_txn(logger);
+		let mut txs = vec![];
+		for tx in commit_txs.iter() {
 			log_info!(logger, "Broadcasting local {}", log_tx!(tx));
-			broadcaster.broadcast_transaction(tx);
+			txs.push(tx);
 		}
+		broadcaster.broadcast_transactions(&txs);
 		self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
 	}
 

lightning/src/chain/onchaintx.rs

@@ -513,7 +513,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 						OnchainClaim::Tx(tx) => {
 							let log_start = if bumped_feerate { "Broadcasting RBF-bumped" } else { "Rebroadcasting" };
 							log_info!(logger, "{} onchain {}", log_start, log_tx!(tx));
-							broadcaster.broadcast_transaction(&tx);
+							broadcaster.broadcast_transactions(&[&tx]);
 						},
 						#[cfg(anchors)]
 						OnchainClaim::Event(event) => {
@@ -767,7 +767,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 				let package_id = match claim {
 					OnchainClaim::Tx(tx) => {
 						log_info!(logger, "Broadcasting onchain {}", log_tx!(tx));
-						broadcaster.broadcast_transaction(&tx);
+						broadcaster.broadcast_transactions(&[&tx]);
 						tx.txid().into_inner()
 					},
 					#[cfg(anchors)]
@@ -960,7 +960,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 				match bump_claim {
 					OnchainClaim::Tx(bump_tx) => {
 						log_info!(logger, "Broadcasting RBF-bumped onchain {}", log_tx!(bump_tx));
-						broadcaster.broadcast_transaction(&bump_tx);
+						broadcaster.broadcast_transactions(&[&bump_tx]);
 					},
 					#[cfg(anchors)]
 					OnchainClaim::Event(claim_event) => {
@@ -1046,7 +1046,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 				match bump_claim {
 					OnchainClaim::Tx(bump_tx) => {
 						log_info!(logger, "Broadcasting onchain {}", log_tx!(bump_tx));
-						broadcaster.broadcast_transaction(&bump_tx);
+						broadcaster.broadcast_transactions(&[&bump_tx]);
 					},
 					#[cfg(anchors)]
 					OnchainClaim::Event(claim_event) => {

lightning/src/ln/channelmanager.rs

@@ -4517,7 +4517,7 @@ where
 
 		if let Some(tx) = funding_broadcastable {
 			log_info!(self.logger, "Broadcasting funding transaction with txid {}", tx.txid());
-			self.tx_broadcaster.broadcast_transaction(&tx);
+			self.tx_broadcaster.broadcast_transactions(&[&tx]);
 		}
 
 		{
@@ -5047,7 +5047,7 @@ where
 		};
 		if let Some(broadcast_tx) = tx {
 			log_info!(self.logger, "Broadcasting {}", log_tx!(broadcast_tx));
-			self.tx_broadcaster.broadcast_transaction(&broadcast_tx);
+			self.tx_broadcaster.broadcast_transactions(&[&broadcast_tx]);
 		}
 		if let Some(chan) = chan_option {
 			if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
@@ -5657,7 +5657,7 @@ where
 								self.issue_channel_close_events(chan, ClosureReason::CooperativeClosure);
 
 								log_info!(self.logger, "Broadcasting {}", log_tx!(tx));
-								self.tx_broadcaster.broadcast_transaction(&tx);
+								self.tx_broadcaster.broadcast_transactions(&[&tx]);
 								update_maps_on_chan_removal!(self, chan);
 								false
 							} else { true }

lightning/src/util/test_utils.rs

@@ -341,17 +341,20 @@ impl TestBroadcaster {
 }
 
 impl chaininterface::BroadcasterInterface for TestBroadcaster {
-	fn broadcast_transaction(&self, tx: &Transaction) {
-		let lock_time = tx.lock_time.0;
-		assert!(lock_time < 1_500_000_000);
-		if bitcoin::LockTime::from(tx.lock_time).is_block_height() && lock_time > self.blocks.lock().unwrap().last().unwrap().1 {
-			for inp in tx.input.iter() {
-				if inp.sequence != Sequence::MAX {
-					panic!("We should never broadcast a transaction before its locktime ({})!", tx.lock_time);
+	fn broadcast_transactions(&self, txs: &[&Transaction]) {
+		for tx in txs {
+			let lock_time = tx.lock_time.0;
+			assert!(lock_time < 1_500_000_000);
+			if bitcoin::LockTime::from(tx.lock_time).is_block_height() && lock_time > self.blocks.lock().unwrap().last().unwrap().1 {
+				for inp in tx.input.iter() {
+					if inp.sequence != Sequence::MAX {
+						panic!("We should never broadcast a transaction before its locktime ({})!", tx.lock_time);
+					}
 				}
 			}
 		}
-		self.txn_broadcasted.lock().unwrap().push(tx.clone());
+		let owned_txs: Vec<Transaction> = txs.iter().map(|tx| (*tx).clone()).collect();
+		self.txn_broadcasted.lock().unwrap().extend(owned_txs);
 	}
 }