exchange

commit b8500e9ef92c9e65fbeb94cd04239f20465c9217
parent 5a60998f34ea19fb1433f63a8c3a6f7f67c134cc
Author: Christian Grothoff <christian@grothoff.org>
Date:   Thu,  6 Aug 2026 17:46:17 +0200

properly fail on commit after refund

Diffstat:
M
src/exchangedb/commit.c
 | 
33
++++++++++++++++++++++++++++++---
M
src/exchangedb/test_regressions.c
 | 
57
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++

2 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/src/exchangedb/commit.c b/src/exchangedb/commit.c
@@ -44,9 +44,36 @@ TALER_EXCHANGEDB_commit (struct TALER_EXCHANGEDB_PostgresContext *pg)
   PREPARE (pg,
            "commit",
            "COMMIT");
-  qs = GNUNET_PQ_eval_prepared_non_select (pg->conn,
-                                           "commit",
-                                           params);
+  {
+    PGresult *result;
+
+    result = GNUNET_PQ_exec_prepared (pg->conn,
+                                      "commit",
+                                      params);
+    qs = GNUNET_PQ_eval_result (pg->conn,
+                                "commit",
+                                result);
+    if ( (0 <= qs) &&
+         (NULL != result) &&
+         (0 == strcmp ("ROLLBACK",
+                       PQcmdStatus (result))) )
+    {
+      /* PostgreSQL accepts COMMIT on a transaction it has already aborted:
+         it silently rolls back and answers PGRES_COMMAND_OK with the command
+         tag ROLLBACK.  That is indistinguishable from a successful commit to
+         GNUNET_PQ_eval_result(), so every caller would take the success path
+         for a transaction whose writes are gone.  Report it as a soft error:
+         the transaction is almost always aborted by a serialization failure
+         (we run SERIALIZABLE), so retrying is the right answer, and callers
+         with a retry loop will give up and fail visibly after
+         MAX_RETRIES. */
+      GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                  "Transaction `%s' was rolled back by the database instead of committed; a previous statement must have failed without being handled\n",
+                  pg->transaction_name);
+      qs = GNUNET_DB_STATUS_SOFT_ERROR;
+    }
+    PQclear (result);
+  }
   pg->transaction_name = NULL;
   return qs;
 }
diff --git a/src/exchangedb/test_regressions.c b/src/exchangedb/test_regressions.c
@@ -28,6 +28,9 @@
 #include "exchange-database/create_tables.h"
 #include "exchange-database/do_aggregate.h"
 #include "exchange-database/compute_shard.h"
+#include "exchange-database/start.h"
+#include "exchange-database/commit.h"
+#include "exchange-database/rollback.h"
 
 
 /**
@@ -294,6 +297,58 @@ check_aggregate_refund_below_deposit_fee (void)
 
 
 /**
+ * E-13: PostgreSQL accepts COMMIT on a transaction it has already aborted,
+ * rolls it back and answers with the command tag ROLLBACK and no error at
+ * all.  TALER_EXCHANGEDB_commit() used to pass that straight through as
+ * GNUNET_DB_STATUS_SUCCESS_NO_RESULTS, i.e. every caller was told the
+ * transaction had committed while its writes were gone.
+ */
+static int
+check_commit_detects_rolled_back_transaction (void)
+{
+  /* A transaction that is aborted mid-way must NOT commit successfully. */
+  FAILIF (GNUNET_OK !=
+          TALER_EXCHANGEDB_start (pg,
+                                  "test-e13-aborted"));
+  FAILIF (GNUNET_OK !=
+          exec_sql ("INSERT INTO kyc_targets (h_normalized_payto)"
+                    " VALUES (decode(repeat('13',32),'hex'));"));
+  /* Provoke an error; from here on the transaction is doomed. */
+  FAILIF (GNUNET_OK ==
+          exec_sql ("SELECT 1/0;"));
+  /* Before the fix this returned SUCCESS_NO_RESULTS (0). */
+  FAILIF (0 <=
+          TALER_EXCHANGEDB_commit (pg));
+  /* ...and the row is indeed gone, so 'success' would have been a lie. */
+  FAILIF (GNUNET_OK !=
+          exec_sql ("DO $$ BEGIN"
+                    " IF EXISTS (SELECT FROM kyc_targets"
+                    "             WHERE h_normalized_payto"
+                    "                   =decode(repeat('13',32),'hex'))"
+                    " THEN RAISE EXCEPTION 'row survived a rollback';"
+                    " END IF; END $$;"));
+
+  /* A clean transaction must still commit and still return exactly 0. */
+  FAILIF (GNUNET_OK !=
+          TALER_EXCHANGEDB_start (pg,
+                                  "test-e13-clean"));
+  FAILIF (GNUNET_OK !=
+          exec_sql ("INSERT INTO kyc_targets (h_normalized_payto)"
+                    " VALUES (decode(repeat('14',32),'hex'));"));
+  FAILIF (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS !=
+          TALER_EXCHANGEDB_commit (pg));
+  FAILIF (GNUNET_OK !=
+          exec_sql ("DO $$ BEGIN"
+                    " IF NOT EXISTS (SELECT FROM kyc_targets"
+                    "                 WHERE h_normalized_payto"
+                    "                       =decode(repeat('14',32),'hex'))"
+                    " THEN RAISE EXCEPTION 'committed row is missing';"
+                    " END IF; END $$;"));
+  return 0;
+}
+
+
+/**
  * All checks we know about.
  */
 static const struct
@@ -303,6 +358,8 @@ static const struct
 } tests[] = {
   { "aggregate-refund-below-deposit-fee",
     &check_aggregate_refund_below_deposit_fee },
+  { "commit-detects-rolled-back-transaction",
+    &check_commit_detects_rolled_back_transaction },
   { NULL, NULL }
 };