View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0006200 | Taler | exchange | public | 2020-04-22 18:59 | 2021-09-02 16:14 |
| Reporter | oec | Assigned To | Christian Grothoff | Resolution deadline | |
| Implementer | Reviewer | ||||
| Release signed by | Deployed by | ||||
| Verifier | Communicator | ||||
| Data protection authority notification required | No | ||||
| CSIRT notification required | No | ||||
| Estimated effort (hours) | Actual effort (hours) | 0 | |||
| Fast-track | No | ||||
| Work package | none | ||||
| Priority | normal | Severity | tweak | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | ||
| Product Version | git (master) | ||||
| Target Version | 0.7.1 | Fixed in Version | 0.7.1 | ||
| Summary | 0006200: postgres_get_coin_transactions does not allow for multiple deposit transactions | ||||
| Description | If 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. | ||||
| Tags | No tags attached. | ||||
| 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? | |
| Ah, yes, confusing indeed. I agree with your suggestion to introduce chs.failed, that would make the logic clearer. | |
| Fixed in 09294481..a039bf4d | |
| Fix committed to master branch. | |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-04-22 18:59 | oec | New Issue | |
| 2020-04-22 18:59 | oec | Status | new => assigned |
| 2020-04-22 18:59 | oec | Assigned To | => Christian Grothoff |
| 2020-04-22 19:10 | Christian Grothoff | Note Added: 0015735 | |
| 2020-04-22 19:10 | Christian Grothoff | Status | assigned => feedback |
| 2020-04-22 19:19 | oec | Note Added: 0015736 | |
| 2020-04-22 19:19 | oec | Status | feedback => assigned |
| 2020-04-22 19:26 | Christian Grothoff | Note Added: 0015737 | |
| 2020-04-22 19:26 | Christian Grothoff | Status | assigned => informing |
| 2020-04-22 19:26 | Christian Grothoff | Resolution | open => fixed |
| 2020-04-22 19:26 | Christian Grothoff | Fixed in Version | => 0.7.1 |
| 2020-04-22 19:27 | Christian Grothoff | Product Version | => git (master) |
| 2020-04-22 19:27 | Christian Grothoff | Target Version | => 0.7.1 |
| 2020-04-22 19:27 | Christian Grothoff | Severity | major => tweak |
| 2021-08-24 14:23 | Christian Grothoff | Status | informing => closed |
| 2021-09-02 16:13 | Christian Grothoff | ||
| 2021-09-02 16:14 | Christian Grothoff | Note Added: 0018258 |