From 3ec792fddb98a76292e50fab76474458660aba94 Mon Sep 17 00:00:00 2001 From: Brandon Black Date: Fri, 9 Oct 2026 09:26:03 -0700 Subject: [PATCH 1/2] test: pin the first two-phase cargo-mutants misses Fee-filter equality, getblocktxn depth 10, txid notfound, and the send-budget read pause now fail if the comparison flips. Dead BIP34 negative-height encoding and the redundant coinbase clause are gone. Co-authored-by: Cursor --- changelog.d/missed-mutant-pins.md | 3 + .../src/block/finality_tests.rs | 118 ++++++++++++++++- crates/rbitcoin-consensus/src/block/mod.rs | 17 +-- .../src/block/sigop_cost_tests.rs | 21 +++ .../src/block/structure_rule_tests.rs | 123 ++++++++++++++++++ .../src/confirm_run/write_idempotent_tests.rs | 47 +++++++ crates/rbitcoin-consensus/src/convert.rs | 25 +++- crates/rbitcoin-consensus/src/header.rs | 12 +- crates/rbitcoin-consensus/src/lib.rs | 7 +- crates/rbitcoin-net/src/fee_history_file.rs | 10 ++ .../src/peer_blocksonly_journey.rs | 87 ++++++++++++- crates/rbitcoin-net/src/peer_tests.rs | 113 +++++++++++++++- 12 files changed, 559 insertions(+), 24 deletions(-) create mode 100644 changelog.d/missed-mutant-pins.md diff --git a/changelog.d/missed-mutant-pins.md b/changelog.d/missed-mutant-pins.md new file mode 100644 index 000000000..c6a2a8716 --- /dev/null +++ b/changelog.d/missed-mutant-pins.md @@ -0,0 +1,3 @@ +Changed + +- Nightly mutants: fee-filter equality, `getblocktxn` depth 10, txid `notfound`, and the send-budget read pause are pinned. BIP34 no longer carries a negative-height encoder. Archive coinbase detection is `OutPoint::is_null`. The no-precompute value sum uses the money-range check. diff --git a/crates/rbitcoin-consensus/src/block/finality_tests.rs b/crates/rbitcoin-consensus/src/block/finality_tests.rs index f909000b1..69bb92b23 100644 --- a/crates/rbitcoin-consensus/src/block/finality_tests.rs +++ b/crates/rbitcoin-consensus/src/block/finality_tests.rs @@ -216,7 +216,18 @@ fn height_zero_timelock_block(sequence: u32) -> bitcoin::Block { block } -/// Height-type locks ignore coin MTP (write path may leave mtps as 0). +/// Exactly 500_000_000 is a timestamp. One below the threshold is a height. +#[test] +fn locktime_at_threshold_is_a_timestamp() { + let t = LOCKTIME_THRESHOLD; + let tx = bare_tx(1, LockTime::from_time(t).unwrap(), Sequence::ZERO); + assert!( + is_final_tx(&tx, 1, t + 1), + "exactly {t} compares as a time, so a later cutoff is final" + ); + assert!(!is_final_tx(&tx, 1, t)); +} + #[test] fn bip68_height_type_ignores_zero_mtp() { let tx = bare_tx(2, LockTime::ZERO, Sequence::from_consensus(10)); @@ -225,3 +236,108 @@ fn bip68_height_type_ignores_zero_mtp() { assert!(sequence_locks_satisfied(&tx, &[100], &[u32::MAX], 110, 0)); assert!(!sequence_locks_satisfied(&tx, &[100], &[0], 109, 0)); } + +fn seq_tx(seqs: &[u32]) -> Transaction { + Transaction { + version: bitcoin::transaction::Version(2), + lock_time: LockTime::ZERO, + input: seqs + .iter() + .map(|seq| TxIn { + previous_output: OutPoint::null(), + script_sig: ScriptBuf::new(), + sequence: Sequence::from_consensus(*seq), + witness: Witness::new(), + }) + .collect(), + output: vec![TxOut { + value: Amount::from_sat(1), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }], + } +} + +fn csv_block(txs: Vec) -> bitcoin::Block { + use bitcoin::block::{Header, Version}; + use bitcoin::hashes::Hash; + use bitcoin::{Block, BlockHash, CompactTarget, TxMerkleNode}; + let mut block = Block { + header: Header { + version: Version::from_consensus(1), + prev_blockhash: BlockHash::from_byte_array([0; 32]), + merkle_root: TxMerkleNode::from_byte_array([0; 32]), + time: 1_231_006_505, + bits: CompactTarget::from_consensus(0x1d00_ffff), + nonce: 0, + }, + txdata: txs, + }; + block.header.merkle_root = block.compute_merkle_root().unwrap(); + block +} + +/// Three two-input txs must connect. `si * n_in` false-rejects the third. +#[test] +fn structural_bip68_multi_input_block_connects() { + let params = Box::leak(Box::new(crate::params::ChainParams::regtest())); + let ctx = super::ValidationContext::at( + params, + rbitcoin_primitives::Height(1), + crate::milestone::Milestone::NONE, + ); + let (_dir, query) = rbitcoin_query::testutil::tiny_query_labeled("bip68-multi"); + let coinbase = bare_tx(1, LockTime::ZERO, Sequence::MAX); + let spend = |seq: u32| seq_tx(&[seq, seq]); + let block = csv_block(vec![ + coinbase, + spend(u32::MAX), + spend(u32::MAX), + spend(u32::MAX), + ]); + let spends = vec![ + ( + [0u8; 32], + 0u32, + rbitcoin_primitives::Fk::NULL, + rbitcoin_primitives::Fk::NULL, + 0u32 + ); + 6 + ]; + let mut cache = rbitcoin_query::U32Map::default(); + cache.insert(0, 1_000); + super::structural_bip68( + &query, + &block, + &ctx, + &spends, + &rbitcoin_query::FkMap::default(), + &mut cache, + ) + .expect("a well-formed multi-input block passes the spend cursor"); +} + +/// Height-type CSV does not prefetch a median. A missing parent header must +/// still pass when the block's own previous median is already cached. +#[test] +fn structural_bip68_height_lock_skips_missing_coin_mtp() { + let params = Box::leak(Box::new(crate::params::ChainParams::regtest())); + let ctx = super::ValidationContext::at( + params, + rbitcoin_primitives::Height(6), + crate::milestone::Milestone::NONE, + ); + let (_dir, query) = rbitcoin_query::testutil::tiny_query_labeled("bip68-height-mtp"); + let create = rbitcoin_primitives::Fk(3); + let mut create_heights = rbitcoin_query::FkMap::default(); + create_heights.insert(create, 5); + let block = csv_block(vec![ + bare_tx(1, LockTime::ZERO, Sequence::MAX), + seq_tx(&[0]), + ]); + let spends = [([0u8; 32], 0u32, rbitcoin_primitives::Fk::NULL, create, 0u32)]; + let mut cache = rbitcoin_query::U32Map::default(); + cache.insert(5, 1_500_000); + super::structural_bip68(&query, &block, &ctx, &spends, &create_heights, &mut cache) + .expect("height-type lock must not read the coin median"); +} diff --git a/crates/rbitcoin-consensus/src/block/mod.rs b/crates/rbitcoin-consensus/src/block/mod.rs index 9f2eed54a..4bac6293b 100644 --- a/crates/rbitcoin-consensus/src/block/mod.rs +++ b/crates/rbitcoin-consensus/src/block/mod.rs @@ -617,20 +617,13 @@ pub fn bip34_height_script(height: u32) -> Vec { } let mut num = Vec::new(); let mut abs = n; - let neg = abs < 0; - if neg { - abs = -abs; - } while abs > 0 { num.push((abs & 0xff) as u8); abs >>= 8; } if let Some(last) = num.last() { if last & 0x80 != 0 { - num.push(if neg { 0x80 } else { 0x00 }); - } else if neg { - let i = num.len() - 1; - num[i] |= 0x80; + num.push(0x00); } } else { num.push(0); @@ -1471,12 +1464,12 @@ fn assemble_tx_value_out( None => { let mut value_out = 0i64; for o in &tx.output { - let sats = o.value.to_sat() as i64; - if sats < 0 { - return Err(ConsensusError::BadTx("negative output")); + let sats_u = o.value.to_sat(); + if exceeds_max_money(sats_u) { + return Err(ConsensusError::BadTx("bad-txns-vout-toolarge")); } value_out = value_out - .checked_add(sats) + .checked_add(sats_u as i64) .ok_or(ConsensusError::BadTx("value out overflow"))?; } Ok(value_out) diff --git a/crates/rbitcoin-consensus/src/block/sigop_cost_tests.rs b/crates/rbitcoin-consensus/src/block/sigop_cost_tests.rs index 5722f1b58..a5944ca4d 100644 --- a/crates/rbitcoin-consensus/src/block/sigop_cost_tests.rs +++ b/crates/rbitcoin-consensus/src/block/sigop_cost_tests.rs @@ -443,6 +443,27 @@ fn verify_one_script_job_skips_anyone_can_spend() { crate::block::ScriptVerifyFlags::buried(true, true, true, true, true), ); assert!(verify_one_script_job(&job).is_ok()); + + let mut witnessed = job; + witnessed.tx = crate::block::JobTx::owned(Transaction { + version: TxVersion::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint { + txid: Txid::from_byte_array([9; 32]), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::MAX, + witness: Witness::from_slice(&[vec![0x01]]), + }], + output: vec![TxOut { + value: Amount::from_sat(1), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }], + }); + let err = verify_one_script_job(&witnessed).expect_err("bare OP_TRUE rejects a witness"); + assert!(format!("{err}").contains("WITNESS_UNEXPECTED"), "{err:?}"); } #[test] diff --git a/crates/rbitcoin-consensus/src/block/structure_rule_tests.rs b/crates/rbitcoin-consensus/src/block/structure_rule_tests.rs index 90ce79100..7a402f539 100644 --- a/crates/rbitcoin-consensus/src/block/structure_rule_tests.rs +++ b/crates/rbitcoin-consensus/src/block/structure_rule_tests.rs @@ -962,6 +962,19 @@ fn script_sigop_count_and_last_push_helpers() { assert!(!is_p2wsh_program(&[0x51, 0x20])); } +#[test] +fn last_script_push_rejects_a_non_push_and_a_short_pushdata() { + // OP_16 (0x60) is an empty push. The next opcode is not a push. + assert_eq!(last_script_push(&[0x60]), Some(&[][..])); + assert!(last_script_push(&[0x61]).is_none()); + // PUSHDATA2 needs two length bytes. One byte, or a length past the script, is no redeem. + assert!(last_script_push(&[0x4d]).is_none()); + assert!(last_script_push(&[0x4d, 0x01]).is_none()); + assert!(last_script_push(&[0x4d, 0x02, 0x00, 0x11]).is_none()); + // PUSHDATA4 needs four length bytes. Three is no redeem. + assert!(last_script_push(&[0x4e, 0x00, 0x00, 0x00]).is_none()); +} + #[test] fn p3_default_milestone_heights() { use crate::params::default_milestone_height; @@ -2814,6 +2827,116 @@ fn max_money_fits_in_i64_so_the_assemble_cast_is_in_range() { assert!(i64::try_from(u64::MAX).is_err()); } +#[test] +fn assemble_without_pres_sums_zero_and_rejects_over_max() { + fn one(sats: u64) -> Transaction { + Transaction { + version: TxVersion::ONE, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint::null(), + script_sig: ScriptBuf::new(), + sequence: Sequence::MAX, + witness: Witness::new(), + }], + output: vec![TxOut { + value: Amount::from_sat(sats), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }], + } + } + assert_eq!(super::assemble_tx_value_out(&one(0), 0, None).unwrap(), 0); + let max_money = Amount::MAX_MONEY.to_sat(); + assert_eq!( + super::assemble_tx_value_out(&one(max_money), 0, None).unwrap(), + max_money as i64 + ); + assert!(super::assemble_tx_value_out(&one(max_money + 1), 0, None).is_err()); +} + +#[test] +fn witness_commitment_leaves_a_legacy_block_unchanged() { + let mut b = block_with(vec![coinbase(1)]); + let outs = b.txdata[0].output.len(); + apply_witness_commitment(&mut b); + assert_eq!(b.txdata[0].output.len(), outs); + assert!(b.txdata[0].input[0].witness.is_empty()); +} + +#[test] +fn resolve_prevout_rejects_a_same_index_parent() { + use super::{resolve_prevout, AsmPrevoutAcc, TxidMap}; + let parent = coinbase(1); + let txid = parent.compute_txid().to_byte_array(); + let spend = Transaction { + version: TxVersion::ONE, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint { + txid: parent.compute_txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::MAX, + witness: Witness::new(), + }], + output: vec![TxOut { + value: Amount::from_sat(1), + script_pubkey: ScriptBuf::from_bytes(vec![0x51]), + }], + }; + let block = block_with(vec![parent, spend.clone()]); + let mut index = TxidMap::default(); + index.insert(txid, 1usize); + match resolve_prevout( + &block, + spend.input[0].previous_output, + &spend.input[0], + None, + &index, + 1, + &rbitcoin_query::BatchParents::new(), + true, + true, + false, + None, + &mut AsmPrevoutAcc::default(), + ) { + Err(err) => assert!(format!("{err}").contains("lookup stage miss"), "{err}"), + Ok(_) => panic!("a tx cannot spend an output at its own index"), + } +} + +#[test] +fn same_height_confirmed_spender_is_not_below_its_create() { + use super::structural_apply_one_meta; + use rbitcoin_primitives::Fk; + let (_dir, query) = rbitcoin_query::testutil::tiny_query_labeled("spend-same-h"); + let store = query.store(); + store + .confirmed + .set(rbitcoin_primitives::Height(0), Fk(1)) + .unwrap(); + store.header_txs.put_range(Fk(1), Fk(1), 1).unwrap(); + store.strong_tx.set_strong(Fk(1), Fk(1)).unwrap(); + store.rebuild_height_fence().unwrap(); + let mut scratch = super::StructuralScratch::default(); + scratch.height_by_id.insert(9, 0); + scratch.field_h_by_id.insert(1, 0); + structural_apply_one_meta( + &query, + Some((Fk(1), 0, 0)), + 9, + 0, + 0, + Fk::NULL, + 0, + Some(0), + &mut scratch, + ) + .expect("spender and create at the same height are not corrupt"); +} + #[test] fn structure_rejects_pres_out_sum_above_max_money_before_assemble_casts_it() { let max_money = Amount::MAX_MONEY.to_sat(); diff --git a/crates/rbitcoin-consensus/src/confirm_run/write_idempotent_tests.rs b/crates/rbitcoin-consensus/src/confirm_run/write_idempotent_tests.rs index 423e0623d..4c6a697d2 100644 --- a/crates/rbitcoin-consensus/src/confirm_run/write_idempotent_tests.rs +++ b/crates/rbitcoin-consensus/src/confirm_run/write_idempotent_tests.rs @@ -183,6 +183,45 @@ fn three_stage_write_filter_and_scripts_surface() { assert!(batch.is_empty()); assert_eq!(batch.approx_wire_bytes(), 0); assert_eq!(batch.parent_count(), 0); + let genesis = bitcoin::blockdata::constants::genesis_block(bitcoin::Network::Regtest); + let mut metered = LoadedBatch { + prepared: Vec::new(), + wire_blocks: vec![std::sync::Arc::new(genesis)], + batch_parents: rbitcoin_query::BatchParents::new(), + script_preverified: ScriptPreverified::new(), + archive_plan: None, + index_want: super::index::IndexWant::default(), + stats: std::sync::Arc::new(rbitcoin_query::ConfirmStats::default()), + }; + assert!(metered.approx_wire_bytes() > 0); + let mut txid = [0u8; 32]; + txid[0] = 1; + metered.batch_parents.insert_owned( + rbitcoin_primitives::Fk(1), + rbitcoin_store::TxRecord { + txid, + version: 1, + locktime: 0, + input_start_fk: rbitcoin_primitives::Fk::NULL, + input_count: 0, + output_start_fk: rbitcoin_primitives::Fk::NULL, + output_count: 1, + }, + vec![(0, rbitcoin_store::OutputRecord::unspent(1, vec![0x51]))], + vec![0], + Some(false), + None, + vec![], + ); + assert_eq!(metered.parent_count(), 1); + let script_ok = super::ScriptOkBatch { + prepared: Vec::new(), + wire_blocks: Vec::new(), + batch_parents: metered.batch_parents, + archive_plan: None, + index_seal: super::index::IndexSeal::default(), + }; + assert_eq!(script_ok.parent_count(), 1); let cur = std::thread::current(); let name = cur.name().unwrap_or("").to_string(); assert!( @@ -3189,6 +3228,14 @@ fn one_shot_load_matches_stamp_then_load_from_plan() { from_plan.batch.parent_count() ); assert_eq!(one_shot.batch.len(), 2); + assert!(one_shot.batch.approx_wire_bytes() > 0); + assert!( + qa.confirm_stats() + .phase_prep_wire_arc_ns + .load(std::sync::atomic::Ordering::Relaxed) + > 0, + "cloning the wire batch records prep time" + ); for (a, b) in one_shot .batch .prepared diff --git a/crates/rbitcoin-consensus/src/convert.rs b/crates/rbitcoin-consensus/src/convert.rs index ab2f7a72c..4fcce1413 100644 --- a/crates/rbitcoin-consensus/src/convert.rs +++ b/crates/rbitcoin-consensus/src/convert.rs @@ -82,9 +82,7 @@ fn tx_to_apply(tx: &Transaction, txid: [u8; 32]) -> Result 0, "the spend must have a non-zero fee rate"); + let announce = |txid| crate::tx_relay::MempoolAnnounce { + txid, + replaced: vec![], + replaced_scripthashes: vec![], + scripthashes: vec![], + }; + let follow = PeerFollowState::new(); + let stand_equal = relay_peer(peers, [127, 0, 0, 1], OutboundFullRelay); + let _stand_equal_rx = writer_of(&stand_equal); + stand_equal.note_minfeefilter_sat_kvb(rate); + let stand_above = relay_peer(peers, [127, 0, 0, 1], OutboundFullRelay); + let _stand_above_rx = writer_of(&stand_above); + stand_above.note_minfeefilter_sat_kvb(rate.saturating_add(1)); + let (eq_tx, mut eq_ann) = mpsc::unbounded_channel(); + let (hi_tx, mut hi_ann) = mpsc::unbounded_channel(); + on_tx_announce_ok( + hub, + &eq_tx, + &follow, + Some(&stand_equal), + announce(relayed.compute_txid()), + ) + .unwrap(); + on_tx_announce_ok( + hub, + &hi_tx, + &follow, + Some(&stand_above), + announce(relayed.compute_txid()), + ) + .unwrap(); + assert_eq!(tx_invs(&mut eq_ann), vec![relayed.compute_wtxid()]); + assert!( + hi_ann.try_recv().is_err(), + "a fee above the filter is not announced" + ); + + let due_equal = relay_peer(peers, [127, 0, 0, 1], OutboundFullRelay); + let _due_equal_rx = writer_of(&due_equal); + due_equal.note_minfeefilter_sat_kvb(rate); + due_equal.request_tx_inv(); + let due_above = relay_peer(peers, [127, 0, 0, 1], OutboundFullRelay); + let _due_above_rx = writer_of(&due_above); + due_above.note_minfeefilter_sat_kvb(rate.saturating_add(1)); + due_above.request_tx_inv(); + let (due_eq_tx, mut due_eq_rx) = mpsc::unbounded_channel(); + let (due_hi_tx, mut due_hi_rx) = mpsc::unbounded_channel(); + queue_due_tx_invs(hub, due_equal.as_ref(), &CappedSet::new(), &due_eq_tx); + queue_due_tx_invs(hub, due_above.as_ref(), &CappedSet::new(), &due_hi_tx); + assert_eq!(tx_invs(&mut due_eq_rx), vec![relayed.compute_wtxid()]); + assert!( + due_hi_rx.try_recv().is_err(), + "periodic inv skips a fee above the filter" + ); + + let force_equal = relay_peer(peers, [127, 0, 0, 1], OutboundFullRelay); + let mut force_equal_rx = writer_of(&force_equal); + force_equal.note_minfeefilter_sat_kvb(rate); + let force_above = relay_peer(peers, [127, 0, 0, 1], OutboundFullRelay); + let mut force_above_rx = writer_of(&force_above); + force_above.note_minfeefilter_sat_kvb(rate.saturating_add(1)); + crate::force_announce_txid(hub, peers, relayed.compute_txid()); + assert_eq!(tx_invs(&mut force_equal_rx), vec![relayed.compute_wtxid()]); + assert!( + force_above_rx.try_recv().is_err(), + "force relay skips a fee above the filter" + ); + mp.set_isolated_broadcast(true); mp.accept_tx(&local).expect("local"); mp.mark_local_origin(local.compute_txid()); @@ -336,7 +409,19 @@ fn force_announce_picks_peers( !mp.is_local_origin(&local.compute_txid()), "confirm must drop the isolated skip" ); - for s in [block_relay, no_writer, seen, pricey, ok] { + for s in [ + block_relay, + no_writer, + seen, + pricey, + ok, + stand_equal, + stand_above, + due_equal, + due_above, + force_equal, + force_above, + ] { peers.unregister(s.id); } } diff --git a/crates/rbitcoin-net/src/peer_tests.rs b/crates/rbitcoin-net/src/peer_tests.rs index e807e53cb..e6866b2ea 100644 --- a/crates/rbitcoin-net/src/peer_tests.rs +++ b/crates/rbitcoin-net/src/peer_tests.rs @@ -857,8 +857,32 @@ fn handle_peer_frame_control_and_inv_paths() { NetworkMessage::BlockTxn(_) )); - // Deeper than 10: full block, not blocktxn (`p2p_compactblocks` :635). - hub.generate_to_script(12, ScriptBuf::from_bytes(vec![0x51]), vec![]) + // Depth 10 still answers with the transactions. One past that is a full block + // (`p2p_compactblocks` :635). + hub.generate_to_script(10, ScriptBuf::from_bytes(vec![0x51]), vec![]) + .unwrap(); + handle_peer_frame( + frame_for(NetworkMessage::GetBlockTxn(GetBlockTxn { + txs_request: BlockTransactionsRequest { + block_hash: tip, + indexes: vec![0], + }, + })), + &hub, + &out_tx, + &mut follow, + None, + ) + .await + .unwrap(); + assert!( + matches!( + out_rx.try_recv().unwrap().expect_msg(), + NetworkMessage::BlockTxn(_) + ), + "getblocktxn at depth 10 still serves the transactions" + ); + hub.generate_to_script(1, ScriptBuf::from_bytes(vec![0x51]), vec![]) .unwrap(); handle_peer_frame( frame_for(NetworkMessage::GetBlockTxn(GetBlockTxn { @@ -3664,6 +3688,39 @@ async fn over_budget_reader_waits_until_one_byte_is_written() { .unwrap(); } +/// `fPauseSend`: an over-budget session does not read. A ping sent while only +/// the send budget is blown gets no pong until the writer drains. +#[tokio::test] +async fn over_budget_session_does_not_read_a_ping() { + use std::time::Duration; + let mut t = paused_serve("serve-pause-read", 1, &["/rbitcoin:test(budget-read)/"]).await; + let (raw, sess) = &mut t.clients[0]; + sess.note_send_queued(2 * crate::peers::PEER_SEND_BUDGET); + tokio::time::sleep(Duration::from_millis(150)).await; + write_peer_msg(raw, NetworkMessage::Ping(7)).await; + let early = tokio::time::timeout(Duration::from_millis(200), async { + loop { + if let NetworkMessage::Pong(7) = next_peer_msg(raw).await.unwrap() { + return; + } + } + }) + .await; + assert!(early.is_err(), "over-budget session must not read the ping"); + sess.note_send_written(2 * crate::peers::PEER_SEND_BUDGET); + tokio::time::timeout(Duration::from_secs(2), async { + loop { + if let NetworkMessage::Pong(7) = next_peer_msg(raw).await.unwrap() { + return; + } + } + }) + .await + .expect("draining the send budget lets the session read the ping"); + t.node.shutdown().await; + let _ = std::fs::remove_dir_all(&t.dir); +} + fn inbound_peer( peers: &std::sync::Arc, ) -> std::sync::Arc { @@ -3978,6 +4035,58 @@ async fn notfound_makes_the_waiting_wtxid_peer_due() { let _ = std::fs::remove_dir_all(dir); } +/// `notfound` for a txid inventory (with or without witness) clears that request. +#[tokio::test] +async fn notfound_for_txid_inventory_wakes_the_waiter() { + use bitcoin::hashes::Hash; + + let (dir, hub) = crate::chain::tiny_regtest_hub_labeled("txid-notfound"); + hub.ensure_genesis().unwrap(); + let t = hub.tip_header().unwrap().time; + hub.clock.set_mock(i64::from(t) + 1); + let mp = crate::tx_relay::MempoolHub::open(dir.join("mp"), std::sync::Arc::clone(&hub.query)) + .unwrap(); + mp.set_relay_enabled(true); + assert!(hub.attach_mempool(mp).is_ok()); + let peers = crate::peers::PeerHub::new(); + for (n, witness) in [(0xab_u8, false), (0xcd, true)] { + let peer1 = inbound_peer(&peers); + let peer2 = inbound_peer(&peers); + let txid = bitcoin::Txid::from_byte_array([n; 32]); + let item = if witness { + Inventory::WitnessTransaction(txid) + } else { + Inventory::Transaction(txid) + }; + let noted = peer1.clock_now(); + let (tx1, mut rx1) = mpsc::unbounded_channel(); + let (tx2, _rx2) = mpsc::unbounded_channel(); + let mut follow = PeerFollowState::new(); + on_inv(&hub, &tx1, &mut follow, Some(&peer1), &[item]).unwrap(); + on_inv(&hub, &tx2, &mut follow, Some(&peer2), &[item]).unwrap(); + assert!(matches!( + rx1.try_recv().unwrap().expect_msg(), + NetworkMessage::GetData(_) + )); + handle_peer_inventory_msg( + &NetworkMessage::NotFound(vec![item]), + &hub, + &tx1, + &mut follow, + Some(&peer1), + ) + .unwrap(); + let due = hub + .mempool() + .unwrap() + .take_due_parent_getdata(peer2.id, noted); + assert_eq!(due.len(), 1, "txid notfound makes the waiter due"); + assert!(!due[0].wtxid); + assert_eq!(due[0].hash, txid.to_byte_array()); + } + let _ = std::fs::remove_dir_all(dir); +} + /// Two inbound peers announce the same unknown wtxid. Only the first is /// asked. The second stays queued until that request's interval ends. #[tokio::test] From 9ea1e676655cd5cd93cd75f109fe2a583a9913c8 Mon Sep 17 00:00:00 2001 From: "rearden-grok[bot]" <317016512+rearden-grok[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:21:08 -0700 Subject: [PATCH 2/2] test: match the no-pres money reject to the structure gate An over-max output with no precompute row now fails as bad-txns-vout-toolarge, the same block error as the structure gate. The BIP34 height encoder no longer keeps a dead empty-body branch. Co-authored-by: Cursor --- changelog.d/missed-mutant-pins.md | 2 +- crates/rbitcoin-consensus/src/block/mod.rs | 41 +++++++++---------- .../src/block/structure_rule_tests.rs | 5 ++- crates/rbitcoin-consensus/src/convert.rs | 21 ---------- .../src/peer_blocksonly_journey.rs | 6 +-- 5 files changed, 27 insertions(+), 48 deletions(-) diff --git a/changelog.d/missed-mutant-pins.md b/changelog.d/missed-mutant-pins.md index c6a2a8716..f6e3ce064 100644 --- a/changelog.d/missed-mutant-pins.md +++ b/changelog.d/missed-mutant-pins.md @@ -1,3 +1,3 @@ Changed -- Nightly mutants: fee-filter equality, `getblocktxn` depth 10, txid `notfound`, and the send-budget read pause are pinned. BIP34 no longer carries a negative-height encoder. Archive coinbase detection is `OutPoint::is_null`. The no-precompute value sum uses the money-range check. +- Nightly mutants: fee-filter equality, `getblocktxn` depth 10, txid `notfound`, and the send-budget read pause are pinned. BIP34 no longer carries a negative-height encoder. Archive coinbase detection is `OutPoint::is_null`. The no-precompute value sum uses the money-range check and rejects an over-max output as `bad-txns-vout-toolarge`. diff --git a/crates/rbitcoin-consensus/src/block/mod.rs b/crates/rbitcoin-consensus/src/block/mod.rs index 4bac6293b..24546e568 100644 --- a/crates/rbitcoin-consensus/src/block/mod.rs +++ b/crates/rbitcoin-consensus/src/block/mod.rs @@ -238,7 +238,8 @@ pub fn validate_block_structure_with_pres( check_bip34_coinbase(&block.txdata[0], ctx.height.0)?; } - // Only money-range gate. `money_range_out_sum` casts a sum that passed here. + // Money-range gate on the precompute path. `assemble_tx_value_out` applies + // the same per-output check when a tx has no precompute row. for (tx, p) in block.txdata.iter().zip(pres.iter()) { for o in &tx.output { if exceeds_max_money(o.value.to_sat()) { @@ -608,25 +609,21 @@ pub(crate) fn check_bip34_coinbase( /// Serialize `height` the same way Core pushes it into the coinbase scriptSig. #[must_use] pub fn bip34_height_script(height: u32) -> Vec { - let n = height as i64; - if n == 0 { + if height == 0 { return vec![0x00]; } - if (1..=16).contains(&n) { - return vec![0x50 + n as u8]; + if (1..=16).contains(&height) { + return vec![0x50 + height as u8]; } + // Height is at least 17, so the little-endian body is never empty. let mut num = Vec::new(); - let mut abs = n; - while abs > 0 { - num.push((abs & 0xff) as u8); - abs >>= 8; + let mut n = height; + while n > 0 { + num.push((n & 0xff) as u8); + n >>= 8; } - if let Some(last) = num.last() { - if last & 0x80 != 0 { - num.push(0x00); - } - } else { - num.push(0); + if num.last().unwrap() & 0x80 != 0 { + num.push(0x00); } let mut out = Vec::with_capacity(1 + num.len()); out.push(num.len() as u8); @@ -1331,13 +1328,13 @@ fn should_use_pres(ti: usize, len: usize) -> bool { ti < len } -/// Core `MAX_MONEY` (21_000_000 BTC). This is the money-range predicate -/// `validate_block_structure_with_pres` uses for each output and for -/// `TxPrecompute::out_sum`. +/// Core `MAX_MONEY` (21_000_000 BTC). Structure validation uses this for +/// each output and for `TxPrecompute::out_sum`. The no-precompute value sum +/// uses it for each output too. /// -/// `Amount::MAX_MONEY` fits in `i64`, so a sum this returns false for cannot -/// become negative when `money_range_out_sum` casts it. That cast is not a -/// second range check. +/// `Amount::MAX_MONEY` fits in `i64`, so a value this returns false for +/// cannot become negative when `money_range_out_sum` or +/// `assemble_tx_value_out` casts it. #[inline] fn exceeds_max_money(sats: u64) -> bool { sats > Amount::MAX_MONEY.to_sat() @@ -1466,7 +1463,7 @@ fn assemble_tx_value_out( for o in &tx.output { let sats_u = o.value.to_sat(); if exceeds_max_money(sats_u) { - return Err(ConsensusError::BadTx("bad-txns-vout-toolarge")); + return Err(ConsensusError::BadBlock("bad-txns-vout-toolarge")); } value_out = value_out .checked_add(sats_u as i64) diff --git a/crates/rbitcoin-consensus/src/block/structure_rule_tests.rs b/crates/rbitcoin-consensus/src/block/structure_rule_tests.rs index 7a402f539..e8ea28a77 100644 --- a/crates/rbitcoin-consensus/src/block/structure_rule_tests.rs +++ b/crates/rbitcoin-consensus/src/block/structure_rule_tests.rs @@ -2851,7 +2851,10 @@ fn assemble_without_pres_sums_zero_and_rejects_over_max() { super::assemble_tx_value_out(&one(max_money), 0, None).unwrap(), max_money as i64 ); - assert!(super::assemble_tx_value_out(&one(max_money + 1), 0, None).is_err()); + assert!(matches!( + super::assemble_tx_value_out(&one(max_money + 1), 0, None), + Err(ConsensusError::BadBlock("bad-txns-vout-toolarge")) + )); } #[test] diff --git a/crates/rbitcoin-consensus/src/convert.rs b/crates/rbitcoin-consensus/src/convert.rs index 4fcce1413..bd9eee072 100644 --- a/crates/rbitcoin-consensus/src/convert.rs +++ b/crates/rbitcoin-consensus/src/convert.rs @@ -245,27 +245,6 @@ mod tests { }; let apply = tx_to_apply(&non_cb, non_cb.compute_txid().to_byte_array()).unwrap(); assert_eq!(apply.inputs[0].prev_index, 3); - // A zero txid with a real vout is not a coinbase. `OutPoint::is_null` - // is the whole marker (txid 0 and vout MAX). - let zero_txid = Transaction { - version: TxVersion::TWO, - lock_time: LockTime::ZERO, - input: vec![TxIn { - previous_output: OutPoint { - txid: bitcoin::Txid::from_byte_array([0; 32]), - vout: 0, - }, - script_sig: ScriptBuf::new(), - sequence: Sequence::MAX, - witness: Witness::new(), - }], - output: vec![TxOut { - value: Amount::from_sat(1), - script_pubkey: ScriptBuf::from_bytes(vec![0x51]), - }], - }; - let apply = tx_to_apply(&zero_txid, zero_txid.compute_txid().to_byte_array()).unwrap(); - assert_eq!(apply.inputs[0].prev_index, 0); } /// Write encode must bind each stamped spend edge to the wire prevout it diff --git a/crates/rbitcoin-net/src/peer_blocksonly_journey.rs b/crates/rbitcoin-net/src/peer_blocksonly_journey.rs index d4dfef43e..e4a27a89e 100644 --- a/crates/rbitcoin-net/src/peer_blocksonly_journey.rs +++ b/crates/rbitcoin-net/src/peer_blocksonly_journey.rs @@ -355,7 +355,7 @@ fn force_announce_picks_peers( assert_eq!(tx_invs(&mut eq_ann), vec![relayed.compute_wtxid()]); assert!( hi_ann.try_recv().is_err(), - "a fee above the filter is not announced" + "a filter above the fee is not announced" ); let due_equal = relay_peer(peers, [127, 0, 0, 1], OutboundFullRelay); @@ -373,7 +373,7 @@ fn force_announce_picks_peers( assert_eq!(tx_invs(&mut due_eq_rx), vec![relayed.compute_wtxid()]); assert!( due_hi_rx.try_recv().is_err(), - "periodic inv skips a fee above the filter" + "periodic inv skips a filter above the fee" ); let force_equal = relay_peer(peers, [127, 0, 0, 1], OutboundFullRelay); @@ -386,7 +386,7 @@ fn force_announce_picks_peers( assert_eq!(tx_invs(&mut force_equal_rx), vec![relayed.compute_wtxid()]); assert!( force_above_rx.try_recv().is_err(), - "force relay skips a fee above the filter" + "force relay skips a filter above the fee" ); mp.set_isolated_broadcast(true);