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

Update polkavm to the upstream toolchain version #6335

Closed
jarkkojs opened this issue Nov 1, 2024 · 4 comments · Fixed by #6419
Closed

Update polkavm to the upstream toolchain version #6335

jarkkojs opened this issue Nov 1, 2024 · 4 comments · Fixed by #6419
Assignees
Labels
I10-unconfirmed Issue might be valid, but it's not yet known.

Comments

@jarkkojs
Copy link
Contributor

jarkkojs commented Nov 1, 2024

Overview

  1. Bump the polkavm(-*)? dependency everywhere.
  2. This will require you to update the executor as it is using polkavm to execute runtimes.(first link)
  3. Update the wasm-builder to use an upstream toolchain (as I did in 6266 for the fixtures). This produces the artifacts used by the executor.

Ideally, wasm-builder could be used to build the test fixtures. However, that is not feasible because it allows only one crate per package. Therefore, a separate build script was implemented in #6266 them.

Relevant Paths

Toolchains

  • Old: riscv32ema-unknown-none-elf
  • New: riscv32emac-unknown-none-polkavm

Test Case

cd polkadot/node/core/pvf
cargo test
@github-actions github-actions bot added the I10-unconfirmed Issue might be valid, but it's not yet known. label Nov 1, 2024
@jarkkojs
Copy link
Contributor Author

jarkkojs commented Nov 1, 2024

The first thing I did:

$ git grep -l riscv32ema-unknown-none-elf | xargs sed -i 's/riscv32ema-unknown-none-elf/riscv32emac-unknown-none-polkavm'

@jarkkojs
Copy link
Contributor Author

jarkkojs commented Nov 1, 2024

@koute Is that a legit test case? I saw that runners set up in src/testing.rs by default set SKIP_WASM_BUILD.

@jarkkojs jarkkojs self-assigned this Nov 1, 2024
@koute
Copy link
Contributor

koute commented Nov 2, 2024

@koute Is that a legit test case? I saw that runners set up in src/testing.rs by default set SKIP_WASM_BUILD.

I'm not entirely sure what you're asking about here, but, the way to test it would be essentially something like this (writing this from memory, so not 100% sure if this still works):

SUBSTRATE_RUNTIME_TARGET=riscv SUBSTRATE_ENABLE_POLKAVM=1 RUST_LOG=polkavm=debug cargo run --release -p staging-node-cli

Basically, setting SUBSTRATE_RUNTIME_TARGET=riscv will make the wasm-builder crate build a RISC-V runtime instead of a WASM runtime, and SUBSTRATE_ENABLE_POLKAVM=1 will allow the executor to load that blob (by default this code path is disabled). With this you should be able to use a normal node client to run a local test chain using it.

@jarkkojs
Copy link
Contributor Author

jarkkojs commented Nov 2, 2024

@koute Is that a legit test case? I saw that runners set up in src/testing.rs by default set SKIP_WASM_BUILD.

I'm not entirely sure what you're asking about here, but, the way to test it would be essentially something like this (writing this from memory, so not 100% sure if this still works):

SUBSTRATE_RUNTIME_TARGET=riscv SUBSTRATE_ENABLE_POLKAVM=1 RUST_LOG=polkavm=debug cargo run --release -p staging-node-cli

Basically, setting SUBSTRATE_RUNTIME_TARGET=riscv will make the wasm-builder crate build a RISC-V runtime instead of a WASM runtime, and SUBSTRATE_ENABLE_POLKAVM=1 will allow the executor to load that blob (by default this code path is disabled). With this you should be able to use a normal node client to run a local test chain using it.

Thank you! At least it is in the right direction.

github-merge-queue bot pushed a commit that referenced this issue Dec 4, 2024
# Description

Closes #6335.

## Integration

N/A

## Review Notes

`RuntimeTarget` is converted to return path to the custom target JSON
file

---------

Signed-off-by: Jarkko Sakkinen <[email protected]>
Co-authored-by: Alexander Theißen <[email protected]>
Co-authored-by: Koute <[email protected]>
sylvaincormier pushed a commit to sylvaincormier/polkadot-sdk that referenced this issue Dec 4, 2024
…ytech#6419)

# Description

Closes paritytech#6335.

## Integration

N/A

## Review Notes

`RuntimeTarget` is converted to return path to the custom target JSON
file

---------

Signed-off-by: Jarkko Sakkinen <[email protected]>
Co-authored-by: Alexander Theißen <[email protected]>
Co-authored-by: Koute <[email protected]>
Krayt78 pushed a commit to Krayt78/polkadot-sdk that referenced this issue Dec 18, 2024
…ytech#6419)

# Description

Closes paritytech#6335.

## Integration

N/A

## Review Notes

`RuntimeTarget` is converted to return path to the custom target JSON
file

---------

Signed-off-by: Jarkko Sakkinen <[email protected]>
Co-authored-by: Alexander Theißen <[email protected]>
Co-authored-by: Koute <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I10-unconfirmed Issue might be valid, but it's not yet known.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants