View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0006200Talerexchangepublic2020-04-22 20:592021-09-02 18:14
Reporteroec Assigned ToChristian Grothoff  
PrioritynormalSeveritytweakReproducibilityhave not tried
Status closedResolutionfixed 
Product Versiongit (master) 
Target Version0.7.1Fixed in Version0.7.1 
Summary0006200: postgres_get_coin_transactions does not allow for multiple deposit transactions
DescriptionIf I understand correctly, a coin can be used in multiple deposit-transactions. The exchange has to ensure that all transactions involving the same coin do not exceed the total value of the coin.

For this purpose, the deposit handler calls TEH_plugin->get_coin_transactions which is basically a call to postgres_get_coin_transactions. That function calls a list of prepared-statements and the results are handled by callbacks:

(from exchange/src/exchangedb/plugin_exchangedb_postgres.c)

```
4488 for (unsigned int i = 0; NULL != work[i].statement; i++)
4489 {
4490 qs = GNUNET_PQ_eval_prepared_multi_select (session->conn,
4491 work[i].statement,
4492 params,
4493 work[i].cb,
4494 &chc);
4495 if ( (0 > qs) ||
4496 (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != chc.status) )
4497 {
4498 if (NULL != chc.head)
4499 common_free_coin_transaction_list (cls,
4500 chc.head);
4501 *tlp = NULL;
4502 if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != chc.status)
4503 qs = chc.status;
4504 return qs;
4505 }
4506 }
```

In line 4496, it is required that each statement returns GNUNET_DB_STATUS_SUCCESS_ONE_RESULT. However, at least one of the callbacks, like `add_coin_deposit`, could have legitimately returned more than one result.

So it seems that `postgres_get_coin_transactions` would fail when multiple deposits of the same coin (with different fractions not summing up to the total value of the coin) would be found in the deposit-table.
TagsNo tags attached.

Activities

Christian Grothoff

2020-04-22 21:10

manager   ~0015735

I think you are mistaken. Admittedly, you could legitimately call the code confusing here.

chc->status is initialized to 1 ( .status = GNUNET_DB_STATUS_SUCCESS_ONE_RESULT). -- even if there are zero results.
Then, in the loop, all callbacks ONLY ever set chc->status to GNUNET_DB_STATUS_HARD_ERROR. The other possibilities (num results, soft error, no results) are never used for this 'status'.

We COULD change 'status' to a bool (maybe call it "failed") and then change

      if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != chc.status)
        qs = chc.status;

to

      if (chc.failed)
        qs = GNUNET_DB_STATUS_HARD_ERROR;

I think that would be _clearer_ in terms of the logic -- albeit without actually changing the (already correct) semantics.

Agreed?

oec

2020-04-22 21:19

developer   ~0015736

Ah, yes, confusing indeed.

I agree with your suggestion to introduce chs.failed, that would make the logic clearer.

Christian Grothoff

2020-04-22 21:26

manager   ~0015737

Fixed in 09294481..a039bf4d

Christian Grothoff

2021-09-02 18:14

manager   ~0018258

Fix committed to master branch.

Related Changesets

exchange: master a039bf4d

2020-04-22 23:21

Christian Grothoff


Details Diff		 fix 0006200 Affected Issues
0006200
mod - src/exchangedb/plugin_exchangedb_postgres.c Diff File

Issue History

Date Modified Username Field Change
2020-04-22 20:59 oec New Issue
2020-04-22 20:59 oec Status new => assigned
2020-04-22 20:59 oec Assigned To => Christian Grothoff
2020-04-22 21:10 Christian Grothoff Note Added: 0015735
2020-04-22 21:10 Christian Grothoff Status assigned => feedback
2020-04-22 21:19 oec Note Added: 0015736
2020-04-22 21:19 oec Status feedback => assigned
2020-04-22 21:26 Christian Grothoff Note Added: 0015737
2020-04-22 21:26 Christian Grothoff Status assigned => resolved
2020-04-22 21:26 Christian Grothoff Resolution open => fixed
2020-04-22 21:26 Christian Grothoff Fixed in Version => 0.7.1
2020-04-22 21:27 Christian Grothoff Product Version => git (master)
2020-04-22 21:27 Christian Grothoff Target Version => 0.7.1
2020-04-22 21:27 Christian Grothoff Severity major => tweak
2021-08-24 16:23 Christian Grothoff Status resolved => closed
2021-09-02 18:13 Christian Grothoff Changeset attached => Taler-exchange master a039bf4d
2021-09-02 18:14 Christian Grothoff Note Added: 0018258