Taproot keysigning is confusing and the API is unhelpful

[apoelstra]

Right now in our Satisfier API we have a lookup_tap_key_spend_sig method which takes no arguments and returns an Option<Signature>.

The premise, I guess, is that there is only one valid key that can sign a keyspend and the satisfier should know this so there's no need to pass in any extra context.

Well, when developing software you might not know the key, at least initially, because

• The policy compiler will pick a sorta-random key from your policy to be the keyspend key, and it doesn't tell you which one it picks, so you need to know that it does this and then inspect the resulting descriptor
• The key in the policy isn't even the key that you sign with. You need to tweak the key in the policy.

This would be greatly improved by the API providing a Pk and maybe also a TweakedPublicKey and maybe even the taptweak? And the docs should be improved to explain these things.

In general I feel the satisfier API could be greatly improved to provide more context and information to the satisfier.

Related to #742

[sanket1729]

This would be greatly improved by the API providing a Pk and maybe also a TweakedPublicKey and maybe even the taptweak? And the docs should be improved to explain these things.

Can you elaborate on this?

I feel that this logic belongs outside the satisfier API? There is only one internal key signature and it is the responsibility of implementer to provide it. Internal keys are treated differently signers anyways because the sighash message algorithm is different keyspend and script spend.

[apoelstra]

I feel that this logic belongs outside the satisfier API?

Where does it then belong? Maybe the placeholder objects from the planning API should have a richer API that explains how to construct non-placeholder versions?

One use case (aside from testing) is to implement a wallet that is willing to sign a given transaction. It has a Transaction or Psbt or whatever and is willing to trust that our software is producing correct sighashes from that. But it doesn't necessarily care whether it is keyspending or script-spending or even using Taproot or not. It just wants to produce a signature that works.

[sanket1729]

@apoelstra
I feel rust-bitcoin psbt APIs in do all the it. You give it a psbt and a bip32 key. I believe our current flow is the most user-friendly we’ve implemented so far. While the documentation could be improved, I don't see how it can be improved further.

// Create an unsigned tx -- CREATOR
let mut psbt = Psbt::from_unsigned_tx();
// Updator role
pbst.witness_utxo : TxOut = ...; // Spending utxo information
psbt.update_input_with_descriptor(input_idx, desc);
// SIGNER:
let master_xprivs : BtreeSet<bitcoin::bip32::XPriv> = todo!(); // set of all private keys that we want to sign with
psbt.sign(&master_xprivs, &secp); // Works for all types of outputs, tr, wsh etc..
// FINALIZER:
psbt.finalize_mut(&secp); 

The API will figure out whether it is the internal key or script spend etc and fill everything as required.

[apoelstra]

I'm aware that the PSBT API exists but (a) you then need to learn to use the Psbt API, which ok is not that complicated, but more importantly (b) there are no examples that show that simple 5-line thing that you posted.

Amusingly the closest one is the "big" example which Antoine wrote as a cargo bloat testcase and which has a big "this will not work if you try to run it" disclaimer.

For specific API improvements:

• Provide accessors for the tweaked keys on Descriptor (or at least Tr), and elaborate in the docs on Tr::internal_key that this is not the key that is to be signed with.
• Provide an accessor for a sighash on Descriptor, which I guess would take a Transaction and an input index and a list of prevouts.

I appreciate that the latter is basically a poor man's PSBT but it's way easier to find and get working than PSBT since it has fewer moving parts.

[apoelstra]

I also think we could do better with the type system to distinguish between taproot and non-taproot keys, but it's hard and I won't try to make specific proposals here. Maybe once my expression and "validation parameters" stuff all gets in I'll give it a shot.

But one thing that comes to mind now, so I might not forget, is to replace the is_xonly_key stuff on MiniscriptKey with an accessor as_taproot_key(&self) -> Option<&dyn TaprootKey> where TaprootKey is a new trait that provides methods for x-only key management, generating unspendable key(s), somehow distinguishing between internal/external keys, etc. But the devil is in the details there because making the trait object-safe probably conflicts with a lot of my other goals for it.