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

fix: Optimize contracts built by nargo info #2259

Merged
merged 1 commit into from
Aug 10, 2023
Merged

Conversation

TomAFrench
Copy link
Member

Description

Problem*

Resolves #2256

Summary*

This PR ensures that we optimize contracts before querying the number of gates they result in in nargo info.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

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

@TomAFrench
Copy link
Member Author

@sirasistant Can you confirm this on the aztec contract you experienced this on? noirup --pr 2259 should install this for you.

Copy link
Contributor

@sirasistant sirasistant left a comment

Choose a reason for hiding this comment

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

LGTM, and verified it working correctly in the contract where it was failing before.

@TomAFrench TomAFrench added this pull request to the merge queue Aug 10, 2023
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

This makes sense. I still hope to find time to clean up all of these code paths

Merged via the queue into master with commit b30b3f4 Aug 10, 2023
@TomAFrench TomAFrench deleted the fix-nargo-info branch August 10, 2023 16:17
TomAFrench added a commit that referenced this pull request Aug 10, 2023
* master: (80 commits)
  fix: properly capture lvalues in closure environments (#2120) (#2257)
  fix: Optimize contracts built by `nargo info` (#2259)
  chore: impl Display for DebugType (#2258)
  chore: update `noir_wasm` build process to match `acvm_js` (#2067)
  feat: Implement traits - parser support #2094 (#2230)
  chore: Refactor DefCollector duplicate errors (#2231)
  chore: Address clippy warnings (#2246)
  feat: Support `contract` package type in `nargo info` command (#2249)
  feat: Add slice append (#2241)
  chore: Bump `async-lsp` to v0.0.5 (#2186)
  chore: Move the remaining `nargo_cli` lib funcs into `nargo` crate (#2225)
  chore: Add test for eddsa (#2237)
  chore: Split `Nargo.toml` operations into separate package (#2224)
  fix(stdlib): correct `tecurve::contains` formula (#1821)
  feat: Remove `comptime` and warn upon usage (#2178)
  fix: Remove last vestige of array of structs to struct of arrays conversion (#2217)
  fix: Add foreign impl error (#2216)
  feat(nargo)!: Replace `--contracts` flag with `contract` package type (#2204)
  feat: Optimize `x < 0` for unsigned `x` to false (#2206)
  fix: Initialize numeric generics' type to a polymorphic integer when used in an expression (#2179)
  ...
TomAFrench added a commit that referenced this pull request Aug 11, 2023
* master: (29 commits)
  feat(nargo): Add support for contracts in `nargo check` (#2267)
  chore(ci): Name wasm job more clearly (#2269)
  chore(ci): Create cache key with consideration to target (#2273)
  chore(ci): Run publish workflow against PRs (#2268)
  chore: Merge in contents of `build-nargo` repository (#2211)
  fix(lsp): Improve dependency resolution in context of `Nargo.toml` (#2226)
  chore: Remove unnecessary duplication in how we test Noir compiler (#2248)
  fix: properly capture lvalues in closure environments (#2120) (#2257)
  fix: Optimize contracts built by `nargo info` (#2259)
  chore: impl Display for DebugType (#2258)
  chore: update `noir_wasm` build process to match `acvm_js` (#2067)
  feat: Implement traits - parser support #2094 (#2230)
  chore: Refactor DefCollector duplicate errors (#2231)
  chore: Address clippy warnings (#2246)
  feat: Support `contract` package type in `nargo info` command (#2249)
  feat: Add slice append (#2241)
  chore: Bump `async-lsp` to v0.0.5 (#2186)
  chore: Move the remaining `nargo_cli` lib funcs into `nargo` crate (#2225)
  chore: Add test for eddsa (#2237)
  chore: Split `Nargo.toml` operations into separate package (#2224)
  ...
TomAFrench added a commit that referenced this pull request Aug 11, 2023
* master:
  feat(nargo): Add support for contracts in `nargo check` (#2267)
  chore(ci): Name wasm job more clearly (#2269)
  chore(ci): Create cache key with consideration to target (#2273)
  chore(ci): Run publish workflow against PRs (#2268)
  chore: Merge in contents of `build-nargo` repository (#2211)
  fix(lsp): Improve dependency resolution in context of `Nargo.toml` (#2226)
  chore: Remove unnecessary duplication in how we test Noir compiler (#2248)
  fix: properly capture lvalues in closure environments (#2120) (#2257)
  fix: Optimize contracts built by `nargo info` (#2259)
  chore: impl Display for DebugType (#2258)
  chore: update `noir_wasm` build process to match `acvm_js` (#2067)
  feat: Implement traits - parser support #2094 (#2230)
  chore: Refactor DefCollector duplicate errors (#2231)
  chore: Address clippy warnings (#2246)
  feat: Support `contract` package type in `nargo info` command (#2249)
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.

Nargo info fails on some contracts
3 participants