extend tests to discover corner cases from #6478, fix code to actually work for those cases

4 files changed, 77 insertions, 37 deletions

diff --git a/src/exchange/taler-exchange-httpd_refund.c b/src/exchange/taler-exchange-httpd_refund.c
index b60a93b5d..955f1c51d 100644
--- a/src/exchange/taler-exchange-httpd_refund.c
+++ b/src/exchange/taler-exchange-httpd_refund.c
@@ -337,7 +337,6 @@ refund_transaction (void *cls,
   if (1 == TALER_amount_cmp (&refund_total,
                              &deposit_total) )
   {
-    GNUNET_break_op (0); /* cannot refund more than original value */
     *mhd_ret = TALER_MHD_reply_json_pack (
       connection,
       MHD_HTTP_CONFLICT,
diff --git a/src/lib/exchange_api_common.c b/src/lib/exchange_api_common.c
index b07ac1112..3591a7bbc 100644
--- a/src/lib/exchange_api_common.c
+++ b/src/lib/exchange_api_common.c
@@ -643,6 +643,7 @@ TALER_EXCHANGE_verify_coin_history (
     {
       struct TALER_MerchantSignatureP sig;
       struct TALER_Amount refund_fee;
+      struct TALER_Amount sig_amount;
       struct TALER_RefundRequestPS rr = {
         .purpose.size = htonl (sizeof (rr)),
         .purpose.purpose = htonl (TALER_SIGNATURE_MERCHANT_REFUND),
@@ -658,7 +659,7 @@ TALER_EXCHANGE_verify_coin_history (
         GNUNET_JSON_spec_fixed_auto ("merchant_pub",
                                      &rr.merchant),
         GNUNET_JSON_spec_uint64 ("rtransaction_id",
-                                 &rr.rtransaction_id), // FIXME: shouldn't this be NBO!?
+                                 &rr.rtransaction_id),
         GNUNET_JSON_spec_end ()
       };
 
@@ -670,9 +671,19 @@ TALER_EXCHANGE_verify_coin_history (
         GNUNET_break_op (0);
         return GNUNET_SYSERR;
       }
-      abort (); // FIXME: this shows the case is not tested! ...
+      if (0 >
+          TALER_amount_add (&sig_amount,
+                            &refund_fee,
+                            &amount))
+      {
+        GNUNET_break_op (0);
+        return GNUNET_SYSERR;
+      }
       TALER_amount_hton (&rr.refund_amount,
-                         &amount);
+                         &sig_amount);
+      rr.rtransaction_id = GNUNET_htonll (rr.rtransaction_id);
+      TALER_amount_hton (&rr.refund_amount,
+                         &sig_amount);
       if (GNUNET_OK !=
           GNUNET_CRYPTO_eddsa_verify (TALER_SIGNATURE_MERCHANT_REFUND,
                                       &rr,
diff --git a/src/lib/exchange_api_refund.c b/src/lib/exchange_api_refund.c
index 7c758ba2a..ee634e7f6 100644
--- a/src/lib/exchange_api_refund.c
+++ b/src/lib/exchange_api_refund.c
@@ -162,10 +162,6 @@ verify_conflict_history_ok (struct TALER_EXCHANGE_RefundHandle *rh,
     GNUNET_break_op (0);
     return GNUNET_SYSERR;
   }
-  // FIXME: check that 'history' contains a coin history that
-  // demonstrates that another refund would exceed the deposit amount!
-
-
   len = json_array_size (history);
   if (0 == len)
   {
@@ -197,13 +193,6 @@ verify_conflict_history_ok (struct TALER_EXCHANGE_RefundHandle *rh,
       GNUNET_break_op (0);
       return GNUNET_SYSERR;
     }
-    if (GNUNET_YES !=
-        TALER_amount_cmp_currency (&amount,
-                                   &rtotal))
-    {
-      GNUNET_break_op (0);
-      return GNUNET_SYSERR;
-    }
     if (0 == strcasecmp (type,
                          "DEPOSIT"))
     {
@@ -264,6 +253,13 @@ verify_conflict_history_ok (struct TALER_EXCHANGE_RefundHandle *rh,
       if (have_deposit)
       {
         /* this cannot really happen, but we conservatively support it anyway */
+        if (GNUNET_YES !=
+            TALER_amount_cmp_currency (&amount,
+                                       &dtotal))
+        {
+          GNUNET_break_op (0);
+          return GNUNET_SYSERR;
+        }
         GNUNET_break (0 <=
                       TALER_amount_add (&dtotal,
                                         &dtotal,
@@ -280,6 +276,7 @@ verify_conflict_history_ok (struct TALER_EXCHANGE_RefundHandle *rh,
     {
       struct TALER_MerchantSignatureP sig;
       struct TALER_Amount refund_fee;
+      struct TALER_Amount sig_amount;
       struct TALER_RefundRequestPS rr = {
         .purpose.size = htonl (sizeof (rr)),
         .purpose.purpose = htonl (TALER_SIGNATURE_MERCHANT_REFUND),
@@ -307,8 +304,17 @@ verify_conflict_history_ok (struct TALER_EXCHANGE_RefundHandle *rh,
         GNUNET_break_op (0);
         return GNUNET_SYSERR;
       }
+      if (0 >
+          TALER_amount_add (&sig_amount,
+                            &refund_fee,
+                            &amount))
+      {
+        GNUNET_break_op (0);
+        return GNUNET_SYSERR;
+      }
       TALER_amount_hton (&rr.refund_amount,
-                         &amount);
+                         &sig_amount);
+      rr.rtransaction_id = GNUNET_htonll (rr.rtransaction_id);
       if (GNUNET_OK !=
           GNUNET_CRYPTO_eddsa_verify (TALER_SIGNATURE_MERCHANT_REFUND,
                                       &rr,
@@ -318,7 +324,6 @@ verify_conflict_history_ok (struct TALER_EXCHANGE_RefundHandle *rh,
         GNUNET_break_op (0);
         return GNUNET_SYSERR;
       }
-
       if ( (0 != GNUNET_memcmp (&rh->depconf.h_contract_terms,
                                 &rr.h_contract_terms)) ||
            (0 != GNUNET_memcmp (&rh->depconf.merchant,
@@ -338,6 +343,13 @@ verify_conflict_history_ok (struct TALER_EXCHANGE_RefundHandle *rh,
 
       if (have_refund)
       {
+        if (GNUNET_YES !=
+            TALER_amount_cmp_currency (&amount,
+                                       &rtotal))
+        {
+          GNUNET_break_op (0);
+          return GNUNET_SYSERR;
+        }
         GNUNET_break (0 <=
                       TALER_amount_add (&rtotal,
                                         &rtotal,
@@ -352,10 +364,10 @@ verify_conflict_history_ok (struct TALER_EXCHANGE_RefundHandle *rh,
     else
     {
       /* unexpected type, new version on server? */
-      GNUNET_break_op (0);
       GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
                   "Unexpected type `%s' in response\n",
                   type);
+      GNUNET_break_op (0);
       return GNUNET_SYSERR;
     }
   }
@@ -561,6 +573,7 @@ handle_refund_finished (void *cls,
                                     j))
     {
       GNUNET_break (0);
+      hr.http_status = 0;
       hr.ec = TALER_EC_REFUND_INVALID_FAILURE_PROOF_BY_EXCHANGE;
       hr.hint = "conflict information provided by exchange is invalid";
       break;
@@ -580,6 +593,7 @@ handle_refund_finished (void *cls,
                                      j))
     {
       GNUNET_break (0);
+      hr.http_status = 0;
       hr.ec = TALER_EC_REFUND_INVALID_FAILURE_PROOF_BY_EXCHANGE;
       hr.hint = "failed precondition proof returned by exchange is invalid";
       break;
@@ -653,29 +667,30 @@ TALER_EXCHANGE_refund (struct TALER_EXCHANGE_Handle *exchange,
                        TALER_EXCHANGE_RefundCallback cb,
                        void *cb_cls)
 {
-  struct TALER_RefundRequestPS rr;
+  struct TALER_RefundRequestPS rr = {
+    .purpose.purpose = htonl (TALER_SIGNATURE_MERCHANT_REFUND),
+    .purpose.size = htonl (sizeof (rr)),
+    .h_contract_terms = *h_contract_terms,
+    .rtransaction_id = GNUNET_htonll (rtransaction_id),
+    .coin_pub = *coin_pub
+  };
   struct TALER_MerchantSignatureP merchant_sig;
+  struct TALER_EXCHANGE_RefundHandle *rh;
+  struct GNUNET_CURL_Context *ctx;
+  json_t *refund_obj;
+  CURL *eh;
+  char arg_str[sizeof (struct TALER_CoinSpendPublicKeyP) * 2 + 32];
 
   GNUNET_assert (GNUNET_YES ==
                  TEAH_handle_is_ready (exchange));
-  rr.purpose.purpose = htonl (TALER_SIGNATURE_MERCHANT_REFUND);
-  rr.purpose.size = htonl (sizeof (struct TALER_RefundRequestPS));
-  rr.h_contract_terms = *h_contract_terms;
-  rr.coin_pub = *coin_pub;
   GNUNET_CRYPTO_eddsa_key_get_public (&merchant_priv->eddsa_priv,
                                       &rr.merchant.eddsa_pub);
-  rr.rtransaction_id = GNUNET_htonll (rtransaction_id);
   TALER_amount_hton (&rr.refund_amount,
                      amount);
   GNUNET_CRYPTO_eddsa_sign (&merchant_priv->eddsa_priv,
                             &rr,
                             &merchant_sig.eddsa_sig);
 
-  struct TALER_EXCHANGE_RefundHandle *rh;
-  struct GNUNET_CURL_Context *ctx;
-  json_t *refund_obj;
-  CURL *eh;
-  char arg_str[sizeof (struct TALER_CoinSpendPublicKeyP) * 2 + 32];
 
   {
     char pub_str[sizeof (struct TALER_CoinSpendPublicKeyP) * 2];
diff --git a/src/testing/test_exchange_api.c b/src/testing/test_exchange_api.c
index e410b1805..d30597b20 100644
--- a/src/testing/test_exchange_api.c
+++ b/src/testing/test_exchange_api.c
@@ -552,20 +552,35 @@ run (void *cls,
      * fakebank and the second to actually check there are not
      * other transfers around. *///
     TALER_TESTING_cmd_check_bank_empty ("check_bank_transfer-pre-refund"),
-    TALER_TESTING_cmd_refund ("refund-ok",
-                              MHD_HTTP_OK,
-                              "EUR:5",
-                              "deposit-refund-1"),
-    TALER_TESTING_cmd_refund ("refund-ok-double",
-                              MHD_HTTP_OK,
-                              "EUR:5",
-                              "deposit-refund-1"),
+    TALER_TESTING_cmd_refund_with_id ("refund-ok",
+                                      MHD_HTTP_OK,
+                                      "EUR:3",
+                                      "deposit-refund-1",
+                                      3),
+    TALER_TESTING_cmd_refund_with_id ("refund-ok-double",
+                                      MHD_HTTP_OK,
+                                      "EUR:3",
+                                      "deposit-refund-1",
+                                      3),
     /* Previous /refund(s) had id == 0.  */
     TALER_TESTING_cmd_refund_with_id ("refund-conflicting",
                                       MHD_HTTP_CONFLICT,
                                       "EUR:5",
                                       "deposit-refund-1",
                                       1),
+    TALER_TESTING_cmd_deposit ("deposit-refund-insufficient-refund",
+                               "withdraw-coin-r1",
+                               0,
+                               bc.user42_payto,
+                               "{\"items\":[{\"name\":\"ice cream\",\"value\":\"EUR:4\"}]}",
+                               GNUNET_TIME_UNIT_MINUTES,
+                               "EUR:4",
+                               MHD_HTTP_CONFLICT),
+    TALER_TESTING_cmd_refund_with_id ("refund-ok-increase",
+                                      MHD_HTTP_OK,
+                                      "EUR:2",
+                                      "deposit-refund-1",
+                                      2),
     /**
      * Spend 4.99 EUR of the refunded 4.99 EUR coin (1ct gone
      * due to refund) (merchant would receive EUR:4.98 due to