Document better optional features#1514
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1514 +/- ##
==========================================
- Coverage 90.93% 90.91% -0.02%
==========================================
Files 80 80
Lines 43469 43469
Branches 43469 43469
==========================================
- Hits 39527 39520 -7
- Misses 3942 3949 +7
Continue to review full report at Codecov.
|
TheBlueMatt
left a comment
There was a problem hiding this comment.
Thanks! This already needs rebase for the ZeroConf feature lol. sorry about that.
lightning/src/ln/features.rs
Outdated
| //! - `DataLossProtect` - requires/supports that a node, which has somehow fallen behind (e.g. has been restored from old backup), | ||
| //! can detect that it has fallen behind | ||
| //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md) for more information). | ||
| //! - `set_data_loss_protect_optional` - sets this feature to optional. |
There was a problem hiding this comment.
Hmm, do we need to list the methods? In general I think users should never change them manually.
There was a problem hiding this comment.
Sounds good - I will remove them.
lightning/src/ln/features.rs
Outdated
| //! And the implementation can interpret a feature if the feature is known to it. | ||
| //! | ||
| //! The following features are currently supported in the LDK: | ||
| //! - `DataLossProtect` - requires/supports that a node, which has somehow fallen behind (e.g. has been restored from old backup), |
There was a problem hiding this comment.
We should mention somewhere what we require vs support. I think currently we may only require StaticRemoteKey, but in the near future we'll probably require VariableLengthOnion too.
There was a problem hiding this comment.
I will make two lists: one for required features (StaticRemoteKey and VariableLengthOnion) and one for supported features (everything else).
lightning/src/ln/features.rs
Outdated
| //! (see [BOLT-3](https://github.com/lightning/bolts/blob/master/03-transactions.md) for more information). | ||
| //! | ||
| //! The following features are currently supported in the LDK: | ||
| //! - `DataLossProtect` - requires/supports that a node, which has somehow fallen behind (e.g. has been restored from old backup), |
There was a problem hiding this comment.
| //! - `DataLossProtect` - requires/supports that a node, which has somehow fallen behind (e.g. has been restored from old backup), | |
| //! - `DataLossProtect` - requires/supports that a node which has somehow fallen behind, e.g., has been restored from an old backup, |
lightning/src/ln/features.rs
Outdated
| //! - `ShutdownAnySegwit` - requires/supports that future segwit versions are allowed in `shutdown` | ||
| //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#closing-initiation-shutdown) for more information). | ||
| //! - `ChannelType` - node supports the channel_type field in open/accept | ||
| //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#closing-initiation-shutdown) for more information). |
There was a problem hiding this comment.
I think this link to BOLT 2 and the following two should not link to the closing-initiation-shutdown subsection.
lightning/src/ln/features.rs
Outdated
| //! - `SCIDPrivacy` - supply channel aliases for routing | ||
| //! (see [BOLT-2](https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#closing-initiation-shutdown) for more information). | ||
| //! - `Keysend` - send funds to a node without an invoice | ||
| //! (see [BOLT-11](https://github.com/lightning/bolts/blob/master/11-payment-encoding.md) for more information). |
There was a problem hiding this comment.
Pointing users towards this URL may be misleading since BOLT 11 does AFAIK not contain any further information on keysend payments?
There was a problem hiding this comment.
I think you are right - I will link to the Keysend feature assignment proposal.
Closes #438.
Issue: Post #428 merge, would be great to document FeatureContext* setters (lightning/src/ln/msgs.rs), I don't think optional features are documented anyway in the current codebase beyond references to BOLT.
Changes: Added documentation in
features.rson what the currently-supported features do in the LDK based on BOLT-9 feature flags.