-remove redundancies in the refresh-melt computation and fix uninitialized bks return value

4 files changed, 184 insertions, 149 deletions

diff --git a/src/lib/exchange_api_melt.c b/src/lib/exchange_api_melt.c
index 07034f144..18596d891 100644
--- a/src/lib/exchange_api_melt.c
+++ b/src/lib/exchange_api_melt.c
@@ -459,8 +459,6 @@ start_melt (struct TALER_EXCHANGE_MeltHandle *mh)
 
   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,
@@ -551,11 +549,15 @@ start_melt (struct TALER_EXCHANGE_MeltHandle *mh)
  * @param[in] mh melt request that failed
  */
 static void
-fail_mh (struct TALER_EXCHANGE_MeltHandle *mh)
+fail_mh (struct TALER_EXCHANGE_MeltHandle *mh,
+         enum TALER_ErrorCode ec)
 {
-  // FIXME: do return more than NULLs if the /csr failed!
+  struct TALER_EXCHANGE_MeltResponse mr = {
+    .hr.ec = ec
+  };
+
   mh->melt_cb (mh->melt_cb_cls,
-               NULL);
+               &mr);
   TALER_EXCHANGE_melt_cancel (mh);
 }
 
@@ -575,6 +577,18 @@ csr_cb (void *cls,
   unsigned int nks_off = 0;
 
   mh->csr = NULL;
+  if (MHD_HTTP_OK != csrr->hr.http_status)
+  {
+    struct TALER_EXCHANGE_MeltResponse mr = {
+      .hr = csrr->hr
+    };
+
+    mr.hr.hint = "/csr failed";
+    mh->melt_cb (mh->melt_cb_cls,
+                 &mr);
+    TALER_EXCHANGE_melt_cancel (mh);
+    return;
+  }
   for (unsigned int i = 0; i<mh->rd->fresh_pks_len; i++)
   {
     const struct TALER_EXCHANGE_DenomPublicKey *fresh_pk =
@@ -585,7 +599,8 @@ csr_cb (void *cls,
     {
     case TALER_DENOMINATION_INVALID:
       GNUNET_break (0);
-      fail_mh (mh);
+      fail_mh (mh,
+               TALER_EC_GENERIC_CLIENT_INTERNAL_ERROR);
       return;
     case TALER_DENOMINATION_RSA:
       GNUNET_assert (TALER_DENOMINATION_RSA == wv->cipher);
@@ -601,7 +616,8 @@ csr_cb (void *cls,
       start_melt (mh))
   {
     GNUNET_break (0);
-    fail_mh (mh);
+    fail_mh (mh,
+             TALER_EC_GENERIC_CLIENT_INTERNAL_ERROR);
     return;
   }
 }
diff --git a/src/lib/exchange_api_refresh_common.c b/src/lib/exchange_api_refresh_common.c
index 02af2a993..e944b79a1 100644
--- a/src/lib/exchange_api_refresh_common.c
+++ b/src/lib/exchange_api_refresh_common.c
@@ -26,16 +26,28 @@
 void
 TALER_EXCHANGE_free_melt_data_ (struct MeltData *md)
 {
+  for (unsigned int i = 0; i < TALER_CNC_KAPPA; i++)
+  {
+    struct TALER_RefreshCoinData *rcds = md->rcd[i];
+
+    if (NULL == rcds)
+      continue;
+    for (unsigned int j = 0; j < md->num_fresh_coins; j++)
+      TALER_blinded_planchet_free (&rcds[j].blinded_planchet);
+    GNUNET_free (rcds);
+  }
   TALER_denom_pub_free (&md->melted_coin.pub_key);
   TALER_denom_sig_free (&md->melted_coin.sig);
-  if (NULL != md->fresh_pks)
+  if (NULL != md->fcds)
   {
-    for (unsigned int i = 0; i<md->num_fresh_coins; i++)
-      TALER_denom_pub_free (&md->fresh_pks[i]);
-    GNUNET_free (md->fresh_pks);
+    for (unsigned int j = 0; j<md->num_fresh_coins; j++)
+    {
+      struct FreshCoinData *fcd = &md->fcds[j];
+
+      TALER_denom_pub_free (&fcd->fresh_pk);
+    }
+    GNUNET_free (md->fcds);
   }
-  for (unsigned int i = 0; i<TALER_CNC_KAPPA; i++)
-    GNUNET_free (md->fresh_coins[i]);
   /* Finally, clean up a bit... */
   GNUNET_CRYPTO_zero_keys (md,
                            sizeof (struct MeltData));
@@ -51,8 +63,7 @@ TALER_EXCHANGE_get_melt_data_ (
 {
   struct TALER_Amount total;
   struct TALER_CoinSpendPublicKeyP coin_pub;
-  struct TALER_TransferSecretP trans_sec[TALER_CNC_KAPPA];
-  struct TALER_RefreshCommitmentEntry rce[TALER_CNC_KAPPA];
+  struct TALER_CsNonce nonces[rd->fresh_pks_len];
 
   GNUNET_CRYPTO_eddsa_key_get_public (&rd->melt_priv.eddsa_priv,
                                       &coin_pub.eddsa_pub);
@@ -73,29 +84,42 @@ TALER_EXCHANGE_get_melt_data_ (
                              &rd->melt_pk.key);
   TALER_denom_sig_deep_copy (&md->melted_coin.sig,
                              &rd->melt_sig);
-  md->fresh_pks = GNUNET_new_array (rd->fresh_pks_len,
-                                    struct TALER_DenominationPublicKey);
-  for (unsigned int i = 0; i<rd->fresh_pks_len; i++)
+  md->fcds = GNUNET_new_array (md->num_fresh_coins,
+                               struct FreshCoinData);
+  for (unsigned int j = 0; j<rd->fresh_pks_len; j++)
   {
-    TALER_denom_pub_deep_copy (&md->fresh_pks[i],
-                               &rd->fresh_pks[i].key);
+    struct FreshCoinData *fcd = &md->fcds[j];
+
+    if (alg_values[j].cipher != rd->fresh_pks[j].key.cipher)
+    {
+      GNUNET_break (0);
+      TALER_EXCHANGE_free_melt_data_ (md);
+      return GNUNET_SYSERR;
+    }
+    if (TALER_DENOMINATION_CS == alg_values[j].cipher)
+    {
+      TALER_cs_refresh_nonce_derive (
+        rms,
+        j,
+        &nonces[j]);
+    }
+    TALER_denom_pub_deep_copy (&fcd->fresh_pk,
+                               &rd->fresh_pks[j].key);
     if ( (0 >
           TALER_amount_add (&total,
                             &total,
-                            &rd->fresh_pks[i].value)) ||
+                            &rd->fresh_pks[j].value)) ||
          (0 >
           TALER_amount_add (&total,
                             &total,
-                            &rd->fresh_pks[i].fee_withdraw)) )
+                            &rd->fresh_pks[j].fee_withdraw)) )
     {
       GNUNET_break (0);
       TALER_EXCHANGE_free_melt_data_ (md);
-      memset (md,
-              0,
-              sizeof (*md));
       return GNUNET_SYSERR;
     }
   }
+
   /* verify that melt_amount is above total cost */
   if (1 ==
       TALER_amount_cmp (&total,
@@ -105,88 +129,79 @@ TALER_EXCHANGE_get_melt_data_ (
        @a melt_amount. This is not OK. */
     GNUNET_break (0);
     TALER_EXCHANGE_free_melt_data_ (md);
-    memset (md,
-            0,
-            sizeof (*md));
     return GNUNET_SYSERR;
   }
 
   /* build up coins */
   for (unsigned int i = 0; i<TALER_CNC_KAPPA; i++)
   {
+    struct TALER_TransferSecretP trans_sec;
+
     TALER_planchet_secret_to_transfer_priv (
       rms,
       i,
-      &md->melted_coin.transfer_priv[i]);
+      &md->transfer_priv[i]);
     GNUNET_CRYPTO_ecdhe_key_get_public (
-      &md->melted_coin.transfer_priv[i].ecdhe_priv,
-      &rce[i].transfer_pub.ecdhe_pub);
+      &md->transfer_priv[i].ecdhe_priv,
+      &md->transfer_pub[i].ecdhe_pub);
     TALER_link_derive_transfer_secret (&rd->melt_priv,
-                                       &md->melted_coin.transfer_priv[i],
-                                       &trans_sec[i]);
-    md->fresh_coins[i] = GNUNET_new_array (rd->fresh_pks_len,
-                                           struct TALER_PlanchetMasterSecretP);
-    rce[i].new_coins = GNUNET_new_array (rd->fresh_pks_len,
-                                         struct TALER_RefreshCoinData);
+                                       &md->transfer_priv[i],
+                                       &trans_sec);
+    md->rcd[i] = GNUNET_new_array (rd->fresh_pks_len,
+                                   struct TALER_RefreshCoinData);
     for (unsigned int j = 0; j<rd->fresh_pks_len; j++)
     {
-      struct TALER_PlanchetMasterSecretP *fc = &md->fresh_coins[i][j];
-      struct TALER_RefreshCoinData *rcd = &rce[i].new_coins[j];
+      struct FreshCoinData *fcd = &md->fcds[j];
+      struct TALER_CoinSpendPrivateKeyP *coin_priv = &fcd->coin_priv;
+      struct TALER_PlanchetMasterSecretP *ps = &fcd->ps[i];
+      struct TALER_RefreshCoinData *rcd = &md->rcd[i][j];
+      union TALER_DenominationBlindingKeyP *bks = &fcd->bks[i];
       struct TALER_PlanchetDetail pd;
       struct TALER_CoinPubHash c_hash;
-      struct TALER_CoinSpendPrivateKeyP coin_priv;
-      union TALER_DenominationBlindingKeyP bks;
 
-      TALER_transfer_secret_to_planchet_secret (&trans_sec[i],
+      TALER_transfer_secret_to_planchet_secret (&trans_sec,
                                                 j,
-                                                fc);
-      TALER_planchet_setup_coin_priv (fc,
+                                                ps);
+      TALER_planchet_setup_coin_priv (ps,
                                       &alg_values[j],
-                                      &coin_priv);
-      TALER_planchet_blinding_secret_create (fc,
+                                      coin_priv);
+      TALER_planchet_blinding_secret_create (ps,
                                              &alg_values[j],
-                                             &bks);
-      /* 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 */
+                                             bks);
       if (TALER_DENOMINATION_CS == alg_values[j].cipher)
-        TALER_cs_refresh_nonce_derive (
-          rms,
-          j,
-          &pd.blinded_planchet.details.cs_blinded_planchet.nonce);
+        pd.blinded_planchet.details.cs_blinded_planchet.nonce = nonces[j];
       if (GNUNET_OK !=
-          TALER_planchet_prepare (&md->fresh_pks[j],
+          TALER_planchet_prepare (&fcd->fresh_pk,
                                   &alg_values[j],
-                                  &bks,
-                                  &coin_priv,
+                                  bks,
+                                  coin_priv,
                                   &c_hash,
                                   &pd))
       {
         GNUNET_break_op (0);
         TALER_EXCHANGE_free_melt_data_ (md);
-        memset (md,
-                0,
-                sizeof (*md));
         return GNUNET_SYSERR;
       }
-      rcd->dk = &md->fresh_pks[j];
       rcd->blinded_planchet = pd.blinded_planchet;
+      rcd->dk = &fcd->fresh_pk;
     }
   }
 
-  /* Compute refresh commitment */
-  TALER_refresh_get_commitment (&md->rc,
-                                TALER_CNC_KAPPA,
-                                rd->fresh_pks_len,
-                                rce,
-                                &coin_pub,
-                                &rd->melt_amount);
-  for (unsigned int i = 0; i < TALER_CNC_KAPPA; i++)
+  /* Finally, compute refresh commitment */
   {
-    for (unsigned int j = 0; j < rd->fresh_pks_len; j++)
-      TALER_blinded_planchet_free (&rce[i].new_coins[j].blinded_planchet);
-    GNUNET_free (rce[i].new_coins);
+    struct TALER_RefreshCommitmentEntry rce[TALER_CNC_KAPPA];
+
+    for (unsigned int i = 0; i<TALER_CNC_KAPPA; i++)
+    {
+      rce[i].transfer_pub = md->transfer_pub[i];
+      rce[i].new_coins = md->rcd[i];
+    }
+    TALER_refresh_get_commitment (&md->rc,
+                                  TALER_CNC_KAPPA,
+                                  rd->fresh_pks_len,
+                                  rce,
+                                  &coin_pub,
+                                  &rd->melt_amount);
   }
   return GNUNET_OK;
 }
