Companion for PR 8913 #3114
Companion for PR 8913 #3114
Conversation
pepyakin
added
A0-please_review
| // SAFETY: this should be safe since the compiled artifact passed here comes from the | ||
| // file created by the prepare workers. These files are obtained by calling | ||
| // [`executor_intf::prepare`]. | ||
| crate::executor_intf::execute(compiled_artifact, params, spawner.clone()) |
There was a problem hiding this comment.
I am not sure if this is enough TBH. The data that comes here is from a file. The file can in theory be compromised or corrupted (e.g. due to hardware failure).
However, there is also little sense in extending the unsafe blocks further. It's awkward and will probably lead marking all the worker_common machinery. Yet, it won't provide any additional benefit - we ultimately will walk into the cross process boundary. Ultimately, it's up to validation host to decide what file will be passed here.
I think we do not take into account the compromisation concern. We mostly assume that if the node is compromised the attacker can do way worse things anyway. The corruption is a bit more severe. However, I am not sure if it's likely: after all, to my knowledge, ELFs do not have checksums.
There was a problem hiding this comment.
If you want to be 100% sure, you could hash them. However, I'm not sure that is really required.
| # | ||
| # Another safeguard is a test `ensure_wasmtime_version` that will fail on each bump and prompt the | ||
| # developer to correspondingly act upon the change. | ||
| wasmtime-jit = "0.24" |
There was a problem hiding this comment.
This is now removed along with the test before because as per #3084 we no longer leave artifacts in the cache and remove them each start. That means we do not need to foresee a possibility of an old artifact getting into the new engine.
polkadot companion: paritytech/substrate#8913
|
Waiting for commit status. |
|
Merge aborted: Checks failed for 3fcf7bf |
polkadot companion: paritytech/substrate#8913