v29.4-roots.1 · Commit 4: [PATCH 04/13] wallet: harden database and signing behavior

src/wallet/sqlite.cpp

Before: src/wallet/sqlite.cpp · After: src/wallet/sqlite.cpp

File metadata

index 896a2fc0f33fdf8e93ec97b6b2f723322b28f973..34429fb34b471e3ce9035223d5741141912c36ce 100644

Download original patch · Release notes

Diff rows 1–156 of 156. Line numbers refer to the original before/after files.

+ Added− RemovedUnmarked lines are context.

Scroll the code area horizontally to read long lines. Before is on the left; after is on the right. Unified view fits narrow screens more easily.

src/wallet/sqlite.cpp CHANGED Viewed

@@ -40,7 +40,7 @@ static void ErrorLogCallback(void* arg, int code, const char* msg)
     // invoked."
     // Assert that this is the case:
     assert(arg == nullptr);
-    LogPrintf("SQLite Error. Code: %d. Message: %s\n", code, msg);
 }
 static int TraceSqlCallback(unsigned code, void* context, void* param1, void* param2)
@@ -69,7 +69,7 @@ static bool BindBlobToStatement(sqlite3_stmt* stmt,
     // instead of the empty blob value X'', which would mess up SQL comparisons.
     int res = sqlite3_bind_blob(stmt, index, blob.data() ? static_cast<const void*>(blob.data()) : "", blob.size(), SQLITE_STATIC);
     if (res != SQLITE_OK) {
-        LogPrintf("Unable to bind %s to statement: %s\n", description, sqlite3_errstr(res));
         sqlite3_clear_bindings(stmt);
         sqlite3_reset(stmt);
         return false;
@@ -182,7 +182,7 @@ void SQLiteDatabase::Cleanup() noexcept
     if (--g_sqlite_count == 0) {
         int ret = sqlite3_shutdown();
         if (ret != SQLITE_OK) {
-            LogPrintf("SQLiteDatabase: Failed to shutdown SQLite: %s\n", sqlite3_errstr(ret));
         }
     }
 }
@@ -254,7 +254,11 @@ void SQLiteDatabase::Open()
     if (m_db == nullptr) {
         if (!m_mock) {
             TryCreateDirectories(m_dir_path);
         }
         int ret = sqlite3_open_v2(m_file_path.c_str(), &m_db, flags, nullptr);
         if (ret != SQLITE_OK) {
             throw std::runtime_error(strprintf("SQLiteDatabase: Failed to open database: %s\n", sqlite3_errstr(ret)));
@@ -267,7 +271,7 @@ void SQLiteDatabase::Open()
         if (LogAcceptCategory(BCLog::WALLETDB, BCLog::Level::Trace)) {
            ret = sqlite3_trace_v2(m_db, SQLITE_TRACE_STMT, TraceSqlCallback, this);
            if (ret != SQLITE_OK) {
-               LogPrintf("Failed to enable SQL tracing for %s\n", Filename());
            }
         }
     }
@@ -294,7 +298,7 @@ void SQLiteDatabase::Open()
     if (m_use_unsafe_sync) {
         // Use normal synchronous mode for the journal
-        LogPrintf("WARNING SQLite is configured to not wait for data to be flushed to disk. Data loss and corruption may occur.\n");
         SetPragma(m_db, "synchronous", "OFF", "Failed to set synchronous mode to OFF");
     }
@@ -353,14 +357,14 @@ bool SQLiteDatabase::Backup(const std::string& dest) const
     }
     sqlite3_backup* backup = sqlite3_backup_init(db_copy, "main", m_db, "main");
     if (!backup) {
-        LogPrintf("%s: Unable to begin backup: %s\n", __func__, sqlite3_errmsg(m_db));
         sqlite3_close(db_copy);
         return false;
     }
     // Specifying -1 will copy all of the pages
     res = sqlite3_backup_step(backup, -1);
     if (res != SQLITE_DONE) {
-        LogPrintf("%s: Unable to backup: %s\n", __func__, sqlite3_errstr(res));
         sqlite3_backup_finish(backup);
         sqlite3_close(db_copy);
         return false;
@@ -412,13 +416,13 @@ void SQLiteBatch::Close()
     // If we began a transaction, and it wasn't committed, abort the transaction in progress
     if (m_txn) {
         if (TxnAbort()) {
-            LogPrintf("SQLiteBatch: Batch closed unexpectedly without the transaction being explicitly committed or aborted\n");
         } else {
             // If transaction cannot be aborted, it means there is a bug or there has been data corruption. Try to recover in this case
             // by closing and reopening the database. Closing the database should also ensure that any changes made since the transaction
             // was opened will be rolled back and future transactions can succeed without committing old data.
             force_conn_refresh = true;
-            LogPrintf("SQLiteBatch: Batch closed and failed to abort transaction, resetting db connection..\n");
         }
     }
@@ -434,7 +438,7 @@ void SQLiteBatch::Close()
     for (const auto& [stmt_prepared, stmt_description] : statements) {
         int res = sqlite3_finalize(*stmt_prepared);
         if (res != SQLITE_OK) {
-            LogPrintf("SQLiteBatch: Batch closed but could not finalize %s statement: %s\n",
                       stmt_description, sqlite3_errstr(res));
         }
         *stmt_prepared = nullptr;
@@ -465,7 +469,7 @@ bool SQLiteBatch::ReadKey(DataStream&& key, DataStream& value)
     if (res != SQLITE_ROW) {
         if (res != SQLITE_DONE) {
             // SQLITE_DONE means "not found", don't log an error in that case.
-            LogPrintf("%s: Unable to execute statement: %s\n", __func__, sqlite3_errstr(res));
         }
         sqlite3_clear_bindings(m_read_stmt);
         sqlite3_reset(m_read_stmt);
@@ -505,7 +509,7 @@ bool SQLiteBatch::WriteKey(DataStream&& key, DataStream&& value, bool overwrite)
     sqlite3_clear_bindings(stmt);
     sqlite3_reset(stmt);
     if (res != SQLITE_DONE) {
-        LogPrintf("%s: Unable to execute statement: %s\n", __func__, sqlite3_errstr(res));
     }
     if (!m_txn) m_database.m_write_semaphore.post();
@@ -529,7 +533,7 @@ bool SQLiteBatch::ExecStatement(sqlite3_stmt* stmt, Span<const std::byte> blob)
     sqlite3_clear_bindings(stmt);
     sqlite3_reset(stmt);
     if (res != SQLITE_DONE) {
-        LogPrintf("%s: Unable to execute statement: %s\n", __func__, sqlite3_errstr(res));
     }
     if (!m_txn) m_database.m_write_semaphore.post();
@@ -567,7 +571,7 @@ DatabaseCursor::Status SQLiteCursor::Next(DataStream& key, DataStream& value)
         return Status::DONE;
     }
     if (res != SQLITE_ROW) {
-        LogPrintf("%s: Unable to execute cursor step: %s\n", __func__, sqlite3_errstr(res));
         return Status::FAIL;
     }
@@ -586,8 +590,8 @@ SQLiteCursor::~SQLiteCursor()
     sqlite3_reset(m_cursor_stmt);
     int res = sqlite3_finalize(m_cursor_stmt);
     if (res != SQLITE_OK) {
-        LogPrintf("%s: cursor closed but could not finalize cursor statement: %s\n",
-                  __func__, sqlite3_errstr(res));
     }
 }
@@ -655,7 +659,7 @@ bool SQLiteBatch::TxnBegin()
     Assert(!m_database.HasActiveTxn());
     int res = Assert(m_exec_handler)->Exec(m_database, "BEGIN TRANSACTION");
     if (res != SQLITE_OK) {
-        LogPrintf("SQLiteBatch: Failed to begin the transaction\n");
         m_database.m_write_semaphore.post();
     } else {
         m_txn = true;
@@ -669,7 +673,7 @@ bool SQLiteBatch::TxnCommit()
     Assert(m_database.HasActiveTxn());
     int res = Assert(m_exec_handler)->Exec(m_database, "COMMIT TRANSACTION");
     if (res != SQLITE_OK) {
-        LogPrintf("SQLiteBatch: Failed to commit the transaction\n");
     } else {
         m_txn = false;
         m_database.m_write_semaphore.post();
@@ -683,7 +687,7 @@ bool SQLiteBatch::TxnAbort()
     Assert(m_database.HasActiveTxn());
     int res = Assert(m_exec_handler)->Exec(m_database, "ROLLBACK TRANSACTION");
     if (res != SQLITE_OK) {
-        LogPrintf("SQLiteBatch: Failed to abort the transaction\n");
     } else {
         m_txn = false;
         m_database.m_write_semaphore.post();

     // invoked."
     // Assert that this is the case:
     assert(arg == nullptr);
+    LogWarning("SQLite Error. Code: %d. Message: %s", code, msg);
 }
 static int TraceSqlCallback(unsigned code, void* context, void* param1, void* param2)
     // instead of the empty blob value X'', which would mess up SQL comparisons.
     int res = sqlite3_bind_blob(stmt, index, blob.data() ? static_cast<const void*>(blob.data()) : "", blob.size(), SQLITE_STATIC);
     if (res != SQLITE_OK) {
+        LogWarning("Unable to bind %s to statement: %s", description, sqlite3_errstr(res));
         sqlite3_clear_bindings(stmt);
         sqlite3_reset(stmt);
         return false;
     if (--g_sqlite_count == 0) {
         int ret = sqlite3_shutdown();
         if (ret != SQLITE_OK) {
+            LogWarning("SQLiteDatabase: Failed to shutdown SQLite: %s", sqlite3_errstr(ret));
         }
     }
 }
     if (m_db == nullptr) {
         if (!m_mock) {
             TryCreateDirectories(m_dir_path);
+            if (!IsDirWritable(m_dir_path)) {
+                throw std::runtime_error(strprintf("SQLiteDatabase: Failed to open database in directory '%s': directory is not writable", fs::PathToString(m_dir_path)));
+            }
         }
         int ret = sqlite3_open_v2(m_file_path.c_str(), &m_db, flags, nullptr);
         if (ret != SQLITE_OK) {
             throw std::runtime_error(strprintf("SQLiteDatabase: Failed to open database: %s\n", sqlite3_errstr(ret)));
         if (LogAcceptCategory(BCLog::WALLETDB, BCLog::Level::Trace)) {
            ret = sqlite3_trace_v2(m_db, SQLITE_TRACE_STMT, TraceSqlCallback, this);
            if (ret != SQLITE_OK) {
+               LogWarning("Failed to enable SQL tracing for %s", Filename());
            }
         }
     }
     if (m_use_unsafe_sync) {
         // Use normal synchronous mode for the journal
+        LogWarning("SQLite is configured to not wait for data to be flushed to disk. Data loss and corruption may occur.");
         SetPragma(m_db, "synchronous", "OFF", "Failed to set synchronous mode to OFF");
     }
     }
     sqlite3_backup* backup = sqlite3_backup_init(db_copy, "main", m_db, "main");
     if (!backup) {
+        LogWarning("Unable to begin sqlite backup: %s", sqlite3_errmsg(m_db));
         sqlite3_close(db_copy);
         return false;
     }
     // Specifying -1 will copy all of the pages
     res = sqlite3_backup_step(backup, -1);
     if (res != SQLITE_DONE) {
+        LogWarning("Unable to continue sqlite backup: %s", sqlite3_errstr(res));
         sqlite3_backup_finish(backup);
         sqlite3_close(db_copy);
         return false;
     // If we began a transaction, and it wasn't committed, abort the transaction in progress
     if (m_txn) {
         if (TxnAbort()) {
+            LogWarning("SQLiteBatch: Batch closed unexpectedly without the transaction being explicitly committed or aborted");
         } else {
             // If transaction cannot be aborted, it means there is a bug or there has been data corruption. Try to recover in this case
             // by closing and reopening the database. Closing the database should also ensure that any changes made since the transaction
             // was opened will be rolled back and future transactions can succeed without committing old data.
             force_conn_refresh = true;
+            LogWarning("SQLiteBatch: Batch closed and failed to abort transaction, resetting db connection..");
         }
     }
     for (const auto& [stmt_prepared, stmt_description] : statements) {
         int res = sqlite3_finalize(*stmt_prepared);
         if (res != SQLITE_OK) {
+            LogWarning("SQLiteBatch: Batch closed but could not finalize %s statement: %s",
                       stmt_description, sqlite3_errstr(res));
         }
         *stmt_prepared = nullptr;
     if (res != SQLITE_ROW) {
         if (res != SQLITE_DONE) {
             // SQLITE_DONE means "not found", don't log an error in that case.
+            LogWarning("Unable to execute read statement: %s", sqlite3_errstr(res));
         }
         sqlite3_clear_bindings(m_read_stmt);
         sqlite3_reset(m_read_stmt);
     sqlite3_clear_bindings(stmt);
     sqlite3_reset(stmt);
     if (res != SQLITE_DONE) {
+        LogWarning("Unable to execute write statement: %s", sqlite3_errstr(res));
     }
     if (!m_txn) m_database.m_write_semaphore.post();
     sqlite3_clear_bindings(stmt);
     sqlite3_reset(stmt);
     if (res != SQLITE_DONE) {
+        LogWarning("Unable to execute exec statement: %s", sqlite3_errstr(res));
     }
     if (!m_txn) m_database.m_write_semaphore.post();
         return Status::DONE;
     }
     if (res != SQLITE_ROW) {
+        LogWarning("Unable to execute cursor step: %s", sqlite3_errstr(res));
         return Status::FAIL;
     }
     sqlite3_reset(m_cursor_stmt);
     int res = sqlite3_finalize(m_cursor_stmt);
     if (res != SQLITE_OK) {
+        LogWarning("Cursor closed but could not finalize cursor statement: %s",
+                   sqlite3_errstr(res));
     }
 }
     Assert(!m_database.HasActiveTxn());
     int res = Assert(m_exec_handler)->Exec(m_database, "BEGIN TRANSACTION");
     if (res != SQLITE_OK) {
+        LogWarning("SQLiteBatch: Failed to begin the transaction");
         m_database.m_write_semaphore.post();
     } else {
         m_txn = true;
     Assert(m_database.HasActiveTxn());
     int res = Assert(m_exec_handler)->Exec(m_database, "COMMIT TRANSACTION");
     if (res != SQLITE_OK) {
+        LogWarning("SQLiteBatch: Failed to commit the transaction");
     } else {
         m_txn = false;
         m_database.m_write_semaphore.post();
     Assert(m_database.HasActiveTxn());
     int res = Assert(m_exec_handler)->Exec(m_database, "ROLLBACK TRANSACTION");
     if (res != SQLITE_OK) {
+        LogWarning("SQLiteBatch: Failed to abort the transaction");
     } else {
         m_txn = false;
         m_database.m_write_semaphore.post();