The current recoup API is broken. I guess this is another example where "trivial" API changes turn out to have (multiple!) unexpected consequences.

The current "/recoup" API does not have clear idempotency semantics, as we've discussed on the phone.  This is already bad by itself, as it makes it hard to write down what the API does other than "whatever the implementation does".

However, it actually breaks correctness in this (admittedly kinda contrived, but not impossible) case:

Say that we have a coin A obtained via withdrawal and a coin B obtained via refreshing coin A. Now the denominations of A gets revoked..

The wallet does a recoup of A for EUR:1.

Now the denomination of B also gets revoked.  The wallet recoups B (incidentally also for EUR:1) and now A can be recouped again for EUR:1.  But now the exchange is in a state where it will refuse a legitimate recoup request for A because the detection for an idempotent request kicks in.

This is IMHO bad API design, and the exchange should simply always recoup the maximum amount.

Furthermore, we usually follow the principle of "API calls that take up DB space are paid".  With the current recoup API, I can do many tiny recoup requests which the exchange then has to store, right?

I guess it would not be a big change to remove the "amount" value from the recoup/recoup-refresh request bodies, right?

- Florian

1 files changed, 0 insertions, 4 deletions

diff --git a/src/include/taler_exchange_service.h b/src/include/taler_exchange_service.h
index 6976293cb..7fb5b4ec0 100644
--- a/src/include/taler_exchange_service.h
+++ b/src/include/taler_exchange_service.h
@@ -2035,7 +2035,6 @@ typedef void
  * @param pk kind of coin to pay back
  * @param denom_sig signature over the coin by the exchange using @a pk
  * @param ps secret internals of the original planchet
- * @param amount value remaining on the coin that is being recouped
  * @param recoup_cb the callback to call when the final result for this request is available
  * @param recoup_cb_cls closure for @a recoup_cb
  * @return NULL
@@ -2047,7 +2046,6 @@ TALER_EXCHANGE_recoup (struct TALER_EXCHANGE_Handle *exchange,
                        const struct TALER_EXCHANGE_DenomPublicKey *pk,
                        const struct TALER_DenominationSignature *denom_sig,
                        const struct TALER_PlanchetSecretsP *ps,
-                       const struct TALER_Amount *amount,
                        TALER_EXCHANGE_RecoupResultCallback recoup_cb,
                        void *recoup_cb_cls);
 
@@ -2097,7 +2095,6 @@ typedef void
  * @param pk kind of coin to pay back
  * @param denom_sig signature over the coin by the exchange using @a pk
  * @param ps secret internals of the original planchet
- * @param amount value remaining on the coin that is being recouped
  * @param recoup_cb the callback to call when the final result for this request is available
  * @param recoup_cb_cls closure for @a recoup_cb
  * @return NULL
@@ -2110,7 +2107,6 @@ TALER_EXCHANGE_recoup_refresh (
   const struct TALER_EXCHANGE_DenomPublicKey *pk,
   const struct TALER_DenominationSignature *denom_sig,
   const struct TALER_PlanchetSecretsP *ps,
-  const struct TALER_Amount *amount,
   TALER_EXCHANGE_RecoupRefreshResultCallback recoup_cb,
   void *recoup_cb_cls);