Assure BroadcasterInterface packages of len > 1 are child-with-parents
What the developer wrote
Authored by Leo Nash
Assure BroadcasterInterface packages of len > 1 are child-with-parents
Implementations MUST NOT assume any topological order on the
transactions.
While Bitcoin Core v29+ `submitpackage` RPC allows packages of length 1
to be submitted via `submitpackage`, it still requires any package
submitted there to be a `child-with-parents` package.
So we remove the possibility that a batch of transactions passed to
a `BroadcasterInterface` implementation contains unrelated transactions,
or multiple children.
What changed, and why it matters
This commit tightens the rules for how Lightning Dev Kit hands groups of Bitcoin transactions to external broadcasting code. Previously, a broadcaster might have received several unrelated transactions or multiple 'child' transactions in one batch, and the documentation only said they 'may or may not' depend on each other. The change guarantees that any batch with more than one transaction is exactly one child transaction plus the parent transactions it depends on. It also updates LDK's own test broadcaster to verify this shape and fixes a test so its fake anchor transaction actually spends the commitment transaction, matching the new guarantee. The main risk is that real broadcaster implementations which relied on the old, looser behavior could have submitted invalid packages to Bitcoin Core's submitpackage RPC, causing broadcasts to fail rather than enabling theft of funds directly.
Review any custom BroadcasterInterface implementation to ensure it does not assume transaction ordering and that it handles exactly one child-with-parents package when len > 1. If the implementation feeds batches directly to Bitcoin Core submitpackage, confirm it re-sorts topologically and rejects unrelated/multi-child packages, or upgrade to a LDK version that already enforces this contract.
Security signals we found
Contract tightening: broadcaster inputs must form a valid Bitcoin Core submitpackage child-with-parents package
New runtime assertion in test code validating package topology
Test fixture corrected so synthetic package satisfies the new contract
Documentation now explicitly forbids assuming topological order
Evidence from the diff
The patch changes the BroadcasterInterface contract in lightning/src/chain/chaininterface.rs: multi-tx batches are now required to be a single child-with-parents package, and implementations must not assume topological ordering. It adds an assertion in TestBroadcaster (lightning/src/util/test_utils.rs) that enforces len <= 2 and, for len == 2, verifies txs[1] spends txs[0] and txs[0] does not spend txs[1]. It also fixes a bump_transaction test (lightning/src/events/bump_transaction/mod.rs) so the anchor outpoint’s txid matches the commitment_tx, making the test package valid under the new rule.
Changed components
lightning/src/chain/chaininterface.rs (BroadcasterInterface trait documentation/contract)lightning/src/util/test_utils.rs (TestBroadcaster broadcast_transactions assertion)lightning/src/events/bump_transaction/mod.rs (bump_transaction test fixture)Inspect captured patch +32 / −4
diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs
index 1f945a5..5dc46d5 100644
--- a/lightning/src/chain/chaininterface.rs
+++ b/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>
diff --git a/lightning/src/events/bump_transaction/mod.rs b/lightning/src/events/bump_transaction/mod.rs
index 6ba46a4..d17fac8 100644
--- a/lightning/src/events/bump_transaction/mod.rs
+++ b/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(),
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 38456e2..b197171 100644
--- a/lightning/src/util/test_utils.rs
+++ b/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);
Why this scored 33/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.