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

Specifying build options #80

Open
JoowonYun opened this issue Apr 19, 2022 · 23 comments
Open

Specifying build options #80

JoowonYun opened this issue Apr 19, 2022 · 23 comments

Comments

@JoowonYun
Copy link

Often contracts have separate features developed.

What does think about that option takes it as a parameter?

https://github.com/CosmWasm/rust-optimizer/blob/317e459b391f2ddd23422477edfd622e1362f10a/build_workspace/src/main.rs#L66-L71

@webmaster128
Copy link
Member

That makes reproducible builds significantly harder since you need to store options as well somewhere. Can't you use default features? What kind of feature flags do you need?

@JoowonYun
Copy link
Author

Actually, I am developing a contract, and I want to use the contract in common on different chains.

For example, Terra has a different concept called tax, and I am thinking about using only that part as features=terra.

@webmaster128
Copy link
Member

Okay, makes sense. However, don't you need different cosmwasm-std versions for Terra and other chains anyways? Also isn't Terra tax 0 these days?

@JoowonYun
Copy link
Author

JoowonYun commented Apr 19, 2022

Yes, it's a different version, but Terra keep following up on the latest version, and I think all projects will be the same. Eventually, I think there will be a release for each version. This is because I do not want to separate and manage the same logic.

Terra tax is also currently 0, but since it is a parameter, it can be changed at any time, so there is a risk to remove it. 😅

@dakom
Copy link

dakom commented Apr 20, 2022

I ran into a similar issue where I wanted to deploy a slightly tweaked version of a contract, but don't want to worry about a fork or branch going stale as other updates are made.

Features are bread-and-butter in Rust dev and a must-have for this tool imho. In the meantime I just run cargo and wasm-opt manually.

@JoowonYun
Copy link
Author

JoowonYun commented Apr 20, 2022

But I found a new problem with what I want.

When wasm is uploaded, it checks for supportedFeatures, which can't distinguish any another features I want.

But it happens in default which I didn't include as a feature. I need to check the code of this part a bit more to see if I did the build wrong.
https://github.com/CosmWasm/wasmvm/blob/cb1747a1393c4e0776185ad2c5c1dca9c1802de2/api/lib.go#L101-L114

@webmaster128
Copy link
Member

When wasm is uploaded, it checks for supportedFeatures, which can't distinguish any another features I want.

Which error do you get? The CosmWasm feature system is independent to the Rust feature system. It's a way to ensure the chain provides everything that the contract needs.

@JoowonYun
Copy link
Author

JoowonYun commented Apr 20, 2022

When wasm is uploaded, it checks for supportedFeatures, which can't distinguish any another features I want.

Which error do you get? The CosmWasm feature system is independent to the Rust feature system. It's a way to ensure the chain provides everything that the contract needs.

As mentioned above, I am developing with default and features=terra. I'm currently building with rust-optimizer in this repo and I'm pretty sure it will build without terra features. But when I do wasmd tx wasm store test.wasm, I get the following error.
Error: rpc error: code = InvalidArgument desc = failed to execute message; message index: 0: Error calling the VM: Error during static Wasm validation: Wasm contract requires unsupported features: {"terra"}: create wasm contract failed: invalid request

I'm not sure if the feature was included in wasm because I defined the feature incorrectly, or if the analyze_code is wrong.

@JoowonYun
Copy link
Author

JoowonYun commented Apr 21, 2022

@webmaster128
I have my work in this commit.
terraswap/terraswap@aea4e66

It is divided into default and terra. If cosmwasm/rust-optimizer is used, it is default, and if terraswap/rust-optimizer is used, it is an optimizer with features=terra added.

@JoowonYun
Copy link
Author

JoowonYun commented May 9, 2022

But I found a new problem with what I want.

When wasm is uploaded, it checks for supportedFeatures, which can't distinguish any another features I want.

But it happens in default which I didn't include as a feature. I need to check the code of this part a bit more to see if I did the build wrong. https://github.com/CosmWasm/wasmvm/blob/cb1747a1393c4e0776185ad2c5c1dca9c1802de2/api/lib.go#L101-L114