diff --git a/src/lib/exchange_api_refresh_common.h b/src/lib/exchange_api_refresh_common.h
index 2115b5a19..ab19ad7d1 100644
--- a/src/lib/exchange_api_refresh_common.h
+++ b/src/lib/exchange_api_refresh_common.h
@@ -53,11 +53,6 @@ struct MeltedCoin
   struct TALER_Amount original_value;
 
   /**
-   * Transfer private keys for each cut-and-choose dimension.
-   */
-  struct TALER_TransferPrivateKeyP transfer_priv[TALER_CNC_KAPPA];
-
-  /**
    * Timestamp indicating when coins of this denomination become invalid.
    */
   struct GNUNET_TIME_Timestamp expire_deposit;
@@ -76,6 +71,37 @@ struct MeltedCoin
 
 
 /**
+ * Data we keep for each fresh coin created in the
+ * melt process.
+ */
+struct FreshCoinData
+{
+  /**
+   * Denomination public key of the coin.
+   */
+  struct TALER_DenominationPublicKey fresh_pk;
+
+  /**
+   * Array of planchet secrets for the coins, depending
+   * on the cut-and-choose.
+   */
+  struct TALER_PlanchetMasterSecretP ps[TALER_CNC_KAPPA];
+
+  /**
+   * Private key of the coin.
+   */
+  struct TALER_CoinSpendPrivateKeyP coin_priv;
+
+  /**
+   * Blinding key secrets for the coins, depending on the
+   * cut-and-choose.
+   */
+  union TALER_DenominationBlindingKeyP bks[TALER_CNC_KAPPA];
+
+};
+
+
+/**
  * Melt data in non-serialized format for convenient processing.
  */
 struct MeltData
