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

Implement NIVC Coprocessors. #677

Merged
merged 3 commits into from
Sep 19, 2023
Merged

Implement NIVC Coprocessors. #677

merged 3 commits into from
Sep 19, 2023

Conversation

porcuquine
Copy link
Collaborator

@porcuquine porcuquine commented Sep 18, 2023

This PR addresses #637, though some details differ from the precise plan described there.

The current approach demonstrates that we can indeed use a coprocessor defined for IVC and the existing framework, then deploy it to NIVC via a new SuperNova prover type. Because the underlying backend is incomplete and its interfaces not quite worked out, this leaves some gaps. We need to fill in those gaps and massage the result to make switching between NIVC and IVC as smooth as possible, but we should not try to do all that now. Rather, we should incorporate the current work quickly then focus on incremental improvements. I will itemize some of these in a separate issue.

The code here is quite focused and does little more than is strictly required to make this work. The main change that needs to be noted is that in order to keep each coprocessor circuit simple, I made a slight change to the coprocessor contract. Previously, the coprocessor circuit was nestled into reduction such that it could return its evaluated argument directly to the ApplyContinuation phase. However, this does not allow for clean division of circuits.

Now coprocessors return an unevaluated argument and a tail continuation. This is conceptually simple, but does increase the cycle count, as well as the size of each coprocessor circuit. Although not optimal, this has the benefit of being understandable and simple enough to fully implement in a way that yields a consistent design.

Alternative approaches to improving this situation can be considered in a separate issue.

@porcuquine
Copy link
Collaborator Author

CI failure should (🤞 ) be fixed once lurk-lab/arecibo#50 merges — though CI will need to be re-triggered to notice.

@porcuquine porcuquine force-pushed the nivc-coprocessors branch 3 times, most recently from a0b1024 to 2312def Compare September 18, 2023 04:38
@arthurpaulino
Copy link
Member

#678 implements some simplifications in Lang

arthurpaulino
arthurpaulino previously approved these changes Sep 18, 2023
Copy link
Member

@arthurpaulino arthurpaulino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on board with these changes, except for supernova.rs for which I don't have enough context

@porcuquine porcuquine mentioned this pull request Sep 18, 2023
@porcuquine
Copy link
Collaborator Author

Ideally, someone could work on #683 before we merge this PR.

@porcuquine
Copy link
Collaborator Author

Meanwhile, here's the outcome of running the sha256_nivc example:

➜  lurk-rs git:(nivc-coprocessors) cargo run --release --example sha256_nivc
    Finished release [optimized] target(s) in 0.50s
     Running `target/release/examples/sha256_nivc`
Beginning proof+public-parameters step...
Proofs took 8.90511775s
Congratulations! You proved a dynamic SHA256 encoding in 8.905117792s time!

arthurpaulino
arthurpaulino previously approved these changes Sep 18, 2023
@arthurpaulino arthurpaulino linked an issue Sep 18, 2023 that may be closed by this pull request
Copy link
Member

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plenty of inline comments, but nothing blocking !

Most important comments are about padding I/O and verification. Thanks a bunch, this is a nice step forward!

Comment on lines +125 to +127
// we need this count, even though folding_config contains reduction_count
// because it might be overridden in the case of a coprocoessor, which should have rc=1.
// This is not ideal and might not *actually* be needed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open an issue.

src/circuit/circuit_frame.rs Show resolved Hide resolved
src/circuit/circuit_frame.rs Show resolved Hide resolved
src/circuit/circuit_frame.rs Show resolved Hide resolved
let mut bytes = hasher.finalize();
bytes.reverse();
let l = bytes.len();
// Discard the two most significant bits.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to leave a TODO & an issue here, as this will need revisiting when we are dealing with a different field.

Throughout the code base, unless I missed it, we probably want to write a map-to-field that's a tad more rigorous and systematic (i.e. works through modular reduction when we don't care about collision, sensible truncation valid for any field passed as parameter when we do).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although you make a reasonable point, I don't think it's especially relevant here in particular. Here sha256 is just being used as an 'expensive circuit', and the example itself doesn't pretend to be meaningful in any way. I do agree that a toolkit and strategies for writing applications that may want to do 'strange things' might be useful. However, I don't think it's core either to Lurk or to this NIVC coprocessors example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, not essential for the sake of this particular example.

Comment on lines +204 to +210
// println!("Verifying proof...");

// let verify_start = Instant::now();
// let res = proof.verify(claim, &z0, &zi).unwrap();
// let verify_end = verify_start.elapsed();

// println!("Verify took {:?}", verify_end);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it would be really great to have this as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would. It's not entirely obvious what the right interface for this will be. Whereas a Nova RecursiveSnark contains the z_i needed to verify itself, a SuperNova RecursiveSnark does not. It needs to also be provided with a RunningClaim. In order to provide an interface as much like Nova's as possible, it may be necessary to refactor SuperNova in arecibo. @mpenciak is looking into this. I'll create an issue to track this understanding, and if we end up deciding to hack around it in Lurk, we can. For now, I assume the right thing is to reshape the upstream interfaces.

