-
Notifications
You must be signed in to change notification settings - Fork 121
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
Standardised verifiable builds #1148
Conversation
We need to approve the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some stray unwraps need removing, there might be some more.
Updated image. I used @deuszx dockerfile for inspiration. I managed to install cargo-contract from crates.io and still have a size of ~890Mb. As I said earlier, we need to be consistent with the system architecture. I have compiled flipper contract using the ARM version of the image and the X86_64 one. The two produced binaries were different. We need to approve the dockerfile of the image first, so I can contact CI team to publish it. Then, I will proceed with finishing off this PR. |
Can you add a CI step somewhere here where you build the same contract on multiple OSes and verify that their outcomes match? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The branch has been merged to master. Please use |
Tested out
Can you advise? |
The entry point now is Just bear in mind, if you run the build manually, the resulted metadata file won't contain |
I tried it out on 4 cases (where all 4 cases were generated from ChainIDE WASM Contract templates and compiled in ChainIDE), and flipper compiled but the other 3 did not. Errors are included below:
We were happy that case 1 generated the same codehash as chainide but any insights to cases 2/3/4 would be appreciated! |
I compiled the given contracts locally (in host environment), and got the same errors. The issue is unrelated to this PR. Please open respective issues in 727-Ventures/openbrush-contracts repo. |
Hi. I am from Chainide. Based on my experience, the last three contracts require +nightly and rustc 1.67.1 to compile successfully (you can compile them successfully at https://staging-9589904a8a.chainide.com/s/dashboard/projects). The reason for this might be related to this: #1058. So, I believe either Openbrush should update their template or the Docker should support "cargo +nightly-2023-02-07 contract build". Either way, I think it requires communication among Openbrush, Paritytech, Chainide, and Polkaholic. |
@wufengtao1 @sourabhniyogi can you please open a separate issue in cargo-contract and ping the relevant stakeholders? The new version of the cargo-contract has not yet been released. So, we still have some time to tweak the image to accommodate all the stakeholders that require this feature. |
@SkymanOne Sure. Let me handle this. |
* docker image + tokio * docker client draft * refactoring execute func * successful docker build * persist flags in the main execution context * docker readme + respect host flags * add build steps * better image, custom image arg, container reuse * fix readme example * fix docs * update README * docs fixes * clarification in the docs * use locked for cargo install * concat install and cleanup commands * use arg instead of env in dockerfile * add version labels * update comments * decompose the function * correct argument passing in dockerfile * refactoring * more fixes * using remote registry + bell & whistles (progress bars) * changelog entry * fixes * refactoring * explicitly specify versions of apt packages * refactoring * add module docs * refactoring * doc fix and optionally use git in docker image * Error handling for builds * use single var in docker and refactor docker module * notes on apple silicon * remove unused arg in dockerfile * use target path correctly * handle building with relative paths + permissions * more flags in docker image + fixes * remove unused code * add git info to docker metadata * use env to detect running OS * detect host OS * non-root docker user * mount contract to home dir in docker * give permission to target to everyone * fix typo * use current uid and gid for docker * conditional compilation * dont cache ci template and use musl in docker * OS specific code blocks * print error logs * remove the message * calculate digest in separate func * filter our status msgs in error * update comments * resolve other merge conflicts * display container name * properly trancate error output * apply Andrews suggestions * proper build step logs * fix generation of ABI * hash the cmd, not entrypoint for digest * fix args parsing and add docs * clippy fix
Refers to #1065
This PR aims to bring the workflow to produce verifiable and reproducible builds using
cargo contract --verifiable
and docker container with the pre-defined based on Debian distro.In detail, the command spawns the container with the binding volume to the contact folder (See image documentation for more details), then executes
cargo contract build --release --output-json <some additional flags> > target/build_result.json
to build the contract in the release mode and save the serializedBuildResult
output to a file. We need that in order to allow the host machine to access the results and print them in the STDOUT in the requested format.After the execution, the container and
build_result.json
files are deleted. The host then accesses metadata files, deserializes them back into the rust object, add theimage
name, and writes them back.It was noted that building an image on different architectures (AMD64 vs ARM64) results in the production of different WASM binaries. Therefore, it was decided to stick with the single AMD64 image for the builds. While it may have an impact on the build times on ARM machines, this is the only way we can guarantee consistency in the reproducible builds.
The image has been published to: https://hub.docker.com/r/paritytech/contracts-verifiable
Since the docker image is supposed to track the
cargo-contract
useparitytech/contracts-verifiable:test
tag which can be manually specified with the--image
flag.Once the PR is merged,
master
tag tracking master branch will be published.The follow up PR will be focused on the CI to automate the image release process.
Tasks
Some extra goals I think worth considering
Allowing to specify docker image to usePoints 2 and 3 can be considered as a follow-up issues