Remove the legacy wallet and BDB dependency by achow101 · Pull Request #28710 · bitcoin/bitcoin

achow101 · 2023-10-23T23:36:07Z

The final step of #20160.

A bare minimum of legacy wallet code is kept in order to perform wallet migration. Migration of legacy wallets uses the independent BDB parser and a minimal LegacyDataSPKM that allows the legacy data to be loaded so that the migration can be completed.

BDB has been removed as a dependency and documentation have been updated to reflect that.

DrahtBot · 2023-10-23T23:36:10Z

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/28710.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	w0xlt, Sjors, maflcko
Concept ACK	rkrux

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)
#32238 (qt, wallet: Convert uint256 to Txid by marcofleon)
#32189 (refactor: Txid type safety (parent PR) by marcofleon)
#31989 (BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only) by jamesob)
#31895 (doc: Improve dependencies.md by NicolaLS)
#31802 (Add bitcoin-{node,gui} to release binaries for IPC by Sjors)
#31679 (cmake: Move internal binaries from bin/ to libexec/ by ryanofsky)
#31665 (build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON by davidgumberg)
#31650 (refactor: Avoid copies by using const references or by move-construction by maflcko)
#31622 (psbt: add non-default sighash types to PSBTs and unify sighash type match checking by achow101)
#31453 (util: detect and warn when using exFAT on MacOS by willcl-ark)
#31404 (descriptors: inference process, do not return unparsable multisig descriptors by furszy)
#31375 (multiprocess: Add bitcoin wrapper executable by ryanofsky)
#31360 (depends: Avoid using helper variables in toolchain file by hebasto)
#31296 (wallet: Translate [default wallet] string in progress messages by ryanofsky)
#31275 (doc: corrected lockunspent rpc quoting by Talej)
#30595 (kernel: Introduce initial C header API by TheCharlatan)
#30437 (multiprocess: add bitcoin-mine test program by ryanofsky)
#30221 (wallet: Ensure best block matches wallet scan state by achow101)
#29694 (fuzz: wallet: add target for spkm migration by brunoerg)
#29675 (wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys by achow101)
#29652 (wallet: Avoid potentially writing incorrect best block locator by ryanofsky)
#29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)
#29136 (wallet: addhdkey RPC to add just keys to wallets via new void(KEY) descriptor by achow101)
#28333 (wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively by achow101)
#27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
#27286 (wallet: Keep track of the wallet's own transaction outputs in memory by achow101)
#21283 (Implement BIP 370 PSBTv2 by achow101)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

by an rescanblockchain -> by a rescanblockchain [Incorrect article "an" used before a word starting with a consonant sound]

w0xlt

ACK be3c13a with 2 non-blocking nits commented earlier.

Regarding the removed RPC functions, I think less is better in this case, and unless I'm missing something, the cases mentioned in #30175 can be performed by importdescriptors.

Deletes LegacyScriptPubKeyMan and related tests Best reviewed with `git diff --patience` or `git diff --histogram`

SOme db functions were for BDB, these are no longer needed.

These RPCs no longer exist.

w0xlt

reACK de054df

Sjors · 2025-05-07T08:31:59Z

re-ACK de054df

rkrux

Concept ACK de054df

Couple of points in 8ede6de, can be done in follow up if no more changes being done in this PR.

rkrux · 2025-05-07T11:34:49Z

-                "\nStops current wallet rescan triggered by an RPC call, e.g. by an importprivkey call.\n"
+                "\nStops current wallet rescan triggered by an RPC call, e.g. by an rescanblockchain call.\n"
                "Note: Use \"getwalletinfo\" to query the scanning progress.\n",
                {},
                RPCResult{RPCResult::Type::BOOL, "", "Whether the abort was successful"},
                RPCExamples{
            "\nImport a private key\n"
-            + HelpExampleCli("importprivkey", "\"mykey\"") +
+            + HelpExampleCli("rescanblockchain", "") +


Besides the typo s/an/a above, the comment "Import a private key" doesn't go well with rescanblockchain.

rkrux · 2025-05-07T11:55:23Z

-    }
-
-    // Only single key descriptors are allowed to be imported to a legacy wallet's keypool
-    bool can_keypool = parsed_descs.at(0)->IsSingleKey();


In #31243, IsSingleKey was added with a TODO to remove it after removing legacy wallets.
I checked that no other usage of IsSingleKey is left, it can be removed.

bitcoin/src/script/descriptor.h

Lines 114 to 117 in 6d5edfc

/** Whether this descriptor only produces single key scripts (i.e. pk(), pkh(), wpkh(), sh() and wsh() nested of those, and combo())

* TODO: Remove this method once legacy wallets are removed as it is only necessary for importmulti.

*/

virtual bool IsSingleKey() const = 0;

maflcko · 2025-05-07T12:17:21Z

re-ACK de054df 🔗

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK de054df6dc32cbd8b127c6761d9c65d95025e08 🔗
BCpwbQ/aUjnaK6cKbU2NA6V6EpINW1XAGCEG7C6Kp8TVtlZz/xsxtLcBwdx6KWznzxX0v4qc0hC17WfOYE41CA==

pablomartin4btc

post-merge ACK de054df

maflcko · 2025-05-07T15:29:44Z

follow-up in #32438

achow101 force-pushed the rm-legacy-wallet branch from d642e26 to ad3080c Compare October 23, 2023 23:42

