View Issue Details

IDProjectCategoryView StatusLast Update
0006413Talerauditorpublic2021-08-24 16:23
Reporteroec Assigned ToChristian Grothoff  
Status closedResolutionfixed 
Product Versiongit (master) 
Target Version0.8Fixed in Version0.8 
Summary0006413: Not enough checks for deposits
Descriptiontaler-helper-auditor-deposits.c implements only one check for the deposits: "deposit confirmation missing".

I suggest to add at least the following checks:
- compare `refund_deadline` and `wire_deadline` to timestamp
- `coin_pub` and `coin_sig` are sound
- `amount_with_fee_val` and `amount_with_fee_frac` are sound
- `merchant_pub` points to an existing merchant

In comparison, for aggregation, coins, wire and reserves there is an average of 20 distict checks implemented.
TagsNo tags attached.


Christian Grothoff

2020-06-24 13:45

manager   ~0016351

- refund_deadline and wire_deadline and timestamp are (almost) orthogonal. You can
  legitimately set them far in the future, and I think the only constraint that applies is
  that refund_deadline <= wire_deadline here. What exactly would you want to check
  that certainly holds?
- coin_pub and coin_sig: these are checked in taler-helper-auditor-coins. It seems
  strictly wasteful to check the same signature multiple times, and would also result
  in duplicate reports (which doesn't make them easier to understand).
- amount_with_fee*-soundness: not sure which check you want to see, but again
  I wonder if this is not checked elsewhere.
- merchant_pub: what do you mean by "existing merchant"? We don't have a
  merchant registry. So very unclear to me what you would want to check here.
  The merchant_pub primarily is of interest because that is the key used to sign
  _refunds_. So we don't really care if it 'exists', the merchant could use a different
   merchant_pub for every deposit.


2020-06-25 10:55

developer   ~0016362

- As long as the tests for coin_pub/con_sig is implemented _somewhere_, that's fine. However, I suggest to put a description and reference to the location in taler-helper-auditor-deposits.c

- amount_with_fee*, same: If there are checks implemented somewhere, a note in the taler-*-deposits.c would be helpful

- merchant_pub: I see. For some reason I thought that merchants are registered. Would it make sense to check the refunds to be signed with a merchant_pub that is listed in a deposit?

Christian Grothoff

2020-06-25 11:05

manager   ~0016365

Merchants are not registered, we want the system to be open. Forcing registration on merchants is not desirable, as that would allow an exchange to refuse to do business with some merchants, and we do not believe that this is socially desirable.

HOWEVER, it _may_ be that in the future _SOME_ form of registration is required from a regulatory perspective (KYC). *IF* this registration _WERE_ tied to a merchant_pub, then it may make sense to add some check here. But, that is only if regulatory requirements are articulated in such a way, and that remains very much unclear. So for the currently specified and implemented protocol, such a check makes no sense.

Christian Grothoff

2020-07-14 21:15

manager   ~0016448

A check that the refund deadline should not be after the wire deadline was added to the taler-helper-auditor-coins in Git 2570b21d..62d5aae1.

Issue History

Date Modified Username Field Change
2020-06-24 13:39 oec New Issue
2020-06-24 13:39 oec Status new => assigned
2020-06-24 13:39 oec Assigned To => Christian Grothoff
2020-06-24 13:45 Christian Grothoff Note Added: 0016351
2020-06-24 13:46 Christian Grothoff Status assigned => feedback
2020-06-24 13:57 Christian Grothoff Severity major => text
2020-06-24 14:00 Christian Grothoff Severity text => tweak
2020-06-25 10:55 oec Note Added: 0016362
2020-06-25 10:55 oec Status feedback => assigned
2020-06-25 11:05 Christian Grothoff Note Added: 0016365
2020-06-25 23:48 Christian Grothoff Product Version => git (master)
2020-06-25 23:48 Christian Grothoff Target Version => 0.8
2020-07-14 21:15 Christian Grothoff Note Added: 0016448
2020-07-14 21:16 Christian Grothoff Status assigned => resolved
2020-07-14 21:16 Christian Grothoff Resolution open => fixed
2020-07-14 21:16 Christian Grothoff Fixed in Version => 0.8
2021-08-24 16:23 Christian Grothoff Status resolved => closed