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

Failure on optimized build of codebase for Ethereum address recovery from signature #1551

Closed
jpine3528 opened this issue Dec 19, 2022 · 13 comments
Labels
Question ❓ User question

Comments

@jpine3528
Copy link

We are running this command to generated optimized version of wasm file for cosmwasm chain upload.

docker run --rm -v "$(pwd)":/code \
  --mount type=volume,source="$(basename "$(pwd)")_cache",target=/code/target \
  --mount type=volume,source=registry_cache,target=/usr/local/cargo/registry \
  cosmwasm/workspace-optimizer:0.12.6

Following issue is being reported consistently

error: failed to run custom build command for `secp256k1-sys v0.4.2`
...
  running: "clang" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=wasm32-unknown-unknown" "-I" "depend/secp256k1/" "-I" "depend/secp256k1/include" "-I" "depend/secp256k1/src" "-I" "wasm-sysroot" "-Wall" "-Wextra" "-DSECP256K1_API=" "-DENABLE_MODULE_ECDH=1" "-DENABLE_MODULE_SCHNORRSIG=1" "-DENABLE_MODULE_EXTRAKEYS=1" "-DUSE_NUM_NONE=1" "-DUSE_FIELD_INV_BUILTIN=1" "-DUSE_SCALAR_INV_BUILTIN=1" "-DECMULT_GEN_PREC_BITS=4" "-DECMULT_WINDOW_SIZE=15" "-DUSE_EXTERNAL_DEFAULT_CALLBACKS=1" "-DENABLE_MODULE_RECOVERY=1" "-o" "/code/target/wasm32-unknown-unknown/release/build/secp256k1-sys-b02908e5d224821e/out/depend/secp256k1/contrib/lax_der_parsing.o" "-c" "depend/secp256k1/contrib/lax_der_parsing.c"
  --- stderr
  error occurred: Failed to find tool. Is `clang` installed?

This issue started to happen when web3 package is added for Ethereum signature verification.
https://github.com/tomusdrw/rust-web3

web3 has dependency of secp256k1 which is not able to be installed using the docker image.
Here's one of the relevant research that was answering what could be the issue.
https://substrate.stackexchange.com/questions/1098/how-to-use-sp-core-in-libraries-that-target-wasm-for-the-web?rq=1

But for optimized build, clang version on docker image won't be able to be used.
It would be nice to get docker image updated to get this working on next release or at least provide alternative of signature verification method.

We are not using Cosmos signature as we believe storing pubkey for signer is not ideal to be put on the contract.
https://docs.cosmwasm.com/docs/0.14/CHANGELOG/

It would be nice to provide optimized direction for the issue. We can provide more details if there's more input. Since our codebase is open-source, the link can be provided as well for reproduction.

@webmaster128
Copy link
Member

The secp256k1-sys crate is "FFI for Pieter Wuille's libsecp256k1 library.". Compiling and linking C code to Wasm is out of scope for rust-optimizer/workspace-optimizer. I don't know if that works at all. A pure Rust implementation should be used instead.

In this particular case however, I wonder if you are better off using the CosmWasm crypto APIs instead of compiling secp256k1 verification into the contract. In https://github.com/CosmWasm/cosmwasm/blob/main/contracts/crypto-verify/src/ethereum.rs you see how you can do secp256k1 verfication using those APIs.

We are not using Cosmos signature as we believe storing pubkey for signer is not ideal to be put on the contract.

I don't understand how this related to Cosmos or Ethereum and which implementation is used. If a contract should do a verification, this is a public operation and the public has access to the public key. You can either store it in the contract or pass it as an argument as part of the transaction. Either way it will be public.

@webmaster128
Copy link
Member

webmaster128 commented Dec 19, 2022

I had a brief look into the web3 codebase. In https://github.com/tomusdrw/rust-web3/blob/0a32ab5a0ca0a8943157174c702069b22f166f08/Cargo.toml#L72 you see that the wasm feature there required wasm-bindgen and all sort of JF stuff. At least wasm-bindgen and getrandom make it impossible to run this code in CosmWasm as it creates wasm-bindgen specific imports and exports that are not accessible when the Wasm blob is executed in CosmWasm. So the web3 dependency seems to be a dead end.

@jpine3528
Copy link
Author

We will try to use another mechanism natively supported by cosmwasm. Thanks for sharing this link.
https://github.com/CosmWasm/cosmwasm/blob/main/contracts/crypto-verify/src/ethereum.rs
Is there any example cosmwasm project using this function?

@webmaster128
Copy link
Member

webmaster128 commented Dec 20, 2022