DrahtBot added the CI failed label Oct 23, 2023

achow101 mentioned this pull request Oct 24, 2023

Proposed Timeline for Legacy Wallet and BDB removal #20160

Closed

48 tasks

achow101 force-pushed the rm-legacy-wallet branch from ad3080c to fcc606a Compare October 24, 2023 02:17

w0xlt reviewed May 6, 2025

View reviewed changes

Comment thread src/wallet/wallettool.cpp Outdated

brunoerg mentioned this pull request May 6, 2025

docs: Improve keypoolrefill RPC docs #32429

Merged

w0xlt reviewed May 6, 2025

View reviewed changes

achow101 added 4 commits May 6, 2025 16:53

wallet: Delete LegacySPKM

83af1a3

Deletes LegacyScriptPubKeyMan and related tests Best reviewed with `git diff --patience` or `git diff --histogram`

wallet: Remove unused db functions

c0f3f32

SOme db functions were for BDB, these are no longer needed.

legacy spkm: Make IsMine() and CanProvide() private and migration only

5dff04a

contrib: Remove legacy wallet RPCs from bash completions

de054df

These RPCs no longer exist.

w0xlt reviewed May 7, 2025

View reviewed changes

rkrux reviewed May 7, 2025

View reviewed changes

pablomartin4btc reviewed May 7, 2025

View reviewed changes

kevkevinpal mentioned this pull request May 7, 2025

test: remove bdb assert in tool_wallet.py #32440

Merged

This was referenced May 7, 2025

Maio 2025 ClubeBitcoinUnB/bitdevs.bsb.br#20

Closed

Maio/2025 poabitdevs/poabitdevs.org#7

Closed

Topicos Maio/25 saopaulobitdevs/saopaulobitdevs.org#49

Closed

fanquake mentioned this pull request May 8, 2025

contrib: remove bdb exception from FORTIFY check #32448

Merged

bucko13 mentioned this pull request May 8, 2025

May 2025 austinbitdevs/austinbitdevs.com#52

Closed

pinheadmz mentioned this pull request May 15, 2025

TSan warning with legacy wallet on macos ("too long mutex cycle found") #31986

Closed

Talkless mentioned this pull request May 18, 2025

Will Bitcoin Core's #31250 end EPS use case (in the current implementation)? chris-belcher/electrum-personal-server#309

Open

hebasto mentioned this pull request May 19, 2025

Delete BDB everywhere hebasto/bitcoin-core-nightly#53

Merged

vicjuma mentioned this pull request May 20, 2025

refactor: remove dump.h/cpp, wallet_bdb_parser.cpp #32569

Closed

achow101 mentioned this pull request May 26, 2025

wallet: Fix wallet interface detection of encrypted wallets #32620

Merged

nickfarrow mentioned this pull request May 29, 2025

May 2025 Meeting Topics MelbourneBitDevs/meetup#39

Closed

kevkevinpal mentioned this pull request Jun 20, 2025

followup: Remove the legacy wallet and BDB dependency kevkevinpal/bitcoin#37

Closed

fanquake mentioned this pull request Sep 19, 2025

Blog: minimizing dependencies bitcoinbrink/website#158

Merged

nahuhh mentioned this pull request Jan 3, 2026

bitcoin: bump to 29.2 basicswap/basicswap#413

Merged

jamescowens mentioned this pull request Mar 1, 2026

Replace Berkeley DB database backend gridcoin-community/Gridcoin-Research#2629

Open

thepastaclaw mentioned this pull request Apr 23, 2026

test: RPC coverage linter + related fixes for listaddressbalances dashpay/dash#7279

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove the legacy wallet and BDB dependency#28710

Remove the legacy wallet and BDB dependency#28710
fanquake merged 9 commits into
bitcoin:masterfrom
achow101:rm-legacy-wallet

achow101 commented Oct 23, 2023 •

edited

Loading

Uh oh!

DrahtBot commented Oct 23, 2023 •

edited

Loading

Uh oh!

Uh oh!

w0xlt left a comment •

edited

Loading

Uh oh!

w0xlt left a comment

Uh oh!

Sjors commented May 7, 2025

Uh oh!

rkrux left a comment •

edited

Loading

Uh oh!

rkrux May 7, 2025

Uh oh!

rkrux May 7, 2025

Uh oh!

maflcko commented May 7, 2025

Uh oh!

pablomartin4btc left a comment

Uh oh!

maflcko commented May 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

	/** Whether this descriptor only produces single key scripts (i.e. pk(), pkh(), wpkh(), sh() and wsh() nested of those, and combo())
	* TODO: Remove this method once legacy wallets are removed as it is only necessary for importmulti.
	*/
	virtual bool IsSingleKey() const = 0;

Conversation

achow101 commented Oct 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Oct 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

Uh oh!

w0xlt left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

w0xlt left a comment

Choose a reason for hiding this comment

Uh oh!

Sjors commented May 7, 2025

Uh oh!

rkrux left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rkrux May 7, 2025

Choose a reason for hiding this comment

Uh oh!

rkrux May 7, 2025

Choose a reason for hiding this comment

Uh oh!

maflcko commented May 7, 2025

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko commented May 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

achow101 commented Oct 23, 2023 •

edited

Loading

DrahtBot commented Oct 23, 2023 •

edited

Loading

w0xlt left a comment •

edited

Loading

rkrux left a comment •

edited

Loading