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

Remove commitment shifting functionality #1377

Merged
merged 10 commits into from
Feb 21, 2024

Conversation

volhovm
Copy link
Member

@volhovm volhovm commented Dec 4, 2023

This removes the unused and obsolete functionality of shifted polynomial commitments from the poly-commitments crate, and from its usages in kimchi.

Probably better to review commit-by-commit.

@volhovm volhovm requested a review from mrmr1993 December 4, 2023 14:16
@volhovm volhovm force-pushed the volhovm/mina14628-remove-commitment-shifting branch from 31b93fa to 1e121b7 Compare December 4, 2023 14:19
@volhovm volhovm changed the title Remove commitment shifting functionality [WIP] Remove commitment shifting functionality Dec 4, 2023
@dannywillems dannywillems marked this pull request as draft December 4, 2023 14:52
@volhovm volhovm changed the title [WIP] Remove commitment shifting functionality Remove commitment shifting functionality Dec 4, 2023
@dannywillems dannywillems self-assigned this Dec 4, 2023
@volhovm volhovm requested a review from dannywillems December 4, 2023 16:10
@volhovm volhovm force-pushed the volhovm/mina14628-remove-commitment-shifting branch from 1504d37 to 7c720ff Compare December 8, 2023 13:51
@volhovm volhovm marked this pull request as ready for review February 12, 2024 12:18
@dannywillems
Copy link
Member

I would not merge it as it requires changes in o1js-bindings.

@volhovm
Copy link
Member Author

volhovm commented Feb 20, 2024

@dannywillems Does it? I'm not exactly sure -- if you see the mina PR (MinaProtocol/mina#15071), the changes in bindings there are minimal/don't propagate upwards. What's the issue exactly?

@dannywillems
Copy link
Member

@dannywillems Does it? I'm not exactly sure -- if you see the mina PR (MinaProtocol/mina#15071), the changes in bindings there are minimal/don't propagate upwards. What's the issue exactly?

See https://github.com/o1-labs/o1js-bindings/blob/main/crypto/bindings/conversion-core.ts#L118. You should update o1js-bindings in o1js and bump up mina in o1js. Otherwise, the next time someone will bump up develop in o1js, they will have errors.

@volhovm
Copy link
Member Author

volhovm commented Feb 21, 2024

@dannywillems OK thanks for the heads-up, will do.

@volhovm
Copy link
Member Author

volhovm commented Feb 21, 2024

@dannywillems was addressed in o1-labs/o1js-bindings#250 o1-labs/o1js#1290, everything seems to work well. Merging!

@volhovm volhovm merged commit 03bb1ff into develop Feb 21, 2024
6 checks passed
@volhovm volhovm deleted the volhovm/mina14628-remove-commitment-shifting branch February 21, 2024 14:50
@volhovm volhovm restored the volhovm/mina14628-remove-commitment-shifting branch February 21, 2024 14:50
@volhovm volhovm deleted the volhovm/mina14628-remove-commitment-shifting branch February 21, 2024 14:50
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.

3 participants