Refactor `BroadcasterInterface` to include `BroadcastType` #4353

tnull · 2026-01-27T13:05:19Z

Add a `BroadcastType` enum to provide context about the type of
transaction being broadcast. This information can be useful for
logging, filtering, or prioritization purposes.

The `BroadcastType` variants are:
- `Funding`: A funding transaction establishing a new channel
- `CooperativeClose`: A cooperative close transaction
- `UnilateralClose`: A transaction for force-close
- `AnchorBump`: An anchor transaction for CPFP fee-bumping
- `Claim`: A transaction claiming outputs from commitment tx
- `Sweep`: A transaction sweeping spendable outputs to wallet

Co-Authored-By: HAL 9000

and:

We add the `ChannelId` as context to the just-added `BroadcastType`
enum.

Co-Authored-By: HAL 9000

ldk-reviews-bot · 2026-01-27T13:05:22Z

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

lightning/src/chain/chaininterface.rs

TheBlueMatt

docs need a lot of love

TheBlueMatt · 2026-01-27T13:26:38Z

lightning/src/chain/chaininterface.rs

+		/// A single funding transaction may establish multiple channels when using batch funding.
+		channel_ids: Vec<ChannelId>,
+	},
+	/// A cooperative close transaction mutually agreed upon by both parties.


I mean all transactions are mutually agreed upon by both parties? Maybe describe what a coop close tx means

Hmm, changed it up a bit, but let me know what exactly is missing for you.

lightning/src/chain/chaininterface.rs

tnull · 2026-01-27T13:33:08Z

lightning/src/chain/onchaintx.rs

