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_exchangedb_plugin.h b/src/include/taler_exchangedb_plugin.h
index 4aa80b674..5eb168e14 100644
--- a/src/include/taler_exchangedb_plugin.h
+++ b/src/include/taler_exchangedb_plugin.h
@@ -2596,7 +2596,6 @@ struct TALER_EXCHANGEDB_Plugin
    * @param cls the `struct PostgresClosure` with the plugin-specific state
    * @param reserve_pub public key of the reserve to credit
    * @param reserve_out_serial_id row in the reserves_out table justifying the recoup
-   * @param requested_amount the amount to be recouped
    * @param coin_bks coin blinding key secret to persist
    * @param coin_pub public key of the coin being recouped
    * @param known_coin_id row of the @a coin_pub in the known_coins table
@@ -2611,7 +2610,6 @@ struct TALER_EXCHANGEDB_Plugin
     void *cls,
     const struct TALER_ReservePublicKeyP *reserve_pub,
     uint64_t reserve_out_serial_id,
-    const struct TALER_Amount *requested_amount,
     const union TALER_DenominationBlindingKeyP *coin_bks,
     const struct TALER_CoinSpendPublicKeyP *coin_pub,
     uint64_t known_coin_id,
@@ -2628,7 +2626,6 @@ struct TALER_EXCHANGEDB_Plugin
    * @param cls the `struct PostgresClosure` with the plugin-specific state
    * @param old_coin_pub public key of the old coin to credit
    * @param rrc_serial row in the refresh_revealed_coins table justifying the recoup-refresh
-   * @param requested_amount the amount to be recouped
    * @param coin_bks coin blinding key secret to persist
    * @param coin_pub public key of the coin being recouped
    * @param known_coin_id row of the @a coin_pub in the known_coins table
@@ -2643,7 +2640,6 @@ struct TALER_EXCHANGEDB_Plugin
     void *cls,
     const struct TALER_CoinSpendPublicKeyP *old_coin_pub,
     uint64_t rrc_serial,
-    const struct TALER_Amount *requested_amount,
     const union TALER_DenominationBlindingKeyP *coin_bks,
     const struct TALER_CoinSpendPublicKeyP *coin_pub,
     uint64_t known_coin_id,