feat(commitments): add ternary mixed-hash integer path - #10
Conversation
0f450b3 to
386d0d7
Compare
|
Review recommendation: fix before merging. [P2] Reproduction: construct a commitment/witness for preimage Enforce the width during reconstruction and test the first out-of-range value for every supported width. At the larger widths, account for ScriptNum overflow during reconstruction, rather than relying only on a final comparison. September 11 local validation (unchanged PR head): Integration: rebase on current main, resolve any catalog/NR/OP ID collisions, and run the affected correctness tests and focused metric checks. Reviewed commit: |
|
Addressed the width-boundary review comment in 59443b5.
Focused checks passed: |
|
Review of The final accumulator bound now checks the declared integer width before the last multiply/add, addressing the earlier out-of-range reconstruction. Preserve the width-1 and width-31 rejection cases when rebasing; reconcile the shared commitment/catalog/metric entries and rerun ternary correctness, benchmark checks, named metrics, and KB validation. Current integration conflicts: Validation scope: source/diff and existing CI review; no new full-repository or field-arithmetic test run was requested. |
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 solving-bitcoin#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>
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>
|
Addressed in
Validation on the committed HEAD with a clean tree:
Evidence is |
Summary
0 -> SS,1 -> SR, and2 -> RSRepresentative result
For a 32-byte preimage and 31-bit value: 924 policy-produced script bytes, 63 serialized witness bytes, 21 witness items, and a 24-item peak. This is intentionally retained as a native three-valued state representation, not as an integer byte-efficiency improvement over the four-way path.
Validation
cargo fmt --all -- --checkpython3 tools/kb.py validatecargo test --locked ternary_hash_path --libcargo test --locked --test primitive_metrics ternary_hash_path_metrics_are_currentcargo test --locked --example ternary_hash_path_benchmarkcargo run --locked --example ternary_hash_path_benchmarkThe full repository-wide suite was not run; field-arithmetic tests remain outside this focused validation.