The problem with this comes from the code below.
https://github.com/terra-money/terra-cosmwasm/blob/094dc24caa9d417e528e32cc2e44fa19c576599b/packages/terra-cosmwasm/src/lib.rs#L14-L17

No problem with supportedFeatures.

@CyberHoward
Copy link

@JoowonYun cargo now allows for weak dependencies that are only compiled when a feature is requested. It was recently stabilized.

I'd also like to compile/optimize my contracts based on feature flags that enable these optional dependencies as our infrastructure aims to be chain-agnostic.

@webmaster128
Copy link
Member

webmaster128 commented Aug 12, 2022

The problem with this comes from the code below. https://github.com/terra-money/terra-cosmwasm/blob/094dc24caa9d417e528e32cc2e44fa19c576599b/packages/terra-cosmwasm/src/lib.rs#L14-L17

No problem with supportedFeatures.

Yeah, this will require the feature "terra". If a chain does not have it, you cannot use terra-cosmwasm as a dependency of you contract.

Those features are a different thing than Rust/Cargo features. Read more about them here: https://github.com/CosmWasm/cosmwasm/blob/main/docs/FEATURES.md

@webmaster128
Copy link
Member

I'd also like to compile/optimize my contracts based on feature flags that enable these optional dependencies as our infrastructure aims to be chain-agnostic.

Can't you create chain-specific wrapper contracts? Like foobar-terra, foobar-juno, foobar-tgrade that all depend on and re-export some variant of foobar. This way you can configure everything in the source code and your builds remain easily reproducible by just providing source code + rust-optimizer version.

@CyberHoward
Copy link

Yes but if you're working with a lot of contracts it becomes a pain to manage. I can fork and try to do it myself if it's not a feature you'd like to add.

@whalelephant
Copy link

Hi, I am also looking for this because we have similar contracts with minor differences for different chains and will communicate with each other via IBC. They are tested and compiled with the features flag for different features.

@CyberHoward / @webmaster128 any updates on your ends?

@kerber0x
Copy link
Contributor

I am also looking for the same feature, is this something that you guys are considering implementing? @webmaster128

@webmaster128
Copy link
Member

webmaster128 commented Oct 20, 2022

To me this is basically blocked by reproducible builds. When you don't need reproducible builds, you can just compile your *.wasms like this.

If you look at the discussions around gov proposals on Osmosis (most recent example here but happened multiple times before), people already have a very hard time providing the relevant information to allow contract verification. At the moment you need 1. source code, 2. builder image incl. version. I don't want to create the need for a 3rd parameter here which would be builder options.

I don't see a solution that adds this flexibility without increasing the communication burden and the burden on verifiers. This is why I think those things should like in the source code with one (wrapper) contract for each target chain.

@ethanfrey
Copy link
Member

I fully understand the request, and we should provide some easy solution here.
I also fully support Simon's concerns about reproducibility.
We will have to find some solution here or I smell some forks.

My first thought is this.

  1. Create your basic package my-protocol-base with all the various feature flags
  2. For each chain/feature, create a new minimal package my-proto-osmo, my-proto-terra. This package would need two thing:
    a. Cargo.toml importing my-protocol-base with the feature enabled
    b. a super minimal lib.rs that just re-exports the msg/entry points (if needed for force import and compiling wasm)

Then rust-optimiser will run proper on my-proto-osmo, my-proto-terra and they would have no code dup (just a git repo saying "compile with this feature flag").

I have no tried it, but would be happy for someone with such a project to try it out and report. If it works, we could at least document this possibility. This does not preclude adding support for feature flags in the optimiser, but would be a valid solution short-term, while we discuss if this is the best long-term solution, or use other.

It does not involve changing core tools, but can be opted in per project, which is easy for adoption

@webmaster128
Copy link
Member

#129 shows a promising approach to solve this problem by letting the optimizer build multiple versions of the contract without taking additional build arguments. It keeps the problem as simple as source code + builder image for the folks reproducing.

