diff --git a/COMPAT.md b/COMPAT.md index fdd431621..76234181a 100644 --- a/COMPAT.md +++ b/COMPAT.md @@ -232,7 +232,7 @@ App `ServeLimits` always on (same model as Electrum). | Address / scripthash | done | stats + `/utxo` + `/txs` + `/txs/mempool` + `/txs/chain[/:last_seen_txid]` + `/txs/summary[/:last_seen_txid]` (dialect; next row). `/utxo` matches Electrum listunspent (mempool funding + drop mempool-spent confirmed); `/txs` and `/txs/mempool` use full Esplora tx JSON for mempool-only rows (wire from the hub). `?after_txid=` on `/txs` and `/txs/summary` skips through that tx (mempool then chain); unknown or unparseable → **422** `after_txid not found`. A mempool-only cursor past the first 50 mempool rows returns the confirmed page. `POST /addresses/txs` and `POST /scripthashes/txs` (and `/txs/summary`) take a JSON array (max **300**; over → **422** `body too long`); unique scripts join once per request and merge newest-first into one page of 25 confirmed rows (mempool rows stay in front on `/txs`, 50 per script). A later page walks past each script's first 25. Unix/loopback `X-Rbitcoin-Client`: last-1 GET join + last-bulk POST (16 MiB packed/client), 30s idle, 256 clients, singleflight on the same `(client, sh)`. Public TCP ignores the header (stack-local join). Concurrent different GET scripts replace last-1. Unbounded process LRU stays **X-M3**. Needs SH finalize. Stamp is visible SH (durable + pending write-behind), matching live tip while jobs sit in RAM. `?asof=` on `/`, `/utxo`, `/txs`, `/txs/chain`, `/txs/summary`: confirmed join at that ancestor **at or behind visible SH**, **no** mempool; headers are the asof hash; 404 if not on chain or ahead of visible SH. | | `/txs/summary` | dialect | **Not** in Blockstream Esplora [`API.md`](https://github.com/Blockstream/esplora/blob/master/API.md). Compact `{txid, value, height, time, tx_position}` like mempool.space `/address/:addr/txs/summary`. Confirmed only (25/page, newest first, including a multi-script post of that same merged page); path cursor `/:last_seen_txid` and mempool.space `?after_txid=` (unknown → **422**). `value` is net sats for that script in that tx (funded − spent). `time` is the confirming header timestamp (`0` if the header is missing). `tx_position` is that tx's index in the block. Mempool rows stay on `/txs` and `/txs/mempool`. A page is served above `--max-sh-creates`; unpaged stats and `/utxo` still **503**. | | Mempool / fees | done | `/mempool`, `/mempool/txids`, `/mempool/txids/page[/:last]`, `/mempool/recent` (accept-order ring), `/fee-estimates` (sat/vB at the estimator's 1 sat/kvB resolution; a target without an estimate is left out, and **503** `fee estimates unavailable` when none has one). mempool.space `/api/v1/fees/recommended` is its backend's surface, not served here (**404**) | -| `POST /tx` | done | broadcast via mempool hub; **503** if hub absent. `GET /broadcast?tx=` is the same admit path (legacy electrs). | +| `POST /tx` | done | broadcast via mempool hub; **503** if hub absent. `GET /broadcast` is not served (**404**). | | `POST /txs/test` | done | JSON array of hex (max 25) → `MempoolHub::test_accept` (no admit). `?maxfeerate=` is BTC/kvB like electrs (`0` unlimited; omitted → 0.1 BTC/kvB). | | `POST /txs/package` | done | JSON array of hex txs → `accept_package`; **503** without hub; max 25 txs | | electrs `/internal/*` | done | **Unix listen only.** TCP GET/POST `/internal/…` → **404** (same unknown-path fallback). `POST /internal/txs` (400 on unparseable id; missing omitted); `POST /internal/mempool/txs` (mempool only); `GET /internal/mempool/txs[/all or /:last]` (txid-sort pages, default `max_txs=10000`; `/all` registered first); `GET /internal/block/:hash/txs` (full list; public `/txs` stays 25/page); `POST /internal/txs/outspends/by-txid` (same-length slots, unknown → `[]`); `POST /internal/txs/outspends/by-outpoint` (`txid:vout`; malformed → `{"spent":false}`). Snapshot JSON is published on the hub (no admit-path build). `GET /mempool/txids/page` is public electrs (both listeners). | diff --git a/changelog.d/bobberb-remote.md b/changelog.d/bobberb-remote.md new file mode 100644 index 000000000..b0eb5dcef --- /dev/null +++ b/changelog.d/bobberb-remote.md @@ -0,0 +1,44 @@ +Fixed + +- **Orphan promotion walks one worklist.** The mempool walk and the relay + hub walk both admit a parked chain child then descendants, without one + stack frame per descendant. A parent published inside that walk, including + a one-parent package, releases its parked spends on the same list. A child + that still misses another parent stays parked. A package admits its own + members before that walk, including on the hub path used by + `submitpackage` and Electrum, so a parked child included in the package + is not rolled out of the mempool. A failed package puts that child back + under the parents it still does not have, keeps the peers who announced + it, and admits it when those parents are already in the mempool. That + admit uses the child's `prioritisetransaction` delta, so a fee under the + raw relay floor is not thrown away. +- **Electrum history refuses a window wider than 8,064 blocks.** A + `from_height` of 1 or more still scans that window and does not use + the scripthash create cap. +- **An Electrum response write uses the idle deadline.** A client that + stops reading loses the connection. A scripthash status push uses that + same deadline, and a timeout ends the connection. +- **An SV2 session goes idle after setup.** The setup timeout also + closes a client that sent one `CoinbaseOutputConstraints` and then + stopped sending. Tip and fee pushes do not refresh it. +- **A replacement may spend an unconfirmed input only from its one + direct conflict.** That parent stays a valid child-pays-for-parent. + Spending the conflict's own output is refused as + `bad-txns-spends-conflicting-tx`. Direct-set feerate is unchanged. +- **The invalid-txid set stops at 4,096.** A newer id does not clear + the ones already stored. +- **Onion and I2P misbehavior is remembered for a day.** The key is + the overlay identity, not the port. Loopback is still not recorded, + and nothing is written to `banlist.dat`. An outbound dial, including + a feeler and an IBD dial, is refused only for that key. A netgroup + that just lost an inbound slot still cannot take another inbound + slot, and it does not block dials to the rest of the prefix. +- **`GET /broadcast` is not served.** `POST /tx` remains the admit path. +- **A panicking script-pool chunk releases the wave.** `wait_done` + returns, and the worker keeps running. +- **IBD plants a stored sibling of an invalid tip block.** The header + store supplies the parent when the work path has not recorded it yet, + so download continues onto that sibling. +- **IBD retries a tip child after BadPrev.** A block whose parent is the + current tip is fetched again when confirm reports a missing previous + header. diff --git a/crates/rbitcoin-consensus/src/script_pool.rs b/crates/rbitcoin-consensus/src/script_pool.rs index bfe83f069..73ce052ab 100644 --- a/crates/rbitcoin-consensus/src/script_pool.rs +++ b/crates/rbitcoin-consensus/src/script_pool.rs @@ -71,31 +71,30 @@ impl Wave { unpark_script_publisher(); } - fn claim_chunk(&self) -> Option> { - if self.failed.load(Ordering::Acquire) { + fn claim_chunk(wave: &Arc) -> Option<(Range, InWave)> { + if wave.failed.load(Ordering::Acquire) { return None; } // `in_wave` before `next`: a last-chunk claimer is visible to // `is_complete` before `next >= n`, so the publisher cannot free ctx // under a worker about to `apply`. - self.in_wave.fetch_add(1, Ordering::AcqRel); - if self.failed.load(Ordering::Acquire) { - self.in_wave.fetch_sub(1, Ordering::AcqRel); - self.notify_if_complete(); + wave.in_wave.fetch_add(1, Ordering::AcqRel); + let hold = InWave { + wave: Arc::clone(wave), + }; + if wave.failed.load(Ordering::Acquire) { return None; } - let chunk = self.chunk.max(1); - let i = self.next.fetch_add(chunk, Ordering::Relaxed); - if i >= self.n { - self.in_wave.fetch_sub(1, Ordering::AcqRel); - self.notify_if_complete(); + let chunk = wave.chunk.max(1); + let i = wave.next.fetch_add(chunk, Ordering::Relaxed); + if i >= wave.n { return None; } #[cfg(test)] if STEAL_CLAIMS_ON.load(Ordering::Relaxed) { STEAL_CLAIMS.fetch_add(1, Ordering::Relaxed); } - Some(i..self.n.min(i.saturating_add(chunk))) + Some((i..wave.n.min(i.saturating_add(chunk)), hold)) } fn is_complete(&self) -> bool { @@ -121,8 +120,6 @@ impl Wave { break; } } - self.in_wave.fetch_sub(1, Ordering::AcqRel); - self.notify_if_complete(); } fn has_unclaimed(&self) -> bool { @@ -191,12 +188,21 @@ fn publish_waves(waves: &[Arc]) { waves_snap().store(Arc::new(waves.to_vec())); } +/// Drops `in_wave` on the success path and on a panic inside the chunk. +struct InWave { + wave: Arc, +} + +impl Drop for InWave { + fn drop(&mut self) { + self.wave.in_wave.fetch_sub(1, Ordering::AcqRel); + self.wave.notify_if_complete(); + } +} + /// Lock-free claim: load the published wave list. Must not lock [`WAVES`]. -fn steal_chunk() -> Option<(Arc, Range)> { - let claimed = waves_snap() - .load() - .iter() - .find_map(|w| w.claim_chunk().map(|range| (Arc::clone(w), range))); +fn steal_chunk() -> Option<(Range, InWave)> { + let claimed = waves_snap().load().iter().find_map(Wave::claim_chunk); #[cfg(test)] if claimed.is_some() { maybe_delay_claim(); @@ -204,6 +210,23 @@ fn steal_chunk() -> Option<(Arc, Range)> { claimed } +fn run_held(range: Range, hold: InWave) { + let panicked = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + hold.wave.run_chunk(range); + })); + if panicked.is_err() { + hold.wave.failed.store(true, Ordering::Release); + let mut g = hold + .wave + .first_err + .lock() + .unwrap_or_else(|p| p.into_inner()); + if g.is_none() { + *g = Some(ConsensusError::Internal("script worker panic")); + } + } +} + /// True when steal workers can still claim a foreground job. pub fn fg_has_unclaimed() -> bool { waves_snap().load().iter().any(|w| w.has_unclaimed()) @@ -217,10 +240,10 @@ pub fn help_steal() -> bool { if on_steal_worker() { return false; } - let Some((w, range)) = steal_chunk() else { + let Some((range, hold)) = steal_chunk() else { return false; }; - w.run_chunk(range); + run_held(range, hold); true } @@ -385,16 +408,16 @@ fn take_detached_job(pool: &ScriptWorkers) -> Option { } fn steal_or_job(pool: &ScriptWorkers) -> bool { - if let Some((w, range)) = steal_chunk() { - w.run_chunk(range); + if let Some((range, hold)) = steal_chunk() { + run_held(range, hold); return true; } if let Some(job) = take_detached_job(pool) { job(); return true; } - if let Some((w, range)) = steal_chunk() { - w.run_chunk(range); + if let Some((range, hold)) = steal_chunk() { + run_held(range, hold); return true; } false @@ -1058,6 +1081,28 @@ mod tests { } } + /// A panicking chunk must not leave `in_wave` elevated. `wait_done` returns + /// inside the test's own bound. + #[test] + fn chunk_panic_releases_the_wave() { + let _gate = STEAL_TEST.lock().unwrap_or_else(|p| p.into_inner()); + fn boom(_: &u32) -> Result<(), ConsensusError> { + panic!("chunk panic"); + } + let wave = start_for_each_owned((0..64u32).collect(), boom) + .unwrap() + .expect("wave"); + let (tx, rx) = mpsc::channel(); + thread::spawn(move || { + let failed = wave.finish().is_err(); + let _ = tx.send(failed); + }); + let failed = rx + .recv_timeout(Duration::from_secs(2)) + .expect("wait_done after a chunk panic"); + assert!(failed, "chunk panic is an error"); + } + /// After `in_wave++` and before `next += chunk`, `is_complete` must be /// false (publisher must not free ctx under the claimer). #[test] diff --git a/crates/rbitcoin-electrum/src/server.rs b/crates/rbitcoin-electrum/src/server.rs index f9c5f0acd..94ca7c6ba 100644 --- a/crates/rbitcoin-electrum/src/server.rs +++ b/crates/rbitcoin-electrum/src/server.rs @@ -404,7 +404,7 @@ where "method": "blockchain.headers.subscribe", "params": [{ "hex": t.header_hex, "height": t.height }] }); - write_line(&mut writer, &msg).await?; + write_line(&mut writer, &msg, idle).await?; } if !conn.outpoint_subs.is_empty() { emit_outpoint_notes( @@ -412,6 +412,7 @@ where &query, mempool.clone(), &conn.outpoint_subs, + idle, ) .await?; } @@ -422,6 +423,7 @@ where ¶ms, sub, t.height, + idle, ) .await?; } @@ -438,6 +440,7 @@ where &conn.sh_subs, &mut last_sent_status, heights, + idle, ) .await?; } @@ -451,6 +454,7 @@ where &conn.sh_subs, &mut last_sent_status, None, + idle, ) .await?; } @@ -481,6 +485,7 @@ where &conn.sh_subs, &mut last_sent_status, scan, + idle, ) .await?; } @@ -547,7 +552,7 @@ where "method": "blockchain.scripthash.subscribe", "params": [hash_hex_rev(&sh), status_json(status)] }); - let _ = write_line(&mut writer, &msg).await; + write_line(&mut writer, &msg, idle).await?; } } } @@ -564,7 +569,7 @@ where "jsonrpc":"2.0","id": null, "error": {"code": -32600, "message": "request line too long"} }); - let _ = write_line(&mut writer, &resp).await; + let _ = write_line(&mut writer, &resp, idle).await; return Err(e); } return Err(e); @@ -595,6 +600,7 @@ where &peer, id, ¶ms_v, + idle, &mut conn, ) .await?; @@ -742,7 +748,7 @@ where json!({"jsonrpc":"2.0","id": id, "error": {"code": 1, "message": e}}) } }; - write_line(&mut writer, &resp).await?; + write_line(&mut writer, &resp, idle).await?; let _ = ¬ify; } } @@ -762,6 +768,7 @@ async fn serve_sp_subscribe( peer: &SocketAddr, id: Value, params_v: &Value, + idle: Duration, conn: &mut ElectrumConn, ) -> Result<(), std::io::Error> where @@ -775,6 +782,7 @@ where write_line( writer, &json!({"jsonrpc":"2.0","id": id, "error": {"code": 1, "message": e}}), + idle, ) .await?; return Ok(()); @@ -784,6 +792,7 @@ where write_line( writer, &json!({"jsonrpc":"2.0","id": id, "error": {"code": 1, "message": "silent payment scan already running"}}), + idle, ) .await?; return Ok(()); @@ -797,7 +806,7 @@ where 0, None, ); - write_line(writer, &rpc_result(&id, &result, None)).await?; + write_line(writer, &rpc_result(&id, &result, None), idle).await?; conn.sp_scan_busy = true; let last = tip.unwrap_or(sub.start); let scanned = scan_sp_off_connection(reader, query, chain, &sub, last).await; @@ -816,7 +825,7 @@ where "history": hits, } }); - write_line(writer, ¬e).await?; + write_line(writer, ¬e, idle).await?; conn.sp_sub = Some(sub); Ok(()) } @@ -845,6 +854,7 @@ async fn emit_sp_tip( chain: &ChainParams, sub: &crate::silent_scan::SpSub, height: u32, + idle: Duration, ) -> Result<(), std::io::Error> { let hits = crate::silent_scan::scan_hits(query, chain, sub, height, height).unwrap_or_default(); if hits.is_empty() { @@ -859,7 +869,7 @@ async fn emit_sp_tip( "history": hits, } }); - write_line(writer, ¬e).await + write_line(writer, ¬e, idle).await } async fn emit_outpoint_notes( @@ -867,6 +877,7 @@ async fn emit_outpoint_notes( query: &Arc, mempool: Option>, subs: &HashSet<([u8; 32], u32)>, + idle: Duration, ) -> Result<(), std::io::Error> { let keys: Vec<([u8; 32], u32)> = subs.iter().copied().collect(); let q = Arc::clone(query); @@ -889,7 +900,7 @@ async fn emit_outpoint_notes( "method": "blockchain.outpoint.subscribe", "params": [txid_hex(&txid), vout, status], }); - write_line(writer, ¬e).await?; + write_line(writer, ¬e, idle).await?; } Ok(()) } @@ -998,7 +1009,7 @@ where let req = match crate::tweaks::parse_req(params_v) { Ok(r) => r, Err(e) => { - tweaks_sub_err(writer, peer, &id, params_v, 0, &e).await?; + tweaks_sub_err(writer, peer, &id, params_v, 0, &e, idle).await?; return Ok(()); } }; @@ -1007,7 +1018,7 @@ where let t0 = Instant::now(); let Some(last) = last else { return serve_tweaks_empty_last( - writer, query, chain, peer, &id, params_v, req, t0, min_dust, + writer, query, chain, peer, &id, params_v, req, t0, min_dust, idle, ) .await; }; @@ -1032,6 +1043,7 @@ where params_v, t0.elapsed().as_millis() as u64, &e, + idle, ) .await?; return Ok(()); @@ -1040,8 +1052,8 @@ where let wall_ms = t0.elapsed().as_millis() as u64; meter_dispatch_wall(t0.elapsed().as_micros() as u64); tweaks_sub_ok_log(peer, params_v, wall_ms); - write_rpc_result(writer, &id, &wave.result_json).await?; - write_raw_lines(writer, &wave.rest_notifies).await?; + write_rpc_result(writer, &id, &wave.result_json, idle).await?; + write_raw_lines(writer, &wave.rest_notifies, idle).await?; let mut next = req.start.saturating_add(wave.consumed.max(1)); let limits = crate::tweaks::subscribe_serve_limits(req.historical); serve_tweaks_rest( @@ -1077,7 +1089,7 @@ where ) .await?; } - write_line(writer, &crate::tweaks::done_notify()).await?; + write_line(writer, &crate::tweaks::done_notify(), idle).await?; Ok(()) } @@ -1088,6 +1100,7 @@ async fn tweaks_sub_err( params_v: &Value, wall_ms: u64, e: &str, + idle: Duration, ) -> Result<(), std::io::Error> { rbitcoin_log::api_call( "electrum", @@ -1100,6 +1113,7 @@ async fn tweaks_sub_err( write_line( writer, &json!({"jsonrpc":"2.0","id": id, "error": {"code": 1, "message": e}}), + idle, ) .await } @@ -1126,6 +1140,7 @@ async fn serve_tweaks_empty_last( req: crate::tweaks::TweakReq, t0: Instant, min_dust: u64, + idle: Duration, ) -> Result<(), std::io::Error> { let first = match crate::tweaks::height_map_json(query, chain, req.start, !req.historical, min_dust) { @@ -1138,12 +1153,13 @@ async fn serve_tweaks_empty_last( params_v, t0.elapsed().as_millis() as u64, &e, + idle, ) .await?; return Ok(()); } }; - serve_tweaks_empty_ok(writer, peer, id, params_v, t0, &first).await + serve_tweaks_empty_ok(writer, peer, id, params_v, t0, &first, idle).await } async fn serve_tweaks_empty_ok( @@ -1153,12 +1169,13 @@ async fn serve_tweaks_empty_ok( params_v: &Value, t0: Instant, first: &str, + idle: Duration, ) -> Result<(), std::io::Error> { let wall_ms = t0.elapsed().as_millis() as u64; meter_dispatch_wall(t0.elapsed().as_micros() as u64); tweaks_sub_ok_log(peer, params_v, wall_ms); - write_rpc_result(writer, id, first).await?; - write_line(writer, &crate::tweaks::done_notify()).await + write_rpc_result(writer, id, first, idle).await?; + write_line(writer, &crate::tweaks::done_notify(), idle).await } #[allow(clippy::too_many_arguments)] @@ -1220,7 +1237,7 @@ where if open { handle = spawn_wave(*next); } - write_raw_lines(writer, &batch.lines).await?; + write_raw_lines(writer, &batch.lines, idle).await?; if !open { break; } @@ -1258,6 +1275,7 @@ where write_line( writer, &json!({"jsonrpc":"2.0","id": ping_id, "result": null}), + idle, ) .await?; } @@ -1279,15 +1297,33 @@ where } } +async fn write_bytes_within( + writer: &mut W, + bytes: &[u8], + deadline: Duration, +) -> Result<(), std::io::Error> { + let write = async { + writer.write_all(bytes).await?; + writer.flush().await?; + Ok(()) + }; + match tokio::time::timeout(deadline, write).await { + Ok(result) => result, + Err(_) => Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "write timeout", + )), + } +} + async fn write_line( writer: &mut W, msg: &Value, + deadline: Duration, ) -> Result<(), std::io::Error> { let mut s = serde_json::to_string(msg).unwrap_or_else(|_| "{}".into()); s.push('\n'); - writer.write_all(s.as_bytes()).await?; - writer.flush().await?; - Ok(()) + write_bytes_within(writer, s.as_bytes(), deadline).await } fn restatus_notes( @@ -1419,6 +1455,7 @@ async fn emit_sh_notes( sh_subs: &HashSet<[u8; 32]>, last_sent: &mut HashMap<[u8; 32], String>, heights: Option>, + idle: Duration, ) -> Result<(), std::io::Error> { let q = Arc::clone(query); let subs: Vec<[u8; 32]> = sh_subs.iter().copied().collect(); @@ -1437,7 +1474,7 @@ async fn emit_sh_notes( "method": "blockchain.scripthash.subscribe", "params": [hash_hex_rev(&sh), status_json(status)] }); - write_line(writer, &msg).await?; + write_line(writer, &msg, idle).await?; } Ok(()) } @@ -1445,16 +1482,18 @@ async fn emit_sh_notes( async fn write_raw_line( writer: &mut W, msg: &str, + deadline: Duration, ) -> Result<(), std::io::Error> { - writer.write_all(msg.as_bytes()).await?; - writer.write_all(b"\n").await?; - writer.flush().await?; - Ok(()) + let mut bytes = Vec::with_capacity(msg.len() + 1); + bytes.extend_from_slice(msg.as_bytes()); + bytes.push(b'\n'); + write_bytes_within(writer, &bytes, deadline).await } async fn write_raw_lines( writer: &mut W, lines: &[String], + deadline: Duration, ) -> Result<(), std::io::Error> { if lines.is_empty() { return Ok(()); @@ -1468,15 +1507,14 @@ async fn write_raw_lines( buf.extend_from_slice(l.as_bytes()); buf.push(b'\n'); } - writer.write_all(&buf).await?; - writer.flush().await?; - Ok(()) + write_bytes_within(writer, &buf, deadline).await } async fn write_rpc_result( writer: &mut W, id: &Value, result_json: &str, + deadline: Duration, ) -> Result<(), std::io::Error> { let mut s = String::with_capacity(result_json.len() + 48); s.push_str("{\"jsonrpc\":\"2.0\",\"id\":"); @@ -1484,7 +1522,7 @@ async fn write_rpc_result( s.push_str(",\"result\":"); s.push_str(result_json); s.push('}'); - write_raw_line(writer, &s).await + write_raw_line(writer, &s, deadline).await } /// Instant methods that must stay on the connection task so a 1-worker runtime diff --git a/crates/rbitcoin-electrum/src/server_tests.rs b/crates/rbitcoin-electrum/src/server_tests.rs index b4ac92d4f..2546b7c86 100644 --- a/crates/rbitcoin-electrum/src/server_tests.rs +++ b/crates/rbitcoin-electrum/src/server_tests.rs @@ -4,7 +4,7 @@ use rbitcoin_query::testutil::FixtureChain; use rbitcoin_query::Query; use std::collections::{HashMap, HashSet}; -use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; +use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader}; use tokio::net::TcpStream; fn tmp_store() -> (rbitcoin_query::testutil::TempDir, Query) { @@ -381,20 +381,31 @@ fn negotiate_protocol_intersection_and_asof_dialect() { .contains("unsupported")); } +/// One line, one byte at a time. A `BufReader` can pull the next server line +/// into its buffer and drop it, so a later read sees the wrong message. +async fn read_exact_line(stream: &mut TcpStream) -> String { + let mut buf = Vec::new(); + let mut byte = [0u8; 1]; + loop { + let n = tokio::time::timeout(std::time::Duration::from_secs(3), stream.read(&mut byte)) + .await + .expect("timeout") + .unwrap(); + assert_ne!(n, 0, "electrum peer closed"); + buf.push(byte[0]); + if byte[0] == b'\n' { + break; + } + } + String::from_utf8(buf).unwrap() +} + async fn electrum_tcp_rpc(stream: &mut TcpStream, id: u32, method: &str, params: Value) -> Value { let req = json!({"jsonrpc":"2.0","id":id,"method":method,"params":params}); let mut line = serde_json::to_string(&req).unwrap(); line.push('\n'); stream.write_all(line.as_bytes()).await.unwrap(); - let mut reader = BufReader::new(&mut *stream); - let mut resp = String::new(); - tokio::time::timeout( - std::time::Duration::from_secs(3), - reader.read_line(&mut resp), - ) - .await - .expect("timeout") - .unwrap(); + let resp = read_exact_line(stream).await; serde_json::from_str(&resp).unwrap() } @@ -461,6 +472,9 @@ async fn accept_client_ping_and_shutdown() { ) .await; assert_eq!(probe["result"], json!({"0": {}})); + let done: Value = serde_json::from_str(&read_exact_line(&mut stream).await).unwrap(); + assert_eq!(done["method"], "blockchain.tweaks.subscribe"); + assert_eq!(done["params"][0]["message"], "done"); let banner_v = electrum_tcp_rpc(&mut stream, 5, "server.banner", json!([])).await; assert_eq!(banner_v["result"].as_str().unwrap(), banner); @@ -1734,4 +1748,233 @@ async fn sp_scan_stops_when_the_client_hangs_up() { ); } -include!("electrum_sh_journey.rs"); +/// A client that never reads must not hold the request. The write itself +/// returns; the connection task drops on that error. +#[tokio::test] +async fn stalled_electrum_write_is_closed() { + use std::pin::Pin; + use std::task::{Context, Poll}; + use tokio::io::AsyncWrite; + + struct PendingWriter; + impl AsyncWrite for PendingWriter { + fn poll_write( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + _buf: &[u8], + ) -> Poll> { + Poll::Pending + } + fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Pending + } + fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + } + + let mut writer = PendingWriter; + let result = tokio::time::timeout( + std::time::Duration::from_secs(2), + super::write_line( + &mut writer, + &json!({"jsonrpc":"2.0","id": 1, "result": null}), + std::time::Duration::from_millis(40), + ), + ) + .await + .expect("a stalled write returns before the test bound"); + let err = result.expect_err("stalled write is an error"); + assert_eq!(err.kind(), std::io::ErrorKind::TimedOut); +} + +/// A scripthash status push that the client never reads ends the connection. +/// The push is the only write after subscribe: the chain height does not move, +/// so the periodic rescan has nothing new to send. +#[tokio::test] +async fn stalled_scripthash_push_ends_the_connection() { + use std::pin::Pin; + use std::task::{Context, Poll}; + use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; + + struct ScriptedRead { + data: Vec, + pos: usize, + } + impl AsyncRead for ScriptedRead { + fn poll_read( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + let this = self.get_mut(); + if this.pos >= this.data.len() { + return Poll::Pending; + } + let n = (this.data.len() - this.pos).min(buf.remaining()); + buf.put_slice(&this.data[this.pos..this.pos + n]); + this.pos += n; + Poll::Ready(Ok(())) + } + } + + struct StallWriter { + got: std::sync::Arc>>, + stall: std::sync::Arc, + } + impl AsyncWrite for StallWriter { + fn poll_write( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + if self.stall.load(std::sync::atomic::Ordering::SeqCst) { + return Poll::Pending; + } + self.got.lock().unwrap().extend_from_slice(buf); + Poll::Ready(Ok(buf.len())) + } + fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + } + + struct Pair { + reader: R, + writer: W, + } + impl AsyncRead for Pair { + fn poll_read( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + Pin::new(&mut self.get_mut().reader).poll_read(cx, buf) + } + } + impl AsyncWrite for Pair { + fn poll_write( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + Pin::new(&mut self.get_mut().writer).poll_write(cx, buf) + } + fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.get_mut().writer).poll_flush(cx) + } + fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.get_mut().writer).poll_shutdown(cx) + } + } + + let (dir, q) = tmp_store(); + let params = ChainParams::regtest(); + let genesis = bitcoin::blockdata::constants::genesis_block(bitcoin::Network::Regtest); + rbitcoin_query::accept_and_connect_block( + &q, + ¶ms, + rbitcoin_primitives::Height::GENESIS, + &genesis, + rbitcoin_consensus::Milestone::NONE, + ) + .unwrap(); + let last = 1 + params.coinbase_maturity(); + let (_tip, _time, cbs) = rbitcoin_query::pad_empty_from( + &q, + ¶ms, + genesis.block_hash(), + genesis.header.time, + 1, + last, + 1, + ); + let q = std::sync::Arc::new(q); + let hub = std::sync::Arc::new( + MempoolHub::open(dir.join("mempool"), std::sync::Arc::clone(&q)).unwrap(), + ); + hub.set_relay_enabled(true); + + let sh = electrum_scripthash_hex(&[0x51]); + let line = format!( + "{{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"blockchain.scripthash.subscribe\",\"params\":[\"{sh}\"]}}\n" + ); + let stall = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let got = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let stream = Pair { + reader: ScriptedRead { + data: line.into_bytes(), + pos: 0, + }, + writer: StallWriter { + got: std::sync::Arc::clone(&got), + stall: std::sync::Arc::clone(&stall), + }, + }; + let mut cfg = ElectrumConfig::for_params("127.0.0.1:0".parse().unwrap(), ¶ms); + cfg.limits.idle_timeout = std::time::Duration::from_millis(80); + let (tip_tx, tip_rx) = tokio::sync::broadcast::channel(1); + let stop = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let task = tokio::spawn(handle_client( + stream, + "127.0.0.1:9".parse().unwrap(), + std::sync::Arc::clone(&q), + std::sync::Arc::new(cfg), + std::sync::Arc::new(params), + tip_rx, + Some(std::sync::Arc::clone(&hub)), + stop, + )); + let _tip_tx = tip_tx; + + let mut saw_reply = false; + for _ in 0..50 { + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + let buf = got.lock().unwrap().clone(); + if String::from_utf8_lossy(&buf).contains("\"id\":1") { + saw_reply = true; + break; + } + } + assert!( + saw_reply, + "subscribe reply must land before the push stalls" + ); + tokio::time::sleep(std::time::Duration::from_millis(30)).await; + stall.store(true, std::sync::atomic::Ordering::SeqCst); + + let tx = bitcoin::Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![bitcoin::TxIn { + previous_output: bitcoin::OutPoint { + txid: cbs[0], + vout: 0, + }, + script_sig: bitcoin::ScriptBuf::new(), + sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: bitcoin::Witness::new(), + }], + output: vec![bitcoin::TxOut { + value: bitcoin::Amount::from_sat(50_0000_0000 - 10_000), + script_pubkey: bitcoin::ScriptBuf::from_bytes(vec![0x51]), + }], + }; + hub.accept_tx(&tx) + .expect("mempool spend of the subscribed script"); + let joined = tokio::time::timeout(std::time::Duration::from_secs(2), task) + .await + .expect("stalled push ends the connection task"); + let err = joined + .expect("connection task") + .expect_err("stalled push is an error"); + assert_eq!(err.kind(), std::io::ErrorKind::TimedOut); + assert!( + err.to_string().contains("write timeout"), + "the push timeout must end the task, got {err}" + ); + let _ = dir; +} diff --git a/crates/rbitcoin-esplora/src/handlers.rs b/crates/rbitcoin-esplora/src/handlers.rs index 637f9464b..63c9dd2f7 100644 --- a/crates/rbitcoin-esplora/src/handlers.rs +++ b/crates/rbitcoin-esplora/src/handlers.rs @@ -1879,28 +1879,6 @@ async fn admit_broadcast(st: AppState, tx: bitcoin::Transaction) -> Response { } } -#[derive(Deserialize, Default)] -pub struct BroadcastQuery { - tx: Option, -} - -pub async fn get_broadcast( - State(st): State, - AxumQuery(q): AxumQuery, -) -> Response { - if st.mempool.is_none() { - return (StatusCode::SERVICE_UNAVAILABLE, "mempool not available").into_response(); - } - let Some(hex) = q.tx.filter(|s| !s.is_empty()) else { - return (StatusCode::BAD_REQUEST, "Missing tx").into_response(); - }; - let tx = match decode_tx_hex_str(&hex) { - Ok(t) => t, - Err(r) => return r, - }; - admit_broadcast(st, tx).await -} - pub async fn post_tx(State(st): State, body: Bytes) -> Response { if st.mempool.is_none() { return (StatusCode::SERVICE_UNAVAILABLE, "mempool not available").into_response(); diff --git a/crates/rbitcoin-esplora/src/internal.rs b/crates/rbitcoin-esplora/src/internal.rs index 31d1befbc..150fca6be 100644 --- a/crates/rbitcoin-esplora/src/internal.rs +++ b/crates/rbitcoin-esplora/src/internal.rs @@ -552,14 +552,13 @@ mod tests { (handle, sock) } - /// `/txs/test` dry-runs; `GET /broadcast` and `POST /tx` admit. + /// `/txs/test` dry-runs; `POST /tx` admits. `GET /broadcast` is gone. async fn test_then_broadcast(sock: &std::path::Path, pad: &Pad, txs: &[Transaction]) { use bitcoin::consensus::encode::serialize_hex; let hex = serialize_hex(&txs[0]); let (st, body) = http_get_unix(sock, "/broadcast").await; - assert_eq!(st, 400, "{body}"); - assert!(body.contains("Missing tx"), "{body}"); + assert_eq!(st, 404, "{body}"); let body = serde_json::to_vec(&json!([&hex])).unwrap(); let (st, resp) = http_post_unix(sock, "/txs/test", &body).await; @@ -582,6 +581,8 @@ mod tests { assert!(resp.contains("Exceeded maximum of 25"), "{resp}"); let (st, resp) = http_get_unix(sock, &format!("/broadcast?tx={hex}")).await; + assert_eq!(st, 404, "{resp}"); + let (st, resp) = http_post_unix(sock, "/tx", hex.as_bytes()).await; assert_eq!(st, 200, "{resp}"); assert_eq!(resp, txs[0].compute_txid().to_string()); for tx in &txs[1..] { diff --git a/crates/rbitcoin-esplora/src/server.rs b/crates/rbitcoin-esplora/src/server.rs index 1315b037d..b2eeef7fa 100644 --- a/crates/rbitcoin-esplora/src/server.rs +++ b/crates/rbitcoin-esplora/src/server.rs @@ -816,7 +816,6 @@ pub async fn run_esplora( .route("/tx/{txid}/outspend/{vout}", get(handlers::tx_outspend)) .route("/tx/{txid}/outspends", get(handlers::tx_outspends)) .route("/tx", post(handlers::post_tx)) - .route("/broadcast", get(handlers::get_broadcast)) .route("/txs/test", post(handlers::post_txs_test)) .route("/txs/outspends", get(handlers::get_txs_outspends)) .route("/txs/package", post(handlers::post_tx_package)) diff --git a/crates/rbitcoin-mempool/src/accept.rs b/crates/rbitcoin-mempool/src/accept.rs index 60a561ee0..eea462774 100644 --- a/crates/rbitcoin-mempool/src/accept.rs +++ b/crates/rbitcoin-mempool/src/accept.rs @@ -452,6 +452,15 @@ enum EvictUntil { WeightBudget, } +/// What to do with orphans of a conflict that was just put back. +#[derive(Clone, Copy)] +enum RestoreOrphans { + /// No walk is running. Start one for each restored tx. + Promote, + /// A walk is already running. Hand the restored txids back to it. + Defer, +} + struct IngestedLive { graph: TxGraph, bodies: std::collections::HashMap>, @@ -503,6 +512,15 @@ pub struct ActiveMempool { loaded_accept: Vec<(Txid, Wtxid, u64)>, } +/// A parked package member, taken out before admit. +/// +/// `announcers` are the peers who announced it. The missing-parent set is +/// not kept: it is resolved again if the package puts the tx back. +pub struct LiftedOrphan { + pub tx: Transaction, + pub announcers: Vec, +} + impl ActiveMempool { /// Share the hub's mock clock so stamped accept times follow `setmocktime`. pub fn set_node_clock(&mut self, clock: Arc) { @@ -1261,6 +1279,47 @@ impl ActiveMempool { }) } + /// Unconfirmed inputs on a replacement must already be parents of the + /// direct conflicts, and there is only one direct conflict. Spending an + /// output of a direct conflict is `bad-txns-spends-conflicting-tx`. + fn reject_new_unconfirmed( + &self, + tx: &Transaction, + direct: &BTreeSet, + ) -> Result<(), AcceptError> { + let mut parents_of_conflicts = BTreeSet::new(); + for id in direct { + let conflict = self + .bodies + .get(id) + .ok_or_else(|| AcceptError::Durable("conflict body missing".into()))?; + for inp in &conflict.input { + parents_of_conflicts.insert(inp.previous_output.txid); + } + } + for inp in &tx.input { + let parent = inp.previous_output.txid; + if !self.bodies.contains_key(&parent) { + continue; + } + if !parents_of_conflicts.contains(&parent) { + // Spending the output of a tx this replacement evicts. Core + // names that case; any other new unconfirmed parent is the + // broader Libre refusal. + if direct.contains(&parent) { + return Err(AcceptError::Policy("bad-txns-spends-conflicting-tx")); + } + return Err(AcceptError::Policy("adds unconfirmed input")); + } + if direct.len() > 1 { + return Err(AcceptError::Policy( + "unconfirmed input has multiple conflicts", + )); + } + } + Ok(()) + } + /// Returns (conflict set, fee, sigop-adjusted weight). fn plan_after_script( &self, @@ -1296,6 +1355,9 @@ impl ActiveMempool { (i128::from(fee_sat).saturating_add(i128::from(prep.fee_delta))).max(0) as u64; let conflict_set = if !scan.direct_conflicts.is_empty() { + // Libre HasNoNewUnconfirmed: parents of the direct conflicts only, + // not their ancestor closure. One direct conflict may be CPFPed. + self.reject_new_unconfirmed(tx, &scan.direct_conflicts)?; let direct: Vec = scan.direct_conflicts.into_iter().collect(); let set = self.graph.conflict_set(&direct); let (old_fee, old_weight) = self.graph.set_fee_weight(&set); @@ -1345,26 +1407,46 @@ impl ActiveMempool { *sha256::Hash::hash(script).as_byte_array() } - /// Re-try orphans that listed `parent` as missing (recursive via accept_tx_with). + /// Re-try orphans that listed `parent` as missing. /// /// Uses `accept_tx_with` (not top-level accept_tx) so stage timers accumulate /// on the parent admit that unlocked the orphan chain. Public for hub staged commit. + /// + /// A worklist, so a parked chain does not keep one frame per descendant. + /// The first child is admitted, then its descendants, then the next sibling. + /// + /// A child that still misses another parent goes back to the orphanage. + /// Conflicts put back by that failure join this same list. pub fn promote_orphans_of( &mut self, parent: Txid, utxos: &impl UtxoProvider, tip: ChainTipCtx, ) { - let children = self.orphanage.take_children_of(&parent); - for child in children { - if let Ok(r) = self.accept_tx_with(&child, utxos, tip, 0, true, None) { - self.promote_orphans_of(r.txid, utxos, tip); - } else { - self.restore_undone(utxos, tip); + let mut queue = VecDeque::new(); + self.push_orphan_children(parent, &mut queue); + while let Some(child) = queue.pop_front() { + match self.accept_tx_with(&child, utxos, tip, 0, true, None) { + Ok(r) => self.push_orphan_children(r.txid, &mut queue), + Err(e) => { + if let AcceptError::Orphaned { missing, .. } = e { + let _ = self.park_orphan(&child, missing); + } + for id in self.restore_conflicts(utxos, tip, RestoreOrphans::Defer) { + self.push_orphan_children(id, &mut queue); + } + } } } } + /// Orphans of `parent`, earlier children still ahead of later ones. + fn push_orphan_children(&mut self, parent: Txid, queue: &mut VecDeque) { + for child in self.orphanage.take_children_of(&parent).into_iter().rev() { + queue.push_front(child); + } + } + /// Ensure the durable slot table has a FREE/DEAD entry for the next append. /// /// Order: if full of LIVE, **grow** the slot table first (weight may still have @@ -1591,23 +1673,103 @@ impl ActiveMempool { None }; let mut accepted: Vec = Vec::with_capacity(txs.len()); + // Members already parked are lifted so the short-circuit for a + // re-announced orphan does not reject them. Promote waits until + // every member is committed, so this walk cannot admit a later + // member and then roll it back as a duplicate. + let mut lifted: Vec = Vec::new(); for tx in txs { - // accept_tx_with + promote (not top-level accept_tx) so stages are not reset per member. + if let Some(parked) = self.lift_parked_member(tx) { + lifted.push(parked); + } + // accept_tx_with (not top-level accept_tx) so stages are not reset per member. match self.accept_tx_with(tx, utxos, tip, 0, true, member_min) { - Ok(r) => { - self.promote_orphans_of(r.txid, utxos, tip); - accepted.push(r); - } + Ok(r) => accepted.push(r), Err(e) => { self.restore_undone(utxos, tip); self.rollback_accepted_package(&accepted, utxos, tip); + for item in self.repark_lifted(&lifted, utxos, tip, &|_| 0) { + match self.accept_tx_with(&item.tx, utxos, tip, 0, true, None) { + Ok(r) => self.promote_orphans_of(r.txid, utxos, tip), + Err(AcceptError::Orphaned { missing, .. }) => { + self.park_announced(&item.tx, missing, &item.announcers); + } + Err(_) => self.restore_undone(utxos, tip), + } + } return Err(e); } } } + let ids: Vec = accepted.iter().map(|r| r.txid).collect(); + for id in ids { + self.promote_orphans_of(id, utxos, tip); + } Ok(accepted) } + /// Take `tx` out of the orphanage so a package can admit it. + /// + /// `prepare_admit` and `plan_after_script` both refuse a tx that is + /// still parked, even when its parents are already in the mempool. + /// The announcing peers come along so a failed package can put them back. + pub fn lift_parked_member(&mut self, tx: &Transaction) -> Option { + let id = tx.compute_txid(); + self.orphanage.missing_of(&id)?; + let announcers = self.orphanage.announcers_of(&id); + let parked = self.orphanage.take_txid(&id)?; + Some(LiftedOrphan { + tx: parked, + announcers, + }) + } + + /// Put lifted members back, or hand back the ones that can be admitted. + /// + /// The missing set saved at the lift is not used. A parent can arrive + /// while the child is out of the orphanage, and parking under that parent + /// leaves the child with nobody left to wake it. Parents still absent are + /// resolved again here. `fee_delta` is the caller's `prioritisetransaction` + /// map: a child under the raw relay floor is still returned when that + /// delta clears it. Announcers are kept on either path. + pub fn repark_lifted( + &mut self, + lifted: &[LiftedOrphan], + utxos: &impl UtxoProvider, + tip: ChainTipCtx, + fee_delta: &impl Fn(&Txid) -> i64, + ) -> Vec { + let mut ready = Vec::new(); + for item in lifted { + let txid = item.tx.compute_txid(); + if self.graph.contains(&txid) { + continue; + } + match self.prepare_admit(&item.tx, utxos, tip, fee_delta(&txid), true, None) { + Err(AcceptError::Orphaned { missing, .. }) => { + self.park_announced(&item.tx, missing, &item.announcers); + } + Err(AcceptError::Duplicate(_)) => {} + Ok(_) => ready.push(LiftedOrphan { + tx: item.tx.clone(), + announcers: item.announcers.clone(), + }), + Err(_) => {} + } + } + ready + } + + /// Park `tx` under `missing` and restore every announcing peer. + fn park_announced(&mut self, tx: &Transaction, missing: BTreeSet, announcers: &[u64]) { + let txid = tx.compute_txid(); + let first = announcers.first().copied(); + let _ = self.park_orphan_from(tx, missing, first); + for peer in announcers.iter().skip(1) { + self.orphanage.add_announcer(&txid, *peer); + } + } + fn rollback_accepted_package( &mut self, accepted: &[AcceptResult], @@ -1626,7 +1788,7 @@ impl ActiveMempool { let _ = self.unlink_tree(&r.txid); } let _ = self.store.sync_deaths(); - self.restore_victims(&victims, &accept_secs, utxos, tip); + let _ = self.restore_victims(&victims, &accept_secs, utxos, tip, RestoreOrphans::Promote); } /// Parents before children, so a restored descendant can see its in-set parent. @@ -1696,25 +1858,42 @@ impl ActiveMempool { accept_secs: &std::collections::HashMap, utxos: &impl UtxoProvider, tip: ChainTipCtx, - ) { + orphans: RestoreOrphans, + ) -> Vec { + let mut noted = Vec::new(); for i in Self::ancestor_first_order(victims) { let at = accept_secs.get(&victims[i].compute_txid()).copied(); match self.reaccept_already_admitted(&victims[i], at, utxos, tip) { - Ok(r) => self.promote_orphans_of(r.txid, utxos, tip), + Ok(r) => match orphans { + RestoreOrphans::Promote => self.promote_orphans_of(r.txid, utxos, tip), + RestoreOrphans::Defer => noted.push(r.txid), + }, Err(_) => { if !self.undone_replacements.is_empty() { - self.restore_undone(utxos, tip); + noted.extend(self.restore_conflicts(utxos, tip, orphans)); } } } } + noted } /// Re-accept conflicts a failed commit already removed. fn restore_undone(&mut self, utxos: &impl UtxoProvider, tip: ChainTipCtx) { + let _ = self.restore_conflicts(utxos, tip, RestoreOrphans::Promote); + } + + /// Like [`Self::restore_undone`]. `Defer` returns the restored txids + /// instead of starting another orphan walk. + fn restore_conflicts( + &mut self, + utxos: &impl UtxoProvider, + tip: ChainTipCtx, + orphans: RestoreOrphans, + ) -> Vec { let victims = std::mem::take(&mut self.undone_replacements); let accept_secs = std::mem::take(&mut self.undone_accept_secs); - self.restore_victims(&victims, &accept_secs, utxos, tip); + self.restore_victims(&victims, &accept_secs, utxos, tip, orphans) } /// Drop one live tx from the graph and `pwrite` its slot DEAD. No `fdatasync`. @@ -1925,7 +2104,7 @@ impl ActiveMempool { fn note_invalid(&mut self, txid: Txid) { if self.recent_invalid.len() >= RECENT_INVALID_CAP { - self.recent_invalid.clear(); + return; } self.recent_invalid.insert(txid); } @@ -3615,9 +3794,164 @@ mod tests { (parent, child) } - /// Conflicts removed by a replacement that does not stay come back even - /// when the rolling floor now prices them out, including a CPFP parent - /// that cannot pay min relay alone. + /// A parent-then-child package must admit a child that was already parked + /// on that parent, and a failed package must leave that child parked. + #[test] + fn package_admits_its_parked_child() { + let dir = tmp_dir(); + let (ops, utxos) = chain_coins(&[100_000, 100_000]); + let mut mp = ActiveMempool::open_or_create(&dir).unwrap(); + let parent = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: ops[0], + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }], + output: vec![ + TxOut { + value: Amount::from_sat(40_000), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }, + TxOut { + value: Amount::from_sat(40_000), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }, + ], + }; + let parent_id = parent.compute_txid(); + let child = spend_tx( + OutPoint { + txid: parent_id, + vout: 0, + }, + 39_000, + ); + let sibling = spend_tx( + OutPoint { + txid: parent_id, + vout: 1, + }, + 39_000, + ); + let child_id = child.compute_txid(); + let sibling_id = sibling.compute_txid(); + for tx in [&child, &sibling] { + let err = mp.accept_tx(tx, &utxos, TIP_OK).unwrap_err(); + assert!( + matches!(err, AcceptError::Orphaned { .. }), + "spend of the missing parent must park, got {err}" + ); + } + let accepted = mp + .accept_package(&[parent.clone(), child.clone()], &utxos, TIP_OK) + .expect("package contains the parked child"); + assert_eq!(accepted.len(), 2); + assert!(mp.graph.contains(&parent_id)); + assert!(mp.graph.contains(&child_id)); + assert!( + mp.graph.contains(&sibling_id), + "a parked sibling that is not in the package must still be admitted" + ); + assert_eq!(mp.orphan_count(), 0); + let _ = std::fs::remove_dir_all(&dir); + + let dir = tmp_dir(); + let mut mp = ActiveMempool::open_or_create(&dir).unwrap(); + let parent = spend_tx(ops[1], 99_000); + let child = spend_tx( + OutPoint { + txid: parent.compute_txid(), + vout: 0, + }, + 98_000, + ); + let child_id = child.compute_txid(); + let err = mp.accept_tx(&child, &utxos, TIP_OK).unwrap_err(); + assert!(matches!(err, AcceptError::Orphaned { .. }), "{err}"); + let bad = spend_tx(ops[1], 100_000); + let err = mp + .accept_package(&[parent, bad], &utxos, TIP_OK) + .expect_err("second member overspends"); + assert!( + !matches!(err, AcceptError::Duplicate(id) if id == child_id), + "a failed package must not report the parked child as a duplicate, got {err}" + ); + assert!( + mp.orphanage.contains(&child_id), + "a failed package must leave the parked child in the orphanage" + ); + assert!(!mp.graph.contains(&child_id)); + let _ = std::fs::remove_dir_all(&dir); + } + + /// One parent arriving must leave an orphan that still misses the other. + #[test] + fn promote_keeps_orphan_that_still_misses_a_parent() { + let dir = tmp_dir(); + let (ops, utxos) = chain_coins(&[100_000, 100_000]); + let mut mp = ActiveMempool::open_or_create(&dir).unwrap(); + let parent_a = spend_tx(ops[0], 99_000); + let parent_b = spend_tx(ops[1], 99_000); + let child = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![ + TxIn { + previous_output: OutPoint { + txid: parent_a.compute_txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }, + TxIn { + previous_output: OutPoint { + txid: parent_b.compute_txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }, + ], + output: vec![TxOut { + value: Amount::from_sat(197_000), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }], + }; + let child_id = child.compute_txid(); + let err = mp.accept_tx(&child, &utxos, TIP_OK).unwrap_err(); + assert!( + matches!(err, AcceptError::Orphaned { .. }), + "child waits on both parents, got {err}" + ); + mp.accept_tx(&parent_a, &utxos, TIP_OK) + .expect("first parent"); + assert!( + mp.orphanage.contains(&child_id), + "child still missing the second parent must stay parked" + ); + assert!(!mp.graph.contains(&child_id)); + mp.accept_tx(&parent_b, &utxos, TIP_OK) + .expect("second parent"); + assert!( + mp.graph.contains(&child_id), + "both parents present must admit the parked child" + ); + assert_eq!(mp.orphan_count(), 0); + let _ = std::fs::remove_dir_all(&dir); + } + + /// Conflicts removed by a replacement come back after that replacement's + /// package child is evicted, even when the rolling floor now prices them + /// out. Includes a CPFP parent that cannot pay min relay alone. + /// + /// The child spends the low-fee anchor. The replacement does not: the + /// anchor is not a parent of the conflict. #[test] fn failed_replacement_restores_below_floor_and_cpfp_set() { let value = 50_000_000u64; @@ -3644,26 +3978,24 @@ mod tests { mp.accept_tx(&anchor, &utxos, TIP_OK).expect("anchor"); mp.accept_tx(&victim, &utxos, TIP_OK) .expect("victim at the pre-eviction floor"); - let replacement = heavy_conflict( - low_op, - OutPoint { - txid: anchor.compute_txid(), - vout: 0, - }, - value + anchor.output[0].value.to_sat(), - 30_000, - victim_wu + 500, + let replacement = spend_tx_fee(low_op, value, 30_000); + let replacement_id = replacement.compute_txid(); + let held = anchor_wu + replacement.weight().to_wu(); + assert!( + held <= mp.max_weight, + "replacement must fit beside the anchor" ); + let child = child_evicted_with_anchor(&replacement, &anchor, 50_000, mp.max_weight - held); let err = mp - .accept_tx(&replacement, &utxos, TIP_OK) - .expect_err("replacement over budget"); + .accept_package(&[replacement, child], &utxos, TIP_OK) + .expect_err("child over budget"); assert!(matches!(err, AcceptError::Policy("mempool full")), "{err}"); let enforced = mp.mempool_min_fee_sat_kvb(); assert!( enforced > victim_rate, "eviction must price out the victim ({victim_rate} vs {enforced})" ); - assert!(!mp.graph.contains(&replacement.compute_txid())); + assert!(!mp.graph.contains(&replacement_id)); assert!( mp.graph.contains(&victim_id), "below-floor conflict must stay" @@ -3691,19 +4023,16 @@ mod tests { mp.accept_tx(&anchor, &utxos, TIP_OK).expect("anchor"); mp.accept_package(&[parent.clone(), child.clone()], &utxos, TIP_OK) .expect("CPFP package"); - let replacement = heavy_conflict( - parent_op, - OutPoint { - txid: anchor.compute_txid(), - vout: 0, - }, - value + anchor.output[0].value.to_sat(), - 90_000, - parent.weight().to_wu() + child.weight().to_wu() + 500, + let replacement = spend_tx_fee(parent_op, value, 90_000); + let held = anchor.weight().to_wu() + replacement.weight().to_wu(); + assert!( + held <= mp.max_weight, + "replacement must fit beside the anchor" ); + let over = child_evicted_with_anchor(&replacement, &anchor, 50_000, mp.max_weight - held); let err = mp - .accept_tx(&replacement, &utxos, TIP_OK) - .expect_err("replacement over budget"); + .accept_package(&[replacement, over], &utxos, TIP_OK) + .expect_err("child over budget"); assert!(matches!(err, AcceptError::Policy("mempool full")), "{err}"); assert!( mp.graph.contains(&parent.compute_txid()), @@ -3747,19 +4076,16 @@ mod tests { mp.accept_tx(&anchor, &utxos, TIP_OK).expect("anchor"); mp.accept_tx(&victim, &utxos, TIP_OK).expect("victim"); clock.store(t0 + 600, Ordering::Relaxed); - let replacement = heavy_conflict( - low_op, - OutPoint { - txid: anchor.compute_txid(), - vout: 0, - }, - value + anchor.output[0].value.to_sat(), - 30_000, - victim_wu + 500, + let replacement = spend_tx_fee(low_op, value, 30_000); + let held = anchor_wu + replacement.weight().to_wu(); + assert!( + held <= mp.max_weight, + "replacement must fit beside the anchor" ); + let child = child_evicted_with_anchor(&replacement, &anchor, 50_000, mp.max_weight - held); let err = mp - .accept_tx(&replacement, &utxos, TIP_OK) - .expect_err("replacement over budget"); + .accept_package(&[replacement, child], &utxos, TIP_OK) + .expect_err("child over budget"); assert!(matches!(err, AcceptError::Policy("mempool full")), "{err}"); assert!(mp.graph.contains(&victim_id), "conflict restored"); mp.store.flush().unwrap(); @@ -3861,27 +4187,37 @@ mod tests { vsize.saturating_mul(sat_kvb).div_ceil(1000).max(1) } - fn heavy_conflict( - conflict: OutPoint, - anchor: OutPoint, - input_value: u64, + /// Package child of `replacement` that also spends `anchor`. + /// + /// Weight above `over_wu` (the room left after the replacement sits beside + /// the anchor) makes eviction drop this child with the anchor. + fn child_evicted_with_anchor( + replacement: &Transaction, + anchor: &Transaction, fee: u64, - min_wu: u64, + over_wu: u64, ) -> Transaction { - let mut n = 64usize; + let input_value = replacement.output[0].value.to_sat() + anchor.output[0].value.to_sat(); + let mut n = 1usize; loop { let tx = Transaction { version: Version::TWO, lock_time: LockTime::ZERO, input: vec![ TxIn { - previous_output: conflict, + previous_output: OutPoint { + txid: replacement.compute_txid(), + vout: 0, + }, script_sig: ScriptBuf::new(), sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, witness: Witness::new(), }, TxIn { - previous_output: anchor, + previous_output: OutPoint { + txid: anchor.compute_txid(), + vout: 0, + }, script_sig: ScriptBuf::new(), sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, witness: Witness::new(), @@ -3892,11 +4228,11 @@ mod tests { script_pubkey: ScriptBuf::from_bytes(vec![0x51; n]), }], }; - if tx.weight().to_wu() > min_wu { + if tx.weight().to_wu() > over_wu { return tx; } n += 32; - assert!(n < 20_000, "conflict replacement never got heavy enough"); + assert!(n < 10_000, "package child never got heavy enough"); } } @@ -4119,6 +4455,32 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// Filling the invalid set must not erase the oldest id. A new id past + /// the cap is not stored. + #[test] + fn recent_invalid_keeps_the_oldest() { + let dir = tmp_dir(); + let mut mp = ActiveMempool::open_or_create(&dir).unwrap(); + let first = Txid::from_byte_array({ + let mut b = [0u8; 32]; + b[0] = 1; + b + }); + mp.note_invalid(first); + // 2..4097 is 4095 ids and does not collide with `first`. + for i in 2..4_097u32 { + let mut b = [0u8; 32]; + b[0..4].copy_from_slice(&i.to_le_bytes()); + mp.note_invalid(Txid::from_byte_array(b)); + } + let extra = Txid::from_byte_array([0xff; 32]); + mp.note_invalid(extra); + assert!(mp.recent_invalid.contains(&first)); + assert!(!mp.recent_invalid.contains(&extra)); + assert_eq!(mp.recent_invalid.len(), RECENT_INVALID_CAP); + let _ = std::fs::remove_dir_all(&dir); + } + /// Free-slot eviction. A session reaches it only when the slot table is /// at `MAX_SLOT_CAP` (1_048_576) and every row is live. The pin pretends /// the table is already there. Weight-budget eviction is the full-pool diff --git a/crates/rbitcoin-mempool/src/accept_life_journey.rs b/crates/rbitcoin-mempool/src/accept_life_journey.rs index b70e0927e..e938f6705 100644 --- a/crates/rbitcoin-mempool/src/accept_life_journey.rs +++ b/crates/rbitcoin-mempool/src/accept_life_journey.rs @@ -248,9 +248,8 @@ fn full_rbf_and_staged_commit(life: &mut Life) -> (OutPoint, Txid) { } /// Pure RBFR: fee 1,500 loses BIP125's absolute fee to the 2,000 parent and -/// child, but pays 1.25x the direct conflict and unpins both. Then a merger -/// of 32 parents replaces their 32 children: 33 members fit the 64 cap only -/// because replaced txs leave the count. +/// child, but pays 1.25x the direct conflict and unpins both. A merger that +/// spends two unconfirmed parents is rejected: more than one direct conflict. fn replacements_unpin_and_leave_the_cluster_count(life: &mut Life) { let f = life.chain.fund(1_000_000); let pinned = spend_tx(f, 999_000); @@ -261,21 +260,39 @@ fn replacements_unpin_and_leave_the_cluster_count(life: &mut Life) { assert_eq!(r.replaced.len(), 2); assert!(!life.live(&pin_child)); - let mut parent_outs = Vec::new(); - let mut kids = Vec::new(); - for _ in 0..MAX_CLUSTER_COUNT / 2 { - let p = spend_tx(life.chain.fund(100_000), 90_000); + let mut pairs = Vec::new(); + for _ in 0..2 { + let coin = life.chain.fund(100_000); + let p = spend_tx(coin, 90_000); let k = spend_tx(out(p.compute_txid(), 0), 80_000); life.accept(&p).unwrap(); life.accept(&k).unwrap(); - parent_outs.push(out(p.compute_txid(), 0)); - kids.push(k); + pairs.push((coin, p, k)); } + let parent_outs: Vec = pairs + .iter() + .map(|(_, p, _)| out(p.compute_txid(), 0)) + .collect(); let merger = spend_all(&parent_outs, 10_000); - life.accept(&merger) - .expect("replaced children must not count toward the cluster cap"); - assert_eq!(life.cluster_len(&merger), MAX_CLUSTER_COUNT / 2 + 1); - assert!(kids.iter().all(|k| !life.live(k))); + let err = life.accept(&merger).unwrap_err(); + assert!( + matches!( + err, + AcceptError::Policy("unconfirmed input has multiple conflicts") + ), + "{err}" + ); + for (coin, p, k) in &pairs { + assert!(life.live(k)); + let bump = spend_tx(out(p.compute_txid(), 0), 10_000); + life.accept(&bump) + .expect("one conflict may spend that conflict's parent"); + assert!(!life.live(k)); + let gone = spend_tx(*coin, 1); + life.accept(&gone).expect("confirmed-input RBF"); + assert!(!life.live(p)); + assert!(!life.live(&bump)); + } } /// 3,000 outputs is ~30 kvB: over the old 101 kWU bug, under the 101 kvB @@ -390,8 +407,8 @@ fn reorg_readmits_and_evicts_nonfinal(life: &mut Life) { /// With -minrelaytxfee at 50 sat/vB a parent below it goes to the extra /// pool. A one-sat child does not carry it; a paying child with a confirmed /// second input does. An unrelated low-fee tx does not ride a package's -/// waiver. A child that fails after its waived parent promoted a parked -/// spender takes the spender down with the parent. +/// waiver. A child that fails after its parent was admitted leaves a +/// parked spender of that parent in the orphanage. fn raised_min_relay_one_parent_one_child(life: &mut Life) { life.mp.set_min_relay_sat_kvb(50_000); let lp = fan_out(life.chain.fund(100_000), &[49_900, 49_900]); @@ -430,7 +447,8 @@ fn raised_min_relay_one_parent_one_child(life: &mut Life) { let bad = with_annex(spend_tx(out(two.compute_txid(), 1), 1_000)); let err = life.package(&[two, bad]).unwrap_err(); assert!(matches!(err, AcceptError::Policy("libre annex")), "{err}"); - assert_eq!((life.mp.live_count(), life.mp.orphan_count()), (0, 0)); + assert_eq!((life.mp.live_count(), life.mp.orphan_count()), (0, 1)); + assert!(life.mp.orphanage.contains(&spender.compute_txid())); } /// Room for three 61 vB txs. Over budget with the new tx as the worst @@ -728,6 +746,121 @@ fn sigop_adjusted_size_budget_and_reopen() { let _ = std::fs::remove_dir_all(&dir); } +/// A parked chain that fills the cluster cap promotes when the root is +/// admitted. Every descendant is live, in one cluster. +#[test] +fn orphan_chain_promotes_to_the_cluster_cap() { + let dir = tmp_dir(); + let mut life = Life { + mp: ActiveMempool::open_or_create(&dir).unwrap(), + chain: LifeChain::default(), + tip: ChainTipCtx { + height: 200, + mtp: u32::MAX, + }, + }; + let descendants = MAX_CLUSTER_COUNT - 1; + let mut value = 100_000 + (descendants as u64) * 1_000; + let root = spend_tx(life.chain.fund(value + 1_000), value); + let mut parent = root.compute_txid(); + let mut parked = Vec::with_capacity(descendants); + for _ in 0..descendants { + value -= 1_000; + let child = spend_tx(out(parent, 0), value); + parent = child.compute_txid(); + parked.push(child); + } + for child in parked.iter().rev() { + let err = life.accept(child).unwrap_err(); + assert!( + matches!(err, AcceptError::Orphaned { fresh: true, .. }), + "{err}" + ); + } + assert_eq!(life.mp.orphan_count(), descendants); + life.accept(&root).expect("root promotes the chain"); + assert_eq!(life.mp.orphan_count(), 0); + assert_eq!(life.mp.live_count(), MAX_CLUSTER_COUNT); + assert_eq!(life.cluster_len(&root), MAX_CLUSTER_COUNT); + assert!(life.live(parked.last().unwrap())); + let _ = std::fs::remove_dir_all(&dir); +} + +/// Libre `HasNoNewUnconfirmed`: an unconfirmed input must already be a parent +/// of the direct conflicts, and there is only one direct conflict. A +/// single-conflict CPFP of that parent stays allowed. +#[test] +fn replacement_unconfirmed_inputs_follow_libre_rule() { + let dir = tmp_dir(); + let mut life = Life { + mp: ActiveMempool::open_or_create(&dir).unwrap(), + chain: LifeChain::default(), + tip: ChainTipCtx { + height: 200, + mtp: u32::MAX, + }, + }; + + let coin = life.chain.fund(200_000); + let parent = spend_tx(coin, 190_000); + life.accept(&parent).unwrap(); + let child = spend_tx(out(parent.compute_txid(), 0), 180_000); + life.accept(&child).unwrap(); + let bump = spend_tx(out(parent.compute_txid(), 0), 100_000); + life.accept(&bump) + .expect("one conflict may spend that conflict's parent"); + assert!(!life.live(&child)); + assert!(life.live(&parent)); + + let c = life.chain.fund(100_000); + let extra = life.chain.fund(100_000); + let extra_tx = spend_tx(extra, 90_000); + life.accept(&extra_tx).unwrap(); + let low = spend_tx(c, 90_000); + life.accept(&low).unwrap(); + let high = spend_all(&[c, out(extra_tx.compute_txid(), 0)], 50_000); + let err = life.accept(&high).unwrap_err(); + assert!( + matches!(err, AcceptError::Policy("adds unconfirmed input")), + "{err}" + ); + assert!(life.live(&low)); + assert!(life.live(&extra_tx)); + + let own = life.chain.fund(100_000); + let victim = spend_tx(own, 90_000); + life.accept(&victim).unwrap(); + let spends_victim = spend_all(&[own, out(victim.compute_txid(), 0)], 50_000); + let err = life.accept(&spends_victim).unwrap_err(); + assert!( + matches!(err, AcceptError::Policy("bad-txns-spends-conflicting-tx")), + "{err}" + ); + assert!(life.live(&victim)); + + let wide_coin = life.chain.fund(300_000); + let wide = fan_out(wide_coin, &[100_000, 100_000]); + life.accept(&wide).unwrap(); + let k0 = spend_tx(out(wide.compute_txid(), 0), 90_000); + let k1 = spend_tx(out(wide.compute_txid(), 1), 90_000); + life.accept(&k0).unwrap(); + life.accept(&k1).unwrap(); + let merger = spend_all( + &[out(wide.compute_txid(), 0), out(wide.compute_txid(), 1)], + 50_000, + ); + let err = life.accept(&merger).unwrap_err(); + assert!( + matches!( + err, + AcceptError::Policy("unconfirmed input has multiple conflicts") + ), + "{err}" + ); + assert!(life.live(&k0) && life.live(&k1)); + let _ = std::fs::remove_dir_all(&dir); +} + /// One mempool from the first orphan to a full pool: relay order, RBF, /// cluster caps, packages, a block, a reorg, a raised `-minrelaytxfee`, and /// eviction, against one chain view. diff --git a/crates/rbitcoin-mempool/src/lib.rs b/crates/rbitcoin-mempool/src/lib.rs index 99d64dd2a..ab6e9fd17 100644 --- a/crates/rbitcoin-mempool/src/lib.rs +++ b/crates/rbitcoin-mempool/src/lib.rs @@ -50,7 +50,7 @@ pub mod testutil; pub use accept::{ check_mempool_structural, AcceptError, AcceptFailureRecord, AcceptResult, AcceptStageUs, - ActiveMempool, ChainPrevout, ChainTipCtx, Coin, PreparedAdmit, UtxoProvider, + ActiveMempool, ChainPrevout, ChainTipCtx, Coin, LiftedOrphan, PreparedAdmit, UtxoProvider, DEFAULT_MAX_MEMPOOL_WEIGHT, MAX_PACKAGE_COUNT, MAX_PACKAGE_WEIGHT, }; pub use error::MempoolError; diff --git a/crates/rbitcoin-mempool/src/orphanage.rs b/crates/rbitcoin-mempool/src/orphanage.rs index ad76c8f20..2b54cbf6f 100644 --- a/crates/rbitcoin-mempool/src/orphanage.rs +++ b/crates/rbitcoin-mempool/src/orphanage.rs @@ -317,10 +317,8 @@ impl Orphanage { false } - fn remove_txid(&mut self, txid: &Txid) { - let Some(e) = self.by_txid.remove(txid) else { - return; - }; + fn remove_txid(&mut self, txid: &Txid) -> Option { + let e = self.by_txid.remove(txid)?; self.by_wtxid.remove(&e.wtxid); self.unaccount(&e); for p in &e.missing { @@ -332,6 +330,12 @@ impl Orphanage { } } // fifo may still hold txid if removed mid-list; leave stale ids (skipped on pop) + Some(e.tx) + } + + /// Remove one parked tx and return it. `None` when it is not parked. + pub fn take_txid(&mut self, txid: &Txid) -> Option { + self.remove_txid(txid) } /// Take all orphans that listed `parent` as missing (for re-accept). diff --git a/crates/rbitcoin-net/src/ibd/dial.rs b/crates/rbitcoin-net/src/ibd/dial.rs index 3e783cc07..5f5de6d78 100644 --- a/crates/rbitcoin-net/src/ibd/dial.rs +++ b/crates/rbitcoin-net/src/ibd/dial.rs @@ -265,6 +265,7 @@ pub(crate) async fn dial_batch( cancel: Option>, dialer: crate::socks::Dialer, clock: &rbitcoin_consensus::NodeClock, + perms: Option>, ) -> DialBatchResult { let mut out = DialBatchResult { slots: Vec::new(), @@ -303,8 +304,11 @@ pub(crate) async fn dial_batch( ); let dialer = dialer.clone(); let to = connect_timeout_for(addr, connect_timeout); + let dial_perms = perms.clone(); handles.push(tokio::spawn(async move { - let fut = spawn_peer(id, addr, magic, local_addr, tip_h, sinks, dialer); + let fut = spawn_peer( + id, addr, magic, local_addr, tip_h, sinks, dialer, dial_perms, + ); match tokio::time::timeout(to, fut).await { Ok(Ok(slot)) => Ok(slot), Ok(Err(e)) => { @@ -1399,6 +1403,7 @@ mod tests { None, crate::socks::Dialer::Direct, &rbitcoin_consensus::NodeClock::new(), + None, )); assert!(r.slots.is_empty() && r.failed.is_empty()); let r2 = rt.block_on(dial_batch( @@ -1414,10 +1419,56 @@ mod tests { None, crate::socks::Dialer::Direct, &rbitcoin_consensus::NodeClock::new(), + None, )); assert!(r2.slots.is_empty() && r2.failed.is_empty()); } + /// IBD dials do not go through `prepare_outbound_session`. + #[tokio::test] + async fn ibd_dial_refuses_a_discouraged_net() { + let peers = crate::peers::PeerHub::new(); + peers.set_mock_now(1_700_000_000); + let onion = crate::NetAddr::Onion { + pk: [4; 32], + port: 8333, + }; + peers.note_misbehavior_key(crate::peers::DiscourageKey::from_net(onion).unwrap()); + let other_port = crate::NetAddr::Onion { + pk: [4; 32], + port: 9735, + }; + let (body_tx, _body_rx) = tokio::sync::mpsc::unbounded_channel(); + let (ctrl_tx, _ctrl_rx) = tokio::sync::mpsc::unbounded_channel(); + let sinks = PeerEventSinks { + body: body_tx, + ctrl: ctrl_tx, + header_fks: super::super::peer_io::empty_header_fk_bus(), + }; + let result = tokio::time::timeout( + Duration::from_millis(200), + super::super::peer_io::spawn_peer( + 1, + other_port, + Magic::REGTEST, + addr(1), + Some(0), + sinks, + crate::socks::Dialer::Direct, + Some(peers), + ), + ) + .await; + let err = match result.expect("a refused IBD dial returns before the connect") { + Err(e) => e, + Ok(_) => panic!("IBD dial connected to a discouraged onion"), + }; + assert!( + matches!(err, NetError::Protocol("discouraged")), + "IBD dialed a discouraged onion on another port, got {err}" + ); + } + #[test] fn alive_dial_addrs_skips_dead() { let a = addr(1); diff --git a/crates/rbitcoin-net/src/ibd/events/confirm_reject_tests.rs b/crates/rbitcoin-net/src/ibd/events/confirm_reject_tests.rs index 4035c741c..e89f7f0dd 100644 --- a/crates/rbitcoin-net/src/ibd/events/confirm_reject_tests.rs +++ b/crates/rbitcoin-net/src/ibd/events/confirm_reject_tests.rs @@ -234,6 +234,39 @@ fn missing_header_consensus_reject_is_not_blacklisted() { let _ = std::fs::remove_dir_all(dir); } +/// BadPrev on the block that spends the current tip is a missing parent +/// slot, not a fork. The same hash stays on the path and densify walks +/// back to it. Dropping it leaves `asked` empty until the pump deadline. +#[test] +fn bad_prev_on_the_tip_child_is_fetched_again() { + let (dir, hub) = crate::chain::tiny_regtest_hub_labeled("bad-prev-tip-child"); + hub.ensure_genesis().unwrap(); + let tip = hub.tip_hash().unwrap(); + let t = hub.tip_height().unwrap(); + let hash = h(0x44); + let mut st = IbdWorkState::new(Vec::new(), Some(tip), Some(t)); + st.record_height(hash, t + 1); + st.note_path_prev(hash, tip); + st.densify_scan_lo = t + 5; + st.densify_ready_through = t + 5; + apply_confirm_reject( + &mut st, + t + 1, + hash, + "consensus: unexpected previous header", + Some(hub.query.as_ref()), + Some(&hub), + ); + assert_eq!(st.height_to_hash.get(&(t + 1)), Some(&hash)); + assert!(st.body.is_missing(&hash)); + assert!( + st.densify_scan_lo <= t + 1, + "scan_lo={}", + st.densify_scan_lo + ); + let _ = std::fs::remove_dir_all(dir); +} + /// `body.rejected ⊆ consensus-invalid set` — Cascade / SoftWire / EngineFault /// / Cancelled never blacklist. #[test] diff --git a/crates/rbitcoin-net/src/ibd/events/mod.rs b/crates/rbitcoin-net/src/ibd/events/mod.rs index d2a6d8587..42e0295bb 100644 --- a/crates/rbitcoin-net/src/ibd/events/mod.rs +++ b/crates/rbitcoin-net/src/ibd/events/mod.rs @@ -1087,6 +1087,9 @@ fn apply_soft_wire_reject( sender: Option, ) { let bad_prev = super::reorg::is_bad_prev_err(err); + // A BadPrev whose parent is the current tip is a missing header slot, + // not a competing fork. Keep the hash so densify asks for it again. + let retry_tip_child = bad_prev && prev_is_hub_tip(st, hub, hash); if bad_prev { st.headers_done = false; if let Some(h) = hub { @@ -1096,7 +1099,7 @@ fn apply_soft_wire_reject( if rewound { return; } - if st.height_to_hash.get(&height) == Some(&hash) { + if !retry_tip_child && st.height_to_hash.get(&height) == Some(&hash) { st.height_to_hash.remove(&height); remove_from_ordered(&mut st.ordered, &mut st.ordered_set, hash); } @@ -1118,11 +1121,11 @@ fn apply_soft_wire_reject( } } } - if !bad_prev { + if !bad_prev || retry_tip_child { // `isolate_if_batched` leaves SoftWire only for a one-block wave, so // `hash` is the block that failed and `sender` sent that copy. The // live pending sender may be a replacement's. - if crate::chain::reject_is_mutated(err) { + if !retry_tip_child && crate::chain::reject_is_mutated(err) { if let Some(peer) = sender { punish_mutated_sender(st, peer, hash); } @@ -1136,7 +1139,13 @@ fn apply_soft_wire_reject( st.reopen_for_densify(&[hash]); st.body.demote_known(hash); st.reopen_for_densify(&[hash]); - warn!("ibd: confirm reject soft @{height} {hash}: {err} (re-getdata, not blacklisted)"); + if retry_tip_child { + warn!( + "ibd: confirm reject BadPrev @{height} {hash}: {err} (parent is tip, re-getdata)" + ); + } else { + warn!("ibd: confirm reject soft @{height} {hash}: {err} (re-getdata, not blacklisted)"); + } } else { warn!( "ibd: confirm reject BadPrev @{height} {hash}: {err} (slot evicted, not re-get same hash)" @@ -1144,6 +1153,25 @@ fn apply_soft_wire_reject( } } +/// True when `hash` chains directly off the hub tip. +fn prev_is_hub_tip( + st: &IbdWorkState, + hub: Option<&crate::chain::ChainHub>, + hash: BlockHash, +) -> bool { + let Some(h) = hub else { + return false; + }; + let Some(tip) = h.tip_hash() else { + return false; + }; + st.path_prev + .get(&hash) + .copied() + .or_else(|| h.prev_of(&hash)) + == Some(tip) +} + /// Core `MaybePunishNodeForBlock` (`BLOCK_MUTATED`): drop the sender and /// cool down its address so the re-get goes to another peer. Redial falls /// back to a cooling address when no other is dialable. A noban peer stays. diff --git a/crates/rbitcoin-net/src/ibd/mod.rs b/crates/rbitcoin-net/src/ibd/mod.rs index 9f376c547..dc0610a5c 100644 --- a/crates/rbitcoin-net/src/ibd/mod.rs +++ b/crates/rbitcoin-net/src/ibd/mod.rs @@ -360,6 +360,7 @@ pub async fn ibd_cancellable( cancel.as_ref().map(Arc::clone), cfg.dialer.clone(), &hub.clock, + perms.clone(), ) .await; let mut boot_cooldown = HashMap::new(); @@ -849,10 +850,11 @@ pub async fn ibd_cancellable( let cancel_c = cancel.as_ref().map(Arc::clone); let dialer_c = cfg.dialer.clone(); let clock_c = Arc::clone(&hub.clock); + let perms_c = st.perms.clone(); redial_handle = Some(tokio::spawn(async move { dial_batch( &book, want, already, &occupied, magic, local_addr, tip_h, sinks_r, cto, - cancel_c, dialer_c, &clock_c, + cancel_c, dialer_c, &clock_c, perms_c, ) .await })); diff --git a/crates/rbitcoin-net/src/ibd/path.rs b/crates/rbitcoin-net/src/ibd/path.rs index f9d95caa4..f97ed8ac6 100644 --- a/crates/rbitcoin-net/src/ibd/path.rs +++ b/crates/rbitcoin-net/src/ibd/path.rs @@ -193,7 +193,7 @@ fn plant_find_valid_child( if st.reorg.invalid.contains(h.to_byte_array()) || st.body.is_rejected(&h) { continue; } - let Some(prev) = st.path_prev.get(&h).copied() else { + let Some(prev) = st.path_prev.get(&h).copied().or_else(|| hub.prev_of(&h)) else { continue; }; if prev != tip_hash { @@ -258,12 +258,12 @@ fn plant_hash_at_height(st: &IbdWorkState, ht: u32) -> Option { fn plant_next_connected( st: &IbdWorkState, - _hub: &ChainHub, + hub: &ChainHub, parent: BlockHash, ht: u32, ) -> Option { let n = plant_hash_at_height(st, ht)?; - let p = st.path_prev.get(&n).copied()?; + let p = st.path_prev.get(&n).copied().or_else(|| hub.prev_of(&n))?; (p == parent).then_some(n) } @@ -508,6 +508,64 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + /// A sibling known only from the header store (no `path_prev` yet) is + /// still the tip child. The unknown-txid IBD pump depends on this after + /// the active tip+1 is marked invalid. + #[test] + fn plant_valid_tip_child_uses_stored_prev_without_path_prev() { + use bitcoin::absolute::LockTime; + use bitcoin::block::{Header, Version}; + use bitcoin::script::ScriptBuf; + use bitcoin::transaction::Version as TxVersion; + use bitcoin::{ + Amount, Block, CompactTarget, OutPoint, Sequence, Transaction, TxIn, TxOut, Witness, + }; + + let (dir, hub) = crate::chain::tiny_regtest_hub_labeled("plant-stored-prev"); + hub.ensure_genesis().unwrap(); + let mine = |prev: BlockHash, time: u32, height: u32| { + let mut ss = rbitcoin_consensus::bip34_height_script(height); + while ss.len() < 2 { + ss.push(0x00); + } + let coinbase = Transaction { + version: TxVersion::ONE, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint::null(), + script_sig: ScriptBuf::from_bytes(ss), + sequence: Sequence::MAX, + witness: Witness::new(), + }], + output: vec![TxOut { + value: Amount::from_sat(50_0000_0000), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }], + }; + let mut block = Block { + header: Header { + version: Version::from_consensus(4), + prev_blockhash: prev, + merkle_root: bitcoin::TxMerkleNode::from_byte_array([0u8; 32]), + time, + bits: CompactTarget::from_consensus(0x207f_ffff), + nonce: 0, + }, + txdata: vec![coinbase], + }; + block.header.merkle_root = block.compute_merkle_root().unwrap(); + rbitcoin_consensus::grind_regtest_pow(&mut block.header); + block + }; + let child = mine(hub.tip_hash().unwrap(), 1_300_000_600, 1); + hub.ensure_header(&child.header).unwrap(); + let mut st = IbdWorkState::new(Vec::new(), hub.tip_hash(), hub.tip_height()); + st.hash_height.insert(child.block_hash(), 1); + super::plant_valid_tip_child(&mut st, &hub); + assert_eq!(st.height_to_hash.get(&1), Some(&child.block_hash())); + let _ = std::fs::remove_dir_all(dir); + } + #[test] fn work_path_tips_respects_live_set_only() { let mut st = IbdWorkState::new(Vec::new(), None, Some(1)); diff --git a/crates/rbitcoin-net/src/ibd/peer_io.rs b/crates/rbitcoin-net/src/ibd/peer_io.rs index b77e49590..584b2aa67 100644 --- a/crates/rbitcoin-net/src/ibd/peer_io.rs +++ b/crates/rbitcoin-net/src/ibd/peer_io.rs @@ -895,6 +895,7 @@ async fn write_ibd_peer( } } +#[allow(clippy::too_many_arguments)] // call-site args stay unbundled pub(crate) async fn spawn_peer( id: usize, addr: crate::NetAddr, @@ -903,7 +904,11 @@ pub(crate) async fn spawn_peer( tip_h: Option, sinks: PeerEventSinks, dialer: crate::socks::Dialer, + perms: Option>, ) -> Result { + if perms.as_ref().is_some_and(|p| p.net_discouraged(addr)) { + return Err(NetError::Protocol("discouraged")); + } let stream = dialer.connect_net(addr).await?; let version_socket = addr .socket_addr() diff --git a/crates/rbitcoin-net/src/peer.rs b/crates/rbitcoin-net/src/peer.rs index 58170cdc1..642c02a2b 100644 --- a/crates/rbitcoin-net/src/peer.rs +++ b/crates/rbitcoin-net/src/peer.rs @@ -145,31 +145,22 @@ async fn accept_received_from_peer( }) } -/// The hub and address a misbehavior exit may record. Noban and manual peers -/// are not recorded. A loopback address is every local connection, so Core -/// disconnects that peer and does not discourage the address. +/// The hub and key a misbehavior exit may record. Noban and manual peers +/// are not recorded. A loopback or `0.0.0.0` identity is every local +/// connection, so Core disconnects that peer and does not discourage it. +/// Onion and I2P use [`crate::peers::LivePeer::net`], not that socket. fn refusable_addr( session: Option<&crate::peers::LivePeer>, -) -> Option<(Arc, std::net::IpAddr)> { +) -> Option<(Arc, crate::peers::DiscourageKey)> { let s = session.filter(|s| !misbehavior_exempt(s))?; - let ip = s.addr.ip(); - if local_addr_skips_discourage(ip) { - return None; - } - Some((s.peer_hub()?, ip)) + let key = crate::peers::DiscourageKey::from_net(s.net)?; + Some((s.peer_hub()?, key)) } /// One-day in-memory refusal. fn note_threshold_refusal(session: Option<&crate::peers::LivePeer>) { - if let Some((hub, ip)) = refusable_addr(session) { - hub.note_misbehavior_addr(ip); - } -} - -fn local_addr_skips_discourage(ip: std::net::IpAddr) -> bool { - match ip { - std::net::IpAddr::V4(v) => v.is_loopback() || v.octets()[0] == 0, - std::net::IpAddr::V6(v) => v.is_loopback(), + if let Some((hub, key)) = refusable_addr(session) { + hub.note_misbehavior_key(key); } } @@ -184,7 +175,7 @@ fn threshold_disconnect(session: Option<&crate::peers::LivePeer>) -> NetError { /// second one within [`crate::peers::PeerHub::RATE_STRIKE_SECS`] is a /// threshold exit, as two overflows in one session were. fn rate_limit_disconnect(session: Option<&crate::peers::LivePeer>) -> NetError { - if refusable_addr(session).is_some_and(|(hub, ip)| hub.note_rate_strike(ip)) { + if refusable_addr(session).is_some_and(|(hub, key)| hub.note_rate_strike(key)) { return threshold_disconnect(session); } NetError::Protocol("peer rate limit") diff --git a/crates/rbitcoin-net/src/peer_tests.rs b/crates/rbitcoin-net/src/peer_tests.rs index 2238e8d0b..c15bbab9a 100644 --- a/crates/rbitcoin-net/src/peer_tests.rs +++ b/crates/rbitcoin-net/src/peer_tests.rs @@ -4054,6 +4054,76 @@ fn misbehavior_disconnect_does_not_refuse_loopback() { ); } +/// An onion session's VERSION socket is `0.0.0.0`. The refusal is the +/// `NetAddr`, and a later session with that net is discouraged. +#[test] +fn overlay_misbehavior_discourages_that_net() { + let peers = crate::peers::PeerHub::new(); + peers.set_mock_now(1_700_000_000); + let onion = crate::NetAddr::Onion { + pk: [9; 32], + port: 8333, + }; + let sock = std::net::SocketAddr::from(([0, 0, 0, 0], 8333)); + let ver = bitcoin::p2p::message_network::VersionMessage { + version: 70016, + services: bitcoin::p2p::ServiceFlags::NETWORK, + timestamp: 0, + receiver: bitcoin::p2p::address::Address::new(&sock, bitcoin::p2p::ServiceFlags::NONE), + sender: bitcoin::p2p::address::Address::new(&sock, bitcoin::p2p::ServiceFlags::NONE), + nonce: 9, + user_agent: "/rbitcoin:test/".into(), + start_height: 0, + relay: true, + }; + let peer = peers.register_net( + sock, + onion, + sock, + &ver, + true, + crate::peers::PeerConnType::Inbound, + ); + let mut score = 0u32; + punish_disconnect(&mut score, Some(peer.as_ref())); + assert!( + peers.net_discouraged(onion), + "a misbehavior disconnect refuses that overlay net" + ); + assert!( + !peers.inbound_discouraged(sock), + "0.0.0.0 must not become a clearnet refusal" + ); +} + +/// The refusal is the onion or I2P identity. Another port is the same peer. +#[test] +fn overlay_refusal_ignores_the_port() { + let peers = crate::peers::PeerHub::new(); + peers.set_mock_now(1_700_000_000); + let onion = |port| crate::NetAddr::Onion { pk: [9; 32], port }; + let key = crate::peers::DiscourageKey::from_net(onion(8333)).unwrap(); + peers.note_misbehavior_key(key); + assert!(peers.net_discouraged(onion(8333))); + assert!( + peers.net_discouraged(onion(9735)), + "the same onion on another port is still refused" + ); + assert!(!peers.net_discouraged(crate::NetAddr::Onion { + pk: [8; 32], + port: 9735, + })); + let i2p = |port| crate::NetAddr::I2p { + dest: [7; 32], + port, + }; + peers.note_misbehavior_key(crate::peers::DiscourageKey::from_net(i2p(0)).unwrap()); + assert!( + peers.net_discouraged(i2p(8333)), + "the same I2P destination on another port is still refused" + ); +} + #[test] fn threshold_exit_refuses_the_address_without_punish_disconnect() { let peers = crate::peers::PeerHub::new(); @@ -4281,6 +4351,44 @@ fn evicted_netgroup_waits_less_than_a_day_and_the_set_is_capped() { ); } +/// A full inbound table evicts one peer and pauses that netgroup. The pause +/// must not refuse outbound dials to the rest of the prefix. +#[test] +fn slot_evict_does_not_block_an_outbound_dial() { + let peers = crate::peers::PeerHub::new(); + peers.set_mock_now(1_800_000_000); + let evicted = "8.8.1.1:8333".parse().unwrap(); + peers.note_slot_evict(crate::eviction::eviction_netgroup(evicted)); + let neighbor: std::net::SocketAddr = "8.8.9.9:8333".parse().unwrap(); + assert!( + peers.inbound_discouraged(neighbor), + "the slot wait still refuses another inbound from that /16" + ); + assert!( + !peers.net_discouraged(crate::NetAddr::Ip(neighbor)), + "the slot wait must not refuse an outbound dial in that /16" + ); + let cjdns_ip = std::net::Ipv6Addr::new(0xfc00, 0, 0, 0, 0, 0, 0, 1); + let cjdns_sock = std::net::SocketAddr::from((cjdns_ip, 8333)); + peers.note_slot_evict(crate::eviction::eviction_netgroup(cjdns_sock)); + assert!( + peers.inbound_discouraged(cjdns_sock), + "the slot wait still refuses another inbound from that cjdns group" + ); + assert!( + !peers.net_discouraged(crate::NetAddr::Cjdns { + ip: cjdns_ip, + port: 8333, + }), + "the slot wait must not refuse an outbound cjdns dial in that group" + ); + peers.note_misbehavior_addr(neighbor.ip()); + assert!( + peers.net_discouraged(crate::NetAddr::Ip(neighbor)), + "a misbehavior key still refuses an outbound dial to that address" + ); +} + #[tokio::test] async fn inv_getdata_charges_send_budget() { use bitcoin::hashes::Hash; diff --git a/crates/rbitcoin-net/src/peers.rs b/crates/rbitcoin-net/src/peers.rs index b7636defd..efc6d1193 100644 --- a/crates/rbitcoin-net/src/peers.rs +++ b/crates/rbitcoin-net/src/peers.rs @@ -1382,6 +1382,40 @@ pub struct PeerInfo { pub best_known: Option, } +/// In-memory misbehavior key. Clearnet and CJDNS are the IP. Onion and I2P +/// are the 32-byte identity. The port is not part of the key. +#[derive(Clone, Copy, PartialEq, Eq, Hash)] +pub(crate) enum DiscourageKey { + Ip(IpAddr), + Onion([u8; 32]), + I2p([u8; 32]), +} + +impl DiscourageKey { + pub(crate) fn from_net(net: crate::NetAddr) -> Option { + match net { + crate::NetAddr::Onion { pk, .. } => Some(Self::Onion(pk)), + crate::NetAddr::I2p { dest, .. } => Some(Self::I2p(dest)), + crate::NetAddr::Cjdns { ip, .. } => Some(Self::Ip(IpAddr::V6(ip))), + crate::NetAddr::Ip(sock) => { + let ip = sock.ip(); + if ip_skips_discourage(ip) { + None + } else { + Some(Self::Ip(ip)) + } + } + } + } +} + +fn ip_skips_discourage(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(v) => v.is_loopback() || v.octets()[0] == 0, + IpAddr::V6(v) => v.is_loopback(), + } +} + #[allow(clippy::type_complexity)] // session table: live peers, manual hosts, dials /// Thread-safe session table + addnode remembered addrs. pub struct PeerHub { @@ -1445,12 +1479,12 @@ pub struct PeerHub { mempool: Mutex>>, /// Core `-whitelist` / `-whitebind` grants (`getpeerinfo.permissions`). net_perms: Mutex, - /// Misbehavior addresses → unix second the refusal ends. In memory only. - discouraged_addrs: Mutex>, + /// Misbehavior keys → unix second the refusal ends. In memory only. + discouraged_addrs: Mutex>, /// Netgroup → unix second a slot-loss refusal ends. discouraged_groups: Mutex>, - /// Rate-limit disconnect address → unix second its strike ends. - rate_strikes: Mutex>, + /// Rate-limit disconnect key → unix second its strike ends. + rate_strikes: Mutex>, } fn canonical_bind(addr: SocketAddr) -> SocketAddr { @@ -2050,32 +2084,37 @@ impl PeerHub { /// Record `ip` until [`Self::DISCOURAGE_TTL_SECS`] from now. At the cap a /// new address is ignored; an address already stored is refreshed. + #[cfg(test)] pub(crate) fn note_misbehavior_addr(&self, ip: IpAddr) { + self.note_misbehavior_key(DiscourageKey::Ip(ip)); + } + + pub(crate) fn note_misbehavior_key(&self, key: DiscourageKey) { let now = self.now_secs(); let until = now.saturating_add(Self::DISCOURAGE_TTL_SECS); let mut g = self .discouraged_addrs .lock() .unwrap_or_else(|e| e.into_inner()); - if !g.contains_key(&ip) && g.len() >= Self::DISCOURAGE_CAP { + if !g.contains_key(&key) && g.len() >= Self::DISCOURAGE_CAP { return; } - g.insert(ip, until); + g.insert(key, until); } /// True when `ip` still has a strike from an earlier rate-limit /// disconnect; that strike is spent. Otherwise records one for /// [`Self::RATE_STRIKE_SECS`]. Expired strikes are dropped here. At /// [`Self::DISCOURAGE_CAP`] a new address is not recorded. - pub(crate) fn note_rate_strike(&self, ip: IpAddr) -> bool { + pub(crate) fn note_rate_strike(&self, key: DiscourageKey) -> bool { let now = self.now_secs(); let mut g = self.rate_strikes.lock().unwrap_or_else(|e| e.into_inner()); g.retain(|_, until| *until > now); - if g.remove(&ip).is_some() { + if g.remove(&key).is_some() { return true; } if g.len() < Self::DISCOURAGE_CAP { - g.insert(ip, now.saturating_add(Self::RATE_STRIKE_SECS)); + g.insert(key, now.saturating_add(Self::RATE_STRIKE_SECS)); } false } @@ -2096,19 +2135,25 @@ impl PeerHub { g.insert(group, until); } - /// True when this inbound address or its accept-time netgroup is still - /// refused. Expired rows are dropped here, which is the only sweep. - pub(crate) fn inbound_discouraged(&self, addr: SocketAddr) -> bool { + /// Misbehavior key for this net. A slot-eviction netgroup stays inbound-only. + pub(crate) fn net_discouraged(&self, net: crate::NetAddr) -> bool { + DiscourageKey::from_net(net).is_some_and(|key| self.key_refused(key)) + } + + fn key_refused(&self, key: DiscourageKey) -> bool { let now = self.now_secs(); self.sweep_discouraged(now); - // `sweep_discouraged` already dropped rows with `until <= now`. - if self - .discouraged_addrs + self.discouraged_addrs .lock() .unwrap_or_else(|e| e.into_inner()) - .get(&addr.ip()) - .is_some() - { + .contains_key(&key) + } + + /// True when this inbound address or its accept-time netgroup is still + /// refused. Expired rows are dropped here, which is the only sweep. + pub(crate) fn inbound_discouraged(&self, addr: SocketAddr) -> bool { + // `key_refused` drops expired rows. + if self.key_refused(DiscourageKey::Ip(addr.ip())) { return true; } let group = crate::eviction::eviction_netgroup(addr); diff --git a/crates/rbitcoin-net/src/service.rs b/crates/rbitcoin-net/src/service.rs index 64806a166..0a2af1f64 100644 --- a/crates/rbitcoin-net/src/service.rs +++ b/crates/rbitcoin-net/src/service.rs @@ -632,6 +632,9 @@ async fn prepare_outbound_session( // already dialling. A queued dial was counted when it was queued. let _dialing = in_flight.unwrap_or_else(|| peers.dial_in_flight(&peer)); let peer_net = peer.net_addr(); + if peers.net_discouraged(peer_net) { + return Err(NetError::Protocol("discouraged")); + } let stream = tokio::time::timeout( crate::connect_timeout_for(peer_net, OUTBOUND_CONNECT_TIMEOUT), dialer.connect_net(peer_net), @@ -753,7 +756,11 @@ async fn run_outbound_session_with_abort( return Err(NetError::Encode("feeler requires ip:port target".into())) } }; - let stream = dialer.connect_net(crate::NetAddr::Ip(peer_addr)).await?; + let peer_net = crate::NetAddr::Ip(peer_addr); + if peers.net_discouraged(peer_net) { + return Err(NetError::Protocol("discouraged")); + } + let stream = dialer.connect_net(peer_net).await?; let height = hub.tip_height().map(|h| h as i32).unwrap_or(0); return crate::peer::run_feeler(stream, magic, local, peer_addr, height, &user_agent).await; } @@ -817,6 +824,43 @@ mod tests { ); } + /// A feeler does not use `prepare_outbound_session`. It still must not + /// open a socket to an address that misbehaved. + #[tokio::test] + async fn feeler_does_not_dial_a_discouraged_address() { + let peers = crate::peers::PeerHub::new(); + peers.set_mock_now(1_700_000_000); + let addr = std::net::SocketAddr::from(([1, 2, 3, 4], 8333)); + peers.note_misbehavior_addr(addr.ip()); + let (dir, hub) = crate::chain::tiny_regtest_hub_labeled("feeler-discouraged"); + let (_tx, rx) = tokio::sync::oneshot::channel(); + let result = tokio::time::timeout( + std::time::Duration::from_millis(200), + run_outbound_session_with_abort( + crate::peers::DialTarget::Socket(addr), + None, + Magic::REGTEST, + std::net::SocketAddr::from(([127, 0, 0, 1], 0)), + std::sync::Arc::new(hub), + peers, + "/rbitcoin:test/".into(), + std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)), + crate::peers::PeerConnType::Feeler, + rx, + crate::socks::Dialer::Direct, + ), + ) + .await; + let _ = std::fs::remove_dir_all(dir.path()); + let err = result + .expect("a refused feeler returns before the connect") + .unwrap_err(); + assert!( + matches!(err, NetError::Protocol("discouraged")), + "feeler dialed a discouraged address, got {err:?}" + ); + } + /// One datadir, two node lifetimes. The first is dropped without /// shutdown. The second binds loopback as with onion listen, adds a /// second listener, and shuts down with one session task that finishes diff --git a/crates/rbitcoin-net/src/tx_relay.rs b/crates/rbitcoin-net/src/tx_relay.rs index 2c3ae31f2..b3c7effd1 100644 --- a/crates/rbitcoin-net/src/tx_relay.rs +++ b/crates/rbitcoin-net/src/tx_relay.rs @@ -6,6 +6,8 @@ //! is not in rust-bitcoin 0.32 `NetworkMessage`; the old private `rbtpkg` //! name is gone). +use crate::fee_history::{FeeHistory, HistoricalFeeBlock}; +use crate::fee_history_file; use crate::perf_meter::{PerfCounter, PerfMax}; use arc_swap::ArcSwap; use bitcoin::hashes::Hash; @@ -27,8 +29,99 @@ use std::sync::{Arc, Mutex, RwLock}; use std::time::{Duration, Instant}; use tokio::sync::broadcast; -use crate::fee_history::{FeeHistory, HistoricalFeeBlock}; -use crate::fee_history_file; +#[cfg(test)] +thread_local! { + static PROMOTE_DEPTH: std::cell::Cell = const { std::cell::Cell::new(0) }; + static PROMOTE_DEPTH_MAX: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +#[cfg(test)] +struct PromoteDepth; + +#[cfg(test)] +impl PromoteDepth { + fn enter() -> Self { + PROMOTE_DEPTH.with(|d| { + let n = d.get().saturating_add(1); + d.set(n); + PROMOTE_DEPTH_MAX.with(|m| m.set(m.get().max(n))); + }); + Self + } +} + +#[cfg(test)] +impl Drop for PromoteDepth { + fn drop(&mut self) { + PROMOTE_DEPTH.with(|d| d.set(d.get().saturating_sub(1))); + } +} + +#[cfg(test)] +fn reset_promote_depth() { + PROMOTE_DEPTH.with(|d| d.set(0)); + PROMOTE_DEPTH_MAX.with(|d| d.set(0)); +} + +#[cfg(test)] +fn promote_depth_max() -> u32 { + PROMOTE_DEPTH_MAX.with(|d| d.get()) +} + +thread_local! { + /// Txids admitted while one promote walk is running on this thread. + /// `None` means no walk is active. + static PROMOTE_NOTES: std::cell::RefCell>> = + const { std::cell::RefCell::new(None) }; +} + +/// Record `txid` when a walk is already running. Returns false when this +/// thread should start the walk. +fn promote_note(txid: Txid) -> bool { + PROMOTE_NOTES.with(|slot| { + let mut slot = slot.borrow_mut(); + let Some(notes) = slot.as_mut() else { + return false; + }; + notes.push(txid); + true + }) +} + +fn promote_take_notes() -> Vec { + PROMOTE_NOTES.with(|slot| { + slot.borrow_mut() + .as_mut() + .map(std::mem::take) + .unwrap_or_default() + }) +} + +/// One promote walk on this thread. Drop ends it, including after a panic. +struct PromoteWalk { + #[cfg(test)] + _depth: PromoteDepth, +} + +impl PromoteWalk { + fn enter() -> Self { + PROMOTE_NOTES.with(|slot| { + *slot.borrow_mut() = Some(Vec::new()); + }); + Self { + #[cfg(test)] + _depth: PromoteDepth::enter(), + } + } +} + +impl Drop for PromoteWalk { + fn drop(&mut self) { + PROMOTE_NOTES.with(|slot| { + *slot.borrow_mut() = None; + }); + } +} #[path = "parent_req.rs"] mod parent_req; @@ -2210,6 +2303,43 @@ impl MempoolHub { self.unindex_evicted(&gone); } + /// Roll the package back, then return members that were parked before it. + /// + /// A child whose parents are already in the mempool is admitted. One that + /// still misses a parent is parked under those parents, with the peers + /// who announced it. + fn abandon_package( + &self, + accepted: &[AcceptResult], + lifted: &[rbitcoin_mempool::LiftedOrphan], + ) { + if !accepted.is_empty() { + self.rollback_package_accepted(accepted); + } + let utxo = self.utxo_provider(); + let tip = self.chain_tip_ctx(); + // Read the priority map before the mempool write lock. A child under + // the raw relay floor is still admissible when its delta clears it. + let deltas = self.prioritised_txs(); + let ready = { + let mut g = self.lock_write(); + g.repark_lifted(lifted, &utxo, tip, &|id| { + deltas.get(id).copied().unwrap_or(0) + }) + }; + for item in ready { + let from = item.announcers.first().copied(); + if let Err(AcceptError::Orphaned { .. }) = + self.accept_with_utxo(&item.tx, &utxo, from, false) + { + let id = item.tx.compute_txid(); + for peer in item.announcers.iter().skip(1) { + self.add_orphan_announcer(&id, *peer); + } + } + } + } + fn rollback_package_accepted(&self, accepted: &[AcceptResult]) { let victims: Vec = accepted .iter() @@ -2367,13 +2497,42 @@ impl MempoolHub { self.promote_orphans_staged(r.txid, utxo); } + /// One frame for every orphan of every tx admitted on this thread. + /// + /// A publish that happens while the walk is running, including a 1p1c + /// parent and a conflict put back after a failed admit, appends that + /// txid here. It does not start another walk. fn promote_orphans_staged(&self, parent: Txid, utxo: &impl rbitcoin_mempool::UtxoProvider) { - let children = { - let mut g = self.lock_write(); - g.take_orphan_children(parent) - }; - for child in children { + if promote_note(parent) { + return; + } + let _walk = PromoteWalk::enter(); + let mut queue = VecDeque::new(); + promote_note(parent); + self.enqueue_noted_orphans(&mut queue); + while let Some(child) = queue.pop_front() { let _ = self.accept_with_utxo(&child, utxo, None, false); + self.enqueue_noted_orphans(&mut queue); + } + } + + /// Move orphans of every tx noted since the last drain onto `queue`. + /// Earlier notes stay ahead of later ones. A second take of the same + /// txid is empty. + fn enqueue_noted_orphans(&self, queue: &mut VecDeque) { + let notes = promote_take_notes(); + if notes.is_empty() { + return; + } + let mut kids = Vec::new(); + { + let mut g = self.lock_write(); + for txid in notes { + kids.extend(g.take_orphan_children(txid)); + } + } + for child in kids.into_iter().rev() { + queue.push_front(child); } } @@ -2702,7 +2861,17 @@ impl MempoolHub { let mut lock_us = 0u64; let mut accepted: Vec = Vec::with_capacity(txs.len()); let mut prevouts: Vec> = Vec::with_capacity(txs.len()); + // A member that is already parked has to leave the orphanage before + // prepare. Otherwise the parent just committed in this package is + // rolled back and the child stays parked. + let mut lifted: Vec = Vec::new(); for tx in txs { + if let Some(parked) = { + let mut g = self.lock_write(); + g.lift_parked_member(tx) + } { + lifted.push(parked); + } utxo.note_spender(tx); let delta = self.fee_delta(&tx.compute_txid()); let spec = AdmitSpec { @@ -2714,9 +2883,7 @@ impl MempoolHub { let prep = match self.admit_staged(tx, &utxo, spec, &mut stages, &mut lock_us) { Ok(p) => p, Err(e) => { - if !accepted.is_empty() { - self.rollback_package_accepted(&accepted); - } + self.abandon_package(&accepted, &lifted); let us = t0.elapsed().as_micros() as u64; self.meter_accept_stages(lock_us, stages); return Err(self.finish_accept_err(us, e).unwrap_err()); @@ -2740,7 +2907,7 @@ impl MempoolHub { Err(e) => { self.unindex_evicted(&committed.failed_evict); self.restore_replaced(&committed.undone); - self.rollback_package_accepted(&accepted); + self.abandon_package(&accepted, &lifted); // The first member can evict and fail with nothing to roll // back, so the rollback publish does not run. self.publish_fee_floor(); @@ -2754,7 +2921,7 @@ impl MempoolHub { if accepted.iter().any(|r| !self.try_contains(&r.txid)) { // Trim dropped a member that had already replaced its conflicts. // Put those conflicts back; the package itself does not stay. - self.rollback_package_accepted(&accepted); + self.abandon_package(&accepted, &lifted); self.publish_fee_floor(); let us = t0.elapsed().as_micros() as u64; self.meter_accept_stages(lock_us, stages); @@ -6795,100 +6962,153 @@ mod tests { let _ = std::fs::remove_dir_all(&store_dir); } - /// Coinbases at heights `1..=n`, each with 100 confirmations. - /// - /// `n == 3` and `n == 12` return the process pad. Callers that connect - /// or disconnect take [`copy_maturity_pad`] and leave this store in place. - fn pad_cbs(n: u32) -> (std::path::PathBuf, Arc, Vec) { - if n == 3 || n == 12 { - let p = maturity_pad(n); - return (p.dir.clone(), Arc::clone(&p.q), p.cbs.clone()); - } - let p = build_maturity_pad(n); - (p.dir, p.q, p.cbs) - } - - struct MaturityPad { - dir: std::path::PathBuf, - q: Arc, - cbs: Vec, - } - - fn maturity_pad(n: u32) -> &'static MaturityPad { - use std::sync::OnceLock; - static PAD3: OnceLock = OnceLock::new(); - static PAD12: OnceLock = OnceLock::new(); - match n { - 3 => PAD3.get_or_init(|| build_maturity_pad(3)), - 12 => PAD12.get_or_init(|| build_maturity_pad(12)), - _ => panic!("maturity pad {n} is built per test"), - } - } - - fn build_maturity_pad(n: u32) -> MaturityPad { - use rbitcoin_consensus::{ChainParams, Milestone}; - use rbitcoin_primitives::Height; - use rbitcoin_query::accept_and_connect_block; + /// A child already in the orphan pool is still a package member. + /// `submitpackage` and Electrum use this hub path. + #[test] + fn hub_package_admits_its_parked_child() { + let (_store, q, cbs) = pad_cbs(3); let dir = tmp(); - let q = Query::open_or_create_tiny(&dir).unwrap(); - let params = ChainParams::regtest(); - let genesis = bitcoin::blockdata::constants::genesis_block(bitcoin::Network::Regtest); - accept_and_connect_block(&q, ¶ms, Height::GENESIS, &genesis, Milestone::NONE).unwrap(); - let last = n + params.coinbase_maturity(); - let (_tip, _t, cbs) = rbitcoin_query::pad_empty_from( - &q, - ¶ms, - genesis.block_hash(), - genesis.header.time, - 1, - last, - n, + let hub = MempoolHub::open(&dir, Arc::clone(&q)).unwrap(); + hub.set_relay_enabled(true); + let spk = ScriptBuf::from_bytes(vec![0x51]); + let parent = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint { + txid: cbs[0], + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }], + output: vec![ + TxOut { + value: Amount::from_sat(20_0000_0000), + script_pubkey: spk.clone(), + }, + TxOut { + value: Amount::from_sat(20_0000_0000), + script_pubkey: spk.clone(), + }, + ], + }; + let parent_id = parent.compute_txid(); + let child = spend_vout( + OutPoint { + txid: parent_id, + vout: 0, + }, + 19_0000_0000, ); - assert_eq!(cbs.len(), n as usize); - // Quiescent after init. Later tests only read this store. - q.flush().expect("flush maturity pad"); - MaturityPad { - dir, - q: Arc::new(q), - cbs, + let sibling = spend_vout( + OutPoint { + txid: parent_id, + vout: 1, + }, + 19_0000_0000, + ); + let child_id = child.compute_txid(); + let sibling_id = sibling.compute_txid(); + for tx in [&child, &sibling] { + let err = hub.accept_tx(tx).unwrap_err(); + assert!( + matches!(err, AcceptError::Orphaned { .. }), + "spend of the missing parent must park, got {err:?}" + ); } - } + let accepted = hub + .accept_package(&[parent.clone(), child.clone()]) + .expect("hub package contains the parked child"); + assert_eq!(accepted.len(), 2); + assert!(hub.try_contains(&parent_id)); + assert!(hub.try_contains(&child_id)); + assert!( + hub.try_contains(&sibling_id), + "a parked sibling outside the package must still be admitted" + ); + assert_eq!(hub.orphan_count(), 0); + let _ = std::fs::remove_dir_all(&dir); - fn copy_maturity_pad(n: u32) -> (std::path::PathBuf, Arc, Vec) { - let src = maturity_pad(n); let dir = tmp(); - std::fs::create_dir_all(&dir).unwrap(); - copy_store_tree(&src.dir, &dir); - let q = Query::open_or_create_tiny(&dir).expect("open maturity copy"); - (dir, Arc::new(q), src.cbs.clone()) - } - - fn copy_store_tree(src: &std::path::Path, dst: &std::path::Path) { - rbitcoin_store::testutil::copy_store_tree(src, dst); + let hub = MempoolHub::open(&dir, Arc::clone(&q)).unwrap(); + hub.set_relay_enabled(true); + let parent = spend_true(cbs[1], 10_000, spk); + let parent_id = parent.compute_txid(); + let child = spend_vout( + OutPoint { + txid: parent_id, + vout: 0, + }, + 49_0000_0000, + ); + let child_id = child.compute_txid(); + let err = hub.accept_tx(&child).unwrap_err(); + assert!(matches!(err, AcceptError::Orphaned { .. }), "{err:?}"); + let bad = spend_vout( + OutPoint { + txid: parent_id, + vout: 0, + }, + 50_0000_0000, + ); + let err = hub + .accept_package(&[parent, child, bad]) + .expect_err("last member overspends the parent output"); + assert!( + !matches!(err, AcceptError::Duplicate(id) if id == child_id), + "a failed hub package must not report the parked child as a duplicate, got {err:?}" + ); + assert!( + !hub.contains(&parent_id), + "a failed package must not leave the parent" + ); + assert!( + !hub.contains(&child_id), + "a failed package must not leave the child in the mempool" + ); + assert_eq!( + hub.orphan_count(), + 1, + "a failed package must put the parked child back, got {err:?}" + ); + let _ = std::fs::remove_dir_all(&dir); } + /// A parent can land while its child is lifted. Putting the child back + /// under that parent leaves it parked with nobody left to wake it. #[test] - fn submit_package_rpc_admits_cpfp_below_minrelay() { - let (_store, owned_q, owned_cbs) = pad_cbs(3); - let q = &owned_q; - let cbs = owned_cbs.as_slice(); + fn failed_package_does_not_park_child_under_a_live_parent() { + let (_store, q, cbs) = pad_cbs(3); + let spk = ScriptBuf::from_bytes(vec![0x51]); let dir = tmp(); - let hub = MempoolHub::open(&dir, Arc::clone(q)).unwrap(); + let hub = MempoolHub::open(&dir, Arc::clone(&q)).unwrap(); hub.set_relay_enabled(true); - let parent = spend_true(cbs[0], 1, ScriptBuf::from_bytes(vec![0x51])); - assert!( - matches!( - hub.accept_tx(&parent), - Err(AcceptError::Policy("min relay fee")) - ), - "parent must fail min-relay alone" + let parent = spend_true(cbs[0], 10_000, spk); + let parent_id = parent.compute_txid(); + hub.accept_tx(&parent).expect("parent pays for itself"); + let child = spend_vout( + OutPoint { + txid: parent_id, + vout: 0, + }, + 49_0000_0000, ); - let child = Transaction { + let child_id = child.compute_txid(); + { + let mut missing = BTreeSet::new(); + missing.insert(parent_id); + let mut g = hub.lock_write(); + g.park_orphan_from(&child, missing, Some(4)); + } + hub.add_orphan_announcer(&child_id, 8); + let dummy = Transaction { version: Version::TWO, lock_time: LockTime::ZERO, input: vec![TxIn { previous_output: OutPoint { - txid: parent.compute_txid(), + txid: Txid::from_byte_array([0x44; 32]), vout: 0, }, script_sig: ScriptBuf::new(), @@ -6896,23 +7116,274 @@ mod tests { witness: Witness::new(), }], output: vec![TxOut { - value: Amount::from_sat(50_0000_0000 - 1 - 50_000), - script_pubkey: ScriptBuf::from_bytes(vec![0x63]), - }], - }; - let unpaid = Transaction { - output: vec![TxOut { - value: Amount::from_sat(50_0000_0000 - 1), - script_pubkey: ScriptBuf::from_bytes(vec![0x63]), + value: Amount::from_sat(1), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), }], - ..child.clone() }; - let missed = hub.submit_package_rpc(&[parent.clone(), unpaid]); + let err = hub + .accept_package(&[child.clone(), dummy]) + .expect_err("the second member has no parent"); assert!( - missed.iter().all(|r| r.fee_with.is_none()), - "a remainder that does not land has no package feerate set, got {missed:?}" + !matches!(err, AcceptError::Duplicate(id) if id == child_id), + "the live parent must not make the lifted child a duplicate, got {err:?}" ); - assert!(!hub.try_contains(&parent.compute_txid())); + assert!( + hub.contains(&child_id), + "a child whose parents are already in the mempool must be admitted, not parked, got {err:?}" + ); + assert_eq!(hub.orphan_count(), 0); + assert!(hub.contains(&parent_id)); + let _ = std::fs::remove_dir_all(&dir); + + let dir = tmp(); + let hub = MempoolHub::open(&dir, Arc::clone(&q)).unwrap(); + hub.set_relay_enabled(true); + let parent = spend_true(cbs[1], 10_000, ScriptBuf::from_bytes(vec![0x51])); + let parent_id = parent.compute_txid(); + hub.accept_tx(&parent).expect("parent"); + let absent = Txid::from_byte_array([0xab; 32]); + let mut child = spend_vout( + OutPoint { + txid: parent_id, + vout: 0, + }, + 49_0000_0000, + ); + child.input.push(TxIn { + previous_output: OutPoint { + txid: absent, + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }); + let child_id = child.compute_txid(); + { + let mut missing = BTreeSet::new(); + missing.insert(parent_id); + missing.insert(absent); + let mut g = hub.lock_write(); + g.park_orphan_from(&child, missing, Some(4)); + } + hub.add_orphan_announcer(&child_id, 8); + let dummy = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint { + txid: Txid::from_byte_array([0x45; 32]), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }], + output: vec![TxOut { + value: Amount::from_sat(1), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }], + }; + let err = hub + .accept_package(&[child, dummy]) + .expect_err("the child still misses a parent"); + assert!( + matches!(err, AcceptError::Orphaned { .. }), + "the child still misses a parent, got {err:?}" + ); + assert!(!hub.contains(&child_id)); + let parked = hub.orphan_snapshot(); + assert_eq!(parked.len(), 1); + assert_eq!( + parked[0].announcers, + vec![4, 8], + "putting the child back must keep the peers who announced it" + ); + let _ = std::fs::remove_dir_all(&dir); + } + + /// A prioritised child is under the relay floor until its delta is applied. + /// A failed package must still admit it once its parent is in the mempool. + #[test] + fn failed_package_admits_prioritised_child() { + let (_store, q, cbs) = pad_cbs(3); + let dir = tmp(); + let hub = MempoolHub::open(&dir, Arc::clone(&q)).unwrap(); + hub.set_relay_enabled(true); + let spk = ScriptBuf::from_bytes(vec![0x51]); + let parent = spend_true(cbs[2], 10_000, spk); + let parent_id = parent.compute_txid(); + hub.accept_tx(&parent).expect("parent pays for itself"); + let parent_value = 50_0000_0000 - 10_000; + let child = spend_vout( + OutPoint { + txid: parent_id, + vout: 0, + }, + parent_value - 1, + ); + let child_id = child.compute_txid(); + let err = hub + .accept_tx(&child) + .expect_err("one sat is under the relay floor"); + assert!( + matches!(err, AcceptError::Policy("min relay fee")), + "raw fee must miss the floor before the delta, got {err:?}" + ); + hub.prioritise_tx(child_id, 1_000_000); + { + let mut missing = BTreeSet::new(); + missing.insert(parent_id); + let mut g = hub.lock_write(); + g.park_orphan_from(&child, missing, Some(4)); + } + let dummy = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint { + txid: Txid::from_byte_array([0x46; 32]), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }], + output: vec![TxOut { + value: Amount::from_sat(1), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }], + }; + let err = hub + .accept_package(&[child, dummy]) + .expect_err("the second member has no parent"); + assert!( + hub.contains(&child_id), + "a prioritised child must be admitted after the package fails, got {err:?}" + ); + assert_eq!(hub.orphan_count(), 0); + assert!(hub.contains(&parent_id)); + let _ = std::fs::remove_dir_all(&dir); + } + + /// Coinbases at heights `1..=n`, each with 100 confirmations. + /// + /// `n == 3` and `n == 12` return the process pad. Callers that connect + /// or disconnect take [`copy_maturity_pad`] and leave this store in place. + fn pad_cbs(n: u32) -> (std::path::PathBuf, Arc, Vec) { + if n == 3 || n == 12 { + let p = maturity_pad(n); + return (p.dir.clone(), Arc::clone(&p.q), p.cbs.clone()); + } + let p = build_maturity_pad(n); + (p.dir, p.q, p.cbs) + } + + struct MaturityPad { + dir: std::path::PathBuf, + q: Arc, + cbs: Vec, + } + + fn maturity_pad(n: u32) -> &'static MaturityPad { + use std::sync::OnceLock; + static PAD3: OnceLock = OnceLock::new(); + static PAD12: OnceLock = OnceLock::new(); + match n { + 3 => PAD3.get_or_init(|| build_maturity_pad(3)), + 12 => PAD12.get_or_init(|| build_maturity_pad(12)), + _ => panic!("maturity pad {n} is built per test"), + } + } + + fn build_maturity_pad(n: u32) -> MaturityPad { + use rbitcoin_consensus::{ChainParams, Milestone}; + use rbitcoin_primitives::Height; + use rbitcoin_query::accept_and_connect_block; + let dir = tmp(); + let q = Query::open_or_create_tiny(&dir).unwrap(); + let params = ChainParams::regtest(); + let genesis = bitcoin::blockdata::constants::genesis_block(bitcoin::Network::Regtest); + accept_and_connect_block(&q, ¶ms, Height::GENESIS, &genesis, Milestone::NONE).unwrap(); + let last = n + params.coinbase_maturity(); + let (_tip, _t, cbs) = rbitcoin_query::pad_empty_from( + &q, + ¶ms, + genesis.block_hash(), + genesis.header.time, + 1, + last, + n, + ); + assert_eq!(cbs.len(), n as usize); + // Quiescent after init. Later tests only read this store. + q.flush().expect("flush maturity pad"); + MaturityPad { + dir, + q: Arc::new(q), + cbs, + } + } + + fn copy_maturity_pad(n: u32) -> (std::path::PathBuf, Arc, Vec) { + let src = maturity_pad(n); + let dir = tmp(); + std::fs::create_dir_all(&dir).unwrap(); + copy_store_tree(&src.dir, &dir); + let q = Query::open_or_create_tiny(&dir).expect("open maturity copy"); + (dir, Arc::new(q), src.cbs.clone()) + } + + fn copy_store_tree(src: &std::path::Path, dst: &std::path::Path) { + rbitcoin_store::testutil::copy_store_tree(src, dst); + } + + #[test] + fn submit_package_rpc_admits_cpfp_below_minrelay() { + let (_store, owned_q, owned_cbs) = pad_cbs(3); + let q = &owned_q; + let cbs = owned_cbs.as_slice(); + let dir = tmp(); + let hub = MempoolHub::open(&dir, Arc::clone(q)).unwrap(); + hub.set_relay_enabled(true); + let parent = spend_true(cbs[0], 1, ScriptBuf::from_bytes(vec![0x51])); + assert!( + matches!( + hub.accept_tx(&parent), + Err(AcceptError::Policy("min relay fee")) + ), + "parent must fail min-relay alone" + ); + let child = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint { + txid: parent.compute_txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }], + output: vec![TxOut { + value: Amount::from_sat(50_0000_0000 - 1 - 50_000), + script_pubkey: ScriptBuf::from_bytes(vec![0x63]), + }], + }; + let unpaid = Transaction { + output: vec![TxOut { + value: Amount::from_sat(50_0000_0000 - 1), + script_pubkey: ScriptBuf::from_bytes(vec![0x63]), + }], + ..child.clone() + }; + let missed = hub.submit_package_rpc(&[parent.clone(), unpaid]); + assert!( + missed.iter().all(|r| r.fee_with.is_none()), + "a remainder that does not land has no package feerate set, got {missed:?}" + ); + assert!(!hub.try_contains(&parent.compute_txid())); let held = hub.submit_package_rpc_eval(&[parent.clone(), child.clone()], false); assert!( held[0].result.is_err(), @@ -7067,92 +7538,46 @@ mod tests { } } - /// A replacement that is itself evicted must not drop the transactions it - /// conflicted with. One-tx submit and a one-member package share that restore. + /// A package whose child is evicted with the low-fee anchor must not drop + /// the conflict the replacement removed. The replacement does not spend + /// the anchor: that input is not a parent of the conflict. #[test] fn failed_replacement_keeps_the_conflict() { let (_store, owned_q, owned_cbs) = pad_cbs(3); let q = &owned_q; let cbs = owned_cbs.as_slice(); - for package in [false, true] { - let mut fat = Vec::with_capacity(5_000); - for _ in 0..9 { - fat.push(0x4d); - fat.extend_from_slice(&520u16.to_le_bytes()); - fat.extend(std::iter::repeat_n(0u8, 520)); - fat.push(0x75); - } - fat.push(0x51); - let parent = spend_true(cbs[0], 2_000, ScriptBuf::from_bytes(fat)); - let parent_id = parent.compute_txid(); - let victim = spend_true(cbs[1], 3_000, ScriptBuf::from_bytes(vec![0x51])); - let victim_id = victim.compute_txid(); - let parent_out = parent.output[0].value.to_sat(); - let repl_fee = 60_000u64; - let replacement = Transaction { - version: Version::TWO, - lock_time: LockTime::ZERO, - input: vec![ - TxIn { - previous_output: OutPoint { - txid: cbs[1], - vout: 0, - }, - script_sig: ScriptBuf::new(), - sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Witness::new(), - }, - TxIn { - previous_output: OutPoint { - txid: parent_id, - vout: 0, - }, - script_sig: ScriptBuf::new(), - sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Witness::new(), - }, - ], - output: vec![TxOut { - value: Amount::from_sat(50_0000_0000 + parent_out - repl_fee), - script_pubkey: ScriptBuf::from_bytes(vec![0x51; 800]), - }], - }; - let parent_wu = parent.weight().to_wu(); - let victim_wu = victim.weight().to_wu(); - let repl_wu = replacement.weight().to_wu(); - let budget = parent_wu + victim_wu + 500; - assert!( - repl_wu > victim_wu + 500, - "replacement weight {repl_wu} must exceed the slack" - ); - let dir = tmp(); - let hub = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); - hub.set_relay_enabled(true); - assert!( - hub.accept_tx(&parent).is_ok(), - "package={package}: parent fits" - ); - assert!( - hub.accept_tx(&victim).is_ok(), - "package={package}: victim fits" - ); - let err = if package { - hub.accept_package(std::slice::from_ref(&replacement)) - .map(|_| ()) - } else { - hub.accept_tx(&replacement).map(|_| ()) - }; - assert!( - matches!(err, Err(AcceptError::Policy("mempool full"))), - "package={package}: replacement must not stay, got {err:?}" - ); - assert!(!hub.contains(&replacement.compute_txid())); - assert!( - hub.contains(&victim_id), - "package={package}: failed replacement must keep the conflict" - ); - let _ = std::fs::remove_dir_all(&dir); + let mut fat = Vec::with_capacity(5_000); + for _ in 0..9 { + fat.push(0x4d); + fat.extend_from_slice(&520u16.to_le_bytes()); + fat.extend(std::iter::repeat_n(0u8, 520)); + fat.push(0x75); } + fat.push(0x51); + let anchor = spend_true(cbs[0], 2_000, ScriptBuf::from_bytes(fat)); + let victim = spend_true(cbs[1], 3_000, ScriptBuf::from_bytes(vec![0x51])); + let victim_id = victim.compute_txid(); + let anchor_wu = anchor.weight().to_wu(); + let budget = anchor_wu + victim.weight().to_wu() + 500; + let dir = tmp(); + let hub = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); + hub.set_relay_enabled(true); + assert!(hub.accept_tx(&anchor).is_ok(), "anchor fits"); + assert!(hub.accept_tx(&victim).is_ok(), "victim fits"); + let [replacement, child] = + package_evicted_with_anchor(&anchor, cbs[1], 60_000, 50_000, budget); + let replacement_id = replacement.compute_txid(); + let err = hub.accept_package(&[replacement, child]).map(|_| ()); + assert!( + matches!(err, Err(AcceptError::Policy("mempool full"))), + "replacement must not stay, got {err:?}" + ); + assert!(!hub.contains(&replacement_id)); + assert!( + hub.contains(&victim_id), + "failed replacement must keep the conflict" + ); + let _ = std::fs::remove_dir_all(&dir); } /// A conflict restored after a failed replacement keeps its accept time @@ -7163,93 +7588,57 @@ mod tests { let (_store, owned_q, owned_cbs) = pad_cbs(3); let q = &owned_q; let cbs = owned_cbs.as_slice(); - for package in [false, true] { - let mut fat = Vec::with_capacity(5_000); - for _ in 0..9 { - fat.push(0x4d); - fat.extend_from_slice(&520u16.to_le_bytes()); - fat.extend(std::iter::repeat_n(0u8, 520)); - fat.push(0x75); - } - fat.push(0x51); - let parent = spend_true(cbs[0], 2_000, ScriptBuf::from_bytes(fat)); - let parent_id = parent.compute_txid(); - let victim = spend_true(cbs[1], 3_000, ScriptBuf::from_bytes(vec![0x51])); - let victim_id = victim.compute_txid(); - let parent_out = parent.output[0].value.to_sat(); - let repl_fee = 60_000u64; - let replacement = Transaction { - version: Version::TWO, - lock_time: LockTime::ZERO, - input: vec![ - TxIn { - previous_output: OutPoint { - txid: cbs[1], - vout: 0, - }, - script_sig: ScriptBuf::new(), - sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Witness::new(), - }, - TxIn { - previous_output: OutPoint { - txid: parent_id, - vout: 0, - }, - script_sig: ScriptBuf::new(), - sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Witness::new(), - }, - ], - output: vec![TxOut { - value: Amount::from_sat(50_0000_0000 + parent_out - repl_fee), - script_pubkey: ScriptBuf::from_bytes(vec![0x51; 800]), - }], - }; - let budget = parent.weight().to_wu() + victim.weight().to_wu() + 500; - let dir = tmp(); - let hub = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); - hub.set_relay_enabled(true); - let t0 = 1_700_000_000u64; - hub.note_mock_now(t0); - hub.accept_tx(&parent).expect("parent fits"); - hub.accept_tx(&victim).expect("victim fits"); - hub.note_mock_now(t0 + 600); - let err = if package { - hub.accept_package(std::slice::from_ref(&replacement)) - .map(|_| ()) - } else { - hub.accept_tx(&replacement).map(|_| ()) - }; - assert!( - matches!(err, Err(AcceptError::Policy("mempool full"))), - "package={package}: got {err:?}" - ); - assert!(hub.contains(&victim_id), "package={package}: restored"); - assert_eq!( - hub.accept_time_txid(&victim_id), - Some(t0), - "package={package}: RAM keeps the original accept time" - ); - hub.flush().expect("shutdown flush"); - drop(hub); - let hub2 = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); - assert!(hub2.contains(&victim_id), "package={package}: reloaded"); - assert_eq!( - hub2.accept_time_txid(&victim_id), - Some(t0), - "package={package}: disk keeps the original accept time" - ); - hub2.set_expiry_hours(1); - hub2.note_mock_now(t0 + 3600 + 5); - assert!( - hub2.expire_stale() >= 1, - "package={package}: expiry counts from the original accept" - ); - assert!(!hub2.contains(&victim_id)); - drop(hub2); - let _ = std::fs::remove_dir_all(&dir); + let mut fat = Vec::with_capacity(5_000); + for _ in 0..9 { + fat.push(0x4d); + fat.extend_from_slice(&520u16.to_le_bytes()); + fat.extend(std::iter::repeat_n(0u8, 520)); + fat.push(0x75); } + fat.push(0x51); + let anchor = spend_true(cbs[0], 2_000, ScriptBuf::from_bytes(fat)); + let victim = spend_true(cbs[1], 3_000, ScriptBuf::from_bytes(vec![0x51])); + let victim_id = victim.compute_txid(); + let budget = anchor.weight().to_wu() + victim.weight().to_wu() + 500; + let dir = tmp(); + let hub = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); + hub.set_relay_enabled(true); + let t0 = 1_700_000_000u64; + hub.note_mock_now(t0); + hub.accept_tx(&anchor).expect("anchor fits"); + hub.accept_tx(&victim).expect("victim fits"); + hub.note_mock_now(t0 + 600); + let [replacement, child] = + package_evicted_with_anchor(&anchor, cbs[1], 60_000, 50_000, budget); + let err = hub.accept_package(&[replacement, child]).map(|_| ()); + assert!( + matches!(err, Err(AcceptError::Policy("mempool full"))), + "got {err:?}" + ); + assert!(hub.contains(&victim_id), "restored"); + assert_eq!( + hub.accept_time_txid(&victim_id), + Some(t0), + "RAM keeps the original accept time" + ); + hub.flush().expect("shutdown flush"); + drop(hub); + let hub2 = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); + assert!(hub2.contains(&victim_id), "reloaded"); + assert_eq!( + hub2.accept_time_txid(&victim_id), + Some(t0), + "disk keeps the original accept time" + ); + hub2.set_expiry_hours(1); + hub2.note_mock_now(t0 + 3600 + 5); + assert!( + hub2.expire_stale() >= 1, + "expiry counts from the original accept" + ); + assert!(!hub2.contains(&victim_id)); + drop(hub2); + let _ = std::fs::remove_dir_all(&dir); } /// A conflict cheaper than the chunk the failed attempt evicted must stay, @@ -7260,71 +7649,50 @@ mod tests { let q = &owned_q; let cbs = owned_cbs.as_slice(); let spk = ScriptBuf::from_bytes(vec![0x51]); - for package in [false, true] { - // Cheap ancestor so the replacement is not its own protected chunk. - let anchor_spk = cheap_anchor_script(); - let anchor_wu = spend_true(cbs[0], 0, anchor_spk.clone()).weight().to_wu(); - let victim_wu = spend_true(cbs[1], 0, spk.clone()).weight().to_wu(); - let budget = anchor_wu + victim_wu + 500; - let dir = tmp(); - let hub = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); - hub.set_relay_enabled(true); - let floor = hub.lock_read().mempool_min_fee_sat_kvb(); - let anchor = spend_true(cbs[0], fee_at_least(anchor_wu, floor), anchor_spk); - let anchor_id = anchor.compute_txid(); - let anchor_out = anchor.output[0].value.to_sat(); - let repl_fee = 80_000u64; - let replacement = heavier_than( - OutPoint { - txid: cbs[1], - vout: 0, - }, - OutPoint { - txid: anchor_id, - vout: 0, - }, - 50_0000_0000 + anchor_out - repl_fee, - victim_wu + 500, - ); - let victim_fee = fee_at_least(victim_wu, floor); - let victim = spend_true(cbs[1], victim_fee, spk.clone()); - let victim_id = victim.compute_txid(); - let victim_rate = - rbitcoin_consensus::policy::fee_rate_sat_per_kvb(victim_fee, victim_wu); - assert!( - hub.accept_tx(&anchor).is_ok(), - "package={package}: anchor fits" - ); - assert!( - hub.accept_tx(&victim).is_ok(), - "package={package}: victim at the pre-eviction floor fits, fee {victim_fee} floor {floor}" - ); - let err = submit_one(&hub, &replacement, package); - assert!( - matches!(err, Err(AcceptError::Policy("mempool full"))), - "package={package}: replacement must not stay, got {err:?}" - ); - let enforced = hub.lock_read().mempool_min_fee_sat_kvb(); - assert!( - enforced > victim_rate, - "package={package}: eviction must price out the victim ({victim_rate} vs {enforced})" - ); - assert!(!hub.contains(&replacement.compute_txid())); - assert!( - hub.contains(&victim_id), - "package={package}: below-floor conflict must stay" - ); - assert!( - !hub.try_recent_reject(&victim.compute_wtxid()), - "package={package}: restored conflict must not be recent-rejected" - ); - assert_eq!(hub.orphan_count(), 0); - assert!( - hub.wtxid_by_txid.lock().unwrap().contains_key(&victim_id), - "package={package}: relay map must still name the conflict" - ); - let _ = std::fs::remove_dir_all(&dir); - } + // Cheap anchor so the package child is not its own protected chunk. + let anchor_spk = cheap_anchor_script(); + let anchor_wu = spend_true(cbs[0], 0, anchor_spk.clone()).weight().to_wu(); + let victim_wu = spend_true(cbs[1], 0, spk.clone()).weight().to_wu(); + let budget = anchor_wu + victim_wu + 500; + let dir = tmp(); + let hub = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); + hub.set_relay_enabled(true); + let floor = hub.lock_read().mempool_min_fee_sat_kvb(); + let anchor = spend_true(cbs[0], fee_at_least(anchor_wu, floor), anchor_spk); + let victim_fee = fee_at_least(victim_wu, floor); + let victim = spend_true(cbs[1], victim_fee, spk.clone()); + let victim_id = victim.compute_txid(); + let victim_rate = rbitcoin_consensus::policy::fee_rate_sat_per_kvb(victim_fee, victim_wu); + assert!(hub.accept_tx(&anchor).is_ok(), "anchor fits"); + assert!( + hub.accept_tx(&victim).is_ok(), + "victim at the pre-eviction floor fits, fee {victim_fee} floor {floor}" + ); + let [replacement, child] = + package_evicted_with_anchor(&anchor, cbs[1], 80_000, 50_000, budget); + let replacement_id = replacement.compute_txid(); + let err = hub.accept_package(&[replacement, child]).map(|_| ()); + assert!( + matches!(err, Err(AcceptError::Policy("mempool full"))), + "replacement must not stay, got {err:?}" + ); + let enforced = hub.lock_read().mempool_min_fee_sat_kvb(); + assert!( + enforced > victim_rate, + "eviction must price out the victim ({victim_rate} vs {enforced})" + ); + assert!(!hub.contains(&replacement_id)); + assert!(hub.contains(&victim_id), "below-floor conflict must stay"); + assert!( + !hub.try_recent_reject(&victim.compute_wtxid()), + "restored conflict must not be recent-rejected" + ); + assert_eq!(hub.orphan_count(), 0); + assert!( + hub.wtxid_by_txid.lock().unwrap().contains_key(&victim_id), + "relay map must still name the conflict" + ); + let _ = std::fs::remove_dir_all(&dir); } /// A conflict set that was only valid as CPFP (parent under min relay, @@ -7335,72 +7703,49 @@ mod tests { let q = &owned_q; let cbs = owned_cbs.as_slice(); let spk = ScriptBuf::from_bytes(vec![0x51]); - for package in [false, true] { - let anchor_spk = cheap_anchor_script(); - let anchor_wu = spend_true(cbs[0], 0, anchor_spk.clone()).weight().to_wu(); - let (parent, child) = cpfp_child_sorts_first(cbs[1], &spk); - assert!( - child.compute_txid() < parent.compute_txid(), - "child txid must sort first so txid order is not ancestor order" - ); - let parent_id = parent.compute_txid(); - let child_id = child.compute_txid(); - let budget = anchor_wu + parent.weight().to_wu() + child.weight().to_wu() + 500; - let dir = tmp(); - let hub = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); - hub.set_relay_enabled(true); - let floor = hub.lock_read().mempool_min_fee_sat_kvb(); - let anchor = spend_true(cbs[0], fee_at_least(anchor_wu, floor), anchor_spk); - let anchor_id = anchor.compute_txid(); - let anchor_out = anchor.output[0].value.to_sat(); - let repl_fee = 80_000u64; - let replacement = heavier_than( - OutPoint { - txid: cbs[1], - vout: 0, - }, - OutPoint { - txid: anchor_id, - vout: 0, - }, - 50_0000_0000 + anchor_out - repl_fee, - parent.weight().to_wu() + child.weight().to_wu() + 500, - ); - assert!( - hub.accept_tx(&anchor).is_ok(), - "package={package}: anchor fits" - ); - hub.accept_package(&[parent.clone(), child.clone()]) - .expect("CPFP package admits a below-min-relay parent"); - assert!(hub.contains(&parent_id)); - assert!(hub.contains(&child_id)); - let err = submit_one(&hub, &replacement, package); - assert!( - matches!(err, Err(AcceptError::Policy("mempool full"))), - "package={package}: replacement must not stay, got {err:?}" - ); - assert!(!hub.contains(&replacement.compute_txid())); - assert!( - hub.contains(&parent_id), - "package={package}: below-min-relay parent must stay" - ); - assert!( - hub.contains(&child_id), - "package={package}: paying child must stay" - ); - assert_eq!( - hub.orphan_count(), - 0, - "package={package}: child must not be parked waiting on its parent" - ); - assert!(!hub.try_recent_reject(&parent.compute_wtxid())); - assert!(!hub.try_recent_reject(&child.compute_wtxid())); - let maps = hub.wtxid_by_txid.lock().unwrap(); - assert!(maps.contains_key(&parent_id)); - assert!(maps.contains_key(&child_id)); - drop(maps); - let _ = std::fs::remove_dir_all(&dir); - } + let anchor_spk = cheap_anchor_script(); + let anchor_wu = spend_true(cbs[0], 0, anchor_spk.clone()).weight().to_wu(); + let (parent, paying) = cpfp_child_sorts_first(cbs[1], &spk); + assert!( + paying.compute_txid() < parent.compute_txid(), + "child txid must sort first so txid order is not ancestor order" + ); + let parent_id = parent.compute_txid(); + let paying_id = paying.compute_txid(); + let budget = anchor_wu + parent.weight().to_wu() + paying.weight().to_wu() + 500; + let dir = tmp(); + let hub = MempoolHub::open_with_weight(&dir, Arc::clone(q), budget).unwrap(); + hub.set_relay_enabled(true); + let floor = hub.lock_read().mempool_min_fee_sat_kvb(); + let anchor = spend_true(cbs[0], fee_at_least(anchor_wu, floor), anchor_spk); + assert!(hub.accept_tx(&anchor).is_ok(), "anchor fits"); + hub.accept_package(&[parent.clone(), paying.clone()]) + .expect("CPFP package admits a below-min-relay parent"); + assert!(hub.contains(&parent_id)); + assert!(hub.contains(&paying_id)); + let [replacement, child] = + package_evicted_with_anchor(&anchor, cbs[1], 80_000, 50_000, budget); + let replacement_id = replacement.compute_txid(); + let err = hub.accept_package(&[replacement, child]).map(|_| ()); + assert!( + matches!(err, Err(AcceptError::Policy("mempool full"))), + "replacement must not stay, got {err:?}" + ); + assert!(!hub.contains(&replacement_id)); + assert!(hub.contains(&parent_id), "below-min-relay parent must stay"); + assert!(hub.contains(&paying_id), "paying child must stay"); + assert_eq!( + hub.orphan_count(), + 0, + "child must not be parked waiting on its parent" + ); + assert!(!hub.try_recent_reject(&parent.compute_wtxid())); + assert!(!hub.try_recent_reject(&paying.compute_wtxid())); + let maps = hub.wtxid_by_txid.lock().unwrap(); + assert!(maps.contains_key(&parent_id)); + assert!(maps.contains_key(&paying_id)); + drop(maps); + let _ = std::fs::remove_dir_all(&dir); } fn cheap_anchor_script() -> ScriptBuf { @@ -7420,12 +7765,36 @@ mod tests { vsize.saturating_mul(sat_kvb).div_ceil(1000).max(1) } - fn submit_one(hub: &MempoolHub, tx: &Transaction, package: bool) -> Result<(), AcceptError> { - if package { - hub.accept_package(std::slice::from_ref(tx)).map(|_| ()) - } else { - hub.accept_tx(tx).map(|_| ()) - } + /// `[replacement, child]`. The replacement spends only the confirmed + /// conflict. The child also spends `anchor`, so evicting that anchor drops + /// the child once their weight exceeds `budget`. + fn package_evicted_with_anchor( + anchor: &Transaction, + conflict: Txid, + repl_fee: u64, + child_fee: u64, + budget: u64, + ) -> [Transaction; 2] { + let replacement = spend_true(conflict, repl_fee, ScriptBuf::from_bytes(vec![0x51])); + let held = anchor.weight().to_wu() + replacement.weight().to_wu(); + assert!( + held <= budget, + "replacement must fit beside the anchor ({held} > {budget})" + ); + let input_value = replacement.output[0].value.to_sat() + anchor.output[0].value.to_sat(); + let child = heavier_than( + OutPoint { + txid: replacement.compute_txid(), + vout: 0, + }, + OutPoint { + txid: anchor.compute_txid(), + vout: 0, + }, + input_value - child_fee, + budget - held, + ); + [replacement, child] } fn heavier_than(a: OutPoint, b: OutPoint, value: u64, min_wu: u64) -> Transaction { @@ -7549,6 +7918,185 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// A 1p1c admit inside the block-connect walk must also release orphans + /// that wait only on the packaged parent. + #[test] + fn package_inside_promote_admits_parent_orphans() { + use rbitcoin_consensus::{mine_regtest_paying, ChainParams, Milestone}; + use rbitcoin_primitives::Height; + use rbitcoin_query::accept_and_connect_block; + + let (store, q, cbs) = copy_maturity_pad(3); + let params = ChainParams::regtest(); + let dir = tmp(); + let hub = MempoolHub::open(&dir, Arc::clone(&q)).unwrap(); + hub.set_relay_enabled(true); + let spk = ScriptBuf::from_bytes(vec![0x51]); + let half = (50_0000_0000 - 1) / 2; + let low_parent = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint { + txid: cbs[0], + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }], + output: vec![ + TxOut { + value: Amount::from_sat(half), + script_pubkey: spk.clone(), + }, + TxOut { + value: Amount::from_sat(50_0000_0000 - 1 - half), + script_pubkey: spk.clone(), + }, + ], + }; + assert!( + hub.try_note_extra_compact(&low_parent), + "low parent is remembered for 1p1c and is not in the mempool" + ); + assert!(!hub.try_contains(&low_parent.compute_txid())); + let sibling = spend_vout( + OutPoint { + txid: low_parent.compute_txid(), + vout: 1, + }, + 50_0000_0000 - 1 - half - 20_000, + ); + let err = hub.accept_tx_from(&sibling, Some(1)).unwrap_err(); + assert!( + matches!(err, AcceptError::Orphaned { .. }), + "sibling waits on the low parent, got {err:?}" + ); + let conf = spend_true(cbs[1], 10_000, spk.clone()); + let child = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![ + TxIn { + previous_output: OutPoint { + txid: conf.compute_txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }, + TxIn { + previous_output: OutPoint { + txid: low_parent.compute_txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }, + ], + output: vec![TxOut { + value: Amount::from_sat(50_0000_0000 - 10_000 + half - 40_000), + script_pubkey: spk.clone(), + }], + }; + let err = hub.accept_tx_from(&child, Some(1)).unwrap_err(); + assert!( + matches!(err, AcceptError::Orphaned { .. }), + "child waits on the confirmed parent and the low parent, got {err:?}" + ); + let tip_h = q.tip_height().unwrap().0; + let fk = q.tip_header_fk().unwrap().unwrap(); + let rec = q.get_header(fk).unwrap(); + let prev = bitcoin::BlockHash::from_byte_array(rec.hash); + let block = mine_regtest_paying( + prev, + rec.timestamp + 600, + tip_h + 1, + spk, + vec![conf.clone()], + ); + accept_and_connect_block(&q, ¶ms, Height(tip_h + 1), &block, Milestone::NONE) + .expect("connect the chain parent"); + super::reset_promote_depth(); + hub.remove_for_block(&[conf.compute_txid()]); + assert!( + hub.contains(&low_parent.compute_txid()), + "1p1c inside the walk must admit the low parent" + ); + assert!(hub.contains(&child.compute_txid())); + assert!( + hub.contains(&sibling.compute_txid()), + "orphans of the packaged parent must be admitted in the same walk" + ); + assert_eq!(hub.orphan_count(), 0); + assert_eq!(super::promote_depth_max(), 1); + let _ = std::fs::remove_dir_all(&dir); + let _ = std::fs::remove_dir_all(&store); + } + + /// A parked chain admits when the root does, on one promote frame. + #[test] + fn parked_chain_promotes_in_one_frame() { + let (_store, q, cbs) = pad_cbs(3); + let dir = tmp(); + let hub = MempoolHub::open(&dir, Arc::clone(&q)).unwrap(); + hub.set_relay_enabled(true); + let spk = ScriptBuf::from_bytes(vec![0x51]); + let mut value = 50_0000_0000 - 10_000; + let root = spend_true(cbs[0], 10_000, spk.clone()); + let mut parent = root.compute_txid(); + let mut parked = Vec::new(); + for _ in 0..4 { + value -= 1_000; + let child = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint { + txid: parent, + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }], + output: vec![TxOut { + value: Amount::from_sat(value), + script_pubkey: spk.clone(), + }], + }; + parent = child.compute_txid(); + parked.push(child); + } + for child in parked.iter().rev() { + let err = hub.accept_tx(child).unwrap_err(); + assert!( + matches!(err, AcceptError::Orphaned { .. }), + "link must park, got {err:?}" + ); + } + assert_eq!(hub.orphan_count(), parked.len()); + super::reset_promote_depth(); + hub.accept_tx(&root).expect("root admits the parked chain"); + assert_eq!(hub.orphan_count(), 0); + assert!(hub.try_contains(&root.compute_txid())); + for child in &parked { + assert!( + hub.try_contains(&child.compute_txid()), + "descendant must be live" + ); + } + assert_eq!( + super::promote_depth_max(), + 1, + "the hub walk must not take a frame per descendant" + ); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn mempool_under_pressure() { let (_store, owned_q, owned_cbs) = pad_cbs(3); diff --git a/crates/rbitcoin-query/src/query_tests.rs b/crates/rbitcoin-query/src/query_tests.rs index eb3f3eb5f..2e02c57d2 100644 --- a/crates/rbitcoin-query/src/query_tests.rs +++ b/crates/rbitcoin-query/src/query_tests.rs @@ -396,6 +396,40 @@ fn spend_op_true( (h1, spend, spend_txid) } +/// A one-block catch-up still returns the spend. A clipped window above +/// eight weeks is refused before the block scan. +#[test] +fn history_window_wider_than_eight_weeks_is_refused() { + let (dir, q) = temp_query("history-window-width"); + q.set_spend_index(true); + let (h0, ta0) = funded_op_true_coinbase(0, Fk::NULL, None); + let create_txid = ta0.tx.txid; + let hfk0 = q.connect_block(Height(0), &h0, &[ta0]).unwrap(); + let create_fk = q.block_tx_fks(Height(0)).unwrap()[0]; + let (h1, spend, spend_txid) = spend_op_true(hfk0, h0.hash, create_fk, create_txid); + q.connect_block(Height(1), &h1, &[spend]).unwrap(); + let sh = rbitcoin_store::script_hash(&[0x51]); + + let narrow = crate::scripthash::HistoryFilter::height_window(1, Some(2)); + let rows = q.scripthash_history_filtered(&sh, &narrow).unwrap(); + assert_eq!( + rows.rows.iter().map(|row| row.txid).collect::>(), + vec![spend_txid] + ); + + let mut wide_view = q.pin_sh_chain_view().unwrap().expect("sh view"); + wide_view.height = Height(1 + 8_064); + let wide = crate::scripthash::HistoryFilter::height_window(1, None); + let err = q + .scripthash_history_filtered_in(&sh, &wide, &wide_view) + .unwrap_err(); + assert!( + matches!(err, StoreError::Rejected("history window too wide")), + "{err}" + ); + let _ = std::fs::remove_dir_all(&dir); +} + /// Slow-pin log is only emitted when load work_ms > 2s — keep the formatter unit. #[test] fn format_slow_pin_omits_retired_tokens() { diff --git a/crates/rbitcoin-query/src/scripthash.rs b/crates/rbitcoin-query/src/scripthash.rs index da4efa7d6..155d6bf6a 100644 --- a/crates/rbitcoin-query/src/scripthash.rs +++ b/crates/rbitcoin-query/src/scripthash.rs @@ -10,6 +10,8 @@ use std::sync::Arc; /// Class A expand / spend-join wave. Bounds decoded `txout` pages in RAM. const SH_JOIN_WAVE: usize = 4096; +/// Electrum Cash catch-up. About eight weeks at 144 blocks a day. +const MAX_HISTORY_WINDOW: u32 = 8_064; /// Strong creates read from the posting tail while the page bound is open. const NEWEST_TAIL_CHUNK: usize = 256; @@ -1543,6 +1545,10 @@ impl Query { if from > high { return Ok(apply_history_filter(&[], filter)); } + let span = high.saturating_sub(from).saturating_add(1); + if span > MAX_HISTORY_WINDOW { + return Err(StoreError::Rejected("history window too wide")); + } let members = self.strong_sh_creates(scripthash, view)?; let heights = self.store.tx_height_get_batch(&members)?; if heights.len() != members.len() { diff --git a/crates/rbitcoin-sv2/src/lib.rs b/crates/rbitcoin-sv2/src/lib.rs index 97c220a53..e62da566f 100644 --- a/crates/rbitcoin-sv2/src/lib.rs +++ b/crates/rbitcoin-sv2/src/lib.rs @@ -73,9 +73,9 @@ pub struct Sv2TpConfig { /// How long a template on a replaced prev hash still answers requests. pub stale_grace: Duration, /// Deadline from TCP accept through the Noise handshake, - /// `SetupConnection`, and the first `CoinbaseOutputConstraints` (without - /// which the session never writes). No read deadline after that: TDP has - /// no keepalive and a client may stay silent while the TP pushes. + /// `SetupConnection`, and the first `CoinbaseOutputConstraints`. After + /// that frame, the same duration is the idle deadline: it moves only + /// when another frame arrives. Tip and fee pushes do not refresh it. pub setup_timeout: Duration, /// A socket write that makes no progress this long closes the session, /// so a client that stops reading cannot hold a slot. diff --git a/crates/rbitcoin-sv2/src/listener_tests.rs b/crates/rbitcoin-sv2/src/listener_tests.rs index 146015643..37e4b6f47 100644 --- a/crates/rbitcoin-sv2/src/listener_tests.rs +++ b/crates/rbitcoin-sv2/src/listener_tests.rs @@ -217,13 +217,35 @@ async fn session_without_constraints_is_dropped_at_the_setup_deadline() { ok.recv().await.expect("setup reply"); ok.coinbase_output_constraints(1, 1).await.unwrap(); + // Same-budget constraints do not rebuild. They do refresh the idle + // deadline, so this session can outlive the original setup deadline. + let since_constraints = Instant::now(); let closed = tokio::time::timeout(setup_timeout * 10, async { - while idle.recv().await.is_ok() {} + loop { + tokio::select! { + biased; + r = idle.recv() => { + if r.is_err() { + break; + } + } + _ = tokio::time::sleep(setup_timeout / 3) => { + ok.coinbase_output_constraints(1, 1) + .await + .expect("same budget keeps the session"); + } + } + } }) .await; assert!(closed.is_ok(), "session without constraints must be closed"); - tokio::time::sleep(setup_timeout * 2).await; + while since_constraints.elapsed() < setup_timeout * 2 { + tokio::time::sleep(setup_timeout / 3).await; + ok.coinbase_output_constraints(1, 1) + .await + .expect("same budget keeps the session"); + } ok.request_transaction_data(u64::MAX).await.unwrap(); loop { let f = ok @@ -237,6 +259,44 @@ async fn session_without_constraints_is_dropped_at_the_setup_deadline() { tp.shutdown().await; } +/// One `CoinbaseOutputConstraints` then silence. The setup deadline has +/// already passed, so only a post-setup idle deadline frees the slot. +#[tokio::test] +async fn session_silent_after_constraints_is_dropped_at_the_idle_deadline() { + let tc = shared_regtest(0); + let chain = Arc::clone(&tc.chain); + let setup_timeout = Duration::from_millis(300); + let tp = run_sv2_tp(Sv2TpConfig { + listen: "127.0.0.1:0".parse().unwrap(), + chain, + authority_secret: [7; 32], + cert_validity: Duration::from_secs(3600), + stale_grace: Duration::from_secs(10), + setup_timeout, + write_timeout: WRITE_TIMEOUT, + fee_delta: FEE_DELTA, + template_interval: TEMPLATE_INTERVAL, + }) + .await + .expect("listen"); + let (addr, pk) = (tp.local_addr, tp.authority_pubkey); + + let mut c = TpClient::connect(addr, pk).await.expect("handshake"); + c.setup_connection(TDP, 2, 2, 0).await.unwrap(); + c.recv().await.expect("setup reply"); + c.coinbase_output_constraints(1, 1).await.unwrap(); + + let closed = tokio::time::timeout(setup_timeout * 10, async { + while c.recv().await.is_ok() {} + }) + .await; + assert!( + closed.is_ok(), + "session silent after constraints must be closed" + ); + tp.shutdown().await; +} + /// A client that floods requests and never reads jams the TP's writes; the /// write deadline must close the session instead of stalling it forever. #[tokio::test] diff --git a/crates/rbitcoin-sv2/src/session.rs b/crates/rbitcoin-sv2/src/session.rs index ff99e81fc..3332eb875 100644 --- a/crates/rbitcoin-sv2/src/session.rs +++ b/crates/rbitcoin-sv2/src/session.rs @@ -181,7 +181,7 @@ pub(crate) async fn serve( fee_check_at: None, seen_updates: 0, }; - s.run(&mut frames, deadline).await + s.run(&mut frames, deadline, setup_timeout).await } fn micros(d: Duration) -> u64 { @@ -218,15 +218,18 @@ struct Session { } impl Session { - /// `setup_deadline` also covers the first `CoinbaseOutputConstraints`: - /// until then the session can never get a template, so it never writes - /// and the write deadline cannot free its slot. + /// `setup_deadline` covers the handshake through the first + /// `CoinbaseOutputConstraints`. After that, `setup_timeout` is an idle + /// deadline refreshed only when a frame is handled, so tip and fee-check + /// wakes do not keep a silent client. async fn run( &mut self, frames: &mut mpsc::Receiver>, setup_deadline: Instant, + setup_timeout: Duration, ) -> io::Result<()> { let mut tips = self.chain.subscribe_tips(); + let mut idle_at = setup_deadline; loop { let retire_at = self.templates.next_retire(); tokio::select! { @@ -235,6 +238,7 @@ impl Session { if !self.on_frame(f?).await? { return Ok(()); } + idle_at = Instant::now() + setup_timeout; } tip = tips.recv() => match tip { Ok(ev) if !self.templates.on_tip(ev.hash.to_byte_array()) => {} @@ -263,6 +267,9 @@ impl Session { _ = tokio::time::sleep_until(setup_deadline), if self.constraints.is_none() => { return Err(missed_setup_deadline()); } + _ = tokio::time::sleep_until(idle_at), if self.constraints.is_some() => { + return Err(io::Error::new(io::ErrorKind::TimedOut, "sv2: idle deadline")); + } } } } diff --git a/crates/rbitcoin-test/tests/cross_surface.rs b/crates/rbitcoin-test/tests/cross_surface.rs index d5f3660ed..8367741fc 100644 --- a/crates/rbitcoin-test/tests/cross_surface.rs +++ b/crates/rbitcoin-test/tests/cross_surface.rs @@ -1609,8 +1609,7 @@ async fn esplora_broadcast_visible_in_rpc_and_electrum() { assert_eq!(st, 200, "POST /tx: {body}"); assert_eq!(body, txid_hex); let (st, body) = http_get(esplora_addr, &format!("/broadcast?tx={hex}")).await; - assert_eq!(st, 400, "GET /broadcast of live tx: {body}"); - assert!(body.contains("duplicate"), "{body}"); + assert_eq!(st, 404, "GET /broadcast is not served: {body}"); let (st, body) = http_get(esplora_addr, &format!("/txs/outspends?txids={cb_hex}")).await; assert_eq!(st, 200, "GET /txs/outspends: {body}"); let outspends: Value = serde_json::from_str(&body).unwrap(); diff --git a/docs/external_findings/036-electrum-public-surface.md b/docs/external_findings/036-electrum-public-surface.md index af8ae1183..607cbe9ec 100644 --- a/docs/external_findings/036-electrum-public-surface.md +++ b/docs/external_findings/036-electrum-public-surface.md @@ -7,7 +7,8 @@ An unpaged scripthash join above `--max-sh-creates` is refused. The default is 10000. **0** is unlimited. A history request that names a page is served even above the cap, and Class A expand stops once that page is -full. +full. A `from_height` of 1 or more scans that window and does not use the +create cap. The clipped span is refused above 8,064 blocks. Silent-payment subscribe does not write the scan secret to the API log. The log file is owner-only. A start of 0 is the last 256 blocks. The diff --git a/docs/external_findings/051-rbfr-direct-set.md b/docs/external_findings/051-rbfr-direct-set.md index 56d808266..e6358ccc3 100644 --- a/docs/external_findings/051-rbfr-direct-set.md +++ b/docs/external_findings/051-rbfr-direct-set.md @@ -10,6 +10,9 @@ package. `pure_rbfr_pays` takes that direct fee and weight. `rbf_allows_replacement` still also allows a BIP125-style payment against the full conflict set. A high-feerate replacement with a lower absolute fee than a fat descendant package is admitted by the direct-set rule. +An unconfirmed input must be a parent of the direct conflicts, and only +one direct conflict is allowed. That input rule does not change +direct-set pricing. **Regression:** `rbitcoin-mempool` `accept::tests::mempool_accept_life`, diff --git a/docs/external_findings/092-orphan-promote-stack.md b/docs/external_findings/092-orphan-promote-stack.md new file mode 100644 index 000000000..614046059 --- /dev/null +++ b/docs/external_findings/092-orphan-promote-stack.md @@ -0,0 +1,11 @@ +# 092 — Orphan promotion recurses down the parked chain + +**Severity:** medium +**Status:** fixed +**Found by:** bobberb and muse, 2026-10-18 + +`promote_orphans_of` called itself once per admitted child. The relay hub did the same: `promote_orphans_staged` called `accept_with_utxo`, and a successful admit called `promote_orphans_staged` again. A parked chain is bounded by the orphan count cap (3,000), and each frame held that child's admit. Both walks are queues. The first child is admitted, then its descendants, then the next sibling. An admit inside the hub walk does not start a second walk. + +A package admit inside that walk publishes the low-fee parent as well as the child. Orphans that wait only on the parent are queued on the same walk. A conflict put back after a failed admit is queued there too. + +**Regression:** `rbitcoin-mempool` `orphan_chain_promotes_to_the_cluster_cap`. `rbitcoin-net` `parked_chain_promotes_in_one_frame`, `package_inside_promote_admits_parent_orphans`. diff --git a/docs/external_findings/093-history-window.md b/docs/external_findings/093-history-window.md new file mode 100644 index 000000000..ebbc733c7 --- /dev/null +++ b/docs/external_findings/093-history-window.md @@ -0,0 +1,9 @@ +# 093 — History catch-up scans every block in the window + +**Severity:** high +**Status:** fixed +**Found by:** bobberb and muse, 2026-10-18 + +`get_history` with `from_height` of 0 is the capped scripthash join. `from_height` of 1 or more is a catch-up: it loads the posting and reads `input.body` for every block in the clipped window, and it does not consult `--max-sh-creates`. A window wider than 8,064 blocks is refused before that scan. A one-block catch-up still returns the spend. + +**Regression:** `rbitcoin-query` `history_window_wider_than_eight_weeks_is_refused`. diff --git a/docs/external_findings/094-electrum-write-deadline.md b/docs/external_findings/094-electrum-write-deadline.md new file mode 100644 index 000000000..cfcc8347c --- /dev/null +++ b/docs/external_findings/094-electrum-write-deadline.md @@ -0,0 +1,9 @@ +# 094 — Electrum writes have no deadline + +**Severity:** high +**Status:** fixed +**Found by:** bobberb and muse, 2026-10-18 + +The idle timer wrapped the next read. `write_line` was `write_all` plus `flush`, so a client that stopped reading held one of 256 connection slots for as long as the socket blocked. A response write now uses that same idle deadline and the connection task ends on the timeout. A scripthash status push uses that write too: a timeout ends the task instead of being ignored. + +**Regression:** `rbitcoin-electrum` `stalled_electrum_write_is_closed`, `stalled_scripthash_push_ends_the_connection`. diff --git a/docs/external_findings/095-sv2-idle-deadline.md b/docs/external_findings/095-sv2-idle-deadline.md new file mode 100644 index 000000000..291b33d1f --- /dev/null +++ b/docs/external_findings/095-sv2-idle-deadline.md @@ -0,0 +1,9 @@ +# 095 — SV2 session has no idle deadline after setup + +**Severity:** high +**Status:** fixed +**Found by:** bobberb and muse, 2026-10-18 + +The setup deadline covered the handshake and the first `CoinbaseOutputConstraints`. After that frame, `frames.recv` waited with no deadline, so a client that went silent held one of eight session slots. The same setup timeout is now an idle deadline. It moves only when a frame is handled. Tip and fee-check wakes do not refresh it. + +**Regression:** `rbitcoin-sv2` `session_silent_after_constraints_is_dropped_at_the_idle_deadline`. diff --git a/docs/external_findings/096-unconfirmed-rbf-input.md b/docs/external_findings/096-unconfirmed-rbf-input.md new file mode 100644 index 000000000..21c4af423 --- /dev/null +++ b/docs/external_findings/096-unconfirmed-rbf-input.md @@ -0,0 +1,9 @@ +# 096 — Replacement may add an unconfirmed input + +**Severity:** medium +**Status:** fixed +**Found by:** bobberb and muse, 2026-10-18 + +A replacement could spend any mempool output that was not itself in the conflict set. Libre Relay keeps BIP125 rule 2 and adds one limit: an unconfirmed input must be a parent of the direct conflicts, and there may be only one direct conflict. A single-conflict CPFP of that parent still replaces, and the direct-set feerate rule is unchanged. + +**Regression:** `rbitcoin-mempool` `replacement_unconfirmed_inputs_follow_libre_rule`. diff --git a/docs/external_findings/097-recent-invalid-cap.md b/docs/external_findings/097-recent-invalid-cap.md new file mode 100644 index 000000000..7d1bcaa13 --- /dev/null +++ b/docs/external_findings/097-recent-invalid-cap.md @@ -0,0 +1,9 @@ +# 097 — Invalid-txid set clears at 4096 + +**Severity:** low +**Status:** fixed +**Found by:** bobberb and muse, 2026-10-18 + +`note_invalid` cleared the whole set at 4,096 entries, so one more invalid txid forgot every earlier one. The set now stops at the cap. A new id is not stored, and the oldest remains. This is the mempool invalid set, separate from the network recent-reject set. + +**Regression:** `rbitcoin-mempool` `recent_invalid_keeps_the_oldest`. diff --git a/docs/external_findings/098-overlay-discourage.md b/docs/external_findings/098-overlay-discourage.md new file mode 100644 index 000000000..8c5a7d3dd --- /dev/null +++ b/docs/external_findings/098-overlay-discourage.md @@ -0,0 +1,11 @@ +# 098 — Overlay misbehavior is not remembered + +**Severity:** medium +**Status:** fixed +**Found by:** bobberb and muse, 2026-10-18 + +Discouragement was keyed by the TCP address. An onion or I2P session's version socket is `0.0.0.0`, which is skipped so the refusal is never stored. The in-memory day-long set now keys onion and I2P by the 32-byte identity, so another port is the same peer. Loopback and `0.0.0.0` identities are still skipped. Nothing is written to `banlist.dat`. + +An outbound dial checks that same key, including a feeler and an IBD dial. A netgroup paused after an inbound slot eviction still cannot take another inbound slot. It does not refuse dials to the rest of the prefix. + +**Regression:** `rbitcoin-net` `overlay_misbehavior_discourages_that_net`, `overlay_refusal_ignores_the_port`, `slot_evict_does_not_block_an_outbound_dial`, `feeler_does_not_dial_a_discouraged_address`, `ibd_dial_refuses_a_discouraged_net`. diff --git a/docs/external_findings/099-esplora-get-broadcast.md b/docs/external_findings/099-esplora-get-broadcast.md new file mode 100644 index 000000000..80cf12ab2 --- /dev/null +++ b/docs/external_findings/099-esplora-get-broadcast.md @@ -0,0 +1,9 @@ +# 099 — GET /broadcast has no Origin check + +**Severity:** medium +**Status:** fixed +**Found by:** bobberb and muse, 2026-10-18 + +`GET /broadcast?tx=` admitted a transaction with no Origin check, so a browser page could submit it. The route is gone. `POST /tx` is the admit path. + +**Regression:** `rbitcoin-esplora` `esplora_unix_internal`. diff --git a/docs/external_findings/100-script-pool-panic.md b/docs/external_findings/100-script-pool-panic.md new file mode 100644 index 000000000..0c15cc440 --- /dev/null +++ b/docs/external_findings/100-script-pool-panic.md @@ -0,0 +1,9 @@ +# 100 — Script-pool panic sticks the wave + +**Severity:** medium +**Status:** fixed +**Found by:** bobberb and muse, 2026-10-18 + +A chunk incremented `in_wave` and decremented it only after the work returned. A panic skipped that decrement, so `wait_done` never returned and the worker thread died. The count now drops with the chunk, including on unwind, and the worker stays up. The wave finishes as an error. + +**Regression:** `rbitcoin-consensus` `chunk_panic_releases_the_wave`. diff --git a/docs/external_findings/101-bobberb-remote-index.md b/docs/external_findings/101-bobberb-remote-index.md new file mode 100644 index 000000000..2a95583ce --- /dev/null +++ b/docs/external_findings/101-bobberb-remote-index.md @@ -0,0 +1,21 @@ +# 101 — Remote-limit review index + +Review by bobberb and muse, 2026-10-18. This index is the status board +for the remote-request and relay items. Each fixed row names the +regression. No reproduction steps. + +| Id | Severity | Topic | Status | Regression | +|----|----------|--------|--------|------------| +| D1 | medium | Orphan promotion recurses down the parked chain | fixed | `orphan_chain_promotes_to_the_cluster_cap`, `parked_chain_promotes_in_one_frame`, `package_inside_promote_admits_parent_orphans` ([092](./092-orphan-promote-stack.md)) | +| D2 | — | Declared vector length preallocates | false | rust-bitcoin 0.32.102 caps `MAX_VEC_SIZE` at 4,000,000 | +| D3 | high | History catch-up scans every block in the window | fixed | `history_window_wider_than_eight_weeks_is_refused` ([093](./093-history-window.md)) | +| D4 | high | Electrum writes have no deadline | fixed | `stalled_electrum_write_is_closed`, `stalled_scripthash_push_ends_the_connection` ([094](./094-electrum-write-deadline.md)) | +| D5 | high | SV2 session has no idle deadline after setup | fixed | `session_silent_after_constraints_is_dropped_at_the_idle_deadline` ([095](./095-sv2-idle-deadline.md)) | +| D6 | — | Replacement feerate uses the full descendant set | won't-fix | Direct-set pricing stays. See [051](./051-rbfr-direct-set.md) | +| D7 | medium | Replacement may add an unconfirmed input | fixed | `replacement_unconfirmed_inputs_follow_libre_rule` ([096](./096-unconfirmed-rbf-input.md)) | +| D8 | low | Invalid-txid set clears at 4096 | fixed | `recent_invalid_keeps_the_oldest` ([097](./097-recent-invalid-cap.md)) | +| D9 | medium | Overlay misbehavior is not remembered | fixed | `overlay_misbehavior_discourages_that_net`, `overlay_refusal_ignores_the_port`, `slot_evict_does_not_block_an_outbound_dial`, `feeler_does_not_dial_a_discouraged_address`, `ibd_dial_refuses_a_discouraged_net` ([098](./098-overlay-discourage.md)) | +| D10 | medium | GET /broadcast has no Origin check | fixed | `esplora_unix_internal` ([099](./099-esplora-get-broadcast.md)) | +| D11 | — | Default history ignores the create cap | partial | The open window is [093](./093-history-window.md). One-argument history stays under `--max-sh-creates` | +| D12 | — | Constraint rebuilds are unbounded | won't-fix | The 1s cooldown is the limit (`template_constraint_rebuilds_are_rate_limited`) | +| D13 | medium | Script-pool panic sticks the wave | fixed | `chunk_panic_releases_the_wave` ([100](./100-script-pool-panic.md)) | diff --git a/docs/external_findings/README.md b/docs/external_findings/README.md index 6ef445584..2e92987be 100644 --- a/docs/external_findings/README.md +++ b/docs/external_findings/README.md @@ -95,6 +95,16 @@ rbitcoin reference, or redteam static analysis). Numbered reports live beside th | [088](./088-tip-chain-selection.md) | high | Tip-follow chain selection: total-work ranking, failed-branch prefix, sibling header check, reorg depth | partial (F3 reorg depth open) | `held_branch_with_more_total_work_beats_an_earlier_local_tie`, `failed_branch_tip_keeps_the_heavier_valid_prefix`, `sibling_claiming_more_work_with_wrong_bits_keeps_the_tip` | | [089](./089-missing-parent-verdict.md) | high | IBD halted on a block that spends an unknown txid; no verdict | fixed | `ibd_spend_of_an_unknown_txid_is_invalid`, `spend_of_a_create_the_head_lost_is_not_invalid`, `spend_of_an_unknown_txid_after_restart_is_invalid` | | [090](./090-getdata-serve-pause.md) | medium | Served getdata past the writer queue was dropped | fixed | `getdata_past_serve_cap_waits_for_writer`, `paused_session_still_times_out_a_silent_peer`, `paused_getdata_tail_is_served_before_the_next_getdata`, `uncounted_bodies_do_not_open_serve_slots` | +| [092](./092-orphan-promote-stack.md) | medium | Orphan promotion recurses down the parked chain | fixed | `orphan_chain_promotes_to_the_cluster_cap`, `parked_chain_promotes_in_one_frame`, `package_inside_promote_admits_parent_orphans` | +| [093](./093-history-window.md) | high | History catch-up scans every block in the window | fixed | `history_window_wider_than_eight_weeks_is_refused` | +| [094](./094-electrum-write-deadline.md) | high | Electrum writes have no deadline | fixed | `stalled_electrum_write_is_closed`, `stalled_scripthash_push_ends_the_connection` | +| [095](./095-sv2-idle-deadline.md) | high | SV2 session has no idle deadline after setup | fixed | `session_silent_after_constraints_is_dropped_at_the_idle_deadline` | +| [096](./096-unconfirmed-rbf-input.md) | medium | Replacement may add an unconfirmed input | fixed | `replacement_unconfirmed_inputs_follow_libre_rule` | +| [097](./097-recent-invalid-cap.md) | low | Invalid-txid set clears at 4096 | fixed | `recent_invalid_keeps_the_oldest` | +| [098](./098-overlay-discourage.md) | medium | Overlay misbehavior is not remembered | fixed | `overlay_misbehavior_discourages_that_net`, `overlay_refusal_ignores_the_port`, `slot_evict_does_not_block_an_outbound_dial`, `feeler_does_not_dial_a_discouraged_address`, `ibd_dial_refuses_a_discouraged_net` | +| [099](./099-esplora-get-broadcast.md) | medium | GET /broadcast has no Origin check | fixed | `esplora_unix_internal` | +| [100](./100-script-pool-panic.md) | medium | Script-pool panic sticks the wave | fixed | `chunk_panic_releases_the_wave` | +| [101](./101-bobberb-remote-index.md) | — | Remote-limit review index | — | — | | [102](./102-fuse8-body-length.md) | high | fuse8 body length cannot wrap the size check | fixed | `fuse8_wrapped_body_length_is_corrupt` | | [103](./103-fuse8-segment-multiple.md) | high | fuse8 segment count must divide the array | fixed | `fuse8_segment_count_must_be_a_multiple_of_segment_length` | | [104](./104-body-read-cap.md) | high | A body read is capped before it allocates | fixed | `body_read_above_the_cap_is_refused`, `index_read_above_the_cap_is_refused` | diff --git a/docs/operator/operations.md b/docs/operator/operations.md index f791b0345..dd5d8fd19 100644 --- a/docs/operator/operations.md +++ b/docs/operator/operations.md @@ -551,7 +551,7 @@ scrape_configs: | Min relay | **0.1 sat/vB** (100 sat/kvB) | | Dust | **not enforced** | | Script templates | allow if consensus-valid (within weight/CPU) | -| RBF | **full RBF** (no BIP125 signaling required) | +| RBF | **full RBF** (no BIP125 signaling required). An unconfirmed input is allowed only when it is a parent of the one direct conflict | | Annex | empty OK; non-empty only if first data byte after `0x50` is `0x00` | | Cluster caps | 64 txs / 101 kvB (404 kWU) | | Eviction | worst linearization **chunk** when over weight budget |