Revert reorged funding payments instead of duplicating them

A funding payment's id is anchored to its first negotiated candidate, but
once it confirms the record is stamped with the candidate that actually
confirmed. After it graduates and its pending-store entry is removed, a
later reorg event carrying the confirmed candidate's txid could no longer
be resolved to the payment, so it was recorded as a duplicate generic
on-chain payment rather than reverting the splice payment. RBF splices,
whose confirmed candidate differs from the first, are the reachable case.

Resolve such an event against the payment store by on-chain txid once the
pending store no longer holds it, and revert the funding payment to
pending so wallet sync re-graduates it when it reconfirms.

Raised by Codex in the review of #888.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: jkczyz

src/wallet/mod.rs

@@ -392,11 +392,6 @@ impl Wallet {
 						continue;
 					};
 
-					// Collect all conflict txids
-					let mut conflict_txids: Vec<Txid> =
-						conflicts.iter().map(|(_, conflict_txid)| *conflict_txid).collect();
-
-					conflict_txids.push(txid);
 					// The payment already exists in the store at this point: `bump_fee_rbf` updates
 					// the payment store with the replacement txid before the next sync cycle, so we
 					// can safely fetch it here.
@@ -407,8 +402,26 @@ impl Wallet {
 					);
 					let payment =
 						self.payment_store.get(&payment_id).ok_or(Error::InvalidPaymentId)?;
+
+					// A graduated funding payment is resolvable here only through
+					// `find_payment_by_txid`'s payment-store fallback. Revert it like the
+					// `TxUnconfirmed`/`TxDropped` arms instead of mirroring a non-`Pending` record
+					// into the pending store, which graduation's pending-only scan would reject.
+					if payment.status != PaymentStatus::Pending
+						&& self.apply_funding_status_update(
+							payment_id,
+							txid,
+							ConfirmationStatus::Unconfirmed,
+						)? {
+						continue;
+					}
+
+					// Collect all conflict txids
+					let mut conflict_txids: Vec<Txid> =
+						conflicts.iter().map(|(_, conflict_txid)| *conflict_txid).collect();
+					conflict_txids.push(txid);
 					let pending_payment_details =
-						self.create_pending_payment_from_tx(payment, conflict_txids.clone());
+						self.create_pending_payment_from_tx(payment, conflict_txids);
 
 					self.runtime.block_on(
 						self.pending_payment_store.insert_or_update(pending_payment_details),
@@ -1454,6 +1467,34 @@ impl Wallet {
 			return Some(replaced_details.details.id);
 		}
 
+		// A funding payment graduates out of the pending store, after which only the payment store
+		// retains it — under its first-candidate-anchored id, but stamped with the confirmed
+		// candidate's txid. Map a later event (e.g. a reorg returning the confirmed candidate to the
+		// mempool) back to that funding payment so it is reverted in place rather than duplicated as
+		// a generic on-chain payment under the candidate's txid. Only one funding record carries a
+		// given confirmed txid (its id is anchored to the first candidate and reclassification
+		// merges into it), so the first match is unambiguous.
+		if let Some(funding) = self
+			.payment_store
+			.list_filter(|p| {
+				matches!(
+					p.kind,
+					PaymentKind::Onchain {
+						txid,
+						tx_type:
+							Some(
+								TransactionType::Funding { .. }
+								| TransactionType::InteractiveFunding { .. },
+							),
+						..
+					} if txid == target_txid
+				)
+			})
+			.first()
+		{
+			return Some(funding.id);
+		}
+
 		None
 	}
 
@@ -1491,6 +1532,14 @@ impl Wallet {
 			}
 		}
 
+		// A reorg returning the transaction to the mempool reverts the payment to pending so wallet
+		// sync re-graduates it once it reconfirms. This also re-establishes the pending-store entry
+		// below (gated on `Pending`) that graduation removed; without it a graduated payment would
+		// be left `Succeeded` with an `Unconfirmed` kind and no way to re-graduate.
+		if matches!(confirmation_status, ConfirmationStatus::Unconfirmed) {
+			payment.status = PaymentStatus::Pending;
+		}
+
 		payment.kind =
 			PaymentKind::Onchain { txid: event_txid, status: confirmation_status, tx_type };
 		self.runtime.block_on(self.payment_store.insert_or_update(payment.clone()))?;

