fix auditor refund fee calculations

src/auditor/generate-auditor-basedb.sh

@@ -176,7 +176,7 @@ taler-exchange-offline -c $CONF \
   download sign \
   enable-account payto://x-taler-bank/localhost/Exchange \
   enable-auditor $AUDITOR_PUB $AUDITOR_URL "TESTKUDOS Auditor" \
-  wire-fee now x-taler-bank TESTKUDOS:0.01 TESTKUDOS:0.01 TESTKUDOS:0.01 \
+  wire-fee now x-taler-bank TESTKUDOS:0.07 TESTKUDOS:0.01 TESTKUDOS:0.01 \
   global-fee now TESTKUDOS:0.01 TESTKUDOS:0.01 TESTKUDOS:0.01 TESTKUDOS:0.01 1h 1h 1year 5 \
   upload &> taler-exchange-offline.log
 

src/auditor/taler-helper-auditor-aggregation.c

@@ -102,7 +102,7 @@ static struct TALER_Amount total_arithmetic_delta_plus;
 static struct TALER_Amount total_arithmetic_delta_minus;
 
 /**
- * Total aggregation fees earned.
+ * Total aggregation fees (wire fees) earned.
  */
 static struct TALER_Amount total_aggregation_fee_income;
 
@@ -398,9 +398,9 @@ check_transaction_history_for_deposit (
   struct TALER_Amount expenditures;
   struct TALER_Amount refunds;
   struct TALER_Amount spent;
+  struct TALER_Amount *deposited = NULL;
   struct TALER_Amount merchant_loss;
   const struct TALER_Amount *deposit_fee;
-  bool refund_deposit_fee;
 
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "Checking transaction history of coin %s\n",
@@ -422,24 +422,33 @@ check_transaction_history_for_deposit (
      compute positive (deposit, melt) and negative (refund) values separately
      here, and then subtract the negative from the positive at the end (after
      the loops). */
-  refund_deposit_fee = false;
   deposit_fee = NULL;
   for (const struct TALER_EXCHANGEDB_TransactionList *tl = tl_head;
        NULL != tl;
        tl = tl->next)
   {
-    const struct TALER_Amount *amount_with_fee;
     const struct TALER_Amount *fee_claimed;
 
     switch (tl->type)
     {
     case TALER_EXCHANGEDB_TT_DEPOSIT:
       /* check wire and h_wire are consistent */
-      amount_with_fee = &tl->details.deposit->amount_with_fee; /* according to exchange*/
+      if (NULL != deposited)
+      {
+        TALER_ARL_report (report_row_inconsistencies,
+                          GNUNET_JSON_PACK (
+                            GNUNET_JSON_pack_string ("table",
+                                                     "deposits"),
+                            GNUNET_JSON_pack_uint64 ("row",
+                                                     tl->serial_id),
+                            GNUNET_JSON_pack_string ("diagnostic",
+                                                     "multiple deposits of the same coin into the same contract detected")));
+      }
+      deposited = &tl->details.deposit->amount_with_fee; /* according to exchange*/
       fee_claimed = &tl->details.deposit->deposit_fee; /* Fee according to exchange DB */
       TALER_ARL_amount_add (&expenditures,
                             &expenditures,
-                            amount_with_fee);
+                            deposited);
       /* Check if this deposit is within the remit of the aggregation
          we are investigating, if so, include it in the totals. */
       if ( (0 == GNUNET_memcmp (merchant_pub,
@@ -450,7 +459,7 @@ check_transaction_history_for_deposit (
         struct TALER_Amount amount_without_fee;
 
         TALER_ARL_amount_subtract (&amount_without_fee,
-                                   amount_with_fee,
+                                   deposited,
                                    fee_claimed);
         TALER_ARL_amount_add (merchant_gain,
                               merchant_gain,
@@ -474,96 +483,116 @@ check_transaction_history_for_deposit (
       }
       break;
     case TALER_EXCHANGEDB_TT_MELT:
-      amount_with_fee = &tl->details.melt->amount_with_fee;
-      fee_claimed = &tl->details.melt->melt_fee;
-      TALER_ARL_amount_add (&expenditures,
-                            &expenditures,
-                            amount_with_fee);
-      /* Check that the fees given in the transaction list and in dki match */
-      if (0 !=
-          TALER_amount_cmp (&issue->fees.refresh,
-                            fee_claimed))
       {
-        /* Disagreement in fee structure between exchange and auditor */
+        const struct TALER_Amount *amount_with_fee;
-        report_amount_arithmetic_inconsistency ("melt fee",
+
-                                                0,
+        amount_with_fee = &tl->details.melt->amount_with_fee;
-                                                fee_claimed,
+        fee_claimed = &tl->details.melt->melt_fee;
-                                                &issue->fees.refresh,
-                                                1);
-      }
-      break;
-    case TALER_EXCHANGEDB_TT_REFUND:
-      amount_with_fee = &tl->details.refund->refund_amount;
-      fee_claimed = &tl->details.refund->refund_fee;
-      TALER_ARL_amount_add (&refunds,
-                            &refunds,
-                            amount_with_fee);
-      TALER_ARL_amount_add (&expenditures,
-                            &expenditures,
-                            fee_claimed);
-      /* Check if this refund is within the remit of the aggregation
-         we are investigating, if so, include it in the totals. */
-      if ( (0 == GNUNET_memcmp (merchant_pub,
-                                &tl->details.refund->merchant_pub)) &&
-           (0 == GNUNET_memcmp (h_contract_terms,
-                                &tl->details.refund->h_contract_terms)) )
-      {
-        GNUNET_log (GNUNET_ERROR_TYPE_INFO,
-                    "Detected applicable refund of %s\n",
-                    TALER_amount2s (amount_with_fee));
-        TALER_ARL_amount_add (&merchant_loss,
-                              &merchant_loss,
-                              amount_with_fee);
-        /* If there is a refund, we give back the deposit fee */
-        /* FIXME: wrong: only if this is a FULL
-           refund we refund the deposit fee! */
-        refund_deposit_fee = true;
-      }
-      /* Check that the fees given in the transaction list and in dki match */
-      if (0 !=
-          TALER_amount_cmp (&issue->fees.refund,
-                            fee_claimed))
-      {
-        /* Disagreement in fee structure between exchange and auditor! */
-        report_amount_arithmetic_inconsistency ("refund fee",
-                                                0,
-                                                fee_claimed,
-                                                &issue->fees.refund,
-                                                1);
-      }
-      break;
-    case TALER_EXCHANGEDB_TT_OLD_COIN_RECOUP:
-      amount_with_fee = &tl->details.old_coin_recoup->value;
-      /* We count recoups of refreshed coins like refunds for the dirty old
-         coin, as they equivalently _increase_ the remaining value on the
-         _old_ coin */
-      TALER_ARL_amount_add (&refunds,
-                            &refunds,
-                            amount_with_fee);
-      break;
-    case TALER_EXCHANGEDB_TT_RECOUP:
-      /* We count recoups of the coin as expenditures, as it
-         equivalently decreases the remaining value of the recouped coin. */
-      amount_with_fee = &tl->details.recoup->value;
-      TALER_ARL_amount_add (&expenditures,
-                            &expenditures,
-                            amount_with_fee);
-      break;
-    case TALER_EXCHANGEDB_TT_RECOUP_REFRESH:
-      /* We count recoups of the coin as expenditures, as it
-         equivalently decreases the remaining value of the recouped coin. */
-      amount_with_fee = &tl->details.recoup_refresh->value;
-      TALER_ARL_amount_add (&expenditures,
-                            &expenditures,
-                            amount_with_fee);
-      break;
-    case TALER_EXCHANGEDB_TT_PURSE_DEPOSIT:
-      amount_with_fee = &tl->details.purse_deposit->amount;
-      if (! tl->details.purse_deposit->refunded)
         TALER_ARL_amount_add (&expenditures,
                               &expenditures,
                               amount_with_fee);
-      break;
+        /* Check that the fees given in the transaction list and in dki match */
+        if (0 !=
+            TALER_amount_cmp (&issue->fees.refresh,
+                              fee_claimed))
+        {
+          /* Disagreement in fee structure between exchange and auditor */
+          report_amount_arithmetic_inconsistency ("melt fee",
+                                                  0,
+                                                  fee_claimed,
+                                                  &issue->fees.refresh,
+                                                  1);
+        }
+        break;
+      }
+    case TALER_EXCHANGEDB_TT_REFUND:
+      {
+        const struct TALER_Amount *amount_with_fee;
+
+        amount_with_fee = &tl->details.refund->refund_amount;
+        fee_claimed = &tl->details.refund->refund_fee;
+        TALER_ARL_amount_add (&refunds,
+                              &refunds,
+                              amount_with_fee);
+        TALER_ARL_amount_add (&expenditures,
+                              &expenditures,
+                              fee_claimed);
+        /* Check if this refund is within the remit of the aggregation
+           we are investigating, if so, include it in the totals. */
+        if ( (0 == GNUNET_memcmp (merchant_pub,
+                                  &tl->details.refund->merchant_pub)) &&
+             (0 == GNUNET_memcmp (h_contract_terms,
+                                  &tl->details.refund->h_contract_terms)) )
+        {
+          GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+                      "Detected applicable refund of %s\n",
+                      TALER_amount2s (amount_with_fee));
+          TALER_ARL_amount_add (&merchant_loss,
+                                &merchant_loss,
+                                amount_with_fee);
+        }
+        /* Check that the fees given in the transaction list and in dki match */
+        if (0 !=
+            TALER_amount_cmp (&issue->fees.refund,
+                              fee_claimed))
+        {
+          /* Disagreement in fee structure between exchange and auditor! */
+          report_amount_arithmetic_inconsistency ("refund fee",
+                                                  0,
+                                                  fee_claimed,
+                                                  &issue->fees.refund,
+                                                  1);
+        }
+        break;
+      }
+    case TALER_EXCHANGEDB_TT_OLD_COIN_RECOUP:
+      {
+        const struct TALER_Amount *amount_with_fee;
+
+        amount_with_fee = &tl->details.old_coin_recoup->value;
+        /* We count recoups of refreshed coins like refunds for the dirty old
+           coin, as they equivalently _increase_ the remaining value on the
+           _old_ coin */
+        TALER_ARL_amount_add (&refunds,
+                              &refunds,
+                              amount_with_fee);
+        break;
+      }
+    case TALER_EXCHANGEDB_TT_RECOUP:
+      {
+        const struct TALER_Amount *amount_with_fee;
+
+        /* We count recoups of the coin as expenditures, as it
+           equivalently decreases the remaining value of the recouped coin. */
+        amount_with_fee = &tl->details.recoup->value;
+        TALER_ARL_amount_add (&expenditures,
+                              &expenditures,
+                              amount_with_fee);
+        break;
+      }
+    case TALER_EXCHANGEDB_TT_RECOUP_REFRESH:
+      {
+        const struct TALER_Amount *amount_with_fee;
+
+        /* We count recoups of the coin as expenditures, as it
+           equivalently decreases the remaining value of the recouped coin. */
+        amount_with_fee = &tl->details.recoup_refresh->value;
+        TALER_ARL_amount_add (&expenditures,
+                              &expenditures,
+                              amount_with_fee);
+        break;
+      }
+    case TALER_EXCHANGEDB_TT_PURSE_DEPOSIT:
+      {
+        const struct TALER_Amount *amount_with_fee;
+
+        amount_with_fee = &tl->details.purse_deposit->amount;
+        if (! tl->details.purse_deposit->refunded)
+          TALER_ARL_amount_add (&expenditures,
+                                &expenditures,
+                                amount_with_fee);
+        break;
+      }
     }
   } /* for 'tl' */
 
@@ -574,14 +603,15 @@ check_transaction_history_for_deposit (
               "Aggregation loss due to refunds is %s\n",
               TALER_amount2s (&merchant_loss));
   *deposit_gain = *merchant_gain;
-  if ( (refund_deposit_fee) &&
+  if ( (NULL != deposited) &&
-       (NULL != deposit_fee) )
+       (NULL != deposit_fee) &&
+       (0 == TALER_amount_cmp (&refunds,
+                               deposited)) )
   {
-    /* We had a /deposit operation AND a /refund operation,
+    /* We had a /deposit operation AND /refund operations adding up to the
-       and should thus not charge the merchant the /deposit fee */
+       total deposited value including deposit fee. Thus, we should not
-    /* FIXME: this is wrong, the merchant never pays either
+       subtract the /deposit fee from the merchant gain (as it was also
-       fee, the deposit fee is simply not charged to the coin
+       refunded). */
-       IF there is a full refund. */
     TALER_ARL_amount_add (merchant_gain,
                           merchant_gain,
                           deposit_fee);
@@ -700,6 +730,10 @@ wire_transfer_information_cb (
   enum GNUNET_DB_QueryStatus qs;
   struct TALER_PaytoHashP hpt;
 
+  GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+              "DEPFEE: %s\n",
+              TALER_amount2s (deposit_fee));
+
   TALER_payto_hash (account_pay_uri,
                     &hpt);
   if (0 !=

src/exchangedb/plugin_exchangedb_postgres.c

@@ -1778,18 +1778,12 @@ prepare_statements (struct PostgresClosure *pg)
       "    FROM refunds"
       "   WHERE coin_pub IN (SELECT coin_pub FROM dep)"
       "     AND deposit_serial_id IN (SELECT deposit_serial_id FROM dep))"
-      " ,coins_with_fees AS (" /* find coins for which deposit fees apply */
-      "  SELECT"
-      "     coin_pub"
-      "    ,deposit_serial_id" /* ensures that if the same coin is deposited twice, it is in the list twice */
-      "    FROM dep"
-      "   WHERE deposit_serial_id NOT IN (SELECT deposit_serial_id FROM ref))"
       " ,fees AS (" /* find deposit fees for non-refunded deposits */
       "  SELECT"
       "    denom.fee_deposit_val AS fee_val"
       "   ,denom.fee_deposit_frac AS fee_frac"
       "   ,cs.deposit_serial_id" /* ensures we get the fee for each coin, not once per denomination */
-      "    FROM coins_with_fees cs"
+      "    FROM dep cs"
       "    JOIN known_coins kc" /* NOTE: may do a full join on the master, maybe find a left-join way to integrate with query above to push it to the shards? */
       "      USING (coin_pub)"
       "    JOIN denominations denom"

src/exchangedb/test_exchangedb.c

@@ -1397,6 +1397,7 @@ run (void *cls)
 
   {
     bool found;
+    bool nonce_ok;
     bool balance_ok;
     struct TALER_EXCHANGEDB_KycStatus kyc;
     uint64_t ruuid;
@@ -1408,9 +1409,11 @@ run (void *cls)
                                  now,
                                  &found,
                                  &balance_ok,
+                                 &nonce_ok,
                                  &kyc,
                                  &ruuid));
     GNUNET_assert (found);
+    GNUNET_assert (nonce_ok);
     GNUNET_assert (balance_ok);
     GNUNET_assert (! kyc.ok);
   }