Expose node-invariant features via NodeStatus#810
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
There was a problem hiding this comment.
I'm not convinced we should expose this as freestanding APIs. Note that for channels and invoices these might also might be confusing as people really would be interested in the negotiated features, not what we generally, maybe support.
I guess we could consider to find a way to expose the invariant node features via an API (or maybe Node::status) if that would be useful, but for channels we should do #305 and maybe expose features in a sane way through list_channels. But note all of this should also work in bindings, so reusing LDK types might be challenging.
enigbe
force-pushed
the
2026-02-expose-node-features-api
branch
from
e1acc25 to
3ac1658
Compare
enigbe
force-pushed
the
2026-02-expose-node-features-api
branch
from
3ac1658 to
5a87041
Compare
enigbe
changed the title
I agree too. I only added them after reviewing cln-grpc and thought parity with them (on ldk-server) would make sense. I've removed the free-standing APIs.
I've just opened #841 in relation to this. |
|
🔔 1st Reminder Hey @joostjager! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @joostjager! This PR has been waiting for your review. |
enigbe
force-pushed
the
2026-02-expose-node-features-api
branch
from
5a87041 to
0668b4f
Compare
For node features, I've decided to expose via |
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @tnull! This PR has been waiting for your review. |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Looks good I think, please squash.
| self.inner.encode() | ||
| } | ||
|
|
||
| /// Whether this node's `node_announcement` advertises support for `option_data_loss_protect` (bit 1). |
There was a problem hiding this comment.
Hmm, maybe we should drop mentioning the bit explicitly. For some features they might change over time in particular (e.g. when new features start out as experimental before finalization), and then we'll risk our bindings docs being off (which will be hard to catch)
| self.channel_manager.node_features() | ||
| | self.chain_monitor.provided_node_features() | ||
| | self.onion_messenger.provided_node_features() | ||
| | gossip_features |
There was a problem hiding this comment.
Is this redundant to the features in channel manager/monitor etc?
There was a problem hiding this comment.
Yes, it is redundant in the sense that we reconstruct it from the underlying LDK components, but not redundant as an API. The idea with NodeStatus::node_features is to expose the aggregate node-level feature set that ldk-node advertises in node_announcement. LDK computes that aggregate internally in PeerManager::broadcast_node_announcement, but PeerManager does not expose the already-computed value, so we reconstruct it here.
Expose the feature flags advertised by the node in its node_announcement message via NodeStatus::node_features. For UniFFI consumers, expose NodeFeatures as an object wrapper with BOLT 9 byte encoding helpers and typed feature accessors instead of relying on a raw custom Vec<u8> conversion. This intentionally avoids adding freestanding Node methods for init, channel, invoice, or node feature contexts. Channel and invoice features are context-specific, and node-level helpers for them could be confused with negotiated per-peer, per-channel, or per-invoice features. Those negotiated feature surfaces are handled separately.
enigbe
force-pushed
the
2026-02-expose-node-features-api
branch
from
0668b4f to
888ba66
Compare
3 participants
What this PR does
For users interested in features announced in
node_announcement, we make node_features available viaNode::status().The plan is to modify
ldk-server'shandle_get_node_info_requestand make features available onGetNodeInfoResponseso that the node's supported features are available to any requesting client.lnrpcandcln-grpcmake these fields available in the response toget_node_info.This is in relation to an exploratory
sim-lnPR seeking to support/integrateldk-node/server.