db: don't start transactions unless we really need to.
What changed, and why it matters
This change is a performance optimization, not a security fix. It delays starting a database transaction until the software actually needs to write or read data, rather than starting one automatically at the beginning of every operation. The commit message explicitly frames this as a speed improvement, especially for PostgreSQL, and provides benchmark numbers. There is no indication of a vulnerability being fixed.
No security action required. Treat as a normal performance optimization during review; verify that read-only queries and migrations still behave correctly under the new lazy transaction model.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch introduces lazy transaction activation in Core Lightning’s database layer. It adds a transaction_started flag to the struct db, splits db_begin_transaction_() into logical transaction marking and actual backend transaction start, and adds db_need_transaction() which calls the backend’s begin_tx_fn only when a prepared statement is actually executed. db_commit_transaction() now skips the backend commit if no real transaction was started. The change touches SQLite3 table manipulation helpers to maintain the existing fake-transaction behavior during migrations.
Changed components
db/common.hdb/db_sqlite3.cdb/exec.cdb/exec.hdb/utils.cInspect captured patch +39 / −4
diff --git a/db/common.h b/db/common.h
index 829a3d2c..2533d7be 100644
--- a/db/common.h
+++ b/db/common.h
@@ -29,6 +29,8 @@
struct db {
char *filename;
const char *in_transaction;
+ /* For lazy transaction activation */
+ bool transaction_started;
/* DB-specific context */
void *conn;
diff --git a/db/db_sqlite3.c b/db/db_sqlite3.c
index 5df248b6..ed63989d 100644
--- a/db/db_sqlite3.c
+++ b/db/db_sqlite3.c
@@ -474,12 +474,14 @@ static char **prepare_table_manip(const tal_t *ctx,
/* But core insists we're "in a transaction" for all ops, so fake it */
db->in_transaction = "Not really";
+ db->transaction_started = true;
/* Turn off foreign keys first. */
db_prepare_for_changes(db);
db_exec_prepared_v2(take(db_prepare_untranslated(db,
"PRAGMA foreign_keys = OFF;")));
db_report_changes(db, NULL, 0);
db->in_transaction = NULL;
+ db->transaction_started = false;
db_begin_transaction(db);
cmd = tal_fmt(tmpctx, "ALTER TABLE %s RENAME TO temp_%s;",
@@ -525,11 +527,13 @@ static bool complete_table_manip(struct db *db,
/* Allow links between them (esp. cascade deletes!) */
db->in_transaction = "Not really";
+ db->transaction_started = true;
db_prepare_for_changes(db);
db_exec_prepared_v2(take(db_prepare_untranslated(db,
"PRAGMA foreign_keys = ON;")));
db_report_changes(db, NULL, 0);
db->in_transaction = NULL;
+ db->transaction_started = false;
/* migrations are performed inside transactions, so start one. */
db_begin_transaction(db);
diff --git a/db/exec.c b/db/exec.c
index 5ab6e9b0..382ad9f1 100644
--- a/db/exec.c
+++ b/db/exec.c
@@ -121,19 +121,29 @@ static void db_data_version_incr(struct db *db)
void db_begin_transaction_(struct db *db, const char *location)
{
- bool ok;
if (db->in_transaction)
db_fatal(db, "Already in transaction from %s", db->in_transaction);
+ db->in_transaction = location;
/* No writes yet. */
db->dirty = false;
+}
+
+void db_need_transaction(struct db *db, const char *location)
+{
+ bool ok;
+
+ if (!db->in_transaction)
+ db_fatal(db, "Not in a transaction for %s", location);
+
+ if (db->transaction_started)
+ return;
db_prepare_for_changes(db);
ok = db->config->begin_tx_fn(db);
if (!ok)
db_fatal(db, "Failed to start DB transaction: %s", db->error);
-
- db->in_transaction = location;
+ db->transaction_started = true;
}
bool db_in_transaction(struct db *db)
@@ -150,6 +160,13 @@ void db_commit_transaction(struct db *db)
{
bool ok;
assert(db->in_transaction);
+
+ if (!db->transaction_started) {
+ db->in_transaction = NULL;
+ assert(!db->dirty);
+ return;
+ }
+
db_assert_no_outstanding_statements(db);
/* Increment before reporting changes to an eventual plugin. */
@@ -164,4 +181,5 @@ void db_commit_transaction(struct db *db)
db->in_transaction = NULL;
db->dirty = false;
+ db->transaction_started = false;
}
diff --git a/db/exec.h b/db/exec.h
index d923ae0f..c852d950 100644
--- a/db/exec.h
+++ b/db/exec.h
@@ -40,6 +40,11 @@ void db_begin_transaction_(struct db *db, const char *location);
bool db_in_transaction(struct db *db);
+/**
+ * db_need_transaction: we now need to actually enable the transaction, if not
+ * already. */
+void db_need_transaction(struct db *db, const char *location);
+
/**
* db_commit_transaction - Commit a running transaction
*
diff --git a/db/utils.c b/db/utils.c
index 25bce2f4..8c478189 100644
--- a/db/utils.c
+++ b/db/utils.c
@@ -3,6 +3,7 @@
#include <ccan/tal/str/str.h>
#include <common/trace.h>
#include <db/common.h>
+#include <db/exec.h>
#include <db/utils.h>
/* Matches the hash function used in devtools/sql-rewrite.py */
@@ -143,6 +144,7 @@ bool db_query_prepared_canfail(struct db_stmt *stmt)
assert(stmt->query->readonly);
trace_span_start("db_query_prepared", stmt);
trace_span_tag(stmt, "query", stmt->query->query);
+ db_need_transaction(stmt->db, stmt->query->query);
ret = stmt->db->config->query_fn(stmt);
stmt->executed = true;
list_del_from(&stmt->db->pending_statements, &stmt->list);
@@ -174,9 +176,12 @@ bool db_step(struct db_stmt *stmt)
void db_exec_prepared_v2(struct db_stmt *stmt TAKES)
{
+ bool ret;
+
+ db_need_transaction(stmt->db, stmt->query->query);
trace_span_start("db_exec_prepared", stmt);
trace_span_tag(stmt, "query", stmt->query->query);
- bool ret = stmt->db->config->exec_fn(stmt);
+ ret = stmt->db->config->exec_fn(stmt);
trace_span_end(stmt);
if (stmt->db->readonly)
@@ -358,6 +363,7 @@ struct db *db_open_(const tal_t *ctx, const char *filename,
db_fatal(db, "Unable to find DB queries for %s", db->config->name);
db->in_transaction = NULL;
+ db->transaction_started = false;
db->changes = NULL;
/* This must be outside a transaction, so catch it */
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.