fix iterate_matching_deposits(), LIMIT does not work with variables in Postgres (#4360)

6 files changed, 110 insertions, 20 deletions

diff --git a/src/exchange/taler-exchange-aggregator.c b/src/exchange/taler-exchange-aggregator.c
index a6b7de211..cfc11a5f9 100644
--- a/src/exchange/taler-exchange-aggregator.c
+++ b/src/exchange/taler-exchange-aggregator.c
@@ -78,9 +78,10 @@ static int test_mode;
  * of the smallest possible unit are aggregated, they do surpass the
  * "tiny" threshold beyond which we never trigger a wire transaction!
  *
- * TODO: make configurable (via config file or command line option)
+ * Note: do not change here, Postgres requires us to hard-code the
+ * LIMIT in the prepared statement.
  */
-static unsigned int aggregation_limit = 10000;
+static unsigned int aggregation_limit = TALER_EXCHANGEDB_MATCHING_DEPOSITS_LIMIT;
 
 
 /**
diff --git a/src/exchangedb/Makefile.am b/src/exchangedb/Makefile.am
index 416d5688f..0a7b0447c 100644
--- a/src/exchangedb/Makefile.am
+++ b/src/exchangedb/Makefile.am
@@ -80,6 +80,7 @@ test_exchangedb_postgres_SOURCES = \
   test_exchangedb.c
 test_exchangedb_postgres_LDADD = \
   libtalerexchangedb.la \
+  $(top_builddir)/src/json/libtalerjson.la \
   $(top_srcdir)/src/util/libtalerutil.la \
   $(top_srcdir)/src/pq/libtalerpq.la \
   -lgnunetutil -ljansson
diff --git a/src/exchangedb/plugin_exchangedb_postgres.c b/src/exchangedb/plugin_exchangedb_postgres.c
index 6807e7563..c38c0827f 100644
--- a/src/exchangedb/plugin_exchangedb_postgres.c
+++ b/src/exchangedb/plugin_exchangedb_postgres.c
@@ -952,7 +952,7 @@ postgres_prepare (PGconn *db_conn)
            " tiny=false AND"
            " done=false"
            " ORDER BY wire_deadline ASC"
-           " LIMIT 1;",
+           " LIMIT 1",
            0, NULL);
 
   /* Used in #postgres_iterate_matching_deposits() */
@@ -975,8 +975,8 @@ postgres_prepare (PGconn *db_conn)
            " h_wire=$2 AND"
            " done=false"
            " ORDER BY wire_deadline ASC"
-           " LIMIT $3",
-           3, NULL);
+           " LIMIT " TALER_EXCHANGEDB_MATCHING_DEPOSITS_LIMIT_STR,
+           2, NULL);
 
   /* Used in #postgres_mark_deposit_tiny() */
   PREPARE ("mark_deposit_tiny",
@@ -2336,7 +2336,6 @@ postgres_iterate_matching_deposits (void *cls,
   struct GNUNET_PQ_QueryParam params[] = {
     GNUNET_PQ_query_param_auto_from_type (merchant_pub),
     GNUNET_PQ_query_param_auto_from_type (h_wire),
-    GNUNET_PQ_query_param_uint32 (&limit),
     GNUNET_PQ_query_param_end
   };
   PGresult *result;
@@ -2344,8 +2343,8 @@ postgres_iterate_matching_deposits (void *cls,
   unsigned int n;
 
   result = GNUNET_PQ_exec_prepared (session->conn,
-                                   "deposits_iterate_matching",
-                                   params);
+                                    "deposits_iterate_matching",
+                                    params);
   if (PGRES_TUPLES_OK !=
       PQresultStatus (result))
   {
@@ -2366,28 +2365,25 @@ postgres_iterate_matching_deposits (void *cls,
     struct TALER_Amount deposit_fee;
     struct GNUNET_TIME_Absolute wire_deadline;
     struct GNUNET_HashCode h_contract;
-    struct TALER_MerchantPublicKeyP merchant_pub;
     struct TALER_CoinSpendPublicKeyP coin_pub;
     uint64_t transaction_id;
     uint64_t serial_id;
     int ret;
     struct GNUNET_PQ_ResultSpec rs[] = {
       GNUNET_PQ_result_spec_uint64 ("serial_id",
-                                   &serial_id),
+                                    &serial_id),
       GNUNET_PQ_result_spec_uint64 ("transaction_id",
-                                   &transaction_id),
+                                    &transaction_id),
       TALER_PQ_result_spec_amount ("amount_with_fee",
                                    &amount_with_fee),
       TALER_PQ_result_spec_amount ("deposit_fee",
                                    &deposit_fee),
       GNUNET_PQ_result_spec_absolute_time ("wire_deadline",
-                                          &wire_deadline),
+                                           &wire_deadline),
       GNUNET_PQ_result_spec_auto_from_type ("h_contract",
-                                           &h_contract),
-      GNUNET_PQ_result_spec_auto_from_type ("merchant_pub",
-                                           &merchant_pub),
+                                            &h_contract),
       GNUNET_PQ_result_spec_auto_from_type ("coin_pub",
-                                           &coin_pub),
+                                            &coin_pub),
       GNUNET_PQ_result_spec_end
     };
     if (GNUNET_OK !=
@@ -2399,7 +2395,7 @@ postgres_iterate_matching_deposits (void *cls,
     }
     ret = deposit_cb (deposit_cb_cls,
                       serial_id,
-                      &merchant_pub,
+                      merchant_pub,
                       &coin_pub,
                       &amount_with_fee,
                       &deposit_fee,
diff --git a/src/exchangedb/test_exchangedb.c b/src/exchangedb/test_exchangedb.c
index f33bb18c3..8d06c0072 100644
--- a/src/exchangedb/test_exchangedb.c
+++ b/src/exchangedb/test_exchangedb.c
@@ -20,6 +20,7 @@
  */
 #include "platform.h"
 #include "taler_exchangedb_lib.h"
+#include "taler_json_lib.h"
 #include "taler_exchangedb_plugin.h"
 
 static int result;
@@ -547,6 +548,70 @@ cb_wtid_check (void *cls,
 
 
 /**
+ * Function called with details about deposits that
+ * have been made.  Called in the test on the
+ * deposit given in @a cls.
+ *
+ * @param cls closure a `struct TALER_EXCHANGEDB_Deposit *`
+ * @param rowid unique ID for the deposit in our DB, used for marking
+ *              it as 'tiny' or 'done'
+ * @param merchant_pub public key of the merchant
+ * @param coin_pub public key of the coin
+ * @param amount_with_fee amount that was deposited including fee
+ * @param deposit_fee amount the exchange gets to keep as transaction fees
+ * @param transaction_id unique transaction ID chosen by the merchant
+ * @param h_contract hash of the contract between merchant and customer
+ * @param wire_deadline by which the merchant adviced that he would like the
+ *        wire transfer to be executed
+ * @param wire wire details for the merchant, NULL from iterate_matching_deposits()
+ * @return #GNUNET_OK to continue to iterate, #GNUNET_SYSERR if deposit does
+ *         not match our expectations
+ */
+static int
+deposit_cb (void *cls,
+            unsigned long long rowid,
+            const struct TALER_MerchantPublicKeyP *merchant_pub,
+            const struct TALER_CoinSpendPublicKeyP *coin_pub,
+            const struct TALER_Amount *amount_with_fee,
+            const struct TALER_Amount *deposit_fee,
+            uint64_t transaction_id,
+            const struct GNUNET_HashCode *h_contract,
+            struct GNUNET_TIME_Absolute wire_deadline,
+            const json_t *wire)
+{
+  struct TALER_EXCHANGEDB_Deposit *deposit = cls;
+  struct GNUNET_HashCode h_wire;
+
+  if (NULL != wire)
+    TALER_JSON_hash (wire, &h_wire);
+  if ( (0 != memcmp (merchant_pub,
+                     &deposit->merchant_pub,
+                     sizeof (struct TALER_MerchantPublicKeyP))) ||
+       (0 != TALER_amount_cmp (amount_with_fee,
+                               &deposit->amount_with_fee)) ||
+       (0 != TALER_amount_cmp (deposit_fee,
+                               &deposit->deposit_fee)) ||
+       (0 != memcmp (h_contract,
+                     &deposit->h_contract,
+                     sizeof (struct GNUNET_HashCode))) ||
+       (0 != memcmp (coin_pub,
+                     &deposit->coin.coin_pub,
+                     sizeof (struct TALER_CoinSpendPublicKeyP))) ||
+       (transaction_id != deposit->transaction_id) ||
+       ( (NULL != wire) &&
+         (0 != memcmp (&h_wire,
+                       &deposit->h_wire,
+                       sizeof (struct GNUNET_HashCode))) ) )
+  {
+    GNUNET_break (0);
+    return GNUNET_SYSERR;
+  }
+
+  return GNUNET_OK;
+}
+
+
+/**
  * Main function that will be run by the scheduler.
  *
  * @param cls closure with config
@@ -739,14 +804,16 @@ run (void *cls,
   RND_BLK (&deposit.csig);
   RND_BLK (&deposit.merchant_pub);
   RND_BLK (&deposit.h_contract);
-  RND_BLK (&deposit.h_wire);
   wire = json_loads (json_wire_str, 0, NULL);
+  TALER_JSON_hash (wire,
+                   &deposit.h_wire);
   deposit.wire = wire;
   deposit.transaction_id =
       GNUNET_CRYPTO_random_u64 (GNUNET_CRYPTO_QUALITY_WEAK, UINT64_MAX);
   deposit.amount_with_fee = value;
   GNUNET_assert (GNUNET_OK ==
                  TALER_amount_get_zero (CURRENCY, &deposit.deposit_fee));
+  result = 8;
   FAILIF (GNUNET_OK !=
           plugin->insert_deposit (plugin->cls,
                                   session, &deposit));
@@ -754,6 +821,15 @@ run (void *cls,
           plugin->have_deposit (plugin->cls,
                                 session,
                                 &deposit));
+  result = 9;
+  FAILIF (1 !=
+          plugin->iterate_matching_deposits (plugin->cls,
+                                             session,
+                                             &deposit.h_wire,
+                                             &deposit.merchant_pub,
+                                             &deposit_cb, &deposit,
+                                             2));
+  result = 10;
   deposit2 = deposit;
   deposit2.transaction_id++;     /* should fail if transaction id is different */
   FAILIF (GNUNET_NO !=
@@ -880,6 +956,9 @@ main (int argc,
     GNUNET_break (0);
     return -1;
   }
+  GNUNET_log_setup (argv[0],
+                    "WARNING",
+                    NULL);
   plugin_name++;
   (void) GNUNET_asprintf (&testname,
                           "test-exchange-db-%s", plugin_name);
diff --git a/src/include/taler_exchangedb_plugin.h b/src/include/taler_exchangedb_plugin.h
index cb1dcb344..3646981cd 100644
--- a/src/include/taler_exchangedb_plugin.h
+++ b/src/include/taler_exchangedb_plugin.h
@@ -947,6 +947,17 @@ struct TALER_EXCHANGEDB_Plugin
                         void *deposit_cb_cls);
 
 
+/**
+ * Maximum number of results we return from iterate_matching_deposits().
+ *
+ * Limit on the number of transactions we aggregate at once.  Note
+ * that the limit must be big enough to ensure that when transactions
+ * of the smallest possible unit are aggregated, they do surpass the
+ * "tiny" threshold beyond which we never trigger a wire transaction!
+ */
+#define TALER_EXCHANGEDB_MATCHING_DEPOSITS_LIMIT 10000
+#define TALER_EXCHANGEDB_MATCHING_DEPOSITS_LIMIT_STR "10000"
+
   /**
    * Obtain information about other pending deposits for the same
    * destination.  Those deposits must not already be "done".
@@ -957,7 +968,9 @@ struct TALER_EXCHANGEDB_Plugin
    * @param merchant_pub public key of the merchant
    * @param deposit_cb function to call for each deposit
    * @param deposit_cb_cls closure for @a deposit_cb
-   * @param limit maximum number of matching deposits to return
+   * @param limit maximum number of matching deposits to return; should
+   *        be #TALER_EXCHANGEDB_MATCHING_DEPOSITS_LIMIT, larger values
+   *        are not supported, smaller values would be inefficient.
    * @return number of rows processed, 0 if none exist,
    *         #GNUNET_SYSERR on error
    */
diff --git a/src/include/taler_json_lib.h b/src/include/taler_json_lib.h
index 3dd661a91..79589dba7 100644
--- a/src/include/taler_json_lib.h
+++ b/src/include/taler_json_lib.h
@@ -89,7 +89,7 @@ TALER_JSON_spec_denomination_signature (const char *field,
  * @return #GNUNET_OK on success, #GNUNET_SYSERR on error
  */
 int
-TALER_JSON_hash (json_t *json,
+TALER_JSON_hash (const json_t *json,
                  struct GNUNET_HashCode *hc);
 
 #endif /* TALER_JSON_LIB_H_ */