commit 925507d139091fec722820faf09971ab4f76e014
parent e85767337f0deb7188190ea950a47104a3d2fe87
Author: Christian Grothoff <christian@grothoff.org>
Date: Wed, 5 Aug 2026 23:15:02 +0200
report duplicate rtransaction_ids as serialization failure so that caller retries
Diffstat:
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/src/backenddb/do_increase_refund.c b/src/backenddb/do_increase_refund.c
@@ -353,7 +353,11 @@ prep_insert_refund (struct TALER_MERCHANTDB_PostgresContext *pg)
",reason"
",refund_amount"
") VALUES"
- "($1, $2, $3, $4, $5, $6)");
+ "($1, $2, $3, $4, $5, $6)"
+ " ON CONFLICT (order_serial"
+ ",coin_pub"
+ ",rtransaction_id)"
+ " DO NOTHING");
return GNUNET_DB_STATUS_SUCCESS_NO_RESULTS;
}
@@ -628,6 +632,17 @@ process_deposits_for_refund_cb (void *cls,
case GNUNET_DB_STATUS_SOFT_ERROR:
ctx->rs = TALER_MERCHANTDB_RS_SOFT_ERROR;
goto cleanup;
+ case GNUNET_DB_STATUS_SUCCESS_NO_RESULTS:
+ /* Another transaction concurrently created a refund with the
+ very rtransaction_id we picked (we saw its predecessors, but
+ not it). Without the ON CONFLICT above this would raise
+ 23505, which maps to 0 == #TALER_MERCHANTDB_RS_NO_SUCH_ORDER
+ (a bogus 404 for an order we just read) and additionally
+ poisons the enclosing transaction. Report a serialization
+ failure instead, so that the caller retries and then observes
+ the concurrent refund. */
+ ctx->rs = TALER_MERCHANTDB_RS_SOFT_ERROR;
+ goto cleanup;
default:
ctx->rs = (enum TALER_MERCHANTDB_RefundStatus) qs;
break;
diff --git a/src/backenddb/test_merchantdb.c b/src/backenddb/test_merchantdb.c
@@ -6404,12 +6404,12 @@ struct TestRefunds_Closure
/**
* The order data.
*/
- struct OrderData orders[2];
+ struct OrderData orders[3];
/**
* The deposit data.
*/
- struct DepositData deposits[3];
+ struct DepositData deposits[5];
/**
* The refund data.
@@ -6445,6 +6445,8 @@ pre_test_refunds (struct TestRefunds_Closure *cls)
&cls->orders[0]);
make_order ("test_refunds_od_1",
&cls->orders[1]);
+ make_order ("test_refunds_od_2",
+ &cls->orders[2]);
/* Deposit */
make_deposit (&cls->instance,
@@ -6462,6 +6464,26 @@ pre_test_refunds (struct TestRefunds_Closure *cls)
&cls->orders[1],
&cls->signkey,
&cls->deposits[2]);
+ /* Two deposits of the *same* coin on order 2, via two different
+ exchanges (and thus two deposit confirmations). The schema allows
+ this: merchant_deposits is unique only on
+ (deposit_confirmation_serial, coin_pub). It makes
+ do_increase_refund derive the same rtransaction_id twice, which is
+ exactly the collision a concurrent refund on another connection
+ produces. */
+ make_deposit (&cls->instance,
+ &cls->account,
+ &cls->orders[2],
+ &cls->signkey,
+ &cls->deposits[3]);
+ make_deposit (&cls->instance,
+ &cls->account,
+ &cls->orders[2],
+ &cls->signkey,
+ &cls->deposits[4]);
+ cls->deposits[4].exchange_url = "https://test-exchange-two/";
+ cls->deposits[4].coin_pub = cls->deposits[3].coin_pub;
+ cls->deposits[4].coin_sig = cls->deposits[3].coin_sig;
/* Refund */
make_refund (&cls->deposits[0],
@@ -6499,6 +6521,7 @@ post_test_refunds (struct TestRefunds_Closure *cls)
free_instance_data (&cls->instance);
free_order_data (&cls->orders[0]);
free_order_data (&cls->orders[1]);
+ free_order_data (&cls->orders[2]);
}
@@ -6672,6 +6695,44 @@ run_test_refunds (struct TestRefunds_Closure *cls)
false,
2,
&cls->refunds[1]));
+ /* Two deposits of the same coin on the same order make
+ do_increase_refund pick the same rtransaction_id twice, which is
+ exactly what a concurrent refund on another connection does. The
+ resulting unique violation must NOT be reported as
+ #TALER_MERCHANTDB_RS_NO_SUCH_ORDER (a bogus 404 for an order that
+ obviously exists), but as a serialization failure the caller can
+ retry. */
+ TEST_RET_ON_FAIL (test_insert_order (&cls->instance,
+ &cls->orders[2],
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
+ TEST_RET_ON_FAIL (test_insert_contract_terms (&cls->instance,
+ &cls->orders[2],
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
+ TEST_RET_ON_FAIL (test_insert_deposit (&cls->instance,
+ &cls->signkey,
+ &cls->deposits[3],
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
+ TEST_RET_ON_FAIL (test_insert_deposit (&cls->instance,
+ &cls->signkey,
+ &cls->deposits[4],
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
+ TEST_RET_ON_FAIL (test_mark_contract_paid (&cls->instance,
+ &cls->orders[2],
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
+ /* More than a single deposit is worth, so that the second (duplicate)
+ deposit has to be refunded as well. */
+ GNUNET_assert (GNUNET_OK ==
+ TALER_string_to_amount ("EUR:60.00",
+ &inc));
+ TEST_COND_RET_ON_FAIL (TALER_MERCHANTDB_RS_SOFT_ERROR ==
+ TALER_MERCHANTDB_do_increase_refund (pg,
+ cls->instance.instance.id,
+ cls->orders[2].id,
+ &inc,
+ NULL,
+ NULL,
+ "duplicate coin"),
+ "Duplicate rtransaction_id not reported as serialization failure\n");
return 0;
}