tests/integration_tests_rust.rs

@@ -1948,6 +1948,166 @@ async fn splice_payment_reorged_to_unconfirmed() {
 	node_b.stop().unwrap();
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+async fn rbf_splice_payment_reverts_after_deep_reorg() {
+	// A graduated RBF splice payment is anchored to the FIRST candidate's id but stamped with the
+	// CONFIRMED (RBF) candidate's txid. Graduation removes its pending-store entry, so a later deep
+	// reorg (deeper than ANTI_REORG_DELAY) that returns the confirmed candidate to the mempool must
+	// still map the event back to the original payment and revert it — not create a duplicate
+	// generic on-chain payment under the confirmed candidate's id.
+
+	// Lower incrementalrelayfee so the RBF feerate bump is relayable (as run_rbf_splice_channel_test).
+	let bitcoind_exe = std::env::var("BITCOIND_EXE")
+		.ok()
+		.or_else(|| corepc_node::downloaded_exe_path().ok())
+		.expect(
+			"you need to provide an env var BITCOIND_EXE or specify a bitcoind version feature",
+		);
+	let mut bitcoind_conf = corepc_node::Conf::default();
+	bitcoind_conf.network = "regtest";
+	bitcoind_conf.args.push("-rest");
+	bitcoind_conf.args.push("-incrementalrelayfee=0.00000100");
+	let bitcoind = BitcoinD::with_conf(bitcoind_exe, &bitcoind_conf).unwrap();
+
+	let electrs_exe = std::env::var("ELECTRS_EXE")
+		.ok()
+		.or_else(electrsd::downloaded_exe_path)
+		.expect("you need to provide env var ELECTRS_EXE or specify an electrsd version feature");
+	let mut electrsd_conf = electrsd::Conf::default();
+	electrsd_conf.http_enabled = true;
+	electrsd_conf.network = "regtest";
+	let electrsd = ElectrsD::with_conf(electrs_exe, &bitcoind, &electrsd_conf).unwrap();
+	let chain_source = random_chain_source(&bitcoind, &electrsd);
+
+	let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, false);
+
+	let address_a = node_a.onchain_payment().new_address().unwrap();
+	let address_b = node_b.onchain_payment().new_address().unwrap();
+	let premine_amount_sat = 5_000_000;
+	premine_and_distribute_funds(
+		&bitcoind.client,
+		&electrsd.client,
+		vec![address_a, address_b],
+		Amount::from_sat(premine_amount_sat),
+	)
+	.await;
+	node_a.sync_wallets().unwrap();
+	node_b.sync_wallets().unwrap();
+
+	open_channel(&node_a, &node_b, 4_000_000, false, &electrsd).await;
+	generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await;
+	node_a.sync_wallets().unwrap();
+	node_b.sync_wallets().unwrap();
+	let _user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id());
+	let user_channel_id_b = expect_channel_ready_event!(node_b, node_a.node_id());
+
+	// node_b splices in, then RBF-bumps it: the funding payment spans two candidates, its id
+	// anchored to the first (original) candidate's txid.
+	node_b.splice_in(&user_channel_id_b, node_a.node_id(), 1_000_000).unwrap();
+	let original_txo = expect_splice_negotiated_event!(node_a, node_b.node_id());
+	expect_splice_negotiated_event!(node_b, node_a.node_id());
+	wait_for_tx(&electrsd.client, original_txo.txid).await;
+	wait_for_classified_funding_payment(&node_b, original_txo.txid).await;
+	node_a.sync_wallets().unwrap();
+	node_b.sync_wallets().unwrap();
+
+	node_b.bump_channel_funding_fee(&user_channel_id_b, node_a.node_id()).unwrap();
+	let rbf_txo = expect_splice_negotiated_event!(node_a, node_b.node_id());
+	expect_splice_negotiated_event!(node_b, node_a.node_id());
+	assert_ne!(original_txo, rbf_txo, "RBF should produce a different funding txo");
+	wait_for_tx(&electrsd.client, rbf_txo.txid).await;
+	wait_for_classified_funding_payment(&node_b, rbf_txo.txid).await;
+	node_a.sync_wallets().unwrap();
+	node_b.sync_wallets().unwrap();
+
+	// Confirm the RBF candidate and graduate it past ANTI_REORG_DELAY (6 confirmations), which
+	// removes the pending-store entry.
+	generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await;
+	node_a.sync_wallets().unwrap();
+	node_b.sync_wallets().unwrap();
+	expect_channel_ready_event!(node_a, node_b.node_id());
+	expect_channel_ready_event!(node_b, node_a.node_id());
+
+	let payment_id = PaymentId(original_txo.txid.to_byte_array());
+	let rbf_payment_id = PaymentId(rbf_txo.txid.to_byte_array());
+
+	// Graduated: anchored to the original candidate's id, stamped with the confirmed RBF
+	// candidate's txid, with no separate record under the RBF candidate's id.
+	let payment = node_b.payment(&payment_id).expect("splice payment graduated");
+	assert_eq!(payment.status, PaymentStatus::Succeeded);
+	assert!(matches!(
+		payment.kind,
+		PaymentKind::Onchain {
+			txid,
+			status: ConfirmationStatus::Confirmed { .. },
+			tx_type: Some(TransactionType::InteractiveFunding { .. }),
+		} if txid == rbf_txo.txid
+	));
+	assert!(
+		node_b.payment(&rbf_payment_id).is_none(),
+		"the graduated splice payment must not be duplicated under the RBF candidate's id",
+	);
+
+	// Deep reorg (deeper than ANTI_REORG_DELAY): drop the 6 graduation blocks and build a longer,
+	// transaction-free chain, returning the confirmed RBF candidate to the mempool.
+	let original_height =
+		bitcoind.client.get_blockchain_info().expect("failed to get blockchain info").blocks;
+	invalidate_blocks(&bitcoind.client, 6);
+	let replacement_address = bitcoind.client.new_address().expect("failed to get new address");
+	for _ in 0..7 {
+		let _res: serde_json::Value = bitcoind
+			.client
+			.call("generateblock", &[json!(replacement_address.to_string()), json!([])])
+			.expect("failed to generate empty block");
+	}
+	wait_for_block(&electrsd.client, original_height as usize + 1).await;
+	// Wait for the reorged-out RBF candidate to reappear in the mempool before syncing, so the sync
+	// reliably observes its TxUnconfirmed event rather than racing electrs's reindex.
+	wait_for_tx(&electrsd.client, rbf_txo.txid).await;
+	node_b.sync_wallets().unwrap();
+
+	// The reorg event for the confirmed RBF candidate's txid must map back to the original payment
+	// and revert it to Pending/Unconfirmed, rather than creating a duplicate generic on-chain
+	// payment under the RBF candidate's id.
+	assert!(
+		node_b.payment(&rbf_payment_id).is_none(),
+		"a reorged-out RBF splice must not produce a duplicate generic on-chain payment",
+	);
+	let payment = node_b.payment(&payment_id).expect("splice payment still exists after the reorg");
+	assert_eq!(payment.status, PaymentStatus::Pending);
+	assert!(matches!(
+		payment.kind,
+		PaymentKind::Onchain {
+			status: ConfirmationStatus::Unconfirmed,
+			tx_type: Some(TransactionType::InteractiveFunding { .. }),
+			..
+		}
+	));
+
+	// The revert re-established the pending-store entry, so once the RBF candidate (still in the
+	// mempool) reconfirms past ANTI_REORG_DELAY the payment re-graduates to Succeeded in place —
+	// without leaving a duplicate behind.
+	generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await;
+	node_b.sync_wallets().unwrap();
+	let payment = node_b.payment(&payment_id).expect("splice payment re-graduated");
+	assert_eq!(payment.status, PaymentStatus::Succeeded);
+	assert!(matches!(
+		payment.kind,
+		PaymentKind::Onchain {
+			txid,
+			status: ConfirmationStatus::Confirmed { .. },
+			tx_type: Some(TransactionType::InteractiveFunding { .. }),
+		} if txid == rbf_txo.txid
+	));
+	assert!(
+		node_b.payment(&rbf_payment_id).is_none(),
+		"re-graduation must not create a duplicate payment under the RBF candidate's id",
+	);
+
+	node_a.stop().unwrap();
+	node_b.stop().unwrap();
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
 async fn splice_in_rbf_joins_counterparty_splice() {
 	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();