gnunet-svn
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[GNUnet-SVN] [taler-merchant] branch master updated: fix bad nesting of


From: gnunet
Subject: [GNUnet-SVN] [taler-merchant] branch master updated: fix bad nesting of transactions during /pay processing
Date: Sat, 02 Sep 2017 15:25:09 +0200

This is an automated email from the git hooks/post-receive script.

grothoff pushed a commit to branch master
in repository merchant.

The following commit(s) were added to refs/heads/master by this push:
     new baff321  fix bad nesting of transactions during /pay processing
baff321 is described below

commit baff321b2bc9979c02a7d0a02e6440c3ee2403be
Author: Christian Grothoff <address@hidden>
AuthorDate: Sat Sep 2 15:25:07 2017 +0200

    fix bad nesting of transactions during /pay processing
---
 src/backend/taler-merchant-httpd_exchanges.c |   5 +-
 src/backend/taler-merchant-httpd_pay.c       | 317 ++++++++++++++-------------
 src/backenddb/plugin_merchantdb_postgres.c   |   6 +
 3 files changed, 176 insertions(+), 152 deletions(-)

diff --git a/src/backend/taler-merchant-httpd_exchanges.c 
b/src/backend/taler-merchant-httpd_exchanges.c
index f53e63e..aca9160 100644
--- a/src/backend/taler-merchant-httpd_exchanges.c
+++ b/src/backend/taler-merchant-httpd_exchanges.c
@@ -387,9 +387,10 @@ process_wire_fees (void *cls,
     }
     if (0 == qs)
     {
-      /* Entry was already in DB, fine, continue as if we succeeded */
+      /* Entry was already in DB, fine, continue as if we had succeeded */
+      GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+                 "Fees already in DB, rolling back transaction attempt!\n");
       db->rollback (db->cls);
