-refactor melt API, add FIXME for discovered bug

5 files changed, 97 insertions, 158 deletions

diff --git a/src/include/taler_exchange_service.h b/src/include/taler_exchange_service.h
index 3b227fe3e..3961aaa10 100644
--- a/src/include/taler_exchange_service.h
+++ b/src/include/taler_exchange_service.h
@@ -1650,6 +1650,7 @@ struct TALER_EXCHANGE_MeltBlindingDetail
   /**
    * Blinding keys used to blind the fresh coins
    */
+  /* FIXME: uninitialized! */
   union TALER_DenominationBlindingKeyP bks;
 
 };
@@ -1678,12 +1679,6 @@ struct TALER_EXCHANGE_MeltResponse
     {
 
       /**
-       * Length of the @a mbds array with the exchange values
-       * and blinding keys we are using.
-       */
-      unsigned int num_mbds;
-
-      /**
        * Information returned per coin.
        */
       const struct TALER_EXCHANGE_MeltBlindingDetail *mbds;
@@ -1694,6 +1689,12 @@ struct TALER_EXCHANGE_MeltResponse
       struct TALER_ExchangePublicKeyP sign_key;
 
       /**
+       * Length of the @a mbds array with the exchange values
+       * and blinding keys we are using.
+       */
+      unsigned int num_mbds;
+
+      /**
        * Gamma value chosen by the exchange.
        */
       uint32_t noreveal_index;
@@ -1710,23 +1711,12 @@ struct TALER_EXCHANGE_MeltResponse
  * #TALER_EXCHANGE_refreshes_reveal().
  *
  * @param cls closure
- * @param hr HTTP response data
- * @param num_coins number of fresh coins to be created, length of the @a exchange_vals array, 0 if the operation failed
- * @param alg_values array @a num_coins of exchange values contributed to the refresh operation
- * @param bks array of @a num_coins blinding keys used to blind the fresh coins
- * @param noreveal_index choice by the exchange in the cut-and-choose protocol,
- *                    UINT32_MAX on error
- * @param sign_key exchange key used to sign the response, or NULL
+ * @param mr response details
  */
 typedef void
 (*TALER_EXCHANGE_MeltCallback) (
   void *cls,
-  const struct TALER_EXCHANGE_HttpResponse *hr,
-  unsigned int num_coins,
-  const struct TALER_ExchangeWithdrawValues *alg_values,
-  const union TALER_DenominationBlindingKeyP *bks,
-  uint32_t noreveal_index,
-  const struct TALER_ExchangePublicKeyP *sign_key);
+  const struct TALER_EXCHANGE_MeltResponse *mr);
 
 
 /**
diff --git a/src/lib/exchange_api_melt.c b/src/lib/exchange_api_melt.c
index 3c608328a..07034f144 100644
--- a/src/lib/exchange_api_melt.c
+++ b/src/lib/exchange_api_melt.c
@@ -86,16 +86,10 @@ struct TALER_EXCHANGE_MeltHandle
   const struct TALER_EXCHANGE_RefreshData *rd;
 
   /**
-   * Array of `num_fresh_coins` contributory values of
-   * the exchange to the melt operation.
+   * Array of `num_fresh_coins` per-coin values
+   * returned from melt operation.
    */
