View Issue Details

IDProjectCategoryView StatusLast Update
0006413Talerauditorpublic2020-06-25 23:48
ReporteroecAssigned ToChristian Grothoff 
PrioritynormalSeveritytweakReproducibilityalways
Status assignedResolutionopen 
Product Versiongit (master) 
Target Version0.8Fixed in Version 
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.

Activities

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.

oec

2020-06-25 10:55

reporter   ~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.

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