towards fixing #3717 and #3633

4 files changed, 148 insertions, 262 deletions

diff --git a/src/include/taler_mintdb_plugin.h b/src/include/taler_mintdb_plugin.h
index 540cb88ca..ffa1b13d3 100644
--- a/src/include/taler_mintdb_plugin.h
+++ b/src/include/taler_mintdb_plugin.h
@@ -682,28 +682,28 @@ struct TALER_MINTDB_Plugin
                   struct TALER_MINTDB_Session *db,
                   struct TALER_MINTDB_Reserve *reserve);
 
-  /* FIXME: add functions to add bank transfers to our DB
-     (and to test if we already did add one) (#3633/#3717) */
-
 
   /**
-   * Insert a incoming transaction into reserves.  New reserves are also created
-   * through this function.
+   * Insert a incoming transaction into reserves.  New reserves are
+   * also created through this function.
    *
    * @param cls the @e cls of this struct with the plugin-specific state
    * @param db the database connection handle
-   * @param reserve the reserve structure.  The public key of the reserve should
-   *          be set here.  Upon successful execution of this function, the
-   *          balance and expiration of the reserve will be updated.
+   * @param reserve_pub public key of the reserve
    * @param balance the amount that has to be added to the reserve
+   * @param details bank transaction details justifying the increment,
+   *        must be unique for each incoming transaction
    * @param expiry the new expiration time for the reserve
-   * @return #GNUNET_OK upon success; #GNUNET_SYSERR upon failures
+   * @return #GNUNET_OK upon success; #GNUNET_NO if the given
+   *         @a details are already known for this @a reserve_pub,
+   *         #GNUNET_SYSERR upon failures (DB error, incompatible currency)
    */
   int
   (*reserves_in_insert) (void *cls,
                          struct TALER_MINTDB_Session *db,
-                         struct TALER_MINTDB_Reserve *reserve,
+                         const struct TALER_ReservePublicKeyP *reserve_pub,
                          const struct TALER_Amount *balance,
+                         const char *details,
                          const struct GNUNET_TIME_Absolute expiry);
 
 
diff --git a/src/mint-tools/Makefile.am b/src/mint-tools/Makefile.am
index a61ab6a93..e22df1ed7 100644
--- a/src/mint-tools/Makefile.am
+++ b/src/mint-tools/Makefile.am
@@ -41,7 +41,6 @@ taler_mint_reservemod_LDADD = \
   $(top_builddir)/src/util/libtalerutil.la \
   $(top_builddir)/src/pq/libtalerpq.la \
   $(top_builddir)/src/mintdb/libtalermintdb.la \
-  -lpq \
   -lgnunetutil  $(XLIB)
 taler_mint_reservemod_LDFLAGS = \
   $(POSTGRESQL_LDFLAGS)
@@ -57,7 +56,6 @@ taler_mint_dbinit_LDADD = \
   $(top_builddir)/src/util/libtalerutil.la \
   $(top_builddir)/src/pq/libtalerpq.la \
   $(top_builddir)/src/mintdb/libtalermintdb.la \
-  -lpq \
   -lgnunetutil  $(XLIB)
 taler_mint_dbinit_LDFLAGS = \
   $(POSTGRESQL_LDFLAGS)
diff --git a/src/mint-tools/taler-mint-reservemod.c b/src/mint-tools/taler-mint-reservemod.c
index 762830783..5bb6bce46 100644
--- a/src/mint-tools/taler-mint-reservemod.c
+++ b/src/mint-tools/taler-mint-reservemod.c
@@ -28,16 +28,15 @@
 #include "taler_mintdb_plugin.h"
 #include "taler_mintdb_lib.h"
 
-
 /**
- * Director of the mint, containing the keys.
+ * After what time to inactive reserves expire?
  */
-static char *mint_directory;
+#define RESERVE_EXPIRATION GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_YEARS, 5)
 
 /**
- * Public key of the reserve to manipulate.
+ * Director of the mint, containing the keys.
  */