-  struct TALER_ExchangeWithdrawValues *alg_values;
-
-  /**
-   * Array of `num_fresh_coins` blinding secrets
-   * used for blinding the coins.
-   */
-  union TALER_DenominationBlindingKeyP *bks;
+  struct TALER_EXCHANGE_MeltBlindingDetail *mbds;
 
   /**
    * Handle for the preflight request, or NULL.
@@ -336,58 +330,45 @@ handle_melt_finished (void *cls,
                       const void *response)
 {
   struct TALER_EXCHANGE_MeltHandle *mh = cls;
-  struct TALER_ExchangePublicKeyP exchange_pub;
   const json_t *j = response;
-  struct TALER_EXCHANGE_HttpResponse hr = {
-    .reply = j,
-    .http_status = (unsigned int) response_code
+  struct TALER_EXCHANGE_MeltResponse mr = {
+    .hr.reply = j,
+    .hr.http_status = (unsigned int) response_code
   };
 
   mh->job = NULL;
   switch (response_code)
   {
   case 0:
-    hr.ec = TALER_EC_GENERIC_INVALID_RESPONSE;
+    mr.hr.ec = TALER_EC_GENERIC_INVALID_RESPONSE;
     break;
   case MHD_HTTP_OK:
     if (GNUNET_OK !=
         verify_melt_signature_ok (mh,
                                   j,
-                                  &exchange_pub))
+                                  &mr.details.success.sign_key))
     {
       GNUNET_break_op (0);
-      hr.http_status = 0;
-      hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
-    }
-    if (NULL != mh->melt_cb)
-    {
-      mh->melt_cb (mh->melt_cb_cls,
-                   &hr,
-                   (0 == hr.http_status)
-                   ? 0
-                   : mh->rd->fresh_pks_len,
-                   (0 == hr.http_status)
-                   ? NULL
-                   : mh->alg_values,
-                   (0 == hr.http_status)
-                   ? NULL
-                   : mh->bks,
-                   mh->noreveal_index,
-                   (0 == hr.http_status)
-                   ? NULL
-                   : &exchange_pub);
-      mh->melt_cb = NULL;
+      mr.hr.http_status = 0;
+      mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
+      break;
     }
+    mr.details.success.noreveal_index = mh->noreveal_index;
+    mr.details.success.num_mbds = mh->rd->fresh_pks_len;
+    mr.details.success.mbds = mh->mbds;
+    mh->melt_cb (mh->melt_cb_cls,
+                 &mr);
+    mh->melt_cb = NULL;
     break;
   case MHD_HTTP_BAD_REQUEST:
     /* This should never happen, either us or the exchange is buggy
        (or API version conflict); just pass JSON reply to the application */
-    hr.ec = TALER_JSON_get_error_code (j);
-    hr.hint = TALER_JSON_get_error_hint (j);
+    mr.hr.ec = TALER_JSON_get_error_code (j);
+    mr.hr.hint = TALER_JSON_get_error_hint (j);
     break;
   case MHD_HTTP_CONFLICT:
-    hr.ec = TALER_JSON_get_error_code (j);
-    switch (hr.ec)
+    mr.hr.ec = TALER_JSON_get_error_code (j);
+    switch (mr.hr.ec)
     {
     case TALER_EC_EXCHANGE_GENERIC_INSUFFICIENT_FUNDS:
       /* Double spending; check signatures on transaction history */
@@ -396,9 +377,9 @@ handle_melt_finished (void *cls,
                                                 j))
       {
         GNUNET_break_op (0);
-        hr.http_status = 0;
-        hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
-        hr.hint = TALER_JSON_get_error_hint (j);
+        mr.hr.http_status = 0;
+        mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
+        mr.hr.hint = TALER_JSON_get_error_hint (j);
       }
       break;
     case TALER_EC_EXCHANGE_GENERIC_COIN_CONFLICTING_DENOMINATION_KEY:
@@ -407,16 +388,16 @@ handle_melt_finished (void *cls,
                                                 j))
       {
         GNUNET_break_op (0);
-        hr.http_status = 0;
-        hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
-        hr.hint = TALER_JSON_get_error_hint (j);
+        mr.hr.http_status = 0;
+        mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
+        mr.hr.hint = TALER_JSON_get_error_hint (j);
       }
       break;
     default:
       GNUNET_break_op (0);
-      hr.http_status = 0;
-      hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
-      hr.hint = TALER_JSON_get_error_hint (j);
+      mr.hr.http_status = 0;
+      mr.hr.ec = TALER_EC_EXCHANGE_MELT_INVALID_SIGNATURE_BY_EXCHANGE;
+      mr.hr.hint = TALER_JSON_get_error_hint (j);
       break;
     }
     break;
