chore: should we remove unused SignerError variants ?

[oleonardolima]

Describe the enhancement

As noticed in #1448 and #1424 there are some unused error variants on SignerError

Use case

Do we need such variants, e.g sighash::TaprootError, transaction::TxInputsIndexError, or should it be merged with InputIndexOutOfRange (for example) ?

Additional context

Issue meant for related discussion.

[ValuedMammal]

Do you want to make this into a super issue to figure out further improvements to the signer module as a whole?

[oleonardolima]

Do you want to make this into a super issue to figure out further improvements to the signer module as a whole?

That could work, I don't see a problem doing it that way.

You mean also considering Steve's comments?

[LLFourn]

Do you want to make this into a super issue to figure out further improvements to the signer module as a whole?

The end goal is to remove the Signer trait and to remove Signers from wallet in favor of a library that helps you sign PSBTs with whatever key material you have. I think we'll tackle this during transaction building which comes in v2.

[ValuedMammal]

Playing with the idea of removing signers from the Wallet structure, I naively imagined simply adding a signer parameter to the Wallet::sign method thinking the user could construct their own SignersContainer just before signing. I realized that the wallet signers are used in create_tx in order to extract_policy which itself is somewhat of a rabbit hole, although it's curious that extracting a policy seems to works fine even when the wallet has no signers.. so maybe there's a way to disentangle that logic. Anyway I see this issue being related to the policy vs plan discussion, which in turn implies a tx-builder redesign. Still brainstorming ways to divide the problem into chunks we can easily tackle.

Regarding the proposal to remove the TransactionSigner trait, I think that would also simplify the whole concept of SignersContainer. Maybe we keep a sign method on Wallet that takes in a keymap or similar and internally calls Psbt::sign.

[notmandatory]

Ok to remove this from 1.0.0-beta milestone? Based on above it sounds like this should likely be fixed in a 2.0 release when signer is removed from wallet.

[oleonardolima]

Ok to remove this from 1.0.0-beta milestone? Based on above it sounds like this should likely be fixed in a 2.0 release when signer is removed from wallet.

Yes, as Lloyd and Mammal mentioned above it'll probably rely on TxBuilder redesign, which is already intended for v2 (please correct me if I'm wrong). Also, do we already have an issue for that?

[ValuedMammal]

Ok to remove this from 1.0.0-beta milestone? Based on above it sounds like this should likely be fixed in a 2.0 release when signer is removed from wallet.

Yes, as Lloyd and Mammal mentioned above it'll probably rely on TxBuilder redesign, which is already intended for v2 (please correct me if I'm wrong). Also, do we already have an issue for that?

These are some tx-builder issues that I'm aware of #899 #988