From 0328ab313b9194f10c154f74518089df9509fe09 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Fri, 17 Jan 2020 15:43:04 +0100 Subject: [PATCH] use more specific SELECT in exchange aggregator on refunds instead of filtering in application logic later --- src/exchange/taler-exchange-aggregator.c | 38 +++-------------- src/exchangedb/plugin_exchangedb_postgres.c | 47 ++++++++++----------- src/exchangedb/test_exchangedb.c | 43 ++----------------- src/include/taler_exchangedb_plugin.h | 22 +++------- 4 files changed, 39 insertions(+), 111 deletions(-) diff --git a/src/exchange/taler-exchange-aggregator.c b/src/exchange/taler-exchange-aggregator.c index df201d694..9032cd5e3 100644 --- a/src/exchange/taler-exchange-aggregator.c +++ b/src/exchange/taler-exchange-aggregator.c @@ -133,11 +133,6 @@ struct AggregationUnit */ struct GNUNET_HashCode h_wire; - /** - * Hash code of contract we are currently looking into. - */ - const struct GNUNET_HashCode *h_contract; - /** * Wire transfer identifier we use. */ @@ -678,36 +673,15 @@ exchange_serve_process_config () * the aggregation unit's total amount. * * @param cls closure with a `struct AggregationUnit *` - * @param merchant_pub public key of merchant who authorized refund - * @param merchant_sig signature of merchant authorizing refund - * @param h_contract hash of contract being refunded - * @param rtransaction_id refund transaction ID - * @param amount_with_fee amount being refunded - * @param refund_fee fee the exchange keeps for the refund processing + * @param amount_with_fee what was the refunded amount with the fee * @return #GNUNET_OK to continue to iterate, #GNUNET_SYSERR to stop */ static int refund_by_coin_cb (void *cls, - const struct TALER_MerchantPublicKeyP *merchant_pub, - const struct TALER_MerchantSignatureP *merchant_sig, - const struct GNUNET_HashCode *h_contract, - uint64_t rtransaction_id, - const struct TALER_Amount *amount_with_fee, - const struct TALER_Amount *refund_fee) + const struct TALER_Amount *amount_with_fee) { struct AggregationUnit *aux = cls; - (void) merchant_sig; - (void) rtransaction_id; - (void) refund_fee; - /* TODO: potential optimization: include these conditions - in the SELECT, and avoid fetching the values we do not need! */ - if (0 != GNUNET_memcmp (merchant_pub, - &aux->merchant_pub)) - return GNUNET_OK; /* different merchant */ - if (0 != GNUNET_memcmp (h_contract, - aux->h_contract)) - return GNUNET_OK; /* different contract */ GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Aggregator subtracts applicable refund of amount %s\n", TALER_amount2s (amount_with_fee)); @@ -776,13 +750,13 @@ deposit_cb (void *cls, TALER_B2S (coin_pub), TALER_amount2s (&au->total_amount)); au->row_id = row_id; - au->h_contract = h_contract_terms; qs = db_plugin->select_refunds_by_coin (db_plugin->cls, au->session, coin_pub, + &au->merchant_pub, + h_contract_terms, &refund_by_coin_cb, au); - au->h_contract = NULL; if (0 > qs) { GNUNET_break (GNUNET_DB_STATUS_SOFT_ERROR == qs); @@ -938,13 +912,13 @@ aggregate_cb (void *cls, return GNUNET_DB_STATUS_SUCCESS_ONE_RESULT; } - au->h_contract = h_contract_terms; qs = db_plugin->select_refunds_by_coin (db_plugin->cls, au->session, coin_pub, + &au->merchant_pub, + h_contract_terms, &refund_by_coin_cb, au); - au->h_contract = NULL; if (0 > qs) { GNUNET_break (GNUNET_DB_STATUS_SOFT_ERROR == qs); diff --git a/src/exchangedb/plugin_exchangedb_postgres.c b/src/exchangedb/plugin_exchangedb_postgres.c index 925f2853b..8a013c40a 100644 --- a/src/exchangedb/plugin_exchangedb_postgres.c +++ b/src/exchangedb/plugin_exchangedb_postgres.c @@ -736,6 +736,17 @@ postgres_get_session (void *cls) " JOIN denominations denom USING (denom_pub_hash)" " WHERE coin_pub=$1;", 1), + /* Query the 'refunds' by coin public key, merchant_pub and contract hash */ + GNUNET_PQ_make_prepare ("get_refunds_by_coin_and_contract", + "SELECT" + " amount_with_fee_val" + ",amount_with_fee_frac" + " FROM refunds" + " WHERE" + " coin_pub=$1" + " AND merchant_pub=$2" + " AND h_contract_terms=$3;", + 3), /* Fetch refunds with rowid '\geq' the given parameter */ GNUNET_PQ_make_prepare ("audit_get_refunds_incr", "SELECT" @@ -3296,25 +3307,10 @@ get_refunds_cb (void *cls, for (unsigned int i = 0; icb (srctx->cb_cls, - &merchant_pub, - &merchant_sig, - &h_contract, - rtransaction_id, - &amount_with_fee, - &refund_fee)) + &amount_with_fee)) return; } } /** - * Select refunds by @a coin_pub. + * Select refunds by @a coin_pub, @a merchant_pub and @a h_contract. * * @param cls closure of plugin * @param session database handle to use * @param coin_pub coin to get refunds for + * @param merchant_pub merchant to get refunds for + * @param h_contract_pub contract (hash) to get refunds for * @param cb function to call for each refund found * @param cb_cls closure for @a cb * @return query result status @@ -3355,13 +3348,19 @@ postgres_select_refunds_by_coin (void *cls, struct TALER_EXCHANGEDB_Session *session, const struct TALER_CoinSpendPublicKeyP *coin_pub, - TALER_EXCHANGEDB_RefundCoinCallback cb, + const struct + TALER_MerchantPublicKeyP *merchant_pub, + const struct GNUNET_HashCode *h_contract, + TALER_EXCHANGEDB_RefundCoinCallback + cb, void *cb_cls) { struct PostgresClosure *pg = cls; enum GNUNET_DB_QueryStatus qs; struct GNUNET_PQ_QueryParam params[] = { GNUNET_PQ_query_param_auto_from_type (coin_pub), + GNUNET_PQ_query_param_auto_from_type (merchant_pub), + GNUNET_PQ_query_param_auto_from_type (h_contract), GNUNET_PQ_query_param_end }; struct SelectRefundContext srctx = { @@ -3372,7 +3371,7 @@ postgres_select_refunds_by_coin (void *cls, }; qs = GNUNET_PQ_eval_prepared_multi_select (session->conn, - "get_refunds_by_coin", + "get_refunds_by_coin_and_contract", params, &get_refunds_cb, &srctx); diff --git a/src/exchangedb/test_exchangedb.c b/src/exchangedb/test_exchangedb.c index e9fe67b43..a880ce621 100644 --- a/src/exchangedb/test_exchangedb.c +++ b/src/exchangedb/test_exchangedb.c @@ -1412,60 +1412,21 @@ wire_missing_cb (void *cls, * to a particular coin. * * @param cls closure with the `struct TALER_EXCHANGEDB_Refund *` we expect to get - * @param merchant_pub public key of merchant who authorized refund - * @param merchant_sig signature of merchant authorizing refund - * @param h_contract hash of contract being refunded - * @param rtransaction_id refund transaction ID * @param amount_with_fee amount being refunded - * @param refund_fee fee the exchange keeps for the refund processing * @return #GNUNET_OK to continue to iterate, #GNUNET_SYSERR to stop */ static int check_refund_cb (void *cls, - const struct TALER_MerchantPublicKeyP *merchant_pub, - const struct TALER_MerchantSignatureP *merchant_sig, - const struct GNUNET_HashCode *h_contract, - uint64_t rtransaction_id, - const struct TALER_Amount *amount_with_fee, - const struct TALER_Amount *refund_fee) + const struct TALER_Amount *amount_with_fee) { const struct TALER_EXCHANGEDB_Refund *refund = cls; - if (0 != GNUNET_memcmp (merchant_pub, - &refund->details.merchant_pub)) - { - GNUNET_break (0); - result = 66; - } - if (0 != GNUNET_memcmp (merchant_sig, - &refund->details.merchant_sig)) - { - GNUNET_break (0); - result = 66; - } - if (0 != GNUNET_memcmp (h_contract, - &refund->details.h_contract_terms)) - { - GNUNET_break (0); - result = 66; - } - if (rtransaction_id != refund->details.rtransaction_id) - { - GNUNET_break (0); - result = 66; - } if (0 != TALER_amount_cmp (amount_with_fee, &refund->details.refund_amount)) { GNUNET_break (0); result = 66; } - if (0 != TALER_amount_cmp (refund_fee, - &refund->details.refund_fee)) - { - GNUNET_break (0); - result = 66; - } return GNUNET_OK; } @@ -1988,6 +1949,8 @@ run (void *cls) plugin->select_refunds_by_coin (plugin->cls, session, &refund.coin.coin_pub, + &refund.details.merchant_pub, + &refund.details.h_contract_terms, &check_refund_cb, &refund)); diff --git a/src/include/taler_exchangedb_plugin.h b/src/include/taler_exchangedb_plugin.h index 633640e09..83c18d63f 100644 --- a/src/include/taler_exchangedb_plugin.h +++ b/src/include/taler_exchangedb_plugin.h @@ -1094,28 +1094,16 @@ typedef int /** * Callback invoked with information about refunds applicable - * to a particular coin. + * to a particular coin and contract. * * @param cls closure - * @param merchant_pub public key of merchant who authorized refund - * @param merchant_sig signature of merchant authorizing refund - * @param h_contract hash of contract being refunded - * @param rtransaction_id refund transaction ID * @param amount_with_fee amount being refunded - * @param refund_fee fee the exchange keeps for the refund processing * @return #GNUNET_OK to continue to iterate, #GNUNET_SYSERR to stop */ typedef int (*TALER_EXCHANGEDB_RefundCoinCallback)(void *cls, const struct - TALER_MerchantPublicKeyP *merchant_pub, - const struct - TALER_MerchantSignatureP *merchant_sig, - const struct GNUNET_HashCode *h_contract, - uint64_t rtransaction_id, - const struct - TALER_Amount *amount_with_fee, - const struct TALER_Amount *refund_fee); + TALER_Amount *amount_with_fee); /** @@ -1947,11 +1935,13 @@ struct TALER_EXCHANGEDB_Plugin const struct TALER_EXCHANGEDB_Refund *refund); /** - * Select refunds by @a coin_pub. + * Select refunds by @a coin_pub, @a merchant_pub and @a h_contract. * * @param cls closure of plugin * @param session database handle to use * @param coin_pub coin to get refunds for + * @param merchant_pub merchant to get refunds for + * @param h_contract_pub contract (hash) to get refunds for * @param cb function to call for each refund found * @param cb_cls closure for @a cb * @return query result status @@ -1960,6 +1950,8 @@ struct TALER_EXCHANGEDB_Plugin (*select_refunds_by_coin)(void *cls, struct TALER_EXCHANGEDB_Session *session, const struct TALER_CoinSpendPublicKeyP *coin_pub, + const struct TALER_MerchantPublicKeyP *merchant_pub, + const struct GNUNET_HashCode *h_contract, TALER_EXCHANGEDB_RefundCoinCallback cb, void *cb_cls);