key: validate BIP32 seed length in CExtKey::SetSeed #35320

BIP32 specifies that the seed must be between 128 and 512 bits (16 to 64 bytes). CExtKey::SetSeed currently accepts any length, which could result in weak master keys being generated. Add Assert at the start of SetSeed that terminates if seed is outside valid range. Add unit tests to validate the conditions: 16 bytes (minimum valid), and 64 bytes (maximum valid).

To test:

cmake --build build -j --target test_bitcoin ./build/bin/test_bitcoin --run_test=bip32_tests/extkey_setseed_bip32_length

Fixes #35308

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

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

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

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

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Two things:

1. I believe return is the wrong failure handling. Silently returning leaves the CExtKey in an uninitialized/partial state with no way for the caller to detect the rejection. Since passing an out-of-spec seed is a programming error with no safe recovery, Assert(seed.size() >= 16 && seed.size() <= 64) is more appropriate because it makes the contract explicit and terminates loudly on violation rather than silently producing a broken object.

2. The tests belongs in src/test/bip32_tests.cpp. That file already exercises CExtKey::SetSeed directly and is the natural home for BIP32 seed validation tests.

Two things:

1. I believe return is the wrong failure handling. Silently returning leaves the CExtKey in an uninitialized/partial state with no way for the caller to detect the rejection. Since passing an out-of-spec seed is a programming error with no safe recovery, Assert(seed.size() >= 16 && seed.size() <= 64) is more appropriate because it makes the contract explicit and terminates loudly on violation rather than silently producing a broken object.
2. The tests belongs in src/test/bip32_tests.cpp. That file already exercises CExtKey::SetSeed directly and is the natural home for BIP32 seed validation tests.

Yeah, very much agree!.

Checked it out - I'll move the tests.

1. nit: I believe that swapping the ordering would improve readability, Assert(16 <= seed.size() && seed.size() <= 64)

2. The commit message needs to be updated as well to match the proposed functionality, it still mentions the early return.

3. The tests are lacking, especially since they do not test arguably the most problematic part which are short seeds. The BOOST_CHECK_THROW (or its variants, I am not familiar with Boost) could be used to test too short and too long seeds.