-      fees = fees->next;
     }
     if (0 < qs)
     {
diff --git a/src/backend/taler-merchant-httpd_pay.c 
b/src/backend/taler-merchant-httpd_pay.c
index f32d448..ff1636d 100644
--- a/src/backend/taler-merchant-httpd_pay.c
+++ b/src/backend/taler-merchant-httpd_pay.c
@@ -620,6 +620,52 @@ pay_context_cleanup (struct TM_HandlerContext *hc)
 
 
 /**
+ * Check if the existing transaction matches our transaction.
+ * Update `transaction_exists` accordingly.
+ *
+ * @param cls closure with the `struct PayContext`
+ * @param merchant_pub merchant's public key
+ * @param exchange_uri URI of the exchange
+ * @param h_contract_terms hashed proposal data
+ * @param h_xwire hash of our wire details
+ * @param timestamp time of the confirmation
+ * @param refund refund deadline
+ * @param total_amount total amount we receive for the contract after fees
+ */
+static void
+check_transaction_exists (void *cls,
+                         const struct TALER_MerchantPublicKeyP *merchant_pub,
+                         const char *exchange_uri,
+                         const struct GNUNET_HashCode *h_contract_terms,
+                         const struct GNUNET_HashCode *h_xwire,
+                         struct GNUNET_TIME_Absolute timestamp,
+                         struct GNUNET_TIME_Absolute refund,
+                         const struct TALER_Amount *total_amount)
+{
+  struct PayContext *pc = cls;
+
+  if ( (0 == memcmp (h_contract_terms,
+                    &pc->h_contract_terms,
+                     sizeof (struct GNUNET_HashCode))) &&
+       (0 == memcmp (h_xwire,
+                    &pc->mi->h_wire,
+                    sizeof (struct GNUNET_HashCode))) &&
+       (timestamp.abs_value_us == pc->timestamp.abs_value_us) &&
+       (refund.abs_value_us == pc->refund_deadline.abs_value_us) &&
+       (0 == TALER_amount_cmp (total_amount,
+                              &pc->amount) ) )
+  {
+    pc->transaction_exists = GNUNET_YES;
+  }
+  else
+  {
+    GNUNET_break_op (0);
+    pc->transaction_exists = GNUNET_SYSERR;
+  }
+}
+
+
+/**
  * Function called with the result of our exchange lookup.
  *
  * @param cls the `struct PayContext`
@@ -639,6 +685,8 @@ process_pay_with_exchange (void *cls,
   struct TALER_Amount wire_fee_delta;
   struct TALER_Amount wire_fee_customer_contribution;
   const struct TALER_EXCHANGE_Keys *keys;
+  enum GNUNET_DB_QueryStatus qs;
+  enum GNUNET_DB_QueryStatus qs_st;
 
   pc->fo = NULL;
   if (NULL == mh)
@@ -869,6 +917,7 @@ process_pay_with_exchange (void *cls,
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "Exchange and fee structure OK. Initiating deposit operation for 
coins\n");
 
+
   if (GNUNET_OK != db->start (db->cls))
   {
     GNUNET_break (0);
@@ -880,6 +929,122 @@ process_pay_with_exchange (void *cls,
     return;
   }
 
+  /* Check if transaction is already known, if not store it. */
+  /* FIXME: What if transaction exists, with a failed payment at
+     a different exchange? */
+  qs = db->find_transaction (db->cls,
+                            &pc->h_contract_terms,
+                            &pc->mi->pubkey,
+                            &check_transaction_exists,
+                            pc);
+  if (0 > qs)
+  {
+    /* single, read-only SQL statements should never cause
+       serialization problems */
+    GNUNET_break (GNUNET_DB_STATUS_SOFT_ERROR != qs);
+    /* Always report on hard error as well to enable diagnostics */
+    GNUNET_break (GNUNET_DB_STATUS_HARD_ERROR == qs);
+    /* FIXME: factor common logic of these calls into a function! */
+    resume_pay_with_response (pc,
+                              MHD_HTTP_INTERNAL_SERVER_ERROR,
+                              TMH_RESPONSE_make_json_pack ("{s:I, s:s}",
+                                                           "code", 
(json_int_t) TALER_EC_PAY_DB_FETCH_TRANSACTION_ERROR,
+                                                           "hint", "Merchant 
database error"));
+    return;
+  }
+  if (GNUNET_SYSERR == pc->transaction_exists)
+  {
+    GNUNET_break (0);
+    /* FIXME: factor common logic of these calls into a function! */
+    resume_pay_with_response (pc,
+                              MHD_HTTP_BAD_REQUEST,
+                              TMH_RESPONSE_make_json_pack ("{s:I, s:s}",
+                                                           "code", 
(json_int_t) TALER_EC_PAY_DB_TRANSACTION_ID_CONFLICT,
+                                                           "hint", 
"Transaction ID reused with different transaction details"));
+    return;
+  }
+  if (GNUNET_NO == pc->transaction_exists)
+  {
+    struct GNUNET_TIME_Absolute now;
+
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "Dealing with new transaction `%s'\n",
+                GNUNET_h2s (&pc->h_contract_terms));
+
+    now = GNUNET_TIME_absolute_get ();
+    if (now.abs_value_us > pc->pay_deadline.abs_value_us)
+    {
+      /* Time expired, we don't accept this payment now! */
+      const char *pd_str;
+
+      pd_str = GNUNET_STRINGS_absolute_time_to_string (pc->pay_deadline);
+      GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                  "Attempt to pay coins for expired contract. Deadline: 
`%s'\n",
+                 pd_str);
+      resume_pay_with_response (pc,
+                                MHD_HTTP_BAD_REQUEST,
+                                TMH_RESPONSE_make_json_pack ("{s:I, s:s}",
+                                                             "code", 
(json_int_t) TALER_EC_PAY_OFFER_EXPIRED,
+                                                             "hint", "The time 
to pay for this contract has expired."));
+      return;
+    }
+
+    qs_st = db->store_transaction (db->cls,
+                                   &pc->h_contract_terms,
+                                   &pc->mi->pubkey,
+                                   pc->chosen_exchange,
+                                   &pc->mi->h_wire,
+                                   pc->timestamp,
+                                   pc->refund_deadline,
+                                   &pc->amount);
+    /* Only retry if SOFT error occurred.  Exit in case of OK or HARD failure 
*/
+    if (GNUNET_DB_STATUS_SOFT_ERROR == qs_st)
+    {
+      db->rollback (db->cls);
+      GNUNET_break (0);  // FIXME: implement proper retries!
+      resume_pay_with_response (pc,
+                                MHD_HTTP_INTERNAL_SERVER_ERROR,
+                                TMH_RESPONSE_make_json_pack ("{s:I, s:s}",
+                                                             "code", 
(json_int_t) TALER_EC_PAY_DB_STORE_TRANSACTION_ERROR,
+                                                             "hint", "Soft 
merchant database error: retries not implemented"));
+      return;
+    }
+    /* Exit in case of HARD failure */
+    if (GNUNET_DB_STATUS_HARD_ERROR == qs_st)
+    {
+      GNUNET_break (0);
+      db->rollback (db->cls);
+      resume_pay_with_response (pc,
+                                MHD_HTTP_INTERNAL_SERVER_ERROR,
+                                TMH_RESPONSE_make_json_pack ("{s:I, s:s}",
+                                                             "code", 
(json_int_t) TALER_EC_PAY_DB_STORE_TRANSACTION_ERROR,
+                                                             "hint", "Merchant 
database error: hard error while storing transaction"));
+    }
+  }
+
+  /**
+   * Break if we couldn't modify one, and only one line; this
+   * includes hard errors.
+   */
+  if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs_st)
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+                "Unexpected query status %d while storing /pay transaction!\n",
+                (int) qs_st);
+    resume_pay_with_response (pc,
+                              MHD_HTTP_INTERNAL_SERVER_ERROR,
+                              TMH_RESPONSE_make_json_pack ("{s:I, s:s}",
+                                                           "code", 
(json_int_t) TALER_EC_PAY_DB_STORE_TRANSACTION_ERROR,
+                                                           "hint", "Merchant 
database error: failed to store transaction"));
+    return;
+  }
+
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Found transaction data for proposal `%s' of merchant `%s', 
initiating deposits\n",
+              GNUNET_h2s (&pc->h_contract_terms),
+              TALER_B2S (&pc->mi->pubkey));
+
+
   /* Initiate /deposit operation for all coins */
   for (unsigned int i=0;i<pc->coins_cnt;i++)
   {
@@ -1006,52 +1171,6 @@ check_coin_paid (void *cls,
 
 
 /**
- * Check if the existing transaction matches our transaction.
- * Update `transaction_exists` accordingly.
- *
- * @param cls closure with the `struct PayContext`
- * @param merchant_pub merchant's public key
- * @param exchange_uri URI of the exchange
- * @param h_contract_terms hashed proposal data
- * @param h_xwire hash of our wire details
- * @param timestamp time of the confirmation
- * @param refund refund deadline
- * @param total_amount total amount we receive for the contract after fees
- */
-static void
-check_transaction_exists (void *cls,
-                         const struct TALER_MerchantPublicKeyP *merchant_pub,
-                         const char *exchange_uri,
-                         const struct GNUNET_HashCode *h_contract_terms,
-                         const struct GNUNET_HashCode *h_xwire,
-                         struct GNUNET_TIME_Absolute timestamp,
-                         struct GNUNET_TIME_Absolute refund,
-                         const struct TALER_Amount *total_amount)
-{
-  struct PayContext *pc = cls;
-
-  if ( (0 == memcmp (h_contract_terms,
-                    &pc->h_contract_terms,
-                     sizeof (struct GNUNET_HashCode))) &&
-       (0 == memcmp (h_xwire,
-                    &pc->mi->h_wire,
-                    sizeof (struct GNUNET_HashCode))) &&
-       (timestamp.abs_value_us == pc->timestamp.abs_value_us) &&
-       (refund.abs_value_us == pc->refund_deadline.abs_value_us) &&
-       (0 == TALER_amount_cmp (total_amount,
-                              &pc->amount) ) )
-  {
-    pc->transaction_exists = GNUNET_YES;
-  }
-  else
-  {
-    GNUNET_break_op (0);
-    pc->transaction_exists = GNUNET_SYSERR;
-  }
-}
-
-
-/**
  * Try to parse the pay request into the given pay context.
  * Schedules an error response in the connection on failure.
  *
@@ -1348,7 +1467,6 @@ handler_pay_json (struct MHD_Connection *connection,
 {
   int ret;
   enum GNUNET_DB_QueryStatus qs;
-  enum GNUNET_DB_QueryStatus qs_st;
 
   ret = parse_pay (connection,
                    root,
@@ -1389,107 +1507,6 @@ handler_pay_json (struct MHD_Connection *connection,
     MHD_destroy_response (resp);
     return ret;
   }
-  /* Check if transaction is already known, if not store it. */
-  /* FIXME: What if transaction exists, with a failed payment at
-     a different exchange? */
-  qs = db->find_transaction (db->cls,
-                            &pc->h_contract_terms,
-                            &pc->mi->pubkey,
-                            &check_transaction_exists,
-                            pc);
-  if (0 > qs)
-  {
-    /* single, read-only SQL statements should never cause
-       serialization problems */
-    GNUNET_break (GNUNET_DB_STATUS_SOFT_ERROR != qs);
-    /* Always report on hard error as well to enable diagnostics */
-    GNUNET_break (GNUNET_DB_STATUS_HARD_ERROR == qs);
-    return TMH_RESPONSE_reply_internal_error (connection,
-                                             
TALER_EC_PAY_DB_FETCH_TRANSACTION_ERROR,
-                                               "Merchant database error");
-  }
-  if (GNUNET_SYSERR == pc->transaction_exists)
-  {
-    GNUNET_break (0);
-    return TMH_RESPONSE_reply_external_error (connection,
-                                              
TALER_EC_PAY_DB_TRANSACTION_ID_CONFLICT,
-                                             "Transaction ID reused with 
different transaction details");
-  }
-  if (GNUNET_NO == pc->transaction_exists)
-  {
-    struct GNUNET_TIME_Absolute now;
-
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "Dealing with new transaction `%s'\n",
-                GNUNET_h2s (&pc->h_contract_terms));
-
-    now = GNUNET_TIME_absolute_get ();
-    if (now.abs_value_us > pc->pay_deadline.abs_value_us)
-    {
-      /* Time expired, we don't accept this payment now! */
-      const char *pd_str;
-
-      pd_str = GNUNET_STRINGS_absolute_time_to_string (pc->pay_deadline);
-      GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
-                  "Attempt to pay coins for expired contract. Deadline: 
`%s'\n",
-                 pd_str);
-      return TMH_RESPONSE_reply_bad_request (connection,
-                                            TALER_EC_PAY_OFFER_EXPIRED,
-                                             "The time to pay for this 
contract has expired.");
-    }
-
-    for (unsigned int i=0;i<MAX_RETRIES;i++)
-    {
-      if (GNUNET_OK != db->start (db->cls))
-      {
-        qs_st = GNUNET_DB_STATUS_HARD_ERROR;
-        break;
-      }
-      qs_st = db->store_transaction (db->cls,
-                                     &pc->h_contract_terms,
-                                     &pc->mi->pubkey,
-                                     pc->chosen_exchange,
-                                     &pc->mi->h_wire,
-                                     pc->timestamp,
-                                     pc->refund_deadline,
-                                     &pc->amount);
-
-      /* Only retry if SOFT error occurred.  Exit in case of OK or HARD 
failure */
-      if (GNUNET_DB_STATUS_SOFT_ERROR == qs_st)
-      {
-        db->rollback (db->cls);
-        continue;
-      }
-      /* Only retry if SOFT error occurred.  Exit in case of OK or HARD 
failure */
-      if (GNUNET_DB_STATUS_HARD_ERROR == qs_st)
-      {
-        GNUNET_break (0);
-        db->rollback (db->cls);
-        return TMH_RESPONSE_reply_internal_error (connection,
-                                                  
TALER_EC_PAY_DB_STORE_TRANSACTION_ERROR,
-                                                 "Merchant database error: 
hard error while storing transaction");
-      }
-      break;
-    }
-
-    /**
-     * Break if we couldn't modify one, and only one line; this
-     * includes hard errors.
-     */
-    if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs_st)
-    {
-      GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
-                  "Unexpected query status %d while storing /pay 
transaction!\n",
-                  (int) qs_st);
-      return TMH_RESPONSE_reply_internal_error (connection,
-                                               
TALER_EC_PAY_DB_STORE_TRANSACTION_ERROR,
-                                               "Merchant database error: 
failed to store transaction");
-    }
-  }
-  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "Found transaction data for proposal `%s' of merchant `%s'\n",
-              GNUNET_h2s (&pc->h_contract_terms),
-              TALER_B2S (&pc->mi->pubkey));
 
   MHD_suspend_connection (connection);
   pc->suspended = GNUNET_YES;
@@ -1514,8 +1531,8 @@ handler_pay_json (struct MHD_Connection *connection,
 
 
 /**
- * Process a payment for a proposal.
- * Takes data from the given MHD connection.
+ * Process a payment for a proposal.  Takes data from the given MHD
+ * connection.
  *
  * @param rh context of the handler
  * @param connection the MHD connection to handle
diff --git a/src/backenddb/plugin_merchantdb_postgres.c 
b/src/backenddb/plugin_merchantdb_postgres.c
index 019af0d..249c0fa 100644
--- a/src/backenddb/plugin_merchantdb_postgres.c
+++ b/src/backenddb/plugin_merchantdb_postgres.c
@@ -528,6 +528,8 @@ postgres_start (void *cls)
   ExecStatusType ex;
 
   check_connection (pg);
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Starting merchant DB transaction\n");
   result = PQexec (pg->conn,
                    "START TRANSACTION ISOLATION LEVEL SERIALIZABLE");
   if (PGRES_COMMAND_OK !=
@@ -557,6 +559,8 @@ postgres_rollback (void *cls)
   struct PostgresClosure *pg = cls;
   PGresult *result;
 
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Rolling back merchant DB transaction\n");
   result = PQexec (pg->conn,
                    "ROLLBACK");
   GNUNET_break (PGRES_COMMAND_OK ==
@@ -579,6 +583,8 @@ postgres_commit (void *cls)
     GNUNET_PQ_query_param_end
   };
 
+  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+              "Committing merchant DB transaction\n");
   return GNUNET_PQ_eval_prepared_non_select (pg->conn,
                                             "end_transaction",
                                             params);

-- 
To stop receiving notification emails like this one, please contact
address@hidden



reply via email to

[Prev in Thread] Current Thread [Next in Thread]