@@ -424,40 +405,35 @@ handle_melt_finished (void *cls,
     /* Nothing really to verify, exchange says one of the signatures is
        invalid; assuming we checked them, this should never happen, we
        should pass the JSON reply to the application */
-    hr.ec = TALER_JSON_get_error_code (j);
-    hr.hint = TALER_JSON_get_error_hint (j);
+    mr.hr.ec = TALER_JSON_get_error_code (j);
+    mr.hr.hint = TALER_JSON_get_error_hint (j);
     break;
   case MHD_HTTP_NOT_FOUND:
     /* Nothing really to verify, this should never
        happen, we should pass the JSON reply to the application */
-    hr.ec = TALER_JSON_get_error_code (j);
-    hr.hint = TALER_JSON_get_error_hint (j);
+    mr.hr.ec = TALER_JSON_get_error_code (j);
+    mr.hr.hint = TALER_JSON_get_error_hint (j);
     break;
   case MHD_HTTP_INTERNAL_SERVER_ERROR:
     /* Server had an internal issue; we should retry, but this API
        leaves this to the application */
-    hr.ec = TALER_JSON_get_error_code (j);
-    hr.hint = TALER_JSON_get_error_hint (j);
+    mr.hr.ec = TALER_JSON_get_error_code (j);
+    mr.hr.hint = TALER_JSON_get_error_hint (j);
     break;
   default:
     /* unexpected response code */
-    hr.ec = TALER_JSON_get_error_code (j);
-    hr.hint = TALER_JSON_get_error_hint (j);
+    mr.hr.ec = TALER_JSON_get_error_code (j);
+    mr.hr.hint = TALER_JSON_get_error_hint (j);
     GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
                 "Unexpected response code %u/%d for exchange melt\n",
                 (unsigned int) response_code,
-                hr.ec);
+                mr.hr.ec);
     GNUNET_break_op (0);
     break;
   }
   if (NULL != mh->melt_cb)
     mh->melt_cb (mh->melt_cb_cls,
-                 &hr,
-                 0,
-                 NULL,
-                 NULL,
-                 UINT32_MAX,
-                 NULL);
+                 &mr);
   TALER_EXCHANGE_melt_cancel (mh);
 }
 
@@ -479,11 +455,16 @@ start_melt (struct TALER_EXCHANGE_MeltHandle *mh)
   struct TALER_CoinSpendSignatureP confirm_sig;
   char arg_str[sizeof (struct TALER_CoinSpendPublicKeyP) * 2 + 32];
   struct TALER_DenominationHash h_denom_pub;
+  struct TALER_ExchangeWithdrawValues alg_values[mh->rd->fresh_pks_len];
 