The project https://github.com/CosmWasm/cosmwasm/blob/main/contracts/crypto-verify is an example contract already. Nothing live and suitable for production but this is how the contract code looks like. In https://github.com/CosmWasm/cosmwasm/blob/v1.1.9/contracts/crypto-verify/src/ethereum.rs#L30 and https://github.com/CosmWasm/cosmwasm/blob/v1.1.9/contracts/crypto-verify/src/ethereum.rs#L35 you see where the secp256k1 APIs are called.

@jpine3528
Copy link
Author

jpine3528 commented Dec 23, 2022

I have modified the contract codebase to

cargo.toml

[dependencies]
multiswap = { path = "../../packages/multiswap", version = "0.14.0" }
cw-storage-plus = { version = "0.14.0" } 
cw-utils = { version = "0.14.0" } 
cosmwasm-std = { version = "1.0.0" }
schemars = "0.8.1"
serde = { version = "1.0.103", default-features = false, features = ["derive"] }
thiserror = { version = "1.0.20" }
rlp = "0.5"
hex = { version = "0.4"}
sha3 = "0.10"
serde_json = { version = "1.0", default-features = false, features = ["alloc"] }

[dev-dependencies]
cosmwasm-schema = { version = "1.0.0" }

use sha3::{Digest, Keccak256};
use std::convert::TryInto;

... 

// get_signer calculate signer from withdraw signed message parameters
pub fn get_signer(
    api: &dyn Api,
    chain_id: String,
    payee: String,
    token: String,
    amount: Uint128,
    salt: String,
    signature: String,
) -> String {
    // get sign message to be used for signature verification
    let message_obj = WithdrawSignMessage {
        chain_id: chain_id,
        payee: payee,
        token: token,
        amount: amount,
        salt: salt,
    };

    // Serialize it to a JSON string.
    let message_encoding = serde_json::to_string(&message_obj);
    let mut message = "".to_string();
    if let Ok(message_str) = message_encoding {
        message = message_str.to_string()
    }

    let message = Keccak256::digest(
        format!(
            "{}{}{}",
            "\x19Ethereum Signed Message:\n",
            message.len(),
            message
        )
        .as_bytes(),
    );
    let signature = hex::decode(signature).unwrap();

    let recovery_id = signature[64] - 27;
    let calculated_pubkey = api
        .secp256k1_recover_pubkey(&message, &signature[..64], recovery_id)
        .unwrap();
    let calculated_address = ethereum_address_raw(&calculated_pubkey).unwrap();
    let address = format!("0x{}", hex::encode(calculated_address));
    return address;
}

Optimized build succeed, but when I try to deploy - I get following issue.

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 import: "env.abort". Required imports: {"env.abort", "env.addr_canonicalize", "env.addr_humanize", "env.addr_validate", "env.db_next", "env.db_read", "env.db_remove", "env.db_scan", "env.db_write", "env.debug", ... 5 more}. Available imports: ["env.db_read", "env.db_write", "env.db_remove", "env.addr_validate", "env.addr_canonicalize", "env.addr_humanize", "env.secp256k1_verify", "env.secp256k1_recover_pubkey", "env.ed25519_verify", "env.ed25519_batch_verify", "env.debug", "env.query_chain", "env.db_scan", "env.db_next"].: create wasm contract failed: invalid request

Can you share me the cause of the issue? FYI, I am trying to deploy on cudos testnet. (Used the reference you have shared)

At the moment, cudos testnet is using github.com/CosmWasm/wasmd v0.25.0 - wasmvm 1.0.0-beta10, cosmwasm-std 1.0

@webmaster128
Copy link
Member

env.abort was added at some point during the CosmWasm 1.0 beta time. It is a panic handler that you probably want to have. The best solution is to update wasmvm/wasmd chain side.

As a workaround you can try to deactivate the abort feature in cosmwasm-std. But for that you have to deactivate the default features and re-activate the ones you need.

@jpine3528
Copy link
Author

jpine3528 commented Dec 26, 2022

I see, we have raised an issue ticket on Cudos to upgrade the chain, cudoventures/cudos-node#69
But it's not the chain maintained by ourselves.
Is there an example codebase that imports except env.abort?

@webmaster128
Copy link
Member

Is there an example codebase that imports except env.abort?

you need to deactivate the abort feature of cosmwasm-std. Something like this.

@jpine3528
Copy link
Author

jpine3528 commented Dec 27, 2022

Made following changes

cosmwasm-std = { version = "1.0.0", default-features = false, features = ["staking"] }

