descriptors: MuSig2

achow101 commented at 6:10 pm on November 7, 2024: member

Implements parsing of BIP 390 musig() descriptors.

Depends on #31243

Split from #29675

DrahtBot commented at 6:10 pm on November 7, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31244.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31590 (descriptors: Try pubkeys of both evenness when retrieving the private keys for an xonly pubkey in a descriptor by achow101)
#31519 (refactor: Use std::span over Span by maflcko)
#31243 (descriptor: Move filling of keys from DescriptorImpl::MakeScripts to PubkeyProvider::GetPubKey by achow101)
#30243 (Tr partial descriptors by Eunovo)
#29675 (wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys by achow101)
#29491 ([DO NOT MERGE] Schnorr batch verification for blocks by fjahr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label Descriptors on Nov 7, 2024

Sjors commented at 1:54 pm on November 26, 2024: member

Can you add an example to doc/descriptors.md?

Did you mean BIP390 in the description?

dergoegge commented at 10:06 am on December 9, 2024: member

0$ echo "cGsobXVzaWcoZGR9dXVlLzAwLylrKA==" | base64 --decode > mocked_descriptor_parse_1.crash
1$ FUZZ=mocked_descriptor_parse fuzz mocked_descriptor_parse_1.crash
2script/descriptor.cpp:1838 ParsePubkey: Assertion `Func("musig", expr)' failed.
3
4$ echo "dHIobXVzaWcoICAgICB0dXVzKG9sZGVwayhnZylnZ2dnZmdnKTwseigoKCgoKCgoKCgoKCgoKCgoKCgoKHN0KQ==" | base64 --decode > mocked_descriptor_parse_2.crash
5$ FUZZ=mocked_descriptor_parse fuzz mocked_descriptor_parse_2.crash
6script/descriptor.cpp:1833 ParsePubkey: Assertion `split.size() <= 2' failed.

achow101 force-pushed on Dec 9, 2024

achow101 commented at 10:34 pm on December 9, 2024: member

Fixed the fuzz crashes and added those descriptors to the unit tests.

dergoegge commented at 9:51 am on December 10, 2024: member

0$ echo "dHIobXVzaWcoJTIyLzMzMmgpSigoKChhZGRyKEJjdXUp" | base64 --decode > mocked_descriptor_parse_3.crash
1$ FUZZ=mocked_descriptor_parse fuzz mocked_descriptor_parse_3.crash
2script/descriptor.cpp:632 GetPubKey: Assertion `pubkey.has_value()' failed.

dergoegge commented at 10:19 am on December 10, 2024: member

0$ echo "dHIobXVzaWcoKS8wMDAwMTEp" | base64 --decode > descriptor_parse_1.crash
1$ FUZZ=descriptor_parse fuzz descriptor_parse_1.crash
2[libsecp256k1] illegal argument: pubkeys != NULL

achow101 force-pushed on Dec 10, 2024

achow101 commented at 5:16 pm on December 10, 2024: member

Fixed those crashes too.

dergoegge commented at 7:41 pm on December 10, 2024: member

0$ echo "dHIobXVzaWcoJWU5LzwwOzAwMDAwMzU7MDYwOzM7MDY+KS82Nik=" | base64 --decode > mocked_descriptor_parse_4.crash
1$ FUZZ=mocked_descriptor_parse fuzz mocked_descriptor_parse_4.crash
2fuzz: script/descriptor.cpp:1942: std::vector<std::unique_ptr<PubkeyProvider>> (anonymous namespace)::ParsePubkey(uint32_t &, const Span<const char> &, ParseScriptContext, FlatSigningProvider &, std::string &): Assertion `pub.size() == 1' failed.

achow101 force-pushed on Dec 30, 2024

DrahtBot commented at 7:46 pm on December 30, 2024: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/34992034826

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

DrahtBot added the label CI failed on Dec 30, 2024

achow101 force-pushed on Jan 3, 2025

DrahtBot removed the label CI failed on Jan 3, 2025

wallet, rpc: Only allow keypool import from single key descriptors

Instead of relying on implicit assumptions about whether pubkeys show up
or now after expanding a descriptor, just explicitly allow only single
key descriptors to import keys into a legacy wallet's keypool.

ed03adcdfc

descriptors: Have GetPubKey fill origins directly

Instead of having ExpandHelper fill in the origins in the
FlatSigningProvider output, have GetPubKey do it by itself. This reduces
the extra variables needed in order to track and set origins in
ExpandHelper.

Also changes GetPubKey to return a std::optional<CPubKey> rather than
using a bool and output parameters.

9c0e2befa7

descriptors: Move FlatSigningProvider pubkey filling to GetPubKey

Instead of MakeScripts inconsistently filling the output
FlatSigningProvider with the pubkeys involved, just do it in GetPubKey.

23de0c317b

descriptors: Have GetPrivKey fill keys directly

Instead of GetPrivKey returning a key and having the caller fill the
FlatSigningProvider, have GetPrivKey take the FlatSigningProvider and
fill it by itself.

GetPrivKey is now changed to void as the caller no longer cares whether
it succeeds or fails.

d7e8de33c3

XOnlyPubKey: Add GetCPubKeys

We need to retrieve the even and odd compressed pubkeys for xonly
pubkeys, so add a function to do that. Also reuse it in GetKeyIDs.

e61e892c18

spanparsing: Allow Const to not skip the found constant cdd784fca0

descriptors: Add PubkeyProvider::IsBIP32() 6d81e6e0b2

build: Enable secp256k1 musig module a08823bf55

sign: Add GetAggregateParticipantPubkeys to SigningProvider 66030a85a7

Add MuSig2 Keyagg Cache class and functions

- MuSig2KeyAggCache contains a MuSig2KeyAggCacheImpl which has the
  secp256ke_musig_keyagg_cache object to avoid having to link libsecp256k1
  everywhere.
- GetMuSig2KeyAggCache creates the MuSig2KeyAggCache from a
  std::vector<CPubKey>
- GetCPubKeyFromMuSig2KeyAggCache creates a CPubKey from a cache created
  with GetMuSig2KeyAggCache
- MuSig2AggregatePubKeys does the two above functions together.

5b8a6c35d3

descriptor: Add MuSigPubkeyProvider 722fdb9782

descriptor: Parse musig() key expressions c3913df2d7

tests: Test musig() parsing 7c65e9663a

achow101 force-pushed on Jan 6, 2025

descriptors: MuSig2 #31244

Code Coverage & Benchmarks

Reviews

Conflicts