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, 2 insertions, 17 deletions

diff --git a/src/exchange/taler-exchange-httpd_recoup.c b/src/exchange/taler-exchange-httpd_recoup.c
index 28e81f9e..0deaa8bb 100644
--- a/src/exchange/taler-exchange-httpd_recoup.c
+++ b/src/exchange/taler-exchange-httpd_recoup.c
@@ -66,11 +66,6 @@ struct RecoupContext
   const struct TALER_CoinSpendSignatureP *coin_sig;
 
   /**
-   * The amount requested to be recouped.
-   */
-  const struct TALER_Amount *requested_amount;
-
-  /**
    * Unique ID of the withdraw operation in the reserves_out table.
    */
   uint64_t reserve_out_serial_id;
@@ -121,7 +116,6 @@ recoup_transaction (void *cls,
   qs = TEH_plugin->do_recoup (TEH_plugin->cls,
                               &pc->reserve_pub,
                               pc->reserve_out_serial_id,
-                              pc->requested_amount,
                               pc->coin_bks,
                               &pc->coin->coin_pub,
                               pc->known_coin_id,
@@ -173,7 +167,6 @@ recoup_transaction (void *cls,
  * @param coin information about the coin
  * @param coin_bks blinding data of the coin (to be checked)
  * @param coin_sig signature of the coin
- * @param requested_amount requested amount to be recouped
  * @return MHD result code
  */
 static MHD_RESULT
@@ -181,8 +174,7 @@ verify_and_execute_recoup (
   struct MHD_Connection *connection,
   const struct TALER_CoinPublicInfo *coin,
   const union TALER_DenominationBlindingKeyP *coin_bks,
-  const struct TALER_CoinSpendSignatureP *coin_sig,
-  const struct TALER_Amount *requested_amount)
+  const struct TALER_CoinSpendSignatureP *coin_sig)
 {
   struct RecoupContext pc;
   const struct TEH_DenominationKey *dk;
@@ -239,7 +231,6 @@ verify_and_execute_recoup (
   if (GNUNET_OK !=
       TALER_wallet_recoup_verify (&coin->denom_pub_hash,
                                   coin_bks,
-                                  requested_amount,
                                   &coin->coin_pub,
                                   coin_sig))
   {
@@ -281,7 +272,6 @@ verify_and_execute_recoup (
   pc.coin_sig = coin_sig;
   pc.coin_bks = coin_bks;
   pc.coin = coin;
-  pc.requested_amount = requested_amount;
 
   {
     MHD_RESULT mhd_ret = MHD_NO;
@@ -369,7 +359,6 @@ TEH_handler_recoup (struct MHD_Connection *connection,
   struct TALER_CoinPublicInfo coin;
   union TALER_DenominationBlindingKeyP coin_bks;
   struct TALER_CoinSpendSignatureP coin_sig;
-  struct TALER_Amount amount;
   struct GNUNET_JSON_Specification spec[] = {
     GNUNET_JSON_spec_fixed_auto ("denom_pub_hash",
                                  &coin.denom_pub_hash),
@@ -379,9 +368,6 @@ TEH_handler_recoup (struct MHD_Connection *connection,
                                  &coin_bks),
     GNUNET_JSON_spec_fixed_auto ("coin_sig",
                                  &coin_sig),
-    TALER_JSON_spec_amount ("amount",
-                            TEH_currency,
-                            &amount),
     GNUNET_JSON_spec_end ()
   };
 
@@ -402,8 +388,7 @@ TEH_handler_recoup (struct MHD_Connection *connection,
     res = verify_and_execute_recoup (connection,
                                      &coin,
                                      &coin_bks,
-                                     &coin_sig,
-                                     &amount);
+                                     &coin_sig);
     GNUNET_JSON_parse_free (spec);
     return res;
   }