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

[feat] avoid double meta.query to same cells #190

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

jonathanpwang
Copy link
Contributor

Avoid calling meta.query_* multiple times on is_paddings when you can do it once and then clone.

@jonathanpwang jonathanpwang merged commit e25a4cb into feat/zkevm-sha256 Oct 19, 2023
@jonathanpwang jonathanpwang deleted the feat/remove_double_query branch October 19, 2023 03:46
jonathanpwang added a commit that referenced this pull request Dec 4, 2023
* feat(zkevm-sha256): Initial commit from Brechtpd/zkevm-circuits@ef90cf0

Copied `sha256_bit` from Brecht's repo

* chore: rename crate zkevm-keccak to zkevm-hashes

* fix: add `input_len` back to `KeccakTable`

* chore: move keccak specific constants to `keccak_packed_multi/util`

* feat: SHA-256 circuit with 2-phase challenge passes MockProver

* feat(halo2-base): add `GateChip::pow_var` (#103)

* make ShaTable public

* make more sha stuff public

* Use halo2curves v0.4.0 and ff v0.13 (#107)

* wip: change import to ff v0.13

* feat: remove `GateInstructions::get_field_element`

halo2curves now has `bn256-table` which creates table of small field
elements at compile time, so we should just use `F::from` always. This
also improves readability.

* chore: fix syntax and imports after update

* chore: add asm feature

* chore: workspace.resolver = 2

* chore: update ethers-core

* chore: add jemallocator feature to zkevm-keccak crate

* test: add bigger test case to keccak prover

* feat: use `configure_with_params`

remove `thread_local!` usage

* chore: bump zkevm-keccak version to 0.1.1

* feat: add `GateThreadBuilder::from_stage` for convenience

* chore: fixes

* fix: removed `lookup_bits` from `GateThreadBuilder::config`

* fix: debug_assert_false should load witness for debugging

* chore: use unreachable to document that Circuit::configure is never used

* chore: fix comment

* feat(keccak): use configure_with_params

* chore: fix halo2-pse errors

* chore: doc comments and folder restructure

* chore(zkevm_hashes): Bump version to v0.2.0

* feat(wip): more folder restructuring

- Move `Sha256CircuitConfig` to `columns.rs`
- Move constants to `param.rs`
- Rename `witness_gen.rs` to `witness.rs`

* feat(sha256): removed RLC from circuit

* feat(sha256): add real prover test

* feat(sha256): more tests

* feat: add readme

* fix: compatibility with halo2-pse

* fix: remove unnecessary `is_final` in `length` update (#166)

* chore: remove use of `Box::leak` for string concat (#167)

* feat: move `q_enable` to `ShaTable` (#168)

* [fix] typo in comment: 32 bytes -> 32 bits (#185)

fix: typo in comment: 32 bytes -> 32 bits

* [feat] Add comment in readme about circuit input limit (#186)

feat: add note in readme about input size limit

* fix: more byte -> bit typos (#187)

* [chore] change gate annotation for better debugging (#188)

chore: change gate annotation for better debugging

* [feat] rename `d_64, h_64` to `d_68, h_68` (#189)

feat: rename `d_64, h_64` to `d_68, h_68`

* [feat] avoid double `meta.query` to same cells (#190)

* feat: avoid double `meta.query` to same cells

* chore: fix fmt (cargo fmt isn't working)

* [chore] use constant instead of hardcoded number (#191)

chore: use constant instead of hardcoded number

* nit: `Rotation(-0)` to `Rotation::cur()` (#192)

* feat: constrain all unused cells to be zero (#193)

For extra security, we constrain every unused cell in the circuit
explicitly to be zero. This also serves as a nice exposition of all the
unused cells in the circuit.

* [feat] reduce num columns (#194)

* feat: combine `word_value`, `output` (hi-lo) columns into one

Previously: Proving time for 14562 SHA256 blocks: 91.113416291s
test sha256::vanilla::tests::bit_sha256_prover::k_20 ... ok

Now: Proving time for 14562 SHA256 blocks: 88.943400583s
test sha256::vanilla::tests::bit_sha256_prover::k_20 ... ok

* feat: remove `is_enabled` from `ShaTable`

It seems extraneous since we have `is_final` and `q_squeeze` is fixed.

* chore: move `is_final` to `ShaTable` (#200)

since it is part of the overall input recovery data
jonathanpwang added a commit that referenced this pull request Jan 18, 2024
* feat(zkevm-sha256): Initial commit from Brechtpd/zkevm-circuits@ef90cf0

Copied `sha256_bit` from Brecht's repo

* chore: rename crate zkevm-keccak to zkevm-hashes

* fix: add `input_len` back to `KeccakTable`

* chore: move keccak specific constants to `keccak_packed_multi/util`

* feat: SHA-256 circuit with 2-phase challenge passes MockProver

* feat(halo2-base): add `GateChip::pow_var` (#103)

* make ShaTable public

* make more sha stuff public

* Use halo2curves v0.4.0 and ff v0.13 (#107)

* wip: change import to ff v0.13

* feat: remove `GateInstructions::get_field_element`

halo2curves now has `bn256-table` which creates table of small field
elements at compile time, so we should just use `F::from` always. This
also improves readability.

* chore: fix syntax and imports after update

* chore: add asm feature

* chore: workspace.resolver = 2

* chore: update ethers-core

* chore: add jemallocator feature to zkevm-keccak crate

* test: add bigger test case to keccak prover

* feat: use `configure_with_params`

remove `thread_local!` usage

* chore: bump zkevm-keccak version to 0.1.1

* feat: add `GateThreadBuilder::from_stage` for convenience

* chore: fixes

* fix: removed `lookup_bits` from `GateThreadBuilder::config`

* fix: debug_assert_false should load witness for debugging

* chore: use unreachable to document that Circuit::configure is never used

* chore: fix comment

* feat(keccak): use configure_with_params

* chore: fix halo2-pse errors

* chore: doc comments and folder restructure

* chore(zkevm_hashes): Bump version to v0.2.0

* feat(wip): more folder restructuring

- Move `Sha256CircuitConfig` to `columns.rs`
- Move constants to `param.rs`
- Rename `witness_gen.rs` to `witness.rs`

* feat(sha256): removed RLC from circuit

* feat(sha256): add real prover test

* feat(sha256): more tests

* feat: add readme

* fix: compatibility with halo2-pse

* fix: remove unnecessary `is_final` in `length` update (#166)

* chore: remove use of `Box::leak` for string concat (#167)

* feat: move `q_enable` to `ShaTable` (#168)

* [fix] typo in comment: 32 bytes -> 32 bits (#185)

fix: typo in comment: 32 bytes -> 32 bits

* [feat] Add comment in readme about circuit input limit (#186)

feat: add note in readme about input size limit

* fix: more byte -> bit typos (#187)

* [chore] change gate annotation for better debugging (#188)

chore: change gate annotation for better debugging

* [feat] rename `d_64, h_64` to `d_68, h_68` (#189)

feat: rename `d_64, h_64` to `d_68, h_68`

* [feat] avoid double `meta.query` to same cells (#190)

* feat: avoid double `meta.query` to same cells

* chore: fix fmt (cargo fmt isn't working)

* [chore] use constant instead of hardcoded number (#191)

chore: use constant instead of hardcoded number

* nit: `Rotation(-0)` to `Rotation::cur()` (#192)

* feat: constrain all unused cells to be zero (#193)

For extra security, we constrain every unused cell in the circuit
explicitly to be zero. This also serves as a nice exposition of all the
unused cells in the circuit.

* [feat] reduce num columns (#194)

* feat: combine `word_value`, `output` (hi-lo) columns into one

Previously: Proving time for 14562 SHA256 blocks: 91.113416291s
test sha256::vanilla::tests::bit_sha256_prover::k_20 ... ok

Now: Proving time for 14562 SHA256 blocks: 88.943400583s
test sha256::vanilla::tests::bit_sha256_prover::k_20 ... ok

* feat: remove `is_enabled` from `ShaTable`

It seems extraneous since we have `is_final` and `q_squeeze` is fixed.

* chore: move `is_final` to `ShaTable` (#200)

since it is part of the overall input recovery data
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.

1 participant