Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resurrecting lookup PR: Fix lookup has_zero_entry bug #1567

Merged
merged 6 commits into from
Dec 21, 2023

Conversation

volhovm
Copy link
Member

@volhovm volhovm commented Dec 14, 2023

Fixes a has_zero_entry bug which was present before.

Part of #1489.

Corresponding mina PR: MinaProtocol/mina#14725

@volhovm
Copy link
Member Author

volhovm commented Dec 15, 2023

@dannywillems Moved + modified test_lookup_with_a_table_with_id_zero_but_no_zero_entry so that it catches the has_zero_entry bug that this PR fixes.

Here's a run with the new test but old has_zero_entry showing that the bug is caught successfully:

test tests::lookup::test_lookup_with_a_table_with_id_zero_but_no_zero_entry - should panic ... FAILED

failures:

---- tests::lookup::test_lookup_with_a_table_with_id_zero_but_no_zero_entry stdout ----
Seed: [191, 117, 112, 82, 242, 129, 181, 41, 122, 7, 94, 248, 106, 106, 39, 56, 108, 160, 27, 189, 156, 251, 235, 210, 246, 27, 36, 192, 185, 196, 110, 70]
- time to create prover index: 6s
note: test did not panic as expected

Do you want me to create corresponding mina/o1js PRs for this one?

@volhovm volhovm force-pushed the volhovm/mina14674-resurrect-lookups-pr-chunk1 branch from 308ad78 to bdc7e32 Compare December 15, 2023 16:14
@volhovm
Copy link
Member Author

volhovm commented Dec 15, 2023

Rebased this into develop since it's a hotfix.

@volhovm volhovm force-pushed the volhovm/mina14674-resurrect-lookups-pr-chunk1 branch from bdc7e32 to 8168b4a Compare December 15, 2023 16:32
kimchi/src/circuits/lookup/tables/mod.rs Outdated Show resolved Hide resolved
kimchi/src/precomputed_srs.rs Outdated Show resolved Hide resolved
kimchi/src/prover_index.rs Outdated Show resolved Hide resolved
kimchi/src/circuits/lookup/index.rs Outdated Show resolved Hide resolved
kimchi/src/circuits/lookup/index.rs Outdated Show resolved Hide resolved
@volhovm
Copy link
Member Author

volhovm commented Dec 21, 2023

@mrmr1993 all fixed. Another quick pass?

@mrmr1993 mrmr1993 merged commit 8ca6cb4 into develop Dec 21, 2023
4 checks passed
@mrmr1993 mrmr1993 deleted the volhovm/mina14674-resurrect-lookups-pr-chunk1 branch December 21, 2023 13:59
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