Remove generate_ed25519_address() from the wallet?

[Thoralf-M]

Description

Move generate_ed25519_address() out of the wallet? Users can just generate with the secret manager before and then provide it to the wallet

Motivation

Simplification and clearer logic, no secret manager required for the wallet at this point

Open questions (optional)

Are there good reasons to not remove it?

Are you planning to do it yourself in a pull request?

No.

[Alex6323]

Pros:

• changes to the signature of that fn in the secretmanager don't have to be replicated in the corresponding wallet fn, so refactoring is easier
• we have get_secret_manager fn in the wallet, so we basically have this fn on the wallet by just one noticeable indirection
• we can more easily make the SecretManager optional in the wallet if we wawnted to: get_secret_manager would then just return a None if none is present.

Cons:

• users need to call get_secret_manager before being able to call that fn (1 more indirection)

So my current take on this matter is, that we SHOULD remove the method from the wallet. I don't see any huge downsides and mostly upsides.

Related questions:

• if we keep requiring a SecretManager, then WalletBuilder::with_address should be removed.

[DaughterOfMars]

On the flip side: The nice thing about this method is that it will use the wallet's configuration (generation bip44, bech32_hrp) but removing it means the user needs to get those things first then call the secret manager method then potentially iterate and convert the addresses to Bech32.

[thibault-martinez]

Closed by #1586