Preserve funding-payment confirmation state on late reclassification

Funding broadcasts are classified into payment records off the
broadcaster's queue, which can run after wallet sync has already recorded
the transaction -- for instance when LDK re-broadcasts a still-pending
funding transaction on restart. In that case the classification overwrote
a record wallet sync had already advanced, downgrading a confirmed or
graduated funding payment back to unconfirmed/pending.

Merge only the classification and our contribution figures into an
existing record, leaving the confirmation state that the wallet-sync
events own in place.

Raised by Codex in the review of #888.

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

Author: jkczyz

src/payment/store.rs

@@ -712,6 +712,33 @@ impl PaymentDetailsUpdate {
 			tx_type: None,
 		}
 	}
+
+	/// Builds an update that merges a freshly-classified funding payment's classification
+	/// (`tx_type`), broadcast txid, and our contribution figures (amount/fee) into an existing
+	/// record, while leaving the top-level [`PaymentStatus`] and the on-chain
+	/// [`ConfirmationStatus`] untouched.
+	///
+	/// Funding classification runs off the broadcaster queue and can land *after* wallet sync has
+	/// already advanced a record's confirmation state (e.g. when LDK re-broadcasts a still-pending
+	/// funding transaction on restart, or when the counterparty's broadcast is observed first).
+	/// Merging only the funding-specific fields keeps such a late classification from downgrading a
+	/// `Confirmed`/`Succeeded` payment back to `Unconfirmed`/`Pending`; the confirmation state is
+	/// owned by the wallet-sync events instead.
+	///
+	/// The txid and figures are taken from the freshly broadcast (active) candidate. LDK only
+	/// re-broadcasts the active/confirmed funding candidate, so for an already-confirmed record
+	/// these equal what graduation stamped and the overwrite is a no-op; we rely on that invariant
+	/// rather than gating the txid/amount/fee merge on the stored confirmation state.
+	pub(crate) fn funding_reclassification(details: PaymentDetails) -> Self {
+		let mut update = Self::new(details.id);
+		update.amount_msat = Some(details.amount_msat);
+		update.fee_paid_msat = Some(details.fee_paid_msat);
+		if let PaymentKind::Onchain { txid, tx_type, .. } = details.kind {
+			update.txid = Some(txid);
+			update.tx_type = Some(tx_type);
+		}
+		update
+	}
 }
 
 impl From<&PaymentDetails> for PaymentDetailsUpdate {
@@ -1022,6 +1049,94 @@ mod tests {
 		}
 	}
 
