Implement Public Key Check in `wallet_name_from_descriptor`

[AmosOO7]

Description

The wallet_name_from_descriptor function in src/wallet/mod.rs has a TODO comment: // TODO: check descriptors contains only public keys. This should prevent generating wallet names from descriptors containing private keys, ensuring names are derived from public information only.

Proposed Implementation

• After parsing the descriptor via into_wallet_descriptor, check if the returned KeyMap is empty.
• If !keymap.is_empty(), return DescriptorError::Miniscript(miniscript::Error::Unexpected("Descriptor must contain only public keys".to_string())).
• Apply the same check to the optional change_descriptor.
• This leverages the fact that KeyMap contains secret keys only when the descriptor string included private keys.

[ValuedMammal]

ensuring names are derived from public information only

I don't think it's a problem if the keymap isn't empty since into_wallet_descriptor returns the public descriptor - this is correct since an xprv and its derived xpub still define the same wallet, so I would update the docs to say the name is defined by the checksums of the wallet's public descriptors.

There is another problem though which is the code currently expects the checksum to be there, relying on an implicit contract via to_string. The implementation would be more self-contained if we computed the checksum ourselves on the parsed descriptors to obtain the wallet name.

[AmosOO7]

ensuring names are derived from public information only

I don't think it's a problem if the keymap isn't empty since into_wallet_descriptor returns the public descriptor - this is correct since an xprv and its derived xpub still define the same wallet, so I would update the docs to say the name is defined by the checksums of the wallet's public descriptors.

There is another problem though which is the code currently expects the checksum to be there, relying on an implicit contract via to_string. The implementation would be more self-contained if we computed the checksum ourselves on the parsed descriptors to obtain the wallet name.

Thanks, that makes sense. I’ll remove the private-key KeyMap check and since you will clarify in the docs that the wallet name is derived from the public descriptor checksums or should I do that and just add a comment above the function that explains that?

I’ll also make the implementation self-contained by computing the checksum from the parsed descriptor string instead of assuming to_string() includes #checksum

[ValuedMammal]

Thanks, that makes sense. I’ll remove the private-key KeyMap check and since you will clarify in the docs that the wallet name is derived from the public descriptor checksums or should I do that and just add a comment above the function that explains that?

If you agree I think it could be added to #482.

[AmosOO7]

Thanks, that makes sense. I’ll remove the private-key KeyMap check and since you will clarify in the docs that the wallet name is derived from the public descriptor checksums or should I do that and just add a comment above the function that explains that?

If you agree I think it could be added to #482.

Yes I will