Skip to content

Integrate 15 reviewed primitive and Taproot improvements - #246

Merged
RobinLinus merged 65 commits into
mainfrom
codex/review-bitcoin-prs-20260928b
Sep 28, 2026
Merged

RobinLinus merged 65 commits into
mainfrom
codex/review-bitcoin-prs-20260928b

Conversation

@RobinLinus

@RobinLinus RobinLinus commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Integrates 15 reviewed PR heads after their previous review findings were addressed: #245, #136, #135, #132, #130, #116, #113, #92, #75, #73, #72, #71, #49, #42, and #10.

The changes add depth-one Taproot commitment fixtures, checked arithmetic adapters, SHA-1/RIPEMD-160 continuation helpers, focused stack/encoding regressions, and measured research boundaries. Numeric-alias tests distinguish consensus from policy; ternary integer checks reject oversized values before reconstruction overflow.

Resolved overlapping documentation/catalog additions and retained both hash-midstate metric helpers and every existing metric function. Removed three scratch exports introduced by #42 (knowledge/primitives/u32.md.bullets.txt, tests/primitive_metrics.rs.fns.txt, tests/primitive_metrics.rs.keys.txt). Original PR heads remain ancestors of this branch.

Validation:

  • All 15 source heads have successful Rust, knowledge/Python/formatting, and pinned-Core CI jobs.
  • Knowledge validation: 122 records, 304 configurations; Python: 50 tests passed.
  • All 15 affected metric tests passed without regeneration.
  • Fresh Bitcoin Core 30.3 comparison: 53 fixtures, all expectations met.
  • Core fixture example tests and signed-window, BLAKE3 boundary, and ternary examples passed.
  • Targeted library tests: 147 passed, 0 failed (no field-arithmetic tests).
  • Integration CI passed all three jobs (run 36435982871): 889 library tests, 148 metric tests, all remaining integration/doc tests, knowledge/Python/formatting, and pinned Core comparisons.

#62, #26 and #4 remain unchanged and require the integration work documented in their existing review comments.

brenorb and others added 30 commits September 11, 2026 01:44
Resolve the seven conflicted files by keeping main's content and adding the
ternary hash-path entries:

- knowledge/catalog.json: take main's schema 1.1 header; keep all 101 main
  records unchanged and add commitment/ternary-hash-path-integer (102 records,
  257 configurations).
- knowledge/negative-results/index.md: rename the PR's colliding NR-043
  ("Ternary mixed-hash paths lose to four-way integer paths") to NR-072 and
  append it after main's NR-065; main's NR-043 is untouched. Update its
  stale 924-byte figure to the current 947 bytes.
- knowledge/open-problems.md: the PR's OP-020 collided with main's OP-020
  (bound-start hash paths); renumber it OP-030 (OP-027 is used by two other
  open PRs) and update the catalog reference.
- knowledge/comparisons/commitments.md, knowledge/index.md: keep main's rows,
  paragraphs and review date; add the ternary row (947/63/24) and a paragraph
  with its 509-byte gap, data/hint items and execution class.
- src/commitments/README.md, tests/primitive_metrics.rs: follow main's
  per-primitive layout. Move the module to src/commitments/ternary_hash_path/
  with its own README (template sections, explicit 0 hint items), and add a
  ternary_hash_path_metrics() group chained into metrics() and checked by the
  existing named ternary_hash_path_metrics_are_current test. The stack metric
  now uses the strict executor; values are unchanged (947/63/24).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add rejects_first_out_of_range_value_at_widths_1_and_31_before_overflow. For
widths 1 and 31 it checks a valid 2^width-1 control and requires 2^width to be
rejected with ExecError::Verify from the pre-final-step width check. At width
31 a bound applied only after the last multiply/add is rejected by ScriptNum
overflow instead, which the existing all-width test cannot distinguish.

The pinned interpreter (a09e87af) now counts every tapscript instruction
position in opcode_count, so the benchmark's executed_opcodes=919 was a static
position count, not an executed-opcode total, and the documented value 0 was
stale. Report static instructions (919), static non-push opcodes (794), the
interpreter position count and executed_opcodes=unavailable, and correct the
README, knowledge page and research note.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e ternary path

