From cc9f907182e88661b3ba9ac9fc9314b876eed1a7 Mon Sep 17 00:00:00 2001 From: moisesPomilio <93723302+moisesPompilio@users.noreply.github.com> Date: Tue, 9 Sep 2025 17:09:18 -0300 Subject: [PATCH 1/4] nit: introduce setup_node and rename setup_node to setup_node_from_config Introduce setup_node for clearer node creation. Rename old setup_node to setup_node_from_config for custom configs. --- tests/common/mod.rs | 11 ++++++++--- tests/integration_tests_rust.rs | 32 +++++++++++++------------------- tests/reorg_test.rs | 17 +++++------------ 3 files changed, 26 insertions(+), 34 deletions(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 817d0edc5..3c020719e 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -285,7 +285,7 @@ pub(crate) fn setup_two_nodes( ) -> (TestNode, TestNode) { println!("== Node A =="); let config_a = random_config(anchor_channels); - let node_a = setup_node(chain_source, config_a, None); + let node_a = setup_node_from_config(chain_source, config_a, None); println!("\n== Node B =="); let mut config_b = random_config(anchor_channels); @@ -301,11 +301,16 @@ pub(crate) fn setup_two_nodes( .trusted_peers_no_reserve .push(node_a.node_id()); } - let node_b = setup_node(chain_source, config_b, None); + let node_b = setup_node_from_config(chain_source, config_b, None); (node_a, node_b) } -pub(crate) fn setup_node( +pub(crate) fn setup_node(chain_source: &TestChainSource, anchor_channels: bool) -> TestNode { + let config = random_config(anchor_channels); + setup_node_from_config(chain_source, config, None) +} + +pub(crate) fn setup_node_from_config( chain_source: &TestChainSource, config: TestConfig, seed_bytes: Option>, ) -> TestNode { setup_node_for_async_payments(chain_source, config, seed_bytes, None) diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 64a78e11b..de9068d15 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -23,7 +23,7 @@ use common::{ generate_blocks_and_wait, open_channel, open_channel_push_amt, premine_and_distribute_funds, premine_blocks, prepare_rbf, random_config, random_listening_addresses, setup_bitcoind_and_electrsd, setup_builder, setup_node, setup_node_for_async_payments, - setup_two_nodes, wait_for_tx, TestChainSource, TestSyncStore, + setup_node_from_config, setup_two_nodes, wait_for_tx, TestChainSource, TestSyncStore, }; use ldk_node::config::{AsyncPaymentsRole, EsploraSyncConfig}; use ldk_node::liquidity::LSPS2ServiceConfig; @@ -596,7 +596,8 @@ fn onchain_wallet_recovery() { let seed_bytes = vec![42u8; 64]; let original_config = random_config(true); - let original_node = setup_node(&chain_source, original_config, Some(seed_bytes.clone())); + let original_node = + setup_node_from_config(&chain_source, original_config, Some(seed_bytes.clone())); let premine_amount_sat = 100_000; @@ -635,7 +636,7 @@ fn onchain_wallet_recovery() { // Now we start from scratch, only the seed remains the same. let recovered_config = random_config(true); - let recovered_node = setup_node(&chain_source, recovered_config, Some(seed_bytes)); + let recovered_node = setup_node_from_config(&chain_source, recovered_config, Some(seed_bytes)); recovered_node.sync_wallets().unwrap(); assert_eq!( @@ -686,18 +687,11 @@ fn run_rbf_test(is_insert_block: bool) { let chain_source_electrsd = TestChainSource::Electrum(&electrsd); let chain_source_esplora = TestChainSource::Esplora(&electrsd); - macro_rules! config_node { - ($chain_source: expr, $anchor_channels: expr) => {{ - let config_a = random_config($anchor_channels); - let node = setup_node(&$chain_source, config_a, None); - node - }}; - } let anchor_channels = false; let nodes = vec![ - config_node!(chain_source_electrsd, anchor_channels), - config_node!(chain_source_bitcoind, anchor_channels), - config_node!(chain_source_esplora, anchor_channels), + setup_node(&chain_source_bitcoind, anchor_channels), + setup_node(&chain_source_electrsd, anchor_channels), + setup_node(&chain_source_esplora, anchor_channels), ]; let (bitcoind, electrs) = (&bitcoind.client, &electrsd.client); @@ -807,7 +801,7 @@ fn sign_verify_msg() { let (_bitcoind, electrsd) = setup_bitcoind_and_electrsd(); let config = random_config(true); let chain_source = TestChainSource::Esplora(&electrsd); - let node = setup_node(&chain_source, config, None); + let node = setup_node_from_config(&chain_source, config, None); // Tests arbitrary message signing and later verification let msg = "OK computer".as_bytes(); @@ -1174,7 +1168,7 @@ fn async_payment() { config_receiver.node_config.node_alias = None; config_receiver.log_writer = TestLogWriter::Custom(Arc::new(MultiNodeLogger::new("receiver ".to_string()))); - let node_receiver = setup_node(&chain_source, config_receiver, None); + let node_receiver = setup_node_from_config(&chain_source, config_receiver, None); let address_sender = node_sender.onchain_payment().new_address().unwrap(); let address_sender_lsp = node_sender_lsp.onchain_payment().new_address().unwrap(); @@ -1296,8 +1290,8 @@ fn test_node_announcement_propagation() { config_b.node_config.listening_addresses = Some(node_b_listening_addresses.clone()); config_b.node_config.announcement_addresses = None; - let node_a = setup_node(&chain_source, config_a, None); - let node_b = setup_node(&chain_source, config_b, None); + let node_a = setup_node_from_config(&chain_source, config_a, None); + let node_b = setup_node_from_config(&chain_source, config_b, None); let address_a = node_a.onchain_payment().new_address().unwrap(); let premine_amount_sat = 5_000_000; @@ -1753,7 +1747,7 @@ fn facade_logging() { config.log_writer = TestLogWriter::LogFacade; println!("== Facade logging starts =="); - let _node = setup_node(&chain_source, config, None); + let _node = setup_node_from_config(&chain_source, config, None); assert!(!logger.retrieve_logs().is_empty()); for (_, entry) in logger.retrieve_logs().iter().enumerate() { @@ -1834,6 +1828,6 @@ async fn drop_in_async_context() { let seed_bytes = vec![42u8; 64]; let config = random_config(true); - let node = setup_node(&chain_source, config, Some(seed_bytes)); + let node = setup_node_from_config(&chain_source, config, Some(seed_bytes)); node.stop().unwrap(); } diff --git a/tests/reorg_test.rs b/tests/reorg_test.rs index 03ace908f..5f832e964 100644 --- a/tests/reorg_test.rs +++ b/tests/reorg_test.rs @@ -9,8 +9,8 @@ use proptest::proptest; use crate::common::{ expect_event, generate_blocks_and_wait, invalidate_blocks, open_channel, - premine_and_distribute_funds, random_config, setup_bitcoind_and_electrsd, setup_node, - wait_for_outpoint_spend, TestChainSource, + premine_and_distribute_funds, setup_bitcoind_and_electrsd, setup_node, wait_for_outpoint_spend, + TestChainSource, }; proptest! { @@ -23,18 +23,11 @@ proptest! { let chain_source_electrsd = TestChainSource::Electrum(&electrsd); let chain_source_esplora = TestChainSource::Esplora(&electrsd); - macro_rules! config_node { - ($chain_source: expr, $anchor_channels: expr) => {{ - let config_a = random_config($anchor_channels); - let node = setup_node(&$chain_source, config_a, None); - node - }}; - } let anchor_channels = true; let nodes = vec![ - config_node!(chain_source_electrsd, anchor_channels), - config_node!(chain_source_bitcoind, anchor_channels), - config_node!(chain_source_esplora, anchor_channels), + setup_node(&chain_source_electrsd, anchor_channels), + setup_node(&chain_source_bitcoind, anchor_channels), + setup_node(&chain_source_esplora, anchor_channels), ]; let (bitcoind, electrs) = (&bitcoind.client, &electrsd.client); From fb1c52c9c1d4b51e9f81bdcfb4c98b43869249e0 Mon Sep 17 00:00:00 2001 From: moisesPomilio <93723302+moisesPompilio@users.noreply.github.com> Date: Mon, 22 Sep 2025 17:30:01 -0300 Subject: [PATCH 2/4] Add manual block generation and direct transaction insertion for RBF testing - Introduce `generate_block_and_insert_transactions` to manually mine a block with arbitrary transactions. - Update `bump_fee_and_broadcast` to optionally insert transactions directly into a block (bypassing the mempool) when `is_insert_block` is true. - Add integration test to insert the original (pre-RBF) transaction into a block instead of the RBF, to cover scenarios where the original transaction is confirmed and the RBF is not. --- tests/common/mod.rs | 150 ++++++++++++++++++++++++++++++-- tests/integration_tests_rust.rs | 92 +++++++++++++++----- 2 files changed, 212 insertions(+), 30 deletions(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 3c020719e..4ef27a7c1 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -16,15 +16,22 @@ use std::env; use std::future::Future; use std::path::PathBuf; use std::pin::Pin; +use std::str::FromStr; use std::sync::{Arc, RwLock}; use std::time::Duration; -use bitcoin::hashes::hex::FromHex; -use bitcoin::hashes::sha256::Hash as Sha256; -use bitcoin::hashes::Hash; +use bitcoin::absolute::LockTime; +use bitcoin::block::{Header, Version as BlockVersion}; +use bitcoin::hashes::{hex::FromHex, sha256::Hash as Sha256, Hash}; +use bitcoin::merkle_tree::calculate_root; +use bitcoin::opcodes::all::OP_RETURN; +use bitcoin::script::Builder as BuilderScriptBitcoin; +use bitcoin::transaction::Version; use bitcoin::{ - Address, Amount, Network, OutPoint, ScriptBuf, Sequence, Transaction, Txid, Witness, + Address, Amount, Block, BlockHash, CompactTarget, Network, OutPoint, ScriptBuf, Sequence, + Transaction, TxMerkleNode, Txid, Witness, Wtxid, }; +use electrsd::corepc_node::TemplateRules; use electrsd::corepc_node::{Client as BitcoindClient, Node as BitcoinD}; use electrsd::{corepc_node, ElectrsD}; use electrum_client::ElectrumApi; @@ -394,6 +401,133 @@ pub(crate) fn setup_node_for_async_payments( node } +pub(crate) fn generate_block_and_insert_transactions( + bitcoind: &BitcoindClient, electrs: &E, txs: &[Transaction], +) { + let _ = bitcoind.create_wallet("ldk_node_test"); + let _ = bitcoind.load_wallet("ldk_node_test"); + let blockchain_info = bitcoind.get_blockchain_info().expect("failed to get blockchain info"); + let cur_height = blockchain_info.blocks; + let address = bitcoind.new_address().expect("failed to get new address"); + + let request_block_template = + corepc_node::TemplateRequest { rules: vec![TemplateRules::Segwit] }; + let bt = + bitcoind.get_block_template(&request_block_template).expect("failed to get block template"); + + // === BIP 141: Witness Commitment Calculation === + let witness_root = if txs.is_empty() { + TxMerkleNode::all_zeros() + } else { + // BIP 141: Create wtxid list starting with all-zeros for coinbase + let wtxids: Vec = std::iter::once(Wtxid::all_zeros()) + .chain(txs.iter().map(|tx| tx.compute_wtxid())) + .collect(); + + calculate_root(wtxids.into_iter()) + .map(|root| TxMerkleNode::from_byte_array(root.to_byte_array())) + .unwrap() + }; + + // BIP 141: Witness reserved value (32 zero bytes) + let witness_reserved_value = [0u8; 32]; + + // BIP 141: Calculate commitment hash = Double-SHA256(witness root || witness reserved value) + let mut commitment_preimage = Vec::new(); + commitment_preimage.extend_from_slice(&witness_root.to_byte_array()); + commitment_preimage.extend_from_slice(&witness_reserved_value); + let commitment_hash = Sha256::hash(&commitment_preimage); + + // === Coinbase Transaction Construction === + // BIP 141: Coinbase witness contains the witness reserved value + let mut coinbase_witness = Witness::new(); + coinbase_witness.push(&witness_reserved_value); + + // BIP 141: Witness commitment output script + let mut bip_141_data = [0u8; 36]; + bip_141_data[0] = 0xaa; + bip_141_data[1] = 0x21; + bip_141_data[2] = 0xa9; + bip_141_data[3] = 0xed; + bip_141_data[4..].copy_from_slice(&commitment_hash.to_byte_array()); + + // Format: OP_RETURN + OP_PUSHBYTES_36 + 0xaa21a9ed + 32-byte commitment hash + let witness_commitment_script = + BuilderScriptBitcoin::new().push_opcode(OP_RETURN).push_slice(bip_141_data).into_script(); + + // BIP 34: Block height in coinbase input script + let block_height = bt.height; + let height_script = BuilderScriptBitcoin::new() + .push_int(block_height as i64) // BIP 34: block height as first item + .push_int(bdk_chain::bitcoin::secp256k1::rand::random()) // Random nonce for uniqueness + .into_script(); + + // Do not use the coinbase value from the block template. + // The template may include transactions not actually mined, so fees may be incorrect. + let coinbase_output_value = 1_250_000_000; + + let coinbase_tx = Transaction { + version: Version::ONE, + lock_time: LockTime::from_height(0).unwrap(), + input: vec![bitcoin::TxIn { + previous_output: bdk_chain::bitcoin::OutPoint::default(), // Null outpoint for coinbase + script_sig: height_script, // BIP 34: height + random data + sequence: Sequence::default(), + witness: coinbase_witness, // BIP 141: witness reserved value + }], + output: vec![ + // Coinbase reward output + bitcoin::TxOut { + value: Amount::from_sat(coinbase_output_value), + script_pubkey: address.script_pubkey(), + }, + // BIP 141: Witness commitment output (must be last output) + bitcoin::TxOut { value: Amount::ZERO, script_pubkey: witness_commitment_script }, + ], + }; + + // === Block Construction === + let bits_hex = bt.bits.clone(); + let bits_vec = Vec::::from_hex(&bits_hex).expect("invalid hex for bits"); + let bits: [u8; 4] = bits_vec.try_into().expect("bits must be 4 bytes"); + let prev_hash_block = BlockHash::from_str(&bt.previous_block_hash).expect("invalid prev hash"); + + let txdata = std::iter::once(coinbase_tx).chain(txs.iter().cloned()).collect::>(); + let mut block = Block { + header: Header { + version: BlockVersion::default(), + prev_blockhash: prev_hash_block, + merkle_root: TxMerkleNode::all_zeros(), // Will be calculated below + time: Ord::max( + bt.min_time, + std::time::UNIX_EPOCH.elapsed().unwrap().as_secs().try_into().unwrap(), + ) as u32, + bits: CompactTarget::from_consensus(u32::from_be_bytes(bits)), + nonce: 0, + }, + txdata, + }; + + block.header.merkle_root = block.compute_merkle_root().expect("must compute"); + + for nonce in 0..=u32::MAX { + block.header.nonce = nonce; + if block.header.target().is_met_by(block.block_hash()) { + break; + } + } + + match bitcoind.submit_block(&block) { + Ok(_) => println!("Generated block 1 manually with {} transactions", txs.len()), + Err(e) => panic!("Failed to submit block: {:?}", e), + } + wait_for_block(electrs, cur_height as usize + 1); + + txs.iter().for_each(|tx| { + wait_for_tx(electrs, tx.compute_txid()); + }); +} + pub(crate) fn generate_blocks_and_wait( bitcoind: &BitcoindClient, electrs: &E, num: usize, ) { @@ -575,11 +709,13 @@ pub(crate) fn bump_fee_and_broadcast( let tx_bytes = Vec::::from_hex(&signed_result.hex).unwrap(); tx = bitcoin::consensus::encode::deserialize::(&tx_bytes).unwrap(); + if is_insert_block { + generate_block_and_insert_transactions(bitcoind, electrs, &[tx.clone()]); + return tx; + } + match bitcoind.send_raw_transaction(&tx) { Ok(res) => { - if is_insert_block { - generate_blocks_and_wait(bitcoind, electrs, 1); - } let new_txid: Txid = res.0.parse().unwrap(); wait_for_tx(electrs, new_txid); return tx; diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index de9068d15..912fb3257 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -20,10 +20,11 @@ use common::{ bump_fee_and_broadcast, distribute_funds_unconfirmed, do_channel_full_cycle, expect_channel_pending_event, expect_channel_ready_event, expect_event, expect_payment_claimable_event, expect_payment_received_event, expect_payment_successful_event, - generate_blocks_and_wait, open_channel, open_channel_push_amt, premine_and_distribute_funds, - premine_blocks, prepare_rbf, random_config, random_listening_addresses, - setup_bitcoind_and_electrsd, setup_builder, setup_node, setup_node_for_async_payments, - setup_node_from_config, setup_two_nodes, wait_for_tx, TestChainSource, TestSyncStore, + generate_block_and_insert_transactions, generate_blocks_and_wait, open_channel, + open_channel_push_amt, premine_and_distribute_funds, premine_blocks, prepare_rbf, + random_config, random_listening_addresses, setup_bitcoind_and_electrsd, setup_builder, + setup_node, setup_node_for_async_payments, setup_node_from_config, setup_two_nodes, + wait_for_tx, TestChainSource, TestSyncStore, }; use ldk_node::config::{AsyncPaymentsRole, EsploraSyncConfig}; use ldk_node::liquidity::LSPS2ServiceConfig; @@ -669,19 +670,24 @@ fn onchain_wallet_recovery() { } #[test] -fn test_rbf_via_mempool() { - run_rbf_test(false); +fn test_rbf_only_in_mempool() { + run_rbf_test(false, false); } #[test] -fn test_rbf_via_direct_block_insertion() { - run_rbf_test(true); +fn test_rbf_direct_block_insertion_rbf_tx() { + run_rbf_test(true, false); +} + +#[test] +fn test_rbf_direct_block_insertion_original_tx() { + run_rbf_test(false, true); } // `is_insert_block`: // - `true`: transaction is mined immediately (no mempool), testing confirmed-Tx handling. // - `false`: transaction stays in mempool until confirmation, testing unconfirmed-Tx handling. -fn run_rbf_test(is_insert_block: bool) { +fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); let chain_source_bitcoind = TestChainSource::BitcoindRpcSync(&bitcoind); let chain_source_electrsd = TestChainSource::Electrum(&electrsd); @@ -724,15 +730,29 @@ fn run_rbf_test(is_insert_block: bool) { }; } + macro_rules! validate_total_onchain_balance { + ($expected_balance_sat: expr) => { + for node in &nodes { + node.sync_wallets().unwrap(); + let balances = node.list_balances(); + assert_eq!(balances.total_onchain_balance_sats, $expected_balance_sat); + } + }; + } + let scripts_buf: HashSet = all_addrs.iter().map(|addr| addr.script_pubkey()).collect(); let mut tx; let mut fee_output_index; - // Modify the output to the nodes + let mut final_amount_sat = 0; + let mut original_tx; + + // Step 1: Bump fee and change output address distribute_funds_all_nodes!(); validate_balances!(amount_sat, false); (tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf); + original_tx = tx.clone(); tx.output.iter_mut().for_each(|output| { if scripts_buf.contains(&output.script_pubkey) { let new_addr = bitcoind.new_address().unwrap(); @@ -740,42 +760,68 @@ fn run_rbf_test(is_insert_block: bool) { } }); bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); - validate_balances!(0, is_insert_block); + if is_insertion_original_tx { + generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]); + } + if is_insertion_original_tx { + final_amount_sat += amount_sat; + } + validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx); - // Not modifying the output scripts, but still bumping the fee. + // Step 2: Bump fee only distribute_funds_all_nodes!(); - validate_balances!(amount_sat, false); + validate_total_onchain_balance!(amount_sat + final_amount_sat); (tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf); + original_tx = tx.clone(); bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); - validate_balances!(amount_sat, is_insert_block); + if is_insertion_original_tx { + generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]); + } + final_amount_sat += amount_sat; + validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx); - let mut final_amount_sat = amount_sat * 2; + // Step 3: Increase output value let value_sat = 21_000; - - // Increase the value of the nodes' outputs distribute_funds_all_nodes!(); + validate_total_onchain_balance!(amount_sat + final_amount_sat); (tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf); + original_tx = tx.clone(); tx.output.iter_mut().for_each(|output| { if scripts_buf.contains(&output.script_pubkey) { output.value = Amount::from_sat(output.value.to_sat() + value_sat); } }); + tx.output[fee_output_index].value -= Amount::from_sat(scripts_buf.len() as u64 * value_sat); bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); - final_amount_sat += value_sat; - validate_balances!(final_amount_sat, is_insert_block); + if is_insertion_original_tx { + generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]); + } + final_amount_sat += amount_sat; + if !is_insertion_original_tx { + final_amount_sat += value_sat; + } + validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx); - // Decreases the value of the nodes' outputs + // Step 4: Decrease output value distribute_funds_all_nodes!(); - final_amount_sat += amount_sat; + validate_total_onchain_balance!(amount_sat + final_amount_sat); (tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf); + original_tx = tx.clone(); tx.output.iter_mut().for_each(|output| { if scripts_buf.contains(&output.script_pubkey) { output.value = Amount::from_sat(output.value.to_sat() - value_sat); } }); + tx.output[fee_output_index].value += Amount::from_sat(scripts_buf.len() as u64 * value_sat); bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); - final_amount_sat -= value_sat; - validate_balances!(final_amount_sat, is_insert_block); + if is_insertion_original_tx { + generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]); + } + final_amount_sat += amount_sat; + if !is_insertion_original_tx { + final_amount_sat -= value_sat; + } + validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx); if !is_insert_block { generate_blocks_and_wait(bitcoind, electrs, 1); From cfabeb2d00be01d4767e10fb230d01fe0fcd7894 Mon Sep 17 00:00:00 2001 From: moisesPomilio <93723302+moisesPompilio@users.noreply.github.com> Date: Mon, 22 Sep 2025 17:37:04 -0300 Subject: [PATCH 3/4] Add property-based reorg test for RBF transactions This test simulates multiple RBF transactions, randomly confirms one, then simulates a reorg and confirms another at random. This ensures the wallet correctly handles cases where any RBF transaction may be confirmed after --- tests/reorg_test.rs | 112 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 104 insertions(+), 8 deletions(-) diff --git a/tests/reorg_test.rs b/tests/reorg_test.rs index 5f832e964..28bd43a02 100644 --- a/tests/reorg_test.rs +++ b/tests/reorg_test.rs @@ -1,20 +1,22 @@ mod common; -use std::collections::HashMap; - -use bitcoin::Amount; +use bitcoin::{Amount, ScriptBuf}; use ldk_node::payment::{PaymentDirection, PaymentKind}; use ldk_node::{Event, LightningBalance, PendingSweepBalance}; -use proptest::prelude::prop; -use proptest::proptest; +use proptest::strategy::Strategy; +use proptest::strategy::ValueTree; +use proptest::{prelude::prop, proptest}; +use std::collections::{HashMap, HashSet}; use crate::common::{ - expect_event, generate_blocks_and_wait, invalidate_blocks, open_channel, - premine_and_distribute_funds, setup_bitcoind_and_electrsd, setup_node, wait_for_outpoint_spend, - TestChainSource, + bump_fee_and_broadcast, distribute_funds_unconfirmed, expect_event, + generate_block_and_insert_transactions, generate_blocks_and_wait, invalidate_blocks, + open_channel, premine_and_distribute_funds, premine_blocks, prepare_rbf, + setup_bitcoind_and_electrsd, setup_node, wait_for_outpoint_spend, TestChainSource, }; proptest! { #![proptest_config(proptest::test_runner::Config::with_cases(5))] + #[test] fn reorg_test(reorg_depth in 1..=6usize, force_close in prop::bool::ANY) { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); @@ -185,4 +187,98 @@ proptest! { assert_eq!(node.next_event(), None); }); } + + #[test] + fn test_reorg_rbf( + reorg_depth in 2..=5usize, + quantity_rbf in 2..=6usize, + ) { + let mut runner = proptest::test_runner::TestRunner::default(); + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + + let chain_source_bitcoind = TestChainSource::BitcoindRpcSync(&bitcoind); + let chain_source_electrsd = TestChainSource::Electrum(&electrsd); + let chain_source_esplora = TestChainSource::Esplora(&electrsd); + + let anchor_channels = true; + let nodes = vec![ + setup_node(&chain_source_bitcoind, anchor_channels), + setup_node(&chain_source_electrsd, anchor_channels), + setup_node(&chain_source_esplora, anchor_channels), + ]; + + let (bitcoind, electrs) = (&bitcoind.client, &electrsd.client); + + let mut amount_sat = 2_100_000; + let all_addrs = + nodes.iter().map(|node| node.onchain_payment().new_address().unwrap()).collect::>(); + let scripts_buf: HashSet = + all_addrs.iter().map(|addr| addr.script_pubkey()).collect(); + + premine_blocks(bitcoind, electrs); + generate_blocks_and_wait(bitcoind, electrs, reorg_depth); + let txid = distribute_funds_unconfirmed(bitcoind, electrs, all_addrs, Amount::from_sat(amount_sat)); + + let mut is_spendable = false; + macro_rules! verify_wallet_balances_and_transactions { + ($expected_balance_sat: expr, $expected_size_list_payments: expr) => { + let spend_balance = if is_spendable { $expected_balance_sat } else { 0 }; + for node in &nodes { + node.sync_wallets().unwrap(); + let balances = node.list_balances(); + assert_eq!(balances.total_onchain_balance_sats, $expected_balance_sat); + assert_eq!(balances.spendable_onchain_balance_sats, spend_balance); + } + }; + } + + let mut tx_to_amount = HashMap::new(); + let (mut tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf); + tx_to_amount.insert(tx.clone(), amount_sat); + generate_block_and_insert_transactions(bitcoind, electrs, &[]); + verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments); + for _ in 0..quantity_rbf { + let is_alterable_value = prop::bool::ANY.new_tree(&mut runner).unwrap().current(); + if is_alterable_value { + let value_sat = (5000..20000u64).new_tree(&mut runner).unwrap().current(); + let is_acrent_value = prop::bool::ANY.new_tree(&mut runner).unwrap().current(); + amount_sat = if is_acrent_value {amount_sat + value_sat} else {amount_sat - value_sat}; + for output in &mut tx.output { + if scripts_buf.contains(&output.script_pubkey) { + output.value = Amount::from_sat(amount_sat); + } + } + let fee_sat = Amount::from_sat(scripts_buf.len() as u64 * value_sat); + if is_acrent_value { + tx.output[fee_output_index].value -= fee_sat; + } else { + tx.output[fee_output_index].value += fee_sat; + } + + } + + tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_spendable); + tx_to_amount.insert(tx.clone(), amount_sat); + + verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments); + } + + is_spendable = true; + let index_tx_confirm = (0..tx_to_amount.len() - 1).new_tree(&mut runner).unwrap().current(); + let tx_to_confirm = tx_to_amount.iter().nth(index_tx_confirm).unwrap(); + generate_block_and_insert_transactions(bitcoind, electrs, &[tx_to_confirm.0.clone()]); + generate_blocks_and_wait(bitcoind, electrs, reorg_depth - 1); + amount_sat = *tx_to_confirm.1; + verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments); + + invalidate_blocks(bitcoind, reorg_depth); + generate_block_and_insert_transactions(bitcoind, electrs, &[]); + + let index_tx_confirm = (0..tx_to_amount.len() - 1).new_tree(&mut runner).unwrap().current(); + let tx_to_confirm = tx_to_amount.iter().nth(index_tx_confirm).unwrap(); + generate_block_and_insert_transactions(bitcoind, electrs, &[tx_to_confirm.0.clone()]); + amount_sat = *tx_to_confirm.1; + generate_blocks_and_wait(bitcoind, electrs, 5); + verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments); + } } From ab6990e441aa234b51550043454a6489ae7c204e Mon Sep 17 00:00:00 2001 From: moisesPomilio <93723302+moisesPompilio@users.noreply.github.com> Date: Tue, 23 Sep 2025 09:59:36 -0300 Subject: [PATCH 4/4] Fix pending payments not confirmed due to invalidation or mempool removal Pending payments that become invalid or are removed from the mempool are now marked as failed. RBF tests were updated to check that only confirmed transactions succeed and all others are failed.only the actually confirmed transactions have the correct --- src/wallet/mod.rs | 30 +++++++++++++++++++++++- tests/integration_tests_rust.rs | 41 ++++++++++++++++++++++++++++----- tests/reorg_test.rs | 35 ++++++++++++++++++++++------ 3 files changed, 92 insertions(+), 14 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 6d79fe02f..d21c388c8 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -46,7 +46,7 @@ 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::store::{ConfirmationStatus, PaymentDetailsUpdate}; use crate::payment::{PaymentDetails, PaymentDirection, PaymentStatus}; use crate::types::{Broadcaster, PaymentStore}; use crate::Error; @@ -153,6 +153,7 @@ impl Wallet { fn update_payment_store<'a>( &self, locked_wallet: &'a mut PersistedWallet, ) -> Result<(), Error> { + let mut unconfirmed_txid = Vec::new(); for wtx in locked_wallet.transactions() { let id = PaymentId(wtx.tx_node.txid.to_byte_array()); let txid = wtx.tx_node.txid; @@ -164,6 +165,7 @@ impl Wallet { { PaymentStatus::Succeeded } else { + unconfirmed_txid.push(id); PaymentStatus::Pending }; let confirmation_status = ConfirmationStatus::Confirmed { @@ -174,6 +176,7 @@ impl Wallet { (payment_status, confirmation_status) }, bdk_chain::ChainPosition::Unconfirmed { .. } => { + unconfirmed_txid.push(id); (PaymentStatus::Pending, ConfirmationStatus::Unconfirmed) }, }; @@ -221,6 +224,31 @@ impl Wallet { self.payment_store.insert_or_update(payment)?; } + self.update_transactions_unconfirmed(&unconfirmed_txid)?; + + Ok(()) + } + + fn update_transactions_unconfirmed( + &self, canonical_payment: &[PaymentId], + ) -> Result<(), Error> { + let failed_payment = self.payment_store.list_filter(|payment| { + payment.status == PaymentStatus::Pending && !canonical_payment.contains(&payment.id) + }); + if failed_payment.is_empty() { + return Ok(()); + } + + for failed_payment in &failed_payment { + let id = failed_payment.id; + let payment_update = PaymentDetailsUpdate { + id, + status: Some(PaymentStatus::Failed), + ..PaymentDetailsUpdate::new(id) + }; + self.payment_store.update(&payment_update)?; + } + Ok(()) } diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 912fb3257..979abb425 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -748,6 +748,9 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) { let mut final_amount_sat = 0; let mut original_tx; + let mut array_original_txid = Vec::new(); + let mut array_rbf_txid = Vec::new(); + // Step 1: Bump fee and change output address distribute_funds_all_nodes!(); validate_balances!(amount_sat, false); @@ -767,18 +770,21 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) { final_amount_sat += amount_sat; } validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx); + array_original_txid.push(original_tx.compute_txid()); // Step 2: Bump fee only distribute_funds_all_nodes!(); validate_total_onchain_balance!(amount_sat + final_amount_sat); (tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf); original_tx = tx.clone(); - bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); + let rbf_tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); if is_insertion_original_tx { generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]); } final_amount_sat += amount_sat; validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx); + array_original_txid.push(original_tx.compute_txid()); + array_rbf_txid.push(rbf_tx.compute_txid()); // Step 3: Increase output value let value_sat = 21_000; @@ -792,7 +798,7 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) { } }); tx.output[fee_output_index].value -= Amount::from_sat(scripts_buf.len() as u64 * value_sat); - bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); + let rbf_tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); if is_insertion_original_tx { generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]); } @@ -801,6 +807,8 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) { final_amount_sat += value_sat; } validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx); + array_original_txid.push(original_tx.compute_txid()); + array_rbf_txid.push(rbf_tx.compute_txid()); // Step 4: Decrease output value distribute_funds_all_nodes!(); @@ -813,7 +821,7 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) { } }); tx.output[fee_output_index].value += Amount::from_sat(scripts_buf.len() as u64 * value_sat); - bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); + let rbf_tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block); if is_insertion_original_tx { generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]); } @@ -822,10 +830,31 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) { final_amount_sat -= value_sat; } validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx); + array_original_txid.push(original_tx.compute_txid()); + array_rbf_txid.push(rbf_tx.compute_txid()); - if !is_insert_block { - generate_blocks_and_wait(bitcoind, electrs, 1); - validate_balances!(final_amount_sat, true); + // Confirm transaction + generate_blocks_and_wait(bitcoind, electrs, 6); + validate_balances!(final_amount_sat, true); + + // Validate the list of payments: all must be succeeded and match the confirmed on-chain txids. + let confirmed_onchain_txids = + if is_insertion_original_tx { array_original_txid } else { array_rbf_txid }; + for node in &nodes { + let all_payments: Vec = node.list_payments(); + let pending: Vec<_> = + all_payments.iter().filter(|p| p.status == PaymentStatus::Pending).collect(); + + assert!(pending.is_empty()); + + let succeeded: Vec<_> = + all_payments.iter().filter(|p| p.status == PaymentStatus::Succeeded).collect(); + assert_eq!(succeeded.len(), confirmed_onchain_txids.len()); + for p in succeeded { + assert_eq!(p.direction, PaymentDirection::Inbound); + assert!(matches!(p.kind, PaymentKind::Onchain { txid: _, .. })); + assert!(confirmed_onchain_txids.contains(&bitcoin::Txid::from_slice(&p.id.0).unwrap())); + } } // Check if it is possible to send all funds from the node diff --git a/tests/reorg_test.rs b/tests/reorg_test.rs index 28bd43a02..3ad4ece9e 100644 --- a/tests/reorg_test.rs +++ b/tests/reorg_test.rs @@ -1,7 +1,9 @@ mod common; +use bitcoin::hashes::Hash; use bitcoin::{Amount, ScriptBuf}; -use ldk_node::payment::{PaymentDirection, PaymentKind}; +use ldk_node::payment::{PaymentDirection, PaymentKind, PaymentStatus}; use ldk_node::{Event, LightningBalance, PendingSweepBalance}; +use lightning::ln::channelmanager::PaymentId; use proptest::strategy::Strategy; use proptest::strategy::ValueTree; use proptest::{prelude::prop, proptest}; @@ -220,11 +222,26 @@ proptest! { let txid = distribute_funds_unconfirmed(bitcoind, electrs, all_addrs, Amount::from_sat(amount_sat)); let mut is_spendable = false; + let mut expect_payment_status = PaymentStatus::Pending; macro_rules! verify_wallet_balances_and_transactions { - ($expected_balance_sat: expr, $expected_size_list_payments: expr) => { + ($expected_balance_sat: expr, $expected_size_list_payments: expr, $expect_payment_id: expr) => { let spend_balance = if is_spendable { $expected_balance_sat } else { 0 }; for node in &nodes { node.sync_wallets().unwrap(); + let all_payments = node.list_payments(); + assert_eq!(all_payments.len(), $expected_size_list_payments); + + let payment: Vec<_> = + all_payments.iter().filter(|p| p.status == expect_payment_status).collect(); + assert_eq!(payment.len(), 1); + assert_eq!(payment[0].id, $expect_payment_id); + + // There should be one payment with the expected status (Succeeded or Pending), + // and all other payments must be marked as Failed. + let failed: Vec<_> = + all_payments.iter().filter(|p| p.status == PaymentStatus::Failed).collect(); + assert_eq!(failed.len() + payment.len(), all_payments.len()); + let balances = node.list_balances(); assert_eq!(balances.total_onchain_balance_sats, $expected_balance_sat); assert_eq!(balances.spendable_onchain_balance_sats, spend_balance); @@ -235,8 +252,9 @@ proptest! { let mut tx_to_amount = HashMap::new(); let (mut tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf); tx_to_amount.insert(tx.clone(), amount_sat); + let mut expected_size_list_payments = 1; generate_block_and_insert_transactions(bitcoind, electrs, &[]); - verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments); + verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx.compute_txid().to_byte_array())); for _ in 0..quantity_rbf { let is_alterable_value = prop::bool::ANY.new_tree(&mut runner).unwrap().current(); if is_alterable_value { @@ -259,8 +277,10 @@ proptest! { tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_spendable); tx_to_amount.insert(tx.clone(), amount_sat); - - verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments); + expected_size_list_payments += 1; + // Bitcoind needs a block to insert the transactions in list payments + generate_block_and_insert_transactions(bitcoind, electrs, &[]); + verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx.compute_txid().to_byte_array())); } is_spendable = true; @@ -269,7 +289,7 @@ proptest! { generate_block_and_insert_transactions(bitcoind, electrs, &[tx_to_confirm.0.clone()]); generate_blocks_and_wait(bitcoind, electrs, reorg_depth - 1); amount_sat = *tx_to_confirm.1; - verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments); + verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx_to_confirm.0.compute_txid().to_byte_array())); invalidate_blocks(bitcoind, reorg_depth); generate_block_and_insert_transactions(bitcoind, electrs, &[]); @@ -278,7 +298,8 @@ proptest! { let tx_to_confirm = tx_to_amount.iter().nth(index_tx_confirm).unwrap(); generate_block_and_insert_transactions(bitcoind, electrs, &[tx_to_confirm.0.clone()]); amount_sat = *tx_to_confirm.1; + expect_payment_status = PaymentStatus::Succeeded; generate_blocks_and_wait(bitcoind, electrs, 5); - verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments); + verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx_to_confirm.0.compute_txid().to_byte_array())); } }