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

Apply witness visibility on a parameter level rather than witness level #712

Merged
merged 2 commits into from
Jan 31, 2023

Conversation

TomAFrench
Copy link
Member

@TomAFrench TomAFrench commented Jan 30, 2023

Related issue(s)

Related to #684 as it makes the witness indices of each param more accessible.

Description

Summary of changes

An argument to main can either be public or private, we disallow situations where e.g. an array has a mixture of public and private elements. This means that we can simplify the evaluator by rather than eagerly pushing each witness onto the public_inputs vector, we can instead calculate all the witnesses associated with a parameter and push all of them if the parameter is public.

This is also useful as it surfaces all the witnesses associated with a particular parameter which makes it easier to build a mapping from a param to its witness indices when it comes to abi encoding.

Dependency additions / changes

N/A

Test additions / changes

N/A

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

Additional context

Copy link
Contributor

@kevaundray kevaundray left a comment

Choose a reason for hiding this comment

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

LGTM

@kevaundray kevaundray merged commit e3bea13 into master Jan 31, 2023
@kevaundray kevaundray deleted the param-visibility branch January 31, 2023 11:42
TomAFrench added a commit that referenced this pull request Feb 3, 2023
* master:
  Rename methods that use `conditionalize` to be more descriptive (#739)
  feat(noir)!:  Returned values are no longer required by the prover (#731)
  chore: explicit versions for dependencies (#727)
  chore: readability improvements (#726)
  feat(nargo): include short git commit in cli version output (#721)
  Remove print to console for named proofs in `nargo prove` (#718)
  chore: clean up serde-related dependencies (#722)
  Handle out-of-bound errors in CSE (#471) (#673)
  Remove unused dependencies and only use workspace inheritance on shared deps (#671)
  feat(std_lib)!: modulus bits/bytes methods, and to_bits -> to_le_bits (#697)
  Implement numeric generics (#620)
  Review some TODO in SSA (#698)
  Replace `toml_map_to_field` and `toml_remap` with traits to map between `InputValue`s and `TomlTypes` (#677)
  Apply witness visibility on a parameter level rather than witness level (#712)
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