Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

fix: solvable intermediate gates for csat #433

Merged
merged 36 commits into from
Jul 25, 2023
Merged

Conversation

guipublic
Copy link
Contributor

@guipublic guipublic commented Jul 12, 2023

Description

Problem*

The width reduction creates intermediate variables from an expression. However, if an unknown variable is used for the intermediate one, from the expression, the intermediate is also unknown. In that case the csat generates an arithmetic gate with 2 unknowns and it cannot be resolved in one step.

Resolves

Summary*

We only use 'known' witnesses when generating intermediate variables.
To do this, we track the 'known' witness in a map, after each opcode that the transformation pass processes.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

Thanks for this! Will do a full review next week.

acvm/src/compiler/mod.rs Outdated Show resolved Hide resolved
@guipublic guipublic changed the title fix: solvable intermediate gates for csat *DO NOT MERGE* fix: solvable intermediate gates for csat Jul 13, 2023
@guipublic guipublic marked this pull request as ready for review July 14, 2023 08:28
acvm/src/compiler/transformers/csat.rs Outdated Show resolved Hide resolved
acvm/src/compiler/mod.rs Outdated Show resolved Hide resolved
acvm/src/compiler/transformers/csat.rs Show resolved Hide resolved
acvm/src/compiler/transformers/csat.rs Outdated Show resolved Hide resolved
acvm/src/compiler/transformers/csat.rs Outdated Show resolved Hide resolved
acvm/src/compiler/transformers/csat.rs Outdated Show resolved Hide resolved
@TomAFrench
Copy link
Member

I think we should track this on the Circuit struct as a "required witnesses to begin execution" field.

I thought a bit more about this and decided that we should instead track the private inputs to the circuit rather than the full set of witnesses required to start execution. We can then calculate this from private+public parameters.

@TomAFrench TomAFrench force-pushed the linear-acvm branch 5 times, most recently from bed1a93 to ffe361c Compare July 17, 2023 10:59
@kevaundray kevaundray merged commit ed54414 into linear-acvm Jul 25, 2023
@kevaundray kevaundray deleted the gd/linear-acvm branch July 25, 2023 07:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants