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:

#31868 ([IBD] specialize block serialization by l0rinc)
#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 ([EXPERIMENTAL] 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

achow101 force-pushed on Jan 6, 2025

DrahtBot added the label Needs rebase on Jan 21, 2025

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

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

482e8c60f7

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.

e743d11aae

descriptors: Move FlatSigningProvider pubkey filling to GetPubKey

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

b04bce2cd9

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.

1bfabb4f4e

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.

f0d92421be

spanparsing: Allow Const to not skip the found constant b301e12923

descriptors: Add PubkeyProvider::IsBIP32() b94bc3f7b6

build: Enable secp256k1 musig module 684496e157

sign: Add GetAggregateParticipantPubkeys to SigningProvider 77db59e6df

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.

e8f76f6942

descriptor: Add MuSigPubkeyProvider f19462e862

descriptor: Parse musig() key expressions 685e3259ce

tests: Test musig() parsing efe85f891e

achow101 force-pushed on Jan 21, 2025

DrahtBot removed the label Needs rebase on Jan 21, 2025

DrahtBot added the label Needs rebase on Mar 20, 2025

DrahtBot commented at 10:16 am on March 20, 2025: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

descriptors: MuSig2 #31244

Code Coverage & Benchmarks

Reviews

Conflicts