Sinsemilla chip with HashDomain #67

therealyingtong · 2021-04-21T16:34:25Z

Part of #96. This PR only covers SinsemillaHash, not SinsemillaCommit.

The SinsemillaCommit functionality is in #118.

codecov-commenter · 2021-05-03T21:29:02Z

Codecov Report

Merging #67 (6eba421) into ecc-chip (334c190) will increase coverage by 7.06%.
The diff coverage is 84.48%.

@@             Coverage Diff              @@
##           ecc-chip      #67      +/-   ##
============================================
+ Coverage     78.25%   85.32%   +7.06%     
============================================
  Files            36       56      +20     
  Lines          1964     4319    +2355     
============================================
+ Hits           1537     3685    +2148     
- Misses          427      634     +207

Impacted Files	Coverage Δ
src/constants/commit_ivk_r.rs	`100.00% <ø> (ø)`
src/constants/note_commit_r.rs	`100.00% <ø> (ø)`
src/constants/nullifier_k.rs	`100.00% <ø> (ø)`
src/constants/spend_auth_g.rs	`100.00% <ø> (ø)`
src/constants/value_commit_r.rs	`100.00% <ø> (ø)`
src/circuit/gadget/sinsemilla/message.rs	`22.72% <22.72%> (ø)`
src/primitives/redpallas.rs	`64.28% <33.33%> (+1.78%)`	⬆️
src/bundle.rs	`44.76% <45.63%> (+44.76%)`	⬆️
src/address.rs	`66.66% <50.00%> (+66.66%)`	⬆️
src/note/commitment.rs	`76.47% <50.00%> (+76.47%)`	⬆️
... and 86 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96be699...6eba421. Read the comment docs.

This toggles the assignment of q_s2 on the last row of each piece. We assign q_s2 = 2 on the last row of the final piece, and q_s2 = 0 on the last row of other pieces. This allows us to process the final_piece in the main loop together with the other pieces. Co-authored-by: Jack Grigg <jack@electriccoin.co>

hash_piece() is an internal API, which means its caller hash_message() is working in the same region. We rely on the caller to have already assigned each piece's initial x_a at the correct offset before making the call to hash_piece(). Co-authored-by: Jack Grigg <jack@electriccoin.co>

The gates "Secant line" and "Sinsemilla gate" were using the same selectors and could be combined. Co-authored-by: Jack Grigg <jack@electriccoin.co>

Co-authored-by: Daira Hopwood <daira@jacaranda.org> Co-authored-by: Jack Grigg <jack@electriccoin.co>

str4d

utACK. One small suggested refactor; the rest is doc comment fixes.

src/circuit/gadget/sinsemilla/chip/hash_to_point.rs

src/circuit/gadget/sinsemilla/chip.rs

src/circuit/gadget/sinsemilla/chip/hash_to_point.rs

src/circuit/gadget/sinsemilla/message.rs

This allows the MockProver to see the fixed_y_q query as semantically connected to q_sinsemilla1. Co-authored-by: Jack Grigg <jack@electriccoin.co>

str4d

ACK bd08808.

daira · 2021-06-21T10:18:06Z

src/circuit/gadget/sinsemilla/chip.rs

            // lambda2^2 - (x_a_next + x_r + x_a_cur) = 0
            let secant_line =
                lambda_2_cur.clone().square() - (x_a_next.clone() + x_r + x_a_cur.clone());


Technically this is implementing two secant lines, because x_r is itself computed by a secant line. ("Secant" means a line between two distinct points on a curve, extended to infinity in both directions.) I think this naming is fine though.

src/circuit/gadget/sinsemilla/chip.rs

daira

utACK bd08808 with minor comments.

daira · 2021-06-21T10:24:57Z

