rust-lightning icon indicating copy to clipboard operation
rust-lightning copied to clipboard

Separate secret key material from config parameters in LSPS2 and LSPS5

Open martinsaposnic opened this issue 8 months ago • 3 comments

As discussed here https://github.com/lightningdevkit/rust-lightning/pull/3662#discussion_r2012088297

Currently, secret key material such as signing_key in LSPS5 and promise_secret in LSPS2 is included directly in the Config structs. While this works for now, it's not ideal to mix sensitive cryptographic material with general configuration parameters.

We should consider refactoring the codebase to store secrets separately from the rest of the configuration data.

martinsaposnic avatar Apr 17 '25 15:04 martinsaposnic

I see that we don't immediately have a better way to do things in the current scheme, but still want to note.

@tnull do you think it would be enough to move the secrets into a separate struct and have the LSPS2ServiceHandler constructor take that as an argument? Or were you thinking of a deeper refactor than that?

martinsaposnic avatar Jun 04 '25 14:06 martinsaposnic

I see that we don't immediately have a better way to do things in the current scheme, but still want to note.

@tnull do you think it would be enough to move the secrets into a separate struct and have the LSPS2ServiceHandler constructor take that as an argument? Or were you thinking of a deeper refactor than that?

Hmm, it would be a bit awkward then that we could configure the LiquidityManager to act as client/service or a specific protocol, while not forcing the user to necessarily configure the required secrets via the API. Say we configure as an LSPS2 service, if we just add another struct parameter to LiquidityManager::new we'd end up with a case where LSPS2 is configured, but not the necessary secret, requiring us to error out at runtime. It would be great to come up with an API that doesn't allow this, e.g., a builder pattern could be used to construct LiquidityManager.

tnull avatar Jun 05 '25 08:06 tnull

signing_key on LSPS5 does not exist anymore. this would only need to happen for promise_secret on LSPS2

martinsaposnic avatar Aug 07 '25 14:08 martinsaposnic