[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[taler-exchange] branch master updated: fix refund handling: allow refun
|
From: |
gnunet |
|
Subject: |
[taler-exchange] branch master updated: fix refund handling: allow refund increases for the same coin |
|
Date: |
Wed, 12 Aug 2020 13:03:01 +0200 |
This is an automated email from the git hooks/post-receive script.
grothoff pushed a commit to branch master
in repository exchange.
The following commit(s) were added to refs/heads/master by this push:
new 26f72f85 fix refund handling: allow refund increases for the same coin
26f72f85 is described below
commit 26f72f8572cf0d04cd0da718d34dad4ba479289c
Author: Christian Grothoff <christian@grothoff.org>
AuthorDate: Wed Aug 12 13:02:47 2020 +0200
fix refund handling: allow refund increases for the same coin
---
contrib/gana | 2 +-
src/exchange/taler-exchange-httpd.c | 4 +-
src/exchange/taler-exchange-httpd_refund.c | 301 ++++++++++++++++------------
src/exchangedb/plugin_exchangedb_postgres.c | 7 +-
src/include/taler_exchangedb_plugin.h | 5 +
src/lib/exchange_api_refund.c | 24 ++-
src/testing/test_exchange_api_twisted.c | 2 +-
7 files changed, 203 insertions(+), 142 deletions(-)
diff --git a/contrib/gana b/contrib/gana
index e8c9ce8c..73f61323 160000
--- a/contrib/gana
+++ b/contrib/gana
@@ -1 +1 @@
-Subproject commit e8c9ce8cb78b3594a1f4e6fe03c42eda5a08fb7e
+Subproject commit 73f61323554df47079e19cd4236d148e2c17a1b3
diff --git a/src/exchange/taler-exchange-httpd.c
b/src/exchange/taler-exchange-httpd.c
index c614b711..bca7a623 100644
--- a/src/exchange/taler-exchange-httpd.c
+++ b/src/exchange/taler-exchange-httpd.c
@@ -238,7 +238,7 @@ handle_post_coins (const struct TEH_RequestHandler *rh,
root);
return TALER_MHD_reply_with_error (connection,
MHD_HTTP_NOT_FOUND,
- TALER_EC_OPERATION_INVALID,
+ TALER_EC_OPERATION_UNKNOWN,
"requested operation on coin unknown");
}
@@ -1145,7 +1145,7 @@ run_main_loop (int fh,
(-1 == fh) ? serve_port : 0,
NULL, NULL,
&handle_mhd_request, NULL,
- MHD_OPTION_THREAD_POOL_SIZE, (unsigned int) 32,
+ MHD_OPTION_THREAD_POOL_SIZE, (unsigned int) 4,
MHD_OPTION_LISTEN_BACKLOG_SIZE, (unsigned int) 1024,
MHD_OPTION_LISTEN_SOCKET, fh,
MHD_OPTION_EXTERNAL_LOGGER, &TALER_MHD_handle_logs,
diff --git a/src/exchange/taler-exchange-httpd_refund.c
b/src/exchange/taler-exchange-httpd_refund.c
index ea261cb5..b60a93b5 100644
--- a/src/exchange/taler-exchange-httpd_refund.c
+++ b/src/exchange/taler-exchange-httpd_refund.c
@@ -105,12 +105,15 @@ refund_transaction (void *cls,
MHD_RESULT *mhd_ret)
{
const struct TALER_EXCHANGEDB_Refund *refund = cls;
- struct TALER_EXCHANGEDB_TransactionList *tl;
- const struct TALER_EXCHANGEDB_DepositListEntry *dep;
- const struct TALER_EXCHANGEDB_RefundListEntry *ref;
+ struct TALER_EXCHANGEDB_TransactionList *tl; /* head of original list */
+ struct TALER_EXCHANGEDB_TransactionList *tlx; /* head of sublist that
applies to merchant and contract */
+ struct TALER_EXCHANGEDB_TransactionList *tln; /* next element, during
iteration */
+ struct TALER_EXCHANGEDB_TransactionList *tlp; /* previous element in 'tl'
list, during iteration */
enum GNUNET_DB_QueryStatus qs;
- int deposit_found;
- int refund_found;
+ bool deposit_found; /* deposit_total initialized? */
+ bool refund_found; /* refund_total initialized? */
+ struct TALER_Amount deposit_total;
+ struct TALER_Amount refund_total;
tl = NULL;
qs = TEH_plugin->get_coin_transactions (TEH_plugin->cls,
@@ -127,71 +130,159 @@ refund_transaction (void *cls,
"database transaction failure");
return qs;
}
- dep = NULL;
- ref = NULL;
- deposit_found = GNUNET_NO;
- refund_found = GNUNET_NO;
- for (struct TALER_EXCHANGEDB_TransactionList *tlp = tl;
- NULL != tlp;
- tlp = tlp->next)
+ deposit_found = false;
+ refund_found = false;
+ tlx = NULL; /* relevant subset of transactions */
+ tln = NULL;
+ tlp = NULL;
+ for (struct TALER_EXCHANGEDB_TransactionList *tli = tl;
+ NULL != tli;
+ tli = tln)
{
- switch (tlp->type)
+ tln = tli->next;
+ switch (tli->type)
{
case TALER_EXCHANGEDB_TT_DEPOSIT:
- if (GNUNET_NO == deposit_found)
{
- if ( (0 == memcmp (&tlp->details.deposit->merchant_pub,
- &refund->details.merchant_pub,
- sizeof (struct TALER_MerchantPublicKeyP))) &&
- (0 == memcmp (&tlp->details.deposit->h_contract_terms,
- &refund->details.h_contract_terms,
- sizeof (struct GNUNET_HashCode))) )
+ const struct TALER_EXCHANGEDB_DepositListEntry *dep;
+
+ dep = tli->details.deposit;
+ if ( (0 == GNUNET_memcmp (&dep->merchant_pub,
+ &refund->details.merchant_pub)) &&
+ (0 == GNUNET_memcmp (&dep->h_contract_terms,
+ &refund->details.h_contract_terms)) )
{
- dep = tlp->details.deposit;
- deposit_found = GNUNET_YES;
+ /* check if we already send the money for this /deposit */
+ if (dep->done)
+ {
+ TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
+ tlx);
+ TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
+ tln);
+ /* money was already transferred to merchant, can no longer refund
*/
+ *mhd_ret = TALER_MHD_reply_with_error (connection,
+ MHD_HTTP_GONE,
+
TALER_EC_REFUND_MERCHANT_ALREADY_PAID,
+ "money already sent to
merchant");
+ return GNUNET_DB_STATUS_HARD_ERROR;
+ }
+
+ /* deposit applies and was not yet wired; add to total (it is NOT
+ the case that multiple deposits of the same coin for the same
+ contract are really allowed (see UNIQUE constraint on 'deposits'
+ table), but in case this changes we tolerate it with this code
+ anyway). *///
+ if (deposit_found)
+ {
+ GNUNET_assert (0 <=
+ TALER_amount_add (&deposit_total,
+ &deposit_total,
+ &dep->amount_with_fee));
+ }
+ else
+ {
+ deposit_total = dep->amount_with_fee;
+ deposit_found = true;
+ }
+ /* move 'tli' from 'tl' to 'tlx' list */
+ if (NULL == tlp)
+ tl = tln;
+ else
+ tlp->next = tln;
+ tli->next = tlx;
+ tlx = tli;
break;
}
+ else
+ {
+ tlp = tli;
+ }
+ break;
}
- break;
case TALER_EXCHANGEDB_TT_MELT:
/* Melts cannot be refunded, ignore here */
break;
case TALER_EXCHANGEDB_TT_REFUND:
- if (GNUNET_NO == refund_found)
{
- /* First, check if existing refund request is identical */
- if ( (0 == memcmp (&tlp->details.refund->merchant_pub,
- &refund->details.merchant_pub,
- sizeof (struct TALER_MerchantPublicKeyP))) &&
- (0 == memcmp (&tlp->details.refund->h_contract_terms,
- &refund->details.h_contract_terms,
- sizeof (struct GNUNET_HashCode))) &&
- (tlp->details.refund->rtransaction_id ==
- refund->details.rtransaction_id) )
+ const struct TALER_EXCHANGEDB_RefundListEntry *ref;
+
+ ref = tli->details.refund;
+ if ( (0 != GNUNET_memcmp (&ref->merchant_pub,
+ &refund->details.merchant_pub)) ||
+ (0 != GNUNET_memcmp (&ref->h_contract_terms,
+ &refund->details.h_contract_terms)) )
{
- ref = tlp->details.refund;
- refund_found = GNUNET_YES;
- break;
+ tlp = tli;
+ break; /* refund does not apply to our transaction */
}
- /* Second, check if existing refund request conflicts */
- if ( (0 == memcmp (&tlp->details.refund->merchant_pub,
- &refund->details.merchant_pub,
- sizeof (struct TALER_MerchantPublicKeyP))) &&
- (0 == memcmp (&tlp->details.refund->h_contract_terms,
- &refund->details.h_contract_terms,
- sizeof (struct GNUNET_HashCode))) &&
- (tlp->details.refund->rtransaction_id !=
- refund->details.rtransaction_id) )
+ /* Check if existing refund request matches in everything but the
amount */
+ if ( (ref->rtransaction_id ==
+ refund->details.rtransaction_id) &&
+ (0 != TALER_amount_cmp (&ref->refund_amount,
+ &refund->details.refund_amount)) )
{
- refund_found = GNUNET_SYSERR;
- /* NOTE: Alternatively we could total up all existing
- refunds and check if the sum still permits the
- refund requested (thus allowing multiple, partial
- refunds). Fow now, we keep it simple. */
- break;
+ /* Generate precondition failed response, with ONLY the conflicting
entry */
+ TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
+ tlx);
+ TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
+ tln);
+ tli->next = NULL;
+ *mhd_ret = TALER_MHD_reply_json_pack (
+ connection,
+ MHD_HTTP_PRECONDITION_FAILED,
+ "{s:s, s:I, s:o}",
+ "hint",
+ "conflicting refund with different amount but same refund
transaction ID",
+ "code", (json_int_t) TALER_EC_REFUND_INCONSISTENT_AMOUNT,
+ "history", TEH_RESPONSE_compile_transaction_history (
+ &refund->coin.coin_pub,
+ tli));
+ TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
+ tli);
+ return GNUNET_DB_STATUS_HARD_ERROR;
+ }
+ /* Check if existing refund request matches in everything including
the amount */
+ if ( (ref->rtransaction_id ==
+ refund->details.rtransaction_id) &&
+ (0 == TALER_amount_cmp (&ref->refund_amount,
+ &refund->details.refund_amount)) )
+ {
+ /* we can blanketly approve, as this request is identical to one
+ we saw before */
+ *mhd_ret = reply_refund_success (connection,
+ &refund->coin.coin_pub,
+ ref);
+ TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
+ tlx);
+ TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
+ tl);
+ /* we still abort the transaction, as there is nothing to be
+ committed! */
+ return GNUNET_DB_STATUS_HARD_ERROR;
+ }
+
+ /* We have another refund, that relates, add to total */
+ if (refund_found)
+ {
+ GNUNET_assert (0 <=
+ TALER_amount_add (&refund_total,
+ &refund_total,
+ &ref->refund_amount));
+ }
+ else
+ {
+ refund_total = ref->refund_amount;
+ refund_found = true;
}
+ /* move 'tli' from 'tl' to 'tlx' list */
+ if (NULL == tlp)
+ tl = tln;
+ else
+ tlp->next = tln;
+ tli->next = tlx;
+ tlx = tli;
+ break;
}
- break;
case TALER_EXCHANGEDB_TT_OLD_COIN_RECOUP:
/* Recoups cannot be refunded, ignore here */
break;
@@ -203,54 +294,30 @@ refund_transaction (void *cls,
break;
}
}
+ /* no need for 'tl' anymore, everything we may still care about is in tlx
now */
+ TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
+ tl);
/* handle if deposit was NOT found */
- if (GNUNET_NO == deposit_found)
+ if (! deposit_found)
{
TALER_LOG_WARNING ("Deposit to /refund was not found\n");
TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
- tl);
+ tlx);
*mhd_ret = TALER_MHD_reply_with_error (connection,
MHD_HTTP_NOT_FOUND,
TALER_EC_REFUND_DEPOSIT_NOT_FOUND,
"deposit unknown");
return GNUNET_DB_STATUS_HARD_ERROR;
}
- /* handle if conflicting refund found */
- if (GNUNET_SYSERR == refund_found)
- {
- *mhd_ret = TALER_MHD_reply_json_pack (
- connection,
- MHD_HTTP_CONFLICT,
- "{s:s, s:I, s:o}",
- "hint", "conflicting refund",
- "code", (json_int_t) TALER_EC_REFUND_CONFLICT,
- "history", TEH_RESPONSE_compile_transaction_history (
- &refund->coin.coin_pub,
- tl));
- TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
- tl);
- return GNUNET_DB_STATUS_HARD_ERROR;
- }
- /* handle if identical refund found */
- if (GNUNET_YES == refund_found)
- {
- /* /refund already done, simply re-transmit confirmation */
- *mhd_ret = reply_refund_success (connection,
- &refund->coin.coin_pub,
- ref);
- TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
- tl);
- return GNUNET_DB_STATUS_HARD_ERROR;
- }
/* check currency is compatible */
if (GNUNET_YES !=
TALER_amount_cmp_currency (&refund->details.refund_amount,
- &dep->amount_with_fee))
+ &deposit_total))
{
GNUNET_break_op (0); /* currency mismatch */
TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
- tl);
+ tlx);
*mhd_ret = TALER_MHD_reply_with_error (connection,
MHD_HTTP_BAD_REQUEST,
TALER_EC_REFUND_CURRENCY_MISMATCH,
@@ -258,56 +325,36 @@ refund_transaction (void *cls,
return GNUNET_DB_STATUS_HARD_ERROR;
}
- /* check if we already send the money for the /deposit */
- qs = TEH_plugin->test_deposit_done (TEH_plugin->cls,
- session,
- &refund->coin.coin_pub,
- &dep->merchant_pub,
- &dep->h_contract_terms,
- &dep->h_wire);
- if (GNUNET_DB_STATUS_HARD_ERROR == qs)
- {
- /* Internal error, we first had the deposit in the history,
- but now it is gone? */
- GNUNET_break (0);
- TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
- tl);
- *mhd_ret = TALER_MHD_reply_with_error (connection,
- MHD_HTTP_INTERNAL_SERVER_ERROR,
- TALER_EC_REFUND_DB_INCONSISTENT,
- "database inconsistent (deposit
data became inaccessible during transaction)");
- return qs;
- }
- if (GNUNET_DB_STATUS_SOFT_ERROR == qs)
- return qs; /* go and retry */
-
- if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT == qs)
- {
- /* money was already transferred to merchant, can no longer refund */
- TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
- tl);
- *mhd_ret = TALER_MHD_reply_with_error (connection,
- MHD_HTTP_GONE,
-
TALER_EC_REFUND_MERCHANT_ALREADY_PAID,
- "money already sent to merchant");
- return GNUNET_DB_STATUS_HARD_ERROR;
- }
-
- /* check refund amount is sufficiently low */
- if (1 == TALER_amount_cmp (&refund->details.refund_amount,
- &dep->amount_with_fee) )
+ /* check total refund amount is sufficiently low */
+ if (refund_found)
+ GNUNET_break (0 <=
+ TALER_amount_add (&refund_total,
+ &refund_total,
+ &refund->details.refund_amount));
+ else
+ refund_total = refund->details.refund_amount;
+
+ if (1 == TALER_amount_cmp (&refund_total,
+ &deposit_total) )
{
GNUNET_break_op (0); /* cannot refund more than original value */
+ *mhd_ret = TALER_MHD_reply_json_pack (
+ connection,
+ MHD_HTTP_CONFLICT,
+ "{s:s, s:I, s:o}",
+ "hint",
+ "total amount refunded exceeds total amount deposited for this coin",
+ "code", (json_int_t) TALER_EC_REFUND_CONFLICT_DEPOSIT_INSUFFICIENT,
+ "history", TEH_RESPONSE_compile_transaction_history (
+ &refund->coin.coin_pub,
+ tlx));
TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
- tl);
- *mhd_ret = TALER_MHD_reply_with_error (connection,
- MHD_HTTP_PRECONDITION_FAILED,
- TALER_EC_REFUND_INSUFFICIENT_FUNDS,
- "refund requested exceeds original
value");
+ tlx);
return GNUNET_DB_STATUS_HARD_ERROR;
}
TEH_plugin->free_coin_transaction_list (TEH_plugin->cls,
- tl);
+ tlx);
+
/* Finally, store new refund data */
qs = TEH_plugin->insert_refund (TEH_plugin->cls,
diff --git a/src/exchangedb/plugin_exchangedb_postgres.c
b/src/exchangedb/plugin_exchangedb_postgres.c
index 1dc58883..e3604ec0 100644
--- a/src/exchangedb/plugin_exchangedb_postgres.c
+++ b/src/exchangedb/plugin_exchangedb_postgres.c
@@ -986,6 +986,7 @@ postgres_get_session (void *cls)
",wire"
",coin_sig"
",deposit_serial_id"
+ ",done"
" FROM deposits"
" JOIN known_coins kc"
" USING (coin_pub)"
@@ -2178,7 +2179,7 @@ postgres_insert_withdraw_info (
return qs;
}
-#if 0
+#if 1
/* update reserve balance */
reserve.pub = collectable->reserve_pub;
if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
@@ -4124,6 +4125,7 @@ add_coin_deposit (void *cls,
chc->have_deposit_or_melt = true;
deposit = GNUNET_new (struct TALER_EXCHANGEDB_DepositListEntry);
{
+ uint8_t done = 0;
struct GNUNET_PQ_ResultSpec rs[] = {
TALER_PQ_RESULT_SPEC_AMOUNT ("amount_with_fee",
&deposit->amount_with_fee),
@@ -4149,6 +4151,8 @@ add_coin_deposit (void *cls,
&deposit->csig),
GNUNET_PQ_result_spec_uint64 ("deposit_serial_id",
&serial_id),
+ GNUNET_PQ_result_spec_auto_from_type ("done",
+ &done),
GNUNET_PQ_result_spec_end
};
@@ -4162,6 +4166,7 @@ add_coin_deposit (void *cls,
chc->failed = true;
return;
}
+ deposit->done = (0 != done);
}
tl = GNUNET_new (struct TALER_EXCHANGEDB_TransactionList);
tl->next = chc->head;
diff --git a/src/include/taler_exchangedb_plugin.h
b/src/include/taler_exchangedb_plugin.h
index f5e5dccc..4f27daef 100644
--- a/src/include/taler_exchangedb_plugin.h
+++ b/src/include/taler_exchangedb_plugin.h
@@ -665,6 +665,11 @@ struct TALER_EXCHANGEDB_DepositListEntry
*/
struct TALER_Amount deposit_fee;
+ /**
+ * Has the deposit been wired?
+ */
+ bool done;
+
};
diff --git a/src/lib/exchange_api_refund.c b/src/lib/exchange_api_refund.c
index 537be7b8..37c60e07 100644
--- a/src/lib/exchange_api_refund.c
+++ b/src/lib/exchange_api_refund.c
@@ -153,7 +153,6 @@ handle_refund_finished (void *cls,
.http_status = (unsigned int) response_code
};
-
rh->job = NULL;
switch (response_code)
{
@@ -179,7 +178,9 @@ handle_refund_finished (void *cls,
break;
case MHD_HTTP_BAD_REQUEST:
/* This should never happen, either us or the exchange is buggy
- (or API version conflict); just pass JSON reply to the application */
+ (or API version conflict); also can happen if the currency
+ differs (which we should obviously never support).
+ Just pass JSON reply to the application */
hr.ec = TALER_JSON_get_error_code (j);
hr.hint = TALER_JSON_get_error_hint (j);
break;
@@ -196,6 +197,13 @@ handle_refund_finished (void *cls,
hr.ec = TALER_JSON_get_error_code (j);
hr.hint = TALER_JSON_get_error_hint (j);
break;
+ case MHD_HTTP_CONFLICT:
+ /* Requested total refunds exceed deposited amount */
+ hr.ec = TALER_JSON_get_error_code (j);
+ hr.hint = TALER_JSON_get_error_hint (j);
+ // FIXME: check that 'history' contains a coin history that
+ // demonstrates that another refund would exceed the deposit amount!
+ break;
case MHD_HTTP_GONE:
/* Kind of normal: the money was already sent to the merchant
(it was too late for the refund). */
@@ -203,16 +211,12 @@ handle_refund_finished (void *cls,
hr.hint = TALER_JSON_get_error_hint (j);
break;
case MHD_HTTP_PRECONDITION_FAILED:
- /* Client request was inconsistent; might be a currency mismatch
- problem. */
- hr.ec = TALER_JSON_get_error_code (j);
- hr.hint = TALER_JSON_get_error_hint (j);
- break;
- case MHD_HTTP_CONFLICT:
- /* Two refund requests were made about the same deposit, but
- carrying different refund transaction ids. */
+ /* Two different refund requests were made about the same deposit, but
+ carrying identical refund transaction ids. */
hr.ec = TALER_JSON_get_error_code (j);
hr.hint = TALER_JSON_get_error_hint (j);
+ // FIXME: check that 'history' contains a duly signed refund request
+ // for the same rtransaction ID but a different amount!
break;
case MHD_HTTP_INTERNAL_SERVER_ERROR:
/* Server had an internal issue; we should retry, but this API
diff --git a/src/testing/test_exchange_api_twisted.c
b/src/testing/test_exchange_api_twisted.c
index 0b8de5b9..1028fb20 100644
--- a/src/testing/test_exchange_api_twisted.c
+++ b/src/testing/test_exchange_api_twisted.c
@@ -202,7 +202,7 @@ run (void *cls,
"EUR:5",
"deposit-refund-to-fail"),
TALER_TESTING_cmd_refund ("refund-insufficient-funds",
- MHD_HTTP_PRECONDITION_FAILED,
+ MHD_HTTP_CONFLICT,
"EUR:50",
"deposit-refund-1"),
TALER_TESTING_cmd_end ()
--
To stop receiving notification emails like this one, please contact
gnunet@gnunet.org.
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- [taler-exchange] branch master updated: fix refund handling: allow refund increases for the same coin,
gnunet <=