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

Standardised verifiable builds #1148

Merged
merged 73 commits into from
Jul 13, 2023
Merged

Standardised verifiable builds #1148

merged 73 commits into from
Jul 13, 2023

Conversation

SkymanOne
Copy link
Contributor

@SkymanOne SkymanOne commented Jun 6, 2023

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 serialized BuildResult 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 the image 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 use paritytech/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

  • Successfully spawn and remove containers upon execution
  • Persist image digest id in the metadata
  • Save output to the host folder
  • Respect cargo flags passed to the host
  • Allowing to specify docker image to use

Some extra goals I think worth considering

  1. Allowing to specify docker image to use
  2. Specify the docker command to be used for the build
  3. Recognise path dependant multi-contract projects and mount directories automatically ( by @deuszx )

Points 2 and 3 can be considered as a follow-up issues

crates/build/src/lib.rs Outdated Show resolved Hide resolved
@SkymanOne SkymanOne marked this pull request as ready for review June 7, 2023 11:01
@SkymanOne
Copy link
Contributor Author

We need to approve the dockerfile first to be published on registry before releasing the cargo-contract. Once the image is approved and published, docker.rs will just pull it from registry instead of trying to find the local copy.

build-image/Dockerfile Outdated Show resolved Hide resolved
Copy link
Collaborator

@ascjones ascjones left a 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.

crates/cargo-contract/src/cmd/extrinsics/call.rs Outdated Show resolved Hide resolved
crates/build/src/docker.rs Outdated Show resolved Hide resolved
crates/build/src/docker.rs Outdated Show resolved Hide resolved
@SkymanOne SkymanOne marked this pull request as draft June 8, 2023 11:58
@SkymanOne
Copy link
Contributor Author

SkymanOne commented Jun 8, 2023

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.

@ascjones @deuszx

@SkymanOne SkymanOne requested review from ascjones and deuszx June 8, 2023 17:18
@SkymanOne SkymanOne marked this pull request as ready for review June 8, 2023 18:23
build-image/Dockerfile Show resolved Hide resolved
build-image/Dockerfile Outdated Show resolved Hide resolved
build-image/Dockerfile Outdated Show resolved Hide resolved
build-image/Dockerfile Outdated Show resolved Hide resolved
build-image/Dockerfile Outdated Show resolved Hide resolved
@deuszx
Copy link
Contributor

deuszx commented Jun 9, 2023

Can you add a CI step somewhere here where you build the same contract on multiple OSes and verify that their outcomes match?

@SkymanOne SkymanOne requested a review from ascjones July 12, 2023 16:51
crates/build/src/docker.rs Outdated Show resolved Hide resolved
crates/build/src/docker.rs Show resolved Hide resolved
Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

LGTM

@SkymanOne SkymanOne merged commit db24299 into master Jul 13, 2023
10 checks passed
@SkymanOne SkymanOne deleted the gn/verifiable-build branch July 13, 2023 16:02
@SkymanOne
Copy link
Contributor Author

SkymanOne commented Jul 13, 2023

The branch has been merged to master. Please use paritytech/contracts-verifiable:master image for now until the crate is published.

@sourabhniyogi
Copy link

Tested out paritytech/contracts-verifiable:master but must be doing it wrong:

# docker run -d --name ink-container --mount type=bind,source=".",target="/contract"  paritytech/contracts-verifiable:master
137b6f0a3aaa1de3d2ceb55314347b3d9596b39ceb3cef8ea889fa1bd807b4b6

# docker logs 137b6f0a3aaa1de3d2ceb55314347b3d9596b39ceb3cef8ea889fa1bd807b4b6
Utilities to develop Wasm smart contracts

Usage: cargo contract <COMMAND>

Commands:
  new          Setup and create a new smart contract project
  build        Compiles the contract, generates metadata, bundles both together in a `<name>.contract` file
  check        Check that the code builds as Wasm; does not output any `<name>.contract` artifact to the `target/` directory
  upload       Upload contract code
  instantiate  Instantiate a contract
  call         Call a contract
  encode       Encodes a contracts input calls and their arguments
  decode       Decodes a contracts input or output data (supplied in hex-encoding)
  remove       Remove contract code
  info         Display information about a contract
  help         Print this message or the help of the given subcommand(s)

Options:
  -h, --help     Print help
  -V, --version  Print version

Can you advise?

@SkymanOne
Copy link
Contributor Author

SkymanOne commented Jul 13, 2023

Tested out paritytech/contracts-verifiable:master but must be doing it wrong:

# docker run -d --name ink-container --mount type=bind,source=".",target="/contract"  paritytech/contracts-verifiable:master
137b6f0a3aaa1de3d2ceb55314347b3d9596b39ceb3cef8ea889fa1bd807b4b6

# docker logs 137b6f0a3aaa1de3d2ceb55314347b3d9596b39ceb3cef8ea889fa1bd807b4b6
Utilities to develop Wasm smart contracts

Usage: cargo contract <COMMAND>

Commands:
  new          Setup and create a new smart contract project
  build        Compiles the contract, generates metadata, bundles both together in a `<name>.contract` file
  check        Check that the code builds as Wasm; does not output any `<name>.contract` artifact to the `target/` directory
  upload       Upload contract code
  instantiate  Instantiate a contract
  call         Call a contract
  encode       Encodes a contracts input calls and their arguments
  decode       Decodes a contracts input or output data (supplied in hex-encoding)
  remove       Remove contract code
  info         Display information about a contract
  help         Print this message or the help of the given subcommand(s)

Options:
  -h, --help     Print help
  -V, --version  Print version

Can you advise?

The entry point now is cargo contract, you need to specify the command to run
https://github.com/paritytech/cargo-contract/blob/db242993af92a9860fd753207cf9583e5a95a23e/build-image/Dockerfile#L111

Just bear in mind, if you run the build manually, the resulted metadata file won't contain image field as it is added as part of post-build step in cargo-contract

@sourabhniyogi
Copy link

sourabhniyogi commented Jul 13, 2023

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:

  1. flipper

  2. my_psp22

# docker run -d --name ink-container --mount type=bind,source="./src",target="/contract" paritytech/contracts-verifiable:master build
# docker logs [...]
 [1/*] Building cargo project
error: failed to parse manifest at `/tmp/cargo-contract_RqgRr5/Cargo.toml`

Caused by:
  the target `my_psp22` is a binary and can't have any crate-types set (currentlERROR: command ["/usr/local/rustup/toolchains/1.69-x86_64-unknown-linux-gnu/bin/cargo", "build", "--color=always", "--target=wasm32-unknown-unknown", "-Zbuild-std=core,alloc", "--no-default-features", "--release", "--target-dir=/contract/target/ink", "--features", "ink/ink-debug"] exited with code 101
  1. shiden34
# docker run -d --name ink-container --mount type=bind,source="./src",target="/contract" paritytech/contracts-verifiable:master build
# docker logs [...]
Some errors have detailed explanations: E0405, E0408, E0412, E0425, E0433, E0463, E0531.
For more information about an error, try `rustc --explain E0405`.
error: could not compile `proc-macro2` due to 451 prevERROR: command ["/usr/local/rustup/toolchains/1.69-x86_64-unknown-linux-gnu/bin/cargo", "build", "--color=always", "--target=wasm32-unknown-unknown", "-Zbuild-std=core,alloc", "--no-default-features", "--release", "--target-dir=/contract/target/ink", "--features", "ink/ink-debug"] exited with code 101
  1. factory_contract
# docker run -d --name ink-container --mount type=bind,source="./src",target="/contract" paritytech/contracts-verifiable:master build
# docker logs [...]
ERROR: Error invoking `cargo metadata` for Cargo.toml

Caused by:
    `cargo metadata` exited with an error:     Updating crates.io index
        Updating git repository `https://github.com/727-Ventures/openbrush-contracts`
    error: failed to get `pair_contract` as a dependency of package `factory_contract v0.1.0 (/contract)`
    
    Caused by:
      failed to load source for dependency `pair_contract`
    
    Caused by:
      Unable to update /pair
    
    Caused by:
      failed to read `/pair/Cargo.toml`
    
    Caused by:
      No such file or directory (os error 2)

We were happy that case 1 generated the same codehash as chainide but any insights to cases 2/3/4 would be appreciated!

@SkymanOne
Copy link
Contributor Author

docker run -d --name ink-container --mount type=bind,source="./src",target="/contract" paritytech/contracts-verifiable:master build

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.

@wufengtao1
Copy link

wufengtao1 commented Jul 14, 2023

docker run -d --name ink-container --mount type=bind,source="./src",target="/contract" paritytech/contracts-verifiable:master build

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.
image

@SkymanOne
Copy link
Contributor Author

@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.

@wufengtao1
Copy link

@SkymanOne Sure. Let me handle this.

@ascjones ascjones mentioned this pull request Jul 27, 2023
forgetso pushed a commit to prosopo/cargo-contract that referenced this pull request Aug 2, 2023
* 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
@smiasojed smiasojed mentioned this pull request Mar 4, 2024
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.

9 participants