+	#[test]
+	fn funding_reclassification_does_not_downgrade_an_advanced_record() {
+		use bitcoin::hashes::Hash;
+		use std::str::FromStr;
+
+		// A splice funding payment wallet sync has already advanced to Succeeded/Confirmed.
+		let txid = Txid::from_byte_array([7u8; 32]);
+		let id = PaymentId(txid.to_byte_array());
+		let tx_type = Some(TransactionType::InteractiveFunding {
+			channels: vec![Channel {
+				counterparty_node_id: PublicKey::from_str(
+					"0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
+				)
+				.unwrap(),
+				channel_id: ChannelId([3u8; 32]),
+			}],
+		});
+		let advanced = PaymentDetails::new(
+			id,
+			PaymentKind::Onchain {
+				txid,
+				status: ConfirmationStatus::Confirmed {
+					block_hash: BlockHash::from_byte_array([8u8; 32]),
+					height: 100,
+					timestamp: 1,
+				},
+				tx_type: tx_type.clone(),
+			},
+			Some(2_000_000),
+			Some(999),
+			PaymentDirection::Outbound,
+			PaymentStatus::Succeeded,
+		);
+
+		// A fresh funding classification for the same payment is always Pending/Unconfirmed.
+		let fresh = PaymentDetails::new(
+			id,
+			PaymentKind::Onchain { txid, status: ConfirmationStatus::Unconfirmed, tx_type },
+			Some(1_000_000),
+			Some(500),
+			PaymentDirection::Outbound,
+			PaymentStatus::Pending,
+		);
+
+		// The naive full update `insert_or_update` applied before the fix downgrades both the
+		// top-level status and the on-chain confirmation status — the bug Codex flagged.
+		let mut downgraded = advanced.clone();
+		downgraded.update((&fresh).into());
+		assert_eq!(
+			downgraded.status,
+			PaymentStatus::Pending,
+			"a full update from a fresh classification downgrades the top-level status",
+		);
+		assert!(
+			matches!(
+				downgraded.kind,
+				PaymentKind::Onchain { status: ConfirmationStatus::Unconfirmed, .. }
+			),
+			"a full update from a fresh classification downgrades the confirmation status",
+		);
+
+		// The narrowed reclassification update merges only the funding fields and preserves the
+		// advanced confirmation state that wallet sync owns.
+		let mut merged = advanced.clone();
+		merged.update(PaymentDetailsUpdate::funding_reclassification(fresh));
+		assert_eq!(
+			merged.status,
+			PaymentStatus::Succeeded,
+			"reclassification must not downgrade the top-level status",
+		);
+		assert!(
+			matches!(
+				merged.kind,
+				PaymentKind::Onchain {
+					status: ConfirmationStatus::Confirmed { .. },
+					tx_type: Some(TransactionType::InteractiveFunding { .. }),
+					..
+				}
+			),
+			"reclassification must preserve the confirmation status and keep the funding tx_type",
+		);
+		// The contribution-derived figures from the fresh classification ARE merged in, replacing
+		// the existing record's: they are authoritative (the wallet can't recompute our share of a
+		// shared funding output), so the merge must carry them.
+		assert_eq!(merged.amount_msat, Some(1_000_000));
+		assert_eq!(merged.fee_paid_msat, Some(500));
+	}
+
 	#[derive(Clone, Debug, PartialEq, Eq)]
 	struct LegacyBolt11JitKind {
 		hash: PaymentHash,

src/wallet/mod.rs

@@ -56,7 +56,8 @@ use persist::KVStoreWalletPersister;
 use crate::config::Config;
 use crate::fee_estimator::{ConfirmationTarget, FeeEstimator, OnchainFeeEstimator};
 use crate::logger::{log_debug, log_error, log_info, log_trace, LdkLogger, Logger};
-use crate::payment::store::ConfirmationStatus;
+use crate::payment::pending_payment_store::PendingPaymentDetailsUpdate;
+use crate::payment::store::{ConfirmationStatus, PaymentDetailsUpdate};
 use crate::payment::{
 	FundingTxCandidate, PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus,
 	PendingPaymentDetails, TransactionType,
@@ -1398,9 +1399,28 @@ impl Wallet {
 	async fn persist_funding_payment(
 		&self, details: PaymentDetails, candidates: Vec<FundingTxCandidate>,
 	) -> Result<(), Error> {
-		self.payment_store.insert_or_update(details.clone()).await?;
-		let pending = PendingPaymentDetails::new(details, Vec::new(), candidates);
-		self.pending_payment_store.insert_or_update(pending).await?;
+		if !self.payment_store.contains_key(&details.id) {
+			// First time we record this funding payment: store it and index it for graduation.
+			self.payment_store.insert_or_update(details.clone()).await?;
+			let pending = PendingPaymentDetails::new(details, Vec::new(), candidates);
+			self.pending_payment_store.insert_or_update(pending).await?;
+		} else {
+			// An earlier candidate or a racing wallet sync already recorded this payment. Merge only
+			// the classification (`tx_type`) and our contribution figures, which the wallet can't
+			// recompute; the confirmation state is owned by wallet-sync events, so a late
+			// classification must not move it (which would downgrade an already-Confirmed/Succeeded
+			// record). `update` is a no-op when the entry is absent, so the pending index is not
+			// re-created for a payment the graduation path already removed.
+			let update = PaymentDetailsUpdate::funding_reclassification(details);
+			let pending_update = PendingPaymentDetailsUpdate {
+				id: update.id,
+				payment_update: Some(update.clone()),
+				conflicting_txids: None,
+				candidates,
+			};
+			self.payment_store.update(update).await?;
+			self.pending_payment_store.update(pending_update).await?;
+		}
 		Ok(())
 	}