MuSig: Add Minimal Compatibility with BIP32 Tweaking

[jonasnick]

In short, musig_pubkey_tweak_add now allows for xonly and "ordinary" tweaking. Also, in order to allow using ec_pubkey_tweak_add on the non-xonly aggregate public key, there's a new function musig_pubkey_get that allows obtaining it from the keyagg_cache.

One alternative would be that instead of adding musig_pubkey_get, we could change pubkey_agg to output an ordinary (non-xonly) pubkey. Then users of the API who do not need ordinary (BIP32) tweaking would be forced to call xonly_pubkey_from_pubkey. And we'd probably want to change the spec. And it would be a bit weird to output a pubkey that can't be directly schnorrsig_verify'd.

Based on #131

[jonasnick]

Another reason why you'd want the full pubkey from key aggregation is that you need the parity bit for OP_TAPLEAF_UPDATE_VERIFY-style coinpools.

[jonasnick]

As @real-or-random remarked, instead of an xonly argument it's probably better to have two functions, ec_tweak and xonly_tweak, because they'd correspond to the existing tweaking functions.

[jonasnick]

rebased

[robot-dreams]

Nice, looks good on a first pass. I'm convinced that if you generate an aggregate pk, apply ordinary tweak to derive a BIP32 child key, then apply an x-only tweak to derive a BIP341 output key, you will get the correct result.

Before I finalize my review, I still want to:

• Check changes to tests_impl.h
• Try adding a test vector with a BIP32 tweak
• Think about if there's a simpler approach (e.g. keeping track of different invariants)

Regarding interaction with BIP32, am I understanding correctly that:

• To sign for a descendent pubkey, you just start with the aggregate pubkey (root of the tree), apply ec_tweak_add repeatedly with the parse_256(I_L) value at each level of the tree, and then proceed with the protocol as usual
• It generally shouldn't be possible to derive a hardened child (nor is this desirable) because no individual signer should have the aggregate private key

[jonasnick]

@robot-dreams Your understanding appears to match mine. As an aside, hardened derivation is not impossible.

[robot-dreams]

ACK 7474bc1 aside from remaining comments

I added a test vector for ordinary tweaking, feel free to grab the commit (see jonasnick#2) or just copy the vectors directly

I haven't thought about a way to simplify the invariants, but in retrospect that should probably happen in a different PR (if at all) instead of blocking this one. By the way I think it was a good idea to define f(i, P_agg[i - 1]) in the explanation.

[jonasnick]

Thanks @robot-dreams ! I added your commit.

[robot-dreams]

ACK 560aa3d

[jonasnick]

Squashed fixups.

[real-or-random]

Got interrupted, but let me leave some nits on the first commit here. I will continue later

[real-or-random]

(not this PR:) We should think about the terminology here. "Non-xonly" is totally fine to get this merged but I think it can be improved.

[real-or-random]

ACK mod nits.

Really all of this are only nits. It's good that we have this example code, it's very helpful.

[real-or-random]

Suggested change

	* Out: output_pubkey: pointer to a public key to store the result. Will be set
	* to an invalid value if this function returns 0. If you
	* do not need it, this arg can be NULL.
	* Out: output_pubkey: pointer to a public key to store the result (can be NULL).
	* Will be set to an invalid value if this function returns 0.

My eyes were grepping for "(can be NULL)" with the brackets and I felt some unease. I think it's good to be this consistent.

edit: Ahh I notice this part and other are copied from below. Well I suggest either ignore it or change it below. Sorry, can't "suggest" the code for the other function here (not in context of changed lines). Next time I'll just make a nit comment on top of it...

[jonasnick]

(can be NULL) is imho less informative than If you do not need it, this arg can be NULL..

[real-or-random]

ok.

[jonasnick]

Perhaps we can find a (tree-wide) wording that's both easily "eye-greppable" and understandable :)

[real-or-random]

I think just putting the entire sentence in parentheses would do the trick. But let's not hold up this PR further.

[real-or-random]

We're also making progress towards a better understanding of when to use which tweaks, and then how to deal with them during signing. I think we should write that up somewhere. It's probably related to usage notes on ordinary keys vs xonly keys.

[jonasnick]

a better understanding of when to use which tweaks

Hm, not sure what you mean. The straight forward answer is x-only tweaks for taproot, ordinary tweaks for everything else (including BIP32).

[real-or-random]

Hm, not sure what you mean. The straight forward answer is x-only tweaks for taproot, ordinary tweaks for everything else (including BIP32).

I may have meant "when to use x-only keys in protocols". But I'm not sure to be honest, ignore this part of my comment.

[jonasnick]

Thx for the review @real-or-random. Squashed fixups.

[robot-dreams]

ACK 8088edd based on #151 (comment) and the following range-diff:

git range-diff \
	a5b5909e8d56eb3c23aeb6eb4cdca0a621a0930b \
	560aa3d3a98081963d3c5d276773d20e07a3932d \
	8088eddc534cbbb89dd5f892828c4013416c4f2b