Second integration of origin/main (3d1001f, after #244). Conflicts:

- src/commitments/mod.rs: keep main's tapbranch module/exports and the
  ternary_hash_path module/exports (alphabetical order).
- tests/primitive_metrics.rs: keep main's tapbranch imports and the ternary
  imports; all 134 main tests/931 metric keys remain, plus the ternary test
  and its three keys.
- knowledge/comparisons/commitments.md: keep main's TapBranch u4 row and the
  ternary row.
- knowledge/negative-results/index.md: keep main's NR-066 and append the
  ternary entry as NR-072 after it (68 unique NR headings).

Catalog auto-merged: 117 main records unchanged plus the ternary record
(118 records, 284 configurations).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve conflicts with main's integrated u4 siblings (threshold, equality,
trichotomy, clamp, MSB, and others) by keeping both sides:

- knowledge/catalog.json: main's 117 records unchanged plus the PR's
  arithmetic/u4-count record (118 records, 284 configurations).
- tests/primitive_metrics.rs: main's 134 metric tests plus
  u4_symbol_count_metrics_are_current (135).
- src/arithmetic/u4/mod.rs, src/arithmetic/u4/README.md,
  knowledge/comparisons/arithmetic.md, knowledge/primitives/index.md:
  union of module declarations, API bounds, and table rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve conflicts against origin/main 3d1001f by keeping both sides:
- knowledge/comparisons/arithmetic.md, knowledge/primitives/index.md,
  src/arithmetic/u32/README.md: keep main's new rows/bullets/paragraphs and
  the constant XOR row/bullet/paragraph.
- src/arithmetic/u32/mod.rs: keep xnor_constant/zero/zero_byte_mask from main
  and add xor_constant.
