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

Fix how cosmwasm-plus compiles #21

Closed
ethanfrey opened this issue Aug 19, 2020 · 14 comments
Closed

Fix how cosmwasm-plus compiles #21

ethanfrey opened this issue Aug 19, 2020 · 14 comments
Assignees

Comments

@ethanfrey
Copy link
Member

We made a great simplification by just running the compile step one time on the top level folder even with multple workspaces: https://github.com/CosmWasm/rust-optimizer/blob/v0.10.1/optimize.sh#L25

However, when we run this on cosmwasm-plus we get cw1_whitelist with a size of 13kb. That is due to the 'library' feature being enabled when imported by subkeys, so no exports and almost everything gets optimized out.

Features are the union of all crates referencing it. In order to allow these crates to be imported as libraries and also compiled alone, we need to run the cargo build script once per contract. We also decided we cannot pass argument on the command-line, so I propose something like the following:

  • cd $1
  • check Cargo.toml exists
  • if there is a [package]name entry then compile there
  • if there is a [package]members entry, then go into each directory listed there and run the build script in each one

This should work the same for cosmwasm and cosmwasm-examples and cosmwasm-template. The only change would be cosmwasm-plus, where we get the previous behavior (listing .contracts/*) without passing that explicitly. We may need a tool to parse the toml file beyond grep. https://github.com/freshautomations/stoml is interesting but not sure if we can apt install it.

@maurolacy
Copy link
Contributor

maurolacy commented Aug 19, 2020

Hi Ethan,

Is this because compilation artifacts are being re-used between contracts (even when feature flags change)? This looks like a cargo bug / limitation then.

What about running cargo clean before compiling each contract? Or is this precisely what you want to avoid?

Regards,

@ethanfrey
Copy link
Member Author

We run cargo build once on the top level, that compiles it all in one pass (and unions feature flags), but make many outputs. In fact we only run one command, so no way to run cargo clean between build steps.

In an earlier version, I looped over contracts/* and ran cargo build in each one and it worked fine (no clean needed).

However, we want this to work without a command-line argument, so we want to do the looping version, but need to auto-detect if we need to loop over packages.

Do you want to take this on?

@ethanfrey
Copy link
Member Author

Basically 5d60546#diff-5c2979e23e9d5a4e3646e200224774beR15-R23 was fine.

But we cannot use $@ but rather auto-detect this (as you suggested on a thread before)

@maurolacy
Copy link
Contributor

Do you want to take this on?

Sure (if it's not too urgent). Looks like an easy fix; I'll try to do it during the week.

@webmaster128 webmaster128 self-assigned this Aug 19, 2020
@webmaster128
Copy link
Member

webmaster128 commented Aug 19, 2020

However, when we run this on cosmwasm-plus we get cw1_whitelist with a size of 13kb. That is due to the 'library' feature being enabled when imported by subkeys, so no exports and almost everything gets optimized out.

Features are the union of all crates referencing it. In order to allow these crates to be imported as libraries and also compiled alone, we need to run the cargo build script once per contract.

I can reproduce this behaviour as follows:

No features here ⬇️

(cd contracts/cw1-whitelist && RUSTFLAGS='-C link-arg=-s' cargo build --release --target wasm32-unknown-unknown -v)
       Fresh unicode-xid v0.2.1
       Fresh itoa v0.4.6
       Fresh base64 v0.11.0
       Fresh proc-macro2 v1.0.19
       Fresh ryu v1.0.5
       Fresh doc-comment v0.3.3
       Fresh quote v1.0.7
       Fresh syn v1.0.36
       Fresh serde_derive_internals v0.25.0
       Fresh serde_derive v1.0.114
       Fresh snafu-derive v0.6.8
       Fresh serde v1.0.114
       Fresh schemars_derive v0.7.6
       Fresh serde_json v1.0.57
       Fresh serde-json-wasm v0.2.1
       Fresh snafu v0.6.8
       Fresh schemars v0.7.6
       Fresh cosmwasm-std v0.10.0
       Fresh cosmwasm-storage v0.10.0
       Fresh cw2 v0.1.1 (/projects/cosmwasm-plus/packages/cw2)
   Compiling cw1-whitelist v0.1.1 (/projects/cosmwasm-plus/contracts/cw1-whitelist)
     Running `rustc --crate-name cw1_whitelist --edition=2018 contracts/cw1-whitelist/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type cdylib --crate-type rlib --emit=dep-info,link -C opt-level=3 -Cembed-bitcode=no -C codegen-units=1 -C overflow-checks=on -C metadata=99935096d613e86b --out-dir /projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps --target wasm32-unknown-unknown -L dependency=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps -L dependency=/projects/cosmwasm-plus/target/release/deps --extern cosmwasm_std=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcosmwasm_std-f708af921c5191bb.rlib --extern cosmwasm_storage=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcosmwasm_storage-be5c489d9d88a38e.rlib --extern cw2=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcw2-a34f2bae0e99ccda.rlib --extern schemars=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libschemars-f094c9d806e7a2db.rlib --extern serde=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libserde-da810fda3465b8b1.rlib --extern snafu=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libsnafu-656438f3b2c0a89f.rlib -C link-arg=-s`
    Finished release [optimized] target(s) in 10.27s

Building workspace adds --cfg 'feature="library"' to the cw1_whitelist build ⬇️

$ RUSTFLAGS='-C link-arg=-s' cargo build --release --target wasm32-unknown-unknown -v                                
       Fresh unicode-xid v0.2.1
       Fresh itoa v0.4.6
       Fresh base64 v0.11.0
       Fresh proc-macro2 v1.0.19
       Fresh ryu v1.0.5
       Fresh doc-comment v0.3.3
       Fresh quote v1.0.7
       Fresh syn v1.0.36
       Fresh serde_derive_internals v0.25.0
       Fresh serde_derive v1.0.114
       Fresh snafu-derive v0.6.8
       Fresh schemars_derive v0.7.6
       Fresh serde v1.0.114
       Fresh snafu v0.6.8
       Fresh serde_json v1.0.57
       Fresh serde-json-wasm v0.2.1
       Fresh schemars v0.7.6
       Fresh cosmwasm-std v0.10.0
       Fresh cosmwasm-storage v0.10.0
       Fresh cw20 v0.1.1 (/projects/cosmwasm-plus/packages/cw20)
       Fresh cw1 v0.1.1 (/projects/cosmwasm-plus/packages/cw1)
       Fresh cw2 v0.1.1 (/projects/cosmwasm-plus/packages/cw2)
   Compiling cw1-whitelist v0.1.1 (/projects/cosmwasm-plus/contracts/cw1-whitelist)
       Fresh cw20-base v0.1.1 (/projects/cosmwasm-plus/contracts/cw20-base)
       Fresh cw20-escrow v0.1.1 (/projects/cosmwasm-plus/contracts/cw20-escrow)
     Running `rustc --crate-name cw1_whitelist --edition=2018 contracts/cw1-whitelist/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type cdylib --crate-type rlib --emit=dep-info,link -C opt-level=3 -Cembed-bitcode=no -C codegen-units=1 -C overflow-checks=on --cfg 'feature="library"' -C metadata=99935096d613e86b --out-dir /projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps --target wasm32-unknown-unknown -L dependency=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps -L dependency=/projects/cosmwasm-plus/target/release/deps --extern cosmwasm_std=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcosmwasm_std-f708af921c5191bb.rlib --extern cosmwasm_storage=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcosmwasm_storage-be5c489d9d88a38e.rlib --extern cw2=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcw2-a34f2bae0e99ccda.rlib --extern schemars=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libschemars-f094c9d806e7a2db.rlib --extern serde=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libserde-da810fda3465b8b1.rlib --extern snafu=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libsnafu-656438f3b2c0a89f.rlib -C link-arg=-s`
   Compiling cw1-subkeys v0.1.1 (/projects/cosmwasm-plus/contracts/cw1-subkeys)
     Running `rustc --crate-name cw1_subkeys --edition=2018 contracts/cw1-subkeys/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type cdylib --crate-type rlib --emit=dep-info,link -C opt-level=3 -Cembed-bitcode=no -C codegen-units=1 -C overflow-checks=on -C metadata=d12d5210728ea57f --out-dir /projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps --target wasm32-unknown-unknown -L dependency=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps -L dependency=/projects/cosmwasm-plus/target/release/deps --extern cosmwasm_std=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcosmwasm_std-f708af921c5191bb.rlib --extern cosmwasm_storage=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcosmwasm_storage-be5c489d9d88a38e.rlib --extern cw1_whitelist=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcw1_whitelist.rlib --extern cw2=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcw2-a34f2bae0e99ccda.rlib --extern cw20=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libcw20-6f19f47ea78a13c6.rlib --extern schemars=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libschemars-f094c9d806e7a2db.rlib --extern serde=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libserde-da810fda3465b8b1.rlib --extern snafu=/projects/cosmwasm-plus/target/wasm32-unknown-unknown/release/deps/libsnafu-656438f3b2c0a89f.rlib -C link-arg=-s`
    Finished release [optimized] target(s) in 14.98s

This is pretty much what is discussed in rust-lang/cargo#5015 and rust-lang/cargo#5364. In order to avoid relying on problematic or unstable behaviour, I agree we should build the contracts individually.

Basically 5d60546#diff-5c2979e23e9d5a4e3646e200224774beR15-R23 was fine.

A quite flexible variation of this would be to pass a single foder as a compile all contracts in here argument and let find do the rest:

cosmwasm-plus

$ export FOLDER_AS_ARGUMENT="./contracts"
$ find "$FOLDER_AS_ARGUMENT" -type d \( -name target -o -name package -o -name .git \) -prune -false -o -type file -name Cargo.toml -exec dirname {} \;
./contracts/cw20-base
./contracts/cw20-escrow
./contracts/cw1-whitelist
./contracts/cw1-subkeys

cosmwasm-examples (what we do now)

$ cd ./erc20
$ export FOLDER_AS_ARGUMENT="."
$ find "$FOLDER_AS_ARGUMENT" -type d \( -name target -o -name package -o -name .git \) -prune -false -o -type file -name Cargo.toml -exec dirname {} \;
.

cosmwasm-examples (what we could do instead)

$ export FOLDER_AS_ARGUMENT="."
$ find "$FOLDER_AS_ARGUMENT" -type d \( -name target -o -name package -o -name .git \) -prune -false -o -type file -name Cargo.toml -exec dirname {} \;
./erc20
./voting
./nameservice
./mask
./escrow

cosmwasm (what we do now)

$ export FOLDER_AS_ARGUMENT="./contracts/burner"
$ find "$FOLDER_AS_ARGUMENT" -type d \( -name target -o -name package -o -name .git \) -prune -false -o -type file -name Cargo.toml -exec dirname {} \;
./contracts/burner

cosmwasm (what we could do instead)

$ export FOLDER_AS_ARGUMENT="./contracts"
$ find "$FOLDER_AS_ARGUMENT" -type d \( -name target -o -name package -o -name .git \) -prune -false -o -type file -name Cargo.toml -exec dirname {} \;
./contracts/reflect
./contracts/staking
./contracts/queue
./contracts/hackatom
./contracts/burner

@ethanfrey
Copy link
Member Author

A quite flexible variation of this would be to pass a single foder as a compile all contracts in here argument and let find do the rest:

That sounds nice. I thought we were trying to avoid arguments for the verification (and the spec of how to run this docker image with no args). But we are updating metadata soon, so maybe we can just add this as a new field?

@webmaster128
Copy link
Member

I thought we were trying to avoid arguments for the verification (and the spec of how to run this docker image with no args). But we are updating metadata soon, so maybe we can just add this as a new field?

Damn, I forgot about that. Well, it works as long as you compile contrats individually. What would even be the source folder of a contract from this repo? Every contracts gets the full repo as its source?

@ethanfrey
Copy link
Member Author

Each contract has a crate

We need the same compile process as with cosmwasm (mount root, compile in each one). All deps are in Cargo.toml

@webmaster128
Copy link
Member

Each contract has a crate

Urg, this is a problem. The build process needs to be the same as the verification proces otherwise all this gets super unrelyable (Cargo.lock changes at least). We should not build contracts with path dependencies on the one hand and verify them with crates.io dependencies later on.

We need the same compile process as with cosmwasm (mount root, compile in each one). All deps are in Cargo.toml

cosmwasm is a not helping for the problems we have here because we don't publish source codes there and have no verification process.

@webmaster128
Copy link
Member

As I was pretty unhappy with my result in #22, which is overly complicated and insufficient at the same time, I made a writeup on the challenges that come in specifically with workspaces: https://hackmd.io/@webmaster128/Sy6VCqiMw

The way forward a see now includes:

  • Keeping cosmwasm/rust-optimizer a single create compiler in order to now sacrifice its stability. This basically means reverting it back to the 0.9.0 state, maybe just chaning the contract output from contract.wasm to artifacts/name.wasm.
  • Create a dedicated workspace builder that uses Python for more advanced scripting and toml parsing; use Rust nightly in order to specify the output directory; hardcode a members pattern that is interpreted as contracts

@ethanfrey
Copy link
Member Author

Okay, let's go with this... single and multiple versions.

For the simple version, we can use the compile logic from 0.9.0 and the output logic from 0.10.1.

I would add a second one with another Dockerfile here, rust-optimizer-plus, which goes and compiles all files in the contract dir one by one, tailored just to the cosmwasm plus use case. Other monorepos can follow this pattern or make their own more complex one with python scripts and all

@ethanfrey
Copy link
Member Author

Can you make this second image, or should I?

@webmaster128
Copy link
Member

Can you make this second image, or should I?

I'm on it right now and can present a solid base later today

@ethanfrey
Copy link
Member Author

Closed by #23

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 a pull request may close this issue.

3 participants