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

Release preparation #4

Merged
merged 18 commits into from
Mar 28, 2024
Merged

Release preparation #4

merged 18 commits into from
Mar 28, 2024

Conversation

h4nsu
Copy link
Member

@h4nsu h4nsu commented Mar 22, 2024

This PR prepares for release and deployment. See RELEASE_TODO.md for detailed release procedure

INB4: No, changing values of constants unused by the contract does not change the WASM build outcome ;)

Cargo.toml Outdated Show resolved Hide resolved
RELEASE_TODO.md Outdated Show resolved Hide resolved
Copy link

@deuszx deuszx left a comment

Choose a reason for hiding this comment

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

Why did you remove the toolchain file?

@h4nsu
Copy link
Member Author

h4nsu commented Mar 22, 2024

Because it prevented cargo contract 3.2.0 from compiling (newer rust was needed for clap)

@deuszx
Copy link

deuszx commented Mar 22, 2024

Because it prevented cargo contract 3.2.0 from compiling (newer rust was needed for clap)

What do you need clap for? Just bump the toolchain then. if we don't include it in the build it will be overwritten from the local env (or docker) and might affect the result.

@h4nsu
Copy link
Member Author

h4nsu commented Mar 22, 2024

cargo contract uses clap, the workflow builds and installs cargo contract

@deuszx
Copy link

deuszx commented Mar 22, 2024

cargo contract uses clap, the workflow builds and installs cargo contract

If we use the docker method I showed, you won't have that problem, as cargo contract binary is already available there.

@h4nsu
Copy link
Member Author

h4nsu commented Mar 22, 2024

You mean use the docker container in CI workflow?

@deuszx
Copy link

deuszx commented Mar 22, 2024

Yes, why not. It's very fast. The docker image is only 300mb and could be cached but I don't think we will re-run it many times.

@h4nsu
Copy link
Member Author

h4nsu commented Mar 22, 2024

I don't know how to do it straight away, and I don't think it's worth the effort of learning. The whole CI is just a cargo test followed by a check if the contract builds at all.

@deuszx
Copy link

deuszx commented Mar 22, 2024

We already do it both in common-amm and MOST. You can check it out there

Copy link

@deuszx deuszx left a comment

Choose a reason for hiding this comment

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

Can we remove the binary wrapped_azero.wasm from the repo ? At least until we provide the command how to build it deterministically.

Also, we need to bring back the rust-toolchain.toml as it might affect the final build result - ie even within the reproducible env, having different toolchains can result in different Wasm produced.

Copy link

@deuszx deuszx left a comment

Choose a reason for hiding this comment

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

Can we remove the binary wrapped_azero.wasm from the repo ? At least until we provide the command how to build it deterministically.

Also, we need to bring back the rust-toolchain.toml as it might affect the final build result - ie even within the reproducible env, having different toolchains can result in different Wasm produced.

@h4nsu
Copy link
Member Author

h4nsu commented Mar 28, 2024

I added the .wasm file with all other compilation artifacts as PLACEHOLDERS that will be replaced with real values after the release happens.

If we build it with the docker image why do we need rust-toolchain file? Isn't the environment fixed by what's inside the image?

Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@h4nsu h4nsu merged commit d513641 into main Mar 28, 2024
2 checks passed
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