commit 4beae60ba82d2ea5cd7997e6e142e4c2c0ded2f5
parent 8ff0c4eb4867726c9928adabc3fbdd7baf8f0a77
Author: Florian Dold <dold@taler.net>
Date: Wed, 23 Sep 2026 17:48:13 +0200
merchant database: settle all orders covered by an aggregated payout
Emit the settlement notification and the order_settled webhook only when
the contract actually transitions to wired, so that a replayed signed
transfer does not report the same order twice.
Repair historical stale confirmation and order flags in migration 0052
without replaying historical webhooks. The repair uses the same
predicate as merchant_do_insert_transfer_details(), so it settles
exactly the rows the current code would have settled itself.
Fixes #11813
Assisted by LLM
Diffstat:
4 files changed, 244 insertions(+), 8 deletions(-)
diff --git a/src/backenddb/insert_transfer_details.sql b/src/backenddb/insert_transfer_details.sql
@@ -1,6 +1,6 @@
--
-- This file is part of TALER
--- Copyright (C) 2024, 2025 Taler Systems SA
+-- Copyright (C) 2024, 2025, 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
@@ -248,13 +248,13 @@ LOOP
UPDATE merchant_contract_terms
SET wired=TRUE
- WHERE (order_serial=my_affected_orders.order_serial);
-
- -- Select merchant_serial and order_id for webhook
- SELECT order_id
- INTO my_order_id
- FROM merchant_contract_terms
- WHERE order_serial=my_affected_orders.order_serial;
+ WHERE (order_serial=my_affected_orders.order_serial)
+ AND NOT wired
+ RETURNING order_id INTO my_order_id;
+ IF NOT FOUND
+ THEN
+ CONTINUE;
+ END IF;
-- Remember ALL orders that completed, not just the last
-- one: an aggregated transfer settles many orders and
-- every one of them needs an ORDER_STATUS_CHANGED event.
diff --git a/src/backenddb/sql-schema/merchant-0052.sql b/src/backenddb/sql-schema/merchant-0052.sql
@@ -0,0 +1,175 @@
+--
+-- 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 merchant-0052.sql
+-- @brief repair settlement flags when reconciliation preceded deposit checking
+
+BEGIN;
+
+SELECT _v.register_patch('merchant-0052', NULL, NULL);
+
+SET search_path TO merchant;
+
+CREATE PROCEDURE merchant.merchant_0052_init(s TEXT)
+ LANGUAGE plpgsql
+ AS $OUTER$
+BEGIN
+
+ EXECUTE format('SET LOCAL search_path TO %I', s);
+
+ -- Reconciliation used to require a successful per-deposit check even
+ -- when the aggregate's signed transfer details already covered the coin.
+ -- Repair the resulting stale flags from those stored associations. Do not
+ -- require another visit to the order detail endpoint (issue 11813).
+ -- The predicate below must stay in sync with the one in
+ -- merchant_do_insert_transfer_details(), so that the rows repaired here
+ -- are exactly those the current code would have settled itself.
+ --
+ -- How to read that predicate. There are two independent proofs that a
+ -- deposit was paid out, and either one on its own suffices:
+ --
+ -- CLEAN taler-merchant-depositcheck asked the exchange about this
+ -- very deposit and got a wire transfer identifier back.
+ -- merchant_insert_deposit_to_transfer() records that as
+ -- NOT settlement_retry_needed
+ -- AND settlement_last_ec = 0
+ -- AND settlement_wtid IS NOT NULL.
+ --
+ -- COVERED taler-merchant-reconciliation verified the exchange's
+ -- signature over an aggregate transfer whose coin list names
+ -- this deposit: a merchant_expected_transfer_to_coin row for
+ -- it, on a transfer to this deposit confirmation's own
+ -- exchange and account, that also carries a
+ -- merchant_transfer_signatures row. This is the evidence
+ -- that migration 0045's predicate did not yet accept.
+ --
+ -- One state overrides both. When the exchange answered authoritatively
+ -- that the money went somewhere we cannot accept -- EC 2558, target
+ -- account unknown -- there is nothing to settle however well the coin is
+ -- covered, so a permanent failure keeps blocking:
+ --
+ -- PERM_FAIL NOT settlement_retry_needed
+ -- AND settlement_last_ec <> 0
+ --
+ -- A single deposit therefore blocks its deposit confirmation iff
+ --
+ -- BLOCKED = PERM_FAIL OR (NOT COVERED AND NOT CLEAN)
+ --
+ -- and a confirmation is wired once it has at least one deposit and no
+ -- BLOCKED one -- which is the EXISTS / NOT EXISTS pair below. Note that
+ -- PERM_FAIL implies NOT CLEAN, so the two disjuncts are not disjoint;
+ -- that is fine, BLOCKED only has to be TRUE often enough.
+ --
+ -- By De Morgan on CLEAN,
+ --
+ -- NOT CLEAN = settlement_retry_needed
+ -- OR settlement_last_ec <> 0
+ -- OR settlement_wtid IS NULL
+ --
+ -- which is the inner three-way OR. Read it as the single concept "the
+ -- per-deposit check has not come back clean" rather than as three
+ -- separate conditions, and keep it in one piece when editing: factoring
+ -- settlement_last_ec out of it, say, still computes BLOCKED but no
+ -- longer reads as the rule it implements.
+ --
+ -- Two tempting simplifications are both wrong:
+ --
+ -- * NOT COVERED alone must not block. A deposit settled through the
+ -- CLEAN route never gets a merchant_expected_transfer_to_coin row,
+ -- since merchant_do_insert_transfer_details() is the only writer of
+ -- that table, so NOT COVERED is the normal case, not an anomaly.
+ -- It is in particular the case for the sibling coins of an order
+ -- whose aggregate has not been reconciled yet.
+ --
+ -- * NOT CLEAN alone must not block either -- that was exactly the
+ -- pre-fix predicate this migration repairs after.
+ --
+ -- The COALESCE calls are load-bearing: settlement_retry_needed and
+ -- settlement_last_ec are both nullable, NULL meaning "never inquired".
+ -- Without them a NULL would make the OR chain evaluate to NULL instead
+ -- of TRUE, and the row would silently fail to block.
+ --
+ -- Why a migration is needed at all, rather than just the fixed
+ -- predicate: wire_pending is cleared in exactly one place in the running
+ -- code, inside merchant_do_insert_transfer_details(), and that function
+ -- returns out_duplicate early -- before its per-coin loop -- once the
+ -- transfer's signature is stored. update_to_expected_transfer_finalized()
+ -- additionally clears retry_needed on the expected transfer, so
+ -- merchant.select_open_transfers() never offers that wtid again, and the
+ -- per-deposit route never writes wire_pending itself. A confirmation
+ -- left pending by the old predicate is therefore stranded for good:
+ -- its order stays paid-but-not-wired no matter how many times
+ -- depositcheck and reconciliation run afterwards.
+ UPDATE merchant_deposit_confirmations mdc
+ SET wire_pending=FALSE
+ WHERE mdc.wire_pending
+ AND EXISTS
+ (SELECT 1 FROM merchant_deposits md
+ WHERE md.deposit_confirmation_serial=mdc.deposit_confirmation_serial)
+ AND NOT EXISTS
+ (SELECT 1 FROM merchant_deposits md
+ WHERE md.deposit_confirmation_serial=mdc.deposit_confirmation_serial
+ AND ((NOT COALESCE(md.settlement_retry_needed, TRUE)
+ AND COALESCE(md.settlement_last_ec, 0) <> 0)
+ OR ((COALESCE(md.settlement_retry_needed, TRUE)
+ OR md.settlement_wtid IS NULL
+ OR COALESCE(md.settlement_last_ec, 0) <> 0)
+ AND NOT EXISTS
+ (SELECT 1
+ FROM merchant_expected_transfer_to_coin tc
+ JOIN merchant_expected_transfers et
+ USING (expected_credit_serial)
+ JOIN merchant_transfer_signatures ts
+ USING (expected_credit_serial)
+ WHERE tc.deposit_serial=md.deposit_serial
+ AND et.exchange_url=mdc.exchange_url
+ AND et.account_serial=mdc.account_serial))));
+
+ -- A paid contract is wired once all of its deposit confirmations are done.
+ -- Require a confirmation so externally marked payments are not changed.
+ -- This migration deliberately does not generate historical notifications
+ -- or order-settled webhooks.
+ UPDATE merchant_contract_terms mct
+ SET wired=TRUE
+ WHERE mct.paid
+ AND NOT mct.wired
+ AND EXISTS
+ (SELECT 1
+ FROM merchant_deposit_confirmations mdc
+ WHERE mdc.order_serial=mct.order_serial)
+ AND NOT EXISTS
+ (SELECT 1
+ FROM merchant_deposit_confirmations mdc
+ WHERE mdc.order_serial=mct.order_serial
+ AND mdc.wire_pending);
+
+ SET LOCAL search_path TO merchant;
+
+END
+$OUTER$;
+
+COMMENT ON PROCEDURE merchant.merchant_0052_init(TEXT)
+ IS 'Clears stale wire_pending and wired flags of one instance schema for transfers that the stored signed transfer details already cover.';
+
+INSERT INTO merchant.instance_fixups
+ (migration_name
+ ,version)
+ VALUES
+ ('merchant_0052_init'
+ ,52);
+-- Apply new fix-up to existing instances
+CALL merchant.fixup_instance_schema (52::INT8);
+
+COMMIT;
diff --git a/src/backenddb/sql-schema/meson.build b/src/backenddb/sql-schema/meson.build
@@ -151,6 +151,7 @@ generated_sql = [
['merchant-0049.sql'],
['merchant-0050.sql'],
['merchant-0051.sql'],
+ ['merchant-0052.sql'],
]
migration_sql = []
diff --git a/src/backenddb/test_merchantdb.c b/src/backenddb/test_merchantdb.c
@@ -7438,6 +7438,66 @@ run_test_transfers (struct TestTransfers_Closure *cls)
false),
goto webhook_listener_cleanup;
);
+ /* Replaying the signed transfer must not report another transition or
+ enqueue another webhook for either order. */
+ TEST_WITH_FAIL_CLAUSE (
+ test_insert_transfer_details_settling (
+ &cls->instance,
+ &cls->account,
+ &cls->transfers[3],
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT,
+ 0),
+ goto webhook_listener_cleanup;
+ );
+
+ /* Simulate flags left stale by an older backend, then run the real
+ migration repair twice. It must repair the unchecked aggregate coin,
+ preserve the permanent failure, and not queue historical webhooks. */
+ TEST_WITH_FAIL_CLAUSE (
+ exec_sql ("UPDATE merchant_deposit_confirmations SET wire_pending=TRUE;"
+ "UPDATE merchant_contract_terms SET wired=FALSE;"),
+ goto webhook_listener_cleanup;
+ );
+ for (unsigned int attempt = 0; attempt < 2; attempt++)
+ {
+ TEST_WITH_FAIL_CLAUSE (
+ exec_sql ("CALL merchant.merchant_0052_init(current_schema()::TEXT)"),
+ goto webhook_listener_cleanup;
+ );
+ for (unsigned int i = 0; i < 2; i++)
+ TEST_WITH_FAIL_CLAUSE (
+ test_lookup_payment_status (cls->instance.instance.id,
+ cls->orders_agg[i].id,
+ NULL,
+ true,
+ true),
+ goto webhook_listener_cleanup;
+ );
+ TEST_WITH_FAIL_CLAUSE (
+ test_lookup_payment_status (cls->instance.instance.id,
+ cls->order_perm.id,
+ NULL,
+ true,
+ false),
+ goto webhook_listener_cleanup;
+ );
+ }
+ GNUNET_PQ_event_do_poll (pg->conn);
+ TEST_WITH_FAIL_CLAUSE (
+ query_sql_num (
+ "SELECT COUNT(*) AS num"
+ " FROM merchant.merchant_pending_webhooks"
+ " WHERE url='https://example.com/order-settled-event-test'",
+ &pending_count),
+ goto webhook_listener_cleanup;
+ );
+ if ( (2 != pending_count) || (1 != event_state.fired) )
+ {
+ GNUNET_break (0);
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Repeated settlement or migration queued duplicate webhooks\n");
+ goto webhook_listener_cleanup;
+ }
webhook_test_result = 0;
webhook_listener_cleanup:
