Repository: bitcoin/bitcoin
File: src/wallet/wallet.cpp (C++)

Code before:
        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;
}

util::Result<void> CWallet::RemoveTxs(std::vector<Txid>& txs_to_remove)
{
    AssertLockHeld(cs_wallet);
    bilingual_str str_err;  // future: make RunWithinTxn return a util::Result
    bool was_txn_committed = RunWithinTxn(GetDatabase(), /*process_desc=*/"remove transactions", [&](WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) {
        util::Result<void> result{RemoveTxs(batch, txs_to_remove)};
        if (!result) str_err = util::ErrorString(result);
        return result.has_value();
    });
    if (!str_err.empty()) return util::Error{str_err};
    if (!was_txn_committed) return util::Error{_("Error starting/committing db txn for wallet transactions removal process")};
    return {}; // all good
}

util::Result<void> CWallet::RemoveTxs(WalletBatch& batch, std::vector<Txid>& txs_to_remove)
{
    AssertLockHeld(cs_wallet);
    if (!batch.HasActiveTxn()) return util::Error{strprintf(_("The transactions removal process can only be executed within a db txn"))};


Review comment (refactor):
In "wallet: refactor: PopulateWalletFromDB use switch statement." (411caf72815bdf2e176e790a4c63f745517c4bb4):
Nit: I was wondering whether refactoring to a switch-case was just stylistic preference, but I take it that this was done to make the compiler catch missing cases. If that’s the motivation it might be nice to include that in the commit message.

Code after:
                                       " 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;
}

util::Result<void> CWallet::RemoveTxs(std::vector<Txid>& txs_to_remove)
{
    AssertLockHeld(cs_wallet);
    bilingual_str str_err;  // future: make RunWithinTxn return a util::Result
    bool was_txn_committed = RunWithinTxn(GetDatabase(), /*process_desc=*/"remove transactions", [&](WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) {
        util::Result<void> result{RemoveTxs(batch, txs_to_remove)};
        if (!result) str_err = util::ErrorString(result);
        return result.has_value();
    });
    if (!str_err.empty()) return util::Error{str_err};
    if (!was_txn_committed) return util::Error{_("Error starting/committing db txn for wallet transactions removal process")};
    return {}; // all good
}

util::Result<void> CWallet::RemoveTxs(WalletBatch& batch, std::vector<Txid>& txs_to_remove)
{
    AssertLockHeld(cs_wallet);
    if (!batch.HasActiveTxn()) return util::Error{strprintf(_("The transactions removal process can only be executed within a db txn"))};