Skip to content

Expose ChannelCounterparty and ReserveType in ChannelDetails#841

Open
enigbe wants to merge 4 commits intolightningdevkit:mainfrom
enigbe:2026-03-channel-reserve-type-exposure
Open

Expose ChannelCounterparty and ReserveType in ChannelDetails#841
enigbe wants to merge 4 commits intolightningdevkit:mainfrom
enigbe:2026-03-channel-reserve-type-exposure

Conversation

@enigbe
Copy link
Copy Markdown
Contributor

@enigbe enigbe commented Mar 24, 2026

What this PR does

In #305, we needed a way to expose channel type and the channel reserve without leaking implementation details not useful to users. The related discussion in #141 proposed a ReserveType abstraction that bakes in both in a user-relevant way. This PR introduces the said enumeration to ChannelDetails with the following variants:

  • Legacy: signifying pre-anchor channel types where on-chain fees paying for broadcast transactions following channel closure were pre-determined
  • TrustedPeersNoReserve: for anchor type channels with trusted peers
  • Adaptive: indicating anchor channel with adaptive reserve, reflecting dynamic best-effort attempt at fee-bumping.
    (see @jkczyz's suggestion)

We modify the ChannelDetails constructor to accept an optional anchor channels config to address the challenge of users cross-referencing the channel details and config to determine if counterparties are trusted.

Additionally, following recommendation in #810 about negotiated features, this PR exposes per-peer InitFeatures, and as a consequence, modifies ChannelDetails by replacing the flattened counterparty_* with ChannelCounterparty that encapsulates them and mirror LDK's.

Issue Addressed

Closes #305.
Builds on discussions and recommendations from #141 and #810.

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Mar 24, 2026

I've assigned @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.

enigbe added a commit to enigbe/ldk-node that referenced this pull request Mar 24, 2026
This fixup moves node feature exposure from freestanding APIs
to NodeStatus, as suggested in review. Rather than exposing
init_features(), channel_features(), bolt11_invoice_features(),
and node_features() as separate public methods on Node, this
embeds NodeFeatures in the NodeStatus struct returned by
Node::status().

Additionally, channel and invoice features at node level are
confusing. Users would expect negotiated per-peer/channel/invoice
features, not what the node generally supports. Access to
negotiated features are addressed in lightningdevkit#841
@ldk-reviews-bot ldk-reviews-bot requested a review from tnull March 24, 2026 21:13
Copy link
Copy Markdown
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Thanks! Looks pretty good, have a few initial comments.

src/types.rs Outdated
}
}

/// Information needed for constructing an invoice route hint for this channel.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Everything related to uniffi should live in ffi/types.rs, which also avoids the feature-gate on individual items.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This seems unresolved, I think we can move this CounterpartyForwardingInfo to ffi/types.rs?

src/types.rs Outdated
uniffi::custom_type!(InitFeatures, Vec<u8>, {
remote,
try_lift: |val| Ok(InitFeatures::from_le_bytes(val)),
lower: |obj| obj.le_flags().to_vec(),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Generally not the biggest fan of exposing this as Vec<u8>, but if we do, why pick little endian over big endian?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was mistaken about the LE/BE ease of bit indexing vs over the wire serialization. I've replaced the Vec with a dedicated wrapper type that stores LdkInitFeatures directly, so the endianness question no longer applies.

@enigbe enigbe force-pushed the 2026-03-channel-reserve-type-exposure branch 4 times, most recently from 33a36bd to 63f32ab Compare March 30, 2026 06:34
Copy link
Copy Markdown
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

One comment, otherwise looks good. Feel free to squash (also the new fixup)

src/types.rs Outdated
}
}

/// Information needed for constructing an invoice route hint for this channel.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This seems unresolved, I think we can move this CounterpartyForwardingInfo to ffi/types.rs?

@enigbe enigbe force-pushed the 2026-03-channel-reserve-type-exposure branch from 63f32ab to 56d86f7 Compare March 30, 2026 09:57
@enigbe
Copy link
Copy Markdown
Contributor Author

