Circuit constraint refinements to reduce proof size #169

str4d · 2021-07-29T13:58:14Z

Per #169 (comment):

Base proof size increases by 64 bytes.
Marginal (per-action) proof size decreases by 160 bytes.

Part of #125.
Part of #168.

This ensures the "LSB check" gate only queries `cur` and `next` rows.

This ensures the Commit^ivk gate only queries `cur` and `next` rows.

codecov-commenter · 2021-07-29T14:33:40Z

Codecov Report

Merging #169 (ee44d2c) into main (8454f86) will increase coverage by 0.26%.
The diff coverage is 89.52%.

@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
+ Coverage   86.65%   86.92%   +0.26%     
==========================================
  Files          64       65       +1     
  Lines        7811     8965    +1154     
==========================================
+ Hits         6769     7793    +1024     
- Misses       1042     1172     +130

Impacted Files	Coverage Δ
src/circuit/gadget/ecc/chip/mul/complete.rs	`83.09% <ø> (ø)`
src/circuit/gadget/poseidon/pow5t3.rs	`91.92% <80.00%> (-1.06%)`	⬇️
src/circuit/gadget/sinsemilla/note_commit.rs	`87.72% <88.52%> (+6.70%)`	⬆️
src/circuit/gadget/ecc/chip/add_incomplete.rs	`94.87% <100.00%> (+0.06%)`	⬆️
src/circuit/gadget/ecc/chip/mul.rs	`85.84% <100.00%> (+0.19%)`	⬆️
src/circuit/gadget/ecc/chip/mul/overflow.rs	`81.81% <100.00%> (ø)`
src/circuit/gadget/ecc/chip/witness_point.rs	`89.18% <100.00%> (ø)`
src/circuit/gadget/sinsemilla/commit_ivk.rs	`80.48% <100.00%> (ø)`
src/address.rs	`60.00% <0.00%> (-30.00%)`	⬇️
src/keys.rs	`82.51% <0.00%> (-10.61%)`	⬇️
... and 20 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 8454f86...ee44d2c. Read the comment docs.

The new regions take up more cells overall, but across fewer columns, and the gates now only query `cur` and `next` rows.

Helps to see why we can't optimise it to remove the `prev` query.

str4d · 2021-07-29T20:07:28Z

Current main:

[src/circuit.rs:1004] measured.proof_size(1) = ProofSize {
    instance: ProofContribution {
        commitments: 0,
        evaluations: 1,
    },
    advice: ProofContribution {
        commitments: 10,
        evaluations: 30,
    },
    fixed: ProofContribution {
        commitments: 0,
        evaluations: 27,
    },
    lookups: ProofContribution {
        commitments: 9,
        evaluations: 15,
    },
    equality: ProofContribution {
        commitments: 3,
        evaluations: 23,
    },
    vanishing: ProofContribution {
        commitments: 9,
        evaluations: 1,
    },
    multiopen: ProofContribution {
        commitments: 1,
        evaluations: 5,
    },
    polycomm: ProofContribution {
        commitments: 23,
        evaluations: 2,
    },
    _marker: PhantomData,
}
1 proof:  5088 bytes
2 proofs: 7520 bytes
3 proofs: 9952 bytes
[src/circuit.rs:1011] measured.marginal_proof_size() = MarginalProofSize {
    instance: ProofContribution {
        commitments: 0,
        evaluations: 1,
    },
    advice: ProofContribution {
        commitments: 10,
        evaluations: 30,
    },
    lookups: ProofContribution {
        commitments: 9,
        evaluations: 15,
    },
    equality: ProofContribution {
        commitments: 3,
        evaluations: 8,
    },
    _marker: PhantomData,
}
Marginal proof size: 2432 bytes
Total gates: 38
Total custom constraint polynomials: 192
Total negations: 396
Total additions: 643
Total multiplications: 750

This PR:

[src/circuit.rs:1004] measured.proof_size(1) = ProofSize {
    instance: ProofContribution {
        commitments: 0,
        evaluations: 1,
    },
    advice: ProofContribution {
        commitments: 10,
        evaluations: 25,
    },
    fixed: ProofContribution {
        commitments: 0,
        evaluations: 29,
    },
    lookups: ProofContribution {
        commitments: 9,
        evaluations: 15,
    },
    equality: ProofContribution {
        commitments: 3,
        evaluations: 23,
    },
    vanishing: ProofContribution {
        commitments: 9,
        evaluations: 1,
    },
    multiopen: ProofContribution {
        commitments: 1,
        evaluations: 5,
    },
    polycomm: ProofContribution {
        commitments: 23,
        evaluations: 2,
    },
    _marker: PhantomData,
}
1 proof:  4992 bytes
2 proofs: 7264 bytes
3 proofs: 9536 bytes
[src/circuit.rs:1011] measured.marginal_proof_size() = MarginalProofSize {
    instance: ProofContribution {
        commitments: 0,
        evaluations: 1,
    },
    advice: ProofContribution {
        commitments: 10,
        evaluations: 25,
    },
    lookups: ProofContribution {
        commitments: 9,
        evaluations: 15,
    },
    equality: ProofContribution {
        commitments: 3,
        evaluations: 8,
    },
    _marker: PhantomData,
}
Marginal proof size: 2272 bytes
Total gates: 54
Total custom constraint polynomials: 192
Total negations: 396
Total additions: 643
Total multiplications: 750