@@ -87,26 +113,47 @@ struct MeltData
   struct TALER_RefreshCommitmentP rc;
 
   /**
-   * Number of coins we are creating
+   * Information about the melted coin.
    */
-  uint16_t num_fresh_coins;
+  struct MeltedCoin melted_coin;
 
   /**
-   * Information about the melted coin.
+   * Array of length @e num_fresh_coins with information
+   * about each fresh coin.
    */
-  struct MeltedCoin melted_coin;
+  struct FreshCoinData *fcds;
+
+  /**
+   * Transfer secrets, one per cut and choose.
+   */
+  struct TALER_TransferSecretP trans_sec[TALER_CNC_KAPPA];
+
+  /**
+   * Transfer private keys for each cut-and-choose dimension.
+   */
+  struct TALER_TransferPrivateKeyP transfer_priv[TALER_CNC_KAPPA];
+
+  /**
+   * Transfer public key of this commitment.
+   */
+  struct TALER_TransferPublicKeyP transfer_pub[TALER_CNC_KAPPA];
 
   /**
-   * Array of @e num_fresh_coins denomination keys for the coins to be
-   * freshly exchangeed.
+   * Transfer secrets, one per cut and choose.
    */
-  struct TALER_DenominationPublicKey *fresh_pks;
+  struct TALER_RefreshCommitmentEntry rce[TALER_CNC_KAPPA];
 
   /**
-   * Arrays of @e num_fresh_coins with information about the fresh
-   * coins to be created, for each cut-and-choose dimension.
+   * Blinded planchets and denominations of the fresh coins, depending on the cut-and-choose.  Array of length
+   * @e num_fresh_coins.
    */