+		// we derive a channel_id from the funding outpoint if not present.
+		let channel_id = channel_id.or_else(|| {
+			channel_parameters.funding_outpoint
+				.map(|op| ChannelId::v1_from_funding_outpoint(op))


Not super excited about the extra persisted field and the fallback here. I explored using ReadableArgs to hand in the ChannelId but that doesn't work with the read order in ChannelMonitor. Any other ideas/preferences? (@TheBlueMatt / @wpaulino)

I swear I wrote a comment here and github just ate it. We don't need to do any handling in onchaintx.rs - the ChannelMonitor that calls it and deserializes it has the channel id, so we just need to pass it in (either in the deser method and store it or to the methods that need it)

I swear I wrote a comment here and github just ate it.

Hmm, I had added, then deleted a similar comment before, maybe it was on there?

We don't need to do any handling in onchaintx.rs - the ChannelMonitor that calls it and deserializes it has the channel id, so we just need to pass it in (either in the deser method and store it or to the methods that need it)

As mentioned above, I explored that briefly (hence why I deleted the original comment, as I briefly thought I found a better solution), but that isn't straightforward as in ChannelMonitor::read we first read the OnchainTxHandler then the channel ID as part of the TLV fields, so we can't pass it via ReadableArgs. We could have the field being an option and add a setter like OnchainTxHandler::set_channel_id to hand it in afterwards, but that is also very ugly, hence why I'm asking for better ideas.

Ah, yea, i think I had the comment on the previous one and it got dropped then I missed your note at the end cause I didn't re-read. Anyway, yea, Option is kinda annoying, we could cheat and just store a [0; 32] and overwrite it after we finish reading the monitor? Given we only need it in a few places we could also pass it in.

Add a `BroadcastType` enum to provide context about the type of transaction being broadcast. This information can be useful for logging, filtering, or prioritization purposes. The `BroadcastType` variants are: - `Funding`: A funding transaction establishing a new channel - `CooperativeClose`: A cooperative close transaction - `UnilateralClose`: A force-close transaction - `AnchorBump`: An anchor transaction for CPFP fee-bumping - `Claim`: A transaction claiming outputs from commitment tx - `Sweep`: A transaction sweeping spendable outputs to wallet Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>

We add the `ChannelId` as context to the just-added `BroadcastType` enum. Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>

codecov · 2026-01-27T16:13:52Z

Codecov Report

❌ Patch coverage is 85.82090% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.09%. Comparing base (9e91b2e) to head (54b52f3).

Files with missing lines	Patch %	Lines
lightning/src/util/sweep.rs	79.59%	6 Missing and 4 partials ⚠️
lightning/src/ln/channelmanager.rs	81.48%	5 Missing ⚠️
lightning/src/chain/onchaintx.rs	85.71%	0 Missing and 3 partials ⚠️
lightning-liquidity/src/lsps2/service.rs	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4353   +/-   ##
=======================================
  Coverage   86.08%   86.09%           
=======================================
  Files         156      156           
  Lines      102428   102489   +61     
  Branches   102428   102489   +61     
=======================================
+ Hits        88179    88236   +57     
- Misses      11754    11758    +4     
  Partials     2495     2495

Flag	Coverage Δ
tests	`86.09% <85.82%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

wpaulino · 2026-01-27T17:57:37Z

lightning/src/chain/chaininterface.rs

+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
+pub enum BroadcastType {
+	/// A funding transaction establishing a new channel.
+	Funding,


Splice transactions should have their own variant, no?

TheBlueMatt · 2026-01-27T20:53:19Z

lightning/src/chain/chaininterface.rs

+		/// A single funding transaction may establish multiple channels when using batch funding.
+		channel_ids: Vec<ChannelId>,
+	},
+	/// A transaction cooperatively closing a channel.


I think the thing missing here and elsewhere is the why - if I'm using this for improving my logging I want to understand what led to this transaction and what the implications of it are. eg this have a link to ChannelManager::close_channel (noting of course that the counterparty could have initiated as well), UnilateralClose should have a link to ChannelManager::force_close_* and note that it could also happen due to other errors.

tnull requested review from TheBlueMatt and wpaulino January 27, 2026 13:05

tnull commented Jan 27, 2026

View reviewed changes

lightning/src/chain/chaininterface.rs Outdated Show resolved Hide resolved

tnull force-pushed the 2026-01-broadcast-type-refactor branch 2 times, most recently from 4f6a8a4 to be02cec Compare January 27, 2026 13:24

TheBlueMatt reviewed Jan 27, 2026

View reviewed changes

tnull force-pushed the 2026-01-broadcast-type-refactor branch from be02cec to 4f6a8a4 Compare January 27, 2026 13:30

tnull commented Jan 27, 2026

View reviewed changes

tnull force-pushed the 2026-01-broadcast-type-refactor branch from 4f6a8a4 to fb8378c Compare January 27, 2026 13:53

tnull requested a review from TheBlueMatt January 27, 2026 13:53

tnull added 4 commits January 27, 2026 15:04

Refactor BroadcastType with channel context

851f00d

We add the `ChannelId` as context to the just-added `BroadcastType` enum. Co-Authored-By: HAL 9000 Signed-off-by: Elias Rohrer <dev@tnull.de>

f Improve docs, merge some variants

447f296

Drop unused imports

54b52f3

tnull force-pushed the 2026-01-broadcast-type-refactor branch from fb8378c to 54b52f3 Compare January 27, 2026 14:05

wpaulino reviewed Jan 27, 2026

View reviewed changes

TheBlueMatt reviewed Jan 27, 2026

View reviewed changes

Refactor BroadcasterInterface to include BroadcastType #4353

Are you sure you want to change the base?

Refactor BroadcasterInterface to include BroadcastType #4353

Conversation

tnull commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

tnull Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

tnull Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 27, 2026

Codecov Report

Uh oh!

wpaulino Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Refactor `BroadcasterInterface` to include `BroadcastType` #4353

Refactor `BroadcasterInterface` to include `BroadcastType` #4353

tnull commented Jan 27, 2026 •

edited

Loading

ldk-reviews-bot commented Jan 27, 2026 •

edited

Loading

tnull Jan 27, 2026 •

edited

Loading

tnull Jan 27, 2026 •

edited

Loading