bump_fee_rbf updates payment store before wallet sees replacement #1117

Summary

OnchainPayment::bump_fee_rbf writes the replacement payment to the payment store and broadcasts, but does not apply the replacement transaction to the BDK wallet. The wallet only learns of it on the next chain sync. During that window the payment store's txid for the payment is one the wallet has never seen. A second bump_fee_rbf on the same payment in that window looks up the new txid in the wallet, finds nothing, and:

• on a release build returns Error::InvalidPaymentId for a well-formed request, and
• on a debug build trips a debug_assert! and panics.

Code

At f375e4d:

• The replacement is built, signed, and the payment store is updated before broadcast, with no apply_unconfirmed_txs on the wallet:
  

  ldk-node/src/wallet/mod.rs

  Lines 2257 to 2278 in f375e4d

  	let new_payment = self.create_payment_from_tx(
  	&locked_wallet,
  	new_txid,
  	payment.id,
  	&fee_bumped_tx,
  	PaymentStatus::Pending,
  	ConfirmationStatus::Unconfirmed,
  	);
  	let pending_payment_store =
  	self.create_pending_payment_from_tx(new_payment.clone(), Vec::new());
  	let change_set = locked_wallet.take_staged().unwrap_or_default();
  	drop(locked_wallet);
  	locked_persister.persist_changeset(change_set).await.map_err(|e| {
  	log_error!(self.logger, "Failed to persist wallet after fee bump of {}: {}", txid, e);
  	Error::PersistenceFailed
  	})?;
  	self.payment_store.insert_or_update(new_payment).await?;
  	self.pending_payment_store.insert_or_update(pending_payment_store).await?;
  	self.broadcaster.broadcast_unclassified_transaction(fee_bumped_tx);

• On entry, the current txid is asserted to exist in the wallet, then looked up again for the error path:
  

  ldk-node/src/wallet/mod.rs

  Lines 2093 to 2103 in f375e4d

  	debug_assert!(
  	locked_wallet.tx_details(txid).is_some(),
  	"Transaction {} expected in wallet but not found",
  	txid,
  	);
  	let old_tx = locked_wallet
  	.tx_details(txid)
  	.ok_or_else(|| {
  	log_error!(self.logger, "Transaction {} not found in wallet", txid);
  	Error::InvalidPaymentId
  	})?

• The wallet only ingests unconfirmed transactions via apply_mempool_txs during sync:
  

  ldk-node/src/wallet/mod.rs

  Lines 267 to 298 in f375e4d

  	pub(crate) async fn apply_mempool_txs(
  	&self, unconfirmed_txs: Vec<(Transaction, u64)>, evicted_txids: Vec<(Txid, u64)>,
  	) -> Result<(), Error> {
  	if unconfirmed_txs.is_empty() && evicted_txids.is_empty() {
  	return Ok(());
  	}
  	let mut locked_persister = self.persister.lock().await;
  	let events = {
  	let mut locked_wallet = self.inner.lock().expect("lock");
  	locked_wallet
  	.events_helper(|wallet| -> Result<(), std::convert::Infallible> {
  	wallet.apply_unconfirmed_txs(unconfirmed_txs);
  	wallet.apply_evicted_txs(evicted_txids);
  	Ok(())
  	})
  	.expect("applying mempool updates cannot fail")
  	};
  	self.update_payment_store(events).await.map_err(|e| {
  	log_error!(self.logger, "Failed to update payment store: {}", e);
  	Error::PersistenceFailed
  	})?;
  	let change_set = self.inner.lock().expect("lock").take_staged().unwrap_or_default();
  	locked_persister.persist_changeset(change_set).await.map_err(|e| {
  	log_error!(self.logger, "Failed to persist wallet: {}", e);
  	Error::PersistenceFailed
  	})?;
  	Ok(())
  	}

Window

• Bitcoind chain source: up to the 2 second poll (CHAIN_POLLING_INTERVAL_SECS, src/chain/bitcoind.rs:49).
• Esplora and Electrum: up to onchain_wallet_sync_interval_secs, default 80 seconds (src/config.rs:28).

Reproduce

1. send_to_address(...) and wait until the payment appears in list_payments.
2. bump_fee_rbf(payment_id, None) succeeds and returns txid_2.
3. Immediately call bump_fee_rbf(payment_id, None) again, before the next sync.

Step 3 returns InvalidPaymentId on release, or panics with Transaction <txid_2> expected in wallet but not found on debug.

Note that the first bump is not affected: send_to_address does not write a payment record, so the payment only becomes visible after a sync has already applied the original transaction to the wallet. Only bump_fee_rbf creates the asymmetry between the two stores.

Why this matters

The payment store and the wallet are two views of the same state and should not disagree for a sync interval. The debug_assert! encodes "the wallet has ingested our last broadcast" as an invariant, but it is only eventually true. Callers have to poll latest_onchain_wallet_sync_timestamp between consecutive bumps to use the API safely, which is what the ldk-server end-to-end tests currently do (wait_for_wallet_sync in lightningdevkit/ldk-server#278).

Possible fixes

1. In bump_fee_rbf, apply the replacement to the wallet before take_staged, so the wallet changeset and the payment store update in the same critical section. BDK's apply_unconfirmed_txs([(tx, now)]) is the intended hook for this. Doing the same in send_to_address would make the two paths consistent, but is not needed to close this race.
2. Alternatively, drop the debug_assert! and keep the explicit error path, so a bump inside the window fails cleanly rather than aborting. This does not remove the window, only the panic.

Option 1 is preferable since it removes the inconsistency rather than papering over it.

Found while reviewing lightningdevkit/ldk-server#278, with AI assistance from Claude Code.