Expose `ChannelDetails::channel_shutdown_state` by f3r10 · Pull Request #827 · lightningdevkit/ldk-node

f3r10 · 2026-03-14T19:06:03Z

Summary

Closes #803.

We currently don't expose ChannelDetails::channel_shutdown_state, so there is no good way for callers to tell whether a channel is actively going through a cooperative shutdown or merely has its peer temporarily disconnected — both situations cause is_usable to become false.

This PR adds:

A new ChannelShutdownState enum (mirroring LDK's own type) with variants NotShuttingDown, ShutdownInitiated, ResolvingHTLCs, NegotiatingClosingFee, and ShutdownComplete.
An Option<ChannelShutdownState> field on ChannelDetails, populated from LdkChannelDetails::channel_shutdown_state via the existing From conversion.
A typedef enum ChannelShutdownState entry in the UniFFI UDL so the field is available to Swift, Kotlin, and Python bindings.
Public re-export of ChannelShutdownState from the crate root.

ldk-reviews-bot · 2026-03-14T19:06:06Z

👋 Thanks for assigning @tnull 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.

Camillarhi · 2026-03-15T00:28:50Z

src/types.rs

 	pub config: ChannelConfig,
+	/// The current shutdown state of the channel, if any.
+	///
+	/// Returns `None` for channels that have not yet started the shutdown process.


Could we replace this to accurately reflect what the source from the library says Returns None for ChannelDetails serialized on LDK versions prior to 0.0.116.

Camillarhi · 2026-03-15T00:33:07Z

Thanks for looking into this. Could we add assertions for channel_shutdown_state in the existing integration tests? For example, after both nodes receive the channel_ready event in do_channel_full_cycle

ldk-reviews-bot · 2026-03-16T19:17:16Z

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-03-17T09:33:32Z

src/types.rs

+/// [`ChannelDetails::channel_shutdown_state`]: crate::ChannelDetails::channel_shutdown_state
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))]
+pub enum ChannelShutdownState {


Do we really need to duplicate this enum here, or could we get away with using the upstream one and only using #[uniffi::remote(Enum)] for uniffi?

yeah totally @tnull I am going to do that

ldk-reviews-bot · 2026-03-21T16:10:29Z

🔔 1st Reminder

Hey @tnull @Camillarhi! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-03-21T16:10:30Z

🔔 1st Reminder

Hey @tnull @Camillarhi! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull

Thanks! Already looks good, just some minor remarks regarding re-exports and test coverage.

tnull · 2026-03-23T08:48:26Z

src/ffi/types.rs

 pub use lightning::chain::channelmonitor::BalanceSource;
 use lightning::events::PaidBolt12Invoice as LdkPaidBolt12Invoice;
 pub use lightning::events::{ClosureReason, PaymentFailureReason};
+pub use lightning::ln::channel_state::ChannelShutdownState;


nit: I believe we don't need this to be pub?

tnull · 2026-03-23T08:49:47Z

src/types.rs

 use lightning::chain::chainmonitor;
 use lightning::impl_writeable_tlv_based;
 use lightning::ln::channel_state::ChannelDetails as LdkChannelDetails;
+pub use lightning::ln::channel_state::ChannelShutdownState;


nit: Let's drop the pub here.

tnull · 2026-03-23T08:50:47Z

src/lib.rs

 	Wallet,
 };
-pub use types::{ChannelDetails, CustomTlvRecord, PeerDetails, SyncAndAsyncKVStore, UserChannelId};
+pub use types::{


nit: Rather than re-exporting via crate::types, lets just add a pub use lightning::ln::channel_state::ChannelShutdownState; line.

tnull · 2026-03-23T08:55:29Z

src/types.rs

 	pub config: ChannelConfig,
+	/// The current shutdown state of the channel, if any.
+	///
+	/// Returns `None` for `ChannelDetails` serialized on LDK versions prior to 0.0.116.


As this is in LDK Node context, we should provide compatibility notes based on LDK Node's versions. In this case that would mean LDK Node versions prior to 0.2 (aruably that's so old we could also omit the note altogether).

I agree, I am going to remove that line

