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

Adds a variable limit for deployments #2444

Merged
merged 8 commits into from
Apr 29, 2024
Merged

Conversation

howardwu
Copy link
Contributor

@howardwu howardwu commented Apr 29, 2024

Motivation

Note: This PR contains breaking changes. First, all verifying keys are serialized with 8 extra bytes to account for the num_variables, which represents the number of constant, public, and private variables in the circuit. In addition, the DataID was updated to address an open TODO which involves reordering the map IDs (and also a breaking change).

Original motivation from @vicsn :

Even though we limit program size and number of constraints, @d0cd identified that it is possible for someone to create huge constants in programs. During deployment verification, an attacker can make a validator take forever or use a lot of memory, without paying for it, because we don't limit or price constants.

The solution is to limit constants similarly to how we limit the number of constraints.

Some design considerations:

  • I'm limiting total number of variables, not just constants, as defense in depth. Our lack of variable limitation was nagging me anyway. Speed to synthesize is the same for constants v.s. variables (for their respective worst case opcode keccak v.s. psd)
  • I'm pricing in variables at the same rate as constraints, to avoid DoS from small programs with lots of constants.
  • credits.aleo uses ~150k variables, the biggest popular program 300k variables, so (1 << 20) should be a sufficient limit
  • verification of a deployment with max constants of (1 << 20) takes ~2 seconds in debug mode, so likely 0.02-0.2 second in release).
  • we cannot add num_constants to CircuitInfo, constants are not a known quantity at the Varuna level. That's why I added the value to the Deployment object. Given that we price deployments by their size, the cost for deployments is increased by 0.01 aleo credit per function. Technically, we could only track the constants and not the total number of variables in the deployment, but I fear its less understandable for outside developers.

Test Plan

Adding two tests, one testing the variable limit, and one testing manipulation of reported variables.

Related PRs

Supersedes #2431.

Similar to: https://github.com/AleoHQ/snarkVM/pull/2271.

@howardwu howardwu marked this pull request as ready for review April 29, 2024 22:52
Copy link
Contributor

@raychu86 raychu86 left a comment

Choose a reason for hiding this comment

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

LGTM

@howardwu howardwu merged commit eedf4c8 into mainnet-staging Apr 29, 2024
80 checks passed
@howardwu howardwu deleted the feat/variable-count branch April 29, 2024 22:55
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.

2 participants