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

Address CWA 2022 002 #178

Merged
merged 5 commits into from
Apr 6, 2022
Merged

Address CWA 2022 002 #178

merged 5 commits into from
Apr 6, 2022

Conversation

the-frey
Copy link
Collaborator

@the-frey the-frey commented Apr 6, 2022

Bump wasmvm to patch vulnerability

Copy link
Collaborator

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Dockerfile needs to be updated as well. Otherwise the old static library (.a) is used.

Copy link
Contributor

@faddat faddat left a comment

Choose a reason for hiding this comment

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

:).

This looks great and I've test-compiled on my machine as well

@webmaster128
Copy link
Collaborator

@the-frey
Copy link
Collaborator Author

the-frey commented Apr 6, 2022

Yep, just spotted the dockerfile issue. Also noticed you've coined a new wasmd, so will just bump 0.24 to that version. Thanks @webmaster128

@the-frey
Copy link
Collaborator Author

the-frey commented Apr 6, 2022

Aha, of course I need to update the CI steps as well. Probably the docker packages too. Right

Dockerfile Outdated Show resolved Hide resolved
@webmaster128
Copy link
Collaborator

webmaster128 commented Apr 6, 2022

Also check https://github.com/CosmWasm/wasmd/wiki/Maintainer-notes for how to do the different docker builds in the multi-architecture world.

Also note: there are no consensus guarantees for ARM yet. Use it in a client, dev network or read-only node. But not on a validator.

@faddat
Copy link
Contributor

faddat commented Apr 6, 2022

I am going to delete the dockerfile so that we can reduce scope and get this merged. @webmaster128 thank you for your review!

removed from juno repo to reduce scope
@faddat
Copy link
Contributor

faddat commented Apr 6, 2022

I should give some context on deleting the dockerfile.

I would prefer to build the rust library here. When we don't build everything from source, we invite problems like the one that we saw here.

Anyone know if we can easily recreate the contract ci stuff without a dockerfile?

Another alternative is later when I've more time I can write separate arm and x86 dockerfiles. This is because when i use env and arg for multiarch, it never works out right.

faddat added 2 commits April 7, 2022 00:06
this is deleted because the script it depends on is no longer present.  We should restore this functionality.
deleting this because we don't have a dockerfile at the moment
@JakeHartnell JakeHartnell merged commit 3ac79a1 into main Apr 6, 2022
@JakeHartnell JakeHartnell deleted the cwa_2022_002 branch April 6, 2022 17:17
@giansalex
Copy link
Member

when deleting the dockerfile there will also be an error in release ci

@webmaster128
Copy link
Collaborator

webmaster128 commented Apr 6, 2022

I should give some context on deleting the dockerfile.

I would prefer to build the rust library here. When we don't build everything from source, we invite problems like the one that we saw here.

I'm not against this approach. But please note that now you are not building libwasmvm here. You are using the shared libraries libwasmvm.dylib and libwasmvm.so via the Go dependency from git. This makes it a bit easier for some use cases, but gives you dynamically linked glibc Linux builds (Intel only) and dynamically linked Mac build (Intel and ARM) which might not be desired. There is probably no straight forward Alpine support that way.

@the-frey
Copy link
Collaborator Author

the-frey commented Apr 6, 2022

Reinstating Dockerfile in #179
Initially will probably just be AMD/Intel, ARM to follow

Copy link
Contributor

@faddat faddat left a comment

Choose a reason for hiding this comment

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

so we regressed after this?

@webmaster128
Copy link
Collaborator

so we regressed after this?

This PR installs beta7, not beta10 because beta7 > beta10. See go list -m github.com/CosmWasm/wasmvm and

$ go list -m -versions github.com/CosmWasm/wasmvm
github.com/CosmWasm/wasmvm v0.1.0 v0.2.0 v0.3.0 v0.3.1 v0.3.2 v0.3.3 v0.4.0 v0.4.1 v0.6.0 v0.6.1 v0.6.2 v0.6.3 v0.7.0 v0.7.1 v0.7.2 v0.8.0-alpha2 v0.8.0-alpha3 v0.8.0 v0.8.1 v0.9.0-alpha2 v0.9.0-alpha3 v0.9.0-beta v0.9.0 v0.9.1 v0.9.4 v0.10.0-alpha2 v0.10.0-alpha3 v0.10.0-alpha4 v0.10.0 v0.11.0-alpha1 v0.11.0-alpha2 v0.11.0-rc v0.11.0 v0.12.0-alpha1 v0.12.0-alpha2 v0.12.0-alpha3 v0.12.0 v0.13.0 v0.13.1 v0.14.0-alpha1 v0.14.0-alpha2 v0.14.0-beta1 v0.14.0-beta2 v0.14.0-beta3 v0.14.0-beta4 v0.14.0-beta5 v0.14.0-rc1 v0.14.0 v0.15.0-rc1 v0.15.0 v0.15.1 v0.16.0-rc1 v0.16.0-rc2 v0.16.0-rc3 v0.16.0 v0.16.1 v0.16.2 v0.16.3 v0.16.5 v0.16.6 v0.16.7 v1.0.0-beta v1.0.0-beta10 v1.0.0-beta2 v1.0.0-beta3 v1.0.0-beta4 v1.0.0-beta5 v1.0.0-beta6 v1.0.0-beta7 v1.0.0-beta8 v1.0.0-beta9 v1.0.0-soon v1.0.0-soon2

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.

5 participants