Skip to content

Commit 698b4a4

Browse files
sameh-faroukclaude
andcommitted
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>
1 parent 3d1145f commit 698b4a4

2 files changed

Lines changed: 55 additions & 0 deletions

File tree

substrate-node/pallets/pallet-tft-bridge/src/tests.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,52 @@ fn refund_signature_after_expiry_resets_block_and_survives() {
610610
});
611611
}
612612

613+
#[test]
614+
fn creating_refund_for_already_executed_transaction_fails() {
615+
new_test_ext().execute_with(|| {
616+
assert_ok!(TFTBridgeModule::add_bridge_validator(
617+
RawOrigin::Root.into(),
618+
alice()
619+
));
620+
621+
let tx_hash = b"some_tx".to_vec();
622+
623+
// Create the refund and add a signature. With a single validator this
624+
// immediately reaches the signature threshold.
625+
assert_ok!(TFTBridgeModule::create_refund_transaction_or_add_sig(
626+
RuntimeOrigin::signed(alice()),
627+
tx_hash.clone(),
628+
b"some_target".to_vec(),
629+
2,
630+
b"some_signature".to_vec(),
631+
b"some_pub_key".to_vec(),
632+
0,
633+
));
634+
635+
// Mark it executed: moves it from RefundTransactions to
636+
// ExecutedRefundTransactions.
637+
assert_ok!(TFTBridgeModule::set_refund_transaction_executed(
638+
RuntimeOrigin::signed(alice()),
639+
tx_hash.clone(),
640+
));
641+
642+
// Re-creating the same refund must be rejected, not silently recreated
643+
// (which would re-arm the on_finalize retry/crash loop).
644+
assert_noop!(
645+
TFTBridgeModule::create_refund_transaction_or_add_sig(
646+
RuntimeOrigin::signed(alice()),
647+
tx_hash.clone(),
648+
b"some_target".to_vec(),
649+
2,
650+
b"some_signature".to_vec(),
651+
b"some_pub_key".to_vec(),
652+
0,
653+
),
654+
Error::<TestRuntime>::RefundTransactionAlreadyExecuted
655+
);
656+
});
657+
}
658+
613659
fn prepare_validators() {
614660
TFTBridgeModule::add_bridge_validator(RawOrigin::Root.into(), alice()).unwrap();
615661
TFTBridgeModule::add_bridge_validator(RawOrigin::Root.into(), bob()).unwrap();

substrate-node/pallets/pallet-tft-bridge/src/tft_bridge.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,15 @@ impl<T: Config> Pallet<T> {
120120
) -> DispatchResultWithPostInfo {
121121
Self::check_if_validator_exists(validator.clone())?;
122122

123+
// check if it already has been executed in the past, mirroring the mint
124+
// and burn paths. Without this guard a refund that was already executed
125+
// (e.g. quarantined by the bridge) would be silently recreated, re-arming
126+
// the on_finalize retry loop.
127+
ensure!(
128+
!ExecutedRefundTransactions::<T>::contains_key(tx_hash.clone()),
129+
Error::<T>::RefundTransactionAlreadyExecuted
130+
);
131+
123132
// make sure we don't duplicate the transaction
124133
// ensure!(!MintTransactions::<T>::contains_key(tx_id.clone()), Error::<T>::MintTransactionExists);
125134
if RefundTransactions::<T>::contains_key(tx_hash.clone()) {

0 commit comments

Comments
 (0)