DA0-DA0 / dao-contracts

CosmWasm smart contracts for Interchain DAOs.
https://docs.daodao.zone
BSD 3-Clause "New" or "Revised" License
207 stars 139 forks source link

Using `voting` in lib-crate makes this lib export `cw_core` functions on linux #483

Closed Buckram123 closed 1 year ago

Buckram123 commented 2 years ago

Short description

So, what happened: we have a lib that wants to import voting, and then contract depend on that lib, what happens: cw-core gets imported (with everything)

Error

note: rust-lld: error: duplicate symbol: instantiate
          >>> defined in /code/target/wasm32-unknown-unknown/release/deps/duplicate_bug.duplicate_bug.8f61a3e5-cgu.0.rcgu.o
          >>> defined in /code/target/wasm32-unknown-unknown/release/deps/libcw_core-bfb32c933257001e.rlib(cw_core-bfb32c933257001e.cw_core.0b07bdab-cgu.0.rcgu.o)

          rust-lld: error: duplicate symbol: execute
          >>> defined in /code/target/wasm32-unknown-unknown/release/deps/duplicate_bug.duplicate_bug.8f61a3e5-cgu.0.rcgu.o
          >>> defined in /code/target/wasm32-unknown-unknown/release/deps/libcw_core-bfb32c933257001e.rlib(cw_core-bfb32c933257001e.cw_core.0b07bdab-cgu.0.rcgu.o)

          rust-lld: error: duplicate symbol: query
          >>> defined in /code/target/wasm32-unknown-unknown/release/deps/duplicate_bug.duplicate_bug.8f61a3e5-cgu.0.rcgu.o
          >>> defined in /code/target/wasm32-unknown-unknown/release/deps/libcw_core-bfb32c933257001e.rlib(cw_core-bfb32c933257001e.cw_core.0b07bdab-cgu.0.rcgu.o)

Minimal example

https://github.com/Buckram123/duplicate_example This is minimal template from here: https://github.com/InterWasm/cw-template One difference is here: https://github.com/Buckram123/duplicate_example/blob/57859b4e0997b7eba359b3da393ae55f2b563d30/duplicate-bug/src/msg.rs#L1 And instantiate method

Reproduce

To reproduce on my machine, simply run optimizer or just wasm

cargo build --release --target wasm32-unknown-unknown
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/rust-optimizer:0.12.6

Version

rustc 1.63.0 (4b91a6ea7 2022-08-08), (optimizer uses 1.60.0) 5.19.8-arch1-1

0xekez commented 2 years ago

thanks for reporting!

@Buckram123: cw-core gets imported (with everything)

can we get around this by adding the library feature to the cw-core import in voting, or is this something more complex? would just need to do that here: https://github.com/DA0-DA0/dao-contracts/blob/693a5d6a36ada1505c5a52e8029b1b80ea557393/packages/voting/Cargo.toml#L15

de-husk commented 2 years ago

Contracts being used by other packages/contracts as library dependencies has caused other problems (and likely will be a hard to debug landmine for future devs even once these are ironed out). The ones I know about:

Honestly, I think we should see how bad it is to just split out the shared library code out of the contracts and into packages. What do you think?

I think its fine to keep them as dev-deps, and used by things that aren't depended on by other packages.

Buckram123 commented 2 years ago

@ezekiiel you are right it is enough, you can check fix branch here: https://github.com/Buckram123/duplicate_example

Buckram123 commented 2 years ago

@de-husk but I agree with you, IMO it's better to not use contracts as a dependency, should be easy to remove that dependency, it's only one function. What you think is the best solution?

0xekez commented 2 years ago

definitely think @de-husk is right here. :) the structure of having a package with the external api and the contract is slick.

this would be a nice refactor to get in with the renaming work, but also adding library flags is fast 🤷‍♂️

JakeHartnell commented 1 year ago

This would be a good thing to get in before the v2 release.

JakeHartnell commented 1 year ago

@Buckram123 is this still an issue since we switch to rust workspaces?

Buckram123 commented 1 year ago

@Buckram123 is this still an issue since we switch to rust workspaces?

Don't think so, updated this repo, still same issue, if I didn't miss anything. This time I used rust-optimizer:0.12.10 which uses 1.65.0 rustc

JakeHartnell commented 1 year ago

Closing. Feel free to re-open if it re-appears.