Skip to content

fix wasm host-in-guest alloc#4729

Merged
tzemanovic merged 4 commits intomainfrom
tomas/fix-wasm-alloc
Jul 17, 2025
Merged

fix wasm host-in-guest alloc#4729
tzemanovic merged 4 commits intomainfrom
tomas/fix-wasm-alloc

Conversation

@tzemanovic
Copy link
Copy Markdown
Collaborator

@tzemanovic tzemanovic commented Jul 11, 2025

Describe your changes

This fixes an issue with tx and VP WASM inputs that are larger than the initial WASM memory and hence require to grow memory. It turned out that simply calling memory.grow for this (as performed by memory::write_memory_bytes from host didn't update the guest properly which was then overriding the already used memory ranges from the initial data and hence corrupting the memory, leading to WASM runtime memory errors.

To fix this, we're now injecting and exporting a simple alloc fn into WASMs and calling it from host to write the initial data.

Additionally, the max WASM memory size is increased as with the current limit it was not possible to decode a tx with 2 MiB of data (the newly added wasm::run::tests::test_tx_alloc test would fail with unreachable when it runs out of memory).

For a tx with data that fits in the initial memory size there is no change in WASM execution (the alloc call will return early).

The Rust src for the alloc fn that's being injected is here https://github.com/namada-net/namada-wasm-min-alloc

Checklist before merging

  • If this PR has some consensus breaking changes, I added the corresponding breaking:: labels
    • This will require 2 reviewers to approve the changes
  • If this PR requires changes to the docs or specs, a corresponding PR is opened in the namada-docs repo
    • Relevant PR if applies:
  • If this PR affects services such as namada-indexer or namada-masp-indexer, a corresponding PR is opened in that repo
    • Relevant PR if applies:

tzemanovic added a commit that referenced this pull request Jul 11, 2025
@tzemanovic tzemanovic requested review from sug0 and yito88 July 11, 2025 13:10
@tzemanovic tzemanovic added wasm breaking:consensus Consensus breaking change that requires a hard-fork labels Jul 11, 2025
@tzemanovic tzemanovic marked this pull request as ready for review July 11, 2025 13:11
@github-actions github-actions bot added the breaking:api public API breaking change label Jul 14, 2025
@tzemanovic tzemanovic removed the breaking:api public API breaking change label Jul 14, 2025
@github-actions github-actions bot added the breaking:api public API breaking change label Jul 14, 2025
@tzemanovic tzemanovic removed the breaking:api public API breaking change label Jul 15, 2025
@sug0
Copy link
Copy Markdown
Collaborator

sug0 commented Jul 15, 2025

quick review of namada-wasm-min-alloc:
https://github.com/namada-net/namada-wasm-min-alloc/blob/5e654ca43c93bf74dfa5ce6628fae35e82dbb3b4/src/lib.rs#L43-L46
shouldn't the panic handler simply be wasm32::unreachable()?

@tzemanovic
Copy link
Copy Markdown
Collaborator Author

tzemanovic commented Jul 15, 2025

quick review of namada-wasm-min-alloc: https://github.com/namada-net/namada-wasm-min-alloc/blob/5e654ca43c93bf74dfa5ce6628fae35e82dbb3b4/src/lib.rs#L43-L46 shouldn't the panic handler simply be wasm32::unreachable()?

it doesn't matter, it doesn't affect the generated code as it's panic-free

@sug0 sug0 self-requested a review July 16, 2025 07:54
@tzemanovic tzemanovic force-pushed the tomas/fix-wasm-alloc branch from 8b83686 to 8b782fb Compare July 16, 2025 18:10
tzemanovic added a commit that referenced this pull request Jul 16, 2025
* tomas/fix-wasm-alloc:
  changelog: add #4729
  avoid growing memory if the initial data fits current mem size
  increase the wasm memory limits
  Fix wasm host-in-guest alloc by injecting, exporting and using fn
tzemanovic added a commit that referenced this pull request Jul 17, 2025
* tomas/fix-wasm-alloc:
  changelog: add #4729
  avoid growing memory if the initial data fits current mem size
  increase the wasm memory limits
  Fix wasm host-in-guest alloc by injecting, exporting and using fn
@tzemanovic tzemanovic merged commit 0f23902 into main Jul 17, 2025
2 checks passed
@tzemanovic tzemanovic deleted the tomas/fix-wasm-alloc branch July 17, 2025 12:57
sug0 pushed a commit that referenced this pull request Nov 19, 2025
@sug0 sug0 mentioned this pull request Nov 19, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking:consensus Consensus breaking change that requires a hard-fork wasm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants