lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.15k stars 364 forks source link

Custom Feature Bits #2131

Closed TheBlueMatt closed 1 year ago

TheBlueMatt commented 1 year ago

Now that we set feature bits based on a message handler call rather than a hard-coded thing, we need to support setting custom feature bits explicitly.

TheBlueMatt commented 1 year ago

Also when we do this we need to allow deserializing a BOLT11 invoice with unknown required bits, as long as the user tells us they support it.

jkczyz commented 1 year ago

Looks like we'll need to expose Features::or as it would be needed in lightning-custom-message crate. Thoughts on exposing it via implementing core::ops::BitOr?

TheBlueMatt commented 1 year ago

As you know I despise compiler magic, but I admit here it would read nice. I don't have a strong opinion, do what you think makes sense.

jkczyz commented 1 year ago

Hmm... so it occurs to me that in PeerManager::handle_message we check InitFeatures::requires_unknown_bits and error if true. Seems we'll instead need to check against those the features required by the message handlers rather than the ones LDK is aware of. Does that sound right?

Related: currently we can only set an unknown feature via Features::from_le_bytes. I assume we don't want to expose our sealed traits. Maybe a method for setting arbitrary feature bits is sufficient?

TheBlueMatt commented 1 year ago

Right, we'll have to call all the handlers and get all their features for that check. And, yes, we'll want a custom features setter, with docs clearly stating you should only set it to values in the experimental range (and probably fail if it's actually a known bit).

jkczyz commented 1 year ago

So there is an experimental range for message types, but not for features, AFAICT.

Also, I suppose it would be possible to set an unknown feature bit in one context even though it may be known in another, unless we check across all context. Not sure how particular we want to get about this, though.

TheBlueMatt commented 1 year ago

bLIP 2 mentions > 255 - https://github.com/lightning/blips/blob/master/blip-0002.md#feature-bits

TheBlueMatt commented 1 year ago

I think we just need to pass feature bits in all the places we check them - mostly invoice deserialization and init/peer connection?

jkczyz commented 1 year ago

I think we just need to pass feature bits in all the places we check them - mostly invoice deserialization and init/peer connection?

I meant when setting the bit.

carlaKC commented 1 year ago

Ran into this https://github.com/lightning/blips/pull/24 working on custom features for LND, might be useful here if you're allowing folks to set custom invoice feature bits.

TheBlueMatt commented 1 year ago

Yep, the above PR should do that for the low-level API, though we may want to do a followup which adds the same for the higher level invoice construction utilities.