From 919fed6bff99ee6bc81fe2c79b0f7f3322fddace Mon Sep 17 00:00:00 2001 From: Chase Date: Wed, 15 Jul 2026 13:51:59 +0200 Subject: [PATCH 1/5] fix(mempool): tolerate missing/foreign edges on eviction Conflicting (RBF) spends can transiently coexist in the local mempool view across snapshot rounds; add() then overwrites the earlier tx's entry in `edges`, so evicting the earlier tx finds its outpoint entry missing or owned by the conflicting tx. This tripped the eviction assert roughly once an hour on mainnet (electrum.blockstream.info canary, 2026-07-15), taking the whole server down for a recoverable bookkeeping inconsistency. Only remove an edge if the evicted tx still owns it; warn otherwise. --- src/new_index/mempool.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/new_index/mempool.rs b/src/new_index/mempool.rs index 57bd6ac00..239926e80 100644 --- a/src/new_index/mempool.rs +++ b/src/new_index/mempool.rs @@ -516,13 +516,27 @@ impl Mempool { prune_history_entries(&mut self.history, &scripthashes, txid); for txin in tx.input { - assert!( - self.edges.remove(&txin.previous_output).is_some(), - "missing mempool edge for outpoint {}:{} (tx {})", - txin.previous_output.txid, - txin.previous_output.vout, - txid - ); + // Don't assert here: when conflicting (RBF) spends transiently + // coexist across mempool snapshot rounds, the later `add()` + // overwrites the earlier tx's entry in `edges`, so evicting + // the earlier tx finds its outpoint entry already gone (or + // owned by the conflicting tx). That's recoverable bookkeeping + // noise, not a reason to crash the server (seen ~1/h on + // mainnet, 2026-07-15). + match self.edges.get(&txin.previous_output) { + Some((spending_txid, _)) if spending_txid == *txid => { + self.edges.remove(&txin.previous_output); + } + other => { + warn!( + "mempool edge for outpoint {}:{} not owned by evicted tx {} (found {:?})", + txin.previous_output.txid, + txin.previous_output.vout, + txid, + other + ); + } + } } } From 43c9664aadf8cec40433388b8f158b3b582ad0b7 Mon Sep 17 00:00:00 2001 From: Chase Date: Wed, 15 Jul 2026 14:01:46 +0200 Subject: [PATCH 2/5] test(rest): de-flake address-prefix assertion test_rest_address asserted the prefix search returns exactly one address, but other randomly-generated wallet addresses (change outputs) can share the 8-char prefix (~1/1024 per address), failing the run by lottery. Assert addr1 is among the matches instead. --- tests/rest.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/rest.rs b/tests/rest.rs index 6d325f25a..5e53260ed 100644 --- a/tests/rest.rs +++ b/tests/rest.rs @@ -191,11 +191,17 @@ fn test_rest_address() -> Result<()> { assert!(txids.is_empty()); // Test GET /address-prefix/:prefix + // Assert addr1 is among the matches rather than the only one: other + // randomly-generated wallet addresses (e.g. change) can legitimately + // share the 8-char prefix (~1/1024 per address), which made this + // assertion flaky as an exact len()==1 check. let addr1_prefix = &addr1.to_string()[0..8]; let res = get_json(rest_addr, &format!("/address-prefix/{}", addr1_prefix))?; let found = res.as_array().expect("array of matching addresses"); - assert_eq!(found.len(), 1); - assert_eq!(found[0].as_str(), Some(addr1.to_string().as_str())); + assert!(!found.is_empty()); + assert!(found + .iter() + .any(|a| a.as_str() == Some(addr1.to_string().as_str()))); rest_handle.stop(); Ok(()) From 87bbcb6d5be3e48d695b30fabca85ba13c091d56 Mon Sep 17 00:00:00 2001 From: Chase Date: Wed, 15 Jul 2026 14:06:52 +0200 Subject: [PATCH 3/5] refactor(mempool): single edges lookup on eviction via entry API Addresses review: avoid the get+remove double hash lookup and split the warning into explicit missing vs conflicting-owner cases. --- src/new_index/mempool.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/new_index/mempool.rs b/src/new_index/mempool.rs index 239926e80..eecbbf37b 100644 --- a/src/new_index/mempool.rs +++ b/src/new_index/mempool.rs @@ -7,7 +7,7 @@ use electrs_macros::trace; #[cfg(feature = "liquid")] use elements::{encode::serialize, AssetId}; -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{hash_map::Entry, BTreeSet, HashMap, HashSet}; use std::iter::FromIterator; use std::sync::{Arc, RwLock}; use std::time::{Duration, Instant}; @@ -523,17 +523,25 @@ impl Mempool { // owned by the conflicting tx). That's recoverable bookkeeping // noise, not a reason to crash the server (seen ~1/h on // mainnet, 2026-07-15). - match self.edges.get(&txin.previous_output) { - Some((spending_txid, _)) if spending_txid == *txid => { - self.edges.remove(&txin.previous_output); + match self.edges.entry(txin.previous_output) { + Entry::Occupied(entry) if entry.get().0 == **txid => { + entry.remove(); } - other => { + Entry::Occupied(entry) => { warn!( - "mempool edge for outpoint {}:{} not owned by evicted tx {} (found {:?})", + "mempool edge for outpoint {}:{} owned by conflicting tx {} (evicting {})", txin.previous_output.txid, txin.previous_output.vout, - txid, - other + entry.get().0, + txid + ); + } + Entry::Vacant(_) => { + warn!( + "mempool edge for outpoint {}:{} already gone (evicting {})", + txin.previous_output.txid, + txin.previous_output.vout, + txid ); } } From 0b434c0ae4c1680efa6f1ff91fb8b59207009b38 Mon Sep 17 00:00:00 2001 From: Chase Date: Wed, 15 Jul 2026 14:24:39 +0200 Subject: [PATCH 4/5] docs(mempool): correct eviction-tolerance comment, trim redundant assert The conflicting-spend coexistence isn't produced by update() itself (evictions precede additions against one consistent snapshot); the injector is broadcast_raw()/submit_package() adding transactions via add_by_txid(s) outside the sync loop. Point the comment at the real mechanism, drop the dated incident detail (lives in the fix's commit message), and remove a test assert made redundant by the any() below it. --- src/new_index/mempool.rs | 16 +++++++++------- tests/rest.rs | 1 - 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/new_index/mempool.rs b/src/new_index/mempool.rs index eecbbf37b..ddb91fe9c 100644 --- a/src/new_index/mempool.rs +++ b/src/new_index/mempool.rs @@ -516,13 +516,15 @@ impl Mempool { prune_history_entries(&mut self.history, &scripthashes, txid); for txin in tx.input { - // Don't assert here: when conflicting (RBF) spends transiently - // coexist across mempool snapshot rounds, the later `add()` - // overwrites the earlier tx's entry in `edges`, so evicting - // the earlier tx finds its outpoint entry already gone (or - // owned by the conflicting tx). That's recoverable bookkeeping - // noise, not a reason to crash the server (seen ~1/h on - // mainnet, 2026-07-15). + // Don't assert here: conflicting (RBF) spends can transiently + // coexist in the local view. update() itself is safe (evictions + // are applied before additions, diffed against one consistent + // bitcoind snapshot), but Query::broadcast_raw()/submit_package() + // inject transactions via add_by_txid(s) outside the sync loop - + // broadcasting a replacement while the original is still indexed + // makes the later add() clobber the original's `edges` entry. + // Evicting the original then finds its entry gone or foreign. + // Recoverable bookkeeping noise, not a reason to crash. match self.edges.entry(txin.previous_output) { Entry::Occupied(entry) if entry.get().0 == **txid => { entry.remove(); diff --git a/tests/rest.rs b/tests/rest.rs index 5e53260ed..bfa13a3eb 100644 --- a/tests/rest.rs +++ b/tests/rest.rs @@ -198,7 +198,6 @@ fn test_rest_address() -> Result<()> { let addr1_prefix = &addr1.to_string()[0..8]; let res = get_json(rest_addr, &format!("/address-prefix/{}", addr1_prefix))?; let found = res.as_array().expect("array of matching addresses"); - assert!(!found.is_empty()); assert!(found .iter() .any(|a| a.as_str() == Some(addr1.to_string().as_str()))); From 96d2174a8e2a5a9b1bee827e59673bb7fdd579fd Mon Sep 17 00:00:00 2001 From: Chase Date: Wed, 15 Jul 2026 14:36:56 +0200 Subject: [PATCH 5/5] test(rest): regression test for RBF mempool eviction panic Reproduces the two-step failure exactly as seen in production: 1. tx A is indexed; its replacement B (same outpoint) is injected via the broadcast endpoint while A is still indexed, clobbering A's edges entry. 2. The sync evicting A passed the old assert by STEALING B's edge (any Some() satisfied it), silently corrupting spend lookups for B. 3. When B itself leaves the mempool (confirmed/replaced/expired), its edge is already gone and the old assert panicked with 'missing mempool edge for outpoint', killing the sync loop. Fails on the pre-fix code with that exact panic; passes with the ownership-checked eviction. --- tests/rest.rs | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/tests/rest.rs b/tests/rest.rs index bfa13a3eb..982462dc9 100644 --- a/tests/rest.rs +++ b/tests/rest.rs @@ -1540,3 +1540,88 @@ fn test_rest_liquid_block() -> Result<()> { rest_handle.stop(); Ok(()) } + +#[cfg(not(feature = "liquid"))] +#[test] +fn test_rest_mempool_rbf_eviction() -> Result<()> { + // Regression test for the mempool eviction panic ("missing mempool edge + // for outpoint"): tx A and its RBF replacement B spend the same outpoint + // and transiently coexist in the local mempool view when B is injected + // through the broadcast endpoint (add_by_txid) while A is still indexed. + // B's add() clobbers A's `edges` entry; the next sync round evicts A and + // must tolerate the missing/foreign edge instead of panicking. + let (rest_handle, rest_addr, mut tester) = common::init_rest_tester().unwrap(); + + // Broadcast tx A via the node wallet, explicitly BIP125-replaceable, + // and index it into the local mempool view. + let addr1 = tester.newaddress()?; + let txid_a: Txid = tester.node_client().call( + "sendtoaddress", + &[ + serde_json::json!(addr1.to_string()), + serde_json::json!(0.5), + serde_json::json!(null), + serde_json::json!(null), + serde_json::json!(false), + serde_json::json!(true), // replaceable + ], + )?; + tester.sync()?; + let res = get_json(rest_addr, &format!("/tx/{}", txid_a))?; + assert_eq!(res["status"]["confirmed"].as_bool(), Some(false)); + + // Build replacement B (same inputs, higher fee) WITHOUT broadcasting it + // through the node, so the node's mempool still holds A. + let bumped: Value = tester + .node_client() + .call("psbtbumpfee", &[serde_json::json!(txid_a.to_string())])?; + let processed: Value = tester + .node_client() + .call("walletprocesspsbt", &[bumped["psbt"].clone()])?; + assert_eq!(processed["complete"].as_bool(), Some(true)); + let finalized: Value = tester + .node_client() + .call("finalizepsbt", &[processed["psbt"].clone()])?; + let b_hex = finalized["hex"].as_str().expect("finalized tx hex"); + + // Inject B through the electrs broadcast endpoint: the node accepts the + // replacement (evicting A node-side), and add_by_txid() indexes B locally + // while A is still present - clobbering A's edges entry for the shared + // outpoint. + let broadcast_resp = ureq::post(&format!("http://{}/tx", rest_addr)).send(b_hex)?; + assert_eq!(broadcast_resp.status(), 200); + let txid_b = broadcast_resp.into_body().read_to_string()?; + + // The next sync evicts A from the local view. The unfixed code passes the + // eviction assert here - but only by STEALING B's edge entry (any Some() + // satisfied it), which is the actual arming step of the crash. + tester.sync()?; + + // B remains queryable in the mempool; A is gone. + let res = get_json(rest_addr, &format!("/tx/{}", txid_b.trim()))?; + assert_eq!(res["status"]["confirmed"].as_bool(), Some(false)); + let gone = ureq::get(&format!("http://{}/tx/{}", rest_addr, txid_a)) + .config() + .http_status_as_error(false) + .build() + .call()?; + assert_eq!(gone.status(), 404); + + // Now B itself leaves the mempool (confirmed here; RBF-of-B or expiry are + // equivalent). Evicting B finds its edge entry gone - stolen by A's + // eviction above - and the unfixed code panics with "missing mempool edge + // for outpoint", killing the sync loop. The fixed code only removes an + // edge its evicted tx still owns, so B's edge survived A's eviction and + // this round stays clean. + tester.mine()?; + tester.sync()?; + + let res = get_json(rest_addr, &format!("/tx/{}", txid_b.trim()))?; + assert_eq!(res["status"]["confirmed"].as_bool(), Some(true)); + + // And the server is still fully alive. + let _tip = get_plain(rest_addr, "/blocks/tip/height")?; + + rest_handle.stop(); + Ok(()) +}