From d5960b50af4aa7f1faaa1d013cb1a142d258cc36 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Wed, 13 May 2015 15:57:35 +0200 Subject: [PATCH] towards fixing #3717 and #3633 --- src/include/taler_mintdb_plugin.h | 20 +- src/mint-tools/Makefile.am | 2 - src/mint-tools/taler-mint-reservemod.c | 291 ++++++++----------------- src/mintdb/plugin_mintdb_postgres.c | 105 ++++----- 4 files changed, 152 insertions(+), 266 deletions(-) diff --git a/src/include/taler_mintdb_plugin.h b/src/include/taler_mintdb_plugin.h index 540cb88ca..ffa1b13d3 100644 --- a/src/include/taler_mintdb_plugin.h +++ b/src/include/taler_mintdb_plugin.h @@ -682,28 +682,28 @@ struct TALER_MINTDB_Plugin struct TALER_MINTDB_Session *db, struct TALER_MINTDB_Reserve *reserve); - /* FIXME: add functions to add bank transfers to our DB - (and to test if we already did add one) (#3633/#3717) */ - /** - * Insert a incoming transaction into reserves. New reserves are also created - * through this function. + * Insert a incoming transaction into reserves. New reserves are + * also created through this function. * * @param cls the @e cls of this struct with the plugin-specific state * @param db the database connection handle - * @param reserve the reserve structure. The public key of the reserve should - * be set here. Upon successful execution of this function, the - * balance and expiration of the reserve will be updated. + * @param reserve_pub public key of the reserve * @param balance the amount that has to be added to the reserve + * @param details bank transaction details justifying the increment, + * must be unique for each incoming transaction * @param expiry the new expiration time for the reserve - * @return #GNUNET_OK upon success; #GNUNET_SYSERR upon failures + * @return #GNUNET_OK upon success; #GNUNET_NO if the given + * @a details are already known for this @a reserve_pub, + * #GNUNET_SYSERR upon failures (DB error, incompatible currency) */ int (*reserves_in_insert) (void *cls, struct TALER_MINTDB_Session *db, - struct TALER_MINTDB_Reserve *reserve, + const struct TALER_ReservePublicKeyP *reserve_pub, const struct TALER_Amount *balance, + const char *details, const struct GNUNET_TIME_Absolute expiry); diff --git a/src/mint-tools/Makefile.am b/src/mint-tools/Makefile.am index a61ab6a93..e22df1ed7 100644 --- a/src/mint-tools/Makefile.am +++ b/src/mint-tools/Makefile.am @@ -41,7 +41,6 @@ taler_mint_reservemod_LDADD = \ $(top_builddir)/src/util/libtalerutil.la \ $(top_builddir)/src/pq/libtalerpq.la \ $(top_builddir)/src/mintdb/libtalermintdb.la \ - -lpq \ -lgnunetutil $(XLIB) taler_mint_reservemod_LDFLAGS = \ $(POSTGRESQL_LDFLAGS) @@ -57,7 +56,6 @@ taler_mint_dbinit_LDADD = \ $(top_builddir)/src/util/libtalerutil.la \ $(top_builddir)/src/pq/libtalerpq.la \ $(top_builddir)/src/mintdb/libtalermintdb.la \ - -lpq \ -lgnunetutil $(XLIB) taler_mint_dbinit_LDFLAGS = \ $(POSTGRESQL_LDFLAGS) diff --git a/src/mint-tools/taler-mint-reservemod.c b/src/mint-tools/taler-mint-reservemod.c index 762830783..5bb6bce46 100644 --- a/src/mint-tools/taler-mint-reservemod.c +++ b/src/mint-tools/taler-mint-reservemod.c @@ -28,181 +28,25 @@ #include "taler_mintdb_plugin.h" #include "taler_mintdb_lib.h" +/** + * After what time to inactive reserves expire? + */ +#define RESERVE_EXPIRATION GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_YEARS, 5) /** * Director of the mint, containing the keys. */ static char *mint_directory; -/** - * Public key of the reserve to manipulate. - */ -static struct GNUNET_CRYPTO_EddsaPublicKey *reserve_pub; - /** * Handle to the mint's configuration */ static struct GNUNET_CONFIGURATION_Handle *cfg; /** - * Database connection handle. + * Our DB plugin. */ -static PGconn *db_conn; - - -/** - * Create a new or add to existing reserve. Fails if currencies do - * not match. - * - * @param denom denomination to add - * @return #GNUNET_OK on success, - * #GNUNET_SYSERR on error - */ -// FIXME: this should use the DB abstraction layer. (#3717) -// FIXME: this should be done by adding an inbound transaction -// to the table with the transactions for this reserve, -// not by modifying some 'total' value for the reserve! -// (we should in fact probably never modify, always just append!) (#3633) -static int -reservemod_add (struct TALER_Amount denom) -{ - PGresult *result; - const void *param_values[] = { - reserve_pub - }; - int param_lengths[] = { - sizeof(struct GNUNET_CRYPTO_EddsaPublicKey) - }; - int param_formats[] = { - 1 - }; - struct TALER_Amount old_denom; - struct TALER_Amount new_denom; - struct TALER_AmountNBO new_denom_nbo; - - result = PQexecParams (db_conn, - "SELECT balance_value, balance_fraction, balance_currency" - " FROM reserves" - " WHERE reserve_pub=$1" - " LIMIT 1;", - 1, - NULL, - (const char * const *) param_values, - param_lengths, - param_formats, - 1); - if (PGRES_TUPLES_OK != PQresultStatus (result)) - { - fprintf (stderr, - "Select failed: %s\n", - PQresultErrorMessage (result)); - return GNUNET_SYSERR; - } - if (0 == PQntuples (result)) - { - struct GNUNET_TIME_AbsoluteNBO exnbo; - uint32_t value = htonl (denom.value); - uint32_t fraction = htonl (denom.fraction); - const void *param_values[] = { - reserve_pub, - &value, - &fraction, - denom.currency, - &exnbo - }; - int param_lengths[] = { - sizeof (struct GNUNET_CRYPTO_EddsaPublicKey), - sizeof (uint32_t), - sizeof (uint32_t), - strlen (denom.currency), - sizeof (struct GNUNET_TIME_AbsoluteNBO) - }; - int param_formats[] = { - 1, 1, 1, 1, 1 - }; - - exnbo = GNUNET_TIME_absolute_hton (GNUNET_TIME_relative_to_absolute (GNUNET_TIME_UNIT_YEARS)); - result = PQexecParams (db_conn, - "INSERT INTO reserves (reserve_pub, balance_value, balance_fraction, balance_currency, expiration_date)" - " VALUES ($1,$2,$3,$4,$5);", - 5, - NULL, - (const char **) param_values, - param_lengths, - param_formats, - 1); - - if (PGRES_COMMAND_OK != PQresultStatus (result)) - { - fprintf (stderr, - "Insert failed: %s\n", - PQresultErrorMessage (result)); - return GNUNET_SYSERR; - } - } - else - { - const void *param_values[] = { - &new_denom_nbo.value, - &new_denom_nbo.fraction, - reserve_pub - }; - int param_lengths[] = { - sizeof (new_denom_nbo.value), - sizeof (new_denom_nbo.fraction), - sizeof (struct GNUNET_CRYPTO_EddsaPublicKey) - }; - int param_formats[] = { - 1, 1, 1 - }; - - GNUNET_assert (GNUNET_OK == - TALER_PQ_extract_amount (result, 0, - "balance_value", - "balance_fraction", - "balance_currency", - &old_denom)); - if (GNUNET_OK != - TALER_amount_add (&new_denom, - &old_denom, - &denom)) - { - fprintf (stderr, - "Integer overflow when computing new balance!\n"); - return GNUNET_SYSERR; - } - TALER_amount_hton (&new_denom_nbo, - &new_denom); - result = PQexecParams (db_conn, - "UPDATE reserves" - " SET balance_value = $1, balance_fraction = $2, status_sig = NULL, status_sign_pub = NULL" - " WHERE reserve_pub = $3;", - 3, - NULL, - (const char **) param_values, - param_lengths, - param_formats, - 1); - - if (PGRES_COMMAND_OK != PQresultStatus (result)) - { - fprintf (stderr, - "Update failed: %s\n", - PQresultErrorMessage (result)); - return GNUNET_SYSERR; - } - /* Yes, for historic reasons libpq returns a 'const char *'... */ - if (0 != strcmp ("1", - PQcmdTuples (result))) - { - fprintf (stderr, - "Update failed (updated `%s' tupes instead of '1')\n", - PQcmdTuples (result)); - return GNUNET_SYSERR; - } - } - return GNUNET_OK; -} +static struct TALER_MINTDB_Plugin *plugin; /** @@ -215,15 +59,23 @@ reservemod_add (struct TALER_Amount denom) int main (int argc, char *const *argv) { - static char *reserve_pub_str; - static char *add_str; - static const struct GNUNET_GETOPT_CommandLineOption options[] = { + char *reserve_pub_str = NULL; + char *add_str = NULL; + struct TALER_Amount add_value; + char *details = NULL; + struct TALER_ReservePublicKeyP reserve_pub; + struct GNUNET_TIME_Absolute expiration; + struct TALER_MINTDB_Session *session; + const struct GNUNET_GETOPT_CommandLineOption options[] = { {'a', "add", "DENOM", "value to add", 1, &GNUNET_GETOPT_set_string, &add_str}, {'d', "mint-dir", "DIR", "mint directory with keys to update", 1, &GNUNET_GETOPT_set_filename, &mint_directory}, + {'D', "details", "JSON", + "details about the bank transaction which justify why we add this amount", 1, + &GNUNET_GETOPT_set_string, &details}, TALER_GETOPT_OPTION_HELP ("Deposit funds into a Taler reserve"), {'R', "reserve", "KEY", "reserve (public key) to modify", 1, @@ -231,7 +83,7 @@ main (int argc, char *const *argv) GNUNET_GETOPT_OPTION_VERSION (VERSION "-" VCS_VERSION), GNUNET_GETOPT_OPTION_END }; - char *connection_cfg_str; + int ret; GNUNET_assert (GNUNET_OK == GNUNET_log_setup ("taler-mint-reservemod", @@ -248,19 +100,35 @@ main (int argc, char *const *argv) "Mint directory not given\n"); return 1; } - - reserve_pub = GNUNET_new (struct GNUNET_CRYPTO_EddsaPublicKey); if ((NULL == reserve_pub_str) || (GNUNET_OK != GNUNET_STRINGS_string_to_data (reserve_pub_str, strlen (reserve_pub_str), - reserve_pub, - sizeof (struct GNUNET_CRYPTO_EddsaPublicKey)))) + &reserve_pub, + sizeof (struct TALER_ReservePublicKeyP)))) { fprintf (stderr, "Parsing reserve key invalid\n"); return 1; } + if ( (NULL == add_str) || + (GNUNET_OK != + TALER_string_to_amount (add_str, + &add_value)) ) + { + fprintf (stderr, + "Failed to parse currency amount `%s'\n", + add_str); + return 1; + } + + if (NULL == details) + { + fprintf (stderr, + "No wiring details given (justification required)\n"); + return 1; + } + cfg = TALER_config_load (mint_directory); if (NULL == cfg) { @@ -268,46 +136,59 @@ main (int argc, char *const *argv) "Failed to load mint configuration\n"); return 1; } - if (GNUNET_OK != - GNUNET_CONFIGURATION_get_value_string (cfg, - "mint", - "db", - &connection_cfg_str)) + ret = 1; + if (NULL == + (plugin = TALER_MINTDB_plugin_load (cfg))) { fprintf (stderr, - "Database configuration string not found\n"); - return 1; + "Failed to initialize database plugin.\n"); + goto cleanup; } - db_conn = PQconnectdb (connection_cfg_str); - if (CONNECTION_OK != PQstatus (db_conn)) - { - fprintf (stderr, - "Database connection failed: %s\n", - PQerrorMessage (db_conn)); - return 1; - } - if (NULL != add_str) - { - struct TALER_Amount add_value; - if (GNUNET_OK != - TALER_string_to_amount (add_str, - &add_value)) - { - fprintf (stderr, - "Failed to parse currency amount `%s'\n", - add_str); - return 1; - } - if (GNUNET_OK != - reservemod_add (add_value)) - { - fprintf (stderr, - "Failed to update reserve.\n"); - return 1; - } + session = plugin->get_session (plugin->cls, + GNUNET_NO); + if (NULL == session) + { + fprintf (stderr, + "Failed to initialize DB session\n"); + goto cleanup; } - return 0; + if (GNUNET_OK != + plugin->start (plugin->cls, + session)) + { + fprintf (stderr, + "Failed to start transaction\n"); + goto cleanup; + } + expiration = GNUNET_TIME_relative_to_absolute (RESERVE_EXPIRATION); + if (GNUNET_OK != + plugin->reserves_in_insert (plugin->cls, + session, + &reserve_pub, + &add_value, + details, + expiration)) + { + fprintf (stderr, + "Failed to update reserve.\n"); + goto cleanup; + } + if (GNUNET_OK != + plugin->commit (plugin->cls, + session)) + { + fprintf (stderr, + "Failed to commit transaction\n"); + goto cleanup; + } + ret = 0; + cleanup: + if (NULL != plugin) + TALER_MINTDB_plugin_unload (plugin); + if (NULL != cfg) + GNUNET_CONFIGURATION_destroy (cfg); + return ret; } /* end taler-mint-reservemod.c */ diff --git a/src/mintdb/plugin_mintdb_postgres.c b/src/mintdb/plugin_mintdb_postgres.c index 8ed374f03..7d3a3e870 100644 --- a/src/mintdb/plugin_mintdb_postgres.c +++ b/src/mintdb/plugin_mintdb_postgres.c @@ -214,11 +214,17 @@ postgres_create_tables (void *cls, ",balance_val INT8 NOT NULL" ",balance_frac INT4 NOT NULL" ",balance_curr VARCHAR("TALER_CURRENCY_LEN_STR") NOT NULL" + ",details VARCHAR NOT NULL " ",expiration_date INT8 NOT NULL" + " CONSTRAINT unique_details PRIMARY KEY (reserve_pub,details)" ");"); - /* Create an index on the foreign key as it is not created automatically by PSQL */ + /* Create indices on reserves_in */ SQLEXEC ("CREATE INDEX reserves_in_reserve_pub_index" " ON reserves_in (reserve_pub);"); + SQLEXEC ("CREATE INDEX reserves_in_reserve_pub_details_index" + " ON reserves_in (reserve_pub,details);"); + SQLEXEC ("CREATE INDEX expiration_index" + " ON reserves_in (expiration_date);"); /* Table with the withdraw operations that have been performed on a reserve. TODO: maybe rename to "reserves_out"? TODO: is blind_ev really a _primary key_? Is this constraint useful? */ @@ -234,7 +240,7 @@ postgres_create_tables (void *cls, SQLEXEC ("CREATE INDEX collectable_blindcoins_reserve_pub_index ON" " collectable_blindcoins (reserve_pub)"); /* Table with coins that have been (partially) spent, used to detect - double-spending. + double-spending. TODO: maybe rename to "spent_coins"? TODO: maybe have two tables, one for spending and one for refreshing, instead of optional refresh_session_hash? */ SQLEXEC("CREATE TABLE IF NOT EXISTS known_coins " @@ -426,8 +432,9 @@ postgres_prepare (PGconn *db_conn) " balance_val," " balance_frac," " balance_curr," + " details," " expiration_date) VALUES (" - " $1, $2, $3, $4, $5);", + " $1, $2, $3, $4, $5, $6);", 5, NULL); PREPARE ("get_reserves_in_transactions", "SELECT" @@ -435,6 +442,7 @@ postgres_prepare (PGconn *db_conn) ",balance_frac" ",balance_curr" ",expiration_date" + ",details" " FROM reserves_in WHERE reserve_pub=$1", 1, NULL); PREPARE ("insert_collectable_blindcoin", @@ -676,6 +684,7 @@ postgres_prepare (PGconn *db_conn) " FROM deposits WHERE " " coin_pub = $1", 1, NULL); + return GNUNET_OK; #undef PREPARE } @@ -905,7 +914,7 @@ postgres_reserve_get (void *cls, if (PGRES_TUPLES_OK != PQresultStatus (result)) { QUERY_ERR (result); - PQclear (result); + PQclear (resultE); return GNUNET_SYSERR; } if (0 == PQntuples (result)) @@ -914,8 +923,8 @@ postgres_reserve_get (void *cls, return GNUNET_NO; } EXITIF (GNUNET_OK != - TALER_PQ_extract_result (result, - rs, + TALER_PQ_extract_result (result, + rs, 0)); PQclear (result); return GNUNET_OK; @@ -974,38 +983,39 @@ postgres_reserves_update (void *cls, * * @param cls the `struct PostgresClosure` with the plugin-specific state * @param session the database connection handle - * @param reserve the reserve structure. The public key of the reserve should - * be set here. Upon successful execution of this function, the - * balance and expiration of the reserve will be updated. + * @param reserve_pub public key of the reserve * @param balance the amount that has to be added to the reserve + * @param details bank transaction details justifying the increment, + * must be unique for each incoming transaction * @param expiry the new expiration time for the reserve - * @return #GNUNET_OK upon success; #GNUNET_SYSERR upon failures + * @return #GNUNET_OK upon success; #GNUNET_NO if the given + * @a details are already known for this @a reserve_pub, + * #GNUNET_SYSERR upon failures (DB error, incompatible currency) */ static int postgres_reserves_in_insert (void *cls, struct TALER_MINTDB_Session *session, - struct TALER_MINTDB_Reserve *reserve, + struct TALER_ReservePublicKeyP *reserve_pub, const struct TALER_Amount *balance, + const char *details, const struct GNUNET_TIME_Absolute expiry) { PGresult *result; int reserve_exists; + struct TALER_MINTDB_Reserve reserve; + struct TALER_MINTDB_Reserve updated_reserve; result = NULL; - if (NULL == reserve) - { - GNUNET_break (0); - return GNUNET_SYSERR; - } if (GNUNET_OK != postgres_start (cls, session)) { GNUNET_break (0); return GNUNET_SYSERR; } + reserve.pub = *reserve_pub; reserve_exists = postgres_reserve_get (cls, session, - reserve); + &reserve); if (GNUNET_SYSERR == reserve_exists) { postgres_rollback (cls, @@ -1017,7 +1027,7 @@ postgres_reserves_in_insert (void *cls, GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Reserve does not exist; creating a new one\n"); struct TALER_PQ_QueryParam params[] = { - TALER_PQ_QUERY_PARAM_PTR (&reserve->pub), + TALER_PQ_QUERY_PARAM_PTR (reserve_pub), TALER_PQ_QUERY_PARAM_AMOUNT (balance), TALER_PQ_QUERY_PARAM_ABSOLUTE_TIME (expiry), TALER_PQ_QUERY_PARAM_END @@ -1031,13 +1041,34 @@ postgres_reserves_in_insert (void *cls, goto rollback; } } + else + { + /* Update 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 (NULL != result) PQclear (result); result = NULL; - /* create new incoming transaction */ + /* create new incoming transaction, SQL "primary key" logic + is used to guard against duplicates! */ struct TALER_PQ_QueryParam params[] = { TALER_PQ_QUERY_PARAM_PTR (&reserve->pub), TALER_PQ_QUERY_PARAM_AMOUNT (balance), + TALER_PQ_QUERY_PARAM_PTR_SIZED (details, strlen (details)), TALER_PQ_QUERY_PARAM_ABSOLUTE_TIME (expiry), TALER_PQ_QUERY_PARAM_END }; @@ -1051,41 +1082,18 @@ postgres_reserves_in_insert (void *cls, } PQclear (result); result = NULL; - if (GNUNET_NO == reserve_exists) - { - if (GNUNET_OK != postgres_commit (cls, - session)) - return GNUNET_SYSERR; - reserve->balance = *balance; - reserve->expiry = expiry; - return GNUNET_OK; - } - /* Update reserve */ - struct TALER_MINTDB_Reserve updated_reserve; - updated_reserve.pub = reserve->pub; - - if (GNUNET_OK != - TALER_amount_add (&updated_reserve.balance, - &reserve->balance, - balance)) - { - return GNUNET_SYSERR; - } - updated_reserve.expiry = GNUNET_TIME_absolute_max (expiry, - reserve->expiry); - if (GNUNET_OK != postgres_reserves_update (cls, - session, - &updated_reserve)) + if ( (GNUNET_YES == reserve_exists) && + (GNUNET_OK != postgres_reserves_update (cls, + session, + &updated_reserve)) ) goto rollback; if (GNUNET_OK != postgres_commit (cls, session)) return GNUNET_SYSERR; - reserve->balance = updated_reserve.balance; - reserve->expiry = updated_reserve.expiry; return GNUNET_OK; - rollback: - PQclear (result); + if (NULL != result) + PQclear (result); postgres_rollback (cls, session); return GNUNET_SYSERR; @@ -2426,7 +2434,6 @@ postgres_get_coin_transactions (void *cls, } - /** * Initialize Postgres database subsystem. *