diff --git a/src/bench/wallet_ismine.cpp b/src/bench/wallet_ismine.cpp index d26f893b98c..6bfbb42de42 100644 --- a/src/bench/wallet_ismine.cpp +++ b/src/bench/wallet_ismine.cpp @@ -36,7 +36,7 @@ static void WalletIsMine(benchmark::Bench& bench, int num_combo = 0) // Loading the wallet will also create it uint64_t create_flags = WALLET_FLAG_DESCRIPTORS; auto database = CreateMockableWalletDatabase(); - auto wallet = TestLoadWallet(std::move(database), context, create_flags); + auto wallet = TestCreateWallet(std::move(database), context, create_flags); // For a descriptor wallet, fill with num_combo combo descriptors with random keys // This benchmarks a non-HD wallet migrated to descriptors diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index f7c78806867..20997ff8927 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -43,7 +43,7 @@ static void WalletLoadingDescriptors(benchmark::Bench& bench) // Loading the wallet will also create it uint64_t create_flags = WALLET_FLAG_DESCRIPTORS; auto database = CreateMockableWalletDatabase(); - auto wallet = TestLoadWallet(std::move(database), context, create_flags); + auto wallet = TestCreateWallet(std::move(database), context, create_flags); // Generate a bunch of transactions and addresses to put into the wallet for (int i = 0; i < 1000; ++i) { @@ -56,7 +56,7 @@ static void WalletLoadingDescriptors(benchmark::Bench& bench) TestUnloadWallet(std::move(wallet)); bench.epochs(5).run([&] { - wallet = TestLoadWallet(std::move(database), context, create_flags); + wallet = TestLoadWallet(std::move(database), context); // Cleanup database = DuplicateMockDatabase(wallet->GetDatabase()); diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index d2ac1306ae4..58b8a6d2ea2 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -77,7 +77,6 @@ void TestAddAddressesToSendBook(interfaces::Node& node) test.m_node.wallet_loader = wallet_loader.get(); node.setContext(&test.m_node); const std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), "", CreateMockableWalletDatabase()); - wallet->LoadWallet(); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); { LOCK(wallet->cs_wallet); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index b62b2a3b82f..2a9ec416f69 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -192,7 +192,6 @@ void SyncUpWallet(const std::shared_ptr& wallet, interfaces::Node& node std::shared_ptr SetupDescriptorsWallet(interfaces::Node& node, TestChain100Setup& test, bool watch_only = false) { std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), "", CreateMockableWalletDatabase()); - wallet->LoadWallet(); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); if (watch_only) { diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index f7f6554b57a..3083bc05d2a 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -203,13 +203,6 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: bool ret = true; std::shared_ptr wallet(new CWallet(/*chain=*/nullptr, name, std::move(database)), WalletToolReleaseWallet); { - LOCK(wallet->cs_wallet); - DBErrors load_wallet_ret = wallet->LoadWallet(); - if (load_wallet_ret != DBErrors::LOAD_OK) { - error = strprintf(_("Error creating %s"), name); - return false; - } - // Get the database handle WalletDatabase& db = wallet->GetDatabase(); std::unique_ptr batch = db.MakeBatch(); diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 84ee9cf10b8..36396193c3f 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -147,7 +147,7 @@ bool LoadWallets(WalletContext& context) } } chain.initMessage(_("Loading wallet…")); - std::shared_ptr pwallet = database ? CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings) : nullptr; + std::shared_ptr pwallet = database ? CWallet::LoadExisting(context, name, std::move(database), error, warnings) : nullptr; if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!pwallet) { chain.initError(error); diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index 1e4fd7aaffc..9ee63f6e228 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -47,11 +47,36 @@ std::unique_ptr CreateSyncedWallet(interfaces::Chain& chain, CChain& cc return wallet; } -std::shared_ptr TestLoadWallet(std::unique_ptr database, WalletContext& context, uint64_t create_flags) +std::shared_ptr TestCreateWallet(std::unique_ptr database, WalletContext& context, uint64_t create_flags) +{ + bilingual_str _error; + std::vector _warnings; + auto wallet = CWallet::CreateNew(context, "", std::move(database), create_flags, _error, _warnings); + NotifyWalletLoaded(context, wallet); + if (context.chain) { + wallet->postInitProcess(); + } + return wallet; +} + +std::shared_ptr TestCreateWallet(WalletContext& context) +{ + DatabaseOptions options; + options.require_create = true; + options.create_flags = WALLET_FLAG_DESCRIPTORS; + DatabaseStatus status; + bilingual_str error; + std::vector warnings; + auto database = MakeWalletDatabase("", options, status, error); + return TestCreateWallet(std::move(database), context, options.create_flags); +} + + +std::shared_ptr TestLoadWallet(std::unique_ptr database, WalletContext& context) { bilingual_str error; std::vector warnings; - auto wallet = CWallet::Create(context, "", std::move(database), create_flags, error, warnings); + auto wallet = CWallet::LoadExisting(context, "", std::move(database), error, warnings); NotifyWalletLoaded(context, wallet); if (context.chain) { wallet->postInitProcess(); @@ -62,12 +87,12 @@ std::shared_ptr TestLoadWallet(std::unique_ptr database std::shared_ptr TestLoadWallet(WalletContext& context) { DatabaseOptions options; - options.create_flags = WALLET_FLAG_DESCRIPTORS; + options.require_existing = true; DatabaseStatus status; bilingual_str error; std::vector warnings; auto database = MakeWalletDatabase("", options, status, error); - return TestLoadWallet(std::move(database), context, options.create_flags); + return TestLoadWallet(std::move(database), context); } void TestUnloadWallet(std::shared_ptr&& wallet) diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index 33f36eb62d9..d5deba295d2 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -32,8 +32,10 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq std::unique_ptr CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key); +std::shared_ptr TestCreateWallet(WalletContext& context); +std::shared_ptr TestCreateWallet(std::unique_ptr database, WalletContext& context, uint64_t create_flags); std::shared_ptr TestLoadWallet(WalletContext& context); -std::shared_ptr TestLoadWallet(std::unique_ptr database, WalletContext& context, uint64_t create_flags); +std::shared_ptr TestLoadWallet(std::unique_ptr database, WalletContext& context); void TestUnloadWallet(std::shared_ptr&& wallet); // Creates a copy of the provided database diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 253e4dd65ae..36782cd187c 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -307,7 +307,7 @@ void TestLoadWallet(const std::string& name, DatabaseFormat format, std::functio std::vector warnings; auto database{MakeWalletDatabase(name, options, status, error)}; auto wallet{std::make_shared(chain.get(), "", std::move(database))}; - BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK); + BOOST_CHECK_EQUAL(wallet->PopulateWalletFromDB(error, warnings), DBErrors::LOAD_OK); WITH_LOCK(wallet->cs_wallet, f(wallet)); } @@ -560,7 +560,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup) BOOST_CHECK_EXCEPTION(vr >> w_desc, std::ios_base::failure, malformed_descriptor); } -//! Test CWallet::Create() and its behavior handling potential race +//! Test CWallet::CreateNew() and its behavior handling potential race //! conditions if it's called the same time an incoming transaction shows up in //! the mempool or a new block. //! @@ -585,7 +585,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) WalletContext context; context.args = &m_args; context.chain = m_node.chain.get(); - auto wallet = TestLoadWallet(context); + auto wallet = TestCreateWallet(context); CKey key = GenerateRandomKey(); AddKey(*wallet, key); TestUnloadWallet(std::move(wallet)); @@ -682,7 +682,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup) { WalletContext context; context.args = &m_args; - auto wallet = TestLoadWallet(context); + auto wallet = TestCreateWallet(context); BOOST_CHECK(wallet); WaitForDeleteWallet(std::move(wallet)); } @@ -693,7 +693,7 @@ BOOST_FIXTURE_TEST_CASE(RemoveTxs, TestChain100Setup) WalletContext context; context.args = &m_args; context.chain = m_node.chain.get(); - auto wallet = TestLoadWallet(context); + auto wallet = TestCreateWallet(context); CKey key = GenerateRandomKey(); AddKey(*wallet, key); diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index ec31cda3895..bc994ba6064 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -41,6 +41,8 @@ public: BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup) { + bilingual_str _error; + std::vector _warnings; std::unique_ptr database = CreateMockableWalletDatabase(); { // Write unknown active descriptor @@ -54,7 +56,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup) { // Now try to load the wallet and verify the error. const std::shared_ptr wallet(new CWallet(m_node.chain.get(), "", std::move(database))); - BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::UNKNOWN_DESCRIPTOR); + BOOST_CHECK_EQUAL(wallet->PopulateWalletFromDB(_error, _warnings), DBErrors::UNKNOWN_DESCRIPTOR); } // Test 2 @@ -80,7 +82,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup) { // Now try to load the wallet and verify the error. const std::shared_ptr wallet(new CWallet(m_node.chain.get(), "", std::move(database))); - BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::CORRUPT); + BOOST_CHECK_EQUAL(wallet->PopulateWalletFromDB(_error, _warnings), DBErrors::CORRUPT); BOOST_CHECK(found); // The error must be logged } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 64d7f191de7..b1562952257 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -283,7 +283,7 @@ std::shared_ptr LoadWalletInternal(WalletContext& context, const std::s } context.chain->initMessage(_("Loading wallet…")); - std::shared_ptr wallet = CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings); + std::shared_ptr wallet = CWallet::LoadExisting(context, name, std::move(database), error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_LOAD; @@ -421,8 +421,8 @@ std::shared_ptr CreateWallet(WalletContext& context, const std::string& } // Make the wallet - context.chain->initMessage(_("Loading wallet…")); - std::shared_ptr wallet = CWallet::Create(context, name, std::move(database), wallet_creation_flags, error, warnings); + context.chain->initMessage(_("Creating wallet…")); + std::shared_ptr wallet = CWallet::CreateNew(context, name, std::move(database), wallet_creation_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_CREATE; @@ -455,6 +455,7 @@ std::shared_ptr CreateWallet(WalletContext& context, const std::string& } } + WITH_LOCK(wallet->cs_wallet, wallet->LogStats()); NotifyWalletLoaded(context, wallet); AddWallet(context, wallet); wallet->postInitProcess(); @@ -2349,7 +2350,7 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve } } -DBErrors CWallet::LoadWallet() +DBErrors CWallet::PopulateWalletFromDB(bilingual_str& error, std::vector& warnings) { LOCK(cs_wallet); @@ -2371,6 +2372,47 @@ DBErrors CWallet::LoadWallet() assert(m_internal_spk_managers.empty()); } + const auto wallet_file = m_database->Filename(); + switch (nLoadWalletRet) { + case DBErrors::LOAD_OK: + break; + case DBErrors::NONCRITICAL_ERROR: + warnings.push_back(strprintf(_("Error reading %s! All keys read correctly, but transaction data" + " or address metadata may be missing or incorrect."), + wallet_file)); + break; + case DBErrors::NEED_RESCAN: + warnings.push_back(strprintf(_("Error reading %s! Transaction data may be missing or incorrect." + " Rescanning wallet."), wallet_file)); + break; + case DBErrors::CORRUPT: + error = strprintf(_("Error loading %s: Wallet corrupted"), wallet_file); + break; + case DBErrors::TOO_NEW: + error = strprintf(_("Error loading %s: Wallet requires newer version of %s"), wallet_file, CLIENT_NAME); + break; + case DBErrors::EXTERNAL_SIGNER_SUPPORT_REQUIRED: + error = strprintf(_("Error loading %s: External signer wallet being loaded without external signer support compiled"), wallet_file); + break; + case DBErrors::NEED_REWRITE: + error = strprintf(_("Wallet needed to be rewritten: restart %s to complete"), CLIENT_NAME); + break; + case DBErrors::UNKNOWN_DESCRIPTOR: + error = strprintf(_("Unrecognized descriptor found. Loading wallet %s\n\n" + "The wallet might have been created on a newer version.\n" + "Please try running the latest software version.\n"), wallet_file); + break; + case DBErrors::UNEXPECTED_LEGACY_ENTRY: + error = strprintf(_("Unexpected legacy entry in descriptor wallet found. Loading wallet %s\n\n" + "The wallet might have been tampered with or created with malicious intent.\n"), wallet_file); + break; + case DBErrors::LEGACY_WALLET: + error = strprintf(_("Error loading %s: Wallet is a legacy wallet. Please migrate to a descriptor wallet using the migration tool (migratewallet RPC)."), wallet_file); + break; + case DBErrors::LOAD_FAIL: + error = strprintf(_("Error loading %s"), wallet_file); + break; + } // no default case, so the compiler can warn about missing cases return nLoadWalletRet; } @@ -2906,72 +2948,168 @@ std::unique_ptr MakeWalletDatabase(const std::string& name, cons return MakeDatabase(*wallet_path, options, status, error_string); } -std::shared_ptr CWallet::Create(WalletContext& context, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) +bool CWallet::LoadWalletArgs(std::shared_ptr wallet, const WalletContext& context, bilingual_str& error, std::vector& warnings) +{ + interfaces::Chain* chain = context.chain; + const ArgsManager& args = *Assert(context.args); + + if (!args.GetArg("-addresstype", "").empty()) { + std::optional parsed = ParseOutputType(args.GetArg("-addresstype", "")); + if (!parsed) { + error = strprintf(_("Unknown address type '%s'"), args.GetArg("-addresstype", "")); + return false; + } + wallet->m_default_address_type = parsed.value(); + } + + if (!args.GetArg("-changetype", "").empty()) { + std::optional parsed = ParseOutputType(args.GetArg("-changetype", "")); + if (!parsed) { + error = strprintf(_("Unknown change type '%s'"), args.GetArg("-changetype", "")); + return false; + } + wallet->m_default_change_type = parsed.value(); + } + + if (const auto arg{args.GetArg("-mintxfee")}) { + std::optional min_tx_fee = ParseMoney(*arg); + if (!min_tx_fee) { + error = AmountErrMsg("mintxfee", *arg); + return false; + } else if (min_tx_fee.value() > HIGH_TX_FEE_PER_KB) { + warnings.push_back(AmountHighWarn("-mintxfee") + Untranslated(" ") + + _("This is the minimum transaction fee you pay on every transaction.")); + } + + wallet->m_min_fee = CFeeRate{min_tx_fee.value()}; + } + + if (const auto arg{args.GetArg("-maxapsfee")}) { + const std::string& max_aps_fee{*arg}; + if (max_aps_fee == "-1") { + wallet->m_max_aps_fee = -1; + } else if (std::optional max_fee = ParseMoney(max_aps_fee)) { + if (max_fee.value() > HIGH_APS_FEE) { + warnings.push_back(AmountHighWarn("-maxapsfee") + Untranslated(" ") + + _("This is the maximum transaction fee you pay (in addition to the normal fee) to prioritize partial spend avoidance over regular coin selection.")); + } + wallet->m_max_aps_fee = max_fee.value(); + } else { + error = AmountErrMsg("maxapsfee", max_aps_fee); + return false; + } + } + + if (const auto arg{args.GetArg("-fallbackfee")}) { + std::optional fallback_fee = ParseMoney(*arg); + if (!fallback_fee) { + error = strprintf(_("Invalid amount for %s=: '%s'"), "-fallbackfee", *arg); + return false; + } else if (fallback_fee.value() > HIGH_TX_FEE_PER_KB) { + warnings.push_back(AmountHighWarn("-fallbackfee") + Untranslated(" ") + + _("This is the transaction fee you may pay when fee estimates are not available.")); + } + wallet->m_fallback_fee = CFeeRate{fallback_fee.value()}; + } + + // Disable fallback fee in case value was set to 0, enable if non-null value + wallet->m_allow_fallback_fee = wallet->m_fallback_fee.GetFeePerK() != 0; + + if (const auto arg{args.GetArg("-discardfee")}) { + std::optional discard_fee = ParseMoney(*arg); + if (!discard_fee) { + error = strprintf(_("Invalid amount for %s=: '%s'"), "-discardfee", *arg); + return false; + } else if (discard_fee.value() > HIGH_TX_FEE_PER_KB) { + warnings.push_back(AmountHighWarn("-discardfee") + Untranslated(" ") + + _("This is the transaction fee you may discard if change is smaller than dust at this level")); + } + wallet->m_discard_rate = CFeeRate{discard_fee.value()}; + } + + if (const auto arg{args.GetArg("-paytxfee")}) { + warnings.push_back(_("-paytxfee is deprecated and will be fully removed in v31.0.")); + + std::optional pay_tx_fee = ParseMoney(*arg); + if (!pay_tx_fee) { + error = AmountErrMsg("paytxfee", *arg); + return false; + } else if (pay_tx_fee.value() > HIGH_TX_FEE_PER_KB) { + warnings.push_back(AmountHighWarn("-paytxfee") + Untranslated(" ") + + _("This is the transaction fee you will pay if you send a transaction.")); + } + + wallet->m_pay_tx_fee = CFeeRate{pay_tx_fee.value(), 1000}; + + if (chain && wallet->m_pay_tx_fee < chain->relayMinFee()) { + error = strprintf(_("Invalid amount for %s=: '%s' (must be at least %s)"), + "-paytxfee", *arg, chain->relayMinFee().ToString()); + return false; + } + } + + if (const auto arg{args.GetArg("-maxtxfee")}) { + std::optional max_fee = ParseMoney(*arg); + if (!max_fee) { + error = AmountErrMsg("maxtxfee", *arg); + return false; + } else if (max_fee.value() > HIGH_MAX_TX_FEE) { + warnings.push_back(strprintf(_("%s is set very high! Fees this large could be paid on a single transaction."), "-maxtxfee")); + } + + if (chain && CFeeRate{max_fee.value(), 1000} < chain->relayMinFee()) { + error = strprintf(_("Invalid amount for %s=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), + "-maxtxfee", *arg, chain->relayMinFee().ToString()); + return false; + } + + wallet->m_default_max_tx_fee = max_fee.value(); + } + + if (const auto arg{args.GetArg("-consolidatefeerate")}) { + if (std::optional consolidate_feerate = ParseMoney(*arg)) { + wallet->m_consolidate_feerate = CFeeRate(*consolidate_feerate); + } else { + error = AmountErrMsg("consolidatefeerate", *arg); + return false; + } + } + + if (chain && chain->relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) { + warnings.push_back(AmountHighWarn("-minrelaytxfee") + Untranslated(" ") + + _("The wallet will avoid paying less than the minimum relay fee.")); + } + + wallet->m_confirm_target = args.GetIntArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET); + wallet->m_spend_zero_conf_change = args.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); + wallet->m_signal_rbf = args.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF); + + wallet->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1}); + wallet->m_notify_tx_changed_script = args.GetArg("-walletnotify", ""); + wallet->SetBroadcastTransactions(args.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); + + return true; +} + +std::shared_ptr CWallet::CreateNew(WalletContext& context, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) { interfaces::Chain* chain = context.chain; - ArgsManager& args = *Assert(context.args); const std::string& walletFile = database->Filename(); const auto start{SteadyClock::now()}; // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. std::shared_ptr walletInstance(new CWallet(chain, name, std::move(database)), FlushAndDeleteWallet); - walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1}); - walletInstance->m_notify_tx_changed_script = args.GetArg("-walletnotify", ""); - // Load wallet - bool rescan_required = false; - DBErrors nLoadWalletRet = walletInstance->LoadWallet(); - if (nLoadWalletRet != DBErrors::LOAD_OK) { - if (nLoadWalletRet == DBErrors::CORRUPT) { - error = strprintf(_("Error loading %s: Wallet corrupted"), walletFile); - return nullptr; - } - else if (nLoadWalletRet == DBErrors::NONCRITICAL_ERROR) - { - warnings.push_back(strprintf(_("Error reading %s! All keys read correctly, but transaction data" - " or address metadata may be missing or incorrect."), - walletFile)); - } - else if (nLoadWalletRet == DBErrors::TOO_NEW) { - error = strprintf(_("Error loading %s: Wallet requires newer version of %s"), walletFile, CLIENT_NAME); - return nullptr; - } - else if (nLoadWalletRet == DBErrors::EXTERNAL_SIGNER_SUPPORT_REQUIRED) { - error = strprintf(_("Error loading %s: External signer wallet being loaded without external signer support compiled"), walletFile); - return nullptr; - } - else if (nLoadWalletRet == DBErrors::NEED_REWRITE) - { - error = strprintf(_("Wallet needed to be rewritten: restart %s to complete"), CLIENT_NAME); - return nullptr; - } else if (nLoadWalletRet == DBErrors::NEED_RESCAN) { - warnings.push_back(strprintf(_("Error reading %s! Transaction data may be missing or incorrect." - " Rescanning wallet."), walletFile)); - rescan_required = true; - } else if (nLoadWalletRet == DBErrors::UNKNOWN_DESCRIPTOR) { - error = strprintf(_("Unrecognized descriptor found. Loading wallet %s\n\n" - "The wallet might have been created on a newer version.\n" - "Please try running the latest software version.\n"), walletFile); - return nullptr; - } else if (nLoadWalletRet == DBErrors::UNEXPECTED_LEGACY_ENTRY) { - error = strprintf(_("Unexpected legacy entry in descriptor wallet found. Loading wallet %s\n\n" - "The wallet might have been tampered with or created with malicious intent.\n"), walletFile); - return nullptr; - } else if (nLoadWalletRet == DBErrors::LEGACY_WALLET) { - error = strprintf(_("Error loading %s: Wallet is a legacy wallet. Please migrate to a descriptor wallet using the migration tool (migratewallet RPC)."), walletFile); - return nullptr; - } else { - error = strprintf(_("Error loading %s"), walletFile); - return nullptr; - } + if (!LoadWalletArgs(walletInstance, context, error, warnings)) { + return nullptr; } - // This wallet is in its first run if there are no ScriptPubKeyMans and it isn't blank or no privkeys - const bool fFirstRun = walletInstance->m_spk_managers.empty() && - !walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && - !walletInstance->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); - if (fFirstRun) + // Initialize version key. + if(!WalletBatch(walletInstance->GetDatabase()).WriteVersion(CLIENT_VERSION)) { + error = strprintf(_("Error creating %s: Could not write version metadata."), walletFile); + return nullptr; + } { LOCK(walletInstance->cs_wallet); @@ -2992,11 +3130,41 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri walletInstance->SetLastBlockProcessed(*tip_height, chain->getBlockHash(*tip_height)); } } - } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { - // Make it impossible to disable private keys after creation - error = strprintf(_("Error loading %s: Private keys can only be disabled during creation"), walletFile); + } + + walletInstance->WalletLogPrintf("Wallet completed creation in %15dms\n", Ticks(SteadyClock::now() - start)); + + // Try to top up keypool. No-op if the wallet is locked. + walletInstance->TopUpKeyPool(); + + if (chain && !AttachChain(walletInstance, *chain, /*rescan_required=*/false, error, warnings)) { + walletInstance->m_chain_notifications_handler.reset(); // Reset this pointer so that the wallet will actually be unloaded return nullptr; - } else if (walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + } + + return walletInstance; +} + +std::shared_ptr CWallet::LoadExisting(WalletContext& context, const std::string& name, std::unique_ptr database, bilingual_str& error, std::vector& warnings) +{ + interfaces::Chain* chain = context.chain; + const std::string& walletFile = database->Filename(); + + const auto start{SteadyClock::now()}; + std::shared_ptr walletInstance(new CWallet(chain, name, std::move(database)), FlushAndDeleteWallet); + + if (!LoadWalletArgs(walletInstance, context, error, warnings)) { + return nullptr; + } + + // Load wallet + auto nLoadWalletRet = walletInstance->PopulateWalletFromDB(error, warnings); + bool rescan_required = nLoadWalletRet == DBErrors::NEED_RESCAN; + if (nLoadWalletRet != DBErrors::LOAD_OK && nLoadWalletRet != DBErrors::NONCRITICAL_ERROR && !rescan_required) { + return nullptr; + } + + if (walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { for (auto spk_man : walletInstance->GetActiveScriptPubKeyMans()) { if (spk_man->HavePrivateKeys()) { warnings.push_back(strprintf(_("Warning: Private keys detected in wallet {%s} with disabled private keys"), walletFile)); @@ -3005,166 +3173,22 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri } } - if (!args.GetArg("-addresstype", "").empty()) { - std::optional parsed = ParseOutputType(args.GetArg("-addresstype", "")); - if (!parsed) { - error = strprintf(_("Unknown address type '%s'"), args.GetArg("-addresstype", "")); - return nullptr; - } - walletInstance->m_default_address_type = parsed.value(); - } - - if (!args.GetArg("-changetype", "").empty()) { - std::optional parsed = ParseOutputType(args.GetArg("-changetype", "")); - if (!parsed) { - error = strprintf(_("Unknown change type '%s'"), args.GetArg("-changetype", "")); - return nullptr; - } - walletInstance->m_default_change_type = parsed.value(); - } - - if (const auto arg{args.GetArg("-mintxfee")}) { - std::optional min_tx_fee = ParseMoney(*arg); - if (!min_tx_fee) { - error = AmountErrMsg("mintxfee", *arg); - return nullptr; - } else if (min_tx_fee.value() > HIGH_TX_FEE_PER_KB) { - warnings.push_back(AmountHighWarn("-mintxfee") + Untranslated(" ") + - _("This is the minimum transaction fee you pay on every transaction.")); - } - - walletInstance->m_min_fee = CFeeRate{min_tx_fee.value()}; - } - - if (const auto arg{args.GetArg("-maxapsfee")}) { - const std::string& max_aps_fee{*arg}; - if (max_aps_fee == "-1") { - walletInstance->m_max_aps_fee = -1; - } else if (std::optional max_fee = ParseMoney(max_aps_fee)) { - if (max_fee.value() > HIGH_APS_FEE) { - warnings.push_back(AmountHighWarn("-maxapsfee") + Untranslated(" ") + - _("This is the maximum transaction fee you pay (in addition to the normal fee) to prioritize partial spend avoidance over regular coin selection.")); - } - walletInstance->m_max_aps_fee = max_fee.value(); - } else { - error = AmountErrMsg("maxapsfee", max_aps_fee); - return nullptr; - } - } - - if (const auto arg{args.GetArg("-fallbackfee")}) { - std::optional fallback_fee = ParseMoney(*arg); - if (!fallback_fee) { - error = strprintf(_("Invalid amount for %s=: '%s'"), "-fallbackfee", *arg); - return nullptr; - } else if (fallback_fee.value() > HIGH_TX_FEE_PER_KB) { - warnings.push_back(AmountHighWarn("-fallbackfee") + Untranslated(" ") + - _("This is the transaction fee you may pay when fee estimates are not available.")); - } - walletInstance->m_fallback_fee = CFeeRate{fallback_fee.value()}; - } - - // Disable fallback fee in case value was set to 0, enable if non-null value - walletInstance->m_allow_fallback_fee = walletInstance->m_fallback_fee.GetFeePerK() != 0; - - if (const auto arg{args.GetArg("-discardfee")}) { - std::optional discard_fee = ParseMoney(*arg); - if (!discard_fee) { - error = strprintf(_("Invalid amount for %s=: '%s'"), "-discardfee", *arg); - return nullptr; - } else if (discard_fee.value() > HIGH_TX_FEE_PER_KB) { - warnings.push_back(AmountHighWarn("-discardfee") + Untranslated(" ") + - _("This is the transaction fee you may discard if change is smaller than dust at this level")); - } - walletInstance->m_discard_rate = CFeeRate{discard_fee.value()}; - } - - if (const auto arg{args.GetArg("-paytxfee")}) { - warnings.push_back(_("-paytxfee is deprecated and will be fully removed in v31.0.")); - - std::optional pay_tx_fee = ParseMoney(*arg); - if (!pay_tx_fee) { - error = AmountErrMsg("paytxfee", *arg); - return nullptr; - } else if (pay_tx_fee.value() > HIGH_TX_FEE_PER_KB) { - warnings.push_back(AmountHighWarn("-paytxfee") + Untranslated(" ") + - _("This is the transaction fee you will pay if you send a transaction.")); - } - - walletInstance->m_pay_tx_fee = CFeeRate{pay_tx_fee.value(), 1000}; - - if (chain && walletInstance->m_pay_tx_fee < chain->relayMinFee()) { - error = strprintf(_("Invalid amount for %s=: '%s' (must be at least %s)"), - "-paytxfee", *arg, chain->relayMinFee().ToString()); - return nullptr; - } - } - - if (const auto arg{args.GetArg("-maxtxfee")}) { - std::optional max_fee = ParseMoney(*arg); - if (!max_fee) { - error = AmountErrMsg("maxtxfee", *arg); - return nullptr; - } else if (max_fee.value() > HIGH_MAX_TX_FEE) { - warnings.push_back(strprintf(_("%s is set very high! Fees this large could be paid on a single transaction."), "-maxtxfee")); - } - - if (chain && CFeeRate{max_fee.value(), 1000} < chain->relayMinFee()) { - error = strprintf(_("Invalid amount for %s=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), - "-maxtxfee", *arg, chain->relayMinFee().ToString()); - return nullptr; - } - - walletInstance->m_default_max_tx_fee = max_fee.value(); - } - - if (const auto arg{args.GetArg("-consolidatefeerate")}) { - if (std::optional consolidate_feerate = ParseMoney(*arg)) { - walletInstance->m_consolidate_feerate = CFeeRate(*consolidate_feerate); - } else { - error = AmountErrMsg("consolidatefeerate", *arg); - return nullptr; - } - } - - if (chain && chain->relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) { - warnings.push_back(AmountHighWarn("-minrelaytxfee") + Untranslated(" ") + - _("The wallet will avoid paying less than the minimum relay fee.")); - } - - walletInstance->m_confirm_target = args.GetIntArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET); - walletInstance->m_spend_zero_conf_change = args.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); - walletInstance->m_signal_rbf = args.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF); - walletInstance->WalletLogPrintf("Wallet completed loading in %15dms\n", Ticks(SteadyClock::now() - start)); // Try to top up keypool. No-op if the wallet is locked. walletInstance->TopUpKeyPool(); - // Cache the first key time - std::optional time_first_key; - for (auto spk_man : walletInstance->GetAllScriptPubKeyMans()) { - int64_t time = spk_man->GetTimeFirstKey(); - if (!time_first_key || time < *time_first_key) time_first_key = time; - } - if (time_first_key) walletInstance->MaybeUpdateBirthTime(*time_first_key); - if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) { walletInstance->m_chain_notifications_handler.reset(); // Reset this pointer so that the wallet will actually be unloaded return nullptr; } - { - LOCK(walletInstance->cs_wallet); - walletInstance->SetBroadcastTransactions(args.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); - walletInstance->WalletLogPrintf("setKeyPool.size() = %u\n", walletInstance->GetKeyPoolSize()); - walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); - walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); - } + WITH_LOCK(walletInstance->cs_wallet, walletInstance->LogStats()); return walletInstance; } + bool CWallet::AttachChain(const std::shared_ptr& walletInstance, interfaces::Chain& chain, const bool rescan_required, bilingual_str& error, std::vector& warnings) { LOCK(walletInstance->cs_wallet); @@ -4165,7 +4189,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, return false; } - data->watchonly_wallet = CWallet::Create(empty_context, wallet_name, std::move(database), options.create_flags, error, warnings); + data->watchonly_wallet = CWallet::CreateNew(empty_context, wallet_name, std::move(database), options.create_flags, error, warnings); if (!data->watchonly_wallet) { error = _("Error: Failed to create new watchonly wallet"); return false; @@ -4204,7 +4228,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, return false; } - data->solvable_wallet = CWallet::Create(empty_context, wallet_name, std::move(database), options.create_flags, error, warnings); + data->solvable_wallet = CWallet::CreateNew(empty_context, wallet_name, std::move(database), options.create_flags, error, warnings); if (!data->solvable_wallet) { error = _("Error: Failed to create new watchonly wallet"); return false; @@ -4281,7 +4305,7 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle } // Make the local wallet - std::shared_ptr local_wallet = CWallet::Create(empty_context, wallet_name, std::move(database), options.create_flags, error, warnings); + std::shared_ptr local_wallet = CWallet::LoadExisting(empty_context, wallet_name, std::move(database), error, warnings); if (!local_wallet) { return util::Error{Untranslated("Wallet loading failed.") + Untranslated(" ") + error}; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4250acca69e..ef5efa71828 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -797,7 +797,7 @@ public: bool IsFromMe(const CTransaction& tx) const; CAmount GetDebit(const CTransaction& tx) const; - DBErrors LoadWallet(); + DBErrors PopulateWalletFromDB(bilingual_str& error, std::vector& warnings); /** Erases the provided transactions from the wallet. */ util::Result RemoveTxs(std::vector& txs_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -871,8 +871,13 @@ public: /** Mark a transaction as replaced by another transaction. */ bool MarkReplaced(const Txid& originalHash, const Txid& newHash); - /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr Create(WalletContext& context, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings); + static bool LoadWalletArgs(std::shared_ptr wallet, const WalletContext& context, bilingual_str& error, std::vector& warnings); + + /* Initializes, creates and returns a new CWallet; returns a null pointer in case of an error */ + static std::shared_ptr CreateNew(WalletContext& context, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings); + + /* Initializes, loads, and returns a CWallet from an existing wallet; returns a null pointer in case of an error */ + static std::shared_ptr LoadExisting(WalletContext& context, const std::string& name, std::unique_ptr database, bilingual_str& error, std::vector& warnings); /** * Wallet post-init setup @@ -938,6 +943,14 @@ public: LogInfo("[%s] %s", LogName(), tfm::format(wallet_fmt, params...)); }; + void LogStats() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) + { + AssertLockHeld(cs_wallet); + WalletLogPrintf("setKeyPool.size() = %u\n", GetKeyPoolSize()); + WalletLogPrintf("mapWallet.size() = %u\n", mapWallet.size()); + WalletLogPrintf("m_address_book.size() = %u\n", m_address_book.size()); + }; + //! Returns all unique ScriptPubKeyMans in m_internal_spk_managers and m_external_spk_managers std::set GetActiveScriptPubKeyMans() const; bool IsActiveScriptPubKeyMan(const ScriptPubKeyMan& spkm) const; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 617b8282d9d..4e168112714 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1175,7 +1175,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) return result; if (!has_last_client || last_client != CLIENT_VERSION) // Update - m_batch->Write(DBKeys::VERSION, CLIENT_VERSION); + this->WriteVersion(CLIENT_VERSION); if (any_unordered) result = pwallet->ReorderTransactions(); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index c4466bfaef3..65ff8c0b60f 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -265,6 +265,9 @@ public: DBErrors LoadWallet(CWallet* pwallet); + //! Write the given client_version. + bool WriteVersion(int client_version) { return m_batch->Write(DBKeys::VERSION, CLIENT_VERSION); } + //! Delete records of the given types bool EraseRecords(const std::unordered_set& types); diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 1ac0bbb72f0..7c24c0e1c6a 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -2,8 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include // IWYU pragma: keep - #include #include @@ -44,6 +42,7 @@ static std::shared_ptr MakeWallet(const std::string& name, const fs::pa { DatabaseStatus status; bilingual_str error; + std::vector warnings; std::unique_ptr database = MakeDatabase(path, options, status, error); if (!database) { tfm::format(std::cerr, "%s\n", error.original); @@ -54,35 +53,22 @@ static std::shared_ptr MakeWallet(const std::string& name, const fs::pa std::shared_ptr wallet_instance{new CWallet(/*chain=*/nullptr, name, std::move(database)), WalletToolReleaseWallet}; DBErrors load_wallet_ret; try { - load_wallet_ret = wallet_instance->LoadWallet(); + load_wallet_ret = wallet_instance->PopulateWalletFromDB(error, warnings); } catch (const std::runtime_error&) { tfm::format(std::cerr, "Error loading %s. Is wallet being used by another process?\n", name); return nullptr; } - if (load_wallet_ret != DBErrors::LOAD_OK) { - if (load_wallet_ret == DBErrors::CORRUPT) { - tfm::format(std::cerr, "Error loading %s: Wallet corrupted", name); - return nullptr; - } else if (load_wallet_ret == DBErrors::NONCRITICAL_ERROR) { - tfm::format(std::cerr, "Error reading %s! All keys read correctly, but transaction data" - " or address book entries might be missing or incorrect.", - name); - } else if (load_wallet_ret == DBErrors::TOO_NEW) { - tfm::format(std::cerr, "Error loading %s: Wallet requires newer version of %s", - name, CLIENT_NAME); - return nullptr; - } else if (load_wallet_ret == DBErrors::NEED_REWRITE) { - tfm::format(std::cerr, "Wallet needed to be rewritten: restart %s to complete", CLIENT_NAME); - return nullptr; - } else if (load_wallet_ret == DBErrors::NEED_RESCAN) { - tfm::format(std::cerr, "Error reading %s! Some transaction data might be missing or" - " incorrect. Wallet requires a rescan.", - name); - } else { - tfm::format(std::cerr, "Error loading %s", name); - return nullptr; - } + if (!error.empty()) { + tfm::format(std::cerr, "%s", error.original); + } + + for (const auto &warning : warnings) { + tfm::format(std::cerr, "%s", warning.original); + } + + if (load_wallet_ret != DBErrors::LOAD_OK && load_wallet_ret != DBErrors::NONCRITICAL_ERROR && load_wallet_ret != DBErrors::NEED_RESCAN) { + return nullptr; } if (options.require_create) WalletCreate(wallet_instance.get(), options.create_flags); diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py index 217a331d26d..8edbbcf4b3f 100755 --- a/test/functional/wallet_createwallet.py +++ b/test/functional/wallet_createwallet.py @@ -168,14 +168,17 @@ class CreateWalletTest(BitcoinTestFramework): assert_raises_rpc_error(-4, 'descriptors argument must be set to "true"; it is no longer possible to create a legacy wallet.', self.nodes[0].createwallet, wallet_name="legacy", descriptors=False) self.log.info("Check that the version number is being logged correctly") - with node.assert_debug_log(expected_msgs=[], unexpected_msgs=["Last client version = "]): - node.createwallet("version_check") - wallet = node.get_wallet_rpc("version_check") + + # Craft the expected version message. client_version = node.getnetworkinfo()["version"] - wallet.unloadwallet() - with node.assert_debug_log( - expected_msgs=[f"Last client version = {client_version}"] - ): + version_message = f"Last client version = {client_version}" + + # Should not be logged when creating. + with node.assert_debug_log(expected_msgs=[], unexpected_msgs=[version_message]): + node.createwallet("version_check") + node.unloadwallet("version_check") + # Should be logged when loading. + with node.assert_debug_log(expected_msgs=[version_message]): node.loadwallet("version_check")