rkrux
commented at 1:00 PM on March 24, 2026:
contributor
I've been meaning to scratch this itch for quite some time. It has been noted
by many developers that the wallet/wallet.cpp file is quite hard to reason
about.
It is more than 4500 lines long currently and this patch is an attempt to
shortern it by around 780 lines by extracting out the wallet migration code into
a dedicated wallet/migration.cpp file.
The other benefits of this PR I see are that it moves the legacy wallet stuff
from the main wallet file that paves the way for it being descriptor specific
(as much as possible), and makes legacy wallet migration code a first-class
citizen in its own file.
I hope it makes it easier for everyone to go through the main wallet file.
This patchset should be reviewed with --color-moved=dimmed-zebra option.
DrahtBot
commented at 1:01 PM on March 24, 2026:
contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
#25665 (refactor: Add util::Result failure types and ability to merge result values by ryanofsky)
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-->
LLM Linter (✨ experimental)
Possible typos and grammar issues:
a multipath descriptors -> a multipath descriptor [plural form is incorrect here]
Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):
AddWalletDescriptor(w_desc, keys, "", false) in src/wallet/migration.cpp
WalletDescriptor(std::move(descs.at(0)), creation_time, 0, 0, 0) in src/wallet/migration.cpp
<sup>2026-05-29 11:16:07</sup>
rkrux force-pushed on Mar 24, 2026
DrahtBot added the label CI failed on Mar 24, 2026
rkrux force-pushed on Mar 24, 2026
rkrux
commented at 1:12 PM on March 24, 2026:
contributor
Fixed two things:
the linter issue where the trailing new line at the end of the file was missing.
forgot to update the migration portion in bench for which I didn't get an error in local, checking why.
DrahtBot removed the label CI failed on Mar 24, 2026
rkrux force-pushed on Mar 25, 2026
rkrux renamed this: wallet, refactor: extract migration portion from the wallet file wallet, refactor: modularise wallet by extracting out legacy wallet migration on Mar 25, 2026
DrahtBot renamed this: wallet, refactor: modularise wallet by extracting out legacy wallet migration wallet, refactor: modularise wallet by extracting out legacy wallet migration on Mar 25, 2026
rkrux
commented at 11:24 AM on March 25, 2026:
contributor
Pushed an update where three more migration specific CWallet methods are extracted out.
rkrux force-pushed on Mar 25, 2026
DrahtBot added the label CI failed on Mar 25, 2026
DrahtBot removed the label CI failed on Mar 25, 2026
DrahtBot added the label Needs rebase on Apr 19, 2026
rkrux force-pushed on Apr 22, 2026
rkrux
commented at 2:21 PM on April 22, 2026:
contributor
DrahtBot added the label CI failed on Apr 22, 2026
DrahtBot
commented at 2:47 PM on April 22, 2026:
contributor
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed.
<sub>Task Windows native, fuzz, VS: https://github.com/bitcoin/bitcoin/actions/runs/24783607828/job/72521322210</sub>
<sub>LLM reason (✨ experimental): CI failed because the spkm_migration fuzz target hit a wallet database format assertion (m_database->Format() != "bdb_ro" && != "mock") in wallet::CWallet::SetupLegacyScriptPubKeyMan and exited 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>
rkrux
commented at 2:48 PM on April 22, 2026:
contributor
Force pushed to fix the CI failures because of incorrectly resolving conflicts during rebase due to the changes in this file of this PR.
DrahtBot removed the label Needs rebase on Apr 23, 2026
DrahtBot removed the label CI failed on Apr 23, 2026
rkrux force-pushed on Apr 23, 2026
rkrux
commented at 9:43 AM on April 23, 2026:
contributor
Split the diff into multiple commits to aid review from 7b87334 to e9f50db.
rkrux force-pushed on Apr 23, 2026
DrahtBot added the label CI failed on Apr 23, 2026
DrahtBot
commented at 9:52 AM on April 23, 2026:
contributor
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>
rkrux
commented at 9:52 AM on April 23, 2026:
contributor
Fixed lint failure due to missing new line in wallet/migration.h.
DrahtBot removed the label CI failed on Apr 23, 2026
DrahtBot added the label Needs rebase on May 2, 2026
rkrux force-pushed on May 5, 2026
rkrux
commented at 9:24 AM on May 5, 2026:
contributor
Rebased over master to incorporate changes from #34544.
DrahtBot removed the label Needs rebase on May 5, 2026
DrahtBot added the label Needs rebase on May 22, 2026
rkrux force-pushed on May 22, 2026
rkrux
commented at 9:50 AM on May 22, 2026:
contributor
Rebased over master to incorporate changes from #34806.
DrahtBot added the label CI failed on May 22, 2026
fanquake
commented at 10:34 AM on May 22, 2026:
member
Build FAILED.
"D:\a\bitcoin\bitcoin\build\ALL_BUILD.vcxproj" (default target) (1) ->
"D:\a\bitcoin\bitcoin\build\src\bench\bench_bitcoin.vcxproj" (default target) (4) ->
"D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj" (default target) (12) ->
(ClCompile target) ->
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(78,14): error C2027: use of undefined type 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(78,39): error C2027: use of undefined type 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(79,34): error C2027: use of undefined type 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(79,26): error C2530: 'value': references must be initialized [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(79,26): error C3531: 'value': a symbol whose type contains 'auto' must have an initializer [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(79,32): error C2143: syntax error: missing ';' before ':' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(79,59): error C2143: syntax error: missing ';' before ')' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(82,9): error C2027: use of undefined type 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(91,14): error C2027: use of undefined type 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(92,64): error C2027: use of undefined type 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(92,31): error C2079: 'new_value' uses undefined class 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(92,64): error C2065: 'VARR': undeclared identifier [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(93,34): error C2027: use of undefined type 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(93,26): error C2530: 'value': references must be initialized [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(93,26): error C3531: 'value': a symbol whose type contains 'auto' must have an initializer [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(93,32): error C2143: syntax error: missing ';' before ':' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(93,59): error C2143: syntax error: missing ';' before ')' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(96,33): error C2027: use of undefined type 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
D:\a\bitcoin\bitcoin\src\wallet\walletutil.cpp(97,23): error C2582: 'operator =' function is unavailable in 'UniValue' [D:\a\bitcoin\bitcoin\build\src\wallet\bitcoin_wallet.vcxproj]
DrahtBot removed the label Needs rebase on May 22, 2026
rkrux force-pushed on May 22, 2026
DrahtBot removed the label CI failed on May 22, 2026
DrahtBot added the label Needs rebase on May 28, 2026
wallet: (move-only) extract out migration specific CWallet methods
From wallet/wallet.cpp to wallet/migration.cpp.
This patch can be reviewed with --color-moved=dimmed-zebra option.
4cc85da6eb
wallet: extract out utility functions from wallet/wallet
To wallet/walletutil.h
Couple of functions had to be made non-static for them to be used by
wallet/wallet.
This patch can be reviewed with --color-moved=dimmed-zebra option.
cece453e47
wallet: extract out public facing MigrateLegacyToDescriptor functions
To wallet/migration. Also, update the includes in the call sites.
This patch can be reviewed with --color-moved=dimmed-zebra.
76ab315d5b
wallet: extract out migration specific LegacySPKM CWallet methods
To wallet/migration.
This patch can be reviewed with --color-moved=dimmed-zebra.
5447bdf536
rkrux force-pushed on May 29, 2026
rkrux
commented at 11:15 AM on May 29, 2026:
contributor
Rebased over master to incorporate changes from #28333.
DrahtBot removed the label Needs rebase on May 29, 2026
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2026-06-04 10:51 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me