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(nargo): Switch order of writing acir file and acir checksum file #895

Merged
merged 1 commit into from
Feb 21, 2023

Conversation

vezenovm
Copy link
Contributor

Related issue(s)

Quick fix found while working on a separate project so no issue

Description

During nargo compile the checksum file is being written first followed by the acir file. We set an extension .acir.sha256 for the sha checksum. When we reset the extension we were mistakenly only resetting the last extension of .sha256 so the acir file being written to target was .acir.acir.

Summary of changes

We write the ACIR file followed by the checksum now. Running nargo compile p for example will now correctly give p.acir and p.acir.sha256.

Dependency additions / changes

Test additions / changes

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.
  • This PR requires documentation updates when merged.

Additional context

@vezenovm vezenovm changed the title Switch order of writing acir file and acir checksum file fix(nargo): Switch order of writing acir file and acir checksum file Feb 21, 2023
@vezenovm vezenovm added this pull request to the merge queue Feb 21, 2023
Merged via the queue into master with commit 4fc94dc Feb 21, 2023
@vezenovm vezenovm deleted the mv/fix-acir-ext branch February 21, 2023 22:00
@Savio-Sou
Copy link
Collaborator

Confirming that this doesn't require doc updates right?

@TomAFrench
Copy link
Member

Yes, this shouldn't require any doc updates (I don't think we document the checksum currently anyway)

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.

4 participants