Expose node features in GetNodeInfoResponse

[enigbe]

What this PR does:

To run payment-activity simulations against networks of ldk-server nodes, clients such as sim-ln need to know whether a server advertises support for capabilities like keysend in its node_announcement.

This PR exposes node-announcement features through GetNodeInfoResponse.features, allowing clients to query the node directly for its advertised feature set. For now, only node-announcement features are populated; the proto documentation reflects this narrower contract while leaving room for future expansion to other feature contexts.

This is needed by the ongoing sim-ln work in bitcoin-dev-project/sim-ln#307.

[ldk-reviews-bot]

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

[benthecarman]

We just get a list like this

  "features": {
    "node": [
      0,
      8,
      128,
      136,
      152,
      136,
      10,
      138,
      81,
      161
    ]
  }

would be better if this had some more info, at least giving the name of each feature so we knew what it represented

[ldk-reviews-bot]

🔔 1st Reminder

Hey @benthecarman! 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.

[benthecarman]

this format is still weird..

We have the name as the key and we put it in the name field. is_known and is_supported are both always true as the way we get them makes it inherent.

Also doesn't make it entirely clear if we are signaling the required or optional bit, just that we are signaling one of them.

We also don't need the node object here

  "features": {
    "node": {
      "DataLossProtect": {
        "name": "DataLossProtect",
        "is_supported": true,
        "is_required": true,
        "is_known": true,
        "supported_bit": 1,
        "required_bit": 0
      },

lnd does a good job of this, where the bit is the key and the data is available about it. They have is_known because you can optionally flag bits on in their config, we don't support this so we don't need this field.

    "features":  {
        "0":  {
            "name":  "data-loss-protect",
            "is_required":  true,
            "is_known":  true
        },
        "5":  {
            "name":  "upfront-shutdown-script",
            "is_required":  false,
            "is_known":  true
        },

[benthecarman]

we are always setting this to true, do we even need this field?

[enigbe]

We have the name as the key and we put it in the name field. is_known and is_supported are both always true as the way we get them makes it inherent. Also doesn't make it entirely clear if we are signaling the required or optional bit, just that we are signaling one of them.

Fair point. I agree that it's clunky and will switch this to LND's bit-keyed style.

We also don't need the node object here

  "features": {
    "node": {
      "DataLossProtect": {
        "name": "DataLossProtect",
        "is_supported": true,
        "is_required": true,
        "is_known": true,
        "supported_bit": 1,
        "required_bit": 0
      },

lnd does a good job of this, where the bit is the key and the data is available about it. They have is_known because you can optionally flag bits on in their config, we don't support this so we don't need this field.

I added the node object to leave room for the same feature-context distinction that cln-grpc exposes through ourFeatures:

{
  "ourFeatures": {
    "init": "800898880a8a59a1",
    "node": "808898880a8a59a1",
    "channel": "",
    "invoice": "02000002024100"
  }
}

The shape I had in mind was something like this, but with decoded, instead of raw bytes:

{
  "init": {
    "0": { "name": "DataLossProtect", "is_required": true },
    "5": { "name": "UpfrontShutdownScript", "is_required": false },
    "8": { "name": "VarOnionOptin", "is_required": true }
  },
  "node": {
    "0": { "name": "DataLossProtect", "is_required": true },
    "55": { "name": "Keysend", "is_required": false }
  },
  "invoice": {
    "8": { "name": "VarOnionOptin", "is_required": true },
    "14": { "name": "PaymentSecret", "is_required": true }
  }
}

That said, since this response currently only exposes node-announcement features, I'm fine flattening it for now and adding contextual grouping later if we expose the other feature sets.

[benthecarman]

Awesome looks good! Can we just sort the features by their bit, currently they are random from the hash map. Feel free to do that in a squash and we should be able to get this in!

[enigbe]

Awesome looks good! Can we just sort the features by their bit, currently they are random from the hash map. Feel free to do that in a squash and we should be able to get this in!

We have feature ordering by bit now by replacing the HashMap with a BtreeMap.
I also squashed the fixups.

[benthecarman]

thanks!