wallet: remove outdated pszSkip arg of database Rewrite func #32990

1. 
   rkrux commented at 9:04 am on July 16, 2025: contributor

   This argument might have been used in the legacy wallets, but I don’t see any implementation using this argument in the SQLite wallets. Removing it cleans up the code a bit.
2. wallet: remove outdated `pszSkip` arg of database `Rewrite` func

   This argument might have been used in the legacy wallets, but I don't
   see any implementation using this argument in the SQLite wallets.
   Removing it cleans up the code a bit.

   2dfeb6668c
3. DrahtBot added the label Wallet on Jul 16, 2025
4. 
   DrahtBot commented at 9:04 am on July 16, 2025: 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/32990.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	brunoerg, achow101

   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:

   • #33034 (wallet: Store transactions in a separate sqlite table by achow101)
   • #33032 (wallet, test: Replace MockableDatabase with in-memory SQLiteDatabase 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.

5. 
   rkrux commented at 9:12 am on July 16, 2025: contributor

   I noted this while reviewing #28333.
6. brunoerg approved
7. 
   brunoerg commented at 8:08 pm on July 17, 2025: contributor

   code review ACK 2dfeb6668cb2e98e3dccf946af084e8a08e1fab5

   I verified that this arg was used only by the legacy wallet. At 2dfeb6668cb2e98e3dccf946af084e8a08e1fab5 (from the PR that added sqlite), this arg was used in the following function from BerkeleyDatabase (which obviously doesn’t exist anymore):

    0bool BerkeleyDatabase::Rewrite(const char* pszSkip)
    1{
    2    while (true) {
    3        {
    4            LOCK(cs_db);
    5            if (m_refcount <= 0) {
    6                // Flush log data to the dat file
    7                env->CloseDb(strFile);
    8                env->CheckpointLSN(strFile);
    9                m_refcount = -1;
   10
   11                bool fSuccess = true;
   12                LogPrintf("BerkeleyBatch::Rewrite: Rewriting %s...\n", strFile);
   13                std::string strFileRes = strFile + ".rewrite";
   14                { // surround usage of db with extra {}
   15                    BerkeleyBatch db(*this, true);
   16                    std::unique_ptr<Db> pdbCopy = MakeUnique<Db>(env->dbenv.get(), 0);
   17
   18                    int ret = pdbCopy->open(nullptr,               // Txn pointer
   19                                            strFileRes.c_str(), // Filename
   20                                            "main",             // Logical db name
   21                                            DB_BTREE,           // Database type
   22                                            DB_CREATE,          // Flags
   23                                            0);
   24                    if (ret > 0) {
   25                        LogPrintf("BerkeleyBatch::Rewrite: Can't create database file %s\n", strFileRes);
   26                        fSuccess = false;
   27                    }
   28
   29                    if (db.StartCursor()) {
   30                        while (fSuccess) {
   31                            CDataStream ssKey(SER_DISK, CLIENT_VERSION);
   32                            CDataStream ssValue(SER_DISK, CLIENT_VERSION);
   33                            bool complete;
   34                            bool ret1 = db.ReadAtCursor(ssKey, ssValue, complete);
   35                            if (complete) {
   36                                break;
   37                            } else if (!ret1) {
   38                                fSuccess = false;
   39                                break;
   40                            }
   41                            if (pszSkip &&
   42                                strncmp(ssKey.data(), pszSkip, std::min(ssKey.size(), strlen(pszSkip))) == 0)
   43                                continue;
   44                            if (strncmp(ssKey.data(), "\x07version", 8) == 0) {
   45                                // Update version:
   46                                ssValue.clear();
   47                                ssValue << CLIENT_VERSION;
   48                            }
   49                            Dbt datKey(ssKey.data(), ssKey.size());
   50                            Dbt datValue(ssValue.data(), ssValue.size());
   51                            int ret2 = pdbCopy->put(nullptr, &datKey, &datValue, DB_NOOVERWRITE);
   52                            if (ret2 > 0)
   53                                fSuccess = false;
   54                        }
   55                        db.CloseCursor();
   56                    }
   57                    if (fSuccess) {
   58                        db.Close();
   59                        env->CloseDb(strFile);
   60                        if (pdbCopy->close(0))
   61                            fSuccess = false;
   62                    } else {
   63                        pdbCopy->close(0);
   64                    }
   65                }
   66                if (fSuccess) {
   67                    Db dbA(env->dbenv.get(), 0);
   68                    if (dbA.remove(strFile.c_str(), nullptr, 0))
   69                        fSuccess = false;
   70                    Db dbB(env->dbenv.get(), 0);
   71                    if (dbB.rename(strFileRes.c_str(), nullptr, strFile.c_str(), 0))
   72                        fSuccess = false;
   73                }
   74                if (!fSuccess)
   75                    LogPrintf("BerkeleyBatch::Rewrite: Failed to rewrite database file %s\n", strFileRes);
   76                return fSuccess;
   77            }
   78        }
   79        UninterruptibleSleep(std::chrono::milliseconds{100});
   80    }
   81}
   

8. 
   achow101 commented at 11:32 pm on July 22, 2025: member

   ACK 2dfeb6668cb2e98e3dccf946af084e8a08e1fab5
9. achow101 merged this on Jul 22, 2025
10. achow101 closed this on Jul 22, 2025

    ---

Contributors
rkrux DrahtBot brunoerg achow101

Labels
Wallet