From c4f3c9be6cdf54d6dcbefbb5960c64d1741182cc Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Fri, 5 Jun 2015 13:48:57 +0200 Subject: [PATCH] clean up postgres_reserves_in_insert logic and improve docu --- src/include/taler_mintdb_plugin.h | 8 +-- src/mintdb/plugin_mintdb_postgres.c | 78 ++++++++++++++++------------- 2 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/include/taler_mintdb_plugin.h b/src/include/taler_mintdb_plugin.h index d41538fb1..c4b89bcd8 100644 --- a/src/include/taler_mintdb_plugin.h +++ b/src/include/taler_mintdb_plugin.h @@ -723,7 +723,7 @@ struct TALER_MINTDB_Plugin * * @param cls the @e cls of this struct with the plugin-specific state * @param sesssion database connection to use - * @param h_blind hash of the blinded message + * @param h_blind hash of the blinded message to be signed * @param collectable corresponding collectable coin (blind signature) * if a coin is found * @return #GNUNET_SYSERR on internal error @@ -743,9 +743,11 @@ struct TALER_MINTDB_Plugin * * @param cls the @e cls of this struct with the plugin-specific state * @param sesssion database connection to use - * @param h_blind hash of the blinded message + * @param h_blind hash of the blinded message which is (blindly) signed by the + * signature in @a collectable * @param withdraw amount by which the reserve will be withdrawn with this - * transaction + * transaction (based on the value of the denomination key + * used for the signature); coin value plus fee. * @param collectable corresponding collectable coin (blind signature) * if a coin is found * @return #GNUNET_SYSERR on internal error diff --git a/src/mintdb/plugin_mintdb_postgres.c b/src/mintdb/plugin_mintdb_postgres.c index d7f37517f..99e9e6cf2 100644 --- a/src/mintdb/plugin_mintdb_postgres.c +++ b/src/mintdb/plugin_mintdb_postgres.c @@ -1108,7 +1108,7 @@ reserves_update (void *cls, /** - * Insert a incoming transaction into reserves. New reserves are also created + * Insert an incoming transaction into reserves. New reserves are also created * through this function. Note that this API call starts (and stops) its * own transaction scope (so the application must not do so). * @@ -1134,9 +1134,7 @@ postgres_reserves_in_insert (void *cls, PGresult *result; int reserve_exists; struct TALER_MINTDB_Reserve reserve; - struct TALER_MINTDB_Reserve updated_reserve; - result = NULL; if (GNUNET_OK != postgres_start (cls, session)) { @@ -1154,7 +1152,11 @@ postgres_reserves_in_insert (void *cls, } if (GNUNET_NO == reserve_exists) { - /* New reserve, create balance for the first time */ + /* New reserve, create balance for the first time; we do this + before adding the actual transaction to "reserves_in", as + for a new reserve it can't be a duplicate 'add' operation, + and as the 'add' operation may need the reserve entry + as a foreign key. */ struct TALER_PQ_QueryParam params[] = { TALER_PQ_query_param_auto_from_type (reserve_pub), TALER_PQ_query_param_amount (balance), @@ -1170,33 +1172,16 @@ postgres_reserves_in_insert (void *cls, if (PGRES_COMMAND_OK != PQresultStatus(result)) { QUERY_ERR (result); + PQclear (result); goto rollback; } - } - else - { - /* Update reserve balance */ - updated_reserve.pub = reserve.pub; - - if (GNUNET_OK != - TALER_amount_add (&updated_reserve.balance, - &reserve.balance, - balance)) - { - /* currency overflow or incompatible currency */ - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "Attempt to deposit incompatible amount into reserve\n"); - goto rollback; - } - updated_reserve.expiry = GNUNET_TIME_absolute_max (expiry, - reserve.expiry); - - } - if (NULL != result) PQclear (result); - result = NULL; - /* create new incoming transaction, SQL "primary key" logic - is used to guard against duplicates! */ + } + /* Create new incoming transaction, SQL "primary key" logic + is used to guard against duplicates. If a duplicate is + detected, we rollback (which really shouldn't undo + anything) and return #GNUNET_NO to indicate that this failure + is kind-of harmless (already executed). */ { struct TALER_PQ_QueryParam params[] = { TALER_PQ_query_param_auto_from_type (&reserve.pub), @@ -1231,19 +1216,40 @@ postgres_reserves_in_insert (void *cls, goto rollback; } PQclear (result); - result = NULL; - if ( (GNUNET_YES == reserve_exists) && - (GNUNET_OK != reserves_update (cls, + + if (GNUNET_YES == reserve_exists) + { + /* If the reserve already existed, we need to still update the + balance; we do this after checking for duplication, as + otherwise we might have to actually pay the cost to roll this + back for duplicate transactions; like this, we should virtually + never actually have to rollback anything. */ + struct TALER_MINTDB_Reserve updated_reserve; + + updated_reserve.pub = reserve.pub; + if (GNUNET_OK != + TALER_amount_add (&updated_reserve.balance, + &reserve.balance, + balance)) + { + /* currency overflow or incompatible currency */ + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Attempt to deposit incompatible amount into reserve\n"); + goto rollback; + } + updated_reserve.expiry = GNUNET_TIME_absolute_max (expiry, + reserve.expiry); + if (GNUNET_OK != reserves_update (cls, session, - &updated_reserve)) ) - goto rollback; + &updated_reserve)) + goto rollback; + } if (GNUNET_OK != postgres_commit (cls, session)) return GNUNET_SYSERR; return GNUNET_OK; + rollback: - if (NULL != result) - PQclear (result); postgres_rollback (cls, session); return GNUNET_SYSERR; @@ -1256,7 +1262,7 @@ postgres_reserves_in_insert (void *cls, * * @param cls the `struct PostgresClosure` with the plugin-specific state * @param session database connection to use - * @param h_blind hash of the blinded message + * @param h_blind hash of the blinded message to be signed * @param collectable corresponding collectable coin (blind signature) * if a coin is found * @return #GNUNET_SYSERR on internal error