Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
9 changes: 9 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ jobs:
workspaces: ". -> target/dev"
- name: cargo clippy
run: cargo clippy --workspace --all-targets -- -D warnings
# Both shapes TESTING.md names: workspace, and the product set that
# nix/rbitcoin.nix and release.yml build (consensus without `corpus`).
- name: cargo check --release
run: |
cargo check --release --workspace
cargo check --release -p rbitcoin-node -p rbitcoin-cli -p rbitcoin-store
# doc(hidden) pub modules are rustdoc-visible; a broken link is an error.
- name: cargo doc
run: cargo doc --workspace --no-deps
- uses: cachix/install-nix-action@448cde739abec1eaf0646249e3d25bb8d9bc7702 # v31.11.2
with:
github_access_token: ${{ github.token }}
Expand Down
53 changes: 33 additions & 20 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,23 +230,33 @@ engineering.
[`docs/code-shape.md`](./docs/code-shape.md). Named 0.6.0 extracts
(**Q-61**) are Completed; residual peels:
[`docs/quality.md`](./docs/quality.md) **R-10**.
11. **Crate visibility is the production graph.** `pub` and crate-root
`pub use` exist only for names another crate actually imports, or for
a **documented** public library API (we do not have one). Unused `pub`
is forbidden: drop to `pub(crate)` or delete; workspace `-D warnings`
then reports truly dead items — delete those too (`#[allow(dead_code)]`
is not a fix). If we ever ship an out-of-tree library surface, write
the allowlist in the owner crate rustdoc and then unused `pub` on
**that** surface is intentional. `#[cfg(test)]` on production items
is a smell: tests drive the shipped function (principle 8), not a
second test-only wrapper or `*_for_test` backdoor. IO-shape pins use
session/table instance stats or on-disk file state, not thread-local
`test_take_*` probes on the hot path. A tiny `#[cfg(test)]` take on
those instance counters is fine. Fuzz-only `pub` is the same
smell — prefer the in-crate `pub(crate)` graph or the published
**binary** surface (`rbitcoin-node` / CLI) even if the harness pays
a little extra setup, rather than exporting helpers solely for
`fuzz/`.
11. **Crate visibility is the production graph; the suite reads it through
`#[doc(hidden)]`.** Library tests live in `tests/suite` and link the
normal rlib, so an item a test reaches is `pub` with `#[doc(hidden)]`.
Plain `pub` (no hidden) and crate-root `pub use` exist only for names
another crate actually imports, or for a **documented** public library
API (we do not have one). If we ever ship an out-of-tree library
surface, write the allowlist in the owner crate rustdoc and then unused
`pub` on **that** surface is intentional. rustc does not report a dead
hidden item, so a change that removes the last caller removes the
callee in the same change (code-shape rule 9); `#[allow(dead_code)]`
is not a fix, in `src/` or in a suite root. Test hooks in production
files are `#[cfg(debug_assertions)]`, and every debug node runs that
code, so a hook is either a bounded counter (`Cell` / atomic increment)
or **opt-in**: off until the test arms it on its thread. Nothing
accumulates unarmed (`spindle_mark` is the pattern). A
`cfg(debug_assertions)` gate never covers an import or re-export that
production code uses, and it never changes a production constant or
code path (the debug-only `REDIAL_INTERVAL`, `SAM_RETRY`, and
`CONSTRAINTS_COOLDOWN` values are the known exceptions to retire); how
CI compiles the release profile is in
[`TESTING.md`](TESTING.md#coverage). Tests drive the shipped function
(principle 8), not a second test-only wrapper or `*_for_test` backdoor.
IO-shape pins prefer session/table instance stats or on-disk file state
over a thread-local probe. Fuzz-only `pub` is the same smell — prefer the
in-crate graph or the published **binary** surface (`rbitcoin-node` /
CLI) even if the harness pays a little extra setup, rather than
exporting helpers solely for `fuzz/`.

## Workflow

Expand Down Expand Up @@ -311,9 +321,12 @@ IO; they do not package zips. GitHub Releases:
([`TESTING.md`](./TESTING.md#true-journeys)).
- [ ] Core-facing RPC / P2P / Electrum / Esplora: [`COMPAT.md`](./COMPAT.md)
- [ ] No new silent dead branches
- [ ] No unused crate-root `pub` / `pub use` (principle 11). Tests drive
shipped functions, not `#[cfg(test)]` wrappers or `*_for_test`
backdoors. Fuzz does not grow `pub` without considering the binary
- [ ] No unused crate-root `pub` / `pub use`; suite-only items are
`#[doc(hidden)]`; a removed caller removes its callee (principle 11).
Tests drive shipped functions, not `*_for_test` backdoors. A
`cfg(debug_assertions)` hook is a bounded counter or opt-in, and
gates no production import, constant, or code path. Fuzz does not
grow `pub` without considering the binary
surface.
- [ ] Store changes respect Class A/B/C and allocate-then-publish
- [ ] Experimental / milestone honesty preserved in user-facing docs when relevant
Expand Down
22 changes: 15 additions & 7 deletions TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,15 @@ a production line already counts that line, so a second in-crate `#[test]`
is not required to make LCOV see it. Library tests live in each crate's
`tests/suite` and link the normal rlib, so a `#[cfg(test)]` arm in `src/`
is no longer how those tests compile. `#[cfg(debug_assertions)]` hooks in
production files still count. llvm-cov hit counts jitter tens of lines on the same
tree; the floor is the gate, not a never-falls ratchet vs master.
production files still count. Those hooks also mean the suites link only in
the `dev` / `test` profiles: `cargo test --release` does not build. The
release profile is compiled on every PR by `cargo check --release` in `qc`
(workspace shape and the `-p rbitcoin-node -p rbitcoin-cli -p rbitcoin-store`
product shape that `nix/rbitcoin.nix` and `release.yml` build) and built for
real only at tag time. The labeled harnesses build `rbitcoin-node` in the
`dev` profile, so no CI job executes a release binary. llvm-cov
hit counts jitter tens of lines on the same tree; the floor is the gate,
not a never-falls ratchet vs master.

The README badge and rbitcoin.org figure are the last **green `master`**
`coverage` job (`badges` branch `coverage.json`, Shields endpoint). The
Expand Down Expand Up @@ -275,11 +282,12 @@ counts that line.
2. Prefer extending that journey over a new small test. A caller of other
tests is not a journey.
3. If a branch is unreachable from any real session, **delete it** or hit it
through a **shipped** config / error / CLI path. Do not add a `pub` or
`*_for_test` injector so a unit can see it
through a **shipped** config / error / CLI path. Do not add a
`#[doc(hidden)] pub` or `*_for_test` injector so a unit can see it
([`CONTRIBUTING.md`](./CONTRIBUTING.md) principle 11).
4. A small unit only for the narrow case in [True journeys](#true-journeys).
Drive the shipped function, not a `#[cfg(test)]` wrapper around it.
Drive the shipped function, not a `#[cfg(debug_assertions)]` wrapper
around it.

### Closing a red region

Expand Down Expand Up @@ -449,8 +457,8 @@ overrides that choice; unset keeps it. A Sunday (or `workflow_dispatch`)
job runs `asmap` and `block_wire` for 120s with `--sanitizer undefined`.
Miri stays `rbitcoin-primitives` only.
Treat crate-root `pub` that exists only so a fuzz target can call it as
the same smell as a test-only export: prefer `pub(crate)` plus an in-crate
harness, or the published `rbitcoin-node` / CLI binary, even if that costs
the same smell as a test-only export: prefer the in-crate graph, or the
published `rbitcoin-node` / CLI binary, even if that costs
a little setup time, rather than growing helpers solely for fuzz. Allowlist
what must stay `pub` for a target; do not treat `fuzz/` as a second public
API.
Expand Down
11 changes: 11 additions & 0 deletions changelog.d/release-build-after-layout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Fixed

- **Release builds compile again.** The crate-layout move gated a `rbitcoin-store` import and two `rbitcoin-query` re-exports behind `cfg(debug_assertions)` while production code used them, so `cargo build --release`, `release.yml`, and the nix package failed. The suites themselves build in the dev and test profiles only.
- **A debug node no longer grows the confirm spindle trace.** `spindle_mark` records only after a test arms it on that thread. Before, every debug build appended to a thread-local on each confirm batch for the life of the process.
- **The coverage badge gate is the 91% floor.** The badge still said 89 after the floor moved, so a run between 89 and 91 would have drawn a passing badge on a failing gate. `coverage-gate.py` is now the only place the number lives; the badge reads it from `gate.json`.
- **`cargo doc --workspace` builds.** The layout move made private modules `#[doc(hidden)] pub`, so rustdoc began resolving 37 links in them that had never been checked.

Changed

- **`qc` compiles the release profile and the workspace rustdoc.** `cargo check --release` runs in the workspace shape and in the product shape nix and `release.yml` build; `cargo doc --workspace --no-deps` runs after it.
- **Suite roots are linted like `src/`.** The `#![allow(unused_imports, dead_code, unused_variables)]` on every `tests/suite.rs` is gone; the imports the move left behind and one dead env-lock helper went with it.
6 changes: 1 addition & 5 deletions crates/rbitcoin-cli/tests/root.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
use rbitcoin_cli::*;
use rbitcoin_primitives::Network;
use serde_json::Value;
use std::ffi::OsString;
use std::io::{Read, Write};
use std::net::TcpStream;
use std::path::{Path, PathBuf};
use std::path::PathBuf;
use std::process::ExitCode;
use std::time::Duration;

use std::net::TcpListener;

Expand Down
1 change: 0 additions & 1 deletion crates/rbitcoin-cli/tests/suite.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
#![allow(unused_imports, dead_code, unused_variables)]
mod root;
2 changes: 1 addition & 1 deletion crates/rbitcoin-consensus/src/block/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ impl AsRef<Transaction> for JobTx {
///
/// Field reads in the interpreter stay a direct bool test (no extra per-opcode
/// branch). Production confirm uses [`Self::consensus_at`]; Core JSON fixtures
/// use [`Self::ALL`] or an explicit struct literal.
/// use an explicit struct literal.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct ScriptVerifyFlags {
pub bip65_active: bool,
Expand Down
2 changes: 1 addition & 1 deletion crates/rbitcoin-consensus/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub fn pow_hash_meets_target(
/// BIP34/66/65 `nVersion` floors and the 2-hour future-time cap (wall clock).
///
/// Core `ContextualCheckBlockHeader`. Assemble uses this instead of a second
/// [`validate_header`] (MTP walk + header rehash).
/// `validate_header` (MTP walk + header rehash).
pub fn check_header_version_and_future_time(
params: &ChainParams,
height: Height,
Expand Down
6 changes: 3 additions & 3 deletions crates/rbitcoin-consensus/src/script/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ pub mod crypto {

/// Parse DER signature + **raw** sighash type byte (as `u32`).
///
/// Important: do **not** run the type through [`EcdsaSighashType::from_consensus`]
/// Important: do **not** run the type through `EcdsaSighashType::from_consensus`
/// before legacy `SignatureHash`. That maps `0 → SIGHASH_ALL(1)`, but mainnet
/// has historical spends signed with hashtype **0** (block 110300 and others).
/// Core hashes with the raw byte; we must too. (**RB-002** in
Expand Down Expand Up @@ -522,8 +522,8 @@ pub mod crypto {

/// P2WPKH BIP143 hash: `script_pubkey` is native spk **or** nested redeem (`00 14 <20>`).
///
/// Fast path: when `raw_ty` round-trips through [`EcdsaSighashType`] (standard
/// 0x01/02/03/81/82/83), use rust-bitcoin's [`SighashCache`] midstate
/// Fast path: when `raw_ty` round-trips through `EcdsaSighashType` (standard
/// 0x01/02/03/81/82/83), use rust-bitcoin's `SighashCache` midstate
/// (hashPrevouts/hashSequence/hashOutputs once per tx). Slow path: non-standard
/// raw types (e.g. mainnet `0x65`) need our raw-uint32 encoder.
pub fn bip143_p2wpkh_signature_hash(
Expand Down
2 changes: 1 addition & 1 deletion crates/rbitcoin-consensus/src/script_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//! from the stage thread — steal workers must not `wait_done` on this pool.
//!
//! Idle steal workers [`thread::park`]. A new wave or detached job bumps an
//! epoch and [`Thread::unpark`]s every worker (unpark-before-park leaves a
//! epoch and [`thread::Thread::unpark`]s every worker (unpark-before-park leaves a
//! permit, so a worker cannot miss work by parking after the wake). The jobs
//! mutex is only the detached-job queue, never the steal wake path.
//!
Expand Down
28 changes: 0 additions & 28 deletions crates/rbitcoin-consensus/tests/block/bip34_tests.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,4 @@
//! bip34_tests (peeled from block.rs).
use bitcoin::block::Block;
use bitcoin::hashes::{sha256d, Hash};
use bitcoin::script::{Script, ScriptBuf};
use bitcoin::{Amount, Transaction, TxOut, Witness};
use rbitcoin_consensus::block::facts::{
add_fee, add_sigops, add_value_in, fee_from_values, fee_sat, header_gap_on_chain,
median_or_bad_header, reject_assemble_block_shape, reject_batch_starts_at, reject_bits,
reject_carried_header_hash, reject_confirm_batch, reject_csv_create_height,
reject_duplicate_input, reject_empty_confirm_batch, reject_genesis_coinbase_spend,
reject_non_final, reject_prev_link, reject_prevout_spent, reject_same_block_create,
reject_sequence_locks, reject_spend_cursor, reject_tx_without_inputs,
reject_tx_without_outputs, reject_txid_count, reject_unresolved_spend,
reject_unspendable_prevout, reject_vout_past_n_out, reject_witness_before_segwit,
require_linked_parent, require_next_work, require_retarget_first, require_same_block_output,
unresolved_parent_create,
};
use rbitcoin_consensus::error::ConsensusError;
use rbitcoin_consensus::milestone::Milestone;
use rbitcoin_consensus::params::ChainParams;
use rbitcoin_consensus::TxPrecompute;
use rbitcoin_consensus::TxidHasher;
use rbitcoin_primitives::script_sigop_count;
use rbitcoin_primitives::Height;
use std::borrow::Borrow;
use std::hash::BuildHasherDefault;
use std::ops::{Deref, DerefMut};
use std::sync::Arc;
use std::time::Instant;

use rbitcoin_consensus::block::bip34_height_script;

Expand Down
22 changes: 2 additions & 20 deletions crates/rbitcoin-consensus/tests/block/block_866342.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,13 @@
//! Floresta's MIT/Apache `spent_utxos.zst` (ordered non-coinbase inputs).
#![cfg(test)]
use bitcoin::block::Block;
use bitcoin::hashes::{sha256d, Hash};
use bitcoin::script::{Script, ScriptBuf};
use bitcoin::hashes::Hash;
use bitcoin::script::ScriptBuf;
use bitcoin::{Amount, Transaction, TxOut, Witness};
use rbitcoin_consensus::block::facts::{
add_fee, add_sigops, add_value_in, fee_from_values, fee_sat, header_gap_on_chain,
median_or_bad_header, reject_assemble_block_shape, reject_batch_starts_at, reject_bits,
reject_carried_header_hash, reject_confirm_batch, reject_csv_create_height,
reject_duplicate_input, reject_empty_confirm_batch, reject_genesis_coinbase_spend,
reject_non_final, reject_prev_link, reject_prevout_spent, reject_same_block_create,
reject_sequence_locks, reject_spend_cursor, reject_tx_without_inputs,
reject_tx_without_outputs, reject_txid_count, reject_unresolved_spend,
reject_unspendable_prevout, reject_vout_past_n_out, reject_witness_before_segwit,
require_linked_parent, require_next_work, require_retarget_first, require_same_block_output,
unresolved_parent_create,
};
use rbitcoin_consensus::error::ConsensusError;
use rbitcoin_consensus::milestone::Milestone;
use rbitcoin_consensus::params::ChainParams;
use rbitcoin_consensus::TxPrecompute;
use rbitcoin_consensus::TxidHasher;
use rbitcoin_primitives::script_sigop_count;
use rbitcoin_primitives::Height;
use std::borrow::Borrow;
use std::hash::BuildHasherDefault;
use std::ops::{Deref, DerefMut};
use std::sync::Arc;
use std::time::Instant;

Expand Down
27 changes: 0 additions & 27 deletions crates/rbitcoin-consensus/tests/block/fact_reject_tests.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,5 @@
use bitcoin::block::Block;
use bitcoin::hashes::{sha256d, Hash};
use bitcoin::script::{Script, ScriptBuf};
use bitcoin::{Amount, Transaction, TxOut, Witness};
use rbitcoin_consensus::block::facts::{
add_fee, add_sigops, add_value_in, fee_from_values, fee_sat, header_gap_on_chain,
median_or_bad_header, reject_assemble_block_shape, reject_batch_starts_at, reject_bits,
reject_carried_header_hash, reject_confirm_batch, reject_csv_create_height,
reject_duplicate_input, reject_empty_confirm_batch, reject_genesis_coinbase_spend,
reject_non_final, reject_prev_link, reject_prevout_spent, reject_same_block_create,
reject_sequence_locks, reject_spend_cursor, reject_tx_without_inputs,
reject_tx_without_outputs, reject_txid_count, reject_unresolved_spend,
reject_unspendable_prevout, reject_vout_past_n_out, reject_witness_before_segwit,
require_linked_parent, require_next_work, require_retarget_first, require_same_block_output,
unresolved_parent_create,
};
use rbitcoin_consensus::block::{reject_bip30_unspent_prior, reject_one_create};
use rbitcoin_consensus::error::ConsensusError;
use rbitcoin_consensus::milestone::Milestone;
use rbitcoin_consensus::params::ChainParams;
use rbitcoin_consensus::TxPrecompute;
use rbitcoin_consensus::TxidHasher;
use rbitcoin_primitives::script_sigop_count;
use rbitcoin_primitives::Height;
use std::borrow::Borrow;
use std::hash::BuildHasherDefault;
use std::ops::{Deref, DerefMut};
use std::sync::Arc;
use std::time::Instant;

#[test]
fn bip30_and_immature_coinbase_reject_from_facts() {
Expand Down
28 changes: 1 addition & 27 deletions crates/rbitcoin-consensus/tests/block/finality_tests.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,6 @@
//! finality_tests (peeled from block.rs).
use bitcoin::block::Block;
use bitcoin::hashes::{sha256d, Hash};
use bitcoin::script::{Script, ScriptBuf};
use bitcoin::script::ScriptBuf;
use bitcoin::{Amount, Transaction, TxOut, Witness};
use rbitcoin_consensus::block::facts::{
add_fee, add_sigops, add_value_in, fee_from_values, fee_sat, header_gap_on_chain,
median_or_bad_header, reject_assemble_block_shape, reject_batch_starts_at, reject_bits,
reject_carried_header_hash, reject_confirm_batch, reject_csv_create_height,
reject_duplicate_input, reject_empty_confirm_batch, reject_genesis_coinbase_spend,
reject_non_final, reject_prev_link, reject_prevout_spent, reject_same_block_create,
reject_sequence_locks, reject_spend_cursor, reject_tx_without_inputs,
reject_tx_without_outputs, reject_txid_count, reject_unresolved_spend,
reject_unspendable_prevout, reject_vout_past_n_out, reject_witness_before_segwit,
require_linked_parent, require_next_work, require_retarget_first, require_same_block_output,
unresolved_parent_create,
};
use rbitcoin_consensus::error::ConsensusError;
use rbitcoin_consensus::milestone::Milestone;
use rbitcoin_consensus::params::ChainParams;
use rbitcoin_consensus::TxPrecompute;
use rbitcoin_consensus::TxidHasher;
use rbitcoin_primitives::script_sigop_count;
use rbitcoin_primitives::Height;
use std::borrow::Borrow;
use std::hash::BuildHasherDefault;
use std::ops::{Deref, DerefMut};
use std::sync::Arc;
use std::time::Instant;

use bitcoin::absolute::LockTime;
use rbitcoin_consensus::block::{
Expand Down
28 changes: 0 additions & 28 deletions crates/rbitcoin-consensus/tests/block/mod.rs
Original file line number Diff line number Diff line change
@@ -1,31 +1,3 @@
use bitcoin::block::Block;
use bitcoin::hashes::{sha256d, Hash};
use bitcoin::script::{Script, ScriptBuf};
use bitcoin::{Amount, Transaction, TxOut, Witness};
use rbitcoin_consensus::block::facts::{
add_fee, add_sigops, add_value_in, fee_from_values, fee_sat, header_gap_on_chain,
median_or_bad_header, reject_assemble_block_shape, reject_batch_starts_at, reject_bits,
reject_carried_header_hash, reject_confirm_batch, reject_csv_create_height,
reject_duplicate_input, reject_empty_confirm_batch, reject_genesis_coinbase_spend,
reject_non_final, reject_prev_link, reject_prevout_spent, reject_same_block_create,
reject_sequence_locks, reject_spend_cursor, reject_tx_without_inputs,
reject_tx_without_outputs, reject_txid_count, reject_unresolved_spend,
reject_unspendable_prevout, reject_vout_past_n_out, reject_witness_before_segwit,
require_linked_parent, require_next_work, require_retarget_first, require_same_block_output,
unresolved_parent_create,
};
use rbitcoin_consensus::error::ConsensusError;
use rbitcoin_consensus::milestone::Milestone;
use rbitcoin_consensus::params::ChainParams;
use rbitcoin_consensus::TxPrecompute;
use rbitcoin_consensus::TxidHasher;
use rbitcoin_primitives::script_sigop_count;
use rbitcoin_primitives::Height;
use std::borrow::Borrow;
use std::hash::BuildHasherDefault;
use std::ops::{Deref, DerefMut};
use std::sync::Arc;
use std::time::Instant;
mod bip34_tests;
mod block_866342;
mod fact_reject_tests;
Expand Down
Loading
Loading