Fix #5010 for keystate

src/exchange/taler-exchange-httpd_db.c

@@ -54,12 +54,14 @@ TEH_DB_run_transaction (struct MHD_Connection *connection,
 {
   struct TALER_EXCHANGEDB_Session *session;
 
-  *mhd_ret = -1; /* invalid value */
+  if (NULL != mhd_ret)
+    *mhd_ret = -1; /* invalid value */
   if (NULL == (session = TEH_plugin->get_session (TEH_plugin->cls)))
   {
     GNUNET_break (0);
-    *mhd_ret = TEH_RESPONSE_reply_internal_db_error (connection,
+    if (NULL != mhd_ret)
-						     TALER_EC_DB_SETUP_FAILED);
+      *mhd_ret = TEH_RESPONSE_reply_internal_db_error (connection,
+						       TALER_EC_DB_SETUP_FAILED);
     return GNUNET_SYSERR;
   }
   for (unsigned int retries = 0;retries < MAX_TRANSACTION_COMMIT_RETRIES; retries++)
@@ -71,8 +73,9 @@ TEH_DB_run_transaction (struct MHD_Connection *connection,
 			   session))                            
     {                                      
       GNUNET_break (0);
-      *mhd_ret = TEH_RESPONSE_reply_internal_db_error (connection, 
+      if (NULL != mhd_ret)
-						       TALER_EC_DB_START_FAILED);
+	*mhd_ret = TEH_RESPONSE_reply_internal_db_error (connection, 
+							 TALER_EC_DB_START_FAILED);
       return GNUNET_SYSERR;
     }
     qs = cb (cb_cls,
@@ -89,19 +92,22 @@ TEH_DB_run_transaction (struct MHD_Connection *connection,
 			       session);                              
     if (GNUNET_DB_STATUS_HARD_ERROR == qs)
     {
-      *mhd_ret = TEH_RESPONSE_reply_commit_error (connection,
+      if (NULL != mhd_ret)
-						  TALER_EC_DB_COMMIT_FAILED_HARD);
+	*mhd_ret = TEH_RESPONSE_reply_commit_error (connection,
+						    TALER_EC_DB_COMMIT_FAILED_HARD);
       return GNUNET_SYSERR;
     }
     /* make sure callback did not violate invariants! */
-    GNUNET_assert (-1 == *mhd_ret);
+    GNUNET_assert ( (NULL == mhd_ret) ||
+		    (-1 == *mhd_ret) );
     if (0 <= qs)
       return GNUNET_OK;
   }
   TALER_LOG_WARNING ("Transaction commit failed %u times\n",
 		     MAX_TRANSACTION_COMMIT_RETRIES);
-  *mhd_ret = TEH_RESPONSE_reply_commit_error (connection,
+  if (NULL != mhd_ret)
-					      TALER_EC_DB_COMMIT_FAILED_ON_RETRY);
+    *mhd_ret = TEH_RESPONSE_reply_commit_error (connection,
+						TALER_EC_DB_COMMIT_FAILED_ON_RETRY);
   return GNUNET_SYSERR;
 }
 

src/exchange/taler-exchange-httpd_keystate.c

@@ -307,6 +307,81 @@ handle_signal (int signal_number)
 }
 
 
+/**
+ * Closure for #add_revocations_transaction().
+ */
+struct AddRevocationContext
+{
+  /**
+   * Denomination key that is revoked.
+   */
+  const struct TALER_EXCHANGEDB_DenominationKeyIssueInformation *dki;
+
+  /**
+   * Signature affirming the revocation.
+   */
+  const struct TALER_MasterSignatureP *revocation_master_sig;
+};
+
+
+/**
+ * Execute transaction to add revocations.  
+ *
+ * @param cls closure with the `struct AddRevocationContext *`
+ * @param connection NULL
+ * @param session database session to use
+ * @param[out] mhd_ret NULL
+ * @return transaction status
+ */
+static enum GNUNET_DB_QueryStatus
+add_revocations_transaction (void *cls,
+			     struct MHD_Connection *connection,
+			     struct TALER_EXCHANGEDB_Session *session,
+			     int *mhd_ret)
+{
+  struct AddRevocationContext *arc = cls;
+  
+  return TEH_plugin->insert_denomination_revocation (TEH_plugin->cls,
+						     session,
+						     &arc->dki->issue.properties.denom_hash,
+						     arc->revocation_master_sig);
+}
+
+
+/**
+ * Execute transaction to add a denomination to the DB.  
+ *
+ * @param cls closure with the `const struct TALER_EXCHANGEDB_DenominationKeyIssueInformation *`
+ * @param connection NULL
+ * @param session database session to use
+ * @param[out] mhd_ret NULL
+ * @return transaction status
+ */
+static enum GNUNET_DB_QueryStatus
+add_denomination_transaction (void *cls,
+			      struct MHD_Connection *connection,
+			      struct TALER_EXCHANGEDB_Session *session,
+			      int *mhd_ret)
+{
+  const struct TALER_EXCHANGEDB_DenominationKeyIssueInformation *dki = cls;
+  enum GNUNET_DB_QueryStatus qs;
+  struct TALER_EXCHANGEDB_DenominationKeyInformationP issue_exists;
+
+  qs = TEH_plugin->get_denomination_info (TEH_plugin->cls,
+					  session,
+					  &dki->denom_pub,
+					  &issue_exists);
+  if (0 > qs)
+    return qs;
+  if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT == qs)
+    return qs;
+  return TEH_plugin->insert_denomination_info (TEH_plugin->cls,
+					       session,
+					       &dki->denom_pub,
+					       &dki->issue);
+}
+
+
 /**
  * Iterator for (re)loading/initializing denomination keys.
  *
@@ -330,10 +405,7 @@ reload_keys_denom_iter (void *cls,
   struct GNUNET_TIME_Absolute horizon;
   struct GNUNET_TIME_Absolute expire_deposit;
   struct GNUNET_HashCode denom_key_hash;
-  struct TALER_EXCHANGEDB_Session *session;
-  unsigned int thresh;
   int res;
-  enum GNUNET_DB_QueryStatus qs;
 
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "Loading denomination key `%s'\n",
@@ -357,14 +429,12 @@ reload_keys_denom_iter (void *cls,
     return GNUNET_OK;
   }
 
-  session = TEH_plugin->get_session (TEH_plugin->cls);
-  if (NULL == session)
-    return GNUNET_SYSERR;
-
   if (NULL != revocation_master_sig)
   {
+    struct AddRevocationContext arc;
+    
     GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-                "Adding denomination key `%s' to revokation set\n",
+                "Adding denomination key `%s' to revocation set\n",
                 alias);
     res = store_in_map (ctx->revoked_map,
                         dki);
@@ -373,47 +443,20 @@ reload_keys_denom_iter (void *cls,
     /* Try to insert DKI into DB until we succeed; note that if the DB
        failure is persistent, we need to die, as we cannot continue
        without the DKI being in the DB). */
-    thresh = 0;
+    arc.dki = dki;
-    qs = GNUNET_DB_STATUS_SOFT_ERROR;
+    arc.revocation_master_sig = revocation_master_sig;
-    while (0 > qs)
+    if (GNUNET_OK !=
+	TEH_DB_run_transaction (NULL,
+				NULL,
+				&add_revocations_transaction,
+				&arc))
     {
-      thresh++;
+      GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-      if ( (thresh > 16) ||
+		  "Giving up, this is fatal. Committing suicide via SIGTERM.\n");
-           (GNUNET_DB_STATUS_HARD_ERROR == qs) )
+      handle_signal (SIGTERM);
-      {
+      return GNUNET_SYSERR;      
-        GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-                    "Giving up, this is fatal. Committing suicide via SIGTERM.\n");
-        handle_signal (SIGTERM);
-        return GNUNET_SYSERR;
-      }
-      res = TEH_plugin->start (TEH_plugin->cls,
-                               session);
-      if (GNUNET_OK != res)
-      {
-        /* Transaction start failed!? Very bad error, log and retry */
-        GNUNET_break (0);
-        continue;
-      }
-      res = TEH_plugin->insert_denomination_revocation (TEH_plugin->cls,
-                                                        session,
-                                                        &dki->issue.properties.denom_hash,
-                                                        revocation_master_sig);
-      if (GNUNET_SYSERR == res)
-      {
-        GNUNET_break (0);
-        TEH_plugin->rollback (TEH_plugin->cls,
-                              session);
-        continue;
-      }
-      if (GNUNET_NO == res)
-      {
-        TEH_plugin->rollback (TEH_plugin->cls,
-                              session);
-        break; /* already in is also OK! */
-      }
-      qs = TEH_plugin->commit (TEH_plugin->cls,
-                               session);
     }
+	
     GNUNET_assert (0 ==
                    json_array_append_new (ctx->payback_array,
                                           GNUNET_JSON_from_data_auto (&dki->issue.properties.denom_hash)));
@@ -437,73 +480,16 @@ reload_keys_denom_iter (void *cls,
                                    sizeof (struct GNUNET_HashCode));
 
 
