sign: use fresh randomness as BIP340 auxiliary data #36409

Bitcoin Core signs Taproot inputs with all-zero BIP340 auxiliary data. BIP340 recommends fresh randomness there, as protection against fault injection and side-channel attacks. I made CreateSchnorrSig pass GetRandHash() instead, unless SignOptions::aux_rand sets a fixed value.

In the unit tests, only script_tests/bip341_keypath_test_vectors depended on the zero value: the BIP341 vectors were made with zero auxiliary data. It now builds the signature creator with aux_rand set to zero and compares its signature byte for byte with the vectors, then signs twice with default options and checks that the two signatures differ. If CreateSchnorrSig ignores aux_rand, the vector check fails for all 7 key path inputs. The signing-related functional tests pass unchanged. The script_sign fuzz target can now reach GetRandHash(), so it seeds the RNG like the other signing targets.

I chose GetRandHash() over GetStrongRandBytes(). Both draw from the same RNG, which is seeded with OS entropy at startup, so the output is unpredictable; GetRandHash() skips the OS call GetStrongRandBytes() makes on each draw. BIP340 says any non-repeating value increases protection against fault injection, and that no security property other than side-channel resistance depends on the quality of this randomness. In unit tests GetRandHash() follows the test seed, so the tests stay reproducible; GetStrongRandBytes() does not.

Default signing is no longer deterministic. SignOptions::aux_rand sets a fixed auxiliary value where one is needed, as in the vector test; Yudis-bit suggested it.

Fixes #31883.

Made with my usual tools: a computer, the Internet and an LLM. The mistakes, as usual, are all mine.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

External sites

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

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

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

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #36122 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36122.svg">} (BIP460: CISA for Taproot key path spends by fjahr)
• #32857 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/32857.svg">} (wallet: allow skipping script paths by Sjors)

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.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Tested on be2bc19726.

I think GetRandHash() is definitely the right choice here. For BIP340, nonce generation already binds the secret key with the message via tagged hashes, so auxiliary randomness is purely for fault injection and side-channel mitigation rather than baseline entropy. Skipping the GetStrongRandBytes() OS syscall keeps transaction signing fast, and crucially keeps fuzzing and unit tests deterministic under SeedRandomStateForTest(SeedRand::ZEROS).

Regarding your question about SignOptions: I'd definitely lean towards adding std::optional<uint256> aux_rand there.

Notice what happens in src/test/script_tests.cpp: because MutableTransactionSignatureCreator doesn't accept an aux value, the test has to bypass the creator entirely and call key.SignSchnorr(...) directly. But the whole point of that test case in script_tests.cpp was to test MutableTransactionSignatureCreator's end-to-end handling of the BIP-341 witness generation against the official vectors.

If SignOptions has:

struct SignOptions {
    int sighash_type{SIGHASH_DEFAULT};
    std::optional<uint256> aux_rand{std::nullopt};
};

and in MutableTransactionSignatureCreator::CreateSchnorrSig:

const uint256 aux_rand = m_options.aux_rand.value_or(GetRandHash());
if (!key.SignSchnorr(*hash, sig, merkle_root, aux_rand)) return false;

Then default transaction signing keeps fresh randomness, but script_tests can still test the creator against the exact BIP-341 vector:

MutableTransactionSignatureCreator creator_zero_aux(tx, txinpos, utxos[txinpos].nValue, &txdata, {.sighash_type = hashtype, .aux_rand = uint256{}});
std::vector<unsigned char> zero_aux_sig;
BOOST_CHECK(creator_zero_aux.CreateSchnorrSig(provider, zero_aux_sig, pubkey, nullptr, &merkle_root, SigVersion::TAPROOT));
BOOST_CHECK_EQUAL(HexStr(zero_aux_sig), expected);

Verified this locally with bip341_wallet_vectors.json. All 7 keypath test vector inputs reproduce the expected witness signature directly through the creator, alongside the two differing non-deterministic signatures under default options.

Done in 1f274eae23, as you suggested: SignOptions::aux_rand, defaulting to GetRandHash(). The vector test goes through the creator again with aux_rand = uint256{}, and two default-options signatures are checked to differ. With aux_rand ignored, the vector check fails for all 7 inputs. PR description updated. Thanks.

ACK 1f274eae23

Re-reviewed the commit. Cleanly addresses the test vector coupling by keeping SignOptions::aux_rand optional while defaulting to GetRandHash(). Verified that bip341_keypath_test_vectors reproduces the BIP-341 witness with zero aux and confirms divergence under default options, with script_sign fuzzing seeded deterministically.