Remove the legacy wallet and BDB dependency

achow101 commented at 11:36 pm on October 23, 2023: member

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.

All tests which tested legacy wallet behavior have been removed. The --descriptors and --legacy-wallet options are removed from the functional tests.

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

Depends on #26596, https://github.com/bitcoin-core/gui/pull/824, #30265, #30328, and #31250

DrahtBot commented at 11:36 pm on October 23, 2023: 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/28710.

Reviews

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31282 (refactor: Make node_id a const& in RemoveBlockRequest by maflcko)
#31278 (wallet, rpc: Settxfeerate by polespinasa)
#31275 (doc: corrected lockunspent rpc quoting by Talej)
#31250 (wallet: Disable creating and loading legacy wallets by achow101)
#31248 (test: Rework wallet_migration.py to use previous releases by achow101)
#31244 (descriptors: MuSig2 by achow101)
#31243 (descriptor: Move filling of keys from DescriptorImpl::MakeScripts to PubkeyProvider::GetPubKey by achow101)
#31241 (wallet: remove BDB dependency from wallet migration benchmark by furszy)
#31221 (ci: Split out native fuzz jobs for macOS and windows (take 2) by dergoegge)
#31158 (build, ci: Fix linking bitcoin-chainstate.exe to bitcoinkernel.dll on Windows by hebasto)
#31072 (refactor: Clean up messy strformat and bilingual_str usages by ryanofsky)
#31061 (refactor: Check translatable format strings at compile-time by maflcko)
#30997 (build: Switch to Qt 6 by hebasto)
#30893 (test: Introduce ensure_for helper by fjahr)
#30860 (test: autogenerate bash completion by BrandonOdiwuor)
#30844 (RPC: improve SFFO arg parsing, error catching and coverage by furszy)
#30727 (rpc: add address_type field in getaddressinfo by jonatack)
#30437 (multiprocess: add bitcoin-mine test program by ryanofsky)
#30328 (wallet: Remove IsMine from migration code by achow101)
#30221 (wallet: Ensure best block matches wallet scan state by achow101)
#30125 (test: improve BDB parser (handle internal/overflow pages, support all page sizes) by theStack)
#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)
#29500 (test: create assert_not_equal util by kevkevinpal)
#29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
#29156 (tests: add functional test for miniscript decaying multisig by mjdietzx)
#29136 (wallet: addhdkey RPC to add just keys to wallets via new void(KEY) descriptor by achow101)
#29124 (wallet: Automatically repair corrupted metadata with doubled derivation path by achow101)
#28944 (wallet, rpc: add anti-fee-sniping to send and sendall by ishaanam)
#28802 (ArgsManager: support subcommand-specific options by ajtowns)
#28724 (wallet: Cleanup accidental encryption keys in watchonly wallets 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)
#27052 (test: rpc: add last block announcement time to getpeerinfo result by LarryRuane)
#25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
#21283 (Implement BIP 370 PSBTv2 by achow101)
#19461 (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
#19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
#10102 (Multiprocess bitcoin 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.

achow101 force-pushed on Oct 23, 2023

DrahtBot added the label CI failed on Oct 23, 2023

achow101 force-pushed on Oct 24, 2023

DrahtBot added the label Needs rebase on Oct 25, 2023

achow101 force-pushed on Oct 25, 2023

DrahtBot removed the label Needs rebase on Oct 25, 2023

DrahtBot added the label Needs rebase on Nov 6, 2023

achow101 force-pushed on Nov 13, 2023

DrahtBot removed the label Needs rebase on Nov 13, 2023

DrahtBot added the label Needs rebase on Nov 16, 2023

achow101 force-pushed on Nov 16, 2023

DrahtBot removed the label Needs rebase on Nov 16, 2023

DrahtBot added the label Needs rebase on Nov 22, 2023

achow101 force-pushed on Nov 28, 2023

DrahtBot removed the label Needs rebase on Nov 28, 2023

DrahtBot added the label Needs rebase on Nov 28, 2023

achow101 force-pushed on Nov 28, 2023

