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

EVM_ARITHMETIZATION_PKG_VER should pass through #368

Closed
0xaatif opened this issue Jul 9, 2024 · 2 comments
Closed

EVM_ARITHMETIZATION_PKG_VER should pass through #368

0xaatif opened this issue Jul 9, 2024 · 2 comments

Comments

@0xaatif
Copy link
Contributor

0xaatif commented Jul 9, 2024

println!(
"cargo::rustc-env=EVM_ARITHMETIZATION_PACKAGE_VERSION={}.{}.x",
// patch version change should not prompt circuits regeneration
version.major,
version.minor
);

We censor the minor version, and drop the pre-release and build meta.
This is why we do this:

while we're not interested in the patch section. Any increment of [it] should not yield circuits regeneration.

@Nashtare in review comment

I think this is backwards coupling - it should be up to the consumer of the environment variable (wherever that is) to decide what they're interested in, and when to rebuild.

Ultimately, they should be running cargo metadata etc - this logic shouldn't live in our repo unless they're outside of the cargo build.

@BGluth
Copy link
Contributor

BGluth commented Jul 9, 2024

I think this is backwards coupling - it should be up to the consumer of the environment variable (wherever that is) to decide what they're interested in, and when to rebuild.

I suppose so... But EVM_ARITHMETIZATION_PACKAGE_VERSION really is just intended for zero_bin only right? I guess using the patch version is technically safer in case someone increments the patch version when they should have incremented minor/major, but at the same time, the only fix they would need to do is just regenerate their circuits.

My slight personal preference would be to ignore patch, but I'm honestly good either way.

@Nashtare Nashtare added this to the Type 1 - Q3 2024 milestone Jul 11, 2024
@Nashtare
Copy link
Collaborator

Nashtare commented Sep 6, 2024

Removed following use of KERNEL hash for package consistency

@Nashtare Nashtare closed this as completed Sep 6, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in Zero EVM Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants