Merge pull request #4173 from tankyleo/25-10-broadcaster-docs

Assure BroadcasterInterface packages of len > 1 are child-with-parents

Author: TheBlueMatt

lightning/src/chain/chaininterface.rs

@@ -36,9 +36,12 @@ pub trait BroadcasterInterface {
 	/// 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.
+	/// If more than one transaction is given, these transactions MUST be a
+	/// single child and its parents and be broadcast together as a package
+	/// (see the [`submitpackage`](https://bitcoincore.org/en/doc/30.0.0/rpc/rawtransactions/submitpackage)
+	/// Bitcoin Core RPC).
+	///
+	/// Implementations MUST NOT assume any topological order on the transactions.
 	///
 	/// Bitcoin transaction packages are defined in BIP 331 and here:
 	/// <https://github.com/bitcoin/bitcoin/blob/master/doc/policy/packages.md>

lightning/src/events/bump_transaction/mod.rs

@@ -1135,6 +1135,7 @@ mod tests {
 		let commitment_tx_bytes = Vec::<u8>::from_hex("02000000000101cc6b0a9dd84b52c07340fff6fab002fc37b4bdccfdce9f39c5ec8391a56b652907000000009b948b80044a01000000000000220020b4182433fdfdfbf894897c98f84d92cec815cee222755ffd000ae091c9dadc2d4a01000000000000220020f83f7dbf90e2de325b5bb6bab0ae370151278c6964739242b2e7ce0cb68a5d81cb4a02000000000022002024add256b3dccee772610caef82a601045ab6f98fd6d5df608cc756b891ccfe63ffa490000000000220020894bf32b37906a643625e87131897c3714c71b3ac9b161862c9aa6c8d468b4c70400473044022060abd347bff2cca0212b660e6addff792b3356bd4a1b5b26672dc2e694c3c5f002202b40b7e346b494a7b1d048b4ec33ba99c90a09ab48eb1df64ccdc768066c865c014730440220554d8361e04dc0ee178dcb23d2d23f53ec7a1ae4312a5be76bd9e83ab8981f3d0220501f23ffb18cb81ccea72d30252f88d5e69fd28ba4992803d03c00d06fa8899e0147522102817f6ce189ab7114f89e8d5df58cdbbaf272dc8e71b92982d47456a0b6a0ceee2102c9b4d2f24aca54f65e13f4c83e2a8d8e877e12d3c71a76e81f28a5cabc652aa352ae626c7620").unwrap();
 		let commitment_tx: Transaction =
 			Readable::read(&mut Cursor::new(&commitment_tx_bytes)).unwrap();
+		let commitment_txid = commitment_tx.compute_txid();
 		let total_commitment_weight =
 			commitment_tx.weight().to_wu() + ANCHOR_INPUT_WITNESS_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT;
 		let commitment_and_anchor_fee = 930 + 330;
@@ -1191,7 +1192,7 @@ mod tests {
 					keys_id: [42; 32],
 					transaction_parameters,
 				},
-				outpoint: OutPoint { txid: Txid::from_byte_array([42; 32]), vout: 0 },
+				outpoint: OutPoint { txid: commitment_txid, vout: 0 },
 				value: Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI),
 			},
 			pending_htlcs: Vec::new(),

lightning/src/util/test_utils.rs

@@ -1123,6 +1123,30 @@ impl TestBroadcaster {
 
 impl chaininterface::BroadcasterInterface for TestBroadcaster {
 	fn broadcast_transactions(&self, txs: &[&Transaction]) {
+		// Assert that any batch of transactions of length greater than 1 is sorted
+		// topologically, and is a `child-with-parents` package as defined in
+		// <https://github.com/bitcoin/bitcoin/blob/master/doc/policy/packages.md>.
+		//
+		// Implementations MUST NOT rely on this, and must re-sort the transactions
+		// themselves.
+		//
+		// Right now LDK only ever broadcasts packages of length 2.
+		assert!(txs.len() <= 2);
+		if txs.len() == 2 {
+			let parent_txid = txs[0].compute_txid();
+			assert!(txs[1]
+				.input
+				.iter()
+				.map(|input| input.previous_output.txid)
+				.any(|txid| txid == parent_txid));
+			let child_txid = txs[1].compute_txid();
+			assert!(txs[0]
+				.input
+				.iter()
+				.map(|input| input.previous_output.txid)
+				.all(|txid| txid != child_txid));
+		}
+
 		for tx in txs {
 			let lock_time = tx.lock_time.to_consensus_u32();
 			assert!(lock_time < 1_500_000_000);