-  struct TALER_PlanchetMasterSecretP *fresh_coins[TALER_CNC_KAPPA];
+  struct TALER_RefreshCoinData *rcd[TALER_CNC_KAPPA];
+
+  /**
+   * Number of coins we are creating
+   */
+  uint16_t num_fresh_coins;
+
 };
 
 
diff --git a/src/lib/exchange_api_refreshes_reveal.c b/src/lib/exchange_api_refreshes_reveal.c
index b17720768..08357c14e 100644
--- a/src/lib/exchange_api_refreshes_reveal.c
+++ b/src/lib/exchange_api_refreshes_reveal.c
@@ -140,7 +140,8 @@ refresh_reveal_ok (struct TALER_EXCHANGE_RefreshesRevealHandle *rrh,
   {
     struct TALER_EXCHANGE_RevealedCoinInfo *rci =
       &rcis[i];
-    struct TALER_DenominationPublicKey *pk;
+    const struct FreshCoinData *fcd = &rrh->md.fcds[i];
+    const struct TALER_DenominationPublicKey *pk;
     json_t *jsonai;
     struct TALER_BlindedDenominationSignature blind_sig;
     struct TALER_CoinSpendPublicKeyP coin_pub;
@@ -153,8 +154,9 @@ refresh_reveal_ok (struct TALER_EXCHANGE_RefreshesRevealHandle *rrh,
     struct TALER_FreshCoin coin;
     union TALER_DenominationBlindingKeyP bks;
 
-    rci->ps = rrh->md.fresh_coins[rrh->noreveal_index][i];
-    pk = &rrh->md.fresh_pks[i];
+    rci->ps = fcd->ps[rrh->noreveal_index];
+    rci->bks = fcd->bks[rrh->noreveal_index];
+    pk = &fcd->fresh_pk;
     jsonai = json_array_get (jsona, i);
     GNUNET_assert (NULL != jsonai);
 
@@ -323,9 +325,7 @@ TALER_EXCHANGE_refreshes_reveal (
   CURL *eh;
   struct GNUNET_CURL_Context *ctx;
   struct MeltData md;
-  struct TALER_TransferPublicKeyP transfer_pub;
   char arg_str[sizeof (struct TALER_RefreshCommitmentP) * 2 + 32];
-  struct TALER_TransferSecretP ts;
 
   GNUNET_assert (num_coins == rd->fresh_pks_len);
   if (noreveal_index >= TALER_CNC_KAPPA)
@@ -353,80 +353,38 @@ TALER_EXCHANGE_refreshes_reveal (
     return NULL;
   }
 
-  /* now transfer_pub */
-  GNUNET_CRYPTO_ecdhe_key_get_public (
-    &md.melted_coin.transfer_priv[noreveal_index].ecdhe_priv,
-    &transfer_pub.ecdhe_pub);
-  TALER_link_recover_transfer_secret (&transfer_pub,
-                                      &rd->melt_priv,
-                                      &ts);
-
   /* now new_denoms */
   GNUNET_assert (NULL != (new_denoms_h = json_array ()));
   GNUNET_assert (NULL != (coin_evs = json_array ()));
   GNUNET_assert (NULL != (link_sigs = json_array ()));
   for (unsigned int i = 0; i<md.num_fresh_coins; i++)
   {
+    const struct TALER_RefreshCoinData *rcd = &md.rcd[noreveal_index][i];
     struct TALER_DenominationHash denom_hash;
-    struct TALER_PlanchetDetail pd;
-    struct TALER_CoinPubHash c_hash;
-    struct TALER_PlanchetMasterSecretP coin_ps;
-    union TALER_DenominationBlindingKeyP bks;
-    struct TALER_CoinSpendPrivateKeyP coin_priv;
 
-    TALER_denom_pub_hash (&md.fresh_pks[i],
+    TALER_denom_pub_hash (&md.fcds[i].fresh_pk,
                           &denom_hash);
     GNUNET_assert (0 ==
                    json_array_append_new (new_denoms_h,
                                           GNUNET_JSON_from_data_auto (
                                             &denom_hash)));
-    TALER_transfer_secret_to_planchet_secret (&ts,
-                                              i,
-                                              &coin_ps);
-    TALER_planchet_setup_coin_priv (&coin_ps,
-                                    &alg_values[i],
-                                    &coin_priv);
-    TALER_planchet_blinding_secret_create (&coin_ps,
-                                           &alg_values[i],
-                                           &bks);
-    if (TALER_DENOMINATION_CS == alg_values[i].cipher)
-      TALER_cs_refresh_nonce_derive (
-        rms,
-        i,
-        &pd.blinded_planchet.details.cs_blinded_planchet.nonce);
-    if (GNUNET_OK !=
-        TALER_planchet_prepare (&md.fresh_pks[i],
-                                &alg_values[i],
-                                &bks,
-                                &coin_priv,
-                                &c_hash,
-                                &pd))
-    {
-      /* This should have been noticed during the preparation stage. */
-      GNUNET_break (0);
-      json_decref (new_denoms_h);
-      json_decref (coin_evs);
-      TALER_EXCHANGE_free_melt_data_ (&md);
-      return NULL;
-    }
-    GNUNET_assert (
-      0 ==
-      json_array_append_new (
-        coin_evs,
-        GNUNET_JSON_PACK (
-          TALER_JSON_pack_blinded_planchet (
-            NULL,
-            &pd.blinded_planchet))));
+    GNUNET_assert (0 ==
+                   json_array_append_new (
+                     coin_evs,
+                     GNUNET_JSON_PACK (
+                       TALER_JSON_pack_blinded_planchet (
+                         NULL,
+                         &rcd->blinded_planchet))));
     {
       struct TALER_CoinSpendSignatureP link_sig;
       struct TALER_BlindedCoinHash bch;
 
-      TALER_coin_ev_hash (&pd.blinded_planchet,
+      TALER_coin_ev_hash (&rcd->blinded_planchet,
                           &denom_hash,
                           &bch);
       TALER_wallet_link_sign (
         &denom_hash,
-        &transfer_pub,
+        &md.transfer_pub[noreveal_index],
         &bch,
         &md.melted_coin.coin_priv,
         &link_sig);
@@ -435,7 +393,6 @@ TALER_EXCHANGE_refreshes_reveal (
                        link_sigs,
                        GNUNET_JSON_from_data_auto (&link_sig)));
     }
-    TALER_blinded_planchet_free (&pd.blinded_planchet);
   }
 
   /* build array of transfer private keys */
@@ -450,13 +407,13 @@ TALER_EXCHANGE_refreshes_reveal (
     GNUNET_assert (0 ==
                    json_array_append_new (transfer_privs,
                                           GNUNET_JSON_from_data_auto (
-                                            &md.melted_coin.transfer_priv[j])));
+                                            &md.transfer_priv[j])));
   }
 
   /* build main JSON request */
   reveal_obj = GNUNET_JSON_PACK (
     GNUNET_JSON_pack_data_auto ("transfer_pub",
-                                &transfer_pub),
+                                &md.transfer_pub[noreveal_index]),
     GNUNET_JSON_pack_array_steal ("transfer_privs",
                                   transfer_privs),
     GNUNET_JSON_pack_array_steal ("link_sigs",