wallet: bound pending MuSig2 sessions #36641

fametrano commented at 6:02 AM on October 24, 2026: none

The wallet keeps the secret nonce of every unfinished MuSig2 session in locked memory for as long as the wallet is loaded. A nonce is removed only when its session makes a partial signature.

I keep at most 2048 sessions per descriptor, about 288 KiB, and drop the oldest first. A dropped nonce is never reused. Its session restarts from a PSBT without our pubnonce. A standard transaction has room for about 1700 key-path inputs. Each MuSig2 leaf with our key adds one session per input, so a PSBT with many such leaves, or several concurrent PSBTs, can reach the limit. I tested this with a unit test that starts 4096 sessions. It fails without the change and passes with it.

This is one of several PRs from a large audit of Bitcoin Core that I did with the help of LLM tools. The audit produced its evidence all at once, which is why the PRs arrive together. I understand each change fully and will answer review questions myself.

DrahtBot added the label Wallet on Oct 24, 2026

DrahtBot commented at 6:02 AM on October 24, 2026: contributor

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

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36641
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36641.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36641#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept NACK	vicjuma

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36697 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36697.svg"> (wallet: Add exportaddresskey RPC for descriptor wallets by jaromil)
#36451 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36451.svg"> (refactor: Use NotNull for CreateNewBlock and pimpls by maflcko)
#35444 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35444.svg"> (wallet: make descriptor SPKM mutex non-recursive by w0xlt)

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 CI failed on Oct 24, 2026

wallet: bound pending MuSig2 sessions

The wallet keeps the secret nonce of every unfinished MuSig2 session in
locked memory for as long as the wallet is loaded. A nonce is removed
only when its session makes a partial signature.

I keep at most 2048 sessions per descriptor, about 288 KiB, and drop the
oldest first. A dropped nonce is never reused. Its session restarts from
a PSBT without our pubnonce. A standard transaction has room for about
1700 key-path inputs. Each MuSig2 leaf with our key adds one session per
input, so a PSBT with many such leaves, or several concurrent PSBTs, can
reach the limit. I tested this with a unit test that starts 4096
sessions. It fails without the change and passes with it.

5c6a090b9d

DrahtBot removed the label CI failed on Oct 24, 2026

in src/musig.h:65 in 5c6a090b9d

  61 | @@ -57,6 +62,34 @@ class MuSig2SecNonce
  62 |      bool IsValid();
  63 |  };
  64 |  
  65 | +/**

vicjuma commented at 11:52 PM on October 26, 2026:

src/script/sign.cpp:192: provider.DeleteMuSig2Session(session_id); currently lgtm tbh. I mean, on top of the current secnonces being memory-based, it is also deleted after it's done. Unless there is a practical scenario warranting this, I do not see the need

in src/musig.cpp:128 in 5c6a090b9d

 124 | @@ -122,6 +125,30 @@ bool MuSig2SecNonce::IsValid()
 125 |      return m_impl->IsValid();
 126 |  }
 127 |  
 128 | +bool MuSig2SecNonces::Add(const uint256& session_id, MuSig2SecNonce&& nonce)

vicjuma commented at 11:56 PM on October 26, 2026:

Any justifications for adding these?

in src/wallet/test/scriptpubkeyman_tests.cpp:79 in 5c6a090b9d

  74 | +    BOOST_REQUIRE(other_provider != nullptr);
  75 | +    BOOST_CHECK(!other_provider->GetMuSig2SecNonce(session_ids.front()).has_value());
  76 | +    BOOST_CHECK(other_provider->GetMuSig2SecNonce(session_ids.back()).has_value());
  77 | +    size_t kept{0};
  78 | +    for (const uint256& id : session_ids) kept += other_provider->GetMuSig2SecNonce(id).has_value();
  79 | +    BOOST_CHECK_LT(kept, size_t{num_sessions});

vicjuma commented at 12:03 AM on October 27, 2026:

Even 1 will pass here, which is not the intention of your PR

vicjuma commented at 12:10 AM on October 27, 2026: contributor

Concept NACK I do not see the practical implications of what you are trying to solve. If there is any justification, point it out.