Adds a set of convenience methods to set non-custom features #2522
Conversation
|
Is this better done by deserializing the features object? We could add a new constructor that makes that a bit more explicit? |
|
Sure, a deserializer that takes a collection of feature bits and builds the |
|
Regardless of what lnd's API does, I still think it's a useful change, and we already have the logic to do it on custom feature bits. |
|
I mean I don't really see the point here if not to support some strange API that outputs a list of features by their bits and we want to "deserialize" that - if you want to build a features object with BOLT-range features, you probably want to select the features via the usual setters, setting it via a bit is pretty weird? |
|
I agree, but still I'd rather just expose this than have the user (who may not be able to change the API they're using) implement it themselves. |
|
Oh, I agree, if there's no better way to munge LND's API responses into features this is better than not-this. |
lightning/src/ln/features.rs
Outdated
| self.set_feature_bit(bit - (bit % 2)) | ||
| } | ||
|
|
||
| /// Sets an custom feature bit. Errors if `bit` is outside the feature range as defined |
There was a problem hiding this comment.
I just realised this read "an custom feature bit" when it should read "an optional feature bit"
I feel your pain and I partially agree with your view, but I don't think there is any other way to get the feature bit data from LND using their RPC interface. If you think this is worth the compromise, I'm happy to fix the nits, if not we'll go with a custom parser downstream |
|
Yea, okay, IMO let's take this, I'm just annoyed. |
lightning/src/ln/features.rs
Outdated
|
|
||
|
|
||
| // Set flags manually | ||
| let mut features = NodeFeatures::empty(); | ||
| assert!(features.set_optional_feature_bit(55).is_ok()); | ||
| assert!(features.supports_keysend()); | ||
| assert!(features.set_optional_feature_bit(255).is_ok()); | ||
| assert!(features.set_required_feature_bit(256).is_err()); | ||
|
|
There was a problem hiding this comment.
nit: there're just a few extra newlines here.
There was a problem hiding this comment.
Should be fixed by 992f102
Currently only custom features can be set by specifying the feature bit. Add also the ability to do so for regular features.
sr-gi
force-pushed
the
202308-set-feature
branch
from
bc02b99 to
992f102
Compare
|
Fixes nits |
Currently only custom features can be set by specifying the feature bit. This PR adds the ability to do so for regular features too.
The rationale for this PR is being able to build a Features instance from a collection of feature bits, as returned different LND's RPC responses (such as
get_info,get_node_info, ...)