+  for (unsigned int i = 0; i<mh->rd->fresh_pks_len; i++)
+    alg_values[i] = mh->mbds[i].alg_value;
+  /* FIXME: get_melt_data computes the 'bks' which
+     we should return, but leave uninitialized => refactor logic! */
   if (GNUNET_OK !=
       TALER_EXCHANGE_get_melt_data_ (&mh->rms,
                                      mh->rd,
-                                     mh->alg_values,
+                                     alg_values,
                                      &mh->md))
   {
     GNUNET_break (0);
@@ -574,11 +555,6 @@ fail_mh (struct TALER_EXCHANGE_MeltHandle *mh)
 {
   // FIXME: do return more than NULLs if the /csr failed!
   mh->melt_cb (mh->melt_cb_cls,
-               NULL,
-               0,
-               NULL,
-               NULL,
-               UINT32_MAX,
                NULL);
   TALER_EXCHANGE_melt_cancel (mh);
 }
@@ -603,7 +579,7 @@ csr_cb (void *cls,
   {
     const struct TALER_EXCHANGE_DenomPublicKey *fresh_pk =
       &mh->rd->fresh_pks[i];
-    struct TALER_ExchangeWithdrawValues *wv = &mh->alg_values[i];
+    struct TALER_ExchangeWithdrawValues *wv = &mh->mbds[i].alg_value;
 
     switch (fresh_pk->key.cipher)
     {
@@ -656,21 +632,18 @@ TALER_EXCHANGE_melt (struct TALER_EXCHANGE_Handle *exchange,
   mh->rms = *rms;
   mh->melt_cb = melt_cb;
   mh->melt_cb_cls = melt_cb_cls;
-  mh->alg_values = GNUNET_new_array (rd->fresh_pks_len,
-                                     struct TALER_ExchangeWithdrawValues);
-  mh->bks = GNUNET_new_array (rd->fresh_pks_len,
-                              union TALER_DenominationBlindingKeyP);
+  mh->mbds = GNUNET_new_array (rd->fresh_pks_len,
+                               struct TALER_EXCHANGE_MeltBlindingDetail);
   for (unsigned int i = 0; i<rd->fresh_pks_len; i++)
   {
     const struct TALER_EXCHANGE_DenomPublicKey *fresh_pk = &rd->fresh_pks[i];
-    struct TALER_ExchangeWithdrawValues *wv = &mh->alg_values[i];
+    struct TALER_ExchangeWithdrawValues *wv = &mh->mbds[i].alg_value;
 
     switch (fresh_pk->key.cipher)
     {
     case TALER_DENOMINATION_INVALID:
       GNUNET_break (0);
-      GNUNET_free (mh->alg_values);
-      GNUNET_free (mh->bks);
+      GNUNET_free (mh->mbds);
       GNUNET_free (mh);
       return NULL;
     case TALER_DENOMINATION_RSA:
@@ -726,8 +699,7 @@ TALER_EXCHANGE_melt_cancel (struct TALER_EXCHANGE_MeltHandle *mh)
     mh->csr = NULL;
   }
   TALER_EXCHANGE_free_melt_data_ (&mh->md); /* does not free 'md' itself */
-  GNUNET_free (mh->alg_values);
-  GNUNET_free (mh->bks);
+  GNUNET_free (mh->mbds);
   GNUNET_free (mh->url);
   TALER_curl_easy_post_finished (&mh->ctx);
   GNUNET_free (mh);
diff --git a/src/lib/exchange_api_refresh_common.c b/src/lib/exchange_api_refresh_common.c
index 4c65e390f..02af2a993 100644
--- a/src/lib/exchange_api_refresh_common.c
+++ b/src/lib/exchange_api_refresh_common.c
@@ -146,14 +146,15 @@ TALER_EXCHANGE_get_melt_data_ (
       TALER_planchet_blinding_secret_create (fc,
                                              &alg_values[j],
                                              &bks);
-      /* Note: we already did this for the /csr request,
+      /* FIXME: we already did this for the /csr request,
          so this computation is redundant, and here additionally
          repeated KAPPA times. Could be avoided with slightly
          more bookkeeping in the future */
-      TALER_cs_refresh_nonce_derive (
-        rms,
-        j,
-        &pd.blinded_planchet.details.cs_blinded_planchet.nonce);
+      if (TALER_DENOMINATION_CS == alg_values[j].cipher)
+        TALER_cs_refresh_nonce_derive (
+          rms,
+          j,
+          &pd.blinded_planchet.details.cs_blinded_planchet.nonce);
       if (GNUNET_OK !=
           TALER_planchet_prepare (&md->fresh_pks[j],
                                   &alg_values[j],
diff --git a/src/lib/exchange_api_refresh_common.h b/src/lib/exchange_api_refresh_common.h
index 923fb76c4..2115b5a19 100644
--- a/src/lib/exchange_api_refresh_common.h
+++ b/src/lib/exchange_api_refresh_common.h
@@ -103,12 +103,6 @@ struct MeltData
   struct TALER_DenominationPublicKey *fresh_pks;
 
   /**
-   * Array of @e num_fresh_coins with exchange contributions
-   * made during the refresh.
-   */
-  struct TALER_ExchangeWithdrawValues *exchange_vals;
-
-  /**
    * Arrays of @e num_fresh_coins with information about the fresh
    * coins to be created, for each cut-and-choose dimension.
    */
diff --git a/src/testing/testing_api_cmd_refresh.c b/src/testing/testing_api_cmd_refresh.c
index 8b0329b38..94fade945 100644
--- a/src/testing/testing_api_cmd_refresh.c
+++ b/src/testing/testing_api_cmd_refresh.c
@@ -117,15 +117,10 @@ struct RefreshMeltState
   struct TALER_EXCHANGE_DenomPublicKey *fresh_pks;
 
   /**
-   * Array of @e num_fresh_coins of exchange values contributed to the refresh operation
+   * Array of @e num_fresh_coins of results from
+   * the melt operation.
    */
-  struct TALER_ExchangeWithdrawValues *alg_values;
-
-  /**
-   * Array of @e num_fresh_coins of blinding key secrets
-   * created during the melt operation.
-   */
-  union TALER_DenominationBlindingKeyP *bks;
+  struct TALER_EXCHANGE_MeltBlindingDetail *mbds;
 
   /**
    * Entropy seed for the refresh-melt operation.
@@ -499,15 +494,20 @@ refresh_reveal_run (void *cls,
   }
   // FIXME: use trait for 'rms'!
   rms = melt_cmd->cls;
-  rrs->rrh = TALER_EXCHANGE_refreshes_reveal (is->exchange,
-                                              &rms->rms,
-                                              &rms->refresh_data,
-                                              rms->num_fresh_coins,
-                                              rms->alg_values,
-                                              rms->noreveal_index,
-                                              &reveal_cb,
-                                              rrs);
-
+  {
+    struct TALER_ExchangeWithdrawValues alg_values[rms->num_fresh_coins];
+
+    for (unsigned int i = 0; i<rms->num_fresh_coins; i++)
+      alg_values[i] = rms->mbds[i].alg_value;
+    rrs->rrh = TALER_EXCHANGE_refreshes_reveal (is->exchange,
+                                                &rms->rms,
+                                                &rms->refresh_data,
+                                                rms->num_fresh_coins,
+                                                alg_values,
+                                                rms->noreveal_index,
+                                                &reveal_cb,
+                                                rrs);
+  }
   if (NULL == rrs->rrh)
   {
     GNUNET_break (0);
@@ -917,26 +917,15 @@ do_melt_retry (void *cls)
  * CMD was set to do so.
  *
  * @param cls closure.
- * @param hr HTTP response details
- * @param num_coins number of fresh coins to be created, length of the @a exchange_vals array, 0 if the operation failed
- * @param alg_values array @a num_coins of exchange values contributed to the refresh operation
- * @param bks array of @a num_coins blinding keys used to blind the fresh coins
- * @param noreveal_index choice by the exchange in the
- *        cut-and-choose protocol, UINT16_MAX on error.
- * @param exchange_pub public key the exchange used for signing.
+ * @param mr melt response details
  */
 static void
 melt_cb (void *cls,
-         const struct TALER_EXCHANGE_HttpResponse *hr,
-         unsigned int num_coins,
-         const struct TALER_ExchangeWithdrawValues *alg_values,
-         const union TALER_DenominationBlindingKeyP *bks,
-         uint32_t noreveal_index,
-         const struct TALER_ExchangePublicKeyP *exchange_pub)
+         const struct TALER_EXCHANGE_MeltResponse *mr)
 {
   struct RefreshMeltState *rms = cls;
+  const struct TALER_EXCHANGE_HttpResponse *hr = &mr->hr;
 
-  (void) exchange_pub;
   rms->rmh = NULL;
   if (rms->expected_response_code != hr->http_status)
   {
@@ -981,24 +970,18 @@ melt_cb (void *cls,
   }
   if (MHD_HTTP_OK == hr->http_status)
   {
-    rms->noreveal_index = noreveal_index;
-    if (num_coins != rms->num_fresh_coins)
+    rms->noreveal_index = mr->details.success.noreveal_index;
+    if (mr->details.success.num_mbds != rms->num_fresh_coins)
     {
       GNUNET_break (0);
       TALER_TESTING_interpreter_fail (rms->is);
       return;
     }
-    GNUNET_free (rms->alg_values);
-    rms->alg_values = GNUNET_new_array (num_coins,
-                                        struct TALER_ExchangeWithdrawValues);
-    memcpy (rms->alg_values,
-            alg_values,
-            num_coins * sizeof (struct TALER_ExchangeWithdrawValues));
-    rms->bks = GNUNET_new_array (num_coins,
-                                 union TALER_DenominationBlindingKeyP);
-    memcpy (rms->bks,
-            bks,
-            num_coins * sizeof (union TALER_DenominationBlindingKeyP));
+    GNUNET_free (rms->mbds);
+    rms->mbds = GNUNET_memdup (mr->details.success.mbds,
+                               mr->details.success.num_mbds
+                               * sizeof (struct
+                                         TALER_EXCHANGE_MeltBlindingDetail));
   }
   if (0 != rms->total_backoff.rel_value_us)
   {
@@ -1199,8 +1182,7 @@ melt_cleanup (void *cls,
       TALER_denom_pub_free (&rms->fresh_pks[i].key);
     GNUNET_free (rms->fresh_pks);
   }
-  GNUNET_free (rms->alg_values);
-  GNUNET_free (rms->bks);
+  GNUNET_free (rms->mbds);
   GNUNET_free (rms->melt_fresh_amounts);
   GNUNET_free (rms);
 }
@@ -1235,9 +1217,9 @@ melt_traits (void *cls,
       TALER_TESTING_make_trait_coin_priv (0,
                                           rms->melt_priv),
       TALER_TESTING_make_trait_blinding_key (index,
-                                             &rms->bks[index]),
+                                             &rms->mbds[index].bks),
       TALER_TESTING_make_trait_exchange_wd_value (index,
-                                                  &rms->alg_values[index]),
+                                                  &rms->mbds[index].alg_value),
       TALER_TESTING_make_trait_refresh_secret (&rms->rms),
       TALER_TESTING_trait_end ()
     };