-static struct GNUNET_CRYPTO_EddsaPublicKey *reserve_pub;
+static char *mint_directory;
 
 /**
  * Handle to the mint's configuration
@@ -45,164 +44,9 @@ static struct GNUNET_CRYPTO_EddsaPublicKey *reserve_pub;
 static struct GNUNET_CONFIGURATION_Handle *cfg;
 
 /**
- * Database connection handle.
- */
-static PGconn *db_conn;
-
-
-/**
- * Create a new or add to existing reserve.  Fails if currencies do
- * not match.
- *
- * @param denom denomination to add
- * @return #GNUNET_OK on success,
- *         #GNUNET_SYSERR on error
+ * Our DB plugin.
  */
-// FIXME: this should use the DB abstraction layer. (#3717)
-// FIXME: this should be done by adding an inbound transaction
-//        to the table with the transactions for this reserve,
-//        not by modifying some 'total' value for the reserve!
-//        (we should in fact probably never modify, always just append!) (#3633)
-static int
-reservemod_add (struct TALER_Amount denom)
-{
-  PGresult *result;
-  const void *param_values[] = {
-    reserve_pub
-  };
-  int param_lengths[] = {
-    sizeof(struct GNUNET_CRYPTO_EddsaPublicKey)
-  };
-  int param_formats[] = {
-    1
-  };
-  struct TALER_Amount old_denom;
-  struct TALER_Amount new_denom;
-  struct TALER_AmountNBO new_denom_nbo;
-
-  result = PQexecParams (db_conn,
-                         "SELECT balance_value, balance_fraction, balance_currency"
-                         " FROM reserves"
-                         " WHERE reserve_pub=$1"
-                         " LIMIT 1;",
-                         1,
-                         NULL,
-                         (const char * const *) param_values,
-                         param_lengths,
-                         param_formats,
-                         1);
-  if (PGRES_TUPLES_OK != PQresultStatus (result))
-  {
-    fprintf (stderr,
-             "Select failed: %s\n",
-             PQresultErrorMessage (result));
-    return GNUNET_SYSERR;
-  }
-  if (0 == PQntuples (result))
-  {
-    struct GNUNET_TIME_AbsoluteNBO exnbo;
-    uint32_t value = htonl (denom.value);
-    uint32_t fraction = htonl (denom.fraction);
-    const void *param_values[] = {
-      reserve_pub,
-      &value,
-      &fraction,
-      denom.currency,
-      &exnbo
-    };
-    int param_lengths[] = {
-      sizeof (struct GNUNET_CRYPTO_EddsaPublicKey),
-      sizeof (uint32_t),
-      sizeof (uint32_t),
-      strlen (denom.currency),
-      sizeof (struct GNUNET_TIME_AbsoluteNBO)
-    };
-    int param_formats[] = {
-      1, 1, 1, 1, 1
-    };
-
-    exnbo = GNUNET_TIME_absolute_hton (GNUNET_TIME_relative_to_absolute (GNUNET_TIME_UNIT_YEARS));
-    result = PQexecParams (db_conn,
-                           "INSERT INTO reserves (reserve_pub, balance_value, balance_fraction, balance_currency, expiration_date)"
-                           " VALUES ($1,$2,$3,$4,$5);",
-                           5,
-                           NULL,
-                           (const char **) param_values,
-                           param_lengths,
-                           param_formats,
-                           1);
-
-    if (PGRES_COMMAND_OK != PQresultStatus (result))
-    {
-      fprintf (stderr,
-               "Insert failed: %s\n",
-               PQresultErrorMessage (result));
-      return GNUNET_SYSERR;
-    }
-  }
-  else
-  {
-    const void *param_values[] = {
-      &new_denom_nbo.value,
-      &new_denom_nbo.fraction,
-      reserve_pub
-    };
-    int param_lengths[] = {
-      sizeof (new_denom_nbo.value),
-      sizeof (new_denom_nbo.fraction),
-      sizeof (struct GNUNET_CRYPTO_EddsaPublicKey)
-    };
-    int param_formats[] = {
-      1, 1, 1
-    };
-
-    GNUNET_assert (GNUNET_OK ==
-                   TALER_PQ_extract_amount (result, 0,
-                                            "balance_value",
-                                            "balance_fraction",
-                                            "balance_currency",
-                                            &old_denom));
-    if (GNUNET_OK !=
-        TALER_amount_add (&new_denom,
-                          &old_denom,
-                          &denom))
-    {
-      fprintf (stderr,
-               "Integer overflow when computing new balance!\n");
-      return GNUNET_SYSERR;
-    }
-    TALER_amount_hton (&new_denom_nbo,
-                       &new_denom);
-    result = PQexecParams (db_conn,
-                           "UPDATE reserves"
-                           " SET balance_value = $1, balance_fraction = $2, status_sig = NULL, status_sign_pub = NULL"
-                           " WHERE reserve_pub = $3;",
-                           3,
-                           NULL,
-                           (const char **) param_values,
-                           param_lengths,
-                           param_formats,
-                           1);
-
-    if (PGRES_COMMAND_OK != PQresultStatus (result))
-    {
-      fprintf (stderr,
-               "Update failed: %s\n",
-               PQresultErrorMessage (result));
-      return GNUNET_SYSERR;
-    }
-    /* Yes, for historic reasons libpq returns a 'const char *'... */
-    if (0 != strcmp ("1",
-                     PQcmdTuples (result)))
-    {
-      fprintf (stderr,
-               "Update failed (updated `%s' tupes instead of '1')\n",
-               PQcmdTuples (result));
-      return GNUNET_SYSERR;
-    }
-  }
-  return GNUNET_OK;
-}
+static struct TALER_MINTDB_Plugin *plugin;
 
 
 /**
@@ -215,15 +59,23 @@ reservemod_add (struct TALER_Amount denom)
 int
 main (int argc, char *const *argv)
 {
-  static char *reserve_pub_str;
-  static char *add_str;
-  static const struct GNUNET_GETOPT_CommandLineOption options[] = {
+  char *reserve_pub_str = NULL;
+  char *add_str = NULL;
+  struct TALER_Amount add_value;
+  char *details = NULL;
+  struct TALER_ReservePublicKeyP reserve_pub;
+  struct GNUNET_TIME_Absolute expiration;
+  struct TALER_MINTDB_Session *session;
+  const struct GNUNET_GETOPT_CommandLineOption options[] = {
     {'a', "add", "DENOM",
      "value to add", 1,
      &GNUNET_GETOPT_set_string, &add_str},
     {'d', "mint-dir", "DIR",
      "mint directory with keys to update", 1,
      &GNUNET_GETOPT_set_filename, &mint_directory},
+    {'D', "details", "JSON",
+     "details about the bank transaction which justify why we add this amount", 1,
+     &GNUNET_GETOPT_set_string, &details},
     TALER_GETOPT_OPTION_HELP ("Deposit funds into a Taler reserve"),
     {'R', "reserve", "KEY",
      "reserve (public key) to modify", 1,
@@ -231,7 +83,7 @@ main (int argc, char *const *argv)
     GNUNET_GETOPT_OPTION_VERSION (VERSION "-" VCS_VERSION),
     GNUNET_GETOPT_OPTION_END
   };
-  char *connection_cfg_str;
+  int ret;
 
   GNUNET_assert (GNUNET_OK ==
                  GNUNET_log_setup ("taler-mint-reservemod",
@@ -248,19 +100,35 @@ main (int argc, char *const *argv)
              "Mint directory not given\n");
     return 1;
   }
-
-  reserve_pub = GNUNET_new (struct GNUNET_CRYPTO_EddsaPublicKey);
   if ((NULL == reserve_pub_str) ||
       (GNUNET_OK !=
        GNUNET_STRINGS_string_to_data (reserve_pub_str,
                                       strlen (reserve_pub_str),
-                                      reserve_pub,
-                                      sizeof (struct GNUNET_CRYPTO_EddsaPublicKey))))
+                                      &reserve_pub,
+                                      sizeof (struct TALER_ReservePublicKeyP))))
   {
     fprintf (stderr,
              "Parsing reserve key invalid\n");
     return 1;
   }
+  if ( (NULL == add_str) ||
+       (GNUNET_OK !=
+        TALER_string_to_amount (add_str,
+                                &add_value)) )
+  {
+    fprintf (stderr,
+             "Failed to parse currency amount `%s'\n",
+             add_str);
+    return 1;
+  }
+
+  if (NULL == details)
+  {
+    fprintf (stderr,
+             "No wiring details given (justification required)\n");
+    return 1;
+  }
+
   cfg = TALER_config_load (mint_directory);
   if (NULL == cfg)
   {
@@ -268,46 +136,59 @@ main (int argc, char *const *argv)
              "Failed to load mint configuration\n");
     return 1;
   }
+  ret = 1;
+  if (NULL ==
+      (plugin = TALER_MINTDB_plugin_load (cfg)))
+  {
+    fprintf (stderr,
+             "Failed to initialize database plugin.\n");
+    goto cleanup;
+  }
+
+  session = plugin->get_session (plugin->cls,
+                                 GNUNET_NO);
+  if (NULL == session)
+  {
+    fprintf (stderr,
+             "Failed to initialize DB session\n");
+    goto cleanup;
+  }
   if (GNUNET_OK !=
-      GNUNET_CONFIGURATION_get_value_string (cfg,
-                                             "mint",
-                                             "db",
-                                             &connection_cfg_str))
+      plugin->start (plugin->cls,
+                     session))
   {
     fprintf (stderr,
-             "Database configuration string not found\n");
-    return 1;
+             "Failed to start transaction\n");
+    goto cleanup;
   }
-  db_conn = PQconnectdb (connection_cfg_str);
-  if (CONNECTION_OK != PQstatus (db_conn))
+  expiration = GNUNET_TIME_relative_to_absolute (RESERVE_EXPIRATION);
+  if (GNUNET_OK !=
+      plugin->reserves_in_insert (plugin->cls,
+                                  session,
+                                  &reserve_pub,
+                                  &add_value,
+                                  details,
+                                  expiration))
   {
     fprintf (stderr,
-             "Database connection failed: %s\n",
-             PQerrorMessage (db_conn));
-    return 1;
+             "Failed to update reserve.\n");
+    goto cleanup;
   }
-  if (NULL != add_str)
+  if (GNUNET_OK !=
+      plugin->commit (plugin->cls,
+                     session))
   {
-    struct TALER_Amount add_value;
-
-    if (GNUNET_OK !=
-        TALER_string_to_amount (add_str,
-                                &add_value))
-    {
-      fprintf (stderr,
-               "Failed to parse currency amount `%s'\n",
-               add_str);
-      return 1;
-    }
-    if (GNUNET_OK !=
-        reservemod_add (add_value))
-    {
-      fprintf (stderr,
-               "Failed to update reserve.\n");
-      return 1;
-    }
+    fprintf (stderr,
+             "Failed to commit transaction\n");
+    goto cleanup;
   }
-  return 0;
+  ret = 0;
+ cleanup:
+  if (NULL != plugin)
+    TALER_MINTDB_plugin_unload (plugin);
+  if (NULL != cfg)
+    GNUNET_CONFIGURATION_destroy (cfg);
+  return ret;
 }
 
 /* end taler-mint-reservemod.c */
