Skip to content

Bring the README, docs and help texts up to date, and check their claims - #25

Merged
mjthatch merged 1 commit into
research/audit-2-fixesfrom
research/docs-review
Oct 27, 2026
Merged

mjthatch merged 1 commit into
research/audit-2-fixesfrom
research/docs-review

Conversation

@mjthatch

Copy link
Copy Markdown
Collaborator

An end-to-end review of the repository's documentation after the fflonk, Noir and refactor work. Every number and command in the README and docs/ was checked against the code or re-measured on this branch, and the stale parts were rewritten or removed.

Generated programs are unchanged, except examples/fp_mul.simpl (below): the code changes are in comments, docstrings, help texts, tools/regtest.sh and tools/demo.py's ptau sizing.

README

  • Rewritten around what the repository is now. It has two verifiers (Groth16 and fflonk) and three bindings (Merkle root, sighash input, two limbs), and it takes circom and Noir circuits.
  • New sections:
    • a support table with the measured costs and weights;
    • "How it works" in five points;
    • requirements with tested versions;
    • a regtest walkthrough whose commands were run as written;
    • "Your circuit", a table of the three binding conventions with their commands;
    • a repository map that includes ci/ and .github/.
  • The status note says the code has not been independently audited.
  • Credits and references now cover every project and paper the code builds on. Added: rust-simplicity and Elements, Noir and Noir-Groth16, the perpetual powers of tau, Garaga, Gabizon–Williamson (fflonk), GLV, Bajard–Didier–Kornerup, the Keccak team, and py_ecc.
  • The architecture diagram is removed. It showed only circom → Groth16, and some of its labels were wrong. A replacement covering circom/Noir → Groth16/fflonk → deploy will come separately.
  • The pointer to the last Liquid testnet spend is removed. That spend used an earlier verifier without the G2 check. The README now says the current verifiers have been run on regtest only.

docs/

  • groth16.md:
    • Per-input costs are now measured: the second input adds 6.6 M mWU, the third 6.2 M.
    • The binding costs 14.5 k mWU at depth 1.
    • A factor raises a relation's degree by 17, not "17 or 8".
    • The addition-slope argument is split into steps, and the historical aside is gone.
    • In the residue witness, the 27th root of unity is ω and the exponent is Λ, so neither clashes with the polynomial variable w or the slope λ.
    • The τ clause is rewritten.
    • "About 4,500 multiplications" and "a reduction about 7.8 k" could not be traced to a measurement, so they are replaced by the benchmarked figures.
  • fflonk.md:
    • The link to a Keccak repository that is not public is removed.
    • The doubling argument now rests on the curve having no point of order 2.
    • The MSM's symbols are defined, and the GLV eigenvalue is μ.
    • The padding estimate is described as the tool computes it: without the groups taken or the binding, so it errs on the safe side.
    • The other-sighash test runs in Keccak mode only, and the doc now says so.
  • noir.md:
    • The nargo warning handling is described as both mechanisms: --deny-warnings, plus the scan of stderr for any bug, error or warning line. With --deny-warnings, nargo needs // Safety: comments.
    • The deploy compile time (about 10 minutes) is given.
    • Noir with fflonk is marked as untested.
    • The R1CS check is described as the tool does it: against the x.r1cs that compile wrote.
  • benchmarks.md:
    • The "before and after (main)" table is dropped.
    • Times are re-measured.
    • The reproduction list covers every test.
    • The regtest txids nobody else can look up are replaced by a table of this branch's regtest runs: Groth16 circom 53,315 WU, Noir 67,803 WU, fflonk SHA-256 126,589 WU, key path 721 WU.
  • SECURITY.md:
    • It says the code has not been independently audited.
    • The scope covers both verifiers and the tools.
    • The out-of-scope list names all three toy circuits, unbound programs, and upstream bugs.

Examples

  • examples/fp_mul.py / .simpl. The example multiplied unchecked witness words. That relies on the bound the verifiers enforce, but the example didn't show it. It now checks that both words are below 2^254, as the verifiers do (23 lines, 22,994 mWU). Without the check, a crafted pair of words near 2^256 gives the expected word for the wrong product. make example passes.
  • examples/README.md.
    • It no longer claims a Liquid testnet spend.
    • It explains fp_mul node by node.
    • It describes the witness nodes as they are (w_in, wl0–wl87, wq; 37 of the 88 Miller values committed).

Code comments, docstrings and help

  • Docstrings and comments. Corrected across groth/ and tests/ to match the code: node names, bounds, the witness layout, the binding, the degree cap. Unused Groth16.root is removed; its callers use binding.bound_signal.
  • The command-line tools. All four print their module docstring under --help. groth16_snarkjs.py's per-input cost note is aligned with the measurements.
  • deploy USAGE. It now lists all three bindings and the key flags that address, sighash, spend and keyspend take.
  • tools/regtest.sh.
    • It failed on a node set up exactly as its header says ("No such mempool transaction. Use -txindex"). It now passes the block hash to getrawtransaction.
    • It hands values to Python through argv, not by splicing them into the source.
    • The misspelled anyonecanspend_aremine line is removed from the documented conf.
  • tools/demo.py. It sized the Groth16 powers-of-tau file by constraints + 2. snarkjs needs 2^power > constraints + public inputs + outputs (zkey_new.js), so the demo now uses that. test_tools.py's stub is updated.
  • Metadata. pyproject.toml and circuits/package.json now describe both verifiers. .gitignore adds *.ptau.part and *.secret.

Verification

  • Regtest. Elements 23.3.1, with the node conf exactly as tools/regtest.sh documents it. regtest.sh passed for all three toy circuits: proof spend, rejected redirect, and key path. The README's manual steps 2 to 5 were also run as written, and the spend was accepted.
  • Costs. Re-measured: Groth16 48,646,805 mWU bound (48,632,331 unbound); Noir 55,251,481; fflonk 123,163,559 (SHA-256) and 171,976,506 (Keccak) before padding, with the parts as in docs/fflonk.md.
  • Lint and tests. ruff, mypy, cargo fmt and clippy pass, and so does the full test suite: all layers with hand, plus test_bn254_ref against py_ecc. make example reproduces examples/ byte for byte.
  • Python. Tested on Python 3.10, 3.13 and 3.14.

🤖 Generated with Claude Code

Rewrite the README for both verifiers, all three bindings and both
frontends, with a regtest walkthrough whose commands were run as written.
Re-measure or remove every number in docs/, drop obsolete material (the
old architecture diagram, the "before and after" table, the Liquid
testnet spend of an earlier verifier, a link to a private repository),
and say that the code has not been independently audited.

Fix what the review found besides the text: tools/regtest.sh failed on
a node set up as documented (no txindex), demo.py sized the Groth16
ptau below snarkjs's bound, and examples/fp_mul.py multiplied words it
did not range-check. Docstrings, comments and --help match the code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mjthatch
mjthatch merged commit 68de9f2 into research/audit-2-fixes Oct 27, 2026
19 checks passed
@mjthatch
mjthatch deleted the research/docs-review branch October 27, 2026 08:42
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.

2 participants