Use record IDs to index proof batches #1350

andyleiserson · 2024-10-15T19:18:57Z

Since we are not guaranteed that all proofs have the same size, I serialize the proof values into fixed-size structures and use one record ID per proof, rather than doing record ID arithmetic, except for PRSS.

Fixes #1269

codecov · 2024-10-15T19:57:29Z

Codecov Report

Attention: Patch coverage is 98.06452% with 6 lines in your changes missing coverage. Please review.

Project coverage is 93.49%. Comparing base (da336af) to head (6bb16e1).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...rc/protocol/ipa_prf/malicious_security/lagrange.rs	0.00%	3 Missing ⚠️
...ol/ipa_prf/validation_protocol/proof_generation.rs	95.65%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1350      +/-   ##
==========================================
- Coverage   93.51%   93.49%   -0.03%     
==========================================
  Files         210      210              
  Lines       35032    35211     +179     
==========================================
+ Hits        32759    32919     +160     
- Misses       2273     2292      +19

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

andyleiserson · 2024-10-15T20:28:40Z

Draft, 5M: https://draft-mpc.vercel.app/query/view/sharp-mover2024-10-15T2018 (failed)

andyleiserson · 2024-10-15T22:41:02Z

ipa-core/src/protocol/context/dzkp_validator.rs

+// $$ depth >= log_8 target_proof_size $$
+// Because multiplication intermediate storage gets rounded up to blocks of 256, leaving
+// some margin is advised.
+pub const MAX_PROOF_RECURSION: usize = 9;


Recursion depth of 9 corresponds to TARGET_PROOF_SIZE of about 117M. Without quicksort batching, this is not quite enough. So either we can wait for quicksort batching, or we can temporarily increase MAX_PROOF_RECURSION slightly. I lean towards the former unless we identify an urgency to take this change.

Fixes private-attribution#1311

Fixes private-attribution#1269

akoshelev · 2024-10-17T05:37:15Z

ipa-core/src/protocol/ipa_prf/validation_protocol/proof_generation.rs

+        //  * We need (SRF - 1) at the last level to have room for the masks.
+        let max_uv_values: usize =
+            (SRF - 1) * SRF.pow(u32::try_from(MAX_PROOF_RECURSION - 2).unwrap());
+        tracing::info!("uv_values.len() = {}", uv_values.len());


I just wanted to see the values in the draft run, I am going to remove this.

akoshelev · 2024-10-17T05:51:53Z

ipa-core/src/protocol/ipa_prf/validation_protocol/proof_generation.rs

+            .map(|buf| Fp61BitPrime::deserialize(buf.try_into().unwrap()))
+            .collect::<Result<Vec<_>, _>>()?
+            .try_into()
+            .unwrap())


oh this is neat, I didn't know you can go from Result<Vec<_>, _> to Result<Box<_>, _>, I guess because there exists blanket From<Vec<T>> for Box<T>

akoshelev · 2024-10-17T05:59:45Z

ipa-core/src/protocol/mod.rs

+pub struct RecordIdRange(Range<RecordId>);
+
+impl RecordIdRange {
+    pub const ALL: RecordIdRange = RecordIdRange(RecordId(0)..RecordId(u32::MAX));


would it be better to add LAST to RecordId and use FIRST..LAST here?

akoshelev · 2024-10-17T06:04:33Z

ipa-core/src/protocol/ipa_prf/validation_protocol/proof_generation.rs

-                (0..length)
-                    .map(|i| async move { receive_channel_right.receive(RecordId::from(i)).await }),
-            )
+            .set_total_records(TotalRecords::Indeterminate)


oh, this is much better than parallel join, can't wait until I see the impact of this change

I don't expect that this is big enough contributor that the difference will show up in overall runtime, but I do think that getting rid of the parallel join more than offsets any cost of space overhead in the fixed-size structures.

akoshelev · 2024-10-17T06:04:46Z

ipa-core/src/protocol/ipa_prf/validation_protocol/proof_generation.rs

