-
Notifications
You must be signed in to change notification settings - Fork 684
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
PVF worker: separate worker binaries and build with musl #650
Comments
Despite my comment at #652, it seems that something like wasm-builder is the only way to embed executables. I had this hack which seemed to work, until I did const PREPARE_EXE: &'static [u8] =
include_bytes!(concat!(env!("OUT_DIR"), "/../../../prepare_worker"));
const EXECUTE_EXE: &'static [u8] =
include_bytes!(concat!(env!("OUT_DIR"), "/../../../execute_worker")); And |
The worker binaries must be built first so that the host could embed them into `polkadot`. Therefore `pvf/worker` could not depend on `pvf`, so to remove the dependency, common functionality was extracted into `pvf/common`. (NOTE: We already needed to do this host/worker/common separation as part of https://github.com/paritytech/polkadot/issues/7116, it's just unfortunate that it had to be done here and complicate this PR.) Integration tests were moved from `pvf/worker/tests` to `pvf/tests` because they need the PVF host.
This issue has been mentioned on Polkadot Forum. There might be relevant details there: https://forum.polkadot.network/t/ux-of-distributing-multiple-binaries-take-2/2854/1 |
With #2009 merged, I believe this is the only task stopping us from enabling the full seccomp rules (only logging violations for now). And, with #1663 we have everything we need to build with musl right in the repo. What's left:
|
If possible we should recommend a specific, set version of the musl toolchain for better determinism. |
Anouncement / Release ScheduleThe musl requirement should be announced at some point. It's not a requirement for on-demand, so a bit lower priority. I'm thinking we give secure-mode some time to settle in before adding the musl requirement, as it will affect all validators. So if we release secure-mode in 1.5 then maybe we can announce the musl requirement in 1.7, adding a runtime warning. Then enforce it in 1.9 or something like this. Reason being, it might make sense to give more time than for secure-mode, because secure-mode will affect hardly anyone. On the other hand, the musl requirement simply involves doing a different build instruction, and not migrating to a different machine or upgrading the kernel, so while it affects everyone the impact is low. |
* Some tweaks for the demo * A few more demo tweaks * Fix execution block hash, now prints out in hex * Investigating block verification failing * Fix hex to bytes function and adds a test for it. * Removes debugging logs. * Cleans up formatting. * More cleanup and refactor. * Import the second last finalized header too. * Adds more logging for the empty sync aggregate. Co-authored-by: claravanstaden <Cats 4 life!>
…aritytech#650) * Do not enter estimate mode when binary flag is enabled * cfg one liner
ISSUE
Overview
Per the security/sandboxing work in #882, we want to separate the PVF worker binaries and build them with musl. There are several advantages in doing this:
With a separate binary we can always apply LTO optimizations to this binary alone. LTO may make compile times prohibitively long for all ofpolkadot
, but the impact on a small binary is much smaller and there may be performance wins. (Related: Optimize validator binaries with LTO polkadot#4311.)Todo
polkadot
executable at build-time (similar to this)The text was updated successfully, but these errors were encountered: