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

feat: enable sign ext for wasm generated with rust 1.70 #344

Merged
merged 4 commits into from
Feb 7, 2024

Conversation

loloicci
Copy link
Contributor

@loloicci loloicci commented Feb 6, 2024

Description

This PR enable sign-ext feature of parity wasm.
This allows wasm to use some additional operators which are sometimes used in wasm generated with rust1.70.

This fix #338.
And, this will be replaced with wasmparser in a feature version (CosmWasm/cosmwasm#1786).

Types of changes

  • Bug fix (changes which fixes an issue)
  • New feature (changes which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • ETC (build, ci, docs, perf, refactor, style, test)

Checklist

loloicci and others added 2 commits February 6, 2024 18:58
This fix Finschia#338.
And, this will be replaced with wasmparser in feature version.
(CosmWasm/cosmwasm#1786)
@loloicci loloicci self-assigned this Feb 6, 2024
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ loloicci
❌ webmaster128
You have signed the CLA already but the status is still pending? Let us recheck it.

@loloicci loloicci changed the title Enable sign ext Enable sign ext for wasm generated with rust v1.70 Feb 6, 2024
@loloicci loloicci changed the title Enable sign ext for wasm generated with rust v1.70 Enable sign ext for wasm generated with rust 1.70 Feb 6, 2024
@loloicci loloicci changed the title Enable sign ext for wasm generated with rust 1.70 feat: enable sign ext for wasm generated with rust 1.70 Feb 6, 2024
@tkxkd0159 tkxkd0159 self-requested a review February 7, 2024 00:38
Copy link
Member

@tkxkd0159 tkxkd0159 left a comment

Choose a reason for hiding this comment

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

Why don't we replace parity-wasm with wasmer::wasmparser?
Upstream already applied it and I assume there is no dependency to apply this.

ref. CosmWasm/cosmwasm#1786

@zemyblue zemyblue self-requested a review February 7, 2024 01:43
@170210
Copy link
Contributor

170210 commented Feb 7, 2024

@tkxkd0159 It has already been merged into the bump_1.5.1 branch, do you mean that we need to merge it into the main branch first? https://github.com/Finschia/cosmwasm/blob/feat/bump_1.5.1/packages/vm/src/cache.rs#L17

@loloicci
Copy link
Contributor Author

loloicci commented Feb 7, 2024

@tkxkd0159

Why don't we replace parity-wasm with wasmer::wasmparser?

This is because some interfaces of wasmparser in wasmer 4.1.0 used by CosmWasm/cosmwasm#1786 is not compatible with wasmer 2.3.0 which we use in main.
So, as I said in the description, this will be replaced in future version.

this will be replaced with wasmparser in a feature version (CosmWasm/cosmwasm#1786).

@tkxkd0159
Copy link
Member

@tkxkd0159

Why don't we replace parity-wasm with wasmer::wasmparser?

This is because some interfaces of wasmparser in wasmer 4.1.0 used by CosmWasm/cosmwasm#1786 is not compatible with wasmer 2.3.0 which we use in main. So, as I said in the description, this will be replaced in future version.

this will be replaced with wasmparser in a feature version (CosmWasm/cosmwasm#1786).

Okay, I understand now. LGTM

@tkxkd0159 tkxkd0159 self-requested a review February 7, 2024 06:06
@loloicci loloicci merged commit 62a4a24 into Finschia:main Feb 7, 2024
20 of 21 checks passed
@loloicci loloicci deleted the enable-sign-ext branch February 7, 2024 09:55
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.

Compiling Wasm with Rust 1.70 >= leads to unsupported Wasm opcodes
6 participants