Add MuSig Key Aggregation spec

[jonasnick]

This PR is for discussing the MuSig (Key Agg) specification. I started this by documenting what the code does currently.
In particular, the open design questions include:

• Should hash index or should hash key?
  • how to encode the index (right now it's least significant first (EDIT: changed that) but it's most significant for every other integer)?
• Should the first MuSig coefficient be the constant 1? What would be the security argument?
• Should sort pubkeys in KeyAgg algorithm?

[jonasnick]

Just realized that this does not reflect the currently implemented key aggregation because I was looking at the wrong branch (master instead of secp256k1-zkp...).

[jonasnick]

Fixed

[jonasnick]

We may also want to consider how this relates to signature aggregation (H/T @roconnor-blockstream)

[roconnor-blockstream]

My specific consideration was whether we want this key aggregation function to be use both for key aggregation and signature aggregation. If we want it to be usable for signature aggregation, I believe that will place some constraints on the answers to Jonas's question.

[roconnor-blockstream]

If we want our Key Aggregation function to be compatible with multi-message signing (i.e. cross-input aggregation), then we want to allow the same public key to be able to aggregate their signatures on multiple (different) messages. This all but precludes hashing by key because each different signed message for the same public key will need a different musig coefficient.

There needs to be an association between the unique musig coefficient and the message being signed. One way of making this association would be to put the signed message directly into the musig coefficient. However we would like to be able to compute the Key Aggregation before the messages are known. Therefore the association between the message and the musig coefficient needs be indirect. Using indexes is one natural way of building this indirection. Another way would be to create a guid and add guid × message pairs to the aggregate message, but that appears to have no advantages over using indexes.

While hashing by key essentially forces unordered keys, hashing by index allows for the possibility of using an ordered set of pubkeys. Protocols wanting an unordered set of pubkeys, such as Bitcoin's multisig descriptor, can easily build upon an ordered api by sorting their keys. Indeed this is what the descriptor currently does.

For the specific application of cross-input aggregation, if the ordered api is available, what we'd likely do is order the pubkey × message pairs by input and then by execution order within the input. If only an unordered api is available, then we would collect those pairs and sort them by pubkey, and then within each repeated set of pubkeys, sort the messages when created the aggregate message. It strikes me that being forced to use the unordered api is slightly more complex than going with the ordered api. OTOH, if the unsorted API handles all the work, maybe it isn't so bad. (Sorry, I've failed to come to a conclusion on the sorting question.) (self-aside: Because Simplicity doesn't have a consensus critical execution order, so I would probably end up sorting the pubkey × message pairs emitted by a Simplicity execution.)

I haven't yet given any consideration to nested multi-message signing.

I understand the jonas has an MBDL security proof sketch for the 1 optimization and tim has a security proof sketch for the OMDL + ROM.

P.S. I hate little endian formats. Also isn't 32-bytes too much?

P.P.S. indexes should be 0 based per Dijkstra.

[jonasnick]

P.S. I hate little endian formats. Also isn't 32-bytes too much?

My mistake, should have been 4 bytes. Also changed spec and C code to encode index with MSB first.

[roconnor-blockstream]

@jonasnick has explained to me in private that signature aggregation does not work the way I had imagined it to work. signature aggregation requires a (modified) Bellare-Neven signature, which is not the same thing as a Schnorr signature. (In particular jonas pointed out that the scheme I had in mind is broken.)

Thus we can disregard my comments about signature aggregation.

This puts hash-by-key back on the table as an option. IMHO it is an option we should seriously consider.

[jonasnick]

What would be the advantage of hash-by-key? My understanding is that we need to serialize the whole multiset of public keys before hashing them to a MuSig coefficient and therefore have to assign an order anyway.

[jonasnick]

As @roconnor-blockstream mentioned to me in a separate channel, there isn't really an advantage of hash-by-key besides perhaps being a little bit simpler.

