From 6173897bbd85d20888f29aca6b465a09938fea56 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 22 Sep 2026 00:01:35 -0700 Subject: [PATCH 1/2] Keep the replaced txid mapped when bumping a payment's fee again Bumping the same on-chain payment twice left the first replacement unmapped. The payment id is derived from the original txid and each bump sets the payment's txid to the newest one, so an earlier replacement is found only through the pending-store entry's conflicting txids. The bump never added the replaced txid to that list, so wallet sync's TxReplaced event for an earlier replacement found no payment, logged an error, and skipped it. The list stayed incomplete until the newest transaction was itself replaced or the earlier one confirmed, when the TxReplaced event for the newest one listed every conflict and repaired it. The bump now records the replaced txid in the pending-store entry, so the event resolves to the payment and the error log stops. Developed with assistance from Claude Code. Co-Authored-By: Claude Fable 5.1 --- src/wallet/mod.rs | 114 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 111 insertions(+), 3 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 13a8ef4e00..b3cb26b4fd 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2260,8 +2260,6 @@ impl Wallet { 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| { @@ -2269,8 +2267,24 @@ impl Wallet { Error::PersistenceFailed })?; + // Wallet sync maps a replaced transaction to its payment with `find_payment_by_txid`. From + // the second bump on, the replaced `txid` matches only through the pending-store entry's + // `conflicting_txids`. A non-empty list in an update replaces the stored one, so extend it + // rather than writing just `txid`. + let mut conflicting_txids = self + .pending_payment_store + .get(&payment_id) + .await? + .map(|pending| pending.conflicting_txids) + .unwrap_or_default(); + if !conflicting_txids.contains(&txid) { + conflicting_txids.push(txid); + } + let pending_payment = + self.create_pending_payment_from_tx(new_payment.clone(), conflicting_txids); + self.payment_store.insert_or_update(new_payment).await?; - self.pending_payment_store.insert_or_update(pending_payment_store).await?; + self.pending_payment_store.insert_or_update(pending_payment).await?; self.broadcaster.broadcast_unclassified_transaction(fee_bumped_tx); @@ -3972,6 +3986,100 @@ mod tests { assert_eq!(wallet.find_payment_by_txid(txid2).await.unwrap(), Some(payment_id)); } + /// A second bump must add the replaced txid to the pending-store entry's `conflicting_txids`. + /// The payment id is derived from the first txid and the payment's txid is now the newest + /// one, so wallet sync's `TxReplaced` event for an earlier replacement resolves the payment + /// only through that list. Without it, the event finds no payment and is skipped. + #[allow(deprecated)] + #[tokio::test] + async fn bump_fee_rbf_keeps_replaced_txid_mapped() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(store, false).await; + + // A confirmed output funds the wallet... + { + let mut locked_wallet = wallet.inner.lock().unwrap(); + let funding_tx = Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: LockTime::ZERO, + input: Vec::new(), + output: vec![TxOut { + value: Amount::from_sat(200_000), + script_pubkey: locked_wallet + .reveal_next_address(KeychainKind::External) + .address + .script_pubkey(), + }], + }; + let funding_txid = funding_tx.compute_txid(); + let block_id = BlockId { + height: locked_wallet.latest_checkpoint().height() + 1, + hash: bitcoin::BlockHash::from_byte_array([42; 32]), + }; + let mut tx_update = TxUpdate::default(); + tx_update.txs = vec![Arc::new(funding_tx)]; + tx_update.anchors = + [(ConfirmationBlockTime { block_id, confirmation_time: 1 }, funding_txid)].into(); + let chain = CheckPoint::from_block_ids([ + locked_wallet.latest_checkpoint().block_id(), + block_id, + ]) + .unwrap(); + locked_wallet + .apply_update(Update { tx_update, chain: Some(chain), ..Default::default() }) + .unwrap(); + } + + // ...which the wallet spends in a replaceable payment sitting in the mempool. This + // transaction (B) has already replaced the original (A). + let tx_b = { + let mut locked_wallet = wallet.inner.lock().unwrap(); + let recipient = ScriptBuf::new_p2wpkh(&WPubkeyHash::from_byte_array([0x42u8; 20])); + let mut builder = locked_wallet.build_tx(); + builder + .add_recipient(recipient, Amount::from_sat(100_000)) + .fee_rate(FeeRate::from_sat_per_kwu(500)); + let mut psbt = builder.finish().unwrap(); + assert!(locked_wallet.sign(&mut psbt, SignOptions::default()).unwrap()); + let tx = psbt.extract_tx().unwrap(); + locked_wallet.apply_unconfirmed_txs([(tx.clone(), 1)]); + tx + }; + let txid_b = tx_b.compute_txid(); + + // The stores hold what the first bump (A -> B) and its sync left behind: the payment id + // is derived from A, the payment's txid is B, and `conflicting_txids` holds A. + let txid_a = Txid::from_byte_array([0xaa; 32]); + let payment_id = PaymentId(txid_a.to_byte_array()); + let details = PaymentDetails::new( + payment_id, + PaymentKind::Onchain { + txid: txid_b, + status: ConfirmationStatus::Unconfirmed, + tx_type: None, + }, + Some(100_000_000), + Some(1_000), + PaymentDirection::Outbound, + PaymentStatus::Pending, + ); + wallet.payment_store.insert_or_update(details.clone()).await.unwrap(); + let entry = PendingPaymentDetails::new(details, vec![txid_a], Vec::new()); + wallet.pending_payment_store.insert_or_update(entry).await.unwrap(); + + // Bump again: B -> C. + let txid_c = wallet.bump_fee_rbf(payment_id, None, 0).await.unwrap(); + + let payment = wallet.payment_store.get(&payment_id).await.unwrap().unwrap(); + assert!(matches!(payment.kind, PaymentKind::Onchain { txid, .. } if txid == txid_c)); + + // `PaymentId(txid_b)` is not the payment id and B is no longer the payment's txid, so B + // must be in `conflicting_txids` for wallet sync to find the payment. + let entry = wallet.pending_payment_store.get(&payment_id).await.unwrap().unwrap(); + assert_eq!(entry.conflicting_txids, vec![txid_a, txid_b]); + assert_eq!(wallet.find_payment_by_txid(txid_b).await.unwrap(), Some(payment_id)); + } + /// Removing a payment must also drop its pending-store entry. The entry indexes the /// payment's txids (current, conflicting, and candidates), so leaving it behind keeps /// resolving those txids to the removed record — routing later wallet events to a payment From a08f1fd5243e0d29be8478d3ac88c7c22cf5521a Mon Sep 17 00:00:00 2001 From: ram0verflow Date: Sat, 3 Oct 2026 08:26:07 +0530 Subject: [PATCH 2/2] Apply RBF replacements to the wallet before sync bump_fee_rbf updated the payment store but not the BDK wallet, so a second bump before sync failed with InvalidPaymentId (or panicked on debug). Apply the replacement to the wallet immediately, with a seen-at after the replaced round. Once the bump applies the replacement to the wallet, sync no longer emits TxReplaced for the replaced transaction (BDK derives events from a before/after diff of the canonical set), so nothing else populates conflicting_txids after a bump until a later eviction or confirmation flips the canonical set. Fixes #1117. AI assistance: OpenAI Codex, Claude Code. --- src/wallet/mod.rs | 17 +++++++++++++++ tests/integration_tests_rust.rs | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index b3cb26b4fd..bd9c2020d6 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2260,6 +2260,23 @@ impl Wallet { ConfirmationStatus::Unconfirmed, ); + let seen_at = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + let previous_seen_at = locked_wallet + .tx_details(txid) + .and_then(|details| match details.chain_position { + bdk_chain::ChainPosition::Unconfirmed { last_seen, .. } => last_seen, + _ => None, + }) + .unwrap_or(0); + // BDK keeps the conflicting tx with the latest last-seen and breaks ties by txid, so a bump + // in the same second as the replaced round must still be seen strictly after it. + locked_wallet.apply_unconfirmed_txs([( + fee_bumped_tx.clone(), + seen_at.max(previous_seen_at.saturating_add(1)), + )]); let change_set = locked_wallet.take_staged().unwrap_or_default(); drop(locked_wallet); locked_persister.persist_changeset(change_set).await.map_err(|e| { diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 83e80c1048..7bfe673c4a 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -4667,6 +4667,43 @@ async fn fs_store_persistence_backwards_compatibility() { node_new.stop().unwrap(); } +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn onchain_fee_bump_rbf_twice_before_sync() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = random_chain_source(&bitcoind, &electrsd); + let (node_a, node_b) = setup_two_nodes(&chain_source, false, false); + + let addr_a = node_a.onchain_payment().new_address().unwrap(); + let addr_b = node_b.onchain_payment().new_address().unwrap(); + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![addr_a.clone(), addr_b], + Amount::from_sat(500_000), + ) + .await; + node_b.sync_wallets().unwrap(); + let txid = node_b.onchain_payment().send_to_address(&addr_a, 100_000, None).unwrap(); + wait_for_tx(&electrsd.client, txid).await; + node_b.sync_wallets().unwrap(); + let payment_id = PaymentId(txid.to_byte_array()); + let first_fee = node_b.payment(&payment_id).unwrap().unwrap().fee_paid_msat.unwrap(); + + let first_replacement = node_b.onchain_payment().bump_fee_rbf(payment_id, None).unwrap(); + let second_replacement = node_b.onchain_payment().bump_fee_rbf(payment_id, None).unwrap(); + assert_ne!(first_replacement, second_replacement); + let payment = node_b.payment(&payment_id).unwrap().unwrap(); + assert!(payment.fee_paid_msat.unwrap() > first_fee); + assert!( + matches!(payment.kind, PaymentKind::Onchain { txid, .. } if txid == second_replacement) + ); + wait_for_tx(&electrsd.client, second_replacement).await; + node_b.sync_wallets().unwrap(); + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_b.sync_wallets().unwrap(); + assert_eq!(node_b.payment(&payment_id).unwrap().unwrap().status, PaymentStatus::Succeeded); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn onchain_fee_bump_rbf() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();