Skip to content

Conversation

@tnull
Copy link
Contributor

@tnull tnull commented Jan 27, 2026

Fixes #3566.

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
Copy link

ldk-reviews-bot commented Jan 27, 2026

👋 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.

@tnull tnull requested review from TheBlueMatt and wpaulino January 27, 2026 13:05
@tnull tnull force-pushed the 2026-01-broadcast-type-refactor branch 2 times, most recently from 4f6a8a4 to be02cec Compare January 27, 2026 13:24
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs need a lot of love

/// 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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@tnull tnull Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@tnull tnull force-pushed the 2026-01-broadcast-type-refactor branch from be02cec to 4f6a8a4 Compare January 27, 2026 13:30
@tnull tnull force-pushed the 2026-01-broadcast-type-refactor branch from 4f6a8a4 to fb8378c Compare January 27, 2026 13:53
@tnull tnull requested a review from TheBlueMatt January 27, 2026 13:53
tnull added 2 commits January 27, 2026 15:04
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>
@tnull tnull force-pushed the 2026-01-broadcast-type-refactor branch from fb8378c to 54b52f3 Compare January 27, 2026 14:05
@codecov
Copy link

codecov bot commented Jan 27, 2026

Codecov Report

❌ Patch coverage is 87.12121% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.08%. Comparing base (9e91b2e) to head (5912fd4).
⚠️ Report is 8 commits behind head on main.

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-liquidity/src/lsps2/service.rs 85.71% 0 Missing and 1 partial ⚠️
lightning/src/chain/onchaintx.rs 94.11% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4353      +/-   ##
==========================================
- Coverage   86.08%   86.08%   -0.01%     
==========================================
  Files         156      156              
  Lines      102428   102488      +60     
  Branches   102428   102488      +60     
==========================================
+ Hits        88179    88227      +48     
- Misses      11754    11767      +13     
+ Partials     2495     2494       -1     
Flag Coverage Δ
tests 86.08% <87.12%> (-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.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splice transactions should have their own variant, no?

Copy link
Contributor Author

@tnull tnull Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, hence the question above regarding which variants we want. So, post #4261 we'll now three variants (SpliceIn/SpliceOut/SpliceInAndOut)?

And AFAICT we need to track the TransactionType across the entire flow (i.e., inititally set in SpliceContribution, track in SpliceInstructions, PendingFunding, and then return via FundingTxSigned to hand it to broadcast_interactive_funding? Or do you see a way to shortcut or re-derive the type of splice contribution at the last step somehow?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, post #4261 we'll now three variants (SpliceIn/SpliceOut/SpliceInAndOut)?

If we want more splicing context we should include the splice metadata in the splice variant rather than having a separate variant for each splice init logic. This feels like a followup though.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify a bit - the differentiation between "splice out" and "splice in" at the top-level API is a bug that we're fixing - there is only a "splice" which can have constituent parts that are in + out. Its a useful differentiation when building splicing instructions but after that it should dissapear.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And AFAICT we need to track the TransactionType across the entire flow (i.e., inititally set in SpliceContribution, track in SpliceInstructions, PendingFunding, and then return via FundingTxSigned to hand it to broadcast_interactive_funding? Or do you see a way to shortcut or re-derive the type of splice contribution at the last step somehow?

The raw transaction is already available since we're broadcasting it so they can inspect its inputs and outputs if they want to distinguish between splice in/out.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 added 4 commits January 28, 2026 10:22
We add the `ChannelId` as context to the just-added `TransactionType`
enum.

Co-Authored-By: HAL 9000

Signed-off-by: Elias Rohrer <dev@tnull.de>
@tnull tnull force-pushed the 2026-01-broadcast-type-refactor branch from 54b52f3 to 5912fd4 Compare January 28, 2026 10:09
@tnull
Copy link
Contributor Author

tnull commented Jan 28, 2026

FWIW, I also now pushed a fixup renaming to TransactionType which seemed like a slightly better naming, let me know if you object.

@tnull tnull self-assigned this Jan 28, 2026
@tnull tnull changed the title Refactor BroadcasterInterface to include BroadcastType Refactor BroadcasterInterface to include TransactionType Jan 28, 2026
@tnull tnull moved this to Goal: Merge in Weekly Goals Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Goal: Merge

Development

Successfully merging this pull request may close these issues.

Add APIs to classify channel-related transaction types

4 participants