commit 5a60998f34ea19fb1433f63a8c3a6f7f67c134cc
parent 649e8a109fadcf049643e2f1e736cfeb7c825d23
Author: Christian Grothoff <christian@grothoff.org>
Date: Thu, 6 Aug 2026 17:45:36 +0200
handle refunds between original amount minus deposit fee correctly in aggregation
Diffstat:
5 files changed, 568 insertions(+), 16 deletions(-)
diff --git a/src/exchangedb/do_aggregate.c b/src/exchangedb/do_aggregate.c
@@ -101,16 +101,45 @@ TALER_EXCHANGEDB_do_aggregate (
" AND norm.batch_deposit_serial_id = cdep.batch_deposit_serial_id"
" AND norm.norm_refund_val = (cdep.amount).val"
" AND norm.norm_refund_frac = (cdep.amount).frac))"
+ " ,remainders AS (" /* what is left of each deposit after refunds */
+ " SELECT"
+ " cdep.coin_pub"
+ " ,cdep.batch_deposit_serial_id"
+ " ,CAST( (cdep.amount).val"
+ " - COALESCE(norm.norm_refund_val,0)"
+ " - CASE WHEN (cdep.amount).frac"
+ " < COALESCE(norm.norm_refund_frac,0)"
+ " THEN 1 ELSE 0 END AS INT8) AS rem_val"
+ " ,CAST( (cdep.amount).frac"
+ " - COALESCE(norm.norm_refund_frac,0)"
+ " + CASE WHEN (cdep.amount).frac"
+ " < COALESCE(norm.norm_refund_frac,0)"
+ " THEN 100000000 ELSE 0 END AS INT8) AS rem_frac"
+ " FROM cdep"
+ " LEFT JOIN norm_ref_by_coin norm"
+ " ON (norm.coin_pub = cdep.coin_pub"
+ " AND norm.batch_deposit_serial_id = cdep.batch_deposit_serial_id))"
" ,fees AS (" /* find deposit fees for not fully refunded deposits */
+ /* The fee is capped at what is left of the deposit after refunds:
+ nothing bounds a partial refund to (deposit - deposit fee), and
+ charging the full fee on top of a larger refund would make the
+ amount to be wired out negative. */
" SELECT"
- " denom.fee_deposit AS fee"
- " ,cs.batch_deposit_serial_id" /* ensures we get the fee for each coin, not once per denomination */
- " FROM cdep cs"
+ " CASE WHEN ( ((denom.fee_deposit).val,(denom.fee_deposit).frac)"
+ " <= (rem.rem_val,rem.rem_frac) )"
+ " THEN (denom.fee_deposit).val"
+ " ELSE rem.rem_val END AS fee_val"
+ " ,CASE WHEN ( ((denom.fee_deposit).val,(denom.fee_deposit).frac)"
+ " <= (rem.rem_val,rem.rem_frac) )"
+ " THEN (denom.fee_deposit).frac"
+ " ELSE rem.rem_frac END AS fee_frac"
+ " ,rem.batch_deposit_serial_id" /* ensures we get the fee for each coin, not once per denomination */
+ " FROM remainders rem"
" 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)"
+ " ON (kc.coin_pub = rem.coin_pub)"
" JOIN denominations denom"
" USING (denominations_serial)"
- " WHERE coin_pub NOT IN (SELECT coin_pub FROM fully_refunded_coins))"
+ " WHERE rem.coin_pub NOT IN (SELECT coin_pub FROM fully_refunded_coins))"
" ,dummy AS (" /* add deposits to aggregation_tracking */
" INSERT INTO aggregation_tracking"
" (batch_deposit_serial_id"
@@ -123,8 +152,8 @@ TALER_EXCHANGEDB_do_aggregate (
" ,COALESCE(SUM((cdep.amount).frac),0) AS sum_deposit_fraction" /* SUM over INT returns INT8 */
" ,CAST(COALESCE(SUM((ref.refund).val),0) AS INT8) AS sum_refund_value"
" ,COALESCE(SUM((ref.refund).frac),0) AS sum_refund_fraction"
- " ,CAST(COALESCE(SUM((fees.fee).val),0) AS INT8) AS sum_fee_value"
- " ,COALESCE(SUM((fees.fee).frac),0) AS sum_fee_fraction"
+ " ,CAST(COALESCE(SUM(fees.fee_val),0) AS INT8) AS sum_fee_value"
+ " ,CAST(COALESCE(SUM(fees.fee_frac),0) AS INT8) AS sum_fee_fraction"
" FROM cdep "
" FULL OUTER JOIN ref ON (FALSE)" /* We just want all sums */
" FULL OUTER JOIN fees ON (FALSE);");
@@ -189,13 +218,33 @@ TALER_EXCHANGEDB_do_aggregate (
sum_fee.value = sum_fee_frac / TALER_AMOUNT_FRAC_BASE
+ sum_fee_value;
sum_fee.fraction = sum_fee_frac % TALER_AMOUNT_FRAC_BASE; \
- GNUNET_assert (0 <=
- TALER_amount_subtract (&delta,
- &sum_deposit,
- &sum_refund));
- GNUNET_assert (0 <=
- TALER_amount_subtract (total,
- &delta,
- &sum_fee));
+ /* With the fee capped at the un-refunded remainder above, neither
+ subtraction can go negative. Should the invariant ever be violated
+ again, refuse the aggregation (the caller rolls the transaction back and
+ reports the failure) rather than abort() the daemon: an abort here stops
+ *all* payouts for the shard and, since the transaction is rolled back,
+ the very same batch is picked up and aborts again on restart. */
+ if (0 >
+ TALER_amount_subtract (&delta,
+ &sum_deposit,
+ &sum_refund))
+ {
+ GNUNET_break (0);
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Refunds (%s) exceed deposits in aggregation\n",
+ TALER_amount2s (&sum_refund));
+ return GNUNET_DB_STATUS_HARD_ERROR;
+ }
+ if (0 >
+ TALER_amount_subtract (total,
+ &delta,
+ &sum_fee))
+ {
+ GNUNET_break (0);
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Deposit fees (%s) exceed what is left of the deposits after refunds in aggregation\n",
+ TALER_amount2s (&sum_fee));
+ return GNUNET_DB_STATUS_HARD_ERROR;
+ }
return qs;
}
diff --git a/src/exchangedb/meson.build b/src/exchangedb/meson.build
@@ -293,6 +293,43 @@ pkg.generate(
)
+test_regressions = executable(
+ 'test_regressions',
+ ['test_regressions.c'],
+ install_rpath: rpath_option,
+ dependencies: [
+ libtalerexchangedb_dep,
+ libtalerutil_dep,
+ libtalerjson_dep,
+ libtalerpq_dep,
+ gnunetutil_dep,
+ gnunetjson_dep,
+ gnunetpq_dep,
+ pq_dep,
+ json_dep,
+ ],
+ include_directories: [incdir, configuration_inc],
+ install: false,
+)
+
+test_regressions_sh = configure_file(
+ input: 'test_regressions.sh',
+ output: 'test_regressions.sh',
+ copy: true,
+)
+
+test(
+ 'test_regressions',
+ test_regressions_sh,
+ workdir: meson.current_build_dir(),
+ suite: ['exchangedb'],
+ depends: [test_regressions, exchangedb_sql_targets],
+ env: {'TALER_BUILD_ROOT': meson.project_build_root()},
+ is_parallel: false,
+ timeout: 300,
+)
+
+
# [oec 20250430] disable test for now
# check_PROGRAMS = \
# test-exchangedb
diff --git a/src/exchangedb/sql-schema/meson.build b/src/exchangedb/sql-schema/meson.build
@@ -189,8 +189,9 @@ generated_sql = [
['tops-0001.sql', ['tops-0001.sql']],
]
+exchangedb_sql_targets = []
foreach g : generated_sql
- custom_target(
+ exchangedb_sql_targets += custom_target(
'gen-exchangedb-' + g[0],
input: g[1],
output: g[0],
diff --git a/src/exchangedb/test_regressions.c b/src/exchangedb/test_regressions.c
@@ -0,0 +1,409 @@
+/*
+ This file is part of TALER
+ Copyright (C) 2026 Taler Systems SA
+
+ TALER is free software; you can redistribute it and/or modify it under the
+ terms of the GNU General Public License as published by the Free Software
+ Foundation; either version 3, or (at your option) any later version.
+
+ TALER is distributed in the hope that it will be useful, but WITHOUT ANY
+ WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+ A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along with
+ TALER; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
+*/
+/**
+ * @file exchangedb/test_regressions.c
+ * @brief regression tests for individual exchangedb operations
+ *
+ * Each check in the #tests table pins down one previously broken behaviour.
+ * The tests share one scratch database, which test_regressions.sh creates and
+ * removes; every check must therefore use fresh keys rather than assume an
+ * empty database.
+ */
+#include "exchangedb_lib.h"
+#include "taler/taler_json_lib.h"
+#include "helper.h"
+#include "exchange-database/create_tables.h"
+#include "exchange-database/do_aggregate.h"
+#include "exchange-database/compute_shard.h"
+
+
+/**
+ * Currency we use, must match test_regressions.sh.
+ */
+#define CURRENCY "EUR"
+
+/**
+ * Report a failed expectation and return 1 from the calling check.
+ */
+#define FAILIF(cond) \
+ do { \
+ if (! (cond)) break; \
+ GNUNET_break (0); \
+ fprintf (stderr, \
+ "FAILED: %s at %s:%u\n", \
+ # cond, __FILE__, __LINE__); \
+ return 1; \
+ } while (0)
+
+
+/**
+ * Our database context.
+ */
+static struct TALER_EXCHANGEDB_PostgresContext *pg;
+
+/**
+ * Name of the single check to run, NULL to run all of them.
+ */
+static char *only;
+
+/**
+ * Return value of the process.
+ */
+static int result;
+
+
+/**
+ * Run @a sql on our database connection, outside of any transaction.
+ *
+ * @param sql statement(s) to run
+ * @return #GNUNET_OK on success
+ */
+static enum GNUNET_GenericReturnValue
+exec_sql (const char *sql)
+{
+ struct GNUNET_PQ_ExecuteStatement es[] = {
+ GNUNET_PQ_make_execute (sql),
+ GNUNET_PQ_EXECUTE_STATEMENT_END
+ };
+
+ return GNUNET_PQ_exec_statements (pg->conn,
+ es);
+}
+
+
+/**
+ * Convert @a val to an amount in our currency.
+ *
+ * @param str amount without the currency prefix, e.g. "10.5"
+ * @param[out] amount set to the parsed amount
+ */
+static void
+parse_amount (const char *str,
+ struct TALER_Amount *amount)
+{
+ char *s;
+
+ GNUNET_asprintf (&s,
+ CURRENCY ":%s",
+ str);
+ GNUNET_assert (GNUNET_OK ==
+ TALER_string_to_amount (s,
+ amount));
+ GNUNET_free (s);
+}
+
+
+/**
+ * Build an INSERT that creates a reserve with a zero balance and no
+ * `reserves_in` row, the way exchange_do_purse_merge() does.
+ *
+ * @param reserve_pub public key of the reserve to create
+ * @return SQL statement, to be freed by the caller
+ */
+static char *
+hex_insert_reserve (const struct TALER_ReservePublicKeyP *reserve_pub)
+{
+ char hex[sizeof (*reserve_pub) * 2 + 1];
+ const unsigned char *raw = (const unsigned char *) reserve_pub;
+ char *sql;
+
+ for (unsigned int i = 0; i<sizeof (*reserve_pub); i++)
+ GNUNET_snprintf (&hex[i * 2],
+ 3,
+ "%02x",
+ raw[i]);
+ GNUNET_asprintf (&sql,
+ "INSERT INTO reserves"
+ " (reserve_pub,current_balance,expiration_date,gc_date)"
+ " VALUES"
+ " (decode('%s','hex')"
+ " ,ROW(0,0)::taler_amount"
+ " ,1770000000000000"
+ " ,1780000000000000);",
+ hex);
+ return sql;
+}
+
+
+/**
+ * Render @a data as a lowercase hex string for use in an SQL literal.
+ *
+ * @param data binary data
+ * @param size number of bytes in @a data
+ * @return hex string, to be freed by the caller
+ */
+static char *
+to_hex (const void *data,
+ size_t size)
+{
+ const unsigned char *raw = data;
+ char *hex;
+
+ hex = GNUNET_malloc (size * 2 + 1);
+ for (size_t i = 0; i<size; i++)
+ GNUNET_snprintf (&hex[i * 2],
+ 3,
+ "%02x",
+ raw[i]);
+ return hex;
+}
+
+
+/**
+ * E-2: a merchant may refund more than (deposit - deposit fee). The
+ * aggregator used to charge the full deposit fee on top of such a refund,
+ * making the amount to be wired out negative, and then abort() on the
+ * GNUNET_assert() guarding the subtraction -- stopping every payout for the
+ * shard, and doing so again on every restart.
+ *
+ * Deposit EUR:1.00 of a denomination with a EUR:0.10 deposit fee, refund
+ * EUR:0.95 of it, and require the aggregation to come out at EUR:0.
+ */
+static int
+check_aggregate_refund_below_deposit_fee (void)
+{
+ struct TALER_MerchantPublicKeyP merchant_pub;
+ struct TALER_FullPaytoHashP h_payto;
+ struct TALER_NormalizedPaytoHashP h_norm;
+ struct TALER_CoinSpendPublicKeyP coin_pub;
+ struct TALER_WireTransferIdentifierRawP wtid;
+ struct TALER_Amount total;
+ struct TALER_Amount zero;
+ char *sql;
+ char *m_hex;
+ char *p_hex;
+ char *n_hex;
+ char *c_hex;
+ enum GNUNET_GenericReturnValue ok;
+
+ parse_amount ("0",
+ &zero);
+ memset (&merchant_pub,
+ 0x41,
+ sizeof (merchant_pub));
+ memset (&h_payto,
+ 0x42,
+ sizeof (h_payto));
+ memset (&h_norm,
+ 0x43,
+ sizeof (h_norm));
+ memset (&coin_pub,
+ 0x44,
+ sizeof (coin_pub));
+ memset (&wtid,
+ 0x45,
+ sizeof (wtid));
+ m_hex = to_hex (&merchant_pub,
+ sizeof (merchant_pub));
+ p_hex = to_hex (&h_payto,
+ sizeof (h_payto));
+ n_hex = to_hex (&h_norm,
+ sizeof (h_norm));
+ c_hex = to_hex (&coin_pub,
+ sizeof (coin_pub));
+ GNUNET_asprintf (
+ &sql,
+ "INSERT INTO kyc_targets (h_normalized_payto)"
+ " VALUES (decode('%s','hex')) ON CONFLICT DO NOTHING;"
+ "INSERT INTO wire_targets"
+ " (wire_target_h_payto,payto_uri,h_normalized_payto)"
+ " VALUES (decode('%s','hex'),'payto://x-taler-bank/h/e2',"
+ " decode('%s','hex')) ON CONFLICT DO NOTHING;"
+ "INSERT INTO denominations"
+ " (denom_pub_hash,denom_type,age_mask,denom_pub,master_sig"
+ " ,valid_from,expire_withdraw,expire_deposit,expire_legal"
+ " ,coin,fee_withdraw,fee_deposit,fee_refresh,fee_refund)"
+ " VALUES (decode(repeat('e2',64),'hex'),1,0,decode('00','hex')"
+ " ,decode(repeat('00',64),'hex'),0,0,0,0"
+ " ,ROW(1,0)::taler_amount,ROW(0,0)::taler_amount"
+ " ,ROW(0,10000000)::taler_amount,ROW(0,0)::taler_amount"
+ " ,ROW(0,0)::taler_amount);"
+ "INSERT INTO known_coins"
+ " (denominations_serial,coin_pub,denom_sig,remaining)"
+ " VALUES ((SELECT denominations_serial FROM denominations"
+ " WHERE denom_pub_hash=decode(repeat('e2',64),'hex'))"
+ " ,decode('%s','hex'),decode('00','hex'),ROW(0,0)::taler_amount);"
+ "INSERT INTO batch_deposits"
+ " (shard,merchant_pub,wallet_timestamp,exchange_timestamp"
+ " ,refund_deadline,wire_deadline,h_contract_terms,wire_salt"
+ " ,wire_target_h_payto,policy_blocked,total_amount,merchant_sig"
+ " ,done,total_without_fee)"
+ " VALUES (%llu,decode('%s','hex'),0,0,1,1"
+ " ,decode(repeat('e2',64),'hex'),decode(repeat('e2',16),'hex')"
+ " ,decode('%s','hex'),FALSE,ROW(1,0)::taler_amount"
+ " ,decode(repeat('00',64),'hex'),FALSE"
+ " ,ROW(0,90000000)::taler_amount);"
+ "INSERT INTO coin_deposits"
+ " (batch_deposit_serial_id,coin_pub,coin_sig,amount_with_fee)"
+ " VALUES ((SELECT batch_deposit_serial_id FROM batch_deposits"
+ " WHERE merchant_pub=decode('%s','hex'))"
+ " ,decode('%s','hex'),decode(repeat('e2',64),'hex')"
+ " ,ROW(1,0)::taler_amount);"
+ "INSERT INTO refunds"
+ " (coin_pub,batch_deposit_serial_id,merchant_sig,rtransaction_id"
+ " ,amount_with_fee)"
+ " VALUES (decode('%s','hex')"
+ " ,(SELECT batch_deposit_serial_id FROM batch_deposits"
+ " WHERE merchant_pub=decode('%s','hex'))"
+ " ,decode(repeat('00',64),'hex'),1"
+ " ,ROW(0,95000000)::taler_amount);",
+ n_hex,
+ p_hex,
+ n_hex,
+ c_hex,
+ (unsigned long long) TALER_EXCHANGEDB_compute_shard (&merchant_pub),
+ m_hex,
+ p_hex,
+ m_hex,
+ c_hex,
+ c_hex,
+ m_hex);
+ ok = exec_sql (sql);
+ GNUNET_free (sql);
+ GNUNET_free (m_hex);
+ GNUNET_free (p_hex);
+ GNUNET_free (n_hex);
+ GNUNET_free (c_hex);
+ FAILIF (GNUNET_OK != ok);
+
+ /* Before the fix this abort()ed inside TALER_EXCHANGEDB_do_aggregate(). */
+ FAILIF (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
+ TALER_EXCHANGEDB_do_aggregate (pg,
+ &h_payto,
+ &merchant_pub,
+ &wtid,
+ &total));
+ FAILIF (0 !=
+ TALER_amount_cmp (&zero,
+ &total));
+ return 0;
+}
+
+
+/**
+ * All checks we know about.
+ */
+static const struct
+{
+ const char *name;
+ int (*fn)(void);
+} tests[] = {
+ { "aggregate-refund-below-deposit-fee",
+ &check_aggregate_refund_below_deposit_fee },
+ { NULL, NULL }
+};
+
+
+/**
+ * Main function that runs the checks.
+ *
+ * @param cls closure
+ * @param args remaining command-line arguments
+ * @param cfgfile name of the configuration file used
+ * @param cfg configuration
+ */
+static void
+run (void *cls,
+ char *const *args,
+ const char *cfgfile,
+ const struct GNUNET_CONFIGURATION_Handle *cfg)
+{
+ unsigned int ran = 0;
+
+ (void) cls;
+ (void) args;
+ (void) cfgfile;
+ pg = TALER_EXCHANGEDB_connect_admin (cfg);
+ if (NULL == pg)
+ {
+ fprintf (stderr,
+ "Failed to connect to the database\n");
+ result = 77;
+ return;
+ }
+ if (GNUNET_OK !=
+ TALER_EXCHANGEDB_create_tables (pg,
+ false,
+ 0))
+ {
+ fprintf (stderr,
+ "Failed to create the database schema\n");
+ result = 77;
+ goto cleanup;
+ }
+ for (unsigned int i = 0; NULL != tests[i].name; i++)
+ {
+ if ( (NULL != only) &&
+ (0 != strcmp (only,
+ tests[i].name)) )
+ continue;
+ fprintf (stderr,
+ "Running check `%s'\n",
+ tests[i].name);
+ ran++;
+ if (0 != tests[i].fn ())
+ {
+ fprintf (stderr,
+ "Check `%s' FAILED\n",
+ tests[i].name);
+ result = 1;
+ }
+ }
+ if (0 == ran)
+ {
+ fprintf (stderr,
+ "No check matched `%s'\n",
+ only);
+ result = 1;
+ }
+cleanup:
+ TALER_EXCHANGEDB_disconnect (pg);
+ pg = NULL;
+}
+
+
+int
+main (int argc,
+ char *const *argv)
+{
+ struct GNUNET_GETOPT_CommandLineOption options[] = {
+ GNUNET_GETOPT_option_string ('t',
+ "test",
+ "NAME",
+ "only run the check called NAME",
+ &only),
+ GNUNET_GETOPT_OPTION_END
+ };
+ enum GNUNET_GenericReturnValue ret;
+
+ result = 0;
+ ret = GNUNET_PROGRAM_run (TALER_EXCHANGE_project_data (),
+ argc,
+ argv,
+ "test-regressions",
+ "Regression tests for the exchange database layer",
+ options,
+ &run,
+ NULL);
+ if (GNUNET_SYSERR == ret)
+ return 3;
+ if (GNUNET_NO == ret)
+ return 0;
+ return result;
+}
+
+
+/* end of test_regressions.c */
diff --git a/src/exchangedb/test_regressions.sh b/src/exchangedb/test_regressions.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+# This file is in the public domain.
+#
+# Driver for the src/exchangedb/ regression tests. It provisions a scratch
+# database of its own, runs ./test_regressions against it and removes the
+# database again. No pre-existing database is ever touched: the name is
+# derived from the PID and is deliberately *not* taken from the environment.
+set -eu
+
+# Where the build tree lives; meson passes this, the fallback is for
+# running the script by hand from its build directory.
+BUILD_ROOT="${TALER_BUILD_ROOT:-$(cd ../../.. && pwd)}"
+SQL_DIR="${TALER_SQL_DIR:-${BUILD_ROOT}/src/exchangedb/sql-schema}"
+
+# Skip (77) rather than fail if there is no usable PostgreSQL around.
+command -v createdb > /dev/null 2>&1 || exit 77
+command -v dropdb > /dev/null 2>&1 || exit 77
+psql -l < /dev/null > /dev/null 2>&1 || exit 77
+
+DBNAME="taler_exdb_reg_$$"
+CONF="test_regressions_$$.conf"
+
+# The freshly built libraries must outrank any installed copy, as
+# LD_LIBRARY_PATH beats the build tree's RUNPATH.
+for d in "${BUILD_ROOT}"/src/*/; do
+ LD_LIBRARY_PATH="${d%/}${LD_LIBRARY_PATH:+:}${LD_LIBRARY_PATH:-}"
+done
+export LD_LIBRARY_PATH
+
+cleanup ()
+{
+ dropdb --if-exists "$DBNAME" > /dev/null 2>&1 || true
+ rm -f "$CONF"
+}
+trap cleanup EXIT
+
+cat > "$CONF" <<EOF
+[exchange]
+CURRENCY = EUR
+BASE_URL = http://localhost/
+[exchangedb-postgres]
+CONFIG = postgres:///${DBNAME}
+SQL_DIR = ${SQL_DIR}/
+[exchangedb]
+MAX_AML_PROGRAM_RUNTIME = 1 minute
+IDLE_RESERVE_EXPIRATION_TIME = 4 weeks
+LEGAL_RESERVE_EXPIRATION_TIME = 7 years
+AGGREGATOR_SHIFT = 1s
+DEFAULT_PURSE_LIMIT = 1
+EOF
+
+createdb "$DBNAME" > /dev/null 2>&1 || exit 77
+
+RET=0
+./test_regressions -c "$CONF" "$@" || RET=$?
+exit $RET