- tests/primitive_metrics.rs: take main and re-add
  u32_xor_constant_metrics_are_current (134 metric tests; main's 133 plus 1).
- knowledge/catalog.json merged cleanly: 118 records/284 configurations
  (main 117/283 plus arithmetic/u32-xor-constant/checked-constant).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve conflicts in knowledge/catalog.json, knowledge/comparisons/arithmetic.md,
knowledge/primitives/index.md, src/arithmetic/u4/README.md,
src/arithmetic/u4/mod.rs and tests/primitive_metrics.rs by keeping every
main-side record, row, module and metric test and adding the PR's
arithmetic/u4-presence record, rows, `pub mod presence` and
u4_presence_bits_metrics_are_current fixture.

Catalog: main 117 records / 283 configurations, PR 56 / 162, merged
118 / 284 with no duplicates; all main records unchanged. Metric fixture
unchanged (1526 bytes, 33 witness bytes, 16 data items, 34-item peak,
936 static non-push opcodes).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Integrate origin/main (3d1001f) into the signed-window schedule branch.

- knowledge/negative-results/index.md: keep every main entry (NR-001..NR-066,
  NR-071, historical PR #3, merged Merkle-branch record) and append this PR's
  two entries under their assigned IDs. The PR's NR-064 and NR-065 collided
  with main's NR-064 (data-only signature budgets) and NR-065 (CSV operand
  narrowing); they are renamed NR-067 (signed-window tables) and NR-068
  (width-5 fixed-base CSFS). No PR-side links referenced the old IDs.
- src/arithmetic/u4/mod.rs: take main's sorted module list, a strict superset
  of the PR's alphabetical reordering (47 vs 37 modules, no PR-only module).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…result to NR-069

Integrate origin/main (3d1001f) into the BLAKE3 keyed-mode boundary PR.

- knowledge/negative-results/index.md: keep main's file byte-identical
  (NR-001..NR-066, NR-071) and append the keyed-mode entry under its
  assigned ID NR-069 instead of the colliding NR-064. Entry text is
  otherwise unchanged; no PR-side link referenced the old anchor.
- src/arithmetic/u4/mod.rs: take main's sorted module list; the PR side
  only reordered declarations and every PR-side module is present on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brenorb and others added 28 commits September 28, 2026 06:23
Run the raw-witness alias regressions as complete leaves under the local
TapscriptProfile::Consensus profile that check all 16 presence bits and
end with a single OP_TRUE, matching the merged u4 equality and
adjacent-equality contract.

- compares_nonminimal_numeric_encodings_by_value: [01 00], [80] and
  [00 00] at every position of a three-item batch, clean-stack checked.
- numeric_aliases_follow_ordered_presence_bits: asymmetric witness
  [01 00], [80], [02 00], 5, [0f 00 00 00] (values 1, 0, 2, 5, 15) with a
  non-palindromic expected vector; canonical control under Consensus and
  Policy; the Policy profile rejects the aliases with MinimalData.
- byte_equality_and_reversed_output_mutants_fail: test-only mutants
  (all 80 OP_NUMEQUAL -> OP_EQUAL; reversed output order) fail the same
  leaf with NumEqualVerify.

Add shared scriptnum/run_tapscript/assert_clean_true helpers to
arithmetic::test_helpers. Record the new tests, the alias contract and
zero hint items in the catalog, knowledge page and u4 README. The
production fragment and its metrics are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…icts

Integrates origin/main (3d1001f) into feat/sha1-midstate-boundary
(94c819e) to address Robin Linus's 2026-09-25 review. Robin's conflict
list named only tests/primitive_metrics.rs; the actual trial merge also
conflicted on src/arithmetic/u4/mod.rs and src/hashes/sha1/mod.rs.

- src/arithmetic/u4/mod.rs: both sides' `pub mod` lists were already a
  subset of main's, so main's file is taken as-is (verified: every PR
  mod line is present in main's list).
- src/hashes/sha1/mod.rs: kept the PR's new sha1_80bytes_from_midstate
  continuation function, but adapted it to main's shared
  u8_reverse_toaltstack() helper (arithmetic::u32::stack) instead of
  restoring the PR's now-superseded local push_reverse_bytes_to_alt,
  per the repo convention of keeping main's refactor.
- tests/primitive_metrics.rs: kept every #[test] fn and metric key from
  both sides (134 tests = 133 from main + 1 PR-only
  sha1_midstate_metrics_are_current; 934 metric keys = union of main's
  931 and the PR's 802, no duplicates). All of main's hash metrics
  (including the new SHA-256 midstate/tagged-hash metrics) remain.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bring u32_xor_constant tests to the embedded-constant OR/AND/XNOR contract
and make table cleanup explicit:
- rejects_malformed_and_nonminimal_limbs: complete leaf under
  TapscriptProfile::Consensus; at all four positions -1/-127/256 -> Verify,
  negative zero / non-minimal one -> EqualVerify, 5-byte ->
  ScriptIntNumericOverflow; 0..3 limbs -> InvalidStackOperation; canonical
  128/255 at every position succeed with the correct XOR; a test-only
  top-limb-only mutant accepts the alias at positions 0-2.
- projects_boundary_and_pattern_words: 8 boundary cases plus 100 seeded
  cases via arithmetic::test_helpers::{run_with_witness, word_witness}.
- drops_whole_table_and_restores_stack_state (new): OP_DEPTH check after
  each of one and three sequential calls with main/alt sentinels; peak 276
  for both; a test-only mutant leaving one table item fails the depth check.
- Keep preserves_surrounding_main_and_alt_stack_items.

Helpers stay local, as in and_constant.rs, because sibling helpers are
private. Update the catalog test list and knowledge page; the table is
transient per call, not persistent or reusable. Fragment bytes unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merges origin/main (3d1001f, which has since integrated #84 signed
less-than, #45 fixed rotations/equality, #40 reverse-byte adapter, and
27 other PRs) into feat/u32-compressed-boundary, addressing Robin
Linus's 2026-09-25 review.

Conflicts resolved:
- knowledge/catalog.json: kept the u32 record's "tests" entries from
  both sides (u32_compression_metrics_are_current and
  u32_reverse_byte_adapter_metrics_are_current), and merged the
  "security"/"stack_contract" prose so both sides' sentences survive.
  main ⊆ merged and PR ⊆ merged verified by record-id and
  (record, configuration)-id set comparison: 117 records / 285
  configurations after merge, no duplicates, no ids outside the union
  of both sides.
- knowledge/primitives/u32.md: concatenated both sides' bullets
  (signed compression boundary / unchecked decoder from the PR;
  reusable routing / byte-plane transpose / fixed-byte rotations /
  SHA-256 rotation from main). All 43 main bullets and 40 PR bullets
  verified present in the merged 45.
- src/arithmetic/u4/mod.rs: union of both sides' `pub mod` lines,
  alphabetically sorted (main added several u4 modules the PR
  predates; no semantic content lost).
- tests/primitive_metrics.rs: kept every #[test] fn from both sides
  (u32_compression_metrics_are_current from the PR;
  u32_reverse_byte_adapter_metrics_are_current, u32_equality_metrics_are_current,
  u32_fixed_rotation_metrics_are_current, u32_byte_planes_metrics_are_current
  from main). 207 fns after merge = union of main's 206 and the PR's
  169 (168 shared).

src/arithmetic/u32/README.md and src/arithmetic/u32/stack.rs merged
without conflict.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…current main

Resolve conflicts against origin/main 3d1001f in
knowledge/comparisons/arithmetic.md, knowledge/primitives/index.md,
src/arithmetic/u32/README.md, src/arithmetic/u32/stack.rs,
src/hashes/sha256/sha2_u32.rs and tests/primitive_metrics.rs by keeping
every main row, test and metric plus the PR's u32_not additions.
knowledge/catalog.json merged cleanly (117 main records kept unchanged,
one PR record added).

SHA256: keep main's sha2_u32.rs verbatim (table sharing, input routing
and its local unchecked u32_not). The PR's only SHA256 change was a
bytecode-identical refactor replacing that local helper with a
crate-private u32_not_unchecked; it is not needed, so it is dropped
together with the now-unused u32_not_unchecked/u32_not_step helpers.
Docs now describe the unchanged SHA256 helper accurately.

Add checked_complement_rejects_malformed_and_nonminimal_limbs following
or_constant's shared contract (complete leaf, Consensus profile, exact
error per malformed encoding at every limb, all-position nonminimal
predicate, top-limb-only mutant, canonical 0/127/128/255 controls), and
give the pushed-limb loop in checked_complement_rejects_non_byte_limbs a
terminal predicate and valid control so it is no longer vacuous.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ical API

Resolve conflicts against origin/main 3d1001f in src/arithmetic/u4/parity.rs,
src/arithmetic/u4/README.md, src/arithmetic/u4/mod.rs and
knowledge/comparisons/arithmetic.md.

- parity.rs: keep #233's numeric-range API, docs and all five tests
  (projects_all_nibble_parities_in_order, distinguishes_asymmetric_output_order,
  rejects_out_of_range_nibbles, rejects_zero_and_overlarge_batches,
  respects_combined_stack_frontier) plus the PR's four canonical tests and the
  separate U4_PARITY_CANONICAL_MAX_BATCH = 981 bound.
- mod.rs: main's module list is a superset of the PR's sorted list.
- README/comparison: keep rows from both sides; describe the numeric-range
  and canonical APIs separately.
- Knowledge page and catalog: split the numeric-range and canonical-encoding
  descriptions; the range-only API is not claimed to enforce canonical
  encoding.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The canonical malformed-input test ran under the default research helper,
whose interpreter enforces MINIMALDATA on numeric operands. It therefore kept
passing when the canonical byte-equality check was removed from
u4_nibbles_to_parity_canonical: the interpreter, not the fragment, rejected
the aliases.

- canonical_projection_rejects_malformed_nibbles: keep the original loop and
  add a local TapscriptProfile::Consensus loop (no MINIMALDATA) with a valid
  control, aliases 0x0100/0x00/0x0f00/0x80 and range failures 0x0001/0x81 at
  every position, asserting the rejection is not MinimalData.
- range_api_accepts_numeric_alias_that_canonical_api_rejects: under the
  consensus profile the numeric-range API accepts 0x0100 while the canonical
  API fails with EqualVerify; the default helper rejects it via MinimalData.
- canonical_respects_combined_stack_frontier: 979 canonical nibbles with one
  main- and one alt-stack item reach exactly 1,000 items; 980 fails StackSize.
- Qualify docs: numeric-range alias acceptance holds without MINIMALDATA.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#236 and Core fixture #225

Resolve conflicts in knowledge/catalog.json, knowledge/primitives/u4-lsb.md,
src/arithmetic/u4/README.md, src/arithmetic/u4/lsb.rs and
src/arithmetic/u4/mod.rs (the last one is new relative to e9d5a66 because
main now also carries #244).

- lsb.rs: keep #236's range-checked wording, imports and its three tests
  (projects_all_nibble_least_significant_bits_in_order,
  rejects_out_of_range_nibbles_at_each_position,
  respects_stack_frontier_and_preserves_state); keep this PR's separate
  u4_nibbles_to_lsb_canonical API, its 981-input bound and its four tests.
  Document the canonical API's contract distinctly.
- mod.rs: take main's sorted module list (superset of the PR's).
- catalog: keep main's range-only summary/security wording and #225's Core
  complete-leaf configuration; add the PR's canonical-checked-batch32
  configuration, list the canonical tests and metric, and state that only
  the canonical API enforces canonical ScriptNum encoding.
- knowledge page / README: keep #236 and #225 text verbatim; add the canonical
  adapter as a separate API with its 1..=981 frontier and the note that the
  Core fixture does not validate it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ality check

canonical_projection_rejects_malformed_nibbles ran under the default local
helper, which enforces minimal ScriptNum decoding (require_minimal), so the
aliases [0x01, 0x00] and [0x80] were rejected with MinimalData by OP_WITHIN
before verify_canonical_nibble ran. With the canonicality check removed from
u4_nibbles_to_lsb_canonical the test still passed.

Run the rejection cases under the local TapscriptProfile::Consensus profile,
which decodes non-minimal numbers: add a valid control, a range-only control
showing u4_nibbles_to_lsb accepts both aliases there, and exact-error
assertions (EqualVerify for aliases, Verify for 256) for the canonical API at
every position. The original minimal-helper rejection check is retained.
With the canonicality check dropped, the test now fails with
"canonical API mishandled [1, 0] at 0" (left: None, right: Some(EqualVerify)).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Integrates origin/main (3d1001f, PR #244's 27-PR batch) into the
ripemd160 midstate-continuation branch. Robin Linus's review on
2026-09-25 named tests/primitive_metrics.rs as the integration
conflict; the actual merge also produced conflicts in
src/hashes/ripemd160/mod.rs and src/arithmetic/u4/mod.rs.

- tests/primitive_metrics.rs: kept every #[test] fn and metric `key`
  from both sides (main's 134 test fns plus the PR's
  ripemd160_midstate_metrics_are_current, merged set has 135, no
  duplicates; sha256_midstate_metrics_are_current and
  sha256_tagged_hash_metrics_are_current preserved as required).
- src/hashes/ripemd160/mod.rs: main independently extracted the
  byte-reversal helper into the shared
  arithmetic::u32::stack::u8_reverse_toaltstack; adapted
  ripemd160_80bytes_from_midstate to call that shared helper instead
  of restoring the PR's now-redundant private
  push_reverse_bytes_to_alt.
- src/arithmetic/u4/mod.rs: main's mod list is a strict superset of
  the PR's (PR only reordered existing entries); took main's version
  (47 pub mod lines) verbatim.
- knowledge/catalog.json: verified main's 117 records and the PR's
  101 records (including the ripemd160-u32 record's
  message-80-midstate configuration) are both subsets of the merged
  117-record file, with no duplicate ids.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The width tests only rejected 2^width. Since 2^width mod 3 is 1 or 2, its
final-step accumulator always equals the quotient, so only the remainder
branch was exercised: deleting the `acc <= q` check left all tests passing
and accepted value 6 at width 2.

Add rejects_out_of_range_values_on_both_width_check_branches_at_every_width.
For every width 1..=31 it runs a valid 2^width-1 control and requires
ExecError::Verify for 58 accumulator-above-quotient values ((q+1)*3 and
3^t-1; the branch is unreachable at widths 1 and 3) and 46
final-trit-above-remainder values. Update the catalog test list, README,
knowledge page and research note.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…licts

Robin Linus requested integrating current main while retaining #239's
checked/unchecked pair-packing and range-error tests on stack.rs
alongside this PR's word-transfer and lookup-lifecycle documentation
and tests.

- knowledge/catalog.json: combined the two independent stack_contract
  sentence additions (main's popcount wording, this PR's transfer/
  lookup wording); kept every record and configuration from both
  sides (117 records, 292 configurations; main's 283 configs + this
  PR's 9 new ones).
- src/arithmetic/u4/mod.rs: re-sorted the module list, keeping every
  `pub mod` from both sides.
- src/arithmetic/u4/stack.rs: kept both `use` imports (this PR's
  execute_script_with_inputs_strict and main's bitcoin_scriptexec::
  ExecError, the latter needed by #239's tests). All 24 tests in the
  module (main's 13 plus this PR's 11 new transfer tests) survive;
  the PR's now-superseded `packs_all_checked_nibble_pairs` is
  correctly dropped in favor of main's renamed/expanded
  `packs_all_nibble_pairs`, per instruction to keep main's
  pair-packing functions unchanged.
- tests/primitive_metrics.rs: both sides had appended new tests at
  the same insertion point; spliced this PR's 4 new u4 metric tests
  ahead of main's 4 new u32 metric tests, preserving all fns from
  both sides (206 main + 4 new = 210 total, verified by name-set
  comparison).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	knowledge/catalog.json
#	knowledge/comparisons/arithmetic.md
#	knowledge/primitives/index.md
#	src/arithmetic/u4/README.md
# Conflicts:
#	knowledge/catalog.json
#	knowledge/comparisons/arithmetic.md
#	knowledge/primitives/index.md
#	src/arithmetic/u32/README.md
# Conflicts:
#	knowledge/comparisons/arithmetic.md
#	src/arithmetic/u4/README.md
# Conflicts:
#	knowledge/negative-results/index.md
#	knowledge/open-problems.md
# Conflicts:
#	tests/primitive_metrics.rs
# Conflicts:
#	knowledge/catalog.json
#	src/arithmetic/u4/README.md
# Conflicts:
#	tests/primitive_metrics.rs
# Conflicts:
#	knowledge/negative-results/index.md
@RobinLinus
RobinLinus merged commit bf9ee0b into main Sep 28, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants