Backporting changes from 0.5.0 to 0.3.0#898
Open
dannywillems wants to merge 9 commits into
Open
Conversation
The incomplete double-and-add loop in `ecc::chip::mul` kept the per-iteration base `(x_p, y_p)` constant across loop rows via `q_mul_2`, but never tied it to the real base: the coordinates were written with `assign_advice`, and the constancy chain reached neither the doubling-row nor the complete-addition base anchors. A prover could therefore run the incomplete loop against a free constant `B' != base`, making the gadget output `[a] base + [b] B'` rather than `[scalar] base`. Anchor the base by `copy_advice`-ing it into the first incomplete row; `q_mul_2` then propagates the equality to every loop row. The `hi` and `lo` halves share the `x_p`/`y_p` columns and run on the same rows, so the single anchor covers both. The fix changes the verifying key, so introduce `CircuitVersion` (`AnchoredBase` / `InsecureUnanchoredBase`): one binary can build both the fixed VK and the prior unanchored VK, the latter only to verify proofs created before the fix. `EccChip::construct` now takes the version explicitly. Add a regression test that drives the real `mul::Config` synthesis through a copy-recording `Assignment`: the fixed circuit's equality constraints are a superset of the prior version's, and the only additions are the two base anchors at the first incomplete-addition row. Rename the stored ECC-chip vk/proof fixtures to `*_insecure` and check them against the `InsecureUnanchoredBase` circuit, which reproduces the historical (deployed) verifying key exactly and still verifies the pre-fix proof under it — so a node can sync from before the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dannywillems
changed the title
Pin the verifying key and a proof for the fixed (anchored) ECC-chip circuit as `*_fixed`, checked by `test_ecc_chip_fixed_against_stored_circuit` and `test_against_stored_ecc_chip_4_5b_fixed`. With the `*_insecure` fixtures from the previous commit this covers both directions: a proof for the fixed circuit verifies under the new verifying key, and the deployed pre-fix proof verifies under the old one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dannywillems
force-pushed
the
dw/halo2_gadgets-0.3.1
branch
from
d26fad6 to
b92d646
Compare
Before this commit, anyone naively building the code would face various issues like, amonst others: - dependencies being yanked - a different Rust edition being required This is caused by the branch halo2_gadgets-0.3 being inactive since ~2023, and the Rust ecosystem evolves a lot since then. A solution can be to pin to precise versions. However, it might be better to copy the state of `main`, as we might need to cherry-pick more fixes later. Decreasing the divergence between `main` and `halo2_gadgets-0.3` seems to me a more maintainable solution on the long term.
The lookup variants called 4_5B has been added post-0.3.0.
Running ```bash CIRCUIT_TEST_GENERATE_NEW_DATA=1 cargo test "test_ecc_chip_insecure_against_stored_circuit" \ --release \ -p halo2_gadgets ```
Running: ```bash CIRCUIT_TEST_GENERATE_NEW_DATA=1 cargo test "test_ecc_chip_fixed_against_stored_circuit" \ --release \ -p halo2_gadgets ```
Appeasing ```bash cargo +stable tarpaulin \ --no-default-features \ --features batch dev-graph gadget-traces \ nightly test-dependencies \ --timeout 600 \ --out Xml ```
dannywillems
force-pushed
the
dw/halo2_gadgets-0.3.1
branch
from
b92d646 to
3553b49
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reviewers are encouraged to review commit by commit.
Changes have been cherry-picked from
halo2_gadgets-0.5.0, slightly modified to match the state of0.3.0(removing4_5blookup variants and generated test artifacts like the proof and vk).The proof and vk added in a1b7bfd could be deleted from the commit to have a cleaner history, where proof and vk are generated by 2b5f92b.