fix(rpc): verify fetched mempool txids before caching and document trust assumption

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"]

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<MempoolEvent, bitcoincore_rpc::Error> {
         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<T: for<'a> serde::de::Deserialize<'a>>(
+                &self,
+                _cmd: &str,
+                _args: &[serde_json::Value],
+            ) -> Result<T, Error> {
+                unreachable!()
+            }
+            fn get_block_count(&self) -> Result<u64, Error> {
+                Ok(100)
+            }
+            fn get_block_hash(&self, _height: u64) -> Result<BlockHash, Error> {
+                Ok(self.tip_hash)
+            }
+            fn get_raw_mempool(&self) -> Result<Vec<Txid>, Error> {
+                Ok(vec![self.announced])
+            }
+            fn get_raw_transaction(
+                &self,
+                _txid: &Txid,
+                _block_hash: Option<&BlockHash>,
+            ) -> Result<Transaction, Error> {
+                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",
+        );
+    }
 }