diff --git a/src/mintdb/plugin_mintdb_postgres.c b/src/mintdb/plugin_mintdb_postgres.c
index 8ed374f03..7d3a3e870 100644
--- a/src/mintdb/plugin_mintdb_postgres.c
+++ b/src/mintdb/plugin_mintdb_postgres.c
@@ -214,11 +214,17 @@ postgres_create_tables (void *cls,
           ",balance_val INT8 NOT NULL"
           ",balance_frac INT4 NOT NULL"
           ",balance_curr VARCHAR("TALER_CURRENCY_LEN_STR") NOT NULL"
+          ",details VARCHAR NOT NULL "
           ",expiration_date INT8 NOT NULL"
+          " CONSTRAINT unique_details PRIMARY KEY (reserve_pub,details)"
           ");");
-  /* Create an index on the foreign key as it is not created automatically by PSQL */
+  /* Create indices on reserves_in */
   SQLEXEC ("CREATE INDEX reserves_in_reserve_pub_index"
            " ON reserves_in (reserve_pub);");
+  SQLEXEC ("CREATE INDEX reserves_in_reserve_pub_details_index"
+           " ON reserves_in (reserve_pub,details);");
+  SQLEXEC ("CREATE INDEX expiration_index"
+           " ON reserves_in (expiration_date);");
   /* Table with the withdraw operations that have been performed on a reserve.
      TODO: maybe rename to "reserves_out"?
      TODO: is blind_ev really a _primary key_? Is this constraint useful? */