+    pub async fn receive_from_right<C>(
+        ctx: &C,
+        record_id: RecordId,
+        length: usize,


How often do we need to shrink?

Since many of our batches end up being smaller than the target, reasonably often... but since we do this only once per proof, it doesn't seem worth a lot of effort to optimize.

akoshelev · 2024-10-17T06:07:18Z

ipa-core/src/protocol/ipa_prf/validation_protocol/proof_generation.rs

        // Prover `P_i` and verifier `P_{i+1}` both compute q(x)
        // therefore the "left" share computed by this verifier corresponds to that which
        // was used by the prover to the left.
-        let (q_mask_from_left_prover, my_q_mask) = ctx.prss().generate_fields(record_counter);
-        record_counter += 1;
+        let (q_mask_from_left_prover, my_q_mask) =


record_id_range.next().unwrap() is dominantly used everywhere it seems. I think we should either implement a helper function to make it look less ugly at callsite and potentially not implement the iterator trait for it as we don't have (unless I missed it) for x in range use case

Thoughts on what to call the helper? Maybe expect_next()?

andyleiserson · 2024-10-18T00:57:18Z

Draft run, 25M: https://draft-mpc.vercel.app/query/view/flaky-stern2024-10-17T2136. 3 hr 16 min, slightly slower than the run in #1355, which was 3 hr 4 min. Not sure if the difference is real or just variation.

akoshelev · 2024-10-18T06:01:52Z

Not sure if the difference is real or just variation.

seems more than just variation to me, but we could try a few runs.

andyleiserson commented Oct 15, 2024

View reviewed changes

andyleiserson marked this pull request as draft October 16, 2024 18:04

andyleiserson force-pushed the dzkp-record-id branch from 55baedb to 2eac4f2 Compare October 16, 2024 18:04

andyleiserson mentioned this pull request Oct 16, 2024

Proof batching for quicksort #1352

Closed

andyleiserson added 5 commits October 16, 2024 14:07

Proof batching for quicksort

683d01c

Fixes private-attribution#1311

PR feedback from private-attribution#1352

a1fa1a6

Use record IDs to index proof batches

e6c79e4

Fixes private-attribution#1269

Fix conditional compilation issue

9e7c6d7

Check max proof size

ee19b93

andyleiserson force-pushed the dzkp-record-id branch from 2eac4f2 to b3bd7eb Compare October 16, 2024 21:07

Revise comment

22a1c22

akoshelev reviewed Oct 17, 2024

View reviewed changes

andyleiserson marked this pull request as ready for review October 17, 2024 21:26

andyleiserson force-pushed the dzkp-record-id branch from b3bd7eb to 22a1c22 Compare October 17, 2024 21:27

PR feedback, and rename record_counter

6bb16e1

akoshelev approved these changes Oct 18, 2024

View reviewed changes

andyleiserson linked an issue Oct 18, 2024 that may be closed by this pull request

Add proper DZKP batching to quicksort #1311

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use record IDs to index proof batches #1350

Use record IDs to index proof batches #1350

andyleiserson commented Oct 15, 2024

codecov bot commented Oct 15, 2024 •

edited

Loading

andyleiserson commented Oct 15, 2024 •

edited

Loading

andyleiserson Oct 15, 2024 •

edited

Loading

akoshelev Oct 17, 2024

andyleiserson Oct 17, 2024

akoshelev Oct 17, 2024

akoshelev Oct 17, 2024

akoshelev Oct 17, 2024

andyleiserson Oct 17, 2024

akoshelev Oct 17, 2024

andyleiserson Oct 17, 2024

akoshelev Oct 17, 2024

andyleiserson Oct 17, 2024

andyleiserson commented Oct 18, 2024

akoshelev commented Oct 18, 2024 •

edited

Loading

Use record IDs to index proof batches #1350

Are you sure you want to change the base?

Use record IDs to index proof batches #1350

Conversation

andyleiserson commented Oct 15, 2024

codecov bot commented Oct 15, 2024 • edited Loading

Codecov Report

andyleiserson commented Oct 15, 2024 • edited Loading

andyleiserson Oct 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyleiserson commented Oct 18, 2024

akoshelev commented Oct 18, 2024 • edited Loading

codecov bot commented Oct 15, 2024 •

edited

Loading

andyleiserson commented Oct 15, 2024 •

edited

Loading

andyleiserson Oct 15, 2024 •

edited

Loading

akoshelev commented Oct 18, 2024 •

edited

Loading