tnull · 2026-03-23T08:56:43Z

tests/integration_tests_rust.rs

-			.confirmations,
-		Some(0)
-	);
+	let client_channel = client_node


Could you expand on why the LSPS2 tests in particular do need additional test coverage for channel shutdown?

you are right, those tests are not necessary. I am going to revert those.

tnull · 2026-03-23T08:58:15Z

tests/common/mod.rs

+			None | Some(ChannelShutdownState::NotShuttingDown)
+		));
+	}
+


Hmm, if we add all these assertions for NotShuttingDown, should we at least also add one later for a case when we are shutting down (i.e., close/force close)?

I have been trying to test this part, but the problem is that for cooperative close, the shutdown completes very quickly in tests, so by the time we call list_channels() after close_channel(), the channel may already be gone.

In the force close phase, LDK doesn't use the cooperative shutdown state machine for force closes, so the field would still be None/NotShuttingDown right up until the channel disappears.

tnull · 2026-03-23T08:58:58Z

tests/integration_tests_rust.rs

 	assert!(channel.channel_value_sats > premine_amount_sat - anchor_reserve_sat - 500);
 	assert_eq!(channel.counterparty_node_id, node_b.node_id());
 	assert_eq!(channel.funding_txo.unwrap(), funding_txo);
+	assert!(matches!(


I think we don't need to add all of these assertions, as the actual behavior should be tested in LDK.

ldk-reviews-bot · 2026-03-23T16:11:30Z

🔔 2nd Reminder

Hey @Camillarhi! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-03-25T16:11:55Z

🔔 3rd Reminder

Hey @Camillarhi! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-03-28T00:00:30Z

🔔 4th Reminder

Hey @Camillarhi! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-03-30T00:01:11Z

🔔 5th Reminder

Hey @Camillarhi! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Add a `ChannelShutdownState` enum mirroring LDK's own type, and expose it as an `Option<ChannelShutdownState>` field on `ChannelDetails`.

tnull

One comment, otherwise LGTM

Though, IMO, you could further reduce the test changes. Maybe just add one at an opportune place? Asserting on NotShuttingDown over and over again isn't super helpful and just increases clutter.

tnull · 2026-03-31T08:45:30Z

src/types.rs

 	pub inbound_htlc_maximum_msat: Option<u64>,
 	/// Set of configurable parameters that affect channel operation.
 	pub config: ChannelConfig,
+	/// The current shutdown state of the channel, if any.


Please add a note that this will be None for objects first serialized with LDK Node v0.1.

ldk-reviews-bot requested a review from tnull March 14, 2026 19:17

Camillarhi suggested changes Mar 15, 2026

View reviewed changes

tnull reviewed Mar 17, 2026

View reviewed changes

f3r10 force-pushed the feat/expose_channel_shutdown_state branch from 30b610c to 32c3eef Compare March 19, 2026 16:09

f3r10 requested review from Camillarhi and tnull March 19, 2026 16:09

tnull reviewed Mar 23, 2026

View reviewed changes

Expose ChannelDetails::channel_shutdown_state

3b68c59

Add a `ChannelShutdownState` enum mirroring LDK's own type, and expose it as an `Option<ChannelShutdownState>` field on `ChannelDetails`.

f3r10 force-pushed the feat/expose_channel_shutdown_state branch from 32c3eef to 3b68c59 Compare March 30, 2026 16:19

f3r10 requested a review from tnull March 30, 2026 17:43

tnull reviewed Mar 31, 2026

View reviewed changes

Conversation

f3r10 commented Mar 14, 2026

Summary

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Camillarhi commented Mar 15, 2026

Uh oh!

ldk-reviews-bot commented Mar 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Mar 21, 2026

Uh oh!

ldk-reviews-bot commented Mar 21, 2026

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Mar 23, 2026

Uh oh!

ldk-reviews-bot commented Mar 25, 2026

Uh oh!

ldk-reviews-bot commented Mar 28, 2026

Uh oh!

ldk-reviews-bot commented Mar 30, 2026

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ldk-reviews-bot commented Mar 14, 2026 •

edited

Loading