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

Always use wasm executor in container collators #412

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

tmpolaczyk
Copy link
Contributor

@tmpolaczyk tmpolaczyk commented Feb 14, 2024

We use a NativeElseWasmExecutor for all chains, but collators don't have the container chain's native runtime. So their executor was trying to use the native dancebox runtime as the container chain runtime, which fortunately resulted in a version mismatch and the wasm container chain runtime was always used.

This can be verified by starting zombienet with export RUST_LOG=info,executor=trace:

2024-02-13 18:33:48.286 TRACE tokio-runtime-worker executor: [Container-2000] Request for native execution failed native=dancebox-500 (dancebox-0.tx1.au1) chain=container-chain-template-500 (container-chain-template-0.tx1.au1)

As a bonus we now have two types of clients, Arc<ContainerChainClient> and Arc<ParachainClient>, and using one client in place of the other results in a compile error.

Copy link
Contributor

github-actions bot commented Feb 14, 2024

Coverage Report

(master)

@@                           Coverage Diff                           @@
##           master   tomasz-container-coll-wasm-executor      +/-   ##
=======================================================================
- Coverage   76.66%                                76.58%   -0.08%     
  Files         107                                   107              
+ Lines       26803                                 26836      +33     
=======================================================================
+ Hits        20547                                 20551       +4     
+ Misses       6256                                  6285      +29     
Files Changed Coverage
/client/node-common/src/service.rs 48.72% (+0.09%) 🔼
/node/src/service.rs 22.66% (-1.07%) 🔽

Coverage generated Wed Feb 14 15:44:24 UTC 2024

@girazoki girazoki added B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes breaking Needs to be mentioned in breaking changes labels Feb 15, 2024
@tmpolaczyk tmpolaczyk merged commit d28aca5 into master Feb 16, 2024
31 of 33 checks passed
@tmpolaczyk tmpolaczyk deleted the tomasz-container-coll-wasm-executor branch February 16, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes breaking Needs to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants