commit 251fee0ec8f187b8bf0dc6e5fc10363c3fbb10a4
parent 4f6b3258eb3ebca62080507c1af70c4d41725d1e
Author: Christian Grothoff <christian@grothoff.org>
Date: Wed, 5 Aug 2026 22:24:01 +0200
fix error handling if we fail to persist a deposit confirmation
Diffstat:
6 files changed, 246 insertions(+), 27 deletions(-)
diff --git a/src/backend/taler-merchant-httpd_post-orders-ORDER_ID-pay.c b/src/backend/taler-merchant-httpd_post-orders-ORDER_ID-pay.c
@@ -1060,6 +1060,7 @@ batch_deposit_transaction (
{
const struct PayContext *pc = eg->pc;
enum GNUNET_DB_QueryStatus qs;
+ enum TALER_MERCHANTDB_DepositConfirmationStatus dcs;
uint64_t b_dep_serial;
uint32_t off = 0;
@@ -1068,7 +1069,7 @@ batch_deposit_transaction (
pc->hc->instance->settings.id);
if (qs <= 0)
return qs; /* failure, we're done */
- qs = TALER_MERCHANTDB_insert_deposit_confirmation (
+ dcs = TALER_MERCHANTDB_insert_deposit_confirmation (
TMH_db,
pc->hc->instance->settings.id,
dr->details.ok.deposit_timestamp,
@@ -1081,8 +1082,36 @@ batch_deposit_transaction (
dr->details.ok.exchange_sig,
dr->details.ok.exchange_pub,
&b_dep_serial);
- if (qs <= 0)
- goto cleanup; /* Entire batch already known or failure, we're done */
+ switch (dcs)
+ {
+ case TALER_MERCHANTDB_DCS_SUCCESS:
+ break;
+ case TALER_MERCHANTDB_DCS_SOFT_ERROR:
+ qs = GNUNET_DB_STATUS_SOFT_ERROR;
+ goto cleanup;
+ case TALER_MERCHANTDB_DCS_CONFLICT:
+ case TALER_MERCHANTDB_DCS_NO_SIGNKEY:
+ case TALER_MERCHANTDB_DCS_NO_ACCOUNT:
+ case TALER_MERCHANTDB_DCS_NO_ORDER:
+ case TALER_MERCHANTDB_DCS_HARD_ERROR:
+ case TALER_MERCHANTDB_DCS_NO_RESULTS:
+ /* We must NOT commit here: the coins were deposited at the
+ exchange, but we failed to persist the deposit confirmation.
+ Committing would leave us with a paid order and no deposit
+ records at all, which breaks our accounting. Note that this
+ is still a VERY bad case: the customer lost their payment,
+ and the exchange will pay *somebody*. It really should not
+ happen as we should not have accepted an unknown order or
+ an account we do not know, etc.; still, best outcome is for
+ the wallet to be forced to replay and then hopefully next
+ time we succeed... */
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Failed to store deposit confirmation for order `%s' (status %d), failing payment\n",
+ pc->hc->infix,
+ (int) dcs);
+ qs = GNUNET_DB_STATUS_HARD_ERROR;
+ goto cleanup;
+ }
for (size_t i = 0; i<pc->parse_pay.coins_cnt; i++)
{
diff --git a/src/backenddb/insert_deposit_confirmation.c b/src/backenddb/insert_deposit_confirmation.c
@@ -24,7 +24,7 @@
#include "helper.h"
-enum GNUNET_DB_QueryStatus
+enum TALER_MERCHANTDB_DepositConfirmationStatus
TALER_MERCHANTDB_insert_deposit_confirmation (
struct TALER_MERCHANTDB_PostgresContext *pg,
const char *instance_id,
@@ -105,31 +105,52 @@ TALER_MERCHANTDB_insert_deposit_confirmation (
params,
rs);
GNUNET_free (nbo_str);
- GNUNET_break (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS != qs);
- if (qs < 0)
- return qs;
- // FIXME: in the future, return these codes to the client and
- // return more specific error codes to the client from the API!
- if (no_order)
+ switch (qs)
{
+ case GNUNET_DB_STATUS_HARD_ERROR:
+ return TALER_MERCHANTDB_DCS_HARD_ERROR;
+ case GNUNET_DB_STATUS_SOFT_ERROR:
+ return TALER_MERCHANTDB_DCS_SOFT_ERROR;
+ case GNUNET_DB_STATUS_SUCCESS_NO_RESULTS:
+ /* stored procedure always returns exactly one row */
GNUNET_break (0);
- return GNUNET_DB_STATUS_SUCCESS_NO_RESULTS;
+ return TALER_MERCHANTDB_DCS_NO_RESULTS;
+ case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT:
+ break;
}
+ /* Checked in the same order as the stored procedure evaluates them. */
if (no_account)
{
- GNUNET_break (0);
- return GNUNET_DB_STATUS_SUCCESS_NO_RESULTS;
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Merchant account `%s' unknown, cannot store deposit confirmation for contract `%s'\n",
+ TALER_B2S (h_wire),
+ GNUNET_h2s (&h_contract_terms->hash));
+ return TALER_MERCHANTDB_DCS_NO_ACCOUNT;
}
if (no_signkey)
{
- GNUNET_break (0);
- return GNUNET_DB_STATUS_SUCCESS_NO_RESULTS;
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Exchange signing key `%s' of `%s' unknown, cannot store deposit confirmation for contract `%s'\n",
+ TALER_B2S (exchange_pub),
+ exchange_url,
+ GNUNET_h2s (&h_contract_terms->hash));
+ return TALER_MERCHANTDB_DCS_NO_SIGNKEY;
+ }
+ if (no_order)
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Order for contract `%s' unknown, cannot store deposit confirmation\n",
+ GNUNET_h2s (&h_contract_terms->hash));
+ return TALER_MERCHANTDB_DCS_NO_ORDER;
}
if (conflict)
{
- GNUNET_break (0);
- return GNUNET_DB_STATUS_SUCCESS_NO_RESULTS;
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Conflicting deposit confirmation already stored for contract `%s' at `%s'\n",
+ GNUNET_h2s (&h_contract_terms->hash),
+ exchange_url);
+ return TALER_MERCHANTDB_DCS_CONFLICT;
}
GNUNET_assert (0 != *deposit_confirmation_serial_id);
- return qs;
+ return TALER_MERCHANTDB_DCS_SUCCESS;
}
diff --git a/src/backenddb/insert_deposit_confirmation.sql b/src/backenddb/insert_deposit_confirmation.sql
@@ -42,9 +42,11 @@ DECLARE
my_credit_amount merchant.taler_amount_currency;
BEGIN
-out_no_order=TRUE;
-out_no_account=TRUE;
-out_no_signkey=TRUE;
+-- Note: exactly one of the out_no_* flags is set on the failure
+-- paths below, the caller distinguishes the causes by them.
+out_no_order=FALSE;
+out_no_account=FALSE;
+out_no_signkey=FALSE;
out_conflict=FALSE;
out_deposit_confirmation_serial=0;
@@ -54,9 +56,9 @@ SELECT account_serial
WHERE h_wire=in_h_wire;
IF NOT FOUND
THEN
+ out_no_account=TRUE;
RETURN;
END IF;
-out_no_account=FALSE;
SELECT signkey_serial
INTO my_signkey_serial
@@ -66,9 +68,9 @@ SELECT signkey_serial
LIMIT 1;
IF NOT FOUND
THEN
+ out_no_signkey=TRUE;
RETURN;
END IF;
-out_no_signkey=FALSE;
SELECT order_serial
INTO my_order_serial
@@ -76,9 +78,9 @@ SELECT order_serial
WHERE h_contract_terms=in_h_contract_terms;
IF NOT FOUND
THEN
+ out_no_order=TRUE;
RETURN;
END IF;
-out_no_order=FALSE;
SELECT deposit_confirmation_serial
,deposit_timestamp
diff --git a/src/backenddb/test_merchantdb.c b/src/backenddb/test_merchantdb.c
@@ -2957,7 +2957,7 @@ test_insert_deposit (const struct InstanceData *instance,
&deposit->deposit_fee));
TEST_SET_INSTANCE (instance->instance.id, expected_result);
TEST_COND_RET_ON_FAIL (
- GNUNET_DB_STATUS_SUCCESS_ONE_RESULT ==
+ TALER_MERCHANTDB_DCS_SUCCESS ==
TALER_MERCHANTDB_insert_deposit_confirmation (pg,
instance->instance.id,
deposit->timestamp,
@@ -2988,6 +2988,60 @@ test_insert_deposit (const struct InstanceData *instance,
/**
+ * Tests that storing a deposit confirmation returns a status that
+ * identifies the *specific* reason why it could not be stored.
+ * Collapsing all of them into #GNUNET_DB_STATUS_SUCCESS_NO_RESULTS
+ * (as we used to) makes the pay handler commit a batch deposit
+ * without any deposit records, silently losing the money.
+ *
+ * @param instance the instance the deposit was made to.
+ * @param deposit the deposit to derive the (valid) defaults from.
+ * @param h_contract_terms contract to claim the deposit is for.
+ * @param h_wire target account to claim the deposit went to.
+ * @param exchange_pub exchange signing key to use.
+ * @param wire_fee wire fee to claim was charged.
+ * @param expected_status status the database should return.
+ * @return 0 on success, 1 otherwise.
+ */
+static int
+test_insert_deposit_confirmation_status (
+ const struct InstanceData *instance,
+ const struct DepositData *deposit,
+ const struct TALER_PrivateContractHashP *h_contract_terms,
+ const struct TALER_MerchantWireHashP *h_wire,
+ const struct TALER_ExchangePublicKeyP *exchange_pub,
+ const struct TALER_Amount *wire_fee,
+ enum TALER_MERCHANTDB_DepositConfirmationStatus expected_status)
+{
+ uint64_t row = 0;
+ struct TALER_Amount awf;
+
+ GNUNET_assert (0 <=
+ TALER_amount_subtract (&awf,
+ &deposit->amount_with_fee,
+ &deposit->deposit_fee));
+ TEST_SET_INSTANCE (instance->instance.id,
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT);
+ TEST_COND_RET_ON_FAIL (
+ expected_status ==
+ TALER_MERCHANTDB_insert_deposit_confirmation (pg,
+ instance->instance.id,
+ deposit->timestamp,
+ h_contract_terms,
+ deposit->exchange_url,
+ deposit->timestamp,
+ &awf,
+ wire_fee,
+ h_wire,
+ &deposit->exchange_sig,
+ exchange_pub,
+ &row),
+ "Insert deposit confirmation returned the wrong status\n");
+ return 0;
+}
+
+
+/**
* Closure for testing deposit lookup
*/
struct TestLookupDeposits_Closure
@@ -3580,6 +3634,60 @@ run_test_deposits (struct TestDeposits_Closure *cls)
&cls->signkey,
&cls->deposits[0],
GNUNET_DB_STATUS_SUCCESS_NO_RESULTS));
+ /* Each reason for not storing a deposit confirmation must be
+ reported distinctly, and none of them may look like success. */
+ {
+ struct TALER_PrivateContractHashP bogus_h_contract;
+ struct TALER_MerchantWireHashP bogus_h_wire;
+ struct TALER_ExchangePublicKeyP bogus_exchange_pub;
+ struct TALER_Amount other_wire_fee;
+
+ GNUNET_CRYPTO_random_block (&bogus_h_contract,
+ sizeof (bogus_h_contract));
+ GNUNET_CRYPTO_random_block (&bogus_h_wire,
+ sizeof (bogus_h_wire));
+ GNUNET_CRYPTO_random_block (&bogus_exchange_pub,
+ sizeof (bogus_exchange_pub));
+ GNUNET_assert (GNUNET_OK ==
+ TALER_string_to_amount ("EUR:3.00",
+ &other_wire_fee));
+ /* unknown merchant account */
+ TEST_RET_ON_FAIL (test_insert_deposit_confirmation_status (
+ &cls->instance,
+ &cls->deposits[0],
+ &cls->deposits[0].h_contract_terms,
+ &bogus_h_wire,
+ &cls->signkey.exchange_pub,
+ &cls->deposits[0].wire_fee,
+ TALER_MERCHANTDB_DCS_NO_ACCOUNT));
+ /* unknown exchange signing key */
+ TEST_RET_ON_FAIL (test_insert_deposit_confirmation_status (
+ &cls->instance,
+ &cls->deposits[0],
+ &cls->deposits[0].h_contract_terms,
+ &cls->deposits[0].h_wire,
+ &bogus_exchange_pub,
+ &cls->deposits[0].wire_fee,
+ TALER_MERCHANTDB_DCS_NO_SIGNKEY));
+ /* unknown order */
+ TEST_RET_ON_FAIL (test_insert_deposit_confirmation_status (
+ &cls->instance,
+ &cls->deposits[0],
+ &bogus_h_contract,
+ &cls->deposits[0].h_wire,
+ &cls->signkey.exchange_pub,
+ &cls->deposits[0].wire_fee,
+ TALER_MERCHANTDB_DCS_NO_ORDER));
+ /* conflicting deposit confirmation (different wire fee) */
+ TEST_RET_ON_FAIL (test_insert_deposit_confirmation_status (
+ &cls->instance,
+ &cls->deposits[0],
+ &cls->deposits[0].h_contract_terms,
+ &cls->deposits[0].h_wire,
+ &cls->signkey.exchange_pub,
+ &other_wire_fee,
+ TALER_MERCHANTDB_DCS_CONFLICT));
+ }
/* Test lookup deposits */
TEST_RET_ON_FAIL (test_lookup_deposits (&cls->instance,
&cls->deposits[0].h_contract_terms,
diff --git a/src/include/merchant-database/insert_deposit_confirmation.h b/src/include/merchant-database/insert_deposit_confirmation.h
@@ -42,9 +42,12 @@ struct TALER_MERCHANTDB_PostgresContext;
* @param exchange_sig signature from exchange that coin was accepted
* @param exchange_pub signing key that was used for @a exchange_sig
* @param[out] deposit_confirmation_serial_id set to the table row
- * @return transaction status
+ * (only on #TALER_MERCHANTDB_DCS_SUCCESS)
+ * @return detailed status of the operation; note that all
+ * non-#TALER_MERCHANTDB_DCS_SUCCESS values are failures,
+ * the caller MUST NOT commit the transaction in those cases
*/
-enum GNUNET_DB_QueryStatus
+enum TALER_MERCHANTDB_DepositConfirmationStatus
TALER_MERCHANTDB_insert_deposit_confirmation (struct TALER_MERCHANTDB_PostgresContext *pg,
const char *instance_id,
struct GNUNET_TIME_Timestamp deposit_timestamp,
diff --git a/src/include/merchantdb_lib.h b/src/include/merchantdb_lib.h
@@ -177,6 +177,62 @@ enum TALER_MERCHANTDB_RefundStatus
};
/**
+ * Results from trying to store a deposit confirmation.
+ * Values that also exist in `enum GNUNET_DB_QueryStatus`
+ * intentionally use the same numeric value.
+ */
+enum TALER_MERCHANTDB_DepositConfirmationStatus
+{
+
+ /**
+ * A deposit confirmation for this order and exchange
+ * exists, but with conflicting details (timestamp, wire
+ * deadline, wire fee or target account).
+ */
+ TALER_MERCHANTDB_DCS_CONFLICT = -6,
+
+ /**
+ * The exchange signing key that signed the deposit
+ * confirmation is not known to us.
+ */
+ TALER_MERCHANTDB_DCS_NO_SIGNKEY = -5,
+
+ /**
+ * The merchant account the deposit was made to is
+ * not known to us.
+ */
+ TALER_MERCHANTDB_DCS_NO_ACCOUNT = -4,
+
+ /**
+ * The order (contract) the deposit is for is not
+ * known to us.
+ */
+ TALER_MERCHANTDB_DCS_NO_ORDER = -3,
+
+ /**
+ * Hard database failure.
+ */
+ TALER_MERCHANTDB_DCS_HARD_ERROR = -2,
+
+ /**
+ * Soft database failure, retry.
+ */
+ TALER_MERCHANTDB_DCS_SOFT_ERROR = -1,
+
+ /**
+ * The stored procedure did not return a row at all.
+ * Should be impossible.
+ */
+ TALER_MERCHANTDB_DCS_NO_RESULTS = 0,
+
+ /**
+ * Deposit confirmation is now in the database.
+ */
+ TALER_MERCHANTDB_DCS_SUCCESS = 1
+
+};
+
+/**
* Details about an OTP device.
*/
struct TALER_MERCHANTDB_OtpDeviceDetails
