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

Use proof-systems master #1942

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

dannywillems
Copy link
Member

See MinaProtocol/mina#16405 for additional context.

@dannywillems dannywillems requested a review from a team as a code owner December 9, 2024 15:38
@dannywillems dannywillems marked this pull request as draft December 9, 2024 15:38
@dannywillems dannywillems marked this pull request as ready for review December 9, 2024 16:10
Copy link
Contributor

@45930 45930 left a comment

Choose a reason for hiding this comment

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

I'm not sure we need this PR at all if there's no functional changes. Whoever next updates the bindings will include this history in their branch right?

@@ -25,6 +25,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Method for optional types to assert none https://github.com/o1-labs/o1js/pull/1922
- Increased maximum supported amount of methods in a `SmartContract` or `ZkProgram` to 30. https://github.com/o1-labs/o1js/pull/1918
- Expose low-level conversion methods `Proof.{_proofToBase64,_proofFromBase64}` https://github.com/o1-labs/o1js/pull/1928
- Improve usage of branches of proof-systems. Mina uses only the master branch
from now. https://github.com/o1-labs/o1js/pull/1942
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this for a markdown-only change

Copy link
Member Author

Choose a reason for hiding this comment

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

There is also an update of the bindings.

@@ -1,22 +1,31 @@
# o1js README-dev

o1js is a TypeScript framework designed for zk-SNARKs and zkApps on the Mina blockchain.
o1js is a TypeScript framework designed for zk-SNARKs and zkApps on the Mina
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these line-length diffs intentional, or just your local IDE settings?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are intentional, yes.
I added these changes in two independent commits that can be removed if there is a strong opinion on not enforcing a limit of 80 characters.

It is quite commonly accepted to keep 80 characters as a limit, not only for historical reasons, but also because it is easier to go through files in modern editors too when we have multiple buffers open on the same screen.

Copy link
Member Author

Choose a reason for hiding this comment

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

(but also my editor is configured to make it relatively automatic when I open a file)

@dannywillems
Copy link
Member Author

Moving back in draft. It seems I did not update the bindings correctly.

@dannywillems dannywillems marked this pull request as draft December 9, 2024 16:53
@dannywillems dannywillems force-pushed the dw/use-proof-systems-master branch 2 times, most recently from 74ccc2f to 9ea496f Compare December 9, 2024 17:08
Use commit which uses proof-systems/master instead of proof-systems/develop,
including additional changes to make the Mina codebase compatible.
Update bindings after checking out to mina branch that uses proof-systems/master
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