Conversation
Update circuit description.
Allow passing `CellValue` as input to Poseidon gadget.
Codecov Report
@@ Coverage Diff @@
## main #237 +/- ##
==========================================
- Coverage 90.62% 89.47% -1.16%
==========================================
Files 66 67 +1
Lines 7830 7989 +159
==========================================
+ Hits 7096 7148 +52
- Misses 734 841 +107
Continue to review full report at Codecov.
|
Co-authored-by: str4d <jack@electriccoin.co>
Co-authored-by: str4d <jack@electriccoin.co>
Generalise Poseidon gadget over `WIDTH`, `RATE`
Migrate to latest halo2 revision
…iable-base scalar mul.
circuit: Use `N - x` instead of `x - N`.
This was referenced Nov 30, 2021
Co-authored-by: Jack Grigg <jack@electriccoin.co>
Co-authored-by: str4d <jack@electriccoin.co>
Benchmark Poseidon gadget for rates {2, 8, 11}
The ECC test chip performs various checks that assume the chip will only be synthesized with witnesses. This assumption is broken by the chip printer test, so we fix the assumption here.
Migrate to halo2 version with `AssignedCell`
The `Sponge` struct's API correctly enforces the properties of a sponge: it can absorb an arbitrary number of elements, and then squeeze an arbitrary number of elements, but cannot absorb after it has squeezed. Co-authored-by: ying tong <yingtong@z.cash>
For almost all the sponge constructions defined in the Poseidon paper, the domain can be defined completely statically. Variable-length hashing requires knowledge of the message length, but that can be provided to the fixed padding function in a subsequent commit, and in any case we can't use variable-length inputs in a circuit.
This exposes a bug in the way padding was being handled by the invalid sponge-duplex hybrid construction.
Sponge constructions pad the entire input message and then split it into rate-sized chunks. The previous implementation was using an incorrect duplex-like hybrid where padding was applied to each chunked input. We now use an enum to distinguish message and padding words being absorbed into the sponge. This also fixes two previous bugs: - If a `ConstantLength` hash had a length greater than the permutation's rate but not a multiple of it, no padding would be generated and the circuit would fail to create proofs. - If a sponge usage required more output than the permutation's rate, the squeeze-side permutations would in some cases incorrectly apply padding, when it should instead use the prior state as-is. We now add zeroes instead. This change doesn't alter the Orchard circuit, because it doesn't need any padding cells, only takes a single field element as output, and padding is still assigned in the same region as before.
In the previous commit, we fixed a bug where padding was being added to the state when the sponge was in squeezing mode. But there's no need to assign a circuit region in which we add constant zeroes to the state :)
`M` was at one point only used as a type marker, but now it stores per-mode state. Co-authored-by: ying tong <yingtong@z.cash>
Also fixes some clippy lints (public docs linking to private items). Co-authored-by: Daira Hopwood <daira@jacaranda.org>
A sponge can only have two modes: absorbing, and squeezing.
Rewrite Poseidon implementation
`PrimeField::from_repr` explicitly leaves the endianness opaque. We therefore can't use it in places we were using `FieldExt::from_bytes` (which was specifically little-endian) generically, but the previous commit replaced it everywhere. We now handle generic contexts on a case-by-case basis: - Where we needed to convert bitstrings into field elements, we now use double-and-add on the field elements directly instead of on bytes. This is less efficient, but visible correct (and a future change to the `ff` crate APIs could enable the more efficient version). - `INV_TWO_POW_K`, which is pre-computed for `pallas::Base`, was being incorrectly used in a field-generic circuit. We now compute it live. - `test_zs_and_us` was only used in tests, and hard-coded a field element encoding length of 32 bytes. It now uses Pallas concretely.
Also fixes some incorrect code comments. Closes #263.
Remove various usages of `FieldExt` methods
Contributor
Author
|
|
Contributor
Author
|
I confirmed locally that this branch only contains merge commits. So there's new to review in this PR, beyond what was already reviewed in the PRs themselves. |
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
4 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.
This is the feature branch for the proposed mainnet version of the Orchard Action circuit.
Closes #231.