So we add two fixed column evaluations (64 bytes) to the base proof size (due to the many extra selectors from splitting up NoteCommit), in exchange for removing five advice column evaluations (160 bytes) from the marginal proof size.

daira · 2021-07-30T10:49:52Z

Reminder that this needs three reviews, because it's not going to be covered by the QEDIT audit as an effective third reviewer. (I'll review it today.)

src/circuit/gadget/poseidon/pow5t3.rs

daira

utACK on everything except note_commit.rs.

We should ensure that the book has up-to-date layout diagrams for the canonicity checks; then I'll review note_commit.rs against those.

therealyingtong

utACK

src/circuit/gadget/sinsemilla/note_commit.rs

Co-authored-by: ying tong <[email protected]>

src/circuit/gadget/sinsemilla/note_commit.rs

daira · 2021-09-02T11:10:27Z

src/circuit/gadget/sinsemilla/note_commit.rs

+
+            // value = d_2 + (2^8)d_3 + (2^58)e_0
+            let value_check = {
+                let two_pow_8 = pallas::Base::from_u64(1 << 8);


Suggested change

let two_pow_8 = pallas::Base::from_u64(1 << 8);

This is already defined above.

daira · 2021-09-02T11:11:07Z

src/circuit/gadget/sinsemilla/note_commit.rs

+
+            // g1_g2_prime = g_1 + (2^9)g_2 + 2^130 - t_P
+            let g1_g2_prime_check = {
+                let two_pow_9 = two_pow_4 * two_pow_5;


Suggested change

let two_pow_9 = two_pow_4 * two_pow_5;

This is already defined above.

src/circuit/gadget/sinsemilla/note_commit.rs

daira · 2021-09-02T11:22:48Z

src/circuit/gadget/sinsemilla/note_commit.rs

+            let canonicity_checks = std::iter::empty()
+                .chain(Some(("h_1 = 1 => h_0", h_0)))
+                .chain(Some(("h_1 = 1 => z13_g", z13_g)))
+                .chain(Some(("h_1 = 1 => z13_g1_g2_prime", z13_g1_g2_prime)))


The book was incorrect for this constraint (using g₀ in place of the correct h₁, which appears to be a cut-and-paste error from the previous section). Fixed in 4c25e3c .

Signed-off-by: Daira Hopwood <[email protected]>

src/circuit/gadget/sinsemilla/note_commit.rs

daira

utACK. Note that I applied my own suggested changes, so my review doesn't count for the last 4 commits. Checked for consistency with #182.

str4d · 2021-09-14T00:19:46Z

src/circuit/gadget/sinsemilla/commit_ivk.rs

+            |   ak  |   a   |   b   |  b_0  |  b_1  |  b_2  | z13_a |  a_prime  |   z13_a_prime  |       1      |
+            |   nk  |   c   |   d   |  d_0  |  d_1  |       | z13_c | b2_c_prime| z14_b2_c_prime |       0      |


Note to self: update the region layout in the book with this change.

Matches the change made in #169.

str4d added 3 commits July 29, 2021 14:56

Add names to some nameless constraints

16e9076

circuit: Rotateq_mul_lsb selector up by one row

0009070

This ensures the "LSB check" gate only queries `cur` and `next` rows.

circuit: Rotate q_commit_ivk selector up by one row

2198675

This ensures the Commit^ivk gate only queries `cur` and `next` rows.

str4d added this to the Core Sprint 2021-30 milestone Jul 29, 2021

str4d added 2 commits July 29, 2021 20:13

circuit: Refactor NoteCommit input processing into multiple regions

6aa85fc

The new regions take up more cells overall, but across fewer columns, and the gates now only query `cur` and `next` rows.

circuit: Add region layout diagrams for y_switch constraint

9af22a8

Helps to see why we can't optimise it to remove the `prev` query.

str4d marked this pull request as ready for review July 29, 2021 20:07

str4d changed the title ~~Circuit constraint refinements~~ Circuit constraint refinements to reduce proof size Jul 29, 2021