cashubtc / cdk

Cashu Development Kit
MIT License
85 stars 44 forks source link

Wallet: Support only nut13 deterministic wallets #125

Closed thesimplekid closed 6 months ago

thesimplekid commented 6 months ago

Currently both randomly generated secrets and NUT-13 derived secrets are supported. While the spec does not require supporting only NUT-13 doing so would reduce code complexity as the two options don't have to be handled separately. This change can also be made transparent to the lib user as if no secret is passed one can be generated for them, that they can choose to never use. Going this route leads to a more sensible default implementation of deterministic secrets allowing for CDK based wallets to be recovered by default instead of an extra feature as they are now.

davidcaseria commented 6 months ago

This change can also be made transparent to the lib user as if no secret is passed one can be generated for them, that they can choose to never use.

Can you explain this a little more? Does this require saving the Xpriv somewhere?

thesimplekid commented 6 months ago

This change can also be made transparent to the lib user as if no secret is passed one can be generated for them, that they can choose to never use.

Can you explain this a little more? Does this require saving the Xpriv somewhere?

Yes good question. If we were to make it transparent by generating a secret it would be like the current Wallet::new where the xpriv is an option and would be generated when calling Wallet::new. https://github.com/cashubtc/cdk/blob/2671c92c00353cd9656d0d678c3d9cb1c67b30f1/crates/cdk/src/wallet.rs#L110-L120

Doing this we would have two options either store it. This would have the benefit being able to restore in the future. However, unless we also required a passphrase we would be storing an unencrypted secret.

The second is to not store the xpriv and generate a new one each time Wallet::new is called in this case a restore wouldn't be possible unless the user stores the xpriv.

The third option which I think might be best is require a seed. This requires an extra step on the user to create a seed before calling Wallet::new but i think its the most clear in expected function. This case allows use to avoid storing a secret like option 1 and avoids the potential for a user to think they have the ability to restore later but never saving the seed as could be the case in option 2.

davidcaseria commented 6 months ago

Ok great. I have a local branch that removes the nut13 conditional feature logic and allows the user to provide a seed to Wallet::new. I think this is the best approach. Should I make a PR while we are still waiting for feedback?

thesimplekid commented 6 months ago

If you have it done already might as well open the PR.

thesimplekid commented 6 months ago

Looks like nutshell actually came up with the same thing

https://github.com/cashubtc/nutshell/blob/52fbfc4b21df7ca94492ba5e5b3f01101969781a/cashu/wallet/secrets.py#L96-L143