View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0009457 | Taler | merchant backend | public | 2025-01-13 16:37 | 2025-01-13 22:01 |
| Reporter | fefe | Assigned To | 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 | implementing | Resolution | open | ||
| Product Version | git (master) | ||||
| Target Version | post-1.0 | ||||
| Summary | 0009457: scary comment about double payment | ||||
| Description | This is in batch_deposit_transaction (taler-merchant-httpd_post-orders-ID-pay.c): ``` 930 /* NOTE: We might want to check if the order was fully paid concurrently 931 by some other wallet here, and if so, issue an auto-refund. Right now, 932 it is possible to over-pay if two wallets literally make a concurrent 933 payment, as the earlier check for 'paid' is not in the same transaction 934 scope as this 'insert' operation. */ ``` Has this been fixed elsewhere? That sounds like a pretty bad race condition that could be triggered, for example, if there is a network outage, a customer tries to pay again, and then when the network comes back up both transactions are retried automatically. | ||||
| Tags | No tags attached. | ||||
| Well, I think the comment is more scary than reality. Not because it is false, but because of the context: - Before a wallet can pay, it needs to "claim" the contract (using the /claim endpoint). - Only one wallet can claim an order, only that wallet gets the contract terms. - Regular wallets would never attempt to pay an order for which they didn't get the contract terms (would be like sending money without knowing how much or for what). I didn't check if we validate that *this* wallet did the claim before in this endpoint, but at least in regular use we can be pretty sure wallets wouldn't pay contracts they didn't claim (and thus don't know the terms). So the only "practical" situation where this may still matter is if a wallet is backed up, restored to another device, filled with different coins (some _other_ transactions are made so that different coins are chosen), and then the user does a concurrent payment on both devices and manages to get the race. VERY, very unlikely. That said, the FIXME is correct and we probably should eventually do something about it. | |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2025-01-13 16:37 | fefe | New Issue | |
| 2025-01-13 16:37 | fefe | Status | new => assigned |
| 2025-01-13 16:37 | fefe | Assigned To | => Christian Grothoff |
| 2025-01-13 22:00 | Christian Grothoff | Note Added: 0023976 | |
| 2025-01-13 22:01 | Christian Grothoff | Assigned To | Christian Grothoff => |
| 2025-01-13 22:01 | Christian Grothoff | Severity | minor => tweak |
| 2025-01-13 22:01 | Christian Grothoff | Status | assigned => implementing |
| 2025-01-13 22:01 | Christian Grothoff | Product Version | => git (master) |
| 2025-01-13 22:01 | Christian Grothoff | Target Version | => post-1.0 |
| 2025-03-22 13:09 | Christian Grothoff | Category | mechant backend => merchant backend |