src/circuit/circuit_frame.rs Show resolved Hide resolved
src/circuit/circuit_frame.rs Show resolved Hide resolved
src/proof/supernova.rs Show resolved Hide resolved
src/proof/supernova.rs Show resolved Hide resolved
@porcuquine porcuquine added this pull request to the merge queue Sep 19, 2023
Merged via the queue into master with commit 2a2d555 Sep 19, 2023
8 checks passed
@porcuquine porcuquine deleted the nivc-coprocessors branch September 19, 2023 16:56
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 20, 2023
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 20, 2023
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 20, 2023
github-merge-queue bot pushed a commit that referenced this pull request Sep 20, 2023
That PR was on auto-merge.
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 27, 2023
- uses the generic MultiFrame trait introduced in the prior commit to generically prove using nova,
- For references on the latest work defining this trait, see lurk-lab#629 (and its history, incl. lurk-lab#642, lurk-lab#707) and lurk-lab#677.

- make the benches use the generic trait, so LEM can bench similarly,
- make the examples use the generic trait, so LEM can example similarly,
- make Supernova use the generic trait, so LEM can NIVC similarly,
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 27, 2023
- uses the generic MultiFrame trait introduced in the prior commit to generically prove using nova,
- For references on the latest work defining this trait, see lurk-lab#629 (and its history, incl. lurk-lab#642, lurk-lab#707) and lurk-lab#677.

- make the benches use the generic trait, so LEM can bench similarly,
- make the examples use the generic trait, so LEM can example similarly,
- make Supernova use the generic trait, so LEM can NIVC similarly,
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 27, 2023
- uses the generic MultiFrame trait introduced in the prior commit to generically prove using nova,
- For references on the latest work defining this trait, see lurk-lab#629 (and its history, incl. lurk-lab#642, lurk-lab#707) and lurk-lab#677.

- make the benches use the generic trait, so LEM can bench similarly,
- make the examples use the generic trait, so LEM can example similarly,
- make Supernova use the generic trait, so LEM can NIVC similarly,
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 27, 2023
- uses the generic MultiFrame trait introduced in the prior commit to generically prove using nova,
- For references on the latest work defining this trait, see lurk-lab#629 (and its history, incl. lurk-lab#642, lurk-lab#707) and lurk-lab#677.

- make the benches use the generic trait, so LEM can bench similarly,
- make the examples use the generic trait, so LEM can example similarly,
- make Supernova use the generic trait, so LEM can NIVC similarly,
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 27, 2023
- uses the generic MultiFrame trait introduced in the prior commit to generically prove using nova,
- For references on the latest work defining this trait, see lurk-lab#629 (and its history, incl. lurk-lab#642, lurk-lab#707) and lurk-lab#677.

- make the benches use the generic trait, so LEM can bench similarly,
- make the examples use the generic trait, so LEM can example similarly,
- make Supernova use the generic trait, so LEM can NIVC similarly,
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 27, 2023
- uses the generic MultiFrame trait introduced in the prior commit to generically prove using nova,
- For references on the latest work defining this trait, see lurk-lab#629 (and its history, incl. lurk-lab#642, lurk-lab#707) and lurk-lab#677.

- make the benches use the generic trait, so LEM can bench similarly,
- make the examples use the generic trait, so LEM can example similarly,
- make Supernova use the generic trait, so LEM can NIVC similarly,
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 27, 2023
- uses the generic MultiFrame trait introduced in the prior commit to generically prove using nova,
- For references on the latest work defining this trait, see lurk-lab#629 (and its history, incl. lurk-lab#642, lurk-lab#707) and lurk-lab#677.

- make the benches use the generic trait, so LEM can bench similarly,
- make the examples use the generic trait, so LEM can example similarly,
- make Supernova use the generic trait, so LEM can NIVC similarly,
huitseeker added a commit to huitseeker/lurk-rs that referenced this pull request Sep 27, 2023
- uses the generic MultiFrame trait introduced in the prior commit to generically prove using nova,
- For references on the latest work defining this trait, see lurk-lab#629 (and its history, incl. lurk-lab#642, lurk-lab#707) and lurk-lab#677.

- make the benches use the generic trait, so LEM can bench similarly,
- make the examples use the generic trait, so LEM can example similarly,
- make Supernova use the generic trait, so LEM can NIVC similarly,
huitseeker added a commit that referenced this pull request Sep 28, 2023
* feat: Introduce MultiFrame trait

Mutability leftovers (see #680)

- Updated benchmarking scripts to no longer mutate the `store` variable during evaluation frames retrieval.
- Reformed `get_evaluation_frames` method across several files to use immutable references to `store` instead of mutable references.

Multiframe definition

- Introduced several new traits - `CEKState`, `FrameLike`, `EvaluationStore` and `MultiFrameTrait` to manage complex evaluation and circuit proof operations in the `proof/mod.rs` file.
- Detailed implementations of the above traits introduced in `circuit/circuit_frame.rs`, providing methods for handling evaluation frames, synthesizing data from multiple frames and more.

* refactor: make nova proofs use the generic trait

- uses the generic MultiFrame trait introduced in the prior commit to generically prove using nova,
- For references on the latest work defining this trait, see #629 (and its history, incl. #642, #707) and #677.

- make the benches use the generic trait, so LEM can bench similarly,
- make the examples use the generic trait, so LEM can example similarly,
- make Supernova use the generic trait, so LEM can NIVC similarly,

* use a type alias to simplify test calls

---------

Co-authored-by: Arthur Paulino <arthurleonardo.ap@gmail.com>
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.

SuperNova test
5 participants