@dakom
Copy link

dakom commented Sep 21, 2023

fwiw the way we're tackling this is perhaps a bit funny, but it works:

SETUP

  1. Contract has default = [] in its [features]
  2. All building goes through a thin shell script with env vars for arm64 or not, features or not, etc. to control things

TYPICAL RUN

  1. just runs the optimizer as you'd expect (or via arm64 version)

FEATURES RUN

  1. Replaces default = [] with the features (e.g. default = ["foo"]) via a sed one-liner
  2. Builds
  3. Adds a feature suffix to the output filename
  4. Sets the default back to blank

Yeah it's a bit goofy, but it works and doesn't need any special support from the optimizer

@apollo-sturdy
Copy link

@dakom Would you be able to share the one liner? Need a quick fix to run on our CI.

@dakom
Copy link

dakom commented Sep 27, 2023

I didn't write this shell script, someone else did, and I suck at shell scripting... won't be able to answer any questions, but hope it helps!

The setup here is that the feature is only needed on the market contract and all contracts are in contracts/

also OPTIMIZER_ARM64 is an env var to use the arm64 optimizer and SEI is an env var to set the feature for sei

lastly, gsed needs to be installed for it to work on OSX

#!/usr/bin/env bash

set -euxo pipefail

SCRIPT=$(readlink -f "$0")
SCRIPTPATH=$(dirname "$SCRIPT")
cd "$SCRIPTPATH"
cd ..

WASM_DIR="$(pwd)/wasm"
TARGET_CACHE="$WASM_DIR/target"
REGISTRY_CACHE="$WASM_DIR/registry"
CARGO_GIT_CACHE="$WASM_DIR/git"
ARTIFACTS="$WASM_DIR/artifacts"

if [[ -n "${OPTIMIZER_ARM64:-}" ]]; then
    echo "sed on OSX is weird. Use gsed instead"
    SED=gsed
else
    SED=sed
fi

if [ -n "${SEI:-}" ]; then
    echo "If this script failed, it would left extra \`default = [\"sei\"]\` line in contracts' Cargo.toml."
    
    for i in contracts/market/; do
        grep -q '^default = \["sei"\]$' "${i}/Cargo.toml" || $SED -i -e '/\[features\]/ a default = ["sei"]' "${i}/Cargo.toml"
    done
fi

if [[ -n "${OPTIMIZER_ARM64:-}" ]]; then
    OPTIMIZER_VERSION="cosmwasm/workspace-optimizer-arm64":0.14.0
else
    OPTIMIZER_VERSION="cosmwasm/workspace-optimizer":0.14.0
fi

mkdir -p "$TARGET_CACHE" "$REGISTRY_CACHE" "$ARTIFACTS" "$CARGO_GIT_CACHE"

# Delete the old file to avoid false positives if the compilation fails
rm -f "$WASM_DIR/artifacts/gitrev"

docker  run --rm --tty \
-u "$(id -u)":"$(id -g)" \
-v "$(pwd)":/code \
-v "$TARGET_CACHE":/target \
-v "$ARTIFACTS":/code/artifacts \
-v "$REGISTRY_CACHE":/usr/local/cargo/registry \
-v "$CARGO_GIT_CACHE":/usr/local/cargo/git \
$OPTIMIZER_VERSION

if [ -n "${SEI:-}" ]; then
    for i in "${ARTIFACTS}/"*market*; do
        mv "${i}" "${i%.wasm}-sei.wasm"
    done
fi

# not sure how this was created since we mapped the tool's /code/artifacts
# but it's empty (the real artifacts are in wasm/artifacts)
rm -rf ./artifacts

# Only write the gitrev file on success
git rev-parse HEAD > "$WASM_DIR/artifacts/gitrev"

if [ -n "${SEI:-}" ]; then
    for i in contracts/market/; do
        $SED -i -e '/default = \["sei"\]/ d' "${i}/Cargo.toml"
    done
fi

@webmaster128
Copy link
Member

Could you check #148? This should solve the issue.

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

No branches or pull requests

8 participants