-
+  if (GNUNET_OK !=
-  session = TEH_plugin->get_session (TEH_plugin->cls);
+      TEH_DB_run_transaction (NULL,
-  if (NULL == session)
+			      NULL,
-    return GNUNET_SYSERR;
+			      &add_denomination_transaction,
-  /* Try to insert DKI into DB until we succeed; note that if the DB
+			      (void *) dki))
-     failure is persistent, we die, as we cannot continue without the
-     DKI being in the DB). */
-  qs = GNUNET_DB_STATUS_SOFT_ERROR;
-  thresh = 0;
-  while (0 > qs)
   {
-    thresh++;
+    GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-    if ( (thresh > 16) ||
+		"Giving up, this is fatal. Committing suicide via SIGTERM.\n");
-         (GNUNET_DB_STATUS_HARD_ERROR == qs) )
+    handle_signal (SIGTERM);
-    {
+    return GNUNET_SYSERR;      
-      GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-                  "Giving up, this is fatal. Committing suicide via SIGTERM.\n");
-      handle_signal (SIGTERM);
-      return GNUNET_SYSERR;
-    }
-
-    res = TEH_plugin->start (TEH_plugin->cls,
-                             session);
-    if (GNUNET_OK != res)
-    {
-      /* Transaction start failed!? Very bad error, log and retry */
-      GNUNET_break (0);
-      continue;
-    }
-    res = TEH_plugin->get_denomination_info (TEH_plugin->cls,
-                                             session,
-                                             &dki->denom_pub,
-                                             NULL);
-    if (GNUNET_SYSERR == res)
-    {
-      /* Fetch failed!? Very bad error, log and retry */
-      GNUNET_break (0);
-      TEH_plugin->rollback (TEH_plugin->cls,
-                            session);
-      continue;
-    }
-    if (GNUNET_OK == res)
-    {
-      /* Record exists, we're good, just exit */
-      TEH_plugin->rollback (TEH_plugin->cls,
-                            session);
-      break;
-    }
-    qs = TEH_plugin->insert_denomination_info (TEH_plugin->cls,
-					       session,
-					       &dki->denom_pub,
-					       &dki->issue);
-    if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs)
-    {
-      /* Insert failed!? Very bad error, log and retry */
-      GNUNET_break (0);
-      TEH_plugin->rollback (TEH_plugin->cls,
-                            session);
-      continue;
-    }
-    qs = TEH_plugin->commit (TEH_plugin->cls,
-                             session);
-    /* If commit succeeded, we're done, otherwise we retry; this
-       time without logging, as theroetically commits can fail
-       in a transactional DB due to concurrent activities that
-       cannot be reconciled. This should be rare for DKIs, but
-       as it is possible we just retry until we succeed. */
   }
 
   res = store_in_map (ctx->denomkey_map,

src/exchangedb/perf_taler_exchangedb_interpreter.c

@@ -1465,16 +1465,16 @@ interpret (struct PERF_TALER_EXCHANGEDB_interpreter_state *state)
       case PERF_TALER_EXCHANGEDB_CMD_GET_DENOMINATION:
         {
           unsigned int denom_index;
-          int ret;
+          enum GNUNET_DB_QueryStatus qs;
           struct PERF_TALER_EXCHANGEDB_Data *data;
 
           denom_index = state->cmd[state->i].details.get_denomination.index_denom;
           data = &state->cmd[denom_index].exposed;
-          ret = state->plugin->get_denomination_info (state->plugin->cls,
+          qs = state->plugin->get_denomination_info (state->plugin->cls,
-                                                      state->session,
+						     state->session,
-                                                      &data->data.dki->denom_pub,
+						     &data->data.dki->denom_pub,
-                                                      &data->data.dki->issue);
+						     &data->data.dki->issue);
-          GNUNET_assert (GNUNET_SYSERR != ret);
+          GNUNET_assert (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT == qs);
         }
         break;
 

src/exchangedb/plugin_exchangedb_postgres.c

@@ -1826,89 +1826,57 @@ postgres_insert_denomination_info (void *cls,
  * @param cls the @e cls of this struct with the plugin-specific state
  * @param session connection to use
  * @param denom_pub the public key used for signing coins of this denomination
- * @param[out] issue set to issue information with value, fees and other info about the coin, can be NULL
+ * @param[out] issue set to issue information with value, fees and other info about the coin
- * @return #GNUNET_OK on success; #GNUNET_NO if no record was found, #GNUNET_SYSERR on failure
+ * @return transaction status code
  */
-static int
+static enum GNUNET_DB_QueryStatus
 postgres_get_denomination_info (void *cls,
                                 struct TALER_EXCHANGEDB_Session *session,
                                 const struct TALER_DenominationPublicKey *denom_pub,
                                 struct TALER_EXCHANGEDB_DenominationKeyInformationP *issue)
 {
-  PGresult *result;
+  enum GNUNET_DB_QueryStatus qs;
   struct GNUNET_PQ_QueryParam params[] = {
     GNUNET_PQ_query_param_rsa_public_key (denom_pub->rsa_public_key),
     GNUNET_PQ_query_param_end
   };
+  struct GNUNET_PQ_ResultSpec rs[] = {
+    GNUNET_PQ_result_spec_auto_from_type ("master_pub",
+					  &issue->properties.master),
+    GNUNET_PQ_result_spec_auto_from_type ("master_sig",
+					  &issue->signature),
+    GNUNET_PQ_result_spec_absolute_time_nbo ("valid_from",
+					     &issue->properties.start),
+    GNUNET_PQ_result_spec_absolute_time_nbo ("expire_withdraw",
+					     &issue->properties.expire_withdraw),
+    GNUNET_PQ_result_spec_absolute_time_nbo ("expire_deposit",
+					     &issue->properties.expire_deposit),
+    GNUNET_PQ_result_spec_absolute_time_nbo ("expire_legal",
+					     &issue->properties.expire_legal),
+    TALER_PQ_result_spec_amount_nbo ("coin",
+				     &issue->properties.value),
+    TALER_PQ_result_spec_amount_nbo ("fee_withdraw",
+				     &issue->properties.fee_withdraw),
+    TALER_PQ_result_spec_amount_nbo ("fee_deposit",
+				     &issue->properties.fee_deposit),
+    TALER_PQ_result_spec_amount_nbo ("fee_refresh",
+				     &issue->properties.fee_refresh),
+    TALER_PQ_result_spec_amount_nbo ("fee_refund",
+				     &issue->properties.fee_refund),
+    GNUNET_PQ_result_spec_end
+  };
 
-  result = GNUNET_PQ_exec_prepared (session->conn,
+  qs = GNUNET_PQ_eval_prepared_singleton_select (session->conn,
-                                    "denomination_get",
+						 "denomination_get",
-                                    params);
+						 params,
-  if (PGRES_TUPLES_OK != PQresultStatus (result))
+						 rs);
-  {
+  if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs)
-    QUERY_ERR (result,
+    return qs;
-               session->conn);
-    PQclear (result);
-    return GNUNET_SYSERR;
-  }
-  if (0 == PQntuples (result))
-  {
-    PQclear (result);
-    return GNUNET_NO;
-  }
-  if (1 != PQntuples (result))
-  {
-    GNUNET_break (0);
-    PQclear (result);
-    return GNUNET_SYSERR;
-  }
-  if (NULL == issue)
-  {
-    PQclear (result);
-    return GNUNET_OK;
-  }
-  {
-    struct GNUNET_PQ_ResultSpec rs[] = {
-      GNUNET_PQ_result_spec_auto_from_type ("master_pub",
-                                            &issue->properties.master),
-      GNUNET_PQ_result_spec_auto_from_type ("master_sig",
-                                            &issue->signature),
-      GNUNET_PQ_result_spec_absolute_time_nbo ("valid_from",
-                                               &issue->properties.start),
-      GNUNET_PQ_result_spec_absolute_time_nbo ("expire_withdraw",
-                                               &issue->properties.expire_withdraw),
-      GNUNET_PQ_result_spec_absolute_time_nbo ("expire_deposit",
-                                               &issue->properties.expire_deposit),
-      GNUNET_PQ_result_spec_absolute_time_nbo ("expire_legal",
-                                               &issue->properties.expire_legal),
-      TALER_PQ_result_spec_amount_nbo ("coin",
-                                       &issue->properties.value),
-      TALER_PQ_result_spec_amount_nbo ("fee_withdraw",
-                                       &issue->properties.fee_withdraw),
-      TALER_PQ_result_spec_amount_nbo ("fee_deposit",
-                                       &issue->properties.fee_deposit),
-      TALER_PQ_result_spec_amount_nbo ("fee_refresh",
-                                       &issue->properties.fee_refresh),
-      TALER_PQ_result_spec_amount_nbo ("fee_refund",
-                                       &issue->properties.fee_refund),
-      GNUNET_PQ_result_spec_end
-    };
-
-    if (GNUNET_OK !=
-	GNUNET_PQ_extract_result (result,
-				  rs,
-				  0))
-    {
-      PQclear (result);
-      return GNUNET_SYSERR;
-    }
-  }
-  PQclear (result);
   issue->properties.purpose.size = htonl (sizeof (struct TALER_DenominationKeyValidityPS));
   issue->properties.purpose.purpose = htonl (TALER_SIGNATURE_MASTER_DENOMINATION_KEY_VALIDITY);
   GNUNET_CRYPTO_rsa_public_key_hash (denom_pub->rsa_public_key,
                                      &issue->properties.denom_hash);
-  return GNUNET_OK;
+  return qs;
 }
 
 
@@ -6173,54 +6141,23 @@ postgres_get_reserve_by_h_blind (void *cls,
  * @param session a session
  * @param denom_pub_hash hash of the revoked denomination key
  * @param master_sig signature affirming the revocation
- * @return #GNUNET_OK on success,
+ * @return transaction status code
- *         #GNUNET_NO if the entry already exists (transaction must be rolled back!)
- *         #GNUNET_SYSERR on DB errors
  */
-static int
+static enum GNUNET_DB_QueryStatus
 postgres_insert_denomination_revocation (void *cls,
                                          struct TALER_EXCHANGEDB_Session *session,
                                          const struct GNUNET_HashCode *denom_pub_hash,
                                          const struct TALER_MasterSignatureP *master_sig)
 {
-  PGresult *result;
-  int ret;
   struct GNUNET_PQ_QueryParam params[] = {
     GNUNET_PQ_query_param_auto_from_type (denom_pub_hash),
     GNUNET_PQ_query_param_auto_from_type (master_sig),
     GNUNET_PQ_query_param_end
   };
 
-  result = GNUNET_PQ_exec_prepared (session->conn,
+  return GNUNET_PQ_eval_prepared_non_select (session->conn,
-                                   "denomination_revocation_insert",
+					     "denomination_revocation_insert",
-                                   params);
+					     params);
-  if (PGRES_COMMAND_OK != PQresultStatus (result))
-  {
-    const char *efield;
-
-    efield = PQresultErrorField (result,
-				 PG_DIAG_SQLSTATE);
-    /* FIXME: what about serialization errors? */
-    if ( (PGRES_FATAL_ERROR == PQresultStatus(result)) &&
-	 (NULL != strstr (PQ_DIAG_SQLSTATE_UNIQUE_VIOLATION,
-			  efield)) )
-    {
-      /* This means we had the same reserve/justification/details
-	 before */
-      GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                  "Uniqueness violation, revocation details already known\n");
-      PQclear (result);
-      return GNUNET_NO;
-    }
-    ret = GNUNET_SYSERR;
-    BREAK_DB_ERR (result, session->conn);
-  }
-  else
-  {
-    ret = GNUNET_OK;
-  }
-  PQclear (result);
-  return ret;
 }
 
 

src/exchangedb/test_exchangedb.c

@@ -269,7 +269,7 @@ create_denom_key_pair (unsigned int size,
     destroy_denom_key_pair (dkp);
     return NULL;
   }
-  if (GNUNET_OK !=
+  if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
       plugin->get_denomination_info (plugin->cls,
                                      session,
                                      &dki.denom_pub,
@@ -1124,7 +1124,7 @@ test_gc (struct TALER_EXCHANGEDB_Session *session)
     destroy_denom_key_pair (dkp);
     return GNUNET_SYSERR;
   }
-  if (GNUNET_OK ==
+  if (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS !=
       plugin->get_denomination_info (plugin->cls,
                                      session,
                                      &dkp->pub,
@@ -1905,7 +1905,7 @@ run (void *cls)
     struct TALER_MasterSignatureP msig;
     uint64_t rev_rowid;
 
-    FAILIF (GNUNET_OK !=
+    FAILIF (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
             plugin->get_denomination_revocation (plugin->cls,
                                                  session,
                                                  &dkp_pub_hash,

src/include/taler_exchangedb_plugin.h

@@ -1150,10 +1150,10 @@ struct TALER_EXCHANGEDB_Plugin
    * @param cls the @e cls of this struct with the plugin-specific state
    * @param session connection to use
    * @param denom_pub the public key used for signing coins of this denomination
-   * @param[out] issue set to issue information with value, fees and other info about the coin, can be NULL
+   * @param[out] issue set to issue information with value, fees and other info about the coin
-   * @return #GNUNET_OK on success; #GNUNET_NO if no record was found, #GNUNET_SYSERR on failure
+   * @return transaction status code
    */
-  int
+  enum GNUNET_DB_QueryStatus
   (*get_denomination_info) (void *cls,
                             struct TALER_EXCHANGEDB_Session *session,
                             const struct TALER_DenominationPublicKey *denom_pub,
@@ -2188,11 +2188,9 @@ struct TALER_EXCHANGEDB_Plugin
    * @param session a session
    * @param denom_pub_hash hash of the revoked denomination key
    * @param master_sig signature affirming the revocation
-   * @return #GNUNET_OK on success,
+   * @return transaction status code
-   *         #GNUNET_NO if the entry already exists (transaction must be rolled back!)
-   *         #GNUNET_SYSERR on DB errors
    */
-  int
+  enum GNUNET_DB_QueryStatus
   (*insert_denomination_revocation)(void *cls,
                                     struct TALER_EXCHANGEDB_Session *session,
                                     const struct GNUNET_HashCode *denom_pub_hash,