[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[taler-exchange] branch master updated: finish review of coins auditor a
|
From: |
gnunet |
|
Subject: |
[taler-exchange] branch master updated: finish review of coins auditor analysis logic, improve error handling |
|
Date: |
Mon, 23 Mar 2020 22:27:35 +0100 |
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 240b2368 finish review of coins auditor analysis logic, improve error
handling
240b2368 is described below
commit 240b23684d3f2b05b903b37f0a5fe3fc9e07497d
Author: Christian Grothoff <address@hidden>
AuthorDate: Mon Mar 23 22:27:31 2020 +0100
finish review of coins auditor analysis logic, improve error handling
---
src/auditor/taler-helper-auditor-coins.c | 364 +++++++++++++++++--------------
1 file changed, 196 insertions(+), 168 deletions(-)
diff --git a/src/auditor/taler-helper-auditor-coins.c
b/src/auditor/taler-helper-auditor-coins.c
index af417215..5ce3e77e 100644
--- a/src/auditor/taler-helper-auditor-coins.c
+++ b/src/auditor/taler-helper-auditor-coins.c
@@ -910,13 +910,13 @@ reveal_data_cb (void *cls,
report_row_inconsistency ("refresh_reveal",
rctx->rowid,
"denomination key not found");
- rctx->err = GNUNET_NO; /* terminate, but return "OK" */
+ rctx->err = GNUNET_NO; /* terminate here, but return "OK" to commit
transaction */
}
else if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs)
{
GNUNET_break (GNUNET_DB_STATUS_SOFT_ERROR == qs);
rctx->qs = qs;
- rctx->err = GNUNET_SYSERR; /* terminate, return GNUNET_SYSERR */
+ rctx->err = GNUNET_SYSERR; /* terminate, return #GNUNET_SYSERR: abort
transaction */
}
}
}
@@ -1101,9 +1101,9 @@ refresh_session_cb (void *cls,
if ( (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs) ||
(0 == reveal_ctx.num_freshcoins) )
{
- /* This can happen if reveal was not yet called or only
- with invalid data, even if the exchange is correctly
- operating. We still report it. */
+ /* This can legitimately happen if reveal was not yet called or only
+ with invalid data, even if the exchange is correctly operating. We
+ still report it. */
TALER_ARL_report (report_refreshs_hanging,
json_pack ("{s:I, s:o, s:o}",
"row", (json_int_t) rowid,
@@ -1162,15 +1162,22 @@ refresh_session_cb (void *cls,
amount_with_fee,
&melt_fee))
{
- // FIXME: handle properly!
- GNUNET_break (0);
- cc->qs = GNUNET_DB_STATUS_HARD_ERROR;
- GNUNET_free_non_null (reveal_ctx.new_issues);
- return GNUNET_SYSERR;
+ /* Melt fee higher than contribution of melted coin; this makes
+ no sense (exchange should never have accepted the operation) */
+ report_amount_arithmetic_inconsistency ("melt contribution vs. fee",
+ rowid,
+ amount_with_fee,
+ &melt_fee,
+ -1);
+ /* To continue, best assumption is the melted coin contributed
+ nothing (=> all withdrawal amounts will be counted as losses) */
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_get_zero (TALER_ARL_currency,
+ &amount_without_fee));
}
}
- /* check old coin covers complete expenses */
+ /* check old coin covers complete expenses (of withdraw operations) */
if (1 == TALER_amount_cmp (&refresh_cost,
&amount_without_fee))
{
@@ -1184,7 +1191,7 @@ refresh_session_cb (void *cls,
return GNUNET_OK;
}
- /* update outstanding denomination amounts */
+ /* update outstanding denomination amounts for fresh coins withdrawn */
for (unsigned int i = 0; i<reveal_ctx.num_freshcoins; i++)
{
struct DenominationSummary *dsi;
@@ -1195,36 +1202,40 @@ refresh_session_cb (void *cls,
&reveal_ctx.new_issues[i]->denom_hash);
if (NULL == dsi)
{
- GNUNET_break (0);
- return GNUNET_SYSERR;
+ report_row_inconsistency ("refresh_reveal",
+ rowid,
+ "denomination key for fresh coin unknown to
auditor");
+ }
+ else
+ {
+ TALER_amount_ntoh (&value,
+ &reveal_ctx.new_issues[i]->value);
+ GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+ "Created fresh coin in denomination `%s' of value %s\n",
+ GNUNET_h2s (&reveal_ctx.new_issues[i]->denom_hash),
+ TALER_amount2s (&value));
+ dsi->num_issued++;
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&dsi->denom_balance,
+ &dsi->denom_balance,
+ &value));
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&dsi->denom_risk,
+ &dsi->denom_risk,
+ &value));
+ GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+ "New balance of denomination `%s' is %s\n",
+ GNUNET_h2s (&reveal_ctx.new_issues[i]->denom_hash),
+ TALER_amount2s (&dsi->denom_balance));
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&total_escrow_balance,
+ &total_escrow_balance,
+ &value));
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&total_risk,
+ &total_risk,
+ &value));
}
- TALER_amount_ntoh (&value,
- &reveal_ctx.new_issues[i]->value);
- GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
- "Created fresh coin in denomination `%s' of value %s\n",
- GNUNET_h2s (&reveal_ctx.new_issues[i]->denom_hash),
- TALER_amount2s (&value));
- dsi->num_issued++;
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&dsi->denom_balance,
- &dsi->denom_balance,
- &value));
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&dsi->denom_risk,
- &dsi->denom_risk,
- &value));
- GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
- "New balance of denomination `%s' is %s\n",
- GNUNET_h2s (&reveal_ctx.new_issues[i]->denom_hash),
- TALER_amount2s (&dsi->denom_balance));
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&total_escrow_balance,
- &total_escrow_balance,
- &value));
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&total_risk,
- &total_risk,
- &value));
}
GNUNET_free_non_null (reveal_ctx.new_issues);
}
@@ -1235,53 +1246,56 @@ refresh_session_cb (void *cls,
&issue->denom_hash);
if (NULL == dso)
{
- // FIXME: handle more nicely!?!
- GNUNET_break (0);
- return GNUNET_SYSERR;
- }
- if (GNUNET_SYSERR ==
- TALER_amount_subtract (&tmp,
- &dso->denom_balance,
- amount_with_fee))
- {
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&dso->denom_loss,
- &dso->denom_loss,
- amount_with_fee));
- dso->report_emergency = GNUNET_YES;
- }
- else
- {
- dso->denom_balance = tmp;
- }
- if (-1 == TALER_amount_cmp (&total_escrow_balance,
- amount_with_fee))
- {
- /* This can theoretically happen if for example the exchange
- never issued any coins (i.e. escrow balance is zero), but
- accepted a forged coin (i.e. emergency situation after
- private key compromise). In that case, we cannot even
- subtract the profit we make from the fee from the escrow
- balance. Tested as part of test-auditor.sh, case #18
*/report_amount_arithmetic_inconsistency (
- "subtracting refresh fee from escrow balance",
- rowid,
- &total_escrow_balance,
- amount_with_fee,
- 0);
+ report_row_inconsistency ("refresh_reveal",
+ rowid,
+ "denomination key for dirty coin unknown to
auditor");
}
else
{
- GNUNET_assert (GNUNET_SYSERR !=
- TALER_amount_subtract (&total_escrow_balance,
- &total_escrow_balance,
- amount_with_fee));
+ if (GNUNET_SYSERR ==
+ TALER_amount_subtract (&tmp,
+ &dso->denom_balance,
+ amount_with_fee))
+ {
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&dso->denom_loss,
+ &dso->denom_loss,
+ amount_with_fee));
+ dso->report_emergency = GNUNET_YES;
+ }
+ else
+ {
+ dso->denom_balance = tmp;
+ }
+ if (-1 == TALER_amount_cmp (&total_escrow_balance,
+ amount_with_fee))
+ {
+ /* This can theoretically happen if for example the exchange
+ never issued any coins (i.e. escrow balance is zero), but
+ accepted a forged coin (i.e. emergency situation after
+ private key compromise). In that case, we cannot even
+ subtract the profit we make from the fee from the escrow
+ balance. Tested as part of test-auditor.sh, case #18 *///
+ report_amount_arithmetic_inconsistency (
+ "subtracting refresh fee from escrow balance",
+ rowid,
+ &total_escrow_balance,
+ amount_with_fee,
+ 0);
+ }
+ else
+ {
+ GNUNET_assert (GNUNET_SYSERR !=
+ TALER_amount_subtract (&total_escrow_balance,
+ &total_escrow_balance,
+ amount_with_fee));
+ }
+ GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+ "New balance of denomination `%s' after melt is %s\n",
+ GNUNET_h2s (&issue->denom_hash),
+ TALER_amount2s (&dso->denom_balance));
}
- GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
- "New balance of denomination `%s' after melt is %s\n",
- GNUNET_h2s (&issue->denom_hash),
- TALER_amount2s (&dso->denom_balance));
-
/* update global melt fees */
{
struct TALER_Amount rfee;
@@ -1440,54 +1454,57 @@ deposit_cb (void *cls,
&issue->denom_hash);
if (NULL == ds)
{
- GNUNET_break (0);
- // FIXME: handle/report more nicely!??!
- return GNUNET_SYSERR;
- }
- if (GNUNET_SYSERR ==
- TALER_amount_subtract (&tmp,
- &ds->denom_balance,
- amount_with_fee))
- {
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&ds->denom_loss,
- &ds->denom_loss,
- amount_with_fee));
- ds->report_emergency = GNUNET_YES;
+ report_row_inconsistency ("deposit",
+ rowid,
+ "denomination key for deposited coin unknown to
auditor");
}
else
{
- ds->denom_balance = tmp;
- }
+ if (GNUNET_SYSERR ==
+ TALER_amount_subtract (&tmp,
+ &ds->denom_balance,
+ amount_with_fee))
+ {
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&ds->denom_loss,
+ &ds->denom_loss,
+ amount_with_fee));
+ ds->report_emergency = GNUNET_YES;
+ }
+ else
+ {
+ ds->denom_balance = tmp;
+ }
- if (-1 == TALER_amount_cmp (&total_escrow_balance,
- amount_with_fee))
- {
- /* This can theoretically happen if for example the exchange
- never issued any coins (i.e. escrow balance is zero), but
- accepted a forged coin (i.e. emergency situation after
- private key compromise). In that case, we cannot even
- subtract the profit we make from the fee from the escrow
- balance. Tested as part of test-auditor.sh, case #18 *///
- report_amount_arithmetic_inconsistency (
- "subtracting deposit fee from escrow balance",
- rowid,
- &total_escrow_balance,
- amount_with_fee,
- 0);
- }
- else
- {
- GNUNET_assert (GNUNET_SYSERR !=
- TALER_amount_subtract (&total_escrow_balance,
- &total_escrow_balance,
- amount_with_fee));
- }
+ if (-1 == TALER_amount_cmp (&total_escrow_balance,
+ amount_with_fee))
+ {
+ /* This can theoretically happen if for example the exchange
+ never issued any coins (i.e. escrow balance is zero), but
+ accepted a forged coin (i.e. emergency situation after
+ private key compromise). In that case, we cannot even
+ subtract the profit we make from the fee from the escrow
+ balance. Tested as part of test-auditor.sh, case #18 *///
+ report_amount_arithmetic_inconsistency (
+ "subtracting deposit fee from escrow balance",
+ rowid,
+ &total_escrow_balance,
+ amount_with_fee,
+ 0);
+ }
+ else
+ {
+ GNUNET_assert (GNUNET_SYSERR !=
+ TALER_amount_subtract (&total_escrow_balance,
+ &total_escrow_balance,
+ amount_with_fee));
+ }
- GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
- "New balance of denomination `%s' after deposit is %s\n",
- GNUNET_h2s (&issue->denom_hash),
- TALER_amount2s (&ds->denom_balance));
+ GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+ "New balance of denomination `%s' after deposit is %s\n",
+ GNUNET_h2s (&issue->denom_hash),
+ TALER_amount2s (&ds->denom_balance));
+ }
/* update global deposit fees */
{
@@ -1622,31 +1639,33 @@ refund_cb (void *cls,
&issue->denom_hash);
if (NULL == ds)
{
- GNUNET_break (0);
- // FIXME: handle more nicely!?!?
- return GNUNET_SYSERR;
+ report_row_inconsistency ("refund",
+ rowid,
+ "denomination key for refunded coin unknown to
auditor");
+ }
+ else
+ {
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&ds->denom_balance,
+ &ds->denom_balance,
+ &amount_without_fee));
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&ds->denom_risk,
+ &ds->denom_risk,
+ &amount_without_fee));
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&total_escrow_balance,
+ &total_escrow_balance,
+ &amount_without_fee));
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&total_risk,
+ &total_risk,
+ &amount_without_fee));
+ GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+ "New balance of denomination `%s' after refund is %s\n",
+ GNUNET_h2s (&issue->denom_hash),
+ TALER_amount2s (&ds->denom_balance));
}
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&ds->denom_balance,
- &ds->denom_balance,
- &amount_without_fee));
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&ds->denom_risk,
- &ds->denom_risk,
- &amount_without_fee));
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&total_escrow_balance,
- &total_escrow_balance,
- &amount_without_fee));
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&total_risk,
- &total_risk,
- &amount_without_fee));
- GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
- "New balance of denomination `%s' after refund is %s\n",
- GNUNET_h2s (&issue->denom_hash),
- TALER_amount2s (&ds->denom_balance));
-
/* update total refund fee balance */
GNUNET_assert (GNUNET_OK ==
TALER_amount_add (&total_refund_fee_income,
@@ -1743,26 +1762,35 @@ check_recoup (struct CoinContext *cc,
ds = get_denomination_summary (cc,
issue,
&issue->denom_hash);
- if (GNUNET_NO == ds->was_revoked)
+ if (NULL == ds)
{
- /* Woopsie, we allowed recoup on non-revoked denomination!? */
- TALER_ARL_report (report_bad_sig_losses,
- json_pack ("{s:s, s:I, s:o, s:o}",
- "operation",
- "recoup (denomination not revoked)",
- "row", (json_int_t) rowid,
- "loss", TALER_JSON_from_amount (amount),
- "coin_pub", GNUNET_JSON_from_data_auto (
- &coin->coin_pub)));
+ report_row_inconsistency ("recoup",
+ rowid,
+ "denomination key for recouped coin unknown to
auditor");
+ }
+ else
+ {
+ if (GNUNET_NO == ds->was_revoked)
+ {
+ /* Woopsie, we allowed recoup on non-revoked denomination!? */
+ TALER_ARL_report (report_bad_sig_losses,
+ json_pack ("{s:s, s:I, s:o, s:o}",
+ "operation",
+ "recoup (denomination not revoked)",
+ "row", (json_int_t) rowid,
+ "loss", TALER_JSON_from_amount (amount),
+ "coin_pub", GNUNET_JSON_from_data_auto (
+ &coin->coin_pub)));
+ }
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&ds->denom_recoup,
+ &ds->denom_recoup,
+ amount));
+ GNUNET_assert (GNUNET_OK ==
+ TALER_amount_add (&total_recoup_loss,
+ &total_recoup_loss,
+ amount));
}
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&ds->denom_recoup,
- &ds->denom_recoup,
- amount));
- GNUNET_assert (GNUNET_OK ==
- TALER_amount_add (&total_recoup_loss,
- &total_recoup_loss,
- amount));
return GNUNET_OK;
}
--
To stop receiving notification emails like this one, please contact
address@hidden.
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- [taler-exchange] branch master updated: finish review of coins auditor analysis logic, improve error handling,
gnunet <=