(Image in #67 (review))

Why does this show some fixed columns being assigned outside a region? (or am I misinterpreting it?)

str4d · 2021-06-21T10:45:37Z

Why does this show some fixed columns being assigned outside a region? (or am I misinterpreting it?)

They aren't. Notice those fixed columns line up with equivalent rectangles in the advice columns. This is just an artifact of non-sequential columns being forced to take a sequence for rendering purposes; I split the same region across several boxes. It's clearer when labels are turned on, but in this case the lables overlapped too much and were distracting. It will be easier with an interactive viewer to indicate which boxes are part of the same region.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

daira

I resolved a remaining minor comment in 8af8447.

str4d

re-ACK 8af8447 (trivial change)

therealyingtong mentioned this pull request Apr 21, 2021

[WIP] Implement Sinsemilla #57

Closed

therealyingtong force-pushed the ecc-chip-config branch 2 times, most recently from f90cee2 to 334c190 Compare April 23, 2021 13:31

therealyingtong force-pushed the sinsemilla-chip-config branch from 7b0155c to 2ad868a Compare April 23, 2021 13:32

therealyingtong changed the base branch from ecc-chip-config to ecc-chip April 23, 2021 13:37

therealyingtong force-pushed the sinsemilla-chip-config branch from 2ad868a to c75bb83 Compare April 26, 2021 07:44

therealyingtong force-pushed the ecc-chip branch from 3a068cf to 4016e7b Compare April 26, 2021 09:02

therealyingtong force-pushed the sinsemilla-chip-config branch 2 times, most recently from 2395853 to a9f51be Compare April 28, 2021 20:45

therealyingtong changed the base branch from ecc-chip to ecc-impl April 28, 2021 20:45

therealyingtong force-pushed the ecc-impl branch from c942bb1 to 46066ae Compare April 29, 2021 12:48

therealyingtong force-pushed the sinsemilla-chip-config branch from a9f51be to c977184 Compare April 29, 2021 12:49

therealyingtong force-pushed the ecc-impl branch from 46066ae to cacf755 Compare April 29, 2021 12:59

therealyingtong force-pushed the sinsemilla-chip-config branch from c977184 to c76db8f Compare April 29, 2021 13:00

therealyingtong force-pushed the ecc-impl branch from cacf755 to 4c032a1 Compare April 29, 2021 14:56

therealyingtong force-pushed the sinsemilla-chip-config branch from c76db8f to 6cca5bc Compare April 29, 2021 14:57

therealyingtong force-pushed the ecc-impl branch from 4c032a1 to 4c6ff3a Compare May 3, 2021 21:16

therealyingtong force-pushed the sinsemilla-chip-config branch from 6cca5bc to b2408ab Compare May 3, 2021 21:17

therealyingtong force-pushed the ecc-impl branch from 4c6ff3a to b77f4b2 Compare May 4, 2021 13:49

therealyingtong force-pushed the sinsemilla-chip-config branch from b2408ab to acc7bd1 Compare May 4, 2021 13:49

therealyingtong changed the title ~~[WIP] Sinsemilla chip~~ Sinsemilla chip May 4, 2021

therealyingtong marked this pull request as ready for review May 4, 2021 13:50

therealyingtong force-pushed the ecc-impl branch from b77f4b2 to 53803b0 Compare May 5, 2021 12:45

therealyingtong force-pushed the sinsemilla-chip-config branch from acc7bd1 to fc36949 Compare May 5, 2021 12:45

therealyingtong force-pushed the ecc-impl branch 2 times, most recently from 6f605a7 to 058e75b Compare May 6, 2021 08:00

therealyingtong force-pushed the sinsemilla-chip-config branch from fc36949 to afcd79a Compare May 6, 2021 08:01

therealyingtong changed the title ~~Sinsemilla chip~~ [WIP] Sinsemilla chip May 11, 2021

therealyingtong marked this pull request as draft May 11, 2021 13:02

therealyingtong and others added 3 commits June 20, 2021 11:00

SinsemillaChip::configure(): Combine and label gates.

744f3d1

The gates "Secant line" and "Sinsemilla gate" were using the same selectors and could be combined. Co-authored-by: Jack Grigg <jack@electriccoin.co>

therealyingtong mentioned this pull request Jun 20, 2021

[MockProver] Consider supporting the assumption that non-zero fixed columns correspond to active gates zcash/halo2#301

Open

Docfixes and cleanups.

002596f

Co-authored-by: Daira Hopwood <daira@jacaranda.org> Co-authored-by: Jack Grigg <jack@electriccoin.co>

therealyingtong force-pushed the sinsemilla-chip-config branch from e58fc69 to 002596f Compare June 20, 2021 04:12

str4d approved these changes Jun 20, 2021

View reviewed changes

Doc comment fixes

5f5238f

This comment has been minimized.

Sign in to view

test: Print layout for Sinsemilla test circuit

a01c2ee

therealyingtong mentioned this pull request Jun 20, 2021

Precompute and store Sinsemilla generators as constants. #124

Closed

SinsemillaChip::configure(): Merge "Initial y_q" gate with main gate

bd08808

This allows the MockProver to see the fixed_y_q query as semantically connected to q_sinsemilla1. Co-authored-by: Jack Grigg <jack@electriccoin.co>

str4d approved these changes Jun 20, 2021

View reviewed changes

daira reviewed Jun 21, 2021

View reviewed changes

src/circuit/gadget/sinsemilla/chip.rs Outdated Show resolved Hide resolved

daira approved these changes Jun 21, 2021

View reviewed changes

r3ld3v modified the milestones: Core Sprint 2021-22, Core Sprint 2021-24 Jun 21, 2021

Rename "Sinsemilla gate" constraint to "y check".

8af8447

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

daira approved these changes Jun 21, 2021

View reviewed changes

str4d approved these changes Jun 21, 2021

View reviewed changes

str4d merged commit 66340e2 into main Jun 22, 2021

str4d deleted the sinsemilla-chip-config branch June 22, 2021 15:21

r3ld3v modified the milestones: Core Sprint 2021-24, Core Sprint 2021-26 Jul 6, 2021

r3ld3v removed the S-committed label Jul 6, 2021

r3ld3v modified the milestones: Core Sprint 2021-26, Core Sprint 2021-28 Jul 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sinsemilla chip with HashDomain #67

Sinsemilla chip with HashDomain #67

therealyingtong commented Apr 21, 2021 •

edited

Loading

codecov-commenter commented May 3, 2021 •

edited

Loading

str4d left a comment

This comment has been minimized.

str4d left a comment

daira Jun 21, 2021 •

edited

Loading

daira left a comment

daira commented Jun 21, 2021 •

edited by str4d

Loading

str4d commented Jun 21, 2021

daira left a comment

str4d left a comment

Sinsemilla chip with HashDomain #67

Sinsemilla chip with HashDomain #67

Conversation

therealyingtong commented Apr 21, 2021 • edited Loading

codecov-commenter commented May 3, 2021 • edited Loading

Codecov Report

str4d left a comment

Choose a reason for hiding this comment

This comment has been minimized.

str4d left a comment

Choose a reason for hiding this comment

daira Jun 21, 2021 • edited Loading

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

daira commented Jun 21, 2021 • edited by str4d Loading

str4d commented Jun 21, 2021

daira left a comment

Choose a reason for hiding this comment

str4d left a comment

Choose a reason for hiding this comment

therealyingtong commented Apr 21, 2021 •

edited

Loading

codecov-commenter commented May 3, 2021 •

edited

Loading

daira Jun 21, 2021 •

edited

Loading

daira commented Jun 21, 2021 •

edited by str4d

Loading