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

Refactor CallStack::Synthesize to produce consistent dummy outputs for faster nested import deployments #2598

Open
wants to merge 9 commits into
base: staging
Choose a base branch
from

Conversation

kpandl
Copy link

@kpandl kpandl commented Jan 17, 2025

Summary

This PR refactors the way CallStack::Synthesize is run, decoupling the Synthesize mode from the old execute_function approach. It produces consistent dummy outputs (inspired by CheckDeployment) more rapidly and skips sub-circuit construction for nested import calls. The result is significantly faster deployments for deeply nested imports, while still generating cryptographically coherent record nonces.

Motivation

Previously, Authorize and Synthesize modes were lumped together. As such, calls in CallStack::Synthesize were forced to run a real sub-circuit via execute_function. This is slow for nested imports and has led to an exponential growth in deployment time w.r.t. the import depth.

Thus, this PR refactors the Synthesize branch to skip full circuit execution and produce “dummy” outputs with consistent record nonces.

Note

This PR changes expected future IDs in 3 of the execute_and_finalize tests. The reason is that previously, Synthesize always ran real sub-circuits, consuming RNG draws. With this PR, certain sub-circuits are skipped by producing dummy outputs. As a result, the RNG is executed less times, leading to different transaction commitments and future IDs.

Test Plan

Tested the deployment and execution of programs with deep imports in a local devnet.

Related PRs

@vicsn
Copy link
Collaborator

vicsn commented Jan 20, 2025

NOTE: this PR will also significantly speed up tests using nested deployments, e.g. test_deep_nested_execution_cost.

synthesizer/process/src/stack/call/mod.rs Outdated Show resolved Hide resolved
synthesizer/process/src/stack/mod.rs Outdated Show resolved Hide resolved
synthesizer/process/src/stack/call/mod.rs Outdated Show resolved Hide resolved
synthesizer/process/src/stack/call/mod.rs Outdated Show resolved Hide resolved
@kpandl kpandl requested a review from vicsn January 21, 2025 17:16
@kpandl kpandl requested a review from vicsn January 23, 2025 16:25
@kpandl
Copy link
Author

kpandl commented Jan 23, 2025

note: the failing snarkvm CI test is a CI/CDN issue

@raychu86
Copy link
Contributor

@d0cd This may be relevant to some of your Stack related work

impl<N: Network> StackKeys<N> for Stack<N> {
/// Returns `true` if the proving key for the given function name exists.
#[inline]
fn contains_proving_key(&self, function_name: &Identifier<N>) -> bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that these functions are defined in impl<N: Network> StackKeys<N> for Stack<N>, can they be removed in impl<N: Network> Stack<N>?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, addressed here: 46c8498

Copy link
Collaborator

@vicsn vicsn left a comment

Choose a reason for hiding this comment

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

LGTM, CI flakiness is unrelated

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.

3 participants