enigbe commented Mar 30, 2026

Squashed all fix-ups.

ReserveType::Adaptive
}
} else {
ReserveType::Adaptive
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm, isn't this wrong? I believe we'd fallback to legacy if we don't have an anchors_channel_config set?

I guess we could only land here if we had already accepted an anchor channel and then the user changed anchors_channels_config afterwards?

Might be good to at least leave some comments here to give some rationale.

Copy link
Copy Markdown
Contributor Author

@enigbe enigbe Apr 2, 2026

Choose a reason for hiding this comment

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

Good catch here. You are right that we only reach this branch if an anchor channel was previously opened with anchor_channels_config set, and then later unset. In that case, Legacy would be incorrect and Adaptive could also be wrong . The channel will still be an anchor type, but if it was originally TrustedPeersNoReserve, we'd silently reclassify it as Adaptive.

I think the main problem is that we derive the reserve_type at query time from the current config rather than recording it when the channel is opened. So any config change could cause a loss of distinction where we can't distinguish between anchor channels that were Adaptive vs TrustedPeersNoReserve.

We could explore persisting the reserve type (keyed by ChannelId) when we open a channel, and have list_channels look up the stored value instead of re-deriving it. What do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd rather not persist it. Can we just determine based on the feature, and then decide between Adaptive and TrustedPeersNoReserve based on whether or not the peer is present in the no reserve list? I guess that's kind of what we're doing already?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we already do this. There's just that one edge case where a user unsets the anchor channels config after previously setting it. We'd be able to tell if it's an anchor channel but not whether it's adaptive or not.

Copy link
Copy Markdown
Collaborator

@tnull tnull Apr 3, 2026

Choose a reason for hiding this comment

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

I guess that's fine. Could you maybe add a comment to reflect the current rationale/thinking and documenting this edge case? And then we can probably move ahead. Not sure if it's worth over-engineering this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll add the comment.

enigbe added 3 commits April 3, 2026 10:23
We previously flattened ChannelCounterparty fields
into ChannelDetails as individual counterparty_*
fields, and InitFeatures was entirely omitted.
This made it impossible for consumers to access per-peer
feature flags, and awkward to access counterparty
forwarding information without navigating the flattened
field names.

This commit replaces the flattened fields with a
structured ChannelCounterparty type that mirrors
LDK's ChannelCounterparty, exposing InitFeatures
and CounterpartyForwardingInfo that were previously
inaccessible.

Breaking change!
We expose the reserve type of each channel through
a new ReserveType enum on ChannelDetails. This tells
users whether a channel uses adaptive anchor reserves,
has no reserve due to a trusted peer, or is a legacy
pre-anchor channel.

The reserve type is derived at query time in list_channels
by checking the channel's type features against
trusted_peers_no_reserve.

Additionally, We replace the From<LdkChannelDetails>
impl with an explicit from_ldk method that takes the
anchor channels config.
document the selection of adaptive reserve type
in the event a user sets and then unsets the anchor
channels config
@enigbe enigbe force-pushed the 2026-03-channel-reserve-type-exposure branch from 56d86f7 to 77dcbc4 Compare April 3, 2026 09:24
///
/// This ensures the non-broadcaster's output pays directly to their specified key,
/// simplifying recovery if a channel is force-closed.
pub fn supports_static_remote_key(&self) -> bool {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It seems we're missing a bunch of features here? E.g. supports_keysend and others seem to be missing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've added the missing features but support_keysend applies to NodeFeatures only.

We expose the complete set of InitFeatures query methods

Add support/require query methods for all BOLT 9 features in the
InitContext, including standard features, as well as taproot and the
experimental anchor_zero_fee_commitments and htlc_hold features.

Also rename supports_zero_conf to requires_zero_conf to match the
underlying LDK method being called.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose a way to access channel or reserve type

3 participants