DrahtBot removed the label Needs rebase on Nov 28, 2023

DrahtBot added the label Needs rebase on Nov 30, 2023

achow101 force-pushed on Dec 11, 2023

DrahtBot removed the label Needs rebase on Dec 11, 2023

achow101 force-pushed on Dec 11, 2023

DrahtBot added the label Needs rebase on Dec 13, 2023

achow101 force-pushed on Dec 19, 2023

DrahtBot removed the label Needs rebase on Dec 19, 2023

achow101 force-pushed on Dec 19, 2023

DrahtBot added the label Needs rebase on Jan 2, 2024

fanquake referenced this in commit 04978c2e18 on Jan 5, 2024

achow101 force-pushed on Jan 6, 2024

DrahtBot removed the label Needs rebase on Jan 6, 2024

DrahtBot added the label Needs rebase on Jan 11, 2024

achow101 force-pushed on Jan 11, 2024

DrahtBot removed the label Needs rebase on Jan 11, 2024

DrahtBot added the label Needs rebase on Jan 16, 2024

achow101 force-pushed on Jan 16, 2024

DrahtBot removed the label Needs rebase on Jan 16, 2024

DrahtBot added the label Needs rebase on Jan 17, 2024

achow101 force-pushed on Jan 25, 2024

achow101 force-pushed on Feb 1, 2024

DrahtBot removed the label Needs rebase on Feb 1, 2024

ryanofsky referenced this in commit 93e10cab5d on Feb 2, 2024

DrahtBot added the label Needs rebase on Feb 3, 2024

achow101 force-pushed on Feb 3, 2024

DrahtBot removed the label Needs rebase on Feb 3, 2024

DrahtBot added the label Needs rebase on Feb 6, 2024

achow101 force-pushed on Feb 8, 2024

DrahtBot removed the label Needs rebase on Feb 8, 2024

DrahtBot added the label Needs rebase on Feb 10, 2024

achow101 force-pushed on Feb 20, 2024

DrahtBot removed the label Needs rebase on Feb 20, 2024

DrahtBot added the label Needs rebase on Feb 26, 2024

achow101 force-pushed on Feb 29, 2024

DrahtBot removed the label Needs rebase on Feb 29, 2024

jess2505 approved

DrahtBot added the label Needs rebase on Mar 11, 2024

achow101 force-pushed on Mar 11, 2024

DrahtBot removed the label Needs rebase on Mar 11, 2024

DrahtBot added the label Needs rebase on Mar 12, 2024

achow101 force-pushed on Mar 12, 2024

DrahtBot removed the label Needs rebase on Mar 12, 2024

DrahtBot added the label Needs rebase on Mar 18, 2024

achow101 force-pushed on Mar 29, 2024

DrahtBot removed the label Needs rebase on Mar 29, 2024

DrahtBot added the label Needs rebase on Apr 1, 2024

achow101 force-pushed on Apr 1, 2024

DrahtBot removed the label Needs rebase on Apr 1, 2024

achow101 force-pushed on Apr 3, 2024

DrahtBot added the label Needs rebase on Apr 8, 2024

laanwj requested review from laanwj on Apr 9, 2024

in src/wallet/migrate.cpp:46 in 7707db3ad5 outdated