OTOH there isn't a disadvantage either. Since the input public keys are 32 byte arrays ("x-only"), hash-by-key wouldn't require additional SHA rounds. (EDIT: that's obviously wrong due to padding.... I always forget that)

[real-or-random]

Here's my take:

Data structure that contains the public keys: ordered list

This could be a set, a multiset (as in the paper), or an ordered list. I think it should be an ordered list.

Duplicate public keys allowed: yes

I don't think there's any use case that strictly needs duplicate public keys when all involved peers are honest. But making duplicate public keys illegal is difficult in higher-level protocols, where an attacker simply copies the key: Say you run MuSig with random strangers of which you don't know their public keys in a higher-level protocol (e.g., in a CoinJoin-like protocol). Moreover say there's a honest peer with public key A, and there's a malicious peer who also claims to have public key A. If key aggregation with [A, A] is illegal, then the higher-level protocol must fail. In this case, you'd at least like to know who is to blame for the failure but this at least requires additional complexity to make it possible to tell apart which of the peers is malicious. You could also canonicalize the key aggregation to [A] then, effectively removing duplicates. But this again adds complexity.

So we should allow for duplicate public keys. However, we don't really need to optimize for this case, I think.

Canonical ordering/sorted: no

Users of the spec know best in what representation they want to deal with the public keys, so I believe we should not put any further restrictions. In particular, we shouldn't require that public keys are in some particular canonical ordering. I don't see how anyone would strictly need order-independence. If users really need this, they can always enforce this by sorting.
But we should not force the users to implement sorting, nor should we make libraries forcing sorting upon their users.

I believe there are many cases where higher-level protocol already have some indexed list of peers, so it should possible to simply take that list and use it.

Optimize first coefficient to be 1: yes

We should do this, simply because it's more efficient. Removing a scalar multiplication is just not a micro-optimization, it can be a real difference for embedded devices, in particular when the number n of peers is small.

Of course, we need to double-check that the security proof still works out.

Hashing the index instead of the public key: probably yes

I don't think this makes much of a difference in terms of complexity. It makes a difference for duplicate public keys but I don't think we should optimize for this case (see above). So if a saves a compression, we should probably hash the index it. But this depends on how the hash is constructed exactly

Additional argument for the hash: yes?

Should we hash an additional byte string that can be chosen by the user? This would make it possible for instance to have a series of aggregate keys ~X that look unrelated to each other if you don't know the underlying list of public keys (by using a counter as the additional argument). Is this useful to avoid key reuse? I believe yes but I'm not entirely sure.

Here we should also take into account possible extensions to threshold signing, where key setup is more difficult and there may be good reasons to avoid doing setup over and user.

[jonasnick]

Users of the spec know best in what representation they want to deal with the public keys,

My thinking is that ideally, they wouldn't have to care about the representation at all. @apoelstra mentioned that wallet devs are in favor of sorting the public keys in a MuSig descriptor. Then why not sort in MuSig key aggregation directly? The argument would be that that's not flexible enough, has additional computational cost for callers who really don't need them signed or has a high penalty in terms of usability.

In the meantime I have been working on an implementation of sorting inside key aggregation to explore these questions. It required a few refactorings but the meat of it happens at the beginning of musig_combine_pubkeys:

    /* Fast sorting algorithms have non-linear best case complexity. Therefore,
     * we only sort if the pubkeys are not already sorted */
    if (!is_sorted(pubkeys, n_pubkeys)) {
        if (signers != NULL) {
            secp256k1_musig_signers_init(signers, n_pubkeys, 1);
            sort_pubkeys(pubkeys, n_pubkeys, signers);
        } else {
            qsort(pubkeys, n_pubkeys, sizeof(*pubkeys), cmppubkey);
        }
    } else if (signers != NULL) {
        secp256k1_musig_signers_init(signers, n_pubkeys, 0);
    }

If the musig_combine_pubkeys call isn't the start of a signing session, we just qsort the pubkeys. Otherwise, we call sort_pubkeys which 1) sorts the signers based on their corresponding pubkey 2) assigns indices to the signers referring to the position of the pubkey in the sorted list 3) puts the pubkeys in the right position 4) signers are put in the same position (not really) they were before. As a result, the pubkeys list is sorted but signer[i] still refers to the i-th pubkey in the original list (Just noticing the nested loop now, so the algorithm is actually in O(n^2) :[).

I haven't fully repaired the tests yet, because the main problem is that the initial pubkey array can not be reused for other signers, because its order changed after the musig_pubkey_combine call. Another issue is that instead of calling partial_sig_verify(pubkeys[i], ...), one has to call partial_sig_verify(pubkeys[signers[i].index], ...) but that may be fixable.

[real-or-random]

Users of the spec know best in what representation they want to deal with the public keys,

My thinking is that ideally, they wouldn't have to care about the representation at all. @apoelstra mentioned that wallet devs are in favor of sorting the public keys in a MuSig descriptor. Then why not sort in MuSig key aggregation directly? The argument would be that that's not flexible enough, has additional computational cost for callers who really don't need them signed or has a high penalty in terms of usability.

Indeed, that's my argument. I don't want to stop anyone from sorting the keys but as I as a spec writer I wouldn't want to force the decision upon users of the spec who may not need it.

    /* Fast sorting algorithms have non-linear best case complexity. Therefore,
     * we only sort if the pubkeys are not already sorted */

Are you aware of https://en.wikipedia.org/wiki/Adaptive_sort ?

[real-or-random]

I think that the fact that we anyway have indexed arrays in our API makes sorting somewhat pointless: The user sometimes needs to pass an index to identify a specific public key. In that sense, the user anyway needs to look at this as an indexed data structure. Since the indices are there anyway, sorting the keys internally won't make anything significantly simpler from the perspective of the user. (Even if we'd require the user to pass public keys instead of indices, the user probably needs to look these public keys up in an array, so that doesn't really help.)

[jonasnick]

Additional argument for the hash: yes?

Alternatively, users can p2c-tweak their keys. The current API only supports doing that once which prevents adding an additional taproot commitment.

[apoelstra]

After talking with @jonasnick I agree with @real-or-random that we should not try to sort the keys internally. It seemed like the simpler option but actually it makes the API a fair bit more complex, as well as (potentially) imposing a performance hit on users who don't want it.

[jonasnick]

I haven't fully repaired the tests yet, because the main problem is [...] Another issue is that instead of calling [...] but that may be fixable.

One way to fix this would be to sort the pubkey array back into the original order, but that would only work by requiring additional space even if no signin session is initiated. Instead, I pushed the following changes

1. The MuSig coefficient is a hash of the public key, not the index. This also allows to completely remove the signers array in MuSig2.
2. A new sort_pubkeys function is added that can be used before combine_pubkey. Since this is now separate, in-place sorting of pubkeys doesn't create a mess in the tests.

[jonasnick]

I pushed a commit to add an optimization where the second unique key in the pubkey list gets MuSig coefficient 1 as per the latest (unpublished) revision of the MuSig2 paper.
The actual spec is still a very rough draft, but it can be improved over time. I think this PR is ready for review and I reordered the commits to make that easier.

To quickly summarize the changes:

• A function for sorting public keys allows to aggregate keys independent of the (initial) order.
• The MuSig coefficient is computed by hashing the key instead of key index because. Otherwise, if the pubkey list gets sorted, the signer needs to translate between key indices pre- and post-sorting. It also means that with MuSig2 we can completely remove the secp256k1_musig_session_signer_data struct.
• The second unique key in the pubkey list gets the constant MuSig coefficient 1 which saves an exponentiation.

It doesn't yet accept an additional argument like a counter to allow generating distinct keys from the same list of pubkeys as @real-or-random suggested.
I think this is an interesting idea, but maybe we can discuss this on a separate PR.

[real-or-random]

This sounds good. What caught my eye on a first glimpse is that there are a few places where secp256k1_memcmp_var should be used instead of memcmp. I'll have a closer look soon.

[jonasnick]

rebased

[real-or-random]

Asan is complaining here

[real-or-random]

ACK mod nits

[real-or-random]

If you want, you could try playing around with https://gcovr.com/en/stable/guide.html#exclusion-markers . (But this can be done later)

[jonasnick]

let's do this later

[real-or-random]

Wait, the sorting function has disappeared. Is this intentional?

[jonasnick]

@real-or-random oops...messed up the rebase

[real-or-random]

ACK fc26ca8

[RCasatta]

this should be 2?

I don't think this is the issue but my toy code is failing on this case (while the other 3 are ok)

[jonasnick]

second_pk_idx = 2 and second_pk_idx = 3 are both correct since they both get keyagg coefficient = 1. But agree that it's better to use the first one encountered here (and perhaps add comment that second_pk_idx = 3 is equally valid).

[RCasatta]

May I know why this is not a tagged hash?

[jonasnick]

The key agg coefficient should be domain separated since the outer hash is tagged. But since to the best of my knowledge domain separation isn't well defined, I'm leaning towards making this tagged. This would also prevent discussions like this in the future (and we're obviously already using a lot of tagged hashes). How about tag "KeyAgg list"?

[RCasatta]

How about tag "KeyAgg list"?

Sounds good