From d4884c0c605db10934f7bb378072a21ecb523d12 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Fri, 23 Jun 2017 13:16:12 +0200 Subject: Fix #5010 for keystate --- src/exchange/taler-exchange-httpd_db.c | 28 +-- src/exchange/taler-exchange-httpd_keystate.c | 212 ++++++++++----------- src/exchangedb/perf_taler_exchangedb_interpreter.c | 12 +- src/exchangedb/plugin_exchangedb_postgres.c | 145 ++++---------- src/exchangedb/test_exchangedb.c | 6 +- src/include/taler_exchangedb_plugin.h | 12 +- 6 files changed, 171 insertions(+), 244 deletions(-) (limited to 'src') diff --git a/src/exchange/taler-exchange-httpd_db.c b/src/exchange/taler-exchange-httpd_db.c index 9eeda6ee0..f2e1f7b39 100644 --- a/src/exchange/taler-exchange-httpd_db.c +++ b/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, - TALER_EC_DB_SETUP_FAILED); + if (NULL != mhd_ret) + *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++) @@ -70,9 +72,10 @@ TEH_DB_run_transaction (struct MHD_Connection *connection, TEH_plugin->start (TEH_plugin->cls, session)) { - GNUNET_break (0); - *mhd_ret = TEH_RESPONSE_reply_internal_db_error (connection, - TALER_EC_DB_START_FAILED); + GNUNET_break (0); + if (NULL != mhd_ret) + *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, - TALER_EC_DB_COMMIT_FAILED_HARD); + if (NULL != mhd_ret) + *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, - TALER_EC_DB_COMMIT_FAILED_ON_RETRY); + if (NULL != mhd_ret) + *mhd_ret = TEH_RESPONSE_reply_commit_error (connection, + TALER_EC_DB_COMMIT_FAILED_ON_RETRY); return GNUNET_SYSERR; } diff --git a/src/exchange/taler-exchange-httpd_keystate.c b/src/exchange/taler-exchange-httpd_keystate.c index 7c499b00b..7cfd13d48 100644 --- a/src/exchange/taler-exchange-httpd_keystate.c +++ b/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; - qs = GNUNET_DB_STATUS_SOFT_ERROR; - while (0 > qs) + arc.dki = dki; + arc.revocation_master_sig = revocation_master_sig; + if (GNUNET_OK != + TEH_DB_run_transaction (NULL, + NULL, + &add_revocations_transaction, + &arc)) { - thresh++; - if ( (thresh > 16) || - (GNUNET_DB_STATUS_HARD_ERROR == qs) ) - { - 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_log (GNUNET_ERROR_TYPE_ERROR, + "Giving up, this is fatal. Committing suicide via SIGTERM.\n"); + handle_signal (SIGTERM); + return GNUNET_SYSERR; } + 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)); - - session = TEH_plugin->get_session (TEH_plugin->cls); - if (NULL == session) - return GNUNET_SYSERR; - /* Try to insert DKI into DB until we succeed; note that if the DB - 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) + if (GNUNET_OK != + TEH_DB_run_transaction (NULL, + NULL, + &add_denomination_transaction, + (void *) dki)) { - thresh++; - if ( (thresh > 16) || - (GNUNET_DB_STATUS_HARD_ERROR == qs) ) - { - 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. */ + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Giving up, this is fatal. Committing suicide via SIGTERM.\n"); + handle_signal (SIGTERM); + return GNUNET_SYSERR; } res = store_in_map (ctx->denomkey_map, diff --git a/src/exchangedb/perf_taler_exchangedb_interpreter.c b/src/exchangedb/perf_taler_exchangedb_interpreter.c index 7a5915807..7ec958c48 100644 --- a/src/exchangedb/perf_taler_exchangedb_interpreter.c +++ b/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, - state->session, - &data->data.dki->denom_pub, - &data->data.dki->issue); - GNUNET_assert (GNUNET_SYSERR != ret); + qs = state->plugin->get_denomination_info (state->plugin->cls, + state->session, + &data->data.dki->denom_pub, + &data->data.dki->issue); + GNUNET_assert (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT == qs); } break; diff --git a/src/exchangedb/plugin_exchangedb_postgres.c b/src/exchangedb/plugin_exchangedb_postgres.c index 9df7fc4f3..2a47f2503 100644 --- a/src/exchangedb/plugin_exchangedb_postgres.c +++ b/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 - * @return #GNUNET_OK on success; #GNUNET_NO if no record was found, #GNUNET_SYSERR on failure + * @param[out] issue set to issue information with value, fees and other info about the coin + * @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, - "denomination_get", - params); - if (PGRES_TUPLES_OK != PQresultStatus (result)) - { - QUERY_ERR (result, - 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); + qs = GNUNET_PQ_eval_prepared_singleton_select (session->conn, + "denomination_get", + params, + rs); + if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs) + return qs; 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, - * #GNUNET_NO if the entry already exists (transaction must be rolled back!) - * #GNUNET_SYSERR on DB errors + * @return transaction status code */ -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, - "denomination_revocation_insert", - 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; + return GNUNET_PQ_eval_prepared_non_select (session->conn, + "denomination_revocation_insert", + params); } diff --git a/src/exchangedb/test_exchangedb.c b/src/exchangedb/test_exchangedb.c index 1b3797dd3..f79839246 100644 --- a/src/exchangedb/test_exchangedb.c +++ b/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, diff --git a/src/include/taler_exchangedb_plugin.h b/src/include/taler_exchangedb_plugin.h index b586f2766..7b7fffef2 100644 --- a/src/include/taler_exchangedb_plugin.h +++ b/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 - * @return #GNUNET_OK on success; #GNUNET_NO if no record was found, #GNUNET_SYSERR on failure + * @param[out] issue set to issue information with value, fees and other info about the coin + * @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, - * #GNUNET_NO if the entry already exists (transaction must be rolled back!) - * #GNUNET_SYSERR on DB errors + * @return transaction status code */ - int + enum GNUNET_DB_QueryStatus (*insert_denomination_revocation)(void *cls, struct TALER_EXCHANGEDB_Session *session, const struct GNUNET_HashCode *denom_pub_hash, -- cgit v1.2.3