From 698b4a4f1232eaf758a7cd520248acfd27d5fe5d Mon Sep 17 00:00:00 2001
From: Sameh Abouel-saad <samehabouelsaad@gmail.com>
Date: Mon, 1 Jun 2026 17:50:17 +0300
Subject: [PATCH] fix(pallet-tft-bridge): reject creating an already-executed
 refund

create_stellar_refund_transaction_or_add_sig only checked whether the
refund existed in RefundTransactions, not whether it had already been
executed. A refund that was already executed (e.g. quarantined by the
bridge after a permanently undeliverable Stellar submission) could be
silently recreated, re-arming the on_finalize retry loop.

Add an ExecutedRefundTransactions guard, mirroring the existing checks in
propose_or_vote_stellar_mint_transaction and
propose_stellar_burn_transaction_or_add_sig. The bridge already recovers
from the resulting RefundTransactionAlreadyExecuted error via the
IsRefundedAlready check in RetryCreateRefundTransactionOrAddSig.

Note: spec_version is intentionally not bumped here; that is handled at
release time via `make version-bump` (see docs/production/releases.md).

Refs #1089

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../pallets/pallet-tft-bridge/src/tests.rs    | 46 +++++++++++++++++++
 .../pallet-tft-bridge/src/tft_bridge.rs       |  9 ++++
 2 files changed, 55 insertions(+)

diff --git a/substrate-node/pallets/pallet-tft-bridge/src/tests.rs b/substrate-node/pallets/pallet-tft-bridge/src/tests.rs
index 6a80112a9..1657aef93 100644
--- a/substrate-node/pallets/pallet-tft-bridge/src/tests.rs
+++ b/substrate-node/pallets/pallet-tft-bridge/src/tests.rs
@@ -610,6 +610,52 @@ fn refund_signature_after_expiry_resets_block_and_survives() {
     });
 }
 
+#[test]
+fn creating_refund_for_already_executed_transaction_fails() {
+    new_test_ext().execute_with(|| {
+        assert_ok!(TFTBridgeModule::add_bridge_validator(
+            RawOrigin::Root.into(),
+            alice()
+        ));
+
+        let tx_hash = b"some_tx".to_vec();
+
+        // Create the refund and add a signature. With a single validator this
+        // immediately reaches the signature threshold.
+        assert_ok!(TFTBridgeModule::create_refund_transaction_or_add_sig(
+            RuntimeOrigin::signed(alice()),
+            tx_hash.clone(),
+            b"some_target".to_vec(),
+            2,
+            b"some_signature".to_vec(),
+            b"some_pub_key".to_vec(),
+            0,
+        ));
+
+        // Mark it executed: moves it from RefundTransactions to
+        // ExecutedRefundTransactions.
+        assert_ok!(TFTBridgeModule::set_refund_transaction_executed(
+            RuntimeOrigin::signed(alice()),
+            tx_hash.clone(),
+        ));
+
+        // Re-creating the same refund must be rejected, not silently recreated
+        // (which would re-arm the on_finalize retry/crash loop).
+        assert_noop!(
+            TFTBridgeModule::create_refund_transaction_or_add_sig(
+                RuntimeOrigin::signed(alice()),
+                tx_hash.clone(),
+                b"some_target".to_vec(),
+                2,
+                b"some_signature".to_vec(),
+                b"some_pub_key".to_vec(),
+                0,
+            ),
+            Error::<TestRuntime>::RefundTransactionAlreadyExecuted
+        );
+    });
+}
+
 fn prepare_validators() {
     TFTBridgeModule::add_bridge_validator(RawOrigin::Root.into(), alice()).unwrap();
     TFTBridgeModule::add_bridge_validator(RawOrigin::Root.into(), bob()).unwrap();
diff --git a/substrate-node/pallets/pallet-tft-bridge/src/tft_bridge.rs b/substrate-node/pallets/pallet-tft-bridge/src/tft_bridge.rs
index ad4976929..4da86c306 100644
--- a/substrate-node/pallets/pallet-tft-bridge/src/tft_bridge.rs
+++ b/substrate-node/pallets/pallet-tft-bridge/src/tft_bridge.rs
@@ -120,6 +120,15 @@ impl<T: Config> Pallet<T> {
     ) -> DispatchResultWithPostInfo {
         Self::check_if_validator_exists(validator.clone())?;
 
+        // check if it already has been executed in the past, mirroring the mint
+        // and burn paths. Without this guard a refund that was already executed
+        // (e.g. quarantined by the bridge) would be silently recreated, re-arming
+        // the on_finalize retry loop.
+        ensure!(
+            !ExecutedRefundTransactions::<T>::contains_key(tx_hash.clone()),
+            Error::<T>::RefundTransactionAlreadyExecuted
+        );
+
         // make sure we don't duplicate the transaction
         // ensure!(!MintTransactions::<T>::contains_key(tx_id.clone()), Error::<T>::MintTransactionExists);
         if RefundTransactions::<T>::contains_key(tx_hash.clone()) {