From 3fba25037097b25302c6592f62d69104ad6dd9cb Mon Sep 17 00:00:00 2001 From: Vihiga Tyonum Date: Wed, 16 Sep 2026 07:22:13 +0100 Subject: [PATCH] fix(rpc): verify fetched mempool tx txids - Add `tx.compute_txid() == txid` check to validate that fetched raw mempool transaction hashes against their requested txid before caching in Emitter::mempool_at. Returns `Error::UnexpectedStructure` for a mismatch. - Add a regression test for the above check. - Document that the crate assumes a trusted `bitcoind` connection. --- crates/bitcoind_rpc/Cargo.toml | 2 + crates/bitcoind_rpc/src/lib.rs | 92 +++++++++++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/crates/bitcoind_rpc/Cargo.toml b/crates/bitcoind_rpc/Cargo.toml index 572d1f5230..6aa732f05b 100644 --- a/crates/bitcoind_rpc/Cargo.toml +++ b/crates/bitcoind_rpc/Cargo.toml @@ -24,6 +24,8 @@ bdk_core = { path = "../core", version = "0.6.1", default-features = false } bdk_bitcoind_rpc = { path = "." } bdk_testenv = { path = "../testenv" } bdk_chain = { path = "../chain" } +serde = "1" +serde_json = "1" [features] default = ["std"] diff --git a/crates/bitcoind_rpc/src/lib.rs b/crates/bitcoind_rpc/src/lib.rs index 9ab4bd1924..663d62a976 100644 --- a/crates/bitcoind_rpc/src/lib.rs +++ b/crates/bitcoind_rpc/src/lib.rs @@ -7,6 +7,18 @@ //! To only get block updates (exclude mempool transactions), the caller can use //! [`Emitter::next_block`] until it returns `Ok(None)` (which means the chain tip is reached). A //! separate method, [`Emitter::mempool`] can be used to emit the whole mempool. +//! +//! # Trust in the RPC connection +//! +//! The recommended practice is to connect only to a `bitcoind` node that **you run and control**. +//! Bitcoin Core's RPC is a privileged, administrative interface rather than a public API. A wallet +//! inherently trusts its chain data source for the validity of blocks and transactions, for +//! confirmation status, and for privacy. +//! +//! If you must reach a node over a network, tunnel the connection (for example over SSH, a VPN, or +//! a Tor hidden service) and never expose RPC to the public internet. As a defense mechanism, the +//! emitter verifies that a fetched transaction's computed txid matches the one requested, but this +//! is not a substitute for connecting to a node you trust. #![cfg_attr(coverage_nightly, feature(coverage_attribute))] #![warn(missing_docs)] @@ -151,6 +163,12 @@ where /// `sync_time` is in unix seconds. /// /// This is the no-std version of [`mempool`](Self::mempool). + /// + /// # Errors + /// + /// Returns [`bitcoincore_rpc::Error::UnexpectedStructure`] if the node returns a transaction + /// body whose computed txid does not match the requested txid. A single mismatch fails the + /// whole poll. Retry on the next call. pub fn mempool_at(&mut self, sync_time: u64) -> Result { let client = &*self.client; @@ -179,6 +197,10 @@ where let tx = match self.mempool_snapshot.get(&txid) { Some(tx) => tx.clone(), None => match client.get_raw_transaction(&txid, None) { + // Reject a tx whose computed txid does not match the requested one. + Ok(tx) if tx.compute_txid() != txid => { + return Some(Err(bitcoincore_rpc::Error::UnexpectedStructure)); + } Ok(tx) => { let tx = Arc::new(tx); self.mempool_snapshot.insert(txid, tx.clone()); @@ -466,8 +488,12 @@ impl BitcoindRpcErrorExt for bitcoincore_rpc::Error { mod test { use crate::{Emitter, NO_EXPECTED_MEMPOOL_TXS}; use bdk_chain::local_chain::LocalChain; - use bdk_testenv::{anyhow, TestEnv}; - use bitcoin::{hashes::Hash, Address, Amount, ScriptBuf, Txid, WScriptHash}; + use bdk_core::CheckPoint; + use bdk_testenv::{anyhow, utils::new_tx, TestEnv}; + use bitcoin::{ + hashes::Hash, Address, Amount, BlockHash, ScriptBuf, Transaction, Txid, WScriptHash, + }; + use bitcoincore_rpc::{Error, RpcApi}; use std::collections::HashSet; #[test] @@ -536,4 +562,66 @@ mod test { Ok(()) } + + //A fetched mempool tx whose body does not match the requested txid is rejected. + #[test] + fn mismatched_tx_body_is_rejected_and_not_cached() { + struct LyingNode { + announced: Txid, + tip_hash: BlockHash, + served: Transaction, + } + + impl RpcApi for LyingNode { + fn call serde::de::Deserialize<'a>>( + &self, + _cmd: &str, + _args: &[serde_json::Value], + ) -> Result { + unreachable!() + } + fn get_block_count(&self) -> Result { + Ok(100) + } + fn get_block_hash(&self, _height: u64) -> Result { + Ok(self.tip_hash) + } + fn get_raw_mempool(&self) -> Result, Error> { + Ok(vec![self.announced]) + } + fn get_raw_transaction( + &self, + _txid: &Txid, + _block_hash: Option<&BlockHash>, + ) -> Result { + Ok(self.served.clone()) + } + } + + let served = new_tx(0); + let announced = new_tx(1).compute_txid(); + assert_ne!( + announced, + served.compute_txid(), + "announced txid must differ from the served body", + ); + + let node = LyingNode { + announced, + tip_hash: BlockHash::from_byte_array([2u8; 32]), + served, + }; + let last_cp = CheckPoint::new(0, BlockHash::all_zeros()); + let mut emitter = Emitter::new(&node, last_cp, 0, NO_EXPECTED_MEMPOOL_TXS); + + let result = emitter.mempool_at(0); + assert!( + matches!(result, Err(Error::UnexpectedStructure)), + "mismatched tx body must be rejected, got {result:?}", + ); + assert!( + !emitter.mempool_snapshot.contains_key(&announced), + "rejected tx must not be cached under the announced txid", + ); + } }