@@ -234,7 +240,7 @@ postgres_create_tables (void *cls,
   SQLEXEC ("CREATE INDEX collectable_blindcoins_reserve_pub_index ON"
            " collectable_blindcoins (reserve_pub)");
   /* Table with coins that have been (partially) spent, used to detect
-     double-spending.  
+     double-spending.
      TODO: maybe rename to "spent_coins"?
      TODO: maybe have two tables, one for spending and one for refreshing, instead of optional refresh_session_hash? */
   SQLEXEC("CREATE TABLE IF NOT EXISTS known_coins "
@@ -426,8 +432,9 @@ postgres_prepare (PGconn *db_conn)
            " balance_val,"
            " balance_frac,"
            " balance_curr,"
+           " details,"
            " expiration_date) VALUES ("
-           " $1, $2, $3, $4, $5);",
+           " $1, $2, $3, $4, $5, $6);",
            5, NULL);
   PREPARE ("get_reserves_in_transactions",
            "SELECT"
@@ -435,6 +442,7 @@ postgres_prepare (PGconn *db_conn)
            ",balance_frac"
            ",balance_curr"
            ",expiration_date"
+           ",details"
            " FROM reserves_in WHERE reserve_pub=$1",
            1, NULL);
   PREPARE ("insert_collectable_blindcoin",
@@ -676,6 +684,7 @@ postgres_prepare (PGconn *db_conn)
            " FROM deposits WHERE "
            " coin_pub = $1",
            1, NULL);
+
   return GNUNET_OK;
 #undef PREPARE
 }
