migrate: Handle HD chains that have identical seeds but different IDs #35437

achow101 commented at 9:43 PM on June 1, 2026: member

The seed ID is calculated from a pubkey produced by treating the seed as a private key. This calculation includes a pubkey compression parameter, even thought that compression is completely irrelevant for the usage of the seed as a BIP 32 seed. Thus migration should detect if a seed has been used multiple times by checking if the computed master key was already processed.

The spkm_migration fuzzer needs to have it's added descriptors accounting updated for this fix.

It should not be possible for users to actually run into this problem as all HD chains use seeds with the pubkey compression option set.

Fixes #35434

DrahtBot commented at 9:43 PM on June 1, 2026: 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/35437.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	marcofleon, kevkevinpal, rkrux

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

DrahtBot added the label CI failed on Jun 1, 2026

DrahtBot commented at 11:10 PM on June 1, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task macOS native, fuzz: https://github.com/bitcoin/bitcoin/actions/runs/26783841124/job/78954750669</sub> <sub>LLM reason (✨ experimental): Fuzz test failure: spkm_migration crashed on an assertion in scriptpubkeyman.cpp (MigrateToDescriptor, line 636), exiting with code 1.</sub>

<details><summary>Hints</summary>

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.

</details>

maflcko added this to the milestone 32.0 on Jun 2, 2026

achow101 force-pushed on Jun 2, 2026

migrate: Handle HD chains that have identical seeds but different IDs

The seed ID is calculated from a pubkey produced by treating the seed as
a private key. This calculation includes a pubkey compression parameter,
even thought that compression is completely irrelevant for the usage of
the seed as a BIP 32 seed. Thus migration should detect if a seed has
been used multiple times by checking if the computed master key was
already processed.

The spkm_migration fuzzer needs to have it's added descriptors
accounting to be updated for this fix.

de92208c2b

achow101 force-pushed on Jun 3, 2026

DrahtBot removed the label CI failed on Jun 3, 2026

kevkevinpal commented at 8:38 PM on June 3, 2026: contributor

I was able to recreate the fuzz failure on master and also test that it passed on de92208c2b508b40fa690624d026c775ed876606. Here are my steps in reproducing the error and checking this fix.

Checking that it fails on master

# Add input
echo 'IQIAJ1sAo6MYAcBQGP+VdP////////83v9sAAAAAAAAAAAAAAP8VAAAAAAAAAUUAAAAAWwD+////////CwAAAAAAAAB///8AAAAAAAAAAAAAAP8VAAAAAAAAAUUAAAAAAAD/k5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5MhAgAnWwCjoxgDwFAY/5V0/////////z+/2wAAAAABAAAAAAAA/xUAAAAAAAABRQAAAAAAAP////////8LAAAAAAAA/////wAA/P8AAAAAAAAA/xUAAAAAAAABRQAAAAAAAP+Tk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTkyECACdbAKOjGAPqUBhRa3Q+Pj4+Pj4+Pj4+Pj4+Pj4+Pj48Pj4+Pj4+wsHBuT4+Pj4+Pj4+Pj4+Pj4+Pj4+Pj4+Pj4+Pj4+/////////zm/2wAAAAAAAAAAhqurq6urAAD/FQAAXAAAANoAAAAAAAAAAAAAAPgA0wAAAAAAAAAA/v/DX+Wel3+Tk5OTk5OTk5OTk5OTk5OTk0aB6YU2k5OTk5OTk5OTk5MhAgAAAAAAAP+Tk5OTk5OTk5OTk5OTk5OTk5OTopOTk5OTk5OTkyECACdbAKOjGAPAUBj/lXT/////////P7/bAAAAAAAAAAAAAAAAAABkAAAAAAFFAAAAAAAA/////////wsAAAAAAAD/////AAD8/wAAAAAAAAD/FQAAAAAAAAFFAAAAAAAA/5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTIQIAJ1sAo6MYA+pQGFFrdEA+Pj4+Pj4+Pj4+Pj4+Pj4+Pjw+Pj4+Pj7CwcG5Pj4+Pj4+Pv83v9sAAAAAAAAAAAAAAP8VAAAAAAAAAUUAAAAAWwD+/////v//CwAAAAAAAAB///8AAAAAAAAAAAAAAP8VAAAAAAAAAUUAAAAAAAD/k5OTk5OTk5OTk5OTk5N6k5OTk5OTk5OTk5OTk5MhAgAnWwCjoxgDwFAY/5V0/////////z+/2wAAAAAAAAAAAAAA/xUAAAAAAAABRQAAAAAAAP////////8LAAAAAAAA/////wAA/P8AAAAAAAAA/xUAAAAAAAABRQAAAAAAAP+Tk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTk5OTkyECACdbAKOjGAPqUBhRa3Q+Pj4+Pj4+Pj4+Pj4+Pj4+Pj48Pj4+Pj4+wsHBuT4+Pj4+Pj4+Pj4+Pj4+Pj4+Pj4+Pj4+/////////zm/2wAAAAAAAAAAhqurq6urAAD/Fd7/WwAAANoAAAAAAAAAAAAAAPgA0wAAAAAAAAAA/v/YX+Wel3+Tk5MAAP+Y/6Ghnpd/k5OTk5OTkwAAAKE='   | base64 -d > spkmmigrate_input

# Checkout master
git checkout master
cmake --preset=libfuzzer && cmake --build build_fuzz --parallel

# Run failing input
FUZZ=spkm_migration build_fuzz/bin/fuzz -runs=1 spkmmigrate_input

Checking that it passes on de92208c2b508b40fa690624d026c775ed876606

# Checkout this PR
git fetch origin pull/35437/head:PR35437
git checkout PR35437

# Build this PR
cmake --preset=libfuzzer && cmake --build build_fuzz --parallel

# Run failing input and observe that it works
FUZZ=spkm_migration build_fuzz/bin/fuzz -runs=1 spkmmigrate_input

marcofleon commented at 11:24 AM on June 4, 2026: contributor

crACK de92208c2b508b40fa690624d026c775ed876606

Confirmed that this passes on the previously crashing input.

kevkevinpal commented at 2:58 PM on June 5, 2026: contributor

It looks like this PR also has changes in the fuzzer itself. I was able to confirm with just the changes to src/wallet/scriptpubkeyman.cpp the crashing input passes

kevkevinpal commented at 2:59 PM on June 5, 2026: contributor

crACK de92208

fanquake requested review from rkrux on Jun 12, 2026

fanquake commented at 9:56 AM on June 17, 2026: member

cc @w0xlt @polespinasa

rkrux approved

rkrux commented at 2:01 PM on June 18, 2026: contributor

code review ACK de92208

fanquake merged this on Jun 18, 2026

fanquake closed this on Jun 18, 2026