And when I am trying to upload to the chain after optimized build, same issue persists.

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 import: "env.abort". Required imports: {"env.abort", "env.addr_canonicalize", "env.addr_humanize", "env.addr_validate", "env.db_next", "env.db_read", "env.db_remove", "env.db_scan", "env.db_write", "env.debug", ... 5 more}. Available imports: ["env.db_read", "env.db_write", "env.db_remove", "env.addr_validate", "env.addr_canonicalize", "env.addr_humanize", "env.secp256k1_verify", "env.secp256k1_recover_pubkey", "env.ed25519_verify", "env.ed25519_batch_verify", "env.debug", "env.query_chain", "env.db_scan", "env.db_next"].: create wasm contract failed: invalid request

Btw, what I have detected is cargo.lock is still using 1.1.9 though I wanted to use 1.1.0 with just "staking" feature.

[[package]]
name = "cosmwasm-std"
version = "1.1.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b13d5a84d15cf7be17dc249a21588cdb0f7ef308907c50ce2723316a7d79c3dc"
dependencies = [
 "base64",
 "cosmwasm-crypto",
 "cosmwasm-derive",
 "derivative",
 "forward_ref",
 "hex",
 "schemars",
 "serde",
 "serde-json-wasm",
 "thiserror",
 "uint",
]

This issue didn't use to exist even though it's an old version of cosmwasm chain when we don't use the ethereum signature related package from cosmwasm, only when it's used, this issue exists.

@webmaster128
Copy link
Member

1.1.9 vs. 1.1.0 should not make any difference. If you really need to pin to a patch version, you need to use =:

cosmwasm-std = { version = "=1.0.0", default-features = false, features = ["staking"] }

Most likely the issue is that some other dependency enables the abort feature. What's your output of cargo tree -i cosmwasm-std?

@webmaster128 webmaster128 added the Question ❓ User question label Jan 2, 2023
@jpine3528
Copy link
Author

jpine3528 commented Jan 5, 2023

Here's the tree, when I run the command

cargo tree -i cosmwasm-std
cosmwasm-std v1.1.9
├── cw-storage-plus v0.14.0
│   ├── cw2 v0.14.0
│   │   └── cw-utils v0.14.0
│   │       ├── fiberrouter v0.14.0 (<route>/ferrum-multiswap-contracts/packages/fiberrouter)
│   │       │   └── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
│   │       ├── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
│   │       ├── multiswap v0.14.0 (<route>/ferrum-multiswap-contracts/packages/multiswap)
│   │       │   ├── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
│   │       │   └── multiswap-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/multiswap-base)
│   │       └── multiswap-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/multiswap-base)
│   ├── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
│   └── multiswap-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/multiswap-base)
├── cw-utils v0.14.0 (*)
├── cw2 v0.14.0 (*)
├── fiberrouter v0.14.0 (<route>/ferrum-multiswap-contracts/packages/fiberrouter) (*)
├── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
├── multiswap v0.14.0 (<route>/ferrum-multiswap-contracts/packages/multiswap) (*)
└── multiswap-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/multiswap-base)

After updating to use "=1.0.0", here's the tree

cosmwasm-std v1.0.0
├── cw-storage-plus v0.14.0
│   ├── cw2 v0.14.0
│   │   └── cw-utils v0.14.0
│   │       ├── fiberrouter v0.14.0 (<route>/ferrum-multiswap-contracts/packages/fiberrouter)
│   │       │   └── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
│   │       ├── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
│   │       ├── multiswap v0.14.0 (<route>/ferrum-multiswap-contracts/packages/multiswap)
│   │       │   ├── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
│   │       │   └── multiswap-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/multiswap-base)
│   │       └── multiswap-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/multiswap-base)
│   ├── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
│   └── multiswap-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/multiswap-base)
├── cw-utils v0.14.0 (*)
├── cw2 v0.14.0 (*)
├── fiberrouter v0.14.0 (<route>/ferrum-multiswap-contracts/packages/fiberrouter) (*)
├── fiberrouter-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/fiberrouter-base)
├── multiswap v0.14.0 (<route>/ferrum-multiswap-contracts/packages/multiswap) (*)
└── multiswap-base v0.0.1 (<route>/ferrum-multiswap-contracts/contracts/multiswap-base)

cw2 or cw-utils could be using abort feature probably?

@jpine3528
Copy link
Author

Thanks for your support @webmaster128
Used =1.0.0 and was able to successfully upload the contract on cudos blockchain.

@webmaster128
Copy link
Member

Glad it works. In 1.0.0 the "about" feature is not enabled by default, which explains why this works.

I think cw2 or cw-utils enables default features, which they should not do in order to allow contract developers to disable things like abort. That's one issue we should solve.

And then you should push your chain developers to upgrade CosmWasm since the abort feature is very useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question ❓ User question
Projects
None yet
Development

No branches or pull requests

2 participants