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 SnarkyJS to be compatible with new Berkeley #1021

Merged
merged 58 commits into from
Jul 12, 2023

Conversation

MartinMinkov
Copy link
Contributor

@MartinMinkov MartinMinkov commented Jul 10, 2023

Description

Merges berkeley into main

Notable Changes

  • Because the Out-Of-Snark dummy verification key hash changed in this PR, the old verification key hash defaults we used (Field(0)) were incorrect. There are several places where this value is hard-coded, so we change each place to use the new dummy value. All changes are contained in this commit: 9b4805d

  • Update the vk regression tests by creating a new dump.

🔗 Accompanying Bindings PR o1-labs/o1js-bindings#80

mitschabaude and others added 30 commits May 5, 2023 17:34
…elds

Remove duplicate user command fields
[berkeley] Make CI run pull requests against `develop`
Copy link
Contributor

@ymekuria ymekuria left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

awesome fix 🙌🏻

Requesting changes because mina-signer should be kept free from OCaml dependencies!

src/mina-signer/src/sign-zkapp-command.ts Outdated Show resolved Hide resolved
src/lib/account_update.ts Outdated Show resolved Hide resolved
"methods": {
"voterRegistration": {
"rows": 1259,
"digest": "252726e585fdd28607537e32214069ab"
"rows": 1260,
Copy link
Member

Choose a reason for hiding this comment

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

that's interesting, I wonder where that extra constraint comes from 🤔

Copy link
Member

Choose a reason for hiding this comment

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

One more constant that wasn't used before takes one half generic gate!

@mitschabaude
Copy link
Member

Two more small change requests:

  • Make this a minor version bump -- we want to release a version that works on berkeley asap
  • Call out in the changelog that all existing zkApp verification keys change

…mmyVerificationKey() with mocks.dummyVerificationKeyHash
…details

This commit updates the unreleased changes link and adds the details of version 0.12.0. The breaking changes in version 0.12.0 are also documented, specifically the fix to the default verification key hash for AccountUpdates. This change adopts the default mechanism provided by Mina Protocol and may affect the verification key of already deployed contracts.
…to inclusion of snarkyjs signature.ts"

This reverts commit 4bc172d.
…s.dummyVerificationKeyHash for simplicity and readability
…ecoratorMetadata, useDefineForClassFields, and strictPropertyInitialization to support decorators and generic constructors in TypeScript
Copy link
Member

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

approving in advance, but it looks like mina-signer still depends on snarkyjs / jsoo

@MartinMinkov
Copy link
Contributor Author

approving in advance, but it looks like mina-signer still depends on snarkyjs / jsoo

Got this fixed, thanks to your comments. Thanks! 🎉

@MartinMinkov MartinMinkov merged commit 79a90a2 into main Jul 12, 2023
10 checks passed
@MartinMinkov MartinMinkov deleted the fix-snarkyjs-berkeley branch July 12, 2023 00:18
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.

6 participants