diff --git a/AGENTS.md b/AGENTS.md index 27e21541..b2e06427 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -197,7 +197,7 @@ No `-u` on push (that would retarget the branch remote away from `origin`). | **One PR per plan** | Push more commits to the same branch. | | **Poll until green** | Do not walk away and call the plan done. | | **Done** | Required checks green **and** the PR is up for review. Do not merge unless asked. | -| **No post-green PR-cite** | After required checks are green, do **not** push a docs-only follow-up whose only change is inserting this PR's number into CHANGELOG / algo-review / similar. That wastes a full CI run. Cite in the **PR body**. Owner docs can omit the GitHub number, or pick it up later in a docs change that was already needed. | +| **No post-green PR-cite** | After required checks are green, do **not** push a docs-only follow-up whose only change is inserting this PR's number into CHANGELOG / quality.md / similar. That wastes a full CI run. Cite in the **PR body**. Owner docs can omit the GitHub number, or pick it up later in a docs change that was already needed. | | **Do not** | Force-push `master`, merge a red PR, collapse `origin` to a single URL, skip polling because “tests passed locally,” or invent **empty commits** to poke Actions. | | **Workflow YAML** | App cannot push `.github/workflows/*`. Ask the operator to `git push`. | diff --git a/CHANGELOG.md b/CHANGELOG.md index 0778d8ae..1bdd20bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,10 +63,10 @@ before 1.0). empty; SpendEdges + CreatePin remain. Write encodes ins from `Arc` + those edges. CreatePin outs stay stamp-time for in-flight. -- **Algo-review open list:** [`docs/algo-review.md`](docs/algo-review.md) - is remaining work only. Landed items from that pass live here and in - owner docs. Close a finding by deleting its row; do not append a Closed - graveyard. +- **Retire `docs/algo-review.md`:** remaining worth-fixing items are + **Q-57–Q-60** in [`docs/quality.md`](docs/quality.md) (store publish/flush, + mempool persist/eviction, RPC/CLI honesty, P2P caps). Inventory, gotchas, + and micro-opts are not a second backlog. - **`load_thr stamp=` nest:** `ibd: perf` prints `stamp=Nms(pack=Xms head=Yms)`. `head=` is leftover TipOnly diff --git a/docs/README.md b/docs/README.md index 5334a4e0..ab4d29a9 100644 --- a/docs/README.md +++ b/docs/README.md @@ -15,7 +15,6 @@ update that file — do not paste a parallel spec. | On-disk | [`SCHEMA.md`](../SCHEMA.md) | Current bytes; history in [`SCHEMA_HISTORY.md`](../SCHEMA_HISTORY.md) | | Confirm / store implementer | [`invariants.md`](./invariants.md) + [`concurrency.md`](./concurrency.md) | Stage IO, leftover union, roles, tip commit | | Tests | [`TESTING.md`](../TESTING.md) | How to run, budgets, coverage | -| Algo / DS findings | [`algo-review.md`](./algo-review.md) | Open algorithm/DS findings; work High first | | Peer full nodes | [`peer-clients.md`](./peer-clients.md) | Hornet / satd comparison; later-consideration tests and ideas | Planning a multi-step change: [`how-we-plan.md`](./how-we-plan.md). @@ -47,7 +46,6 @@ quality backlog). | [`rust-bitcoin-limitations.md`](./rust-bitcoin-limitations.md) | Workarounds where rust-bitcoin is not Core-faithful. | | [`mempool-fee-estimation.md`](./mempool-fee-estimation.md) | Fee estimator notes. | | [`errata.md`](./errata.md) | Known one-off store/confirm quirks. | -| [`algo-review.md`](./algo-review.md) | Open algorithm/DS findings. Close by deleting the row in the same PR; do not copy into quality.md. | | [`peer-clients.md`](./peer-clients.md) | Hornet Node and satd: what to steal (tests/ideas) and what not to copy. Ranked items stay here; do not copy into quality.md. | | [`external_findings/`](./external_findings/) | Numbered audit reports + regression pointers. Do not flatten into CHANGELOG. | @@ -81,6 +79,6 @@ table. Do not paste a second Allowed/Forbidden IO copy. 1. Find the owner in the tables above. Edit that file. 2. If the fact has no owner, add a row here in the same commit as the new file. 3. Do not resurrect `docs/future-features/`, `docs/store-format.md`, - `docs/startup-states.md`, `docs/design-ibd-most-work-reorg.md`, or - `COVERAGE.md`. Their content lives in SCHEMA / invariants / architecture / - TESTING. + `docs/startup-states.md`, `docs/design-ibd-most-work-reorg.md`, + `docs/algo-review.md`, or `COVERAGE.md`. Their content lives in SCHEMA / + invariants / architecture / TESTING / quality.md. diff --git a/docs/algo-review.md b/docs/algo-review.md deleted file mode 100644 index f490b1a5..00000000 --- a/docs/algo-review.md +++ /dev/null @@ -1,361 +0,0 @@ -# Algorithms & data structures — open findings - -Crate-by-crate pass 2026-08-25 against production sources, judged against -[`concurrency.md`](./concurrency.md), [`invariants.md`](./invariants.md), -[`ibd-memory.md`](./ibd-memory.md), [`SCHEMA.md`](../SCHEMA.md), -[`COMPAT.md`](../COMPAT.md). This file is the **open** list. Close an item -in the same PR as the fix by **deleting** its row here. Do not grow a Closed -graveyard; landed behavior lives in owner docs / [`CHANGELOG.md`](../CHANGELOG.md). -Do not copy these tables into [`quality.md`](./quality.md). - -Deliberate design (lock-free roles, RAM-only body queue, purpose-built -io_uring machines, Frozen→ArcSwap pins, leftover maps as `txid → one fk`) -is not a finding. Items already in `quality.md`, `errata.md`, or external -findings 001–023 are excluded unless the code still diverges. - -Severity: **High** = wrong result / consensus split / production DoS; -**Medium** = wrong under a plausible schedule, or measurably slow at mainnet -scale; **Low** = edge, fragile, or minor waste. - ---- - -## 1. Algorithm / data-structure inventory - -What the node runs. Findings follow. - -### Store (`rbitcoin-store`) - -| Piece | Algorithm | Notes | -|-------|-----------|--------| -| `TableFile` | fallocate + published HWM (Release) | body → idx → count/HWM; readers use HWM only | -| `VarTable` / `ArrayTable` | append / dense slots + L2 write-behind | Class A/C. Seqlock on `(count, body_end)` | -| `HashHead` | 24-byte OA, 16-byte prefix key, linear probe | load 7/8; insert full → sibling gen (`header.head.gN`) or Corrupt. Open-only rewrite of undersized single gen. No occupied rewrite while serving | -| `ScriptHashHead` | 32-byte OA ingest | seals at 0.80; no occupied rewrite while serving | -| `AddressHead` | 4 KiB pages of relative fks | torn 4-byte writes accepted; body-txid verifies | -| Sealed `tx.head` | BDZ MPHF + fuse8 filter | FdOnly `g`; fingerprints in RAM | -| `binary_fuse8` | 3-hash XOR fingerprint | `contains` indexes fingerprints unchecked | -| `HeightFence` | sorted run vec behind `Arc` | leftover snapshot is O(1); extend/pop COW | -| Spender overflow | linked list of `next` fks | no cycle bound | -| SH pages | 4 KiB delta pages, `next` pointer | zero page = terminator | -| `sorted_run` | LSM-style runs + manifest | `runs_io` mutex is a comment, not a type | -| `BlockQueue` | FIFO + `height → id` first-wins | RAM-only by design | -| `U64IdentityHasher` | identity hash of fk | sequential fks spread; 7-bit control tag clusters | -| io_uring machines | multi-stage SQE pipelines | idx-body, spend annotate, SH, sp_tweaks | - -### Query / confirm pipeline (`rbitcoin-query`, `rbitcoin-consensus`) - -| Piece | Algorithm | Notes | -|-------|-----------|--------| -| Stamp | in-flight → live_union → RecentCreates → TipOnly | miss is permanent | -| `LiveUnion` | ArcSwap chain of identity layers | get walks; splice keep by BQ / taken / horizon | -| `RecentCreates` | Arc layer chain + COW pending | write-published; head CAS/RCU | -| `SharedParentPin` | Frozen `OnceLock` → `ArcSwap` RCU | compose-only; no in-place mutation | -| `PinOuts` / `ParentLayout` | sorted sparse vecs, `binary_search` | `covers_need` = examined, not necessarily live | -| `TxidHasher` | last 8-byte write wins | sound only for bare `[u8;32]` | -| `OutPointHasher` | FNV-ish mix every write | `pending_spent` | -| `TxPrecompute` | one-pass txid/wtxid/midstates | lookup publishes; scripts reuse | -| Confirm queues | loadq=14, scriptq=4, writeq=14 | bounded sync channels | -| Script pool | lock-free steal waves | `in_wave` + `failed` atomics (Acquire/Release) | -| Merkle | Bitcoin duplicate-last | CVE-2012-2459 covered by whole-block txid set | -| Difficulty | Core retarget + 4× clamps | testnet 20-min min-diff | -| MTP | 11-header median | fence-tip is an 11-slot ring; historical heights still walk | - -### Script engine - -| Piece | Algorithm | Notes | -|-------|-----------|--------| -| `eval_script` | Core-shaped opcode walk | tapscript OP_SUCCESS pre-scan, 520 B stack | -| CHECKSIG | DER lax + optional BIP66; Schnorr BIP340 | BIP342 validation-weight budget | -| P2SH | `eval_script` scriptSig then IsPushOnly | last stack item is redeem | -| Sigops | `script_sigop_count` byte walk | truncated PUSHDATA2/4 stops (Core `GetOp`) | -| Signet | challenge script + witness commitment | last exact 38-byte BIP141 | - -### Net / IBD (`rbitcoin-net`) - -| Piece | Algorithm | Notes | -|-------|-----------|--------| -| Headers sync | one `sync_started` peer | 50 ms session tick → `on_session_heartbeat` → stall timeout | -| Inbound handshake | 60 s VERSION/VERACK bound | `inbound_connect_and_handshake`; drops the `max_inbound` permit | -| IBD assign | densify walk, `FAR_SCAN_BUDGET` 65 536 | `densify_scan_lo` skips BQ-ready prefix; request-limited soft budget | -| Compact blocks | short-id + prefilled interleave | prefilled index validation weaker than Core | -| AddrMan | unbounded `HashMap` | no tried/new bucket caps | -| Tx relay | graph + per-peer announced set | `HashMap` on `TxGraph`; `relay_seq`/`accept_at` dropped in `unindex_txid` | -| Chain work | RAM prefix `work through h` | rebuilt from wire headers; no durable `nChainWork` | -| `BlockCache` | `Vec` of hashes | prefix eviction O(chain) | -| Peer rate limit | **fixed** 1 s window | documented as sliding | -| Eviction of pending/held | `HashMap::keys().next()` | random, not FIFO | -| v2 decode | command + payload | no sha256d checksum / v1 reframe | - -### Mempool / RPC / wallet APIs - -| Piece | Algorithm | Notes | -|-------|-----------|--------| -| Mempool graph | clusters + linearization + chunks | eviction = first `(rate, rep)` map entry | -| Orphanage | count + weight FIFO | no 20-minute expiry | -| Mempool store | slot file + body image | `persist_all` rewrites whole files; body after slots | -| Fee estimator | log buckets | | -| `testmempoolaccept` | `MempoolHub::test_accept` | prepare + scripts + RBF/cluster; no commit | -| RPC `active` | id-keyed map | `dispatch` removes by id | -| Electrum status | `sha256(concat rows)` | confirmed rows include **blockhash** (COMPAT A-B-A); mempool appended last (same as `get_history`) | -| Esplora `/blocks` | reconstruct 10 full blocks | for size/weight only | -| SH join cache | process-wide single slot | sequential REST reuse; concurrent different SHs re-join | - -### Node / primitives - -| Piece | Algorithm | Notes | -|-------|-----------|--------| -| CLI / conf | hand-rolled parsers | `milestone=0` in conf indistinguishable from unset | -| Log | global level, no module filter | `format_args` not gated | -| `script_sigop_count` | opcode walk | truncated PUSHDATA2/4 stops (Core `GetOp`) | - ---- - -## 2. Highest-priority findings - -*(none remaining — Electrum status extra `blockhash` is COMPAT, not High.)* - ---- - -## 3. Medium findings (correctness / durability) - -### Store - -- **S-M1.** `VarTable::published_meta` seqlock (`var_table.rs`): data loads - are `Relaxed` with no fence before the second seq load. x86 TSO hides it; - ARM can tear `(count, end)` → spurious `Corrupt("record range")`. Fix: - `Acquire` fence after data, or load data `Acquire`. -- **S-M2.** `ArrayTable::flush_dirty` / `StrongTxTable::flush_dirty`: snapshot - under read lock, drop, write, then clear `dirty`. A `set` in the window is - lost until some later set re-dirties. Roles intend a single Class C - writer, but flush can be a sidecar. Strong-bit miss recreates the - "unstrong tip txs" case `store.rs` comments guard. Fix: `swap` dirty false - **then** snapshot. -- **S-M3.** Fuse8 `decode_body` (`fuse8_filter.rs`) does not check - `segment_count_length + 2*segment_length ≤ fp_len`. `BinaryFuse8::contains` - indexes fingerprints unchecked → panic on corrupt-but-decodable sidecar - instead of `NeedsRewrite`. -- **S-M4.** `for_each_spender_create` (`point_table.rs`): overflow `next` - walk has no cycle bound. Bit-rot loops the query thread. Bound by - `spenders.count()`. -- **S-M5.** Sidecar `std::fs::write` + `rename` without `sync_all` (segment - meta, `.mphf`, SH `.idx`). Crash can publish empty metadata. Open-time - `Corrupt` is the recovery — against the "no silent wipe, no surprise - repair" policy. -- **S-M6.** `sorted_run` orphan GC requires every caller to hold `runs_io` - (comment only). List without the lock during "file written, manifest not - yet updated" deletes a live run. - -### Consensus - -*(none remaining)* - -### Query - -*(none remaining)* - -### Net / mempool / RPC / esplora - -- **N-M1.** Compact-block prefilled indexes: weaker than Core's strictly - increasing + in-bounds check; short-id cursor can misalign (`compact.rs`). -- **N-M2.** `HashMap::keys().next()` eviction for pending blocks, held - bodies, fork-tips — random under cap. `hold_body` already has `held_seq` - unused for eviction. -- **N-M3.** `cmpct_fills` and `requested_blocks` grow until success/arrival; - no prune on abandon/timeout. -- **N-M5.** AddrMan unbounded; Core caps tried/new. -- **N-M6.** `announced_wtx` cap = clear entire set → INV burst. -- **N-M7.** `disconnect_to` clones all disconnected txs even with no mempool - (IBD reorg RAM). `invalidate_block` / `note_confirmed_tip` reconstruct - full blocks when `header_at_height` would suffice for hashes. -- **M-M1.** Known-parent out-of-range vout parked as orphan forever - (`accept.rs`). Hard-reject. -- **M-M2.** Eviction `worst_chunk` on rate **tie** can pick an earlier chunk - and strand descendants (no `removeRecursive`). -- **M-M3.** `evict_to_budget` `removed` is cumulative; later no-op iteration - does not break → spin. -- **M-M4.** `persist_all` writes meta, then slots, then body — inverted vs - body-before-claim. Crash → LIVE slot, garbage body. -- **M-M5.** `accept_package` has no package feerate (0-fee CPFP parent - rejected). Name vs Core `submitpackage`. -- **R-M1.** `submitblock` gated to regtest; help / [`rpc.md`](./rpc.md) say - all networks. -- **R-M2.** `gettxout include_mempool` does not hide mempool-spent confirmed - outs (`spends_outpoint` exists). -- **R-M3.** `getnetworkhashps` assumes ~2 hashes/block (regtest). -- **R-M4.** Unbounded JSON-RPC batch under one work permit. -- **R-M5.** `getmininginfo` `blockmintxfee` is a sat/kvB feerate passed - through `json_btc_amount` (amount helper). Use a feerate formatter. -- **R-M6.** `sendrawtransaction` / `submitpackage` accept `maxfeerate` / - `maxburnamount` and ignore them. Enforce or reject the params. -- **E-M1.** `scripthash_mempool_stats` undercounts chained unconfirmed vs - Esplora. -- **X-M2.** Esplora WS does store IO on the tokio thread (`ws.rs` `on_tip` / - `on_mempool_announce`). REST uses `spawn_blocking`. -- **O-M1.** Conf `milestone=0` overwritten by network default (`cli.rs`). - CLI `--milestone 0` works; conf cannot disable skip. -- **O-M2.** `--minrelaytxfee` parse failure silently ignored; negatives → 0. -- **O-M3.** Tip-follow stale redial uses AddrMan cloned once after IBD. - ---- - -## 4. Performance (big-O, allocation, IO) - -Grouped by cost at mainnet scale. Many overlap §3. - -| ID | Where | Cost | Standard replacement | -|----|--------|------|----------------------| -| P4 | BDZ `index_batch_fd` fill | O(pages × keys) | pre-group by page | -| P5 | `HashHead::bulk_fill_empty` | full table in RAM + dense write | dirty 4 KiB pages only | -| P6 | SH `insert_many` `entries.iter().find` | O(N²) ingest | `HashMap` once | -| P9 | `queue_due_tx_invs` | O(peers × mempool), 1 Inv/tx | batch `Inv`; iterate `relay_seq` delta | -| P11 | Esplora `/blocks` | 10 full reconstructs / page | persist size/weight at connect | -| P12 | Electrum status | 1 header read / history row / notify | cache confirmed-prefix hash | -| P13 | `find_free_slot` | O(cap) per admit, O(cap²) fill | free-list `VecDeque` | -| P14 | `persist_all` | rewrite entire mempool files | append body + patch slot | -| P15 | `evict_nonfinal` | O(n²) per reorg | worklist of affected txs | -| P18 | BQ `dequeue` height remap | O(queue) `index.iter().find` | `HashMap>` | -| P19 | `BlockCache` prefix drop | O(chain) per connect | `VecDeque` + base height | -| P20 | GBT `depends` | O(txs × inputs) linear scan | `HashMap` | -| P21 | Log macros | eval args + TLS even if disabled | `if enabled { … }` in macro | -| P22 | `api_log` global mutex + `write_all` | blocks async workers | dedicated writer thread | -| P23 | Esplora WS `scripts_touched_full` | store/mempool IO per announced input per conn | intersect `ann.scripthashes` | - -Low (still real): `StrongTxTable::count_ones_bits` bit-by-bit vs -`u8::count_ones`; `PendingHead::queued_fk` reverse scan of 262 144; -per-slot pread in occupied walks; AddressHead probe copies of 32-byte keys; -`InFlightLayer.creates` SipHash vs `TxidHasher`; SH `Vec::contains` dedup; -`check_libre_annex` clones the witness; `find_and_delete_sig` always -allocates; one-shot confirm `block.clone()`; seeds `Vec::contains`; -GBT longpoll parks a blocking thread with no timeout. - ---- - -## 5. Memory - -| ID | Where | Shape | Cap today? | -|----|--------|--------|------------| -| Mem2 | AddrMan | peer `addr` feed | **no** | -| Mem3 | `announced_wtx` | 50k then **clear all** | yes, but bursty | -| Mem4 | Mempool dead body bytes | RBF stall, compact not on admit | disk + rewrite | -| Mem5 | `bulk_fill_empty` | transient hundreds of MiB | per grow | -| Mem6 | IBD `disconnect_to` | whole losing branch cloned | skip if no mempool | -| Mem7 | `--api-log` JSONL | unbounded | **no** | -| Mem8 | Orphanage / Electrum subs / WS tracks | count/weight / `max_*` | yes | -| Mem11 | `TX_FULL_GETS` thread-local | prod `Vec` push per get | test-shaped leak | - -Body queue and RecentCreates pins are request-/horizon-limited **by design** -([`ibd-memory.md`](./ibd-memory.md)). [`errata.md`](./errata.md) leftover -maps (`txid → one fk`) stay as-is (Won't-fix unless a mainnet miss). - ---- - -## 6. Simplifications (ad-hoc → standard) - -Do not flatten io_uring machines. Do not add a process pin FIFO. - -1. **Mempool slots:** free list (P13). -2. **Block queue / densify:** keyed multimaps instead of `iter().find`. -3. **Held/pending eviction:** FIFO via existing `held_seq` / `VecDeque`. -4. **Announced-tx sets:** rolling bloom (Core `CRollingBloomFilter`). -5. **BlockCache:** `VecDeque` + height offset. -6. **Esplora `/blocks`:** stored summary, not reconstruct. -7. **`last_push_data`:** `Script::instructions()` (gets PUSHDATA4). -8. **`U64IdentityHasher`:** multiply by odd golden-ratio constant if - hashbrown clustering shows. -9. **Seqlock:** fences, or stop rolling your own for a 16-byte pair. -10. **CLI parsers:** table-driven `take_parsed` (node + bench). -11. **Bench hex:** use `rbitcoin_primitives::hex_*`. -12. **Bit count:** `u8::count_ones`. -13. **SH `put_sorted_creates` `seen`:** `put_chain` already sorts+dedups. - ---- - -## 7. Gotchas / fragile invariants (not bugs today) - -- Stamp / leftover-union miss is **permanent** — load bugs must not invent a - spentness fallback (`docs/invariants.md`). -- `PinOuts::covers_need` means *examined*, including proven-absent vouts. -- Assemble sticky can miss a peer-batch widen (perf only; immutable snaps). -- `TxidHasher` last-write-wins: never put it on `(txid, vout)`. -- `AddressHead` torn 4-byte slots are OK only because body-txid verifies. -- `eval_script` `Ok(false)` discarded in `verify_bare` — load-bearing - (tapscript OP_SUCCESS only). -- Store `Corrupt` string-matched to `MissingPrevout` in consensus `error.rs`. -- Future-time uses wall clock, not Core adjusted time. -- `PeerRateLimiter` is a fixed window (2× burst at boundary). -- Compact first-wins BQ + hash-checked claim mutates the queue from - read-shaped helpers (`assign.rs`). -- Log `RUST_LOG=info,hyper=trace` sets **global** Trace. -- `spawn_sptweaks_backfill` process-global `Once` captures first `query`. -- Signal handler installed after store open. -- Electrum/Esplora `Lagged` broadcast: missed notifies never resync - (COMPAT says best-effort). -- Mempool `commit_after_script` trusts the tip captured at `prepare_admit`; - a reorg between prepare and commit relies on the caller running - `evict_nonfinal`. - ---- - -## 8. Checked and found sound - -- Publish order body → idx → count/HWM on Class A; readers use HWM. -- Confirm pipeline: bounded downstream channels; no cycle; write condvar has - a timeout. -- CVE-2012-2459: whole-block duplicate-txid set. -- Difficulty 4× clamps and `first_block_time` off-by-one match Core - (mainnet). Testnet 20-minute min-diff walk-back matches Core. -- MTP median index matches Core `GetMedianTimePast`. -- BIP30 exception blocks 91842/91880 present. -- Duplicate inputs via `OutPointSet`; weight/MAX_MONEY saturating then range. -- CLTV/CSV 5-byte scriptnum + type match; tapscript OP_SUCCESS, MINIMALIF, - 520 B initial stack, annex, sighash midstate reuse; P2WSH 520 B; BIP342 - validation-weight budget. -- P2SH scriptSig is `eval_script` + IsPushOnly; truncated PUSHDATA2/4 does - not count leftover CHECKSIG; witness sigops gated on segwit. -- Same-block spends = pending only; spender height before create = unspent. -- `PinHalf` Frozen→ArcSwap: `OnceLock::set` loser re-applies `f` — no lost - compose. -- Stamp priority matches `docs/invariants.md`. -- Soft densify is request-limited only. -- Merkle odd-layer duplicate-last. -- `TxPrecompute` vs rust-bitcoin oracles (incl. zero-input BIP144). -- Body-queue budget accounting matches `docs/ibd-memory.md`. -- Orphanage count/weight caps; Electrum/WS per-conn caps. -- `unindex_txid` drops scripthash index, unbroadcast, `relay_seq`, and - `accept_at`. - ---- - -## 9. Per-crate finding index - -Counts are unique **open** items in this document. -Intentional COMPAT Electrum status extra field is **not** counted as High. - -| Crate | High | Medium | Perf/Mem notable | -|-------|------|--------|------------------| -| store | 0 | seqlock, flush lost-update, fuse8 OOB, spender cycle, sidecar fsync, runs_io | BDZ fill, bulk_fill, SH N² | -| consensus + primitives | 0 | *(none remaining)* | historical MTP walks, rehash txids | -| query | 0 | *(none remaining)* | BQ scan, SipHash in-flight | -| net | 0 | compact indexes, random eviction, AddrMan/cmpct unbounded, v2 copies | INV flush, BlockCache | -| mempool | 0 | orphan vout, eviction tie, persist order, package feerate | free slot, persist_all | -| rpc | 0 | submitblock gate, gettxout, hashps, unbounded batch, blockmintxfee, maxfeerate | GBT depends, longpoll | -| electrum | 0 | mempool_stats | status full-history, announce O(subs) | -| esplora | 0 | WS on runtime | `/blocks` reconstruct, WS announce IO | -| node/cli/log/bench | 0 | milestone=0 conf, minrelay silent, frozen AddrMan | log gating, api_log mutex | - ---- - -## 10. Suggested work order - -Not a plan (no red/green steps). Split-risk, then operator-visible, then -IBD CPU. - -1. Mempool persist order, AddrMan cap. -2. Esplora `/blocks` summaries (P11). - -Out of scope (Won't-fix / policy): flattening uring, process pin FIFO, -leftover `Vec`, explorer APIs, `rbitcoin-bench` in required CI, -Esplora SH join LRU / per-IP / large cache (**X-M3**). -Dropped from this list as too small or already owner-doc: ping RTT in -whole seconds, bench Electrum response pairing, CLI `--` for negative -RPC args, chunked HTTP in `rbitcoin-cli`. diff --git a/docs/quality.md b/docs/quality.md index 5d2276ea..b91ab0c0 100644 --- a/docs/quality.md +++ b/docs/quality.md @@ -24,9 +24,9 @@ This is not a security audit. Numbers are order-of-magnitude. **1.0 product gates** (what an operator can count on) live in [`road-to-1.0.md`](./road-to-1.0.md). Do not copy that list here. -Open algorithm/data-structure findings live in -[`algo-review.md`](./algo-review.md). Close an item by deleting it there in -the same PR; do not paste those tables into Open. +Former `algo-review.md` items that are still worth doing are **Q-57–Q-60**. +Inventory tables, gotchas, and micro-opts were not a second backlog — they +died with that file. Close a Q-id by moving it to Completed in the same PR. Peer full-node notes (Hornet, satd) live in [`peer-clients.md`](./peer-clients.md). Ranked later-consideration items @@ -93,12 +93,16 @@ evidence (failed Core corpus, new dual path, red required CI, MSRV drift). |-----:|----|------|-----|-----------------| | 1 | **Q-30** | Continuous differential fuzz | reliability | A nightly/weekly job that feeds BIP324 + header/block (and script) wire. Crashes → `docs/external_findings/` + named regression. **Today: `fuzz/` `block_wire` + nightly `fuzz.yml` (not a required PR check).** Grow corpus / more targets. Findings 001–021 came from an external fuzzamoto campaign — that is not a substitute for the in-tree job. | | 2 | **Q-41** | Grow Core functional `run` set | test | Inventory `run` covers the wallet-client / P2P / mempool / buried-activation scripts we **claim**. **Today: 53 run / 214 skip (30 rpc-missing, 26 core-log).** COMPAT-done leftovers are `rpc-dialect` (not `rpc-missing`). Next `run` candidates: `mempool_accept` type-check, `mining_basic` weight, `rpc_getblockfrompeer`. Product-never skips stay skip. Unlabeled PRs stay cargo-only; nightly green | -| 3 | **Q-48** | BIP331 rust-bitcoin package types | interop | Native BIP331 `NetworkMessage` when rust-bitcoin exposes it (**RB-007**). Packages today are RPC `submitpackage` / Esplora `POST /txs/package` only — no private P2P command. Blocked upstream — ranked below unblocked ops work. **After this:** Electrum 1.6 then 1.7 (`protocol_max` bump in the same work) — [`COMPAT.md`](../COMPAT.md) § Protocol versions | -| 4 | **Q-31** | Hermetic tip fixtures | ops | Frozen signet/mainnet tip packs for offline consensus/Electrum regression (no live API). Unblocks Q-30 corpora | -| 5 | **R-10** | Residual god-files | code | Peel **only** when a higher row needs a seam. After extracting `peer_tests` / `methods_tests` / `scripthash_tests`: production `query/lib` **4.2k**, `electrum/server` **3.7k**, `scripthash` **3.4k**, `sorted_run` **3.4k**, `methods` **3.3k**, `chain` **3.3k**, `store` **3.2k**. Further production peels wait for a real seam. **Q-54** may need a seam if a cap rule cannot match a god-file. | -| 6 | **Q-54** | Grow ast-grep rules from `ibd-memory.md` | code | One rule per named cap that is easy to delete: `pending_blocks` 128, `held_bodies` 320, `MAX_SERVE_BLOCKS` 16, `follow_live` vs `max_outbound`. Each rule has `lint/ast-grep/fixtures/{good,bad}/`. Peel god-files (**R-10**) only if a rule needs a seam. | -| 7 | **Q-55** | CRAP `--fail-regression` | test | Commit `crap_baseline.json` (`--format json --sort file`) from a green coverage artifact. PRs fail if a function’s CRAP rises. Still no `--fail-above 30` while clippy allows `cognitive_complexity`. | -| 8 | **Q-56** | Miri islands beyond primitives | reliability | `cfg(miri)` tests for FFI-free helpers (scriptnum, pack_ud-style integers) that do not pull secp/store. Never workspace miri. | +| 3 | **Q-57** | Store publish / Class C flush / sidecar | store | `VarTable::published_meta` loads count/end `Acquire` (ARM cannot tear the pair). `ArrayTable` / `StrongTxTable` `flush_dirty` cannot lose a `set` in the write window (clear dirty then snapshot, or equivalent). fuse8 `decode_body` fails closed (`NeedsRewrite`) instead of indexing fingerprints OOB. Spender overflow walk bounded by `spenders.count()`. Sidecar meta / `.mphf` / SH `.idx` do not rename an unsynced empty file into place. `sorted_run` orphan GC cannot delete a live run (lock is a type, not a comment). | +| 4 | **Q-58** | Mempool persist order + eviction | mempool | `persist_all` writes body before claiming LIVE slots. Known-parent out-of-range vout hard-rejects (not orphan-forever). `worst_chunk` rate-tie does not strand descendants. `evict_to_budget` no-op iterations break (no spin). | +| 5 | **Q-59** | RPC / CLI honesty | ops | `submitblock` matches [`rpc.md`](./rpc.md) / COMPAT (all networks) or those docs say regtest-only. `gettxout include_mempool` hides mempool-spent confirmed outs. `sendrawtransaction` / `submitpackage` enforce or reject `maxfeerate` / `maxburnamount`. Conf `milestone=0` is not overwritten by the network default. `--minrelaytxfee` parse failure is an error (negatives rejected). `getmininginfo` `blockmintxfee` uses a feerate formatter. `getnetworkhashps` is not a dummy ~2 hashes/block (or is labeled). JSON-RPC batch is bounded under the work permit. | +| 6 | **Q-60** | P2P caps + compact reconstruction | p2p | Compact-block prefilled indexes are strictly increasing and in-bounds. AddrMan has tried/new caps. `cmpct_fills` / `requested_blocks` prune on abandon/timeout. `announced_wtx` rolls instead of clear-all INV burst. Pending/held eviction is FIFO (`held_seq`), not `HashMap::keys().next()`. Esplora WS store IO uses `spawn_blocking` like REST. IBD `disconnect_to` skips cloning the losing branch when there is no mempool. | +| 7 | **Q-48** | BIP331 rust-bitcoin package types | interop | Native BIP331 `NetworkMessage` when rust-bitcoin exposes it (**RB-007**). Packages today are RPC `submitpackage` / Esplora `POST /txs/package` only — no private P2P command. Blocked upstream — ranked below unblocked ops work. **After this:** Electrum 1.6 then 1.7 (`protocol_max` bump in the same work) — [`COMPAT.md`](../COMPAT.md) § Protocol versions | +| 8 | **Q-31** | Hermetic tip fixtures | ops | Frozen signet/mainnet tip packs for offline consensus/Electrum regression (no live API). Unblocks Q-30 corpora | +| 9 | **R-10** | Residual god-files | code | Peel **only** when a higher row needs a seam. After extracting `peer_tests` / `methods_tests` / `scripthash_tests`: production `query/lib` **4.2k**, `electrum/server` **3.7k**, `scripthash` **3.4k**, `sorted_run` **3.4k**, `methods` **3.3k**, `chain` **3.3k**, `store` **3.2k**. Further production peels wait for a real seam. **Q-54** may need a seam if a cap rule cannot match a god-file. | +| 10 | **Q-54** | Grow ast-grep rules from `ibd-memory.md` | code | One rule per named cap that is easy to delete: `pending_blocks` 128, `held_bodies` 320, `MAX_SERVE_BLOCKS` 16, `follow_live` vs `max_outbound`. Each rule has `lint/ast-grep/fixtures/{good,bad}/`. Peel god-files (**R-10**) only if a rule needs a seam. | +| 11 | **Q-55** | CRAP `--fail-regression` | test | Commit `crap_baseline.json` (`--format json --sort file`) from a green coverage artifact. PRs fail if a function’s CRAP rises. Still no `--fail-above 30` while clippy allows `cognitive_complexity`. | +| 12 | **Q-56** | Miri islands beyond primitives | reliability | `cfg(miri)` tests for FFI-free helpers (scriptnum, pack_ud-style integers) that do not pull secp/store. Never workspace miri. | ### Still valid? (this reaudit) @@ -114,10 +118,11 @@ in store IO sessions + `script_pool` + confirm `head_drain`. | **Q-41** | Keep rank 2. 44 → **53** `run`. 214 skips; `rpc-missing` 30 + `core-log` 26 are the only growth matching claimed surface. | | **Q-50** | **Closed.** Write/lookup/load inventory includes `drain_join` / `dequeue` / wave nested tokens; `other=` is the explicit residual (`format_info` pins). A fat `other=` on a later IBD is confirm-perf, not a missing-meter program. 2026-08-18 dark-time numbers retired. | | **Q-36** | **Closed.** Default INFO is `ibd: progress`; `ibd: perf` / `ibd: sizes` / `perf_dbg` are DEBUG (`log_sample`). | -| **Q-48** | Keep, rank 3. Waits on rust-bitcoin (**RB-007**). | +| **Q-48** | Keep, rank 7. Waits on rust-bitcoin (**RB-007**). | | **Q-31** | Keep. Useful for Q-30; not blocking operators. | | **Q-34** | **Closed.** `OPERATOR.md` § First hour (regtest); README points at that section. | -| **R-10** | Keep last. Inline tests left `peer.rs` / `methods.rs` / `scripthash.rs`. Remaining giants are production-sized (`query/lib` **4.2k**). | +| **R-10** | Keep last among peels. Inline tests left `peer.rs` / `methods.rs` / `scripthash.rs`. Remaining giants are production-sized (`query/lib` **4.2k**). | +| **Q-57–Q-60** | **Added 2026-08-25** from retired `algo-review.md`. Not a reaudit of the rest of this table. | Prior-reaudit closures (Q-37, Q-47, Q-49) and Won't-fix calls (Q-24/25/32/33/35/38) stand — evidence unchanged. This pass adds @@ -128,7 +133,7 @@ and bench-in-CI (shipped shape, not a backlog). R-ids were the 2026-08-12 ranked slice. Canonical Open/Completed/Won't-fix id is in **bold**. Do not start **R-11+** — new work is the next unused -**Q-id (Q-57+)**. +**Q-id (Q-61+)**. | R-id | Canonical | Where | |------|-----------|-------| @@ -136,9 +141,9 @@ id is in **bold**. Do not start **R-11+** — new work is the next unused | R-07 | **Q-30** | Open rank 1 | | R-08 | **Q-20** | Completed | | R-09 | **Q-16** | Completed | -| R-10 | **R-10** | Open rank 5 | +| R-10 | **R-10** | Open rank 9 | -Next unused Q-id is **Q-57**. +Next unused Q-id is **Q-61**. --- @@ -157,6 +162,9 @@ Retired on purpose. Not a backlog. Not a failure. | **—** | Darwin notarization / Developer ID | Ad-hoc `codesign -s -` on the macos snapshot. Notarization is still not a product | | **—** | Leftover maps as `txid → Vec` | [`errata.md`](./errata.md): only if a mainnet miss is shown | | **X-M3** | Esplora process-wide `sh_join` LRU / per-IP / large cache | HTTP is not a session. A tiny LRU still evicts wallets; per-IP is NAT/DoS; a large cache is RSS (join payload × addresses × clients). Sticky joins stay on Electrum TCP (one slot per connection). Esplora keeps one last SH for sequential REST. | +| **—** | Package-level feerate on `accept_package` / `submitpackage` | COMPAT: sequential `accept_tx`; a 0-fee CPFP parent is rejected on its own min-relay. Core `submitpackage` parity is not 1.0. | +| **—** | Esplora `/blocks` reconstruct + chained `scripthash_mempool_stats` | Explorer page cost / dialect. Persist size/weight is a schema ask; graphical explorer APIs are already Won't-fix. | +| **—** | Retired algo-review micro-opts | BDZ page fill, `HashHead::bulk_fill_empty` RAM, SH `insert_many` N², INV O(peers×mempool), Electrum per-row header read, mempool `find_free_slot` O(cap), `evict_nonfinal` O(n²), BQ `index.iter().find`, `BlockCache` prefix O(chain), GBT `depends` scan, log-macro eval when disabled, `api_log` mutex, bit-by-bit `count_ones`, `U64IdentityHasher` clustering, `last_push_data` PUSHDATA4, `--api-log` unbounded JSONL. Not a second backlog. Reopen a named Q-id only with a mainnet profile that names the cost. | | **—** | Headerless SH extent interior pages | Extent is a span-read of the existing 4 KiB delta-page record. Interiors keep `ver`/`n_fks`/`next` so one decoder serves leftovers, tails, and last-page append. Full-page payload is ~0.2% and a schema bump | | **—** | Restore `rbtc-script-coord-*` | `ibd-confirm` publishes waves, polls lock-free completion, feeds `scriptq` when steal is empty. Steal workers unpark the publisher. Do not add coordinator threads to keep the pool fed | | **—** | Flatten purpose-built io_uring machines | AGENTS.md: fix the machine; do not replace it with batched `pread`/`pwrite` without an explicit ask | @@ -203,7 +211,7 @@ findings 001–022, CI split, map-free README, …) live in | Do | Do not | |----|--------| | Close work by **moving the Open row into Completed** in the same edit as the landing change | Leave `Status: fixed` in Open, or start a second table | -| New item: next unused **Q-id (Q-57+)** inserted at an explicit rank | Fill historical gaps (Q-06–Q-09, Q-17–Q-19, Q-26–Q-29) or start **R-11+** | +| New item: next unused **Q-id (Q-61+)** inserted at an explicit rank | Fill historical gaps (Q-06–Q-09, Q-17–Q-19, Q-26–Q-29) or start **R-11+** | | Retire a row to **Won't fix** when the product will not do it | Leave dead Open rows “for completeness” | | God-file peels only when a higher Open row needs a seam (**R-10**) | Split `query/lib` / `interpreter.rs` / `scripthash.rs` as a standalone “modularity” project | | Suite: no new remine-100 / default test **>2 s** without justification ([TESTING.md](../TESTING.md)) | Time the full workspace as a planning spike | @@ -286,8 +294,7 @@ included; tree at #177): | Audience | Read | |----------|------| -| Next quality slice | **Open**, rank 1 (**Q-30** fuzz). Active program: **Q-41** (Core functional `run` set) | -| Algo / DS findings | [`algo-review.md`](./algo-review.md) — not a fourth backlog in this file | +| Next quality slice | **Open**, rank 1 (**Q-30** fuzz). Active program: **Q-41** (Core functional `run` set). Folded store/mempool/RPC/P2P leftovers: **Q-57–Q-60** | | Peer full nodes | [`peer-clients.md`](./peer-clients.md) — Hornet / satd notes; not a fourth backlog | | Release engineering | **Q-20**, **Q-21**, **Q-23** (completed) | | Security / adversarial | Protect Q-01–Q-02; next **Q-30** |