41+enum class RecordType : uint8_t
42+{
43+    KEYDATA = 1,
44+    DUPLICATE = 2,
45+    OVERFLOW_DATA = 3,
46+    DELETE = 0x80, // Indicate this record is deleted. This is AND'd with the real type.

laanwj commented at 10:26 am on April 9, 2024:

OR’ed, i guess?

achow101 force-pushed on Apr 25, 2024

achow101 force-pushed on Apr 26, 2024

achow101 force-pushed on May 21, 2024

DrahtBot removed the label Needs rebase on May 21, 2024

DrahtBot added the label Needs rebase on May 22, 2024

achow101 force-pushed on May 22, 2024

DrahtBot removed the label Needs rebase on May 22, 2024

DrahtBot added the label Needs rebase on May 23, 2024

achow101 force-pushed on May 29, 2024

achow101 force-pushed on Jun 4, 2024

DrahtBot removed the label Needs rebase on Jun 5, 2024

DrahtBot added the label Needs rebase on Jun 5, 2024

achow101 force-pushed on Jun 6, 2024

DrahtBot removed the label Needs rebase on Jun 7, 2024

achow101 force-pushed on Jun 7, 2024

achow101 force-pushed on Jun 10, 2024

DrahtBot removed the label CI failed on Jun 11, 2024

DrahtBot added the label Needs rebase on Jun 11, 2024

achow101 force-pushed on Jun 11, 2024

DrahtBot removed the label Needs rebase on Jun 11, 2024

DrahtBot added the label Needs rebase on Jun 12, 2024

achow101 force-pushed on Jun 13, 2024

DrahtBot removed the label Needs rebase on Jun 13, 2024

DrahtBot added the label CI failed on Jun 14, 2024

DrahtBot commented at 0:09 am on June 14, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is 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.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/26201268639

achow101 force-pushed on Jun 14, 2024

DrahtBot removed the label CI failed on Jun 14, 2024

DrahtBot added the label Needs rebase on Jun 17, 2024

Sjors commented at 4:10 pm on June 27, 2024: member

You can also drop the BerkeleyDatabaseSanityCheck related suppression in contrib/devtools/check-devs.sh

achow101 force-pushed on Jun 27, 2024

DrahtBot removed the label Needs rebase on Jun 27, 2024

DrahtBot added the label CI failed on Jun 27, 2024

DrahtBot commented at 8:59 pm on June 27, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is 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.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/26774987403

achow101 force-pushed on Jul 1, 2024

DrahtBot removed the label CI failed on Jul 1, 2024

DrahtBot added the label Needs rebase on Jul 3, 2024

achow101 force-pushed on Jul 11, 2024

DrahtBot removed the label Needs rebase on Jul 11, 2024

DrahtBot added the label Needs rebase on Jul 16, 2024

achow101 force-pushed on Jul 22, 2024

DrahtBot removed the label Needs rebase on Jul 22, 2024

DrahtBot added the label Needs rebase on Jul 25, 2024

achow101 force-pushed on Aug 29, 2024

DrahtBot removed the label Needs rebase on Aug 29, 2024

DrahtBot added the label CI failed on Aug 30, 2024

DrahtBot commented at 2:19 am on August 30, 2024: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/29441840067

Make sure to run all tests locally, according to the documentation.

The failure may 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.

DrahtBot added the label Needs rebase on Aug 31, 2024

achow101 force-pushed on Sep 3, 2024

DrahtBot removed the label Needs rebase on Sep 3, 2024

DrahtBot added the label Needs rebase on Sep 4, 2024

achow101 force-pushed on Sep 10, 2024

DrahtBot removed the label Needs rebase on Sep 10, 2024

DrahtBot added the label Needs rebase on Sep 12, 2024

achow101 force-pushed on Sep 17, 2024

DrahtBot removed the label Needs rebase on Sep 17, 2024

DrahtBot added the label Needs rebase on Sep 20, 2024

achow101 force-pushed on Oct 4, 2024

DrahtBot removed the label Needs rebase on Oct 4, 2024

achow101 force-pushed on Oct 4, 2024

DrahtBot added the label Needs rebase on Oct 5, 2024

achow101 force-pushed on Oct 11, 2024

DrahtBot removed the label Needs rebase on Oct 11, 2024

DrahtBot added the label Needs rebase on Oct 24, 2024

achow101 force-pushed on Oct 24, 2024

DrahtBot removed the label Needs rebase on Oct 24, 2024

DrahtBot removed the label CI failed on Oct 24, 2024

DrahtBot added the label Needs rebase on Oct 25, 2024

achow101 force-pushed on Oct 25, 2024

DrahtBot removed the label Needs rebase on Oct 25, 2024

DrahtBot added the label Needs rebase on Oct 28, 2024

achow101 force-pushed on Oct 28, 2024

DrahtBot removed the label Needs rebase on Oct 28, 2024

DrahtBot added the label Needs rebase on Oct 29, 2024

achow101 force-pushed on Oct 29, 2024

DrahtBot removed the label Needs rebase on Oct 29, 2024

DrahtBot added the label Needs rebase on Nov 1, 2024

achow101 force-pushed on Nov 1, 2024

DrahtBot removed the label Needs rebase on Nov 1, 2024

murchandamus commented at 9:43 pm on November 5, 2024: contributor

It looks like all three dependencies got merged, is this ready for review?

achow101 commented at 10:48 pm on November 5, 2024: member

It looks like all three dependencies got merged, is this ready for review?

Currently it is still dependent on #30328 but I suppose it doesn’t have to be.

DrahtBot added the label Needs rebase on Nov 6, 2024

achow101 force-pushed on Nov 6, 2024

DrahtBot added the label CI failed on Nov 6, 2024

DrahtBot removed the label Needs rebase on Nov 6, 2024

achow101 force-pushed on Nov 7, 2024

DrahtBot added the label Needs rebase on Nov 11, 2024

fanquake referenced this in commit 2b33322169 on Nov 12, 2024

test: Rework migratewallet to use previous releases 68b333daa5

wallet: remove BDB dependency from wallet migration benchmark

Stops creating a bdb database in the wallet migration benchmark.
Instead, the benchmark now creates the db in memory and re-uses
it for the migration process.

eb5e102921

test: rpcs disabled for descriptor wallets were removed fe9c186c4d

test: Remove legacy wallet unit tests 69bcc88d86

test: wallet_transactiontime_rescan importdescriptors always rescans 0b36badbad

test: wallet_signer.py bdb will be removed d8f70183d5

test: Remove legacy wallet tests from wallet_backwards_compatibility.py fc6802629e

test: Remove legacy wallet tests from wallet_reindex.py ead3aad252

test: remove legacy wallet functional tests

Removes all legacy wallet specific functional tests.

Also removes the --descriptor and --legacy-wallet options as these are
no longer necessary with the legacy wallet removed.

f3c254d5a4

wallet: Remove -format and bdb from wallet tool's createfromdump a0bf60141e

wallet: Remove wallettool salvage

Salvage is bdb only which is about to be removed.

59d07dd628

bench: Remove WalletLoadingLegacy benchmark 47590ec300

wallet: Don't reload legacy wallet on failed migration

We can't load legacy wallet anymore, so if migration fails, don't try to
load the failed wallet.

827ea87105

build: Require sqlite when --enable-wallet

Require sqlite is available in order to compile the wallet. Removes
instances of USE_SQLITE since it is no longer possible to not have
sqlite available.

537009d0e1

wallet: Disallow legacy wallets

Legacy wallets do not have the descriptors flag set. Don't load wallets
without the descriptors flag.

At the same time, we will no longer load BDB databases since they are
only used for legacy wallets.

83b6dd2448

wallet: Remove IsMine from migration

As IsMine will be removed, the relevant components of IsMine are inlined
into the migration functions.

3a2de31763

Revert "wallet: Move LegacyScriptPubKeyMan::IsMine to LegacyDataSPKM"

This reverts commit bbb1d51e1240da61db2ca1036f9ec91fd2f36f2d.

577f4d9f58

build, wallet, doc: Remove BDB 1432ad2827

wallet, rpc: Remove legacy wallet only RPCs e74ce48224

wallet: Delete LegacySPKM

Deletes LegacyScriptPubKeyMan and related tests

323bc8982a

wallet: Remove unused db functions

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

a2f58e7b18

achow101 force-pushed on Nov 13, 2024

DrahtBot removed the label Needs rebase on Nov 13, 2024

DrahtBot added the label Needs rebase on Nov 15, 2024

DrahtBot commented at 4:23 pm on November 15, 2024: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Remove the legacy wallet and BDB dependency #28710

Code Coverage & Benchmarks

Reviews

Conflicts