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

Code before:
    // This operation is not atomic, but if it fails, updated entries are still backwards compatible with older software
    try {
        pwallet->UpgradeKeyMetadata();
    } catch (...) {
        result = DBErrors::CORRUPT;
    }

    // Upgrade all of the descriptor caches to cache the last hardened xpub
    // This operation is not atomic, but if it fails, only new entries are added so it is backwards compatible
    try {
        pwallet->UpgradeDescriptorCache();
    } catch (...) {
        result = DBErrors::CORRUPT;
    }

    return result;
}

static bool RunWithinTxn(WalletBatch& batch, std::string_view process_desc, const std::function<bool(WalletBatch&)>& func)
{
    if (!batch.TxnBegin()) {
        LogDebug(BCLog::WALLETDB, "Error: cannot create db txn for %s\n", process_desc);
        return false;
    }

    // Run procedure
    if (!func(batch)) {
        LogDebug(BCLog::WALLETDB, "Error: %s failed\n", process_desc);
        batch.TxnAbort();
        return false;
    }

    if (!batch.TxnCommit()) {
        LogDebug(BCLog::WALLETDB, "Error: cannot commit db txn for %s\n", process_desc);
        return false;
    }

    // All good
    return true;
}

bool RunWithinTxn(WalletDatabase& database, std::string_view process_desc, const std::function<bool(WalletBatch&)>& func)
{
    WalletBatch batch(database);
    return RunWithinTxn(batch, process_desc, func);
}

void MaybeCompactWalletDB(WalletContext& context)
{
    static std::atomic<bool> fOneThread(false);
    if (fOneThread.exchange(true)) {

Review comment (suggestion):
I was wondering if if could be a good idea to log the removed keys at this point as a backup (safe-ish seeing as we are deleting), but I'm struggling to think of a scenario where a user with this type of wallet might somehow need these keys again...

Code after:

    if (any_unordered)
        result = pwallet->ReorderTransactions();

    // Upgrade all of the wallet keymetadata to have the hd master key id
    // This operation is not atomic, but if it fails, updated entries are still backwards compatible with older software
    try {
        pwallet->UpgradeKeyMetadata();
    } catch (...) {
        result = DBErrors::CORRUPT;
    }

    // Upgrade all of the descriptor caches to cache the last hardened xpub
    // This operation is not atomic, but if it fails, only new entries are added so it is backwards compatible
    try {
        pwallet->UpgradeDescriptorCache();
    } catch (...) {
        result = DBErrors::CORRUPT;
    }

    // Since it was accidentally possible to "encrypt" a wallet with private keys disabled, we should check if this is
    // such a wallet and remove the encryption key records to avoid any future issues.
    // Although wallets without private keys should not have *ckey records, we should double check that.
    // Removing the mkey records is only safe if there are no *ckey records.
    if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && pwallet->HasEncryptionKeys() && !pwallet->HaveCryptedKeys()) {
        pwallet->WalletLogPrintf("Detected extraneous encryption keys in this wallet without private keys. Removing extraneous encryption keys.\n");
        for (const auto& [id, _] : pwallet->mapMasterKeys) {
            if (!EraseMasterKey(id)) {
                pwallet->WalletLogPrintf("Error: Unable to remove extraneous encryption key '%u'. Wallet corrupt.\n", id);
                return DBErrors::CORRUPT;
            }
        }
        pwallet->mapMasterKeys.clear();
    }

    return result;
}

static bool RunWithinTxn(WalletBatch& batch, std::string_view process_desc, const std::function<bool(WalletBatch&)>& func)
{
    if (!batch.TxnBegin()) {
        LogDebug(BCLog::WALLETDB, "Error: cannot create db txn for %s\n", process_desc);
        return false;
    }

    // Run procedure
    if (!func(batch)) {
        LogDebug(BCLog::WALLETDB, "Error: %s failed\n", process_desc);
        batch.TxnAbort();
        return false;
    }