Merge pull request #605 from moisesPompilio/fix-evicted-transation

Fix evicted transation

Author: tnull

src/chain/bitcoind.rs

@@ -370,8 +370,11 @@ impl BitcoindChainSource {
 		let cur_height = channel_manager.current_best_block().height;
 
 		let now = SystemTime::now();
-		let unconfirmed_txids = self.onchain_wallet.get_unconfirmed_txids();
-		match self.api_client.get_updated_mempool_transactions(cur_height, unconfirmed_txids).await
+		let bdk_unconfirmed_txids = self.onchain_wallet.get_unconfirmed_txids();
+		match self
+			.api_client
+			.get_updated_mempool_transactions(cur_height, bdk_unconfirmed_txids)
+			.await
 		{
 			Ok((unconfirmed_txs, evicted_txids)) => {
 				log_trace!(
@@ -754,7 +757,7 @@ impl BitcoindClient {
 	async fn get_raw_transaction_rpc(
 		rpc_client: Arc<RpcClient>, txid: &Txid,
 	) -> std::io::Result<Option<Transaction>> {
-		let txid_hex = bitcoin::consensus::encode::serialize_hex(txid);
+		let txid_hex = txid.to_string();
 		let txid_json = serde_json::json!(txid_hex);
 		match rpc_client
 			.call_method::<GetRawTransactionResponse>("getrawtransaction", &[txid_json])
@@ -792,7 +795,7 @@ impl BitcoindClient {
 	async fn get_raw_transaction_rest(
 		rest_client: Arc<RestClient>, txid: &Txid,
 	) -> std::io::Result<Option<Transaction>> {
-		let txid_hex = bitcoin::consensus::encode::serialize_hex(txid);
+		let txid_hex = txid.to_string();
 		let tx_path = format!("tx/{}.json", txid_hex);
 		match rest_client
 			.request_resource::<JsonResponse, GetRawTransactionResponse>(&tx_path)
@@ -889,7 +892,7 @@ impl BitcoindClient {
 	async fn get_mempool_entry_inner(
 		client: Arc<RpcClient>, txid: Txid,
 	) -> std::io::Result<Option<MempoolEntry>> {
-		let txid_hex = bitcoin::consensus::encode::serialize_hex(&txid);
+		let txid_hex = txid.to_string();
 		let txid_json = serde_json::json!(txid_hex);
 
 		match client.call_method::<GetMempoolEntryResponse>("getmempoolentry", &[txid_json]).await {
@@ -964,11 +967,12 @@ impl BitcoindClient {
 	/// - mempool transactions, alongside their first-seen unix timestamps.
 	/// - transactions that have been evicted from the mempool, alongside the last time they were seen absent.
 	pub(crate) async fn get_updated_mempool_transactions(
-		&self, best_processed_height: u32, unconfirmed_txids: Vec<Txid>,
+		&self, best_processed_height: u32, bdk_unconfirmed_txids: Vec<Txid>,
 	) -> std::io::Result<(Vec<(Transaction, u64)>, Vec<(Txid, u64)>)> {
 		let mempool_txs =
 			self.get_mempool_transactions_and_timestamp_at_height(best_processed_height).await?;
-		let evicted_txids = self.get_evicted_mempool_txids_and_timestamp(unconfirmed_txids).await?;
+		let evicted_txids =
+			self.get_evicted_mempool_txids_and_timestamp(bdk_unconfirmed_txids).await?;
 		Ok((mempool_txs, evicted_txids))
 	}
 
@@ -1078,22 +1082,22 @@ impl BitcoindClient {
 	// To this end, we first update our local mempool_entries_cache and then return all unconfirmed
 	// wallet `Txid`s that don't appear in the mempool still.
 	async fn get_evicted_mempool_txids_and_timestamp(
-		&self, unconfirmed_txids: Vec<Txid>,
+		&self, bdk_unconfirmed_txids: Vec<Txid>,
 	) -> std::io::Result<Vec<(Txid, u64)>> {
 		match self {
 			BitcoindClient::Rpc { latest_mempool_timestamp, mempool_entries_cache, .. } => {
 				Self::get_evicted_mempool_txids_and_timestamp_inner(
 					latest_mempool_timestamp,
 					mempool_entries_cache,
-					unconfirmed_txids,
+					bdk_unconfirmed_txids,
 				)
 				.await
 			},
 			BitcoindClient::Rest { latest_mempool_timestamp, mempool_entries_cache, .. } => {
 				Self::get_evicted_mempool_txids_and_timestamp_inner(
 					latest_mempool_timestamp,
 					mempool_entries_cache,
-					unconfirmed_txids,
+					bdk_unconfirmed_txids,
 				)
 				.await
 			},
@@ -1103,13 +1107,13 @@ impl BitcoindClient {
 	async fn get_evicted_mempool_txids_and_timestamp_inner(
 		latest_mempool_timestamp: &AtomicU64,
 		mempool_entries_cache: &tokio::sync::Mutex<HashMap<Txid, MempoolEntry>>,
-		unconfirmed_txids: Vec<Txid>,
+		bdk_unconfirmed_txids: Vec<Txid>,
 	) -> std::io::Result<Vec<(Txid, u64)>> {
 		let latest_mempool_timestamp = latest_mempool_timestamp.load(Ordering::Relaxed);
 		let mempool_entries_cache = mempool_entries_cache.lock().await;
-		let evicted_txids = unconfirmed_txids
+		let evicted_txids = bdk_unconfirmed_txids
 			.into_iter()
-			.filter(|txid| mempool_entries_cache.contains_key(txid))
+			.filter(|txid| !mempool_entries_cache.contains_key(txid))
 			.map(|txid| (txid, latest_mempool_timestamp))
 			.collect();
 		Ok(evicted_txids)
@@ -1236,7 +1240,7 @@ impl TryInto<GetRawMempoolResponse> for JsonResponse {
 
 		for hex in res {
 			let txid = if let Some(hex_str) = hex.as_str() {
-				match bitcoin::consensus::encode::deserialize_hex(hex_str) {
+				match hex_str.parse::<Txid>() {
 					Ok(txid) => txid,
 					Err(_) => {
 						return Err(std::io::Error::new(
@@ -1407,3 +1411,164 @@ impl std::fmt::Display for HttpError {
 		write!(f, "status_code: {}, contents: {}", self.status_code, contents)
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use bitcoin::hashes::Hash;
+	use bitcoin::{FeeRate, OutPoint, ScriptBuf, Transaction, TxIn, TxOut, Txid, Witness};
+	use lightning_block_sync::http::JsonResponse;
+	use proptest::{arbitrary::any, collection::vec, prop_assert_eq, prop_compose, proptest};
+	use serde_json::json;
+
+	use crate::chain::bitcoind::{
+		FeeResponse, GetMempoolEntryResponse, GetRawMempoolResponse, GetRawTransactionResponse,
+		MempoolMinFeeResponse,
+	};
+
+	prop_compose! {
+		fn arbitrary_witness()(
+			witness_elements in vec(vec(any::<u8>(), 0..100), 0..20)
+		) -> Witness {
+			let mut witness = Witness::new();
+			for element in witness_elements {
+				witness.push(element);
+			}
+			witness
+		}
+	}
+
+	prop_compose! {
+		fn arbitrary_txin()(
+			outpoint_hash in any::<[u8; 32]>(),
+			outpoint_vout in any::<u32>(),
+			script_bytes in vec(any::<u8>(), 0..100),
+			witness in arbitrary_witness(),
+			sequence in any::<u32>()
+		) -> TxIn {
+			TxIn {
+				previous_output: OutPoint {
+					txid: Txid::from_byte_array(outpoint_hash),
+					vout: outpoint_vout,
+				},
+				script_sig: ScriptBuf::from_bytes(script_bytes),
+				sequence: bitcoin::Sequence::from_consensus(sequence),
+				witness,
+			}
+		}
+	}
+
+	prop_compose! {
+		fn arbitrary_txout()(
+			value in 0u64..21_000_000_00_000_000u64,
+			script_bytes in vec(any::<u8>(), 0..100)
+		) -> TxOut {
+			TxOut {
+				value: bitcoin::Amount::from_sat(value),
+				script_pubkey: ScriptBuf::from_bytes(script_bytes),
+			}
+		}
+	}
+
+	prop_compose! {
+		fn arbitrary_transaction()(
+			version in any::<i32>(),
+			inputs in vec(arbitrary_txin(), 1..20),
+			outputs in vec(arbitrary_txout(), 1..20),
+			lock_time in any::<u32>()
+		) -> Transaction {
+			Transaction {
+				version: bitcoin::transaction::Version(version),
+				input: inputs,
+				output: outputs,
+				lock_time: bitcoin::absolute::LockTime::from_consensus(lock_time),
+			}
+		}
+	}
+
+	proptest! {
+		#![proptest_config(proptest::test_runner::Config::with_cases(20))]
+
+		#[test]
+		fn prop_get_raw_mempool_response_roundtrip(txids in vec(any::<[u8;32]>(), 0..10)) {
+			let txid_vec: Vec<Txid> = txids.into_iter().map(Txid::from_byte_array).collect();
+			let original = GetRawMempoolResponse(txid_vec.clone());
+
+			let json_vec: Vec<String> = txid_vec.iter().map(|t| t.to_string()).collect();
+			let json_val = serde_json::Value::Array(json_vec.iter().map(|s| json!(s)).collect());
+
+			let resp = JsonResponse(json_val);
+			let decoded: GetRawMempoolResponse = resp.try_into().unwrap();
+
+			prop_assert_eq!(original.0.len(), decoded.0.len());
+
+			prop_assert_eq!(original.0, decoded.0);
+		}
+
+		#[test]
+		fn prop_get_mempool_entry_response_roundtrip(
+			time in any::<u64>(),
+			height in any::<u32>()
+		) {
+			let json_val = json!({
+				"time": time,
+				"height": height
+			});
+
+			let resp = JsonResponse(json_val);
+			let decoded: GetMempoolEntryResponse = resp.try_into().unwrap();
+
+			prop_assert_eq!(decoded.time, time);
+			prop_assert_eq!(decoded.height, height);
+		}
+
+		#[test]
+		fn prop_get_raw_transaction_response_roundtrip(tx in arbitrary_transaction()) {
+			let hex = bitcoin::consensus::encode::serialize_hex(&tx);
+			let json_val = serde_json::Value::String(hex.clone());
+
+			let resp = JsonResponse(json_val);
+			let decoded: GetRawTransactionResponse = resp.try_into().unwrap();
+
+			prop_assert_eq!(decoded.0.compute_txid(), tx.compute_txid());
+			prop_assert_eq!(decoded.0.compute_wtxid(), tx.compute_wtxid());
+
+			prop_assert_eq!(decoded.0, tx);
+		}
+
+		#[test]
+		fn prop_fee_response_roundtrip(fee_rate in any::<f64>()) {
+			let fee_rate = fee_rate.abs();
+			let json_val = json!({
+				"feerate": fee_rate,
+				"errors": serde_json::Value::Null
+			});
+
+			let resp = JsonResponse(json_val);
+			let decoded: FeeResponse = resp.try_into().unwrap();
+
+			let expected = {
+				let fee_rate_sat_per_kwu = (fee_rate * 25_000_000.0).round() as u64;
+				FeeRate::from_sat_per_kwu(fee_rate_sat_per_kwu)
+			};
+			prop_assert_eq!(decoded.0, expected);
+		}
+
+		#[test]
+		fn prop_mempool_min_fee_response_roundtrip(fee_rate in any::<f64>()) {
+			let fee_rate = fee_rate.abs();
+			let json_val = json!({
+				"mempoolminfee": fee_rate
+			});
+
+			let resp = JsonResponse(json_val);
+			let decoded: MempoolMinFeeResponse = resp.try_into().unwrap();
+
+			let expected = {
+				let fee_rate_sat_per_kwu = (fee_rate * 25_000_000.0).round() as u64;
+				FeeRate::from_sat_per_kwu(fee_rate_sat_per_kwu)
+			};
+			prop_assert_eq!(decoded.0, expected);
+		}
+
+	}
+}

tests/common/mod.rs

@@ -30,8 +30,10 @@ use lightning_types::payment::{PaymentHash, PaymentPreimage};
 use lightning_persister::fs_store::FilesystemStore;
 
 use bitcoin::hashes::sha256::Hash as Sha256;
-use bitcoin::hashes::Hash;
-use bitcoin::{Address, Amount, Network, OutPoint, Txid};
+use bitcoin::hashes::{hex::FromHex, Hash};
+use bitcoin::{
+	Address, Amount, Network, OutPoint, ScriptBuf, Sequence, Transaction, Txid, Witness,
+};
 
 use electrsd::corepc_node::Client as BitcoindClient;
 use electrsd::corepc_node::Node as BitcoinD;
@@ -42,7 +44,7 @@ use rand::distributions::Alphanumeric;
 use rand::{thread_rng, Rng};
 use serde_json::{json, Value};
 
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::env;
 use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
@@ -487,12 +489,25 @@ where
 pub(crate) fn premine_and_distribute_funds<E: ElectrumApi>(
 	bitcoind: &BitcoindClient, electrs: &E, addrs: Vec<Address>, amount: Amount,
 ) {
+	premine_blocks(bitcoind, electrs);
+
+	distribute_funds_unconfirmed(bitcoind, electrs, addrs, amount);
+	generate_blocks_and_wait(bitcoind, electrs, 1);
+}
+
+pub(crate) fn premine_blocks<E: ElectrumApi>(bitcoind: &BitcoindClient, electrs: &E) {
 	let _ = bitcoind.create_wallet("ldk_node_test");
 	let _ = bitcoind.load_wallet("ldk_node_test");
 	generate_blocks_and_wait(bitcoind, electrs, 101);
+}
 
-	let amounts: HashMap<String, f64> =
-		addrs.iter().map(|addr| (addr.to_string(), amount.to_btc())).collect();
+pub(crate) fn distribute_funds_unconfirmed<E: ElectrumApi>(
+	bitcoind: &BitcoindClient, electrs: &E, addrs: Vec<Address>, amount: Amount,
+) -> Txid {
+	let mut amounts = HashMap::<String, f64>::new();
+	for addr in &addrs {
+		amounts.insert(addr.to_string(), amount.to_btc());
+	}
 
 	let empty_account = json!("");
 	let amounts_json = json!(amounts);
@@ -505,7 +520,70 @@ pub(crate) fn premine_and_distribute_funds<E: ElectrumApi>(
 		.unwrap();
 
 	wait_for_tx(electrs, txid);
-	generate_blocks_and_wait(bitcoind, electrs, 1);
+
+	txid
+}
+
+pub(crate) fn prepare_rbf<E: ElectrumApi>(
+	electrs: &E, txid: Txid, scripts_buf: &HashSet<ScriptBuf>,
+) -> (Transaction, usize) {
+	let tx = electrs.transaction_get(&txid).unwrap();
+
+	let fee_output_index = tx
+		.output
+		.iter()
+		.position(|output| !scripts_buf.contains(&output.script_pubkey))
+		.expect("No output available for fee bumping");
+
+	(tx, fee_output_index)
+}
+
+pub(crate) fn bump_fee_and_broadcast<E: ElectrumApi>(
+	bitcoind: &BitcoindClient, electrs: &E, mut tx: Transaction, fee_output_index: usize,
+	is_insert_block: bool,
+) -> Transaction {
+	let mut bump_fee_amount_sat = tx.vsize() as u64;
+	let attempts = 5;
+
+	for _ in 0..attempts {
+		let fee_output = &mut tx.output[fee_output_index];
+		let new_fee_value = fee_output.value.to_sat().saturating_sub(bump_fee_amount_sat);
+		if new_fee_value < 546 {
+			panic!("Warning: Fee output approaching dust limit ({} sats)", new_fee_value);
+		}
+		fee_output.value = Amount::from_sat(new_fee_value);
+
+		for input in &mut tx.input {
+			input.sequence = Sequence::ENABLE_RBF_NO_LOCKTIME;
+			input.script_sig = ScriptBuf::new();
+			input.witness = Witness::new();
+		}
+
+		let signed_result = bitcoind.sign_raw_transaction_with_wallet(&tx).unwrap();
+		assert!(signed_result.complete, "Failed to sign RBF transaction");
+
+		let tx_bytes = Vec::<u8>::from_hex(&signed_result.hex).unwrap();
+		tx = bitcoin::consensus::encode::deserialize::<Transaction>(&tx_bytes).unwrap();
+
+		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;
+			},
+			Err(_) => {
+				bump_fee_amount_sat += bump_fee_amount_sat * 5;
+				if tx.output[fee_output_index].value.to_sat() < bump_fee_amount_sat {
+					panic!("Insufficient funds to increase fee");
+				}
+			},
+		}
+	}
+
+	panic!("Failed to bump fee after {} attempts", attempts);
 }
 
 pub fn open_channel(