@@ -905,7 +914,7 @@ postgres_reserve_get (void *cls,
   if (PGRES_TUPLES_OK != PQresultStatus (result))
   {
     QUERY_ERR (result);
-    PQclear (result);
+    PQclear (resultE);
     return GNUNET_SYSERR;
   }
   if (0 == PQntuples (result))
@@ -914,8 +923,8 @@ postgres_reserve_get (void *cls,
     return GNUNET_NO;
   }
   EXITIF (GNUNET_OK !=
-	  TALER_PQ_extract_result (result, 
-				   rs, 
+	  TALER_PQ_extract_result (result,
+				   rs,
 				   0));
   PQclear (result);
   return GNUNET_OK;
@@ -974,38 +983,39 @@ postgres_reserves_update (void *cls,
  *
  * @param cls the `struct PostgresClosure` with the plugin-specific state
  * @param session the database connection handle
- * @param reserve the reserve structure.  The public key of the reserve should
- *          be set here.  Upon successful execution of this function, the
- *          balance and expiration of the reserve will be updated.
+ * @param reserve_pub public key of the reserve
  * @param balance the amount that has to be added to the reserve
+ * @param details bank transaction details justifying the increment,
+ *        must be unique for each incoming transaction
  * @param expiry the new expiration time for the reserve
- * @return #GNUNET_OK upon success; #GNUNET_SYSERR upon failures
+ * @return #GNUNET_OK upon success; #GNUNET_NO if the given
+ *         @a details are already known for this @a reserve_pub,
+ *         #GNUNET_SYSERR upon failures (DB error, incompatible currency)
  */
 static int
 postgres_reserves_in_insert (void *cls,
                              struct TALER_MINTDB_Session *session,
-                             struct TALER_MINTDB_Reserve *reserve,
+                             struct TALER_ReservePublicKeyP *reserve_pub,
                              const struct TALER_Amount *balance,
+                             const char *details,
                              const struct GNUNET_TIME_Absolute expiry)
 {
   PGresult *result;
   int reserve_exists;
+  struct TALER_MINTDB_Reserve reserve;
+  struct TALER_MINTDB_Reserve updated_reserve;
 
   result = NULL;
-  if (NULL == reserve)
-  {
-    GNUNET_break (0);
-    return GNUNET_SYSERR;
-  }
   if (GNUNET_OK != postgres_start (cls,
                                    session))
   {
     GNUNET_break (0);
     return GNUNET_SYSERR;
   }
+  reserve.pub = *reserve_pub;
   reserve_exists = postgres_reserve_get (cls,
                                          session,
-                                         reserve);
+                                         &reserve);
   if (GNUNET_SYSERR == reserve_exists)
   {
     postgres_rollback (cls,
@@ -1017,7 +1027,7 @@ postgres_reserves_in_insert (void *cls,
     GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
                 "Reserve does not exist; creating a new one\n");
     struct TALER_PQ_QueryParam params[] = {
-      TALER_PQ_QUERY_PARAM_PTR (&reserve->pub),
+      TALER_PQ_QUERY_PARAM_PTR (reserve_pub),
       TALER_PQ_QUERY_PARAM_AMOUNT (balance),
       TALER_PQ_QUERY_PARAM_ABSOLUTE_TIME (expiry),
       TALER_PQ_QUERY_PARAM_END
@@ -1031,13 +1041,34 @@ postgres_reserves_in_insert (void *cls,
       goto rollback;
     }
   }
+  else
+  {
+    /* Update reserve */
+    updated_reserve.pub = reserve->pub;
+
+    if (GNUNET_OK !=
+        TALER_amount_add (&updated_reserve.balance,
+                          &reserve->balance,
+                          balance))
+    {
+      /* currency overflow or incompatible currency */
+      GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+                  "Attempt to deposit incompatible amount into reserve\n");
+      goto rollback;
+    }
+    updated_reserve.expiry = GNUNET_TIME_absolute_max (expiry,
+                                                       reserve->expiry);
+
+  }
   if (NULL != result)
     PQclear (result);
   result = NULL;
-  /* create new incoming transaction */
+  /* create new incoming transaction, SQL "primary key" logic
+     is used to guard against duplicates! */
   struct TALER_PQ_QueryParam params[] = {
     TALER_PQ_QUERY_PARAM_PTR (&reserve->pub),
     TALER_PQ_QUERY_PARAM_AMOUNT (balance),
+    TALER_PQ_QUERY_PARAM_PTR_SIZED (details, strlen (details)),
     TALER_PQ_QUERY_PARAM_ABSOLUTE_TIME (expiry),
     TALER_PQ_QUERY_PARAM_END
   };
@@ -1051,41 +1082,18 @@ postgres_reserves_in_insert (void *cls,
   }
   PQclear (result);
   result = NULL;
-  if (GNUNET_NO == reserve_exists)
-  {
-    if (GNUNET_OK != postgres_commit (cls,
-                                      session))
-      return GNUNET_SYSERR;
-    reserve->balance = *balance;
-    reserve->expiry = expiry;
-    return GNUNET_OK;
-  }
-  /* Update reserve */
-  struct TALER_MINTDB_Reserve updated_reserve;
-  updated_reserve.pub = reserve->pub;
-
-  if (GNUNET_OK !=
-      TALER_amount_add (&updated_reserve.balance,
-                        &reserve->balance,
-                        balance))
-  {
-    return GNUNET_SYSERR;
-  }
-  updated_reserve.expiry = GNUNET_TIME_absolute_max (expiry,
-						     reserve->expiry);
-  if (GNUNET_OK != postgres_reserves_update (cls,
-                                             session,
-                                             &updated_reserve))
+  if ( (GNUNET_YES == reserve_exists) &&
+       (GNUNET_OK != postgres_reserves_update (cls,
+                                               session,
+                                               &updated_reserve)) )
     goto rollback;
   if (GNUNET_OK != postgres_commit (cls,
                                     session))
     return GNUNET_SYSERR;
-  reserve->balance = updated_reserve.balance;
-  reserve->expiry = updated_reserve.expiry;
   return GNUNET_OK;
-
  rollback:
-  PQclear (result);
+  if (NULL != result)
+    PQclear (result);
   postgres_rollback (cls,
                      session);
   return GNUNET_SYSERR;
@@ -2426,7 +2434,6 @@ postgres_get_coin_transactions (void *cls,
 }
 
 
-
 /